Re: [PATCH net-next v1 4/4] selftests: net: add leased devmem RX and TX netdev unregister tests

[Dragos Tatulea]

On 04.11.26 00:26, Mina Almasry wrote:
> Add devmem_lease_bind_rx_unregister_check and
> devmem_lease_bind_tx_unregister_check to nl_netdev.py to verify that
> leased RX and TX devmem bindings on a netkit interface backed by a
> netdevsim queue detach their dma-buf cleanly while the netlink socket
> remains open, both when the virtual device (bind_dev) is unregistered
> before the physical device (map_dev) and when the physical device is
> unregistered directly (which uninstalls the physical queue memory
> provider and cascades unregistration to the leasing netkit).
> 
> Signed-off-by: Mina Almasry <almasrymina@xxxxxxxxxx>
> ---
>  tools/testing/selftests/net/nl_netdev.py | 132 ++++++++++++++++++++++-
>  1 file changed, 129 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/nl_netdev.py b/tools/testing/selftests/net/nl_netdev.py
> index dd0321c0b463a..fa4b5be093257 100755
> --- a/tools/testing/selftests/net/nl_netdev.py
> +++ b/tools/testing/selftests/net/nl_netdev.py
> @@ -12,8 +12,9 @@ import os
>  import struct
>  from os import system
>  from lib.py import ksft_run, ksft_exit, KsftFailEx, KsftSkipEx
> -from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_raises, ksft_busy_wait
> -from lib.py import EthtoolFamily, NetdevFamily, NetdevSimDev, NlError, defer, ip
> +from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_not_in, ksft_raises, ksft_busy_wait
> +from lib.py import EthtoolFamily, NetdevFamily, NetdevSimDev, Netlink
> +from lib.py import NlError, RtnlFamily, cmd, defer, ip
>  
>  
>  def empty_check(nf) -> None:
> @@ -483,6 +484,129 @@ def devmem_bind_tx_unregister_check(_nf) -> None:
>      nf_priv.close()
>  
>  
> +def _create_netkit_single(rxqueues=2):
> +    """Create a single-device netkit interface for queue lease tests."""
> +    ifname = f"nk{os.getpid()}"
> +    try:
> +        with RtnlFamily() as rtnl:
> +            rtnl.newlink(
> +                {
> +                    "ifname": ifname,
> +                    "linkinfo": {
> +                        "kind": "netkit",
> +                        "data": {
> +                            "mode": "l2",
> +                            "pairing": "single",
> +                        },
> +                    },
> +                    "num-rx-queues": rxqueues,
> +                },
> +                flags=[Netlink.NLM_F_CREATE, Netlink.NLM_F_EXCL],
> +            )
> +    except NlError as e:
> +        if e.error in (errno.EOPNOTSUPP, errno.ENOENT):
> +            raise KsftSkipEx(f"netkit single mode not supported: {e}") from e
> +        raise
> +    defer(cmd, f"ip link del dev {ifname}", fail=False)
> +
> +    link = ip(f"-d link show dev {ifname}", json=True)[0]
> +    return ifname, link["ifindex"]
> +
> +
> +def devmem_lease_bind_rx_unregister_check(nf) -> None:
> +    """Verify leased RX devmem bindings detach cleanly on virt/phys unregister."""
> +    for remove_virt_first in (False, True):
> +        dmabuf_fd = _create_udmabuf()
> +        defer(os.close, dmabuf_fd)
> +        nf_priv = NetdevFamily()
> +        defer(nf_priv.close)
> +        nsimdev = NetdevSimDev(queue_count=2)
> +        defer(nsimdev.remove)
> +        nsim = nsimdev.nsims[0]
> +
> +        ip(f"link set dev {nsim.ifname} up")
> +        with EthtoolFamily() as ef:
> +            ef.rings_set({"header": {"dev-index": nsim.ifindex},
> +                          "tcp-data-split": "enabled"})
> +
> +        nk_ifname, nk_ifindex = _create_netkit_single(rxqueues=2)
> +        ip(f"link set dev {nk_ifname} up")
> +
> +        res = nf.queue_create({
> +            "ifindex": nk_ifindex,
> +            "type": "rx",
> +            "lease": {
> +                "ifindex": nsim.ifindex,
> +                "queue": {"id": 1, "type": "rx"},
> +            },
> +        })
> +
> +        _devmem_bind(nf_priv.bind_rx, {
> +            "ifindex": nk_ifindex,
> +            "fd": dmabuf_fd,
> +            "queues": [{"id": res["id"], "type": "rx"}],
> +        })
> +        ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
> +
> +        if remove_virt_first:
> +            ip(f"link del dev {nk_ifname}")
> +            phys_q = nf.queue_get({"ifindex": nsim.ifindex, "id": 1,
> +                                   "type": "rx"})
> +            ksft_not_in("dmabuf", phys_q)
> +            ksft_not_in("lease", phys_q)
> +            ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
> +            nsimdev.remove()
> +        else:
> +            nsimdev.remove()
> +            ksft_ne(cmd(f"ip link show dev {nk_ifname}", fail=False).ret, 0)
> +        ksft_eq(_dmabuf_attach_count(dmabuf_fd), 0)
> +
> +        nf_priv.close()
Any value in leaving this here? defer() should handle it.

Other than that:
Reviewed-by: Dragos Tatulea <dtatulea@xxxxxxxxxx>

Thanks,
Dragos