From: Eugenio Pérez <eperezma@redhat.com>

Commit a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net
structures if peer nic is present") effectively delayed the backend
cleanup, allowing the frontend or the guest to access it resources as
long as the frontend is still visible to the guest.

However it does not clean up the resources until the qemu process is
over.  This causes an effective leak if the device is deleted with
device_del, as there is no way to close the vdpa device.  This makes
impossible to re-add that device to this or other QEMU instances until
the first instance of QEMU is finished.

Move the cleanup from qemu_cleanup to the NIC deletion and to
net_cleanup.

Fixes: a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net structures if peer nic is present")
Reported-by: Lei Yang <leiyang@redhat.com>
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/net.c        | 33 +++++++++++++++++++++++++++------
 net/vhost-vdpa.c |  8 --------
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/net/net.c b/net/net.c
index 4eb78a1299..39d6f28158 100644
--- a/net/net.c
+++ b/net/net.c
@@ -428,7 +428,13 @@ void qemu_del_net_client(NetClientState *nc)
         object_unparent(OBJECT(nf));
     }
 
-    /* If there is a peer NIC, delete and cleanup client, but do not free. */
+    /*
+     * If there is a peer NIC, transfer ownership to it.  Delete the client
+     * from net_client list but do not cleanup nor free.  This way NIC can
+     * still access to members of the backend.
+     *
+     * The cleanup and free will be done when the NIC is free.
+     */
     if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_NIC) {
         NICState *nic = qemu_get_nic(nc->peer);
         if (nic->peer_deleted) {
@@ -438,16 +444,13 @@ void qemu_del_net_client(NetClientState *nc)
 
         for (i = 0; i < queues; i++) {
             ncs[i]->peer->link_down = true;
+            QTAILQ_REMOVE(&net_clients, ncs[i], next);
         }
 
         if (nc->peer->info->link_status_changed) {
             nc->peer->info->link_status_changed(nc->peer);
         }
 
-        for (i = 0; i < queues; i++) {
-            qemu_cleanup_net_client(ncs[i], true);
-        }
-
         return;
     }
 
@@ -465,8 +468,12 @@ void qemu_del_nic(NICState *nic)
 
     for (i = 0; i < queues; i++) {
         NetClientState *nc = qemu_get_subqueue(nic, i);
-        /* If this is a peer NIC and peer has already been deleted, free it now. */
+        /*
+         * If this is a peer NIC and peer has already been deleted, clean it up
+         * and free it now.
+         */
         if (nic->peer_deleted) {
+            qemu_cleanup_net_client(nc->peer, false);
             qemu_free_net_client(nc->peer);
         } else if (nc->peer) {
             /* if there are RX packets pending, complete them */
@@ -1681,6 +1688,9 @@ void net_cleanup(void)
      * of the latest NET_CLIENT_DRIVER_NIC, and operate on *p as we walk
      * the list.
      *
+     * However, the NIC may have peers that trust to be clean beyond this
+     * point.  For example, if they have been removed with device_del.
+     *
      * The 'nc' variable isn't part of the list traversal; it's purely
      * for convenience as too much '(*p)->' has a tendency to make the
      * readers' eyes bleed.
@@ -1688,6 +1698,17 @@ void net_cleanup(void)
     while (*p) {
         nc = *p;
         if (nc->info->type == NET_CLIENT_DRIVER_NIC) {
+            NICState *nic = qemu_get_nic(nc);
+
+            if (nic->peer_deleted) {
+                int queues = MAX(nic->conf->peers.queues, 1);
+
+                for (int i = 0; i < queues; i++) {
+                    nc = qemu_get_subqueue(nic, i);
+                    qemu_cleanup_net_client(nc->peer, false);
+                }
+            }
+
             /* Skip NET_CLIENT_DRIVER_NIC entries */
             p = &QTAILQ_NEXT(nc, next);
         } else {
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index bd01866878..f7a54f46aa 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -224,14 +224,6 @@ static void vhost_vdpa_cleanup(NetClientState *nc)
 {
     VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
 
-    /*
-     * If a peer NIC is attached, do not cleanup anything.
-     * Cleanup will happen as a part of qemu_cleanup() -> net_cleanup()
-     * when the guest is shutting down.
-     */
-    if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_NIC) {
-        return;
-    }
     munmap(s->cvq_cmd_out_buffer, vhost_vdpa_net_cvq_cmd_page_len());
     munmap(s->status, vhost_vdpa_net_cvq_cmd_page_len());
     if (s->vhost_net) {
-- 
2.42.0

On Mon, 2025-03-10 at 20:22 +0800, Jason Wang wrote:
> From: Eugenio Pérez <eperezma@redhat.com>
> 
> Commit a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net
> structures if peer nic is present") effectively delayed the backend
> cleanup, allowing the frontend or the guest to access it resources as
> long as the frontend is still visible to the guest.
> 
> However it does not clean up the resources until the qemu process is
> over.  This causes an effective leak if the device is deleted with
> device_del, as there is no way to close the vdpa device.  This makes
> impossible to re-add that device to this or other QEMU instances until
> the first instance of QEMU is finished.
> 
> Move the cleanup from qemu_cleanup to the NIC deletion and to
> net_cleanup.
> 
> Fixes: a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net structures if peer nic is present")
> Reported-by: Lei Yang <leiyang@redhat.com>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

This crashes QEMU when I launch an emulated Xen guest with a Xen PV
NIC, and quit (using Ctrl-A x on the monitor).

$ gdb --args  ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel-irqchip=split -serial mon:stdio     -display none  -m 1G  -kernel /boot/vmlinuz-6.13.8-200.fc41.x86_64/boot/vmlinuz-6.13.8-200.fc41.x86_64 -append "console=ttyS0" 
(gdb) handle SIGUSR1 pass nostop noprint
(gdb) run
[    0.000000] Linux version 6.13.8-200.fc41.x86_64 (mockbuild@cdcecfee8b71420eb7f55566b7811804) (gcc (GCC) 14.2.1 20250110 (Red Hat 14.2.1-7), GNU ld version 2.43.1-5.fc41) #1 SMP PREEMPT_DYNAMIC Sun Mar 23 05:03:09 UTC 2025[    0.000000] Linux version 6.13.8-200.fc41.x86_64 (mockbuild@cdcecfee8b71420eb7f55566b7811804) (gcc (GCC) 14.2.1 20250110 (Red Hat 14.2.1-7), GNU ld version 2.43.1-5.fc41) #1 SMP PREEMPT_DYNAMIC Sun Mar 23 05:03:09 UTC 2025
[    0.000000] Command line: console=ttyS0
[    0.000000] BIOS-provided physical RAM map:
[    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
[    0.000000] BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
[    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000003ffdffff] usable
[    0.000000] BIOS-e820: [mem 0x000000003ffe0000-0x000000003fffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000feff8000-0x00000000feffffff] reserved
[    0.000000] BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
[    0.000000] NX (Execute Disable) protection: active
[    0.000000] APIC: Static calls initialized
[    0.000000] SMBIOS 2.8 present.
[    0.000000] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
[    0.000000] DMI: Memory slots populated: 1/1
[    0.000000] Hypervisor detected: Xen HVM
…
<Ctrl-A x>
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x000055555584821c in net_hub_port_cleanup (nc=0x555557ce23d0) at ../net/hub.c:132
132	    QLIST_REMOVE(port, next);
(gdb) bt
#0  0x000055555584821c in net_hub_port_cleanup (nc=0x555557ce23d0) at ../net/hub.c:132
#1  0x000055555584a8c9 in qemu_cleanup_net_client (nc=<optimized out>, remove_from_net_clients=false)
    at ../net/net.c:392
#2  qemu_del_nic (nic=0x555558866400) at ../net/net.c:476
#3  0x00005555557cae41 in xen_device_unrealize (dev=<optimized out>) at ../hw/xen/xen-bus.c:988
#4  0x0000555555c414ff in notifier_list_notify
    (list=list@entry=0x5555570671f0 <exit_notifiers>, data=data@entry=0x0) at ../util/notify.c:39
#5  0x00005555557efd2c in qemu_run_exit_notifiers () at ../system/runstate.c:854
#6  0x00007ffff52996c1 in __run_exit_handlers () at /lib64/libc.so.6
#7  0x00007ffff529978e in exit () at /lib64/libc.so.6
#8  0x0000555555b90a5c in qemu_default_main (opaque=opaque@entry=0x0) at ../system/main.c:52
#9  0x00005555555622d0 in main (argc=<optimized out>, argv=<optimized out>) at ../system/main.c:76

On Wed, Aug 20, 2025 at 12:13 AM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Mon, 2025-03-10 at 20:22 +0800, Jason Wang wrote:
> > From: Eugenio Pérez <eperezma@redhat.com>
> >
> > Commit a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net
> > structures if peer nic is present") effectively delayed the backend
> > cleanup, allowing the frontend or the guest to access it resources as
> > long as the frontend is still visible to the guest.
> >
> > However it does not clean up the resources until the qemu process is
> > over.  This causes an effective leak if the device is deleted with
> > device_del, as there is no way to close the vdpa device.  This makes
> > impossible to re-add that device to this or other QEMU instances until
> > the first instance of QEMU is finished.
> >
> > Move the cleanup from qemu_cleanup to the NIC deletion and to
> > net_cleanup.
> >
> > Fixes: a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net structures if peer nic is present")
> > Reported-by: Lei Yang <leiyang@redhat.com>
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
>
> This crashes QEMU when I launch an emulated Xen guest with a Xen PV
> NIC, and quit (using Ctrl-A x on the monitor).

Eugenio and Jonah, any thoughts on this? It looks like the code
doesn't deal with hub correctly.

Thanks

>
> $ gdb --args  ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel-irqchip=split -serial mon:stdio     -display none  -m 1G  -kernel /boot/vmlinuz-6.13.8-200.fc41.x86_64/boot/vmlinuz-6.13.8-200.fc41.x86_64 -append "console=ttyS0"
> (gdb) handle SIGUSR1 pass nostop noprint
> (gdb) run
> [    0.000000] Linux version 6.13.8-200.fc41.x86_64 (mockbuild@cdcecfee8b71420eb7f55566b7811804) (gcc (GCC) 14.2.1 20250110 (Red Hat 14.2.1-7), GNU ld version 2.43.1-5.fc41) #1 SMP PREEMPT_DYNAMIC Sun Mar 23 05:03:09 UTC 2025[    0.000000] Linux version 6.13.8-200.fc41.x86_64 (mockbuild@cdcecfee8b71420eb7f55566b7811804) (gcc (GCC) 14.2.1 20250110 (Red Hat 14.2.1-7), GNU ld version 2.43.1-5.fc41) #1 SMP PREEMPT_DYNAMIC Sun Mar 23 05:03:09 UTC 2025
> [    0.000000] Command line: console=ttyS0
> [    0.000000] BIOS-provided physical RAM map:
> [    0.000000] BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] usable
> [    0.000000] BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] reserved
> [    0.000000] BIOS-e820: [mem 0x0000000000100000-0x000000003ffdffff] usable
> [    0.000000] BIOS-e820: [mem 0x000000003ffe0000-0x000000003fffffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000feff8000-0x00000000feffffff] reserved
> [    0.000000] BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] reserved
> [    0.000000] NX (Execute Disable) protection: active
> [    0.000000] APIC: Static calls initialized
> [    0.000000] SMBIOS 2.8 present.
> [    0.000000] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> [    0.000000] DMI: Memory slots populated: 1/1
> [    0.000000] Hypervisor detected: Xen HVM
> …
> <Ctrl-A x>
> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> 0x000055555584821c in net_hub_port_cleanup (nc=0x555557ce23d0) at ../net/hub.c:132
> 132         QLIST_REMOVE(port, next);
> (gdb) bt
> #0  0x000055555584821c in net_hub_port_cleanup (nc=0x555557ce23d0) at ../net/hub.c:132
> #1  0x000055555584a8c9 in qemu_cleanup_net_client (nc=<optimized out>, remove_from_net_clients=false)
>     at ../net/net.c:392
> #2  qemu_del_nic (nic=0x555558866400) at ../net/net.c:476
> #3  0x00005555557cae41 in xen_device_unrealize (dev=<optimized out>) at ../hw/xen/xen-bus.c:988
> #4  0x0000555555c414ff in notifier_list_notify
>     (list=list@entry=0x5555570671f0 <exit_notifiers>, data=data@entry=0x0) at ../util/notify.c:39
> #5  0x00005555557efd2c in qemu_run_exit_notifiers () at ../system/runstate.c:854
> #6  0x00007ffff52996c1 in __run_exit_handlers () at /lib64/libc.so.6
> #7  0x00007ffff529978e in exit () at /lib64/libc.so.6
> #8  0x0000555555b90a5c in qemu_default_main (opaque=opaque@entry=0x0) at ../system/main.c:52
> #9  0x00005555555622d0 in main (argc=<optimized out>, argv=<optimized out>) at ../system/main.c:76
>

On Wed, 2025-08-20 at 10:34 +0800, Jason Wang wrote:
> On Wed, Aug 20, 2025 at 12:13 AM David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > On Mon, 2025-03-10 at 20:22 +0800, Jason Wang wrote:
> > > From: Eugenio Pérez <eperezma@redhat.com>
> > > 
> > > Commit a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net
> > > structures if peer nic is present") effectively delayed the backend
> > > cleanup, allowing the frontend or the guest to access it resources as
> > > long as the frontend is still visible to the guest.
> > > 
> > > However it does not clean up the resources until the qemu process is
> > > over.  This causes an effective leak if the device is deleted with
> > > device_del, as there is no way to close the vdpa device.  This makes
> > > impossible to re-add that device to this or other QEMU instances until
> > > the first instance of QEMU is finished.
> > > 
> > > Move the cleanup from qemu_cleanup to the NIC deletion and to
> > > net_cleanup.
> > > 
> > > Fixes: a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net structures if peer nic is present")
> > > Reported-by: Lei Yang <leiyang@redhat.com>
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > 
> > This crashes QEMU when I launch an emulated Xen guest with a Xen PV
> > NIC, and quit (using Ctrl-A x on the monitor).
> 
> Eugenio and Jonah, any thoughts on this? It looks like the code
> doesn't deal with hub correctly.

The interesting part about Xen netback is that it does its own teardown
from xen_device_unrealize() because in the case of running under true
Xen, it needs to clean up XenStore nodes which are externally visible.
It doesn't all just go away when QEMU exits.

We fixed a potentially similar issue in commit 84f85eb95f14a ("net: do
not delete nics in net_cleanup()")?

I should add a test case for this...

On Wed, Aug 20, 2025 at 8:47 AM David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Wed, 2025-08-20 at 10:34 +0800, Jason Wang wrote:
> > On Wed, Aug 20, 2025 at 12:13 AM David Woodhouse <dwmw2@infradead.org> wrote:
> > >
> > > On Mon, 2025-03-10 at 20:22 +0800, Jason Wang wrote:
> > > > From: Eugenio Pérez <eperezma@redhat.com>
> > > >
> > > > Commit a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net
> > > > structures if peer nic is present") effectively delayed the backend
> > > > cleanup, allowing the frontend or the guest to access it resources as
> > > > long as the frontend is still visible to the guest.
> > > >
> > > > However it does not clean up the resources until the qemu process is
> > > > over.  This causes an effective leak if the device is deleted with
> > > > device_del, as there is no way to close the vdpa device.  This makes
> > > > impossible to re-add that device to this or other QEMU instances until
> > > > the first instance of QEMU is finished.
> > > >
> > > > Move the cleanup from qemu_cleanup to the NIC deletion and to
> > > > net_cleanup.
> > > >
> > > > Fixes: a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net structures if peer nic is present")
> > > > Reported-by: Lei Yang <leiyang@redhat.com>
> > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> > > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> > >
> > > This crashes QEMU when I launch an emulated Xen guest with a Xen PV
> > > NIC, and quit (using Ctrl-A x on the monitor).
> >
> > Eugenio and Jonah, any thoughts on this? It looks like the code
> > doesn't deal with hub correctly.
>
> The interesting part about Xen netback is that it does its own teardown
> from xen_device_unrealize() because in the case of running under true
> Xen, it needs to clean up XenStore nodes which are externally visible.
> It doesn't all just go away when QEMU exits.
>
> We fixed a potentially similar issue in commit 84f85eb95f14a ("net: do
> not delete nics in net_cleanup()")?
>

I was not aware of that code to be honest :(.

Jonah, could you take a look at this? It should be a matter of
replicating what the series does in the list on NICs, in this linked
list.

Thanks!

On 8/20/25 3:14 AM, Eugenio Perez Martin wrote:
> On Wed, Aug 20, 2025 at 8:47 AM David Woodhouse <dwmw2@infradead.org> wrote:
>>
>> On Wed, 2025-08-20 at 10:34 +0800, Jason Wang wrote:
>>> On Wed, Aug 20, 2025 at 12:13 AM David Woodhouse <dwmw2@infradead.org> wrote:
>>>>
>>>> On Mon, 2025-03-10 at 20:22 +0800, Jason Wang wrote:
>>>>> From: Eugenio Pérez <eperezma@redhat.com>
>>>>>
>>>>> Commit a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net
>>>>> structures if peer nic is present") effectively delayed the backend
>>>>> cleanup, allowing the frontend or the guest to access it resources as
>>>>> long as the frontend is still visible to the guest.
>>>>>
>>>>> However it does not clean up the resources until the qemu process is
>>>>> over.  This causes an effective leak if the device is deleted with
>>>>> device_del, as there is no way to close the vdpa device.  This makes
>>>>> impossible to re-add that device to this or other QEMU instances until
>>>>> the first instance of QEMU is finished.
>>>>>
>>>>> Move the cleanup from qemu_cleanup to the NIC deletion and to
>>>>> net_cleanup.
>>>>>
>>>>> Fixes: a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net structures if peer nic is present")
>>>>> Reported-by: Lei Yang <leiyang@redhat.com>
>>>>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>>>>> Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
>>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>>
>>>> This crashes QEMU when I launch an emulated Xen guest with a Xen PV
>>>> NIC, and quit (using Ctrl-A x on the monitor).
>>>
>>> Eugenio and Jonah, any thoughts on this? It looks like the code
>>> doesn't deal with hub correctly.
>>
>> The interesting part about Xen netback is that it does its own teardown
>> from xen_device_unrealize() because in the case of running under true
>> Xen, it needs to clean up XenStore nodes which are externally visible.
>> It doesn't all just go away when QEMU exits.
>>
>> We fixed a potentially similar issue in commit 84f85eb95f14a ("net: do
>> not delete nics in net_cleanup()")?
>>
> 
> I was not aware of that code to be honest :(.
> 
> Jonah, could you take a look at this? It should be a matter of
> replicating what the series does in the list on NICs, in this linked
> list.
> 
> Thanks!
> 

Sure, I'll see what I can do here.

On Wed, 2025-08-20 at 08:22 -0400, Jonah Palmer wrote:
> > > > > > From: Eugenio Pérez <eperezma@redhat.com>
> > > > > > 
> > > > > > Commit a0d7215e33 ("vhost-vdpa: do not cleanup the vdpa/vhost-net
> > > > > > structures if peer nic is present") effectively delayed the backend
> > > > > > cleanup, allowing the frontend or the guest to access it resources as
> > > > > > long as the frontend is still visible to the guest.
> > > > > > 
> > > > > > However it does not clean up the resources until the qemu process is
> > > > > > over.  This causes an effective leak if the device is deleted with
> > > > > > device_del, as there is no way to close the vdpa device.  This makes
> > > > > > impossible to re-add that device to this or other QEMU instances until
> > > > > > the first instance of QEMU is finished.
> > > > > > 
> > > > > > Move the cleanup from qemu_cleanup to the NIC deletion and to
> > > > > > net_cleanup.
> > > > > > 
> > > > > > Fixes: a0d7215e33 ("vhost-vdpa: do not cleanup the
> > > > > > vdpa/vhost-net structures if peer nic is present")
> > > > > > Reported-by: Lei Yang <leiyang@redhat.com>
> > > > > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > > > > Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com>
> > > > > > Signed-off-by: Jason Wang <jasowang@redhat.com>

Is it just that VDPA devices should have an unrealize() method to
properly tear themselves down? The problem appears to happen to the Xen
netdev when it gets torn down twice, precisely because it *was* doing
the right thing for itself?