mbox series

[net-next,v11,0/3] netdevsim: link and forward skbs between ports

Message ID 20240215194325.1364466-1-dw@davidwei.uk (mailing list archive)
Headers show
Series netdevsim: link and forward skbs between ports | expand

Message

David Wei Feb. 15, 2024, 7:43 p.m. UTC 
This patchset adds the ability to link two netdevsim ports together and
forward skbs between them, similar to veth. The goal is to use netdevsim
for testing features e.g. zero copy Rx using io_uring.

This feature was tested locally on QEMU, and a selftest is included.

---
v10->v11:
- add udevadm settle after creating netdevsims in selftest

v9->v10:
- fix not freeing skb when not there is no peer
- prevent possible id clashes in selftest
- cleanup selftest on error paths

v8->v9:
- switch to getting netns using fd rather than id
- prevent linking a netdevsim to itself
- update tests

v7->v8:
- fix not dereferencing RCU ptr using rcu_dereference()
- remove unused variables in selftest

v6->v7:
- change link syntax to netnsid:ifidx
- replace dev_get_by_index() with __dev_get_by_index()
- check for NULL peer when linking
- add a sysfs attribute for unlinking
- only update Tx stats if not dropped
- update selftest

v5->v6:
- reworked to link two netdevsims using sysfs attribute on the bus
  device instead of debugfs due to deadlock possibility if a netdevsim
  is removed during linking
- removed unnecessary patch maintaining a list of probed nsim_devs
- updated selftest

v4->v5:
- reduce nsim_dev_list_lock critical section
- fixed missing mutex unlock during unwind ladder
- rework nsim_dev_peer_write synchronization to take devlink lock as
  well as rtnl_lock
- return err msgs to user during linking if port doesn't exist or
  linking to self
- update tx stats outside of RCU lock

v3->v4:
- maintain a mutex protected list of probed nsim_devs instead of using
  nsim_bus_dev
- fixed synchronization issues by taking rtnl_lock
- track tx_dropped skbs

v2->v3:
- take lock when traversing nsim_bus_dev_list
- take device ref when getting a nsim_bus_dev
- return 0 if nsim_dev_peer_read cannot find the port
- address code formatting
- do not hard code values in selftests
- add Makefile for selftests

v1->v2:
- renamed debugfs file from "link" to "peer"
- replaced strstep() with sscanf() for consistency
- increased char[] buf sz to 22 for copying id + port from user
- added err msg w/ expected fmt when linking as a hint to user
- prevent linking port to itself
- protect peer ptr using RCU

David Wei (3):
  netdevsim: allow two netdevsim ports to be connected
  netdevsim: forward skbs from one connected port to another
  netdevsim: add selftest for forwarding skb between connected ports

 drivers/net/netdevsim/bus.c                   | 135 +++++++++++++++++
 drivers/net/netdevsim/netdev.c                |  40 ++++-
 drivers/net/netdevsim/netdevsim.h             |   3 +
 .../selftests/drivers/net/netdevsim/Makefile  |   1 +
 .../selftests/drivers/net/netdevsim/peer.sh   | 139 ++++++++++++++++++
 5 files changed, 313 insertions(+), 5 deletions(-)
 create mode 100755 tools/testing/selftests/drivers/net/netdevsim/peer.sh

Comments

Paolo Abeni Feb. 16, 2024, 10:38 a.m. UTC | #1 
Hi,

On Thu, 2024-02-15 at 11:43 -0800, David Wei wrote:
> This patchset adds the ability to link two netdevsim ports together and
> forward skbs between them, similar to veth. The goal is to use netdevsim
> for testing features e.g. zero copy Rx using io_uring.
> 
> This feature was tested locally on QEMU, and a selftest is included.

this apparently causes rtnetlink.sh self-tests failures:

https://netdev.bots.linux.dev/flakes.html?tn-needle=rtnetlink-sh

example failure:

https://netdev-3.bots.linux.dev/vmksft-net/results/467721/18-rtnetlink-sh/stdout

the ipsec_offload test (using netdevsim) fails.

@Jakub: it looks like the rtnetlink.sh test is currently ignored by
patchwork, skimming over the recent failures they are roughly
correlated to this series submission: the test looks otherwise
reasonably stable to me.

Cheers,

Paolo
Jakub Kicinski Feb. 16, 2024, 2:44 p.m. UTC | #2 
On Fri, 16 Feb 2024 11:38:17 +0100 Paolo Abeni wrote:
> On Thu, 2024-02-15 at 11:43 -0800, David Wei wrote:
> > This patchset adds the ability to link two netdevsim ports together and
> > forward skbs between them, similar to veth. The goal is to use netdevsim
> > for testing features e.g. zero copy Rx using io_uring.
> > 
> > This feature was tested locally on QEMU, and a selftest is included.  
> 
> this apparently causes rtnetlink.sh self-tests failures:
> 
> https://netdev.bots.linux.dev/flakes.html?tn-needle=rtnetlink-sh
> 
> example failure:
> 
> https://netdev-3.bots.linux.dev/vmksft-net/results/467721/18-rtnetlink-sh/stdout
> 
> the ipsec_offload test (using netdevsim) fails.
> 
> @Jakub: it looks like the rtnetlink.sh test is currently ignored by
> patchwork, skimming over the recent failures they are roughly
> correlated to this series submission: the test looks otherwise
> reasonably stable to me.

Wow, great detective work! This is what the diff says:

#     1c1
# < SA count=2 tx=0
# ---
# > SA count=2 tx=3

I'm guessing that because we clear IFF_NOARP and ipsec code doesn't
connect any peer it can't transmit? Any suggestions how to fix it?
Insert a neigh entry manually?
David Wei Feb. 16, 2024, 7:13 p.m. UTC | #3 
On 2024-02-16 03:38, Paolo Abeni wrote:
> Hi,
> 
> On Thu, 2024-02-15 at 11:43 -0800, David Wei wrote:
>> This patchset adds the ability to link two netdevsim ports together and
>> forward skbs between them, similar to veth. The goal is to use netdevsim
>> for testing features e.g. zero copy Rx using io_uring.
>>
>> This feature was tested locally on QEMU, and a selftest is included.
> 
> this apparently causes rtnetlink.sh self-tests failures:
> 
> https://netdev.bots.linux.dev/flakes.html?tn-needle=rtnetlink-sh
> 
> example failure:
> 
> https://netdev-3.bots.linux.dev/vmksft-net/results/467721/18-rtnetlink-sh/stdout
> 
> the ipsec_offload test (using netdevsim) fails.

Thanks for catching this Paulo. Can I sort this out in a separate patch,
as the rtnetlink.sh test is disabled in CI right now?

> 
> @Jakub: it looks like the rtnetlink.sh test is currently ignored by
> patchwork, skimming over the recent failures they are roughly
> correlated to this series submission: the test looks otherwise
> reasonably stable to me.
> 
> Cheers,
> 
> Paolo
>
Jakub Kicinski Feb. 17, 2024, 1:47 a.m. UTC | #4 
On Fri, 16 Feb 2024 12:13:42 -0700 David Wei wrote:
> > this apparently causes rtnetlink.sh self-tests failures:
> > 
> > https://netdev.bots.linux.dev/flakes.html?tn-needle=rtnetlink-sh
> > 
> > example failure:
> > 
> > https://netdev-3.bots.linux.dev/vmksft-net/results/467721/18-rtnetlink-sh/stdout
> > 
> > the ipsec_offload test (using netdevsim) fails.  
> 
> Thanks for catching this Paulo. Can I sort this out in a separate patch,
> as the rtnetlink.sh test is disabled in CI right now?

Looks like the code needs fixing, still, so please tack the rtnetlink
fix onto the v12
Jakub Kicinski Feb. 17, 2024, 1:48 a.m. UTC | #5 
Also looks like the new test managed to flake once while it was sitting
in patchwork ?

https://netdev-3.bots.linux.dev/vmksft-netdevsim-dbg/results/468440/13-peer-sh/stdout
David Wei Feb. 17, 2024, 2:52 a.m. UTC | #6 
On 2024-02-16 18:47, Jakub Kicinski wrote:
> On Fri, 16 Feb 2024 12:13:42 -0700 David Wei wrote:
>>> this apparently causes rtnetlink.sh self-tests failures:
>>>
>>> https://netdev.bots.linux.dev/flakes.html?tn-needle=rtnetlink-sh
>>>
>>> example failure:
>>>
>>> https://netdev-3.bots.linux.dev/vmksft-net/results/467721/18-rtnetlink-sh/stdout
>>>
>>> the ipsec_offload test (using netdevsim) fails.  
>>
>> Thanks for catching this Paulo. Can I sort this out in a separate patch,
>> as the rtnetlink.sh test is disabled in CI right now?
> 
> Looks like the code needs fixing, still, so please tack the rtnetlink
> fix onto the v12

Okay I'll add it on!
David Wei Feb. 17, 2024, 3:27 a.m. UTC | #7 
On 2024-02-16 18:48, Jakub Kicinski wrote:
> Also looks like the new test managed to flake once while it was sitting
> in patchwork ?
> 
> https://netdev-3.bots.linux.dev/vmksft-netdevsim-dbg/results/468440/13-peer-sh/stdout

I can't repro it locally in QEMU. Maybe it's due to the leaky ref bug?

I'll send another revision and hopefully this doesn't happen again.
Jakub Kicinski Feb. 19, 2024, 7:20 p.m. UTC | #8 
On Fri, 16 Feb 2024 20:27:09 -0700 David Wei wrote:
> On 2024-02-16 18:48, Jakub Kicinski wrote:
> > Also looks like the new test managed to flake once while it was sitting
> > in patchwork ?
> > 
> > https://netdev-3.bots.linux.dev/vmksft-netdevsim-dbg/results/468440/13-peer-sh/stdout  
> 
> I can't repro it locally in QEMU. Maybe it's due to the leaky ref bug?
> 
> I'll send another revision and hopefully this doesn't happen again.

Grep for wait_local_port_listen, maybe we should make sure listener
had fully started?
David Wei Feb. 20, 2024, 3:42 a.m. UTC | #9 
On 2024-02-19 12:20, Jakub Kicinski wrote:
> On Fri, 16 Feb 2024 20:27:09 -0700 David Wei wrote:
>> On 2024-02-16 18:48, Jakub Kicinski wrote:
>>> Also looks like the new test managed to flake once while it was sitting
>>> in patchwork ?
>>>
>>> https://netdev-3.bots.linux.dev/vmksft-netdevsim-dbg/results/468440/13-peer-sh/stdout  
>>
>> I can't repro it locally in QEMU. Maybe it's due to the leaky ref bug?
>>
>> I'll send another revision and hopefully this doesn't happen again.
> 
> Grep for wait_local_port_listen, maybe we should make sure listener
> had fully started?

Ah, thanks. I'll add that into the selftest.