Re: [Intel-wired-lan] [PATCH iwl-net v5] ice: Do not get coalesce settings while in reset
On Fri, 7 Jun 2024 14:15:52 +0200 Dawid Osuchowski wrote: > We cannot use ice_wait_for_reset() since both the ethtool handler and the > adapter reset flow call rtnl_lock() during operation. If we wait for > reset completion inside of an ethtool handling function such as > ice_get_coalesce(), the wait will always timeout due to reset being > blocked by rtnl_lock() inside of ice_queue_set_napi() (which is called > during reset process), and in turn we will always return -EBUSY anyways, > with the added hang time of the timeout value. Why does the reset not call netif_device_detach()? Then core will know not to call the driver. > Fixes: 67fe64d78c43 ("ice: Implement getting and setting ethtool coalesce") Isn't ice_queue_set_napi() much more recent than this commit?
Re: [PATCH net-next] selftests: hsr: Extend the hsr_redbox.sh test to use fixed MAC addresses
On Mon, 3 Jun 2024 11:33:22 +0200 Lukasz Majewski wrote: > Fixed MAC addresses help with debugging as last four bytes identify the > network namespace. > > Moreover, it allows to mimic the real life setup with for example bridge > having the same MAC address on each port. Doesn't seem like a particularly scalable method of making tests more debugable but alright...
Re: [Intel-wired-lan] [PATCH iwl-next v1 0/5] ice: add standard stats
On Tue, 4 Jun 2024 15:13:20 -0700 Jesse Brandeburg wrote: > The main point of this series is to implement the standard stats for the > ice driver, but also add a related documentation fix and finish the > series off with a cleanup that removes a bunch of code. With the nit on patch 1 fixed: Reviewed-by: Jakub Kicinski queue stats next? :]
Re: [Intel-wired-lan] [PATCH iwl-next v1 1/5] net: docs: add missing features that can have stats
On Tue, 4 Jun 2024 15:13:21 -0700 Jesse Brandeburg wrote: > - - `ETHTOOL_MSG_PAUSE_GET` >- `ETHTOOL_MSG_FEC_GET` > + - 'ETHTOOL_MSG_LINKSTATE_GET' >- `ETHTOOL_MSG_MM_GET` > + - `ETHTOOL_MSG_PAUSE_GET` > + - 'ETHTOOL_MSG_TSINFO_GET' I was going to steal this directly but: ` vs ' so I'll let it go via the Intel tree :)
Re: [ovs-dev] [PATCH net-next 1/2] selftests: openvswitch: fix action formatting
On Mon, 03 Jun 2024 15:00:03 -0400 Aaron Conole wrote: > I agree, this is an issue. BUT I think it might be better to just > filter by field type up front. See: > > https://github.com/apconole/linux-next-work/commit/7262107de7170d44b6dbf6c5ea6f7e6c0bb71d36#diff-3e72e7405c6bb4e9842bed5f63883ca930387086bb40d4034e92ed83a5decb4bR441 > > That version I think ends up being much easier to follow. If you want > to take it for your series, feel free. If you disagree, maybe there's > something I'm not considering about it. > > NOTE that version is just a bunch of independent changes that are > squashed together. I have a cleaner version. > > I can also bundle up the series I have so far and submit, but I didn't > want to do that until I got all the pmtu.sh support working. Maybe it > makes sense to send it now though. Simon, Jakub - wdyt? I'd say - hold onto the changes until pmtu.sh works, unless there's *any* reason for a particular patch to go in early, eg: - patch fixes existing bug - someone else needs a patch - ... - a patch which falls under some of the criteria above depends on the patch ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [PATCH net-next 1/2] selftests: openvswitch: fix action formatting
On Mon, 03 Jun 2024 15:00:03 -0400 Aaron Conole wrote: > I agree, this is an issue. BUT I think it might be better to just > filter by field type up front. See: > > https://github.com/apconole/linux-next-work/commit/7262107de7170d44b6dbf6c5ea6f7e6c0bb71d36#diff-3e72e7405c6bb4e9842bed5f63883ca930387086bb40d4034e92ed83a5decb4bR441 > > That version I think ends up being much easier to follow. If you want > to take it for your series, feel free. If you disagree, maybe there's > something I'm not considering about it. > > NOTE that version is just a bunch of independent changes that are > squashed together. I have a cleaner version. > > I can also bundle up the series I have so far and submit, but I didn't > want to do that until I got all the pmtu.sh support working. Maybe it > makes sense to send it now though. Simon, Jakub - wdyt? I'd say - hold onto the changes until pmtu.sh works, unless there's *any* reason for a particular patch to go in early, eg: - patch fixes existing bug - someone else needs a patch - ... - a patch which falls under some of the criteria above depends on the patch
Re: [PATCH] net: mac802154: Fix racy device stats updates by DEV_STATS_INC() and DEV_STATS_ADD()
On Mon, 3 Jun 2024 11:33:28 +0200 Stefan Schmidt wrote: > Hello. > > On 31.05.24 10:07, Yunshui Jiang wrote: > > mac802154 devices update their dev->stats fields locklessly. Therefore > > these counters should be updated atomically. Adopt SMP safe DEV_STATS_INC() > > and DEV_STATS_ADD() to achieve this. > > > > Signed-off-by: Yunshui Jiang > > --- > > net/mac802154/tx.c | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c > > index 2a6f1ed763c9..6fbed5bb5c3e 100644 > > --- a/net/mac802154/tx.c > > +++ b/net/mac802154/tx.c > > @@ -34,8 +34,8 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) > > if (res) > > goto err_tx; > > > > - dev->stats.tx_packets++; > > - dev->stats.tx_bytes += skb->len; > > + DEV_STATS_INC(dev, tx_packets); > > + DEV_STATS_ADD(dev, tx_bytes, skb->len); > > > > ieee802154_xmit_complete(>hw, skb, false); > > > > @@ -90,8 +90,8 @@ ieee802154_tx(struct ieee802154_local *local, struct > > sk_buff *skb) > > if (ret) > > goto err_wake_netif_queue; > > > > - dev->stats.tx_packets++; > > - dev->stats.tx_bytes += len; > > + DEV_STATS_INC(dev, tx_packets); > > + DEV_STATS_ADD(dev, tx_bytes, len); > > } else { > > local->tx_skb = skb; > > queue_work(local->workqueue, >sync_tx_work); > > This patch has been applied to the wpan tree and will be > part of the next pull request to net. Thanks! Hi! I haven't looked in detail, but FWIW $ git grep LLTX net/mac802154/ $ and similar patch from this author has been rejected: https://lore.kernel.org/all/cann89ilpyoojmxnjbvhy7gwpfbguxwrom9gzz-fwuuyfyjm...@mail.gmail.com/
Re: [PATCH V2] net: qrtr: ns: Ignore ENODEV failures in ns
On Thu, 30 May 2024 16:06:17 +0530 Sarannya S wrote: > -static int service_announce_del(struct sockaddr_qrtr *dest, > +static void service_announce_del(struct sockaddr_qrtr *dest, > struct qrtr_server *srv) nit: please realign the continuation line
Re: [PATCH net-next v2] net: mana: Allow variable size indirection table
On Thu, 30 May 2024 17:37:02 +0300 Leon Romanovsky wrote: > Once you are ok with this patch, let me create shared branch for it. > It is -rc1 and Konstantin already submitted some changes to qp.c > https://lore.kernel.org/all/1716366242-558-1-git-send-email-kotara...@linux.microsoft.com/ > > This specific patch applies on top of Konstantin's changes cleanly. Yeah, once it's not buggy shared branch SG! Just to be sure, on top of -rc1, right?
Re: [PATCH net-next v2] net: mana: Allow variable size indirection table
On Tue, 28 May 2024 22:35:55 -0700 Shradha Gupta wrote: > + save_table = kcalloc(apc->indir_table_sz, sizeof(u32), GFP_KERNEL); > + if (!save_table) > + return -ENOMEM; > + > if (rxfh->indir) { > - for (i = 0; i < MANA_INDIRECT_TABLE_SIZE; i++) > + for (i = 0; i < apc->indir_table_sz; i++) > if (rxfh->indir[i] >= apc->num_queues) > return -EINVAL; leaks save_table
Re: [Intel-wired-lan] [PATCH iwl-next 11/12] idpf: convert header split mode to libeth + napi_build_skb()
On Tue, 28 May 2024 15:48:45 +0200 Alexander Lobakin wrote: > Currently, idpf uses the following model for the header buffers: > > * buffers are allocated via dma_alloc_coherent(); > * when receiving, napi_alloc_skb() is called and then the header is > copied to the newly allocated linear part. > > This is far from optimal as DMA coherent zone is slow on many systems > and memcpy() neutralizes the idea and benefits of the header split. Not > speaking of that XDP can't be run on DMA coherent buffers, but at the > same time the idea of allocating an skb to run XDP program is ill. > Instead, use libeth to create page_pools for the header buffers, allocate > them dynamically and then build an skb via napi_build_skb() around them > with no memory copy. With one exception... > When you enable header split, you except you'll always have a separate accept > header buffer, so that you could reserve headroom and tailroom only > there and then use full buffers for the data. For example, this is how > TCP zerocopy works -- you have to have the payload aligned to PAGE_SIZE. > The current hardware running idpf does *not* guarantee that you'll > always have headers placed separately. For example, on my setup, even > ICMP packets are written as one piece to the data buffers. You can't > build a valid skb around a data buffer in this case. > To not complicate things and not lose TCP zerocopy etc., when such thing > happens, use the empty header buffer and pull either full frame (if it's > short) or the Ethernet header there and build an skb around it. GRO > layer will pull more from the data buffer later. This W/A will hopefully > be removed one day. Hopefully soon, cause it will prevent you from mapping data buffers to user space or using DMABUF memory :(
Re: [Intel-wired-lan] [PATCH iwl-next 01/12] libeth: add cacheline / struct alignment helpers
On Tue, 28 May 2024 15:48:35 +0200 Alexander Lobakin wrote: > diff --git a/scripts/kernel-doc b/scripts/kernel-doc > index 95a59ac78f82..d0cf9a2d82de 100755 > --- a/scripts/kernel-doc > +++ b/scripts/kernel-doc > @@ -1155,6 +1155,7 @@ sub dump_struct($$) { > $members =~ s/\bstruct_group_attr\s*\(([^,]*,){2}/STRUCT_GROUP(/gos; > $members =~ s/\bstruct_group_tagged\s*\(([^,]*),([^,]*),/struct $1 > $2; STRUCT_GROUP(/gos; > $members =~ s/\b__struct_group\s*\(([^,]*,){3}/STRUCT_GROUP(/gos; > +$members =~ s/\blibeth_cacheline_group\s*\(([^,]*,)/struct { } $1; > STRUCT_GROUP(/gos; > $members =~ > s/\bSTRUCT_GROUP(\(((?:(?>[^)(]+)|(?1))*)\))[^;]*;/$2/gos; > > my $args = qr{([^,)]+)}; Having per-driver grouping defines is a no-go. Do you need the defines in the first place? Are you sure the assert you're adding are not going to explode on some weird arch? Honestly, patch 5 feels like a little too much for a driver..
Re: [Intel-wired-lan] [PATCH iwl-next v6 11/12] iavf: handle SIOCSHWTSTAMP and SIOCGHWTSTAMP
On Tue, 28 May 2024 07:23:00 -0400 Mateusz Polchlopek wrote: > + .ndo_eth_ioctl = iavf_do_ioctl, Please use the new ndos: * int (*ndo_hwtstamp_get)(struct net_device *dev, * struct kernel_hwtstamp_config *kernel_config); * Get the currently configured hardware timestamping parameters for the * NIC device. * * int (*ndo_hwtstamp_set)(struct net_device *dev, * struct kernel_hwtstamp_config *kernel_config, * struct netlink_ext_ack *extack); * Change the hardware timestamping parameters for NIC device.
Re: [Intel-wired-lan] [PATCH iwl-net v2] ice: Add support for devlink loopback param.
On Tue, 28 May 2024 13:01:32 +0200 Wojciech Drewek wrote: > Subject: [PATCH iwl-net v2] ice: Add support for devlink loopback param. iwl-next, presumably Param makes sense, although the name keeps giving me pause. I expect "loopback" will control either port loopback or something like hairpin. Would "local-forwarding" not be a better name? Not a big deal, I guess. > +``enabled`` - loopback traffic is not allowed on port > + > +``disabled`` - loopback traffic is allowed on this port The meaning seems inverted?
Re: [linux-next:master] BUILD REGRESSION 6dc544b66971c7f9909ff038b62149105272d26a
On Wed, 29 May 2024 02:19:47 +0800 kernel test robot wrote: > | `-- > net-ipv6-route.c-rt6_fill_node()-error:we-previously-assumed-dst-could-be-null-(see-line-) Is there a way for us to mark this as false positive?
Re: [linux-next:master] BUILD REGRESSION 6dc544b66971c7f9909ff038b62149105272d26a
On Wed, 29 May 2024 02:19:47 +0800 kernel test robot wrote: > | `-- > net-ipv6-route.c-rt6_fill_node()-error:we-previously-assumed-dst-could-be-null-(see-line-) Is there a way for us to mark this as false positive?
Re: [linux-next:master] BUILD REGRESSION 6dc544b66971c7f9909ff038b62149105272d26a
On Wed, 29 May 2024 02:19:47 +0800 kernel test robot wrote: > | `-- > net-ipv6-route.c-rt6_fill_node()-error:we-previously-assumed-dst-could-be-null-(see-line-) Is there a way for us to mark this as false positive?
Re: [linux-next:master] BUILD REGRESSION 6dc544b66971c7f9909ff038b62149105272d26a
On Wed, 29 May 2024 02:19:47 +0800 kernel test robot wrote: > | `-- > net-ipv6-route.c-rt6_fill_node()-error:we-previously-assumed-dst-could-be-null-(see-line-) Is there a way for us to mark this as false positive?
Re: [PATCH net-next] net: mana: Allow variable size indirection table
On Thu, 23 May 2024 09:55:14 -0700 Shradha Gupta wrote: > Allow variable size indirection table allocation in MANA instead > of using a constant value MANA_INDIRECT_TABLE_SIZE. > The size is now derived from the MANA_QUERY_VPORT_CONFIG and the > indirection table is allocated dynamically. > > Signed-off-by: Shradha Gupta > Reviewed-by: Dexuan Cui > Reviewed-by: Haiyang Zhang ## Form letter - net-next-closed The merge window for v6.10 has begun and we have already posted our pull request. Therefore net-next is closed for new drivers, features, code refactoring and optimizations. We are currently accepting bug fixes only. Please repost when net-next reopens after May 26th. RFC patches sent for review only are obviously welcome at any time. See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle -- pw-bot: defer
Re: [PATCH v5 00/68] Define _GNU_SOURCE for sources using
On Wed, 22 May 2024 10:19:33 -0600 Shuah Khan wrote: > On 5/21/24 18:56, Edward Liaw wrote: > > Centralizes the definition of _GNU_SOURCE into KHDR_INCLUDES and removes > > redefinitions of _GNU_SOURCE from source code. > > > > 809216233555 ("selftests/harness: remove use of LINE_MAX") introduced > > asprintf into kselftest_harness.h, which is a GNU extension and needs > > Easier solution to define LINE_MAX locally. In gerenal it is advisable > to not add local defines, but it is desirable in some cases to avoid > churn like this one. Will the patch that Andrew applied: https://lore.kernel.org/all/20240519213733.2ae81c32...@smtp.kernel.org/ make its way to Linus? As you say that's a much simpler fix.
Re: [PATCH v3 0/2] Fix Kernel CI issues
On Fri, 3 May 2024 20:25:49 +0300 Tomi Valkeinen wrote: > > I think the second patch also needs to go to drm-misc-next-fixes? The > > clang warning fixed by it has returned in next-20240503 because it > > appears that for-linux-next was switch from drm-misc-next to > > drm-misc-next-fixes, as I see for-linux-next was pointing to commit > > 235e60653f8d ("drm/debugfs: Drop conditionals around of_node pointers") > > on drm-misc-next in next-20240502 but it is now pointing to commit > > be3f3042391d ("drm: zynqmp_dpsub: Always register bridge") on > > drm-misc-next-fixes in next-20240503. > > Oh. Hmm, did I just hit the feature-freeze point with the fixes... > > Now I'm unsure where I should push these (if anywhere), as they already > are in drm-misc-next. > > DRM Misc maintainers, can you give me a hint? =) This is now breaking allmodconfig build of Linus's tree. Could you please get it fixed ASAP?
Re: [PATCHv2 net] selftests/net: use tc rule to filter the na packet
On Fri, 17 May 2024 10:14:02 +0100 Simon Horman wrote: > Jakub, is there a way to tell how long a test took to execute? > Perhaps it's obvious, but I couldn't see it. It's not obvious, and it was broken. There's an 'info' file with extra metadata in the directory with results: https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/599300/39-arp-ndisc-untracked-subnets-sh/info but it's currently reporting fractional seconds rather than total seconds: https://github.com/linux-netdev/nipa/commit/fb7c45fd3b68b379b7bceb8f79c8df06aaf53ee0 Once we have a proper DB (any day now), I'll add it to the JSON output so it appears in the web UI.
Re: [PATCH] vhost/vsock: always initialize seqpacket_allow
On Wed, 15 May 2024 11:05:43 -0400 Michael S. Tsirkin wrote: > There are two issues around seqpacket_allow: > 1. seqpacket_allow is not initialized when socket is >created. Thus if features are never set, it will be >read uninitialized. > 2. if VIRTIO_VSOCK_F_SEQPACKET is set and then cleared, >then seqpacket_allow will not be cleared appropriately >(existing apps I know about don't usually do this but > it's legal and there's no way to be sure no one relies > on this). Acked-by: Jakub Kicinski -- pw-bot: nap
Re: [PATCH net] selftests/net: use tc rule to filter the na packet
On Tue, 14 May 2024 15:11:30 +0800 Hangbin Liu wrote: > Hi Jakub, would you please help check if this fix the > arp_ndisc_untracked_subnets flake issue on debug kernel? It didn't get ingested by the CI because there's a conflict with something else that got merged into lib.sh. Could you rebase / repost? At a glance the problem in the CI is that it times out on debug kernels: # overriding timeout to 7200 # selftests: net: arp_ndisc_untracked_subnets.sh # TEST: test_arp: accept_arp=0 [ OK ] # TEST: test_arp: accept_arp=1 [ OK ] # TEST: test_arp: accept_arp=2 same_subnet=0[ OK ] # TEST: test_arp: accept_arp=2 same_subnet=1[ OK ] # not ok 1 selftests: net: arp_ndisc_untracked_subnets.sh # TIMEOUT 7200 seconds So it consumed full 2 hours and didn't finish.
[PATCH net v3] selftests: net: local_termination: annotate the expected failures
Vladimir said when adding this test: The bridge driver fares particularly badly [...] mainly because it does not implement IFF_UNICAST_FLT. See commit 90b9566aa5cd ("selftests: forwarding: add a test for local_termination.sh"). We don't want to hide the known gaps, but having a test which always fails prevents us from catching regressions. Report the cases we know may fail as XFAIL. Signed-off-by: Jakub Kicinski --- CC: liuhang...@gmail.com CC: sh...@kernel.org CC: linux-kselftest@vger.kernel.org CC: Petr Machata CC: vladimir.olt...@nxp.com v3: - use xfail_on_veth correctly as a "prefix" call - dropping Vladimir's tags since the code is quite different now v2: https://lore.kernel.org/r/20240509235553.5740-1-k...@kernel.org/ - remove duplicated log_test_xfail v1: https://lore.kernel.org/all/20240509235553.5740-1-k...@kernel.org/ --- .../net/forwarding/local_termination.sh | 30 +++ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/net/forwarding/local_termination.sh b/tools/testing/selftests/net/forwarding/local_termination.sh index c5b0cbc85b3e..4b364cdf3ef0 100755 --- a/tools/testing/selftests/net/forwarding/local_termination.sh +++ b/tools/testing/selftests/net/forwarding/local_termination.sh @@ -155,25 +155,30 @@ run_test() "$smac > $MACVLAN_ADDR, ethertype IPv4 (0x0800)" \ true - check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \ - "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \ - false + xfail_on_veth $h1 \ + check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \ + "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \ + false check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, promisc" \ "$smac > $UNKNOWN_UC_ADDR2, ethertype IPv4 (0x0800)" \ true - check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, allmulti" \ - "$smac > $UNKNOWN_UC_ADDR3, ethertype IPv4 (0x0800)" \ - false + xfail_on_veth $h1 \ + check_rcv $rcv_if_name \ + "Unicast IPv4 to unknown MAC address, allmulti" \ + "$smac > $UNKNOWN_UC_ADDR3, ethertype IPv4 (0x0800)" \ + false check_rcv $rcv_if_name "Multicast IPv4 to joined group" \ "$smac > $JOINED_MACV4_MC_ADDR, ethertype IPv4 (0x0800)" \ true - check_rcv $rcv_if_name "Multicast IPv4 to unknown group" \ - "$smac > $UNKNOWN_MACV4_MC_ADDR1, ethertype IPv4 (0x0800)" \ - false + xfail_on_veth $h1 \ + check_rcv $rcv_if_name \ + "Multicast IPv4 to unknown group" \ + "$smac > $UNKNOWN_MACV4_MC_ADDR1, ethertype IPv4 (0x0800)" \ + false check_rcv $rcv_if_name "Multicast IPv4 to unknown group, promisc" \ "$smac > $UNKNOWN_MACV4_MC_ADDR2, ethertype IPv4 (0x0800)" \ @@ -187,9 +192,10 @@ run_test() "$smac > $JOINED_MACV6_MC_ADDR, ethertype IPv6 (0x86dd)" \ true - check_rcv $rcv_if_name "Multicast IPv6 to unknown group" \ - "$smac > $UNKNOWN_MACV6_MC_ADDR1, ethertype IPv6 (0x86dd)" \ - false + xfail_on_veth $h1 \ + check_rcv $rcv_if_name "Multicast IPv6 to unknown group" \ + "$smac > $UNKNOWN_MACV6_MC_ADDR1, ethertype IPv6 (0x86dd)" \ + false check_rcv $rcv_if_name "Multicast IPv6 to unknown group, promisc" \ "$smac > $UNKNOWN_MACV6_MC_ADDR2, ethertype IPv6 (0x86dd)" \ -- 2.45.0
Re: [PATCH net-next] selftests: net: local_termination: annotate the expected failures
On Wed, 15 May 2024 11:02:28 +0200 Petr Machata wrote: > >> And then either replace the existing xfail_on_veth's (there are just a > >> handful) or convert xfail_on_veth to a wrapper around xfail_on_kind. > > > > I think the bridge thing we can workaround by just checking > > if ${NETIFS[p1]} is veth, rather than $rcv_if_name. > > Since the two behave the same. > > I don't follow. The test has two legs, one creates a VRF and attaches > p2, the other creates a bridge and attaches p2. Whether p1 and p2 are > veth or HW seems orthogonal to whether $rcv_if_name is a bridge or a > veth. Right, my superficial understanding was that the main distinction is whether p2/h2 can do the filtering (or possibly some offload happens). So if p1,p2 are veths we know to XFAIL, doesn't matter if we're in the vrf or bridge configuration, cause these construct will not filter either. If I'm not making sense - I'm probably confused, I can code up what you suggested, it will work, just more LoC :) --->8-- From 7a645eb425530f647e88590ba7ba01681e73812e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 9 May 2024 16:28:41 -0700 Subject: selftests: net: local_termination: annotate the expected failures Vladimir said when adding this test: The bridge driver fares particularly badly [...] mainly because it does not implement IFF_UNICAST_FLT. See commit 90b9566aa5cd ("selftests: forwarding: add a test for local_termination.sh"). We don't want to hide the known gaps, but having a test which always fails prevents us from catching regressions. Report the cases we know may fail as XFAIL. Reviewed-by: Vladimir Oltean Tested-by: Vladimir Oltean Signed-off-by: Jakub Kicinski --- CC: liuhang...@gmail.com CC: sh...@kernel.org CC: linux-kselftest@vger.kernel.org v2: - remove duplicated log_test_xfail v1: https://lore.kernel.org/all/20240509235553.5740-1-k...@kernel.org/ --- tools/testing/selftests/net/forwarding/lib.sh | 7 +++ .../selftests/net/forwarding/local_termination.sh | 13 - 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 112c85c35092..79aa3c8bc288 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -473,6 +473,13 @@ ret_set_ksft_status() # Whether FAILs should be interpreted as XFAILs. Internal. FAIL_TO_XFAIL= +# Clear internal failure tracking for the next test case +begin_test() +{ +RET=0 +FAIL_TO_XFAIL= +} + check_err() { local err=$1 diff --git a/tools/testing/selftests/net/forwarding/local_termination.sh b/tools/testing/selftests/net/forwarding/local_termination.sh index c5b0cbc85b3e..a241acc02498 100755 --- a/tools/testing/selftests/net/forwarding/local_termination.sh +++ b/tools/testing/selftests/net/forwarding/local_termination.sh @@ -73,9 +73,12 @@ check_rcv() local pattern=$3 local should_receive=$4 local should_fail= + local xfail_sw=$5 [ $should_receive = true ] && should_fail=0 || should_fail=1 - RET=0 + begin_test + # check if main interface is veth + [ "$xfail_sw" == true ] && xfail_on_veth $h1 tcpdump_show $if_name | grep -q "$pattern" @@ -157,7 +160,7 @@ run_test() check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \ "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \ - false + false true check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, promisc" \ "$smac > $UNKNOWN_UC_ADDR2, ethertype IPv4 (0x0800)" \ @@ -165,7 +168,7 @@ run_test() check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, allmulti" \ "$smac > $UNKNOWN_UC_ADDR3, ethertype IPv4 (0x0800)" \ - false + false true check_rcv $rcv_if_name "Multicast IPv4 to joined group" \ "$smac > $JOINED_MACV4_MC_ADDR, ethertype IPv4 (0x0800)" \ @@ -173,7 +176,7 @@ run_test() check_rcv $rcv_if_name "Multicast IPv4 to unknown group" \ "$smac > $UNKNOWN_MACV4_MC_ADDR1, ethertype IPv4 (0x0800)" \ - false + false true check_rcv $rcv_if_name "Multicast IPv4 to unknown group, promisc" \ "$smac > $UNKNOWN_MACV4_MC_ADDR2, ethertype IPv4 (0x0800)" \ @@ -189,7 +192,7 @@ run_test() check_rcv $rcv_if_name "Multicast IPv6 to unknown group" \ "$smac > $UNKNOWN_MACV6_MC_ADDR1, ethertype IPv6 (0x86dd)" \ - false + false true check_rcv $rcv_if_name "Multicast IPv6 to unknown group, promisc" \ "$smac > $UNKNOWN_MACV6_MC_ADDR2, ethertype IPv6 (0x86dd)" \ -- 2.45.0
Re: [PATCH net-next] selftests: net: local_termination: annotate the expected failures
On Mon, 13 May 2024 15:25:38 +0200 Petr Machata wrote: > For veth specifically there is xfail_on_veth: > > xfail_on_veth $rcv_if_name \ > check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \ > "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \ > false > > Which is IMHO clearer than passing an extra boolean. The extra boolean is because we want to only fail particular subcases. I think that's legit. If the other cases regress we want to know. Otherwise running the test on SW bridge is only useful for catching crashes (so less useful). So we probably need to reset FAIL_TO_XFAIL in this case. Any preference on how to go about that? I'm tempted to: diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 112c85c35092..79aa3c8bc288 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -473,6 +473,13 @@ ret_set_ksft_status() # Whether FAILs should be interpreted as XFAILs. Internal. FAIL_TO_XFAIL= +# Clear internal failure tracking for the next test case +begin_test() +{ +RET=0 +FAIL_TO_XFAIL= +} + check_err() { local err=$1 diff --git a/tools/testing/selftests/net/forwarding/local_termination.sh b/tools/testing/selftests/net/forwarding/local_termination.sh index 65694cdf2dbb..0781ceba1348 100755 --- a/tools/testing/selftests/net/forwarding/local_termination.sh +++ b/tools/testing/selftests/net/forwarding/local_termination.sh @@ -76,7 +76,7 @@ check_rcv() local xfail_sw=$5 [ $should_receive = true ] && should_fail=0 || should_fail=1 - RET=0 + begin_test tcpdump_show $if_name | grep -q "$pattern" > Not sure what to do about the bridge bit though. In principle the > various xfail_on_'s can be chained, so e.g. this should work: > > xfail_on_bridge $rcv_if_name \ > xfail_on_veth $rcv_if_name \ > check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \ > "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \ > false > > I find this preferable to adding these ad-hoc tweaks to each test > individually. Maybe it would make sense to have: > > xfail_on_kind $rcv_if_name veth bridge \ > check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \ > "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \ > false > > And then either replace the existing xfail_on_veth's (there are just a > handful) or convert xfail_on_veth to a wrapper around xfail_on_kind. I think the bridge thing we can workaround by just checking if ${NETIFS[p1]} is veth, rather than $rcv_if_name. Since the two behave the same.
Re: [PATCH net-next v10 2/3] net: gro: move L3 flush checks to tcp_gro_receive and udp_gro_receive_segment
On Tue, 14 May 2024 17:56:44 +0200 Richard Gobert wrote: > > Hi Richard, any chance of getting this fixed within the next 2 hours? > > I can't send the net-next PR if it doesn't build on one of the arches.. > > Hi Jakub and Geert, > I'm only seeing this mail now, sorry for the late response. > I can fix this within the next two hours, would you prefer a standalone > patch or should I add it to this patch series? Standalone patch, please!
Re: [PATCH net-next v10 2/3] net: gro: move L3 flush checks to tcp_gro_receive and udp_gro_receive_segment
On Tue, 14 May 2024 14:13:21 +0200 Geert Uytterhoeven wrote: > On Thu, May 9, 2024 at 9:09 PM Richard Gobert > wrote: > > {inet,ipv6}_gro_receive functions perform flush checks (ttl, flags, > > iph->id, ...) against all packets in a loop. These flush checks are used in > > all merging UDP and TCP flows. > > > > These checks need to be done only once and only against the found p skb, > > since they only affect flush and not same_flow. > > > > This patch leverages correct network header offsets from the cb for both > > outer and inner network headers - allowing these checks to be done only > > once, in tcp_gro_receive and udp_gro_receive_segment. As a result, > > NAPI_GRO_CB(p)->flush is not used at all. In addition, flush_id checks are > > more declarative and contained in inet_gro_flush, thus removing the need > > for flush_id in napi_gro_cb. > > > > This results in less parsing code for non-loop flush tests for TCP and UDP > > flows. > > > > To make sure results are not within noise range - I've made netfilter drop > > all TCP packets, and measured CPU performance in GRO (in this case GRO is > > responsible for about 50% of the CPU utilization). > > > > perf top while replaying 64 parallel IP/TCP streams merging in GRO: > > (gro_receive_network_flush is compiled inline to tcp_gro_receive) > > net-next: > > 6.94% [kernel] [k] inet_gro_receive > > 3.02% [kernel] [k] tcp_gro_receive > > > > patch applied: > > 4.27% [kernel] [k] tcp_gro_receive > > 4.22% [kernel] [k] inet_gro_receive > > > > perf top while replaying 64 parallel IP/IP/TCP streams merging in GRO (same > > results for any encapsulation, in this case inet_gro_receive is top > > offender in net-next) > > net-next: > > 10.09% [kernel] [k] inet_gro_receive > > 2.08% [kernel] [k] tcp_gro_receive > > > > patch applied: > > 6.97% [kernel] [k] inet_gro_receive > > 3.68% [kernel] [k] tcp_gro_receive > > > > Signed-off-by: Richard Gobert > > Thanks for your patch, which is now commit 4b0ebbca3e167976 ("net: gro: > move L3 flush checks to tcp_gro_receive and udp_gro_receive_segment") > in net-next/main (next-20240514). > > nore...@ellerman.id.au reports build failures on m68k, e.g. > http://kisskb.ellerman.id.au/kisskb/buildresult/15168903/ > > net/core/gro.c: In function ‘dev_gro_receive’: > ././include/linux/compiler_types.h:460:38: error: call to > ‘__compiletime_assert_654’ declared with attribute error: BUILD_BUG_ON > failed: !IS_ALIGNED(offsetof(struct napi_gro_cb, zeroed), sizeof(u32)) Hi Richard, any chance of getting this fixed within the next 2 hours? I can't send the net-next PR if it doesn't build on one of the arches..
Re: [PATCH net-next] net: mana: Enable MANA driver on ARM64 with 4K page size
On Mon, 13 May 2024 20:50:37 + Haiyang Zhang wrote: > From the document, it can be: > "ARM cores support both modes, but are most commonly used in, and typically > default to little-endian mode. Most Linux distributions for ARM tend to be > little-endian only." > https://developer.arm.com/documentation/den0042/a/Coding-for-Cortex-R-Processors/Endianness > > MANA driver doesn't support big endian. Alright, but please prioritize at least adding the 64k page support. Linux drivers are supposed to be as platform independent as possible. If you use the right APIs you shouldn't have to worry about the endian or the page size.
Re: [PATCH net-next v9 00/14] Device Memory TCP
On Fri, 10 May 2024 16:21:11 -0700 Mina Almasry wrote: > Device Memory TCP Sorry Mina, this is too big to apply during the merge window :( -- pw-bot: defer
Re: [PATCH net-next v9 00/14] Device Memory TCP
On Fri, 10 May 2024 16:21:11 -0700 Mina Almasry wrote: > Device Memory TCP Sorry Mina, this is too big to apply during the merge window :( -- pw-bot: defer
Re: [PATCH net-next] net: mana: Enable MANA driver on ARM64 with 4K page size
On Mon, 13 May 2024 13:29:01 -0700 Haiyang Zhang wrote: > - depends on PCI_MSI && X86_64 > + depends on PCI_MSI > + depends on X86_64 || (ARM64 && !CPU_BIG_ENDIAN && ARM64_4K_PAGES) Can ARM64 be big endian?
[PATCH net-next v2] selftests: net: local_termination: annotate the expected failures
Vladimir said when adding this test: The bridge driver fares particularly badly [...] mainly because it does not implement IFF_UNICAST_FLT. See commit 90b9566aa5cd ("selftests: forwarding: add a test for local_termination.sh"). We don't want to hide the known gaps, but having a test which always fails prevents us from catching regressions. Report the cases we know may fail as XFAIL. Reviewed-by: Vladimir Oltean Tested-by: Vladimir Oltean Signed-off-by: Jakub Kicinski --- CC: liuhang...@gmail.com CC: sh...@kernel.org CC: linux-kselftest@vger.kernel.org v2: - remove duplicated log_test_xfail v1: https://lore.kernel.org/all/20240509235553.5740-1-k...@kernel.org/ --- .../net/forwarding/local_termination.sh | 21 ++- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/net/forwarding/local_termination.sh b/tools/testing/selftests/net/forwarding/local_termination.sh index c5b0cbc85b3e..4bba9c78db3e 100755 --- a/tools/testing/selftests/net/forwarding/local_termination.sh +++ b/tools/testing/selftests/net/forwarding/local_termination.sh @@ -73,6 +73,10 @@ check_rcv() local pattern=$3 local should_receive=$4 local should_fail= + local xfail_sw=$5 + + local kind=$(ip -j -d link show dev $if_name | +jq -r '.[].linkinfo.info_kind') [ $should_receive = true ] && should_fail=0 || should_fail=1 RET=0 @@ -81,7 +85,14 @@ check_rcv() check_err_fail "$should_fail" "$?" "reception" - log_test "$if_name: $type" + # If not a SW interface, ignore the XFAIL allowance + [ "$kind" != veth ] && [ "$kind" != bridge ] && xfail_sw= + + if [ $RET -ne 0 ] && [ "$xfail_sw" == true ]; then + log_test_xfail "$if_name: $type" + else + log_test "$if_name: $type" + fi } mc_route_prepare() @@ -157,7 +168,7 @@ run_test() check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \ "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \ - false + false true check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, promisc" \ "$smac > $UNKNOWN_UC_ADDR2, ethertype IPv4 (0x0800)" \ @@ -165,7 +176,7 @@ run_test() check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, allmulti" \ "$smac > $UNKNOWN_UC_ADDR3, ethertype IPv4 (0x0800)" \ - false + false true check_rcv $rcv_if_name "Multicast IPv4 to joined group" \ "$smac > $JOINED_MACV4_MC_ADDR, ethertype IPv4 (0x0800)" \ @@ -173,7 +184,7 @@ run_test() check_rcv $rcv_if_name "Multicast IPv4 to unknown group" \ "$smac > $UNKNOWN_MACV4_MC_ADDR1, ethertype IPv4 (0x0800)" \ - false + false true check_rcv $rcv_if_name "Multicast IPv4 to unknown group, promisc" \ "$smac > $UNKNOWN_MACV4_MC_ADDR2, ethertype IPv4 (0x0800)" \ @@ -189,7 +200,7 @@ run_test() check_rcv $rcv_if_name "Multicast IPv6 to unknown group" \ "$smac > $UNKNOWN_MACV6_MC_ADDR1, ethertype IPv6 (0x86dd)" \ - false + false true check_rcv $rcv_if_name "Multicast IPv6 to unknown group, promisc" \ "$smac > $UNKNOWN_MACV6_MC_ADDR2, ethertype IPv6 (0x86dd)" \ -- 2.45.0
Re: [PATCH net-next 1/2] selftests: net: fix timestamp not arriving in cmsg_time.sh
On Thu, 09 May 2024 22:49:50 -0400 Willem de Bruijn wrote: > > struct sock_extended_err *see; > > struct scm_timestamping *ts; > > + unsigned int ts_seen = 0; > > nit: mixing unsigned long and unsigned int Fixed when applying, thanks!
Re: [PATCH net-next] selftests: net: use upstream mtools
On Fri, 10 May 2024 14:28:56 +0300 Vladimir Oltean wrote: > Check that the deployed mtools version is 3.0 or above. Note that the > version check breaks compatibility with my fork And Joachim's tree from before the tag, the PR was merged a while back ;) > where I didn't bump the version, but I assume that won't be a problem. Agreed, no point accumulating more bespoke checks. I updated NIPA, next run should have tagged 3.0. Speaking of requirement checks - I run the test on a fresh Fedora install yesterday and it was failing with no indication of why. Then I realized tcpdump wasn't installed :(
Re: [PATCH net-next] selftests: net: local_termination: annotate the expected failures
On Fri, 10 May 2024 11:15:58 +0800 Hangbin Liu wrote: > I may missed something, I saw there is already a log_test_xfail() in lib.sh > > log_test_skip() > { > RET=$ksft_skip retmsg= log_test "$@" > } > > log_test_xfail() > { > RET=$ksft_xfail retmsg= log_test "$@" > } > > log_info() > { > ... > } > > Added by a923af1ceee7 ("selftests: forwarding: Convert log_test() to > recognize RET values") Good catch, sorry. Must have had net checked out when I wrote it :(
[PATCH net-next 2/2] selftests: net: increase the delay for relative cmsg_time.sh test
Slow machines can delay scheduling of the packets for milliseconds. Increase the delay to 8ms if KSFT_MACHINE_SLOW. Try to limit the variability by moving setsockopts earlier (before we read time). This fixes the "TXTIME rel" failures on debug kernels, like: Case ICMPv4 - TXTIME rel returned '', expected 'OK' Signed-off-by: Jakub Kicinski --- CC: sh...@kernel.org CC: linux-kselftest@vger.kernel.org --- tools/testing/selftests/net/cmsg_sender.c | 32 +-- tools/testing/selftests/net/cmsg_time.sh | 7 +++-- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/tools/testing/selftests/net/cmsg_sender.c b/tools/testing/selftests/net/cmsg_sender.c index f25268504937..b2df05ef71cb 100644 --- a/tools/testing/selftests/net/cmsg_sender.c +++ b/tools/testing/selftests/net/cmsg_sender.c @@ -260,15 +260,8 @@ cs_write_cmsg(int fd, struct msghdr *msg, char *cbuf, size_t cbuf_sz) SOL_IPV6, IPV6_HOPLIMIT, ); if (opt.txtime.ena) { - struct sock_txtime so_txtime = { - .clockid = CLOCK_MONOTONIC, - }; __u64 txtime; - if (setsockopt(fd, SOL_SOCKET, SO_TXTIME, - _txtime, sizeof(so_txtime))) - error(ERN_SOCKOPT, errno, "setsockopt TXTIME"); - txtime = time_start_mono.tv_sec * (1000ULL * 1000 * 1000) + time_start_mono.tv_nsec + opt.txtime.delay * 1000; @@ -284,13 +277,6 @@ cs_write_cmsg(int fd, struct msghdr *msg, char *cbuf, size_t cbuf_sz) memcpy(CMSG_DATA(cmsg), , sizeof(txtime)); } if (opt.ts.ena) { - __u32 val = SOF_TIMESTAMPING_SOFTWARE | - SOF_TIMESTAMPING_OPT_TSONLY; - - if (setsockopt(fd, SOL_SOCKET, SO_TIMESTAMPING, - , sizeof(val))) - error(ERN_SOCKOPT, errno, "setsockopt TIMESTAMPING"); - cmsg = (struct cmsghdr *)(cbuf + cmsg_len); cmsg_len += CMSG_SPACE(sizeof(__u32)); if (cbuf_sz < cmsg_len) @@ -426,6 +412,24 @@ static void ca_set_sockopts(int fd) setsockopt(fd, SOL_SOCKET, SO_PRIORITY, , sizeof(opt.sockopt.priority))) error(ERN_SOCKOPT, errno, "setsockopt SO_PRIORITY"); + + if (opt.txtime.ena) { + struct sock_txtime so_txtime = { + .clockid = CLOCK_MONOTONIC, + }; + + if (setsockopt(fd, SOL_SOCKET, SO_TXTIME, + _txtime, sizeof(so_txtime))) + error(ERN_SOCKOPT, errno, "setsockopt TXTIME"); + } + if (opt.ts.ena) { + __u32 val = SOF_TIMESTAMPING_SOFTWARE | + SOF_TIMESTAMPING_OPT_TSONLY; + + if (setsockopt(fd, SOL_SOCKET, SO_TIMESTAMPING, + , sizeof(val))) + error(ERN_SOCKOPT, errno, "setsockopt TIMESTAMPING"); + } } int main(int argc, char *argv[]) diff --git a/tools/testing/selftests/net/cmsg_time.sh b/tools/testing/selftests/net/cmsg_time.sh index af85267ad1e3..1d7e756644bc 100755 --- a/tools/testing/selftests/net/cmsg_time.sh +++ b/tools/testing/selftests/net/cmsg_time.sh @@ -66,10 +66,13 @@ for i in "-4 $TGT4" "-6 $TGT6"; do awk '/SND/ { if ($3 > 1000) print "OK"; }') check_result $? "$ts" "OK" "$prot - TXTIME abs" - ts=$(ip netns exec $NS ./cmsg_sender -p $p $i 1234 -t -d 1000 | + [ "$KSFT_MACHINE_SLOW" = yes ] && delay=8000 || delay=1000 + + ts=$(ip netns exec $NS ./cmsg_sender -p $p $i 1234 -t -d $delay | awk '/SND/ {snd=$3} /SCHED/ {sch=$3} - END { if (snd - sch > 500) print "OK"; }') + END { if (snd - sch > '$((delay/2))') print "OK"; + else print snd, "-", sch, "<", '$((delay/2))'; }') check_result $? "$ts" "OK" "$prot - TXTIME rel" done done -- 2.45.0
[PATCH net-next 1/2] selftests: net: fix timestamp not arriving in cmsg_time.sh
On slow machines the SND timestamp sometimes doesn't arrive before we quit. The test only waits as long as the packet delay, so it's easy for a race condition to happen. Double the wait but do a bit of polling, once the SND timestamp arrives there's no point to wait any longer. This fixes the "TXTIME abs" failures on debug kernels, like: Case ICMPv4 - TXTIME abs returned '', expected 'OK' Signed-off-by: Jakub Kicinski --- CC: sh...@kernel.org CC: linux-kselftest@vger.kernel.org --- tools/testing/selftests/net/cmsg_sender.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/net/cmsg_sender.c b/tools/testing/selftests/net/cmsg_sender.c index c79e65581dc3..f25268504937 100644 --- a/tools/testing/selftests/net/cmsg_sender.c +++ b/tools/testing/selftests/net/cmsg_sender.c @@ -333,16 +333,17 @@ static const char *cs_ts_info2str(unsigned int info) return "unknown"; } -static void +static unsigned long cs_read_cmsg(int fd, struct msghdr *msg, char *cbuf, size_t cbuf_sz) { struct sock_extended_err *see; struct scm_timestamping *ts; + unsigned int ts_seen = 0; struct cmsghdr *cmsg; int i, err; if (!opt.ts.ena) - return; + return 0; msg->msg_control = cbuf; msg->msg_controllen = cbuf_sz; @@ -396,8 +397,11 @@ cs_read_cmsg(int fd, struct msghdr *msg, char *cbuf, size_t cbuf_sz) printf(" %5s ts%d %lluus\n", cs_ts_info2str(see->ee_info), i, rel_time); + ts_seen |= 1 << see->ee_info; } } + + return ts_seen; } static void ca_set_sockopts(int fd) @@ -509,10 +513,16 @@ int main(int argc, char *argv[]) err = ERN_SUCCESS; if (opt.ts.ena) { - /* Make sure all timestamps have time to loop back */ - usleep(opt.txtime.delay); + unsigned long seen; + int i; - cs_read_cmsg(fd, , cbuf, sizeof(cbuf)); + /* Make sure all timestamps have time to loop back */ + for (i = 0; i < 40; i++) { + seen = cs_read_cmsg(fd, , cbuf, sizeof(cbuf)); + if (seen & (1 << SCM_TSTAMP_SND)) + break; + usleep(opt.txtime.delay / 20); + } } err_out: -- 2.45.0
[PATCH net-next] selftests: net: local_termination: annotate the expected failures
Vladimir said when adding this test: The bridge driver fares particularly badly [...] mainly because it does not implement IFF_UNICAST_FLT. See commit 90b9566aa5cd ("selftests: forwarding: add a test for local_termination.sh"). We don't want to hide the known gaps, but having a test which always fails prevents us from catching regressions. Report the cases we know may fail as XFAIL. Signed-off-by: Jakub Kicinski --- CC: vladimir.olt...@nxp.com CC: sh...@kernel.org CC: pe...@nvidia.com CC: liuhang...@gmail.com CC: bpoir...@nvidia.com CC: linux-kselftest@vger.kernel.org --- tools/testing/selftests/net/forwarding/lib.sh | 9 .../net/forwarding/local_termination.sh | 21 ++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 3353a1745946..4fe28ab5d8b9 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -605,6 +605,15 @@ log_test_xfail() RET=$ksft_xfail retmsg= log_test "$@" } +log_test_xfail() +{ + local test_name=$1 + local opt_str=$2 + + printf "TEST: %-60s [XFAIL]\n" "$test_name $opt_str" + return 0 +} + log_info() { local msg=$1 diff --git a/tools/testing/selftests/net/forwarding/local_termination.sh b/tools/testing/selftests/net/forwarding/local_termination.sh index c5b0cbc85b3e..4bba9c78db3e 100755 --- a/tools/testing/selftests/net/forwarding/local_termination.sh +++ b/tools/testing/selftests/net/forwarding/local_termination.sh @@ -73,6 +73,10 @@ check_rcv() local pattern=$3 local should_receive=$4 local should_fail= + local xfail_sw=$5 + + local kind=$(ip -j -d link show dev $if_name | +jq -r '.[].linkinfo.info_kind') [ $should_receive = true ] && should_fail=0 || should_fail=1 RET=0 @@ -81,7 +85,14 @@ check_rcv() check_err_fail "$should_fail" "$?" "reception" - log_test "$if_name: $type" + # If not a SW interface, ignore the XFAIL allowance + [ "$kind" != veth ] && [ "$kind" != bridge ] && xfail_sw= + + if [ $RET -ne 0 ] && [ "$xfail_sw" == true ]; then + log_test_xfail "$if_name: $type" + else + log_test "$if_name: $type" + fi } mc_route_prepare() @@ -157,7 +168,7 @@ run_test() check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address" \ "$smac > $UNKNOWN_UC_ADDR1, ethertype IPv4 (0x0800)" \ - false + false true check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, promisc" \ "$smac > $UNKNOWN_UC_ADDR2, ethertype IPv4 (0x0800)" \ @@ -165,7 +176,7 @@ run_test() check_rcv $rcv_if_name "Unicast IPv4 to unknown MAC address, allmulti" \ "$smac > $UNKNOWN_UC_ADDR3, ethertype IPv4 (0x0800)" \ - false + false true check_rcv $rcv_if_name "Multicast IPv4 to joined group" \ "$smac > $JOINED_MACV4_MC_ADDR, ethertype IPv4 (0x0800)" \ @@ -173,7 +184,7 @@ run_test() check_rcv $rcv_if_name "Multicast IPv4 to unknown group" \ "$smac > $UNKNOWN_MACV4_MC_ADDR1, ethertype IPv4 (0x0800)" \ - false + false true check_rcv $rcv_if_name "Multicast IPv4 to unknown group, promisc" \ "$smac > $UNKNOWN_MACV4_MC_ADDR2, ethertype IPv4 (0x0800)" \ @@ -189,7 +200,7 @@ run_test() check_rcv $rcv_if_name "Multicast IPv6 to unknown group" \ "$smac > $UNKNOWN_MACV6_MC_ADDR1, ethertype IPv6 (0x86dd)" \ - false + false true check_rcv $rcv_if_name "Multicast IPv6 to unknown group, promisc" \ "$smac > $UNKNOWN_MACV6_MC_ADDR2, ethertype IPv6 (0x86dd)" \ -- 2.45.0
Re: [PATCH net] selftests: net: move amt to socat for better compatibility
On Thu, 09 May 2024 19:39:36 +0200 Paolo Abeni wrote: > Acked-by: Paolo Abeni Thanks! > As a minor note, shell variable expansion should already trim all the > trailing/leading spaces from the socat command output, so it should not > be necessary replace the string comparison with the grep command: > > RESULT6=$(ip netns exec "${LISTENER}" timeout 15 socat - > UDP6-LISTEN:6000,readbytes=128 || true) > if [ "$RESULT6" == "2001:db8:3::2" ]; then 99% sure I tried that exact thing, but it wasn't enough. $ msg=$(socat - UDP4-LISTEN:1234,readbytes=128) $ echo ">$msg<" >127.0.0.1 > < $ msg=$msg $ echo ">$msg<" >127.0.0.1 > < $ msg=$(echo $msg) $ echo ">$msg<" >127.0.0.1< IOW we'd need to feed it thru an echo or some such. Possibly something like: [ $(echo $msg) == "127.0.0.1" ] But I personally find that a touch too magical. The grep works 路️
Re: [PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.h
On Thu, 9 May 2024 13:08:36 +0800 Tao Su wrote: > > SGTM, FWIW. The print is printing a test summary line, printing more > > than 1k seems rather unreasonable. Other facilities, like TH_LOG(), > > should be used for displaying longer info. > > Thanks, do you think 1k is enough for test_name? Definitely.
Re: [PATCH net] selftests: net: avoid waiting for server in amt.sh forever when it fails.
On Wed, 8 May 2024 04:06:43 + Taehee Yoo wrote: > In the forwarding testcase, it opens a server and a client with the nc. > The server receives the correct message from NC, it prints OK. > The server prints FAIL if it receives the wrong message from the client. > > But If the server can't receive any message, it will not close so > the amt.sh waits forever. > There are several reasons. > 1. crash of smcrouted. > 2. Send a message from the client to the server before the server is up. > > To avoid this problem, the server waits only for 10 seconds. > The client sends messages for 10 seconds. > If the server is successfully closed, it kills the client. Since this didn't fix the problem of smcroute crashing I had to take a closer look myself. I filed https://github.com/troglobit/smcroute/issues/207 for smcroute And sent: https://lore.kernel.org/all/20240509161919.3939966-1-k...@kernel.org/ and https://lore.kernel.org/all/20240509161952.3940476-1-k...@kernel.org/ Please don't use netcat in tests in the future. There are two incompatible implementations which always cause hard to repro issues.
[PATCH net] selftests: net: move amt to socat for better compatibility
The test seems to expect that nc will exit after the first received message. This is not the case with Ncat 7.94. There are multiple versions of nc out there, switch to socat for better compatibility. Tell socat to exit after 128 bytes and pad the message. Since the test sets -e make sure we don't set exit code (|| true) and print the pass / fail rather then silently moving over the test and just setting non-zero exit code with no output indicating what failed. Fixes: c08e8baea78e ("selftests: add amt interface selftest script") Signed-off-by: Jakub Kicinski --- CC: sh...@kernel.org CC: ap420...@gmail.com CC: linux-kselftest@vger.kernel.org --- tools/testing/selftests/net/amt.sh | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/net/amt.sh b/tools/testing/selftests/net/amt.sh index 75528788cb95..5175a42cbe8a 100755 --- a/tools/testing/selftests/net/amt.sh +++ b/tools/testing/selftests/net/amt.sh @@ -210,8 +210,8 @@ check_features() test_ipv4_forward() { - RESULT4=$(ip netns exec "${LISTENER}" nc -w 1 -l -u 239.0.0.1 4000) - if [ "$RESULT4" == "172.17.0.2" ]; then + RESULT4=$(ip netns exec "${LISTENER}" timeout 15 socat - UDP4-LISTEN:4000,readbytes=128 || true) + if echo "$RESULT4" | grep -q "172.17.0.2"; then printf "TEST: %-60s [ OK ]\n" "IPv4 amt multicast forwarding" exit 0 else @@ -222,8 +222,8 @@ test_ipv4_forward() test_ipv6_forward() { - RESULT6=$(ip netns exec "${LISTENER}" nc -w 1 -l -u ff0e::5:6 6000) - if [ "$RESULT6" == "2001:db8:3::2" ]; then + RESULT6=$(ip netns exec "${LISTENER}" timeout 15 socat - UDP6-LISTEN:6000,readbytes=128 || true) + if echo "$RESULT6" | grep -q "2001:db8:3::2"; then printf "TEST: %-60s [ OK ]\n" "IPv6 amt multicast forwarding" exit 0 else @@ -236,14 +236,14 @@ send_mcast4() { sleep 2 ip netns exec "${SOURCE}" bash -c \ - 'echo 172.17.0.2 | nc -w 1 -u 239.0.0.1 4000' & + 'printf "%s %128s" 172.17.0.2 | nc -w 1 -u 239.0.0.1 4000' & } send_mcast6() { sleep 2 ip netns exec "${SOURCE}" bash -c \ - 'echo 2001:db8:3::2 | nc -w 1 -u ff0e::5:6 6000' & + 'printf "%s %128s" 2001:db8:3::2 | nc -w 1 -u ff0e::5:6 6000' & } check_features -- 2.45.0
[PATCH net] selftests: net: add missing config for amt.sh
Test needs IPv6 multicast. smcroute currently crashes when trying to install a route in a kernel without IPv6 multicast. Fixes: c08e8baea78e ("selftests: add amt interface selftest script") Signed-off-by: Jakub Kicinski --- CC: sh...@kernel.org CC: ap420...@gmail.com CC: linux-kselftest@vger.kernel.org --- tools/testing/selftests/net/config | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config index 5e4390cac17e..04de7a6ba6f3 100644 --- a/tools/testing/selftests/net/config +++ b/tools/testing/selftests/net/config @@ -30,6 +30,7 @@ CONFIG_IP_GRE=m CONFIG_NETFILTER=y CONFIG_NETFILTER_ADVANCED=y CONFIG_NF_CONNTRACK=m +CONFIG_IPV6_MROUTE=y CONFIG_IPV6_SIT=y CONFIG_IP_DCCP=m CONFIG_NF_NAT=m -- 2.45.0
Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers
On Wed, 8 May 2024 10:11:35 -0700 Kees Cook wrote: > > Split this per subsystem, please. > > I've done a few painful API transitions before, and I don't think the > complexity of these changes needs a per-subsystem constification pass. I > think this series is the right approach, but that patch 11 will need > coordination with Linus. We regularly do system-wide prototype changes > like this right at the end of the merge window before -rc1 comes out. Right. I didn't read the code closely enough before responding. Chalk my response up to being annoyed by the constant stream of cross-tree changes in procfs without proper cover letter explaining how they will be merged :| ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers
On Wed, 8 May 2024 10:11:35 -0700 Kees Cook wrote: > > Split this per subsystem, please. > > I've done a few painful API transitions before, and I don't think the > complexity of these changes needs a per-subsystem constification pass. I > think this series is the right approach, but that patch 11 will need > coordination with Linus. We regularly do system-wide prototype changes > like this right at the end of the merge window before -rc1 comes out. Right. I didn't read the code closely enough before responding. Chalk my response up to being annoyed by the constant stream of cross-tree changes in procfs without proper cover letter explaining how they will be merged :|
Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers
On Wed, 8 May 2024 10:11:35 -0700 Kees Cook wrote: > > Split this per subsystem, please. > > I've done a few painful API transitions before, and I don't think the > complexity of these changes needs a per-subsystem constification pass. I > think this series is the right approach, but that patch 11 will need > coordination with Linus. We regularly do system-wide prototype changes > like this right at the end of the merge window before -rc1 comes out. Right. I didn't read the code closely enough before responding. Chalk my response up to being annoyed by the constant stream of cross-tree changes in procfs without proper cover letter explaining how they will be merged :|
Re: [PATCH net-next v3] selftest: epoll_busy_poll: epoll busy poll tests
On Tue, 7 May 2024 19:21:50 -0700 Joe Damato wrote: > On Wed, May 08, 2024 at 12:43:26AM +, Joe Damato wrote: > > Add a simple test for the epoll busy poll ioctls, using the kernel > > selftest harness. > > > > This test ensures that the ioctls have the expected return codes and > > that the kernel properly gets and sets epoll busy poll parameters. > > > > The test can be expanded in the future to do real busy polling (provided > > another machine to act as the client is available). > > Ah, built and worked for me, but of course fails remotely: > > epoll_busy_poll.c:20:10: fatal error: sys/capability.h: No such file or > directory >20 | #include > | ^~ > > Looks like selftests/bpf/cap_helpers.c avoids a similar-ish issue? Not sure if > there's a better way or if I should do something like that? > > I assume it is not possible to add deps like libcap-dev to the test harness > somehow? Ah sorry, CI builder was missing the libcap-devel package. I can't retrigger the CI easily, unfortunately, could you repost? (without the 24h wait)
Re: [PATCH] selftests: Add _GNU_SOURCE definition when including kselftest_harness.h
On Wed, 8 May 2024 10:55:05 +0800 Tao Su wrote: > Back to commit 38c957f07038, I don't see any advantage in using LINE_MAX. > Can we use a fixed value instead of LINE_MAX? E.g., 1024, 2048. Then we > just need to revert commit 809216233555. SGTM, FWIW. The print is printing a test summary line, printing more than 1k seems rather unreasonable. Other facilities, like TH_LOG(), should be used for displaying longer info.
Re: WARNING: CPU: 1 PID: 1 at net/core/netpoll.c:370 netpoll_send_skb+0x1fc/0x20c at boot when netconsole is enabled (kernel v6.9-rc5, v6.8.7, sungem, PowerMac G4 DP)
On Wed, 8 May 2024 10:55:05 +0200 Erhard Furtner wrote: > I could do that with the explanation you stated. But should any > further questions arise in this process I would also lack the > technical background to deal with them. ;) Alright, submitted :) > I also noticed a similar #ifdef CONFIG_NET_POLL_CONTROLLER logic shows up in > many network drivers, e.g. net/ethernet/realtek/8139too.c: > > #ifdef CONFIG_NET_POLL_CONTROLLER > static void rtl8139_poll_controller(struct net_device *dev); > #endif > [...] > #ifdef CONFIG_NET_POLL_CONTROLLER > /* > * Polling receive - used by netconsole and other diagnostic tools > * to allow network i/o with interrupts disabled. > */ > static void rtl8139_poll_controller(struct net_device *dev) > { > struct rtl8139_private *tp = netdev_priv(dev); > const int irq = tp->pci_dev->irq; > > disable_irq_nosync(irq); > rtl8139_interrupt(irq, dev); > enable_irq(irq); > } > #endif > [...] > #ifdef CONFIG_NET_POLL_CONTROLLER > .ndo_poll_controller= rtl8139_poll_controller, > #endif > > > Should it be removed here too? This would be more cards I can test. > So far I only see this on my G4 and I think something similar on an > old Pentium4 box I no longer have. That one looks legit. Note the _nosync() which solves the immediate IRQ masking / deadlock issue. And the rtl8139_interrupt() function actually does the packet cleanup in case of 8139too.
Re: [PATCH net-next v9 0/3] net: gro: remove network_header use, move p->{flush/flush_id} calculations to L4
On Tue, 7 May 2024 18:23:45 +0200 Richard Gobert wrote: > The cb fields network_offset and inner_network_offset are used instead of > skb->network_header throughout GRO. > > These fields are then leveraged in the next commit to remove flush_id state > from napi_gro_cb, and stateful code in {ipv6,inet}_gro_receive which may be > unnecessarily complicated due to encapsulation support in GRO. These fields > are checked in L4 instead. > > 3rd patch adds tests for different flush_id flows in GRO. Doesn't apply to either tree, please rebase. -- pw-bot: cr
Re: [PATCH net-next 0/2] gve: Minor cleanups
On Fri, 03 May 2024 21:31:25 +0100 Simon Horman wrote: > This short patchset provides two minor cleanups for the gve driver. > > These were found by tooling as mentioned in each patch, > and otherwise by inspection. > > No change in run time behaviour is intended. > Each patch is compile tested only. Looks like it conflicts now, please rebase -- pw-bot: cr
Re: [PATCH net-next] netdevice: define and allocate _device _properly_
On Tue, 7 May 2024 14:39:37 +0200 Alexander Lobakin wrote: > There are several instances of the structure embedded into other > structures, but also there's ongoing effort to remove them and we > could in the meantime declare _device properly. Is there a reason you're reposting this before that effort is completed? The warnings this adds come from sparse and you think they should be ignored? TBH since Breno is doing the heavy lifting of changing the embedders it'd seem more fair to me if he got to send this at the end. Or at least, you know, got a mention or a CC.
Re: [PATCH net-next v2] selftest: epoll_busy_poll: epoll busy poll tests
On Mon, 6 May 2024 18:40:00 -0700 Joe Damato wrote: > Ah, sorry -- this is because I had assumed the test would run without > CAP_NET_ADMIN, but since: > > epoll_busy_poll.c:204:test_set_invalid:Expected -1 (-1) == ret (0) > > succeeds (ret = 0), clearly I am mistaken. Sorry about that. > > I think I'll spin up a v3 and I'll add a test with and without > CAP_NET_ADMIN to check both cases, which would probably be better anyway. FWIW the tests run a in separate process from the harness, so it may be possible to drop privileges inside the test, without affecting other test cases. But I've never done it myself, so not sure how easy it is to do in practice..
Re: [PATCH net-next v2] selftest: epoll_busy_poll: epoll busy poll tests
On Mon, 6 May 2024 20:53:22 + Joe Damato wrote: > Add a simple test for the epoll busy poll ioctls, using the kernel > selftest harness. > > This test ensures that the ioctls have the expected return codes and > that the kernel properly gets and sets epoll busy poll parameters. > > The test can be expanded in the future to do real busy polling (provided > another machine to act as the client is available). Hm, we get: # timeout set to 3600 # selftests: net: epoll_busy_poll # TAP version 13 # 1..5 # # Starting 5 tests from 2 test cases. # # RUN invalid_fd.test_invalid_fd ... # #OK invalid_fd.test_invalid_fd # ok 1 invalid_fd.test_invalid_fd # # RUN epoll_busy_poll.test_get_params ... # #OK epoll_busy_poll.test_get_params # ok 2 epoll_busy_poll.test_get_params # # RUN epoll_busy_poll.test_set_invalid ... # # epoll_busy_poll.c:204:test_set_invalid:Expected -1 (-1) == ret (0) # # epoll_busy_poll.c:205:test_set_invalid:EPIOCSPARAMS should error busy_poll_budget > NAPI_POLL_WEIGHT # # epoll_busy_poll.c:207:test_set_invalid:Expected EPERM (1) == errno (22) # # epoll_busy_poll.c:208:test_set_invalid:EPIOCSPARAMS errno should be EPERM busy_poll_budget > NAPI_POLL_WEIGHT # # test_set_invalid: Test failed # # FAIL epoll_busy_poll.test_set_invalid # not ok 3 epoll_busy_poll.test_set_invalid # # RUN epoll_busy_poll.test_set_and_get_valid ... # #OK epoll_busy_poll.test_set_and_get_valid # ok 4 epoll_busy_poll.test_set_and_get_valid # # RUN epoll_busy_poll.test_invalid_ioctl ... # #OK epoll_busy_poll.test_invalid_ioctl # ok 5 epoll_busy_poll.test_invalid_ioctl https://netdev-3.bots.linux.dev/vmksft-net/results/584001/98-epoll-busy-poll/stdout
Re: WARNING: CPU: 1 PID: 1 at net/core/netpoll.c:370 netpoll_send_skb+0x1fc/0x20c at boot when netconsole is enabled (kernel v6.9-rc5, v6.8.7, sungem, PowerMac G4 DP)
On Tue, 7 May 2024 02:42:58 +0200 Erhard Furtner wrote: > And indeed without gem_poll_controller() I don't hit the "WARNING: CPU: 1 > PID: 1 at net/core/netpoll.c:370 netpoll_send_skb+0x1fc/0x20c" and "WARNING: > CPU: 1 PID: 1 at kernel/locking/irqflag-debug.c:10 > warn_bogus_irq_restore+0x30/0x44" or the according lockdep bug at boot! > > Re-booted the machine about 20 times without anything suspicious showing up > in the dmesg. With the unpatched kernel I got the WARNING at the 2nd reboot. Excellent! Do you want to submit that as an official patch? The explanation is that we can't call disable_irq() from atomic context (which which netpoll runs). But the callback is no longer necessary as we can depend on NAPI to do the polling these days. > What I still get with 'modprobe -v dev_addr_lists_test', even with > gem_poll_controller() removed is: > > [...] > KTAP version 1 > 1..1 > KTAP version 1 > # Subtest: dev-addr-list-test > # module: dev_addr_lists_test > 1..6 > > > WARNING: kunit_try_catch/1770 still has locks held! > 6.9.0-rc6-PMacG4-dirty #5 Tainted: GWN > > 1 lock held by kunit_try_catch/1770: > #0: c0dbfce4 (rtnl_mutex){}-{3:3}, at: dev_addr_test_init+0xbc/0xc8 > [dev_addr_lists_test] I think that's fixed in net-next.
Re: [PATCH net-next v3 2/2] selftests/net: add ICMP unreachable over IPsec tunnel
On Mon, 6 May 2024 17:37:54 +0200 Antony Antony wrote: > This appears to be an error from the v2 run, which was sent yesterday. > The v3 patch should have superseded it. > > The branch net-dev-testing/net-next-2024-05-06--12-00 contains the v2 patch. > I wonder if net-dev testing recognized v3 patch. You're right! I guess the pw-bot didn't discard v2 because of the capitalization change in the subject of the cover letter. > git diff net-next-2024-05-06--12-00 net-next-2024-05-06--03-00 > ./tools/testing/selftests/net/xfrm_state.sh > is missing the expected one line diff in IFS. > > > # ./xfrm_state.sh: line 91: test_: command not found > > # TEST: unreachable_gw_ipv6IPv6 unreachable from IPsec gateway s2 [ FAIL ] > > # ./xfrm_state.sh: line 91: test_: command not found > > # TEST: mtu_ipv6_r2IPv6 MTU exceeded from ESP router r2 [ FAIL ] > > # ./xfrm_state.sh: line 91: test_: command not found > > # TEST: mtu_ipv6_r3IPv6 MTU exceeded router r3 [ FAIL ] > > not ok 1 selftests: net: xfrm_state.sh # exit=1 > > I suspect there is another another issue with > tools/testing/selftests/net/config . It does not appear to support nftables > match for ESP. Which this script assumes. > > # ip netns exec ns_r2-39oUmE nft add rule inet filter FORWARD counter ip > protocol esp counter log accept > # > # Error: Could not process rule: No such file or directory > # add rule inet filter FORWARD counter ip protocol esp counter log accept > # ^^ > > I learning vng also. I will send v4 with change to config, then I hope the > test runner will pick up the latest patch. ️
Re: WARNING: CPU: 1 PID: 1 at net/core/netpoll.c:370 netpoll_send_skb+0x1fc/0x20c at boot when netconsole is enabled (kernel v6.9-rc5, v6.8.7, sungem, PowerMac G4 DP)
On Sun, 5 May 2024 23:27:13 +0200 Erhard Furtner wrote: > > On Sun, 28 Apr 2024 12:53:06 +0200 Erhard Furtner wrote: > > > With netconsole enabled I get this "WARNING: CPU: 1 PID: 1 at > > > net/core/netpoll.c:370 netpoll_send_skb+0x1fc/0x20c" and "WARNING: > > > CPU: 1 PID: 1 at kernel/locking/irqflag-debug.c:10 > > > warn_bogus_irq_restore+0x30/0x44" at boot on my PowerMac G4 DP. > > > Happens more often than not (6-7 out of 10 times booting): > > > > Could you try with LOCKDEP enabled? > > I wonder if irqs_disabled() behaves differently than we expect. > > Ok, after a few tries I got a "BUG: spinlock wrong CPU on CPU#0, swapper/0/1" > LOCKDEP hit. But this does not happen every time when I get the netpoll_send > WARNING: Oh, can you try deleting the gem_poll_controller() function? Unhook it from ndo_poll_controller and remove it completely.
Re: [PATCH net-next v3 2/2] selftests/net: add ICMP unreachable over IPsec tunnel
On Mon, 6 May 2024 10:05:54 +0200 Antony Antony wrote: > Add IPsec tunnel, aka xfrm state, tests with ICMP flags enabled. > IPv4 and IPv6, unreachable tests over xfrm/IPsec tunnels, > xfrm SA with "flag icmp" set. Doesn't seem to work: # selftests: net: xfrm_state.sh # ./xfrm_state.sh: line 91: test_: command not found # TEST: unreachable_ipv4IPv6 unreachable from router r3 [ FAIL ] # ./xfrm_state.sh: line 91: test_: command not found # TEST: unreachable_gw_ipv6IPv6 unreachable from IPsec gateway s2 [ FAIL ] # ./xfrm_state.sh: line 91: test_: command not found # TEST: mtu_ipv6_r2IPv6 MTU exceeded from ESP router r2 [ FAIL ] # ./xfrm_state.sh: line 91: test_: command not found # TEST: mtu_ipv6_r3IPv6 MTU exceeded router r3 [ FAIL ] not ok 1 selftests: net: xfrm_state.sh # exit=1
Re: [PATCH net-next] selftest: epoll_busy_poll: epoll busy poll tests
On Fri, 3 May 2024 16:09:45 -0700 Joe Damato wrote: > > "GEN" is for files which are built for other tests to use. > > IOW unless there's also a wrapper script under TEST_PROGS > > (or the C code is itself under TEST_PROGS) this test won't > > be executed by most CIs. > > Ah, I see. OK. > > If I decided to go with the kselftest_harness as mentioned below, I'd need > to include a wrapper script to run the binary with the right cmd line > arg(s) and put that in TEST_PROGS? harness or not, the only two real requirements for including in TEST_PROGS directly is to: - return non-zero exit code on failure; and - not require any command line arguments.
Re: [PATCH net-next] selftest: epoll_busy_poll: epoll busy poll tests
On Thu, 2 May 2024 21:20:11 + Joe Damato wrote: > --- a/tools/testing/selftests/net/Makefile > +++ b/tools/testing/selftests/net/Makefile > @@ -84,6 +84,7 @@ TEST_GEN_FILES += sctp_hello > TEST_GEN_FILES += csum > TEST_GEN_FILES += ip_local_port_range > TEST_GEN_FILES += bind_wildcard > +TEST_GEN_FILES += epoll_busy_poll "GEN" is for files which are built for other tests to use. IOW unless there's also a wrapper script under TEST_PROGS (or the C code is itself under TEST_PROGS) this test won't be executed by most CIs. FWIW here's how we run the tests in our CI upstream CI: https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style > TEST_PROGS += test_vxlan_mdb.sh > TEST_PROGS += test_bridge_neigh_suppress.sh > TEST_PROGS += test_vxlan_nolocalbypass.sh > +static void do_simple_test(void) > +{ > + int fd; > + > + fd = epoll_create1(0); > + if (fd == -1) > + error(1, errno, "epoll_create"); > + > + do_simple_test_invalid_fd(); > + do_simple_test_invalid_ioctl(fd); > + do_simple_test_get_params(fd); > + do_simple_test_set_invalid(fd); > + do_simple_test_set_and_get_valid(fd); You don't want to use the kselftest_harness for this? No strong preference here, but seems like you could pop the epoll_create1 into a FIXTURE() and then the test cases into TEST_F() and we'd get the KTAP output formatting, ability to run the tests selectively etc. for free. tools/testing/selftests/net/tap.c is probably a good example to take a look at
Re: [PATCH v1 10/12] sfc: falcon: Make I2C terminology more inclusive
On Tue, 30 Apr 2024 17:38:09 + Easwar Hariharan wrote: > I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced "master/slave" > with more appropriate terms. Inspired by and following on to Wolfram's > series to fix drivers/i2c/[1], fix the terminology for users of > I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists > in the specification. > > Compile tested, no functionality changes intended FWIW we're assuming someone (Wolfram?) will take all of these, instead of area maintainers picking them individually. Please let us know if that's incorrect.
Re: [PATCH v1 10/12] sfc: falcon: Make I2C terminology more inclusive
On Tue, 30 Apr 2024 17:38:09 + Easwar Hariharan wrote: > I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced "master/slave" > with more appropriate terms. Inspired by and following on to Wolfram's > series to fix drivers/i2c/[1], fix the terminology for users of > I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists > in the specification. > > Compile tested, no functionality changes intended FWIW we're assuming someone (Wolfram?) will take all of these, instead of area maintainers picking them individually. Please let us know if that's incorrect.
Re: [PATCH v1 10/12] sfc: falcon: Make I2C terminology more inclusive
On Tue, 30 Apr 2024 17:38:09 + Easwar Hariharan wrote: > I2C v7, SMBus 3.2, and I3C 1.1.1 specifications have replaced "master/slave" > with more appropriate terms. Inspired by and following on to Wolfram's > series to fix drivers/i2c/[1], fix the terminology for users of > I2C_ALGOBIT bitbanging interface, now that the approved verbiage exists > in the specification. > > Compile tested, no functionality changes intended FWIW we're assuming someone (Wolfram?) will take all of these, instead of area maintainers picking them individually. Please let us know if that's incorrect.
Re: [PATCH net-next] selftests: drv-net: add checksum tests
On Wed, 01 May 2024 21:38:24 -0400 Willem de Bruijn wrote: > A few more may be in scope eventually: toeplitz, udpgso_bench, gro, > so_txtime. Move them on a case-by-case basis? If you plan to use them all in hw tests - no preference, you can move them all in one fell swoop if you prefer. > > Should I send a fix for that? > > Please do. I did not grasp your suggestion well enough to take a > stab. I may have already spotted the zero conversion test returning > success, while explicit logging of the stderr output shows otherwise. On it..
Re: [PATCH net-next] selftests: drv-net: add checksum tests
Great! I run it on a couple of older machines. mlx5: TAP version 13 1..1 # timeout set to 0 # selftests: drivers/net/hw: csum.py # KTAP version 1 # 1..12 # ok 1 csum.ipv4_rx_tcp # SKIP Test requires IPv4 connectivity # ok 2 csum.ipv4_rx_tcp_invalid # SKIP Test requires IPv4 connectivity # ok 3 csum.ipv4_rx_udp # SKIP Test requires IPv4 connectivity # ok 4 csum.ipv4_rx_udp_invalid # SKIP Test requires IPv4 connectivity # ok 5 csum.ipv4_tx_udp_csum_offload # SKIP Test requires IPv4 connectivity # ok 6 csum.ipv4_tx_udp_zero_checksum # SKIP Test requires IPv4 connectivity # ok 7 csum.ipv6_rx_tcp # ok 8 csum.ipv6_rx_tcp_invalid # ok 9 csum.ipv6_rx_udp # ok 10 csum.ipv6_rx_udp_invalid # ok 11 csum.ipv6_tx_udp_csum_offload # ok 12 csum.ipv6_tx_udp_zero_checksum # # Totals: pass:6 fail:0 xfail:0 xpass:0 skip:6 error:0 ok 1 selftests: drivers/net/hw: csum.py bnxt: TAP version 13 1..1 # timeout set to 0 # selftests: drivers/net/hw: csum.py # KTAP version 1 # 1..12 # ok 1 csum.ipv4_rx_tcp # SKIP Test requires IPv4 connectivity # ok 2 csum.ipv4_rx_tcp_invalid # SKIP Test requires IPv4 connectivity # ok 3 csum.ipv4_rx_udp # SKIP Test requires IPv4 connectivity # ok 4 csum.ipv4_rx_udp_invalid # SKIP Test requires IPv4 connectivity # ok 5 csum.ipv4_tx_udp_csum_offload # SKIP Test requires IPv4 connectivity # ok 6 csum.ipv4_tx_udp_zero_checksum # SKIP Test requires IPv4 connectivity # ok 7 csum.ipv6_rx_tcp # ok 8 csum.ipv6_rx_tcp_invalid # ok 9 csum.ipv6_rx_udp # ok 10 csum.ipv6_rx_udp_invalid # ok 11 csum.ipv6_tx_udp_csum_offload # SKIP Test requires tx checksum offload on eth0 # ok 12 csum.ipv6_tx_udp_zero_checksum # SKIP Test requires tx checksum offload on eth0 # # Totals: pass:4 fail:0 xfail:0 xpass:0 skip:8 error:0 ok 1 selftests: drivers/net/hw: csum.py On Wed, 1 May 2024 14:51:34 -0400 Willem de Bruijn wrote: > Run tools/testing/selftest/net/csum.c as part of drv-net. > This binary covers multiple scenarios, based on arguments given, > for both IPv4 and IPv6: The use of csum.c is the only real concern I have. Could you move it to net/lib? I made net/lib into an automatically included target in commit b86761ff6374 ("selftests: net: add scaffolding for Netlink tests in Python"). It has a makefile like any selftest directory, so you should be able to do a simple move and minor path adjustments. Without this if someone builds and deploys just the drivers/net{,/hw} targets the csum binary won't be there :( We could auto-include all of net but using the lib target felt a little cleaner. > - Accept UDP correct checksum > - Detect UDP invalid checksum > - Accept TCP correct checksum > - Detect TCP invalid checksum > > - Transmit UDP: basic checksum offload > - Transmit UDP: zero checksum conversion > > The test direction is reversed between receive and transmit tests, so > that the NIC under test is always the local machine. > > In total this adds up to 12 testcases, with more to follow. For > conciseness, I replaced individual functions with a function factory. > It saves a lot of boilerplate, but is a little harder to follow, so > partially here as a point for discussion. LGTM, FWIW, but let's hear if anyone feels it's too magical. > Warning that for now transmit errors are not detected, as for those > the receiver runs remotely and failures with bkg are ignored. Should I send a fix for that?
Re: [PATCH net-next v8] net/ipv4: add tracepoint for icmp_send
On Mon, 29 Apr 2024 17:15:57 +0800 (CST) xu.xi...@zte.com.cn wrote: > Introduce a tracepoint for icmp_send, which can help users to get more > detail information conveniently when icmp abnormal events happen. Rebase please, it doesn't apply. And please put the change log under the --- delimiter. -- pw-bot: cr
Re: [PATCH v3 0/9] Fix Kselftest's vfork() side effects
On Tue, 30 Apr 2024 15:54:38 +0200 Mickaël Salaün wrote: > Jakub, can you please review it? I looked thru it. I don't have the cycles to investigate and suggest a better approach but the sprinkling of mmaps(), if nothing else, feels a bit band-aid-y 路️
Re: [Intel-wired-lan] [RFC net-next (what uAPI?) ice: add support for more than 16 RSS queues for VF
On Fri, 26 Apr 2024 15:22:02 +0200 Przemek Kitszel wrote: > ## devlink resources (with current API) > `devlink resource` is compelling, partially given the name sounds like a > perfect match. But when we dig just a little bit, the current Path+sizes > (min,max,step) is totally off to what is the most elegant picture of the > situation. In order to fit into existing uAPI, I would need to register > VFs as PF's resource, then GLOBAL LUT and PF LUT as a sub resource to > that (each VF gets two entries under it; plus two additional ones for > PF) I don't like it, I also feel like there is not that much use of > current resources API (it's not natural to use it for distribution, only > for limitation). Can you share more on how that would look like? From the description it does not sound so bad. Maybe with some CLI / UI changes it will be fine? > ## devlink resources (with extended API) > It is possible to extend current `devlink resource` so instead of only > Path+size, there would be also Path+Owner option to use. > The default state for ice driver would be that PFs owns PF LUTs, GLOBAL > LUTs are all free. > > example proposed flow to assign a GLOBAL LUT to VF0 and PF LUT to VF1: > pf=:03:00.0 # likely more meaningful than VSI idx, but open for > vf0=:03:00.1 # suggestions > vf1=:03:00.2 > devlink resource set pci/$pf path /lut/lut_table_512 owner $pf > devlink resource set pci/$pf path /lut/lut_table_2048 owner free > devlink resource set pci/$pf path /lut/lut_table_512 owner $vf0 > devlink resource set pci/$pf path /lut/lut_table_2048 owner $vf1 Don't we want some level of over-subscription to be allowed?
Re: WARNING: CPU: 1 PID: 1 at net/core/netpoll.c:370 netpoll_send_skb+0x1fc/0x20c at boot when netconsole is enabled (kernel v6.9-rc5, v6.8.7, sungem, PowerMac G4 DP)
On Sun, 28 Apr 2024 12:53:06 +0200 Erhard Furtner wrote: > With netconsole enabled I get this "WARNING: CPU: 1 PID: 1 at > net/core/netpoll.c:370 netpoll_send_skb+0x1fc/0x20c" and "WARNING: > CPU: 1 PID: 1 at kernel/locking/irqflag-debug.c:10 > warn_bogus_irq_restore+0x30/0x44" at boot on my PowerMac G4 DP. > Happens more often than not (6-7 out of 10 times booting): Could you try with LOCKDEP enabled? I wonder if irqs_disabled() behaves differently than we expect.
Re: [PATCH v2 4/9] selftests/harness: Fix interleaved scheduling leading to race conditions
On Mon, 29 Apr 2024 08:52:36 -0700 Kees Cook wrote: > > +/* Wait for the child process to end but without sharing memory mapping. */ > > +static pid_t __attribute__((__unused__)) clone3_vfork(void) > > Why "unused"? Right, static inline is enough
Re: [PATCH net-next 6/6] selftests: drv-net-hw: add test for memory allocation failures with page pool
On Mon, 29 Apr 2024 11:02:51 -0400 Willem de Bruijn wrote: > The only interesting points so far are the use of deploy (which I > assume you have on some internal patch already) Yup, they need a touch more cleaning up but the PSP tests use it. > and that with bkg would not fail the test if the background process > exits with error. Ah, that's a bug, yes. We should record the value of fail from the constructor and use it in __exit__().
Re: [PATCH net-next 0/6] selftests: net: page_poll allocation error injection
On Mon, 29 Apr 2024 17:01:55 +0200 Andrew Lunn wrote: > > I'm not super happy with the traffic generation using iperf3, > > my initial approach was to use mausezahn. But it turned out to be > > 5x slower in terms of PPS. Hopefully this is good enough for now. > > How important is PPS? In order to get 'Maintained' status, automotive > vendors are going to want to test their 10Mbps T1 links. s/Maintained/Supported/ ? PPS isn't important in itself, that said, I wanted to set a floor to make sure that the failure path is actually well exercised. Some drivers may be doing internal recycling or whatever other magic, which would make them barely call the page_pool alloc. Even though this is not a performance tests the check is based on expected perf. My thinking is that once we have some data points about various system we can abstract the perf expectations a bit more systematically than if speed < 10GE: pps //= 10
Re: [PATCH net-next 6/6] selftests: drv-net-hw: add test for memory allocation failures with page pool
On Mon, 29 Apr 2024 17:12:29 +0200 Andrew Lunn wrote: > > # ./drivers/net/hw/pp_alloc_fail.py > > KTAP version 1 > > 1..1 > > # ethtool -G change retval: success > > ok 1 pp_alloc_fail.test_pp_alloc > > # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0 > > If i'm reading the traffic generator correctly, this test runs for 24 > hours. Do we want some sort of warning here about the test duration? > Some sort of alive indication very minute? That's just the max value for the time param. The generator is stopped / killed after we go thru the test steps.
Re: [PATCH net-next 6/6] selftests: drv-net-hw: add test for memory allocation failures with page pool
On Sat, 27 Apr 2024 09:49:28 -0400 Willem de Bruijn wrote: > Eventually probably want a more generic fault injection class. > > And for both fault injection and background traffic the with object > construct to ensure cleanup in all cases. > > Maybe even the same for ethtool, as ip and ethtool config changes that > need to be reverted to original state will be common. Agreed, the nice way of wrapping all that has not revealed itself to me yet. When we discussed it with Petr a while back he was suggesting "with", and I was thinking of creating an object with test as the parent. The with is nicer but here we'd end up doing: with a(): # some code with b(): # more code with c(): # check traffic which offends my sensibilities. There are many options, hard to say which one is best without having a bunch of tests to convert as a litmus test :S So I stuck to "finally"
[PATCH net-next v2 6/6] selftests: drv-net-hw: add test for memory allocation failures with page pool
Bugs in memory allocation failure paths are quite common. Add a test exercising those paths based on qstat and page pool failure hook. Running on bnxt: # ./drivers/net/hw/pp_alloc_fail.py KTAP version 1 1..1 # ethtool -G change retval: success ok 1 pp_alloc_fail.test_pp_alloc # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0 I initially wrote this test to validate commit be43b7489a3c ("net/mlx5e: RX, Fix page_pool allocation failure recovery for striding rq") but mlx5 still doesn't have qstat. So I run it on bnxt, and while bnxt survives I found the problem fixed in commit 730117730709 ("eth: bnxt: fix counting packets discarded due to OOM and netpoll"). Reviewed-by: Willem de Bruijn Signed-off-by: Jakub Kicinski --- .../testing/selftests/drivers/net/hw/Makefile | 1 + .../selftests/drivers/net/hw/pp_alloc_fail.py | 129 ++ tools/testing/selftests/net/lib/py/ksft.py| 4 + 3 files changed, 134 insertions(+) create mode 100755 tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile index 95f32158b095..1dd732855d76 100644 --- a/tools/testing/selftests/drivers/net/hw/Makefile +++ b/tools/testing/selftests/drivers/net/hw/Makefile @@ -9,6 +9,7 @@ TEST_PROGS = \ hw_stats_l3.sh \ hw_stats_l3_gre.sh \ loopback.sh \ + pp_alloc_fail.py \ # TEST_FILES := \ diff --git a/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py b/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py new file mode 100755 index ..026d98976c35 --- /dev/null +++ b/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0 + +import time +import os +from lib.py import ksft_run, ksft_exit, ksft_pr +from lib.py import KsftSkipEx, KsftFailEx +from lib.py import NetdevFamily, NlError +from lib.py import NetDrvEpEnv +from lib.py import cmd, tool, GenerateTraffic + + +def _write_fail_config(config): +for key, value in config.items(): +with open("/sys/kernel/debug/fail_function/" + key, "w") as fp: +fp.write(str(value) + "\n") + + +def _enable_pp_allocation_fail(): +if not os.path.exists("/sys/kernel/debug/fail_function"): +raise KsftSkipEx("Kernel built without function error injection (or DebugFS)") + +if not os.path.exists("/sys/kernel/debug/fail_function/page_pool_alloc_pages"): +with open("/sys/kernel/debug/fail_function/inject", "w") as fp: +fp.write("page_pool_alloc_pages\n") + +_write_fail_config({ +"verbose": 0, +"interval": 511, +"probability": 100, +"times": -1, +}) + + +def _disable_pp_allocation_fail(): +if not os.path.exists("/sys/kernel/debug/fail_function"): +return + +if os.path.exists("/sys/kernel/debug/fail_function/page_pool_alloc_pages"): +with open("/sys/kernel/debug/fail_function/inject", "w") as fp: +fp.write("\n") + +_write_fail_config({ +"probability": 0, +"times": 0, +}) + + +def test_pp_alloc(cfg, netdevnl): +def get_stats(): +return netdevnl.qstats_get({"ifindex": cfg.ifindex}, dump=True)[0] + +def check_traffic_flowing(): +stat1 = get_stats() +time.sleep(1) +stat2 = get_stats() +if stat2['rx-packets'] - stat1['rx-packets'] < 15000: +raise KsftFailEx("Traffic seems low:", stat2['rx-packets'] - stat1['rx-packets']) + + +try: +stats = get_stats() +except NlError as e: +if e.nl_msg.error == -95: +stats = {} +else: +raise +if 'rx-alloc-fail' not in stats: +raise KsftSkipEx("Driver does not report 'rx-alloc-fail' via qstats") + +set_g = False +traffic = None +try: +traffic = GenerateTraffic(cfg) + +check_traffic_flowing() + +_enable_pp_allocation_fail() + +s1 = get_stats() +time.sleep(3) +s2 = get_stats() + +if s2['rx-alloc-fail'] - s1['rx-alloc-fail'] < 1: +raise KsftSkipEx("Allocation failures not increasing") +if s2['rx-alloc-fail'] - s1['rx-alloc-fail'] < 100: +raise KsftSkipEx("Allocation increasing too slowly", s2['rx-alloc-fail'] - s1['rx-alloc-fail'], + "packets:", s2['rx-packets'] - s1['rx-packets']) + +# Basic failures are fine, try to wobble some settings to catch extra failures +check_traffic_flowing() +g = tool("ethtool", "-g " + cfg.ifname, json=True)[0] +
[PATCH net-next v2 5/6] selftests: drv-net: support generating iperf3 load
While we are not very interested in testing performance it's useful to be able to generate a lot of traffic. iperf is the simplest way of getting relatively high PPS. Reviewed-by: Willem de Bruijn Signed-off-by: Jakub Kicinski --- v2: - fix import order in __init__.py --- .../selftests/drivers/net/lib/py/__init__.py | 1 + .../selftests/drivers/net/lib/py/load.py | 41 +++ 2 files changed, 42 insertions(+) create mode 100644 tools/testing/selftests/drivers/net/lib/py/load.py diff --git a/tools/testing/selftests/drivers/net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/lib/py/__init__.py index 4789c1a4282d..401e70f7f136 100644 --- a/tools/testing/selftests/drivers/net/lib/py/__init__.py +++ b/tools/testing/selftests/drivers/net/lib/py/__init__.py @@ -15,4 +15,5 @@ KSFT_DIR = (Path(__file__).parent / "../../../..").resolve() sys.exit(4) from .env import * +from .load import * from .remote import Remote diff --git a/tools/testing/selftests/drivers/net/lib/py/load.py b/tools/testing/selftests/drivers/net/lib/py/load.py new file mode 100644 index ..abdb677bdb1c --- /dev/null +++ b/tools/testing/selftests/drivers/net/lib/py/load.py @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: GPL-2.0 + +import time + +from lib.py import ksft_pr, cmd, ip, rand_port, wait_port_listen + +class GenerateTraffic: +def __init__(self, env): +env.require_cmd("iperf3", remote=True) + +self.env = env + +port = rand_port() +self._iperf_server = cmd(f"iperf3 -s -p {port}", background=True) +wait_port_listen(port) +time.sleep(0.1) +self._iperf_client = cmd(f"iperf3 -c {env.addr} -P 16 -p {port} -t 86400", + background=True, host=env.remote) + +# Wait for traffic to ramp up +pkt = ip("-s link show dev " + env.ifname, json=True)[0]["stats64"]["rx"]["packets"] +for _ in range(50): +time.sleep(0.1) +now = ip("-s link show dev " + env.ifname, json=True)[0]["stats64"]["rx"]["packets"] +if now - pkt > 1000: +return +pkt = now +self.stop(verbose=True) +raise Exception("iperf3 traffic did not ramp up") + +def stop(self, verbose=None): +self._iperf_client.process(terminate=True) +if verbose: +ksft_pr(">> Client:") +ksft_pr(self._iperf_client.stdout) +ksft_pr(self._iperf_client.stderr) +self._iperf_server.process(terminate=True) +if verbose: +ksft_pr(">> Server:") +ksft_pr(self._iperf_server.stdout) +ksft_pr(self._iperf_server.stderr) -- 2.44.0
[PATCH net-next v2 4/6] selftests: net: py: avoid all ports < 10k
When picking TCP ports to use, avoid all below 10k. This should lower the chance of collision or running afoul whatever random policies may be on the host. Reviewed-by: Willem de Bruijn Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/lib/py/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index 4930a90a64ea..b57d467afd0f 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -77,7 +77,7 @@ import time """ Get unprivileged port, for now just random, one day we may decide to check if used. """ -return random.randint(1024, 65535) +return random.randint(1, 65535) def wait_port_listen(port, proto="tcp", ns=None, host=None, sleep=0.005, deadline=5): -- 2.44.0
[PATCH net-next v2 3/6] selftests: net: py: extract tool logic
The main use of the ip() wrapper over cmd() is that it can parse JSON. cmd("ip -j link show") will return stdout as a string, and test has to call json.loads(). With ip("link show", json=True) the return value will be already parsed. More tools (ethtool, bpftool etc.) support the --json switch. To avoid having to wrap all of them individually create a tool() helper. Switch from -j to --json (for ethtool). While at it consume the netns attribute at the ip() level. Reviewed-by: Willem de Bruijn Signed-off-by: Jakub Kicinski --- v2: - use consistent quote type - use format string to force string conversion of NetNS class --- tools/testing/selftests/net/lib/py/utils.py | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index d3715e6c21f2..4930a90a64ea 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -56,10 +56,10 @@ import time return self.process(terminate=self.terminate) -def ip(args, json=None, ns=None, host=None): -cmd_str = "ip " +def tool(name, args, json=None, ns=None, host=None): +cmd_str = name + ' ' if json: -cmd_str += '-j ' +cmd_str += '--json ' cmd_str += args cmd_obj = cmd(cmd_str, ns=ns, host=host) if json: @@ -67,6 +67,12 @@ import time return cmd_obj +def ip(args, json=None, ns=None, host=None): +if ns: +args = f'-netns {ns} ' + args +return tool('ip', args, json=json, host=host) + + def rand_port(): """ Get unprivileged port, for now just random, one day we may decide to check if used. -- 2.44.0
[PATCH net-next v2 2/6] selftests: drv-net-hw: support using Python from net hw tests
We created a separate directory for HW-only tests, recently. Glue in the Python test library there, Python is a bit annoying when it comes to using library code located "lower" in the directory structure. Reuse the Env class, but let tests require non-nsim setup. Reviewed-by: Willem de Bruijn Signed-off-by: Jakub Kicinski --- tools/testing/selftests/Makefile | 2 +- tools/testing/selftests/drivers/net/hw/Makefile | 1 + .../selftests/drivers/net/hw/lib/py/__init__.py | 16 .../testing/selftests/drivers/net/lib/py/env.py | 10 -- 4 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/drivers/net/hw/lib/py/__init__.py diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 2c940e9c4ced..9039f3709aff 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -119,7 +119,7 @@ TARGETS_HOTPLUG = cpu-hotplug TARGETS_HOTPLUG += memory-hotplug # Networking tests want the net/lib target, include it automatically -ifneq ($(filter net drivers/net,$(TARGETS)),) +ifneq ($(filter net drivers/net drivers/net/hw,$(TARGETS)),) ifeq ($(filter net/lib,$(TARGETS)),) INSTALL_DEP_TARGETS := net/lib endif diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile index 2259a39a70ed..95f32158b095 100644 --- a/tools/testing/selftests/drivers/net/hw/Makefile +++ b/tools/testing/selftests/drivers/net/hw/Makefile @@ -16,6 +16,7 @@ TEST_FILES := \ # TEST_INCLUDES := \ + $(wildcard lib/py/*.py ../lib/py/*.py) \ ../../../net/lib.sh \ ../../../net/forwarding/lib.sh \ ../../../net/forwarding/ipip_lib.sh \ diff --git a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py new file mode 100644 index ..b582885786f5 --- /dev/null +++ b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py @@ -0,0 +1,16 @@ +# SPDX-License-Identifier: GPL-2.0 + +import sys +from pathlib import Path + +KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve() + +try: +sys.path.append(KSFT_DIR.as_posix()) +from net.lib.py import * +from drivers.net.lib.py import * +except ModuleNotFoundError as e: +ksft_pr("Failed importing `net` library from kernel sources") +ksft_pr(str(e)) +ktap_result(True, comment="SKIP") +sys.exit(4) diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py index e2ab637e56dc..5c8f695b2536 100644 --- a/tools/testing/selftests/drivers/net/lib/py/env.py +++ b/tools/testing/selftests/drivers/net/lib/py/env.py @@ -2,7 +2,7 @@ import os from pathlib import Path -from lib.py import KsftSkipEx +from lib.py import KsftSkipEx, KsftXfailEx from lib.py import cmd, ip from lib.py import NetNS, NetdevSimDev from .remote import Remote @@ -76,7 +76,7 @@ from .remote import Remote nsim_v4_pfx = "192.0.2." nsim_v6_pfx = "2001:db8::" -def __init__(self, src_path): +def __init__(self, src_path, nsim_test=None): self.env = _load_env_file(src_path) @@ -88,7 +88,10 @@ from .remote import Remote self._ns_peer = None if "NETIF" in self.env: +if nsim_test is True: +raise KsftXfailEx("Test only works on netdevsim") self._check_env() + self.dev = ip("link show dev " + self.env['NETIF'], json=True)[0] self.v4 = self.env.get("LOCAL_V4") @@ -98,6 +101,9 @@ from .remote import Remote kind = self.env["REMOTE_TYPE"] args = self.env["REMOTE_ARGS"] else: +if nsim_test is False: +raise KsftXfailEx("Test does not work on netdevsim") + self.create_local() self.dev = self._ns.nsims[0].dev -- 2.44.0
[PATCH net-next v2 1/6] net: page_pool: support error injection
Because of caching / recycling using the general page allocation failures to induce errors in page pool allocation is very hard. Add direct error injection support to page_pool_alloc_pages(). Reviewed-by: Willem de Bruijn Signed-off-by: Jakub Kicinski --- CC: h...@kernel.org CC: ilias.apalodi...@linaro.org --- net/core/page_pool.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 273c24429bce..8bcc7014a61a 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -5,6 +5,7 @@ * Copyright (C) 2016 Red Hat, Inc. */ +#include #include #include #include @@ -550,6 +551,7 @@ struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp) return page; } EXPORT_SYMBOL(page_pool_alloc_pages); +ALLOW_ERROR_INJECTION(page_pool_alloc_pages, NULL); /* Calculate distance between two u32 values, valid if distance is below 2^(31) * https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution -- 2.44.0
[PATCH net-next v2 0/6] selftests: net: page_poll allocation error injection
Add a test for exercising driver memory allocation failure paths. page pool is a bit tricky to inject errors into at the page allocator level because of the bulk alloc and recycling, so add explicit error injection support "in front" of the caches. Add a test to exercise that using only the standard APIs. This is the first useful test for the new tests with an endpoint. There's no point testing netdevsim here, so this is also the first HW-only test in Python. I'm not super happy with the traffic generation using iperf3, my initial approach was to use mausezahn. But it turned out to be 5x slower in terms of PPS. Hopefully this is good enough for now. v2: - fix the string formatting for tool wrapper change (patch 3) - fix import order for load.py v1: https://lore.kernel.org/all/20240426232400.624864-1-k...@kernel.org/ Jakub Kicinski (6): net: page_pool: support error injection selftests: drv-net-hw: support using Python from net hw tests selftests: net: py: extract tool logic selftests: net: py: avoid all ports < 10k selftests: drv-net: support generating iperf3 load selftests: drv-net-hw: add test for memory allocation failures with page pool net/core/page_pool.c | 2 + tools/testing/selftests/Makefile | 2 +- .../testing/selftests/drivers/net/hw/Makefile | 2 + .../drivers/net/hw/lib/py/__init__.py | 16 +++ .../selftests/drivers/net/hw/pp_alloc_fail.py | 129 ++ .../selftests/drivers/net/lib/py/__init__.py | 1 + .../selftests/drivers/net/lib/py/env.py | 10 +- .../selftests/drivers/net/lib/py/load.py | 41 ++ tools/testing/selftests/net/lib/py/ksft.py| 4 + tools/testing/selftests/net/lib/py/utils.py | 14 +- 10 files changed, 214 insertions(+), 7 deletions(-) create mode 100644 tools/testing/selftests/drivers/net/hw/lib/py/__init__.py create mode 100755 tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py create mode 100644 tools/testing/selftests/drivers/net/lib/py/load.py -- 2.44.0
Re: [PATCH net-next 3/6] selftests: net: py: extract tool logic
On Sat, 27 Apr 2024 09:51:01 -0400 Willem de Bruijn wrote: > > +def ip(args, json=None, ns=None, host=None): > > +if ns: > > +args = '-netns ' + ns + " " + args > > Minor: inconsistent use of single and double comma strings. Maybe > there's a reasoning that I'm just missing. I also need to coerce ns to be a string, it may be class NetNS. v2 coming in 3... 2... 1...
[PATCH net-next 6/6] selftests: drv-net-hw: add test for memory allocation failures with page pool
Bugs in memory allocation failure paths are quite common. Add a test exercising those paths based on qstat and page pool failure hook. Running on bnxt: # ./drivers/net/hw/pp_alloc_fail.py KTAP version 1 1..1 # ethtool -G change retval: success ok 1 pp_alloc_fail.test_pp_alloc # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0 I initially wrote this test to validate commit be43b7489a3c ("net/mlx5e: RX, Fix page_pool allocation failure recovery for striding rq") but mlx5 still doesn't have qstat. So I run it on bnxt, and while bnxt survives I found the problem fixed in commit 730117730709 ("eth: bnxt: fix counting packets discarded due to OOM and netpoll"). Signed-off-by: Jakub Kicinski --- .../testing/selftests/drivers/net/hw/Makefile | 1 + .../selftests/drivers/net/hw/pp_alloc_fail.py | 129 ++ tools/testing/selftests/net/lib/py/ksft.py| 4 + 3 files changed, 134 insertions(+) create mode 100755 tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile index 95f32158b095..1dd732855d76 100644 --- a/tools/testing/selftests/drivers/net/hw/Makefile +++ b/tools/testing/selftests/drivers/net/hw/Makefile @@ -9,6 +9,7 @@ TEST_PROGS = \ hw_stats_l3.sh \ hw_stats_l3_gre.sh \ loopback.sh \ + pp_alloc_fail.py \ # TEST_FILES := \ diff --git a/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py b/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py new file mode 100755 index ..026d98976c35 --- /dev/null +++ b/tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0 + +import time +import os +from lib.py import ksft_run, ksft_exit, ksft_pr +from lib.py import KsftSkipEx, KsftFailEx +from lib.py import NetdevFamily, NlError +from lib.py import NetDrvEpEnv +from lib.py import cmd, tool, GenerateTraffic + + +def _write_fail_config(config): +for key, value in config.items(): +with open("/sys/kernel/debug/fail_function/" + key, "w") as fp: +fp.write(str(value) + "\n") + + +def _enable_pp_allocation_fail(): +if not os.path.exists("/sys/kernel/debug/fail_function"): +raise KsftSkipEx("Kernel built without function error injection (or DebugFS)") + +if not os.path.exists("/sys/kernel/debug/fail_function/page_pool_alloc_pages"): +with open("/sys/kernel/debug/fail_function/inject", "w") as fp: +fp.write("page_pool_alloc_pages\n") + +_write_fail_config({ +"verbose": 0, +"interval": 511, +"probability": 100, +"times": -1, +}) + + +def _disable_pp_allocation_fail(): +if not os.path.exists("/sys/kernel/debug/fail_function"): +return + +if os.path.exists("/sys/kernel/debug/fail_function/page_pool_alloc_pages"): +with open("/sys/kernel/debug/fail_function/inject", "w") as fp: +fp.write("\n") + +_write_fail_config({ +"probability": 0, +"times": 0, +}) + + +def test_pp_alloc(cfg, netdevnl): +def get_stats(): +return netdevnl.qstats_get({"ifindex": cfg.ifindex}, dump=True)[0] + +def check_traffic_flowing(): +stat1 = get_stats() +time.sleep(1) +stat2 = get_stats() +if stat2['rx-packets'] - stat1['rx-packets'] < 15000: +raise KsftFailEx("Traffic seems low:", stat2['rx-packets'] - stat1['rx-packets']) + + +try: +stats = get_stats() +except NlError as e: +if e.nl_msg.error == -95: +stats = {} +else: +raise +if 'rx-alloc-fail' not in stats: +raise KsftSkipEx("Driver does not report 'rx-alloc-fail' via qstats") + +set_g = False +traffic = None +try: +traffic = GenerateTraffic(cfg) + +check_traffic_flowing() + +_enable_pp_allocation_fail() + +s1 = get_stats() +time.sleep(3) +s2 = get_stats() + +if s2['rx-alloc-fail'] - s1['rx-alloc-fail'] < 1: +raise KsftSkipEx("Allocation failures not increasing") +if s2['rx-alloc-fail'] - s1['rx-alloc-fail'] < 100: +raise KsftSkipEx("Allocation increasing too slowly", s2['rx-alloc-fail'] - s1['rx-alloc-fail'], + "packets:", s2['rx-packets'] - s1['rx-packets']) + +# Basic failures are fine, try to wobble some settings to catch extra failures +check_traffic_flowing() +g = tool("ethtool", "-g " + cfg.ifname, json=True)[0] +if 'rx' in g and g["rx"] * 2 <= g[&q
[PATCH net-next 5/6] selftests: drv-net: support generating iperf3 load
While we are not very interested in testing performance it's useful to be able to generate a lot of traffic. iperf is the simplest way of getting relatively high PPS. Signed-off-by: Jakub Kicinski --- .../selftests/drivers/net/lib/py/__init__.py | 1 + .../selftests/drivers/net/lib/py/load.py | 41 +++ 2 files changed, 42 insertions(+) create mode 100644 tools/testing/selftests/drivers/net/lib/py/load.py diff --git a/tools/testing/selftests/drivers/net/lib/py/__init__.py b/tools/testing/selftests/drivers/net/lib/py/__init__.py index 4789c1a4282d..2a2dbb1b4ad7 100644 --- a/tools/testing/selftests/drivers/net/lib/py/__init__.py +++ b/tools/testing/selftests/drivers/net/lib/py/__init__.py @@ -2,6 +2,7 @@ import sys from pathlib import Path +from .load import * KSFT_DIR = (Path(__file__).parent / "../../../..").resolve() diff --git a/tools/testing/selftests/drivers/net/lib/py/load.py b/tools/testing/selftests/drivers/net/lib/py/load.py new file mode 100644 index ..abdb677bdb1c --- /dev/null +++ b/tools/testing/selftests/drivers/net/lib/py/load.py @@ -0,0 +1,41 @@ +# SPDX-License-Identifier: GPL-2.0 + +import time + +from lib.py import ksft_pr, cmd, ip, rand_port, wait_port_listen + +class GenerateTraffic: +def __init__(self, env): +env.require_cmd("iperf3", remote=True) + +self.env = env + +port = rand_port() +self._iperf_server = cmd(f"iperf3 -s -p {port}", background=True) +wait_port_listen(port) +time.sleep(0.1) +self._iperf_client = cmd(f"iperf3 -c {env.addr} -P 16 -p {port} -t 86400", + background=True, host=env.remote) + +# Wait for traffic to ramp up +pkt = ip("-s link show dev " + env.ifname, json=True)[0]["stats64"]["rx"]["packets"] +for _ in range(50): +time.sleep(0.1) +now = ip("-s link show dev " + env.ifname, json=True)[0]["stats64"]["rx"]["packets"] +if now - pkt > 1000: +return +pkt = now +self.stop(verbose=True) +raise Exception("iperf3 traffic did not ramp up") + +def stop(self, verbose=None): +self._iperf_client.process(terminate=True) +if verbose: +ksft_pr(">> Client:") +ksft_pr(self._iperf_client.stdout) +ksft_pr(self._iperf_client.stderr) +self._iperf_server.process(terminate=True) +if verbose: +ksft_pr(">> Server:") +ksft_pr(self._iperf_server.stdout) +ksft_pr(self._iperf_server.stderr) -- 2.44.0
[PATCH net-next 4/6] selftests: net: py: avoid all ports < 10k
When picking TCP ports to use, avoid all below 10k. This should lower the chance of collision or running afoul whatever random policies may be on the host. Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/lib/py/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index 11b588a2bb9d..074b792ec943 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -77,7 +77,7 @@ import time """ Get unprivileged port, for now just random, one day we may decide to check if used. """ -return random.randint(1024, 65535) +return random.randint(1, 65535) def wait_port_listen(port, proto="tcp", ns=None, host=None, sleep=0.005, deadline=5): -- 2.44.0
[PATCH net-next 3/6] selftests: net: py: extract tool logic
The main use of the ip() wrapper over cmd() is that it can parse JSON. cmd("ip -j link show") will return stdout as a string, and test has to call json.loads(). With ip("link show", json=True) the return value will be already parsed. More tools (ethtool, bpftool etc.) support the --json switch. To avoid having to wrap all of them individually create a tool() helper. Switch from -j to --json (for ethtool). While at it consume the netns attribute at the ip() level. Signed-off-by: Jakub Kicinski --- tools/testing/selftests/net/lib/py/utils.py | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/net/lib/py/utils.py b/tools/testing/selftests/net/lib/py/utils.py index d3715e6c21f2..11b588a2bb9d 100644 --- a/tools/testing/selftests/net/lib/py/utils.py +++ b/tools/testing/selftests/net/lib/py/utils.py @@ -56,10 +56,10 @@ import time return self.process(terminate=self.terminate) -def ip(args, json=None, ns=None, host=None): -cmd_str = "ip " +def tool(name, args, json=None, ns=None, host=None): +cmd_str = name + " " if json: -cmd_str += '-j ' +cmd_str += '--json ' cmd_str += args cmd_obj = cmd(cmd_str, ns=ns, host=host) if json: @@ -67,6 +67,12 @@ import time return cmd_obj +def ip(args, json=None, ns=None, host=None): +if ns: +args = '-netns ' + ns + " " + args +return tool("ip", args, json=json, host=host) + + def rand_port(): """ Get unprivileged port, for now just random, one day we may decide to check if used. -- 2.44.0
[PATCH net-next 2/6] selftests: drv-net-hw: support using Python from net hw tests
We created a separate directory for HW-only tests, recently. Glue in the Python test library there, Python is a bit annoying when it comes to using library code located "lower" in the directory structure. Reuse the Env class, but let tests require non-nsim setup. Signed-off-by: Jakub Kicinski --- tools/testing/selftests/Makefile | 2 +- tools/testing/selftests/drivers/net/hw/Makefile | 1 + .../selftests/drivers/net/hw/lib/py/__init__.py | 16 .../testing/selftests/drivers/net/lib/py/env.py | 10 -- 4 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 tools/testing/selftests/drivers/net/hw/lib/py/__init__.py diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 2c940e9c4ced..9039f3709aff 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -119,7 +119,7 @@ TARGETS_HOTPLUG = cpu-hotplug TARGETS_HOTPLUG += memory-hotplug # Networking tests want the net/lib target, include it automatically -ifneq ($(filter net drivers/net,$(TARGETS)),) +ifneq ($(filter net drivers/net drivers/net/hw,$(TARGETS)),) ifeq ($(filter net/lib,$(TARGETS)),) INSTALL_DEP_TARGETS := net/lib endif diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile index 2259a39a70ed..95f32158b095 100644 --- a/tools/testing/selftests/drivers/net/hw/Makefile +++ b/tools/testing/selftests/drivers/net/hw/Makefile @@ -16,6 +16,7 @@ TEST_FILES := \ # TEST_INCLUDES := \ + $(wildcard lib/py/*.py ../lib/py/*.py) \ ../../../net/lib.sh \ ../../../net/forwarding/lib.sh \ ../../../net/forwarding/ipip_lib.sh \ diff --git a/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py new file mode 100644 index ..b582885786f5 --- /dev/null +++ b/tools/testing/selftests/drivers/net/hw/lib/py/__init__.py @@ -0,0 +1,16 @@ +# SPDX-License-Identifier: GPL-2.0 + +import sys +from pathlib import Path + +KSFT_DIR = (Path(__file__).parent / "../../../../..").resolve() + +try: +sys.path.append(KSFT_DIR.as_posix()) +from net.lib.py import * +from drivers.net.lib.py import * +except ModuleNotFoundError as e: +ksft_pr("Failed importing `net` library from kernel sources") +ksft_pr(str(e)) +ktap_result(True, comment="SKIP") +sys.exit(4) diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py index e2ab637e56dc..5c8f695b2536 100644 --- a/tools/testing/selftests/drivers/net/lib/py/env.py +++ b/tools/testing/selftests/drivers/net/lib/py/env.py @@ -2,7 +2,7 @@ import os from pathlib import Path -from lib.py import KsftSkipEx +from lib.py import KsftSkipEx, KsftXfailEx from lib.py import cmd, ip from lib.py import NetNS, NetdevSimDev from .remote import Remote @@ -76,7 +76,7 @@ from .remote import Remote nsim_v4_pfx = "192.0.2." nsim_v6_pfx = "2001:db8::" -def __init__(self, src_path): +def __init__(self, src_path, nsim_test=None): self.env = _load_env_file(src_path) @@ -88,7 +88,10 @@ from .remote import Remote self._ns_peer = None if "NETIF" in self.env: +if nsim_test is True: +raise KsftXfailEx("Test only works on netdevsim") self._check_env() + self.dev = ip("link show dev " + self.env['NETIF'], json=True)[0] self.v4 = self.env.get("LOCAL_V4") @@ -98,6 +101,9 @@ from .remote import Remote kind = self.env["REMOTE_TYPE"] args = self.env["REMOTE_ARGS"] else: +if nsim_test is False: +raise KsftXfailEx("Test does not work on netdevsim") + self.create_local() self.dev = self._ns.nsims[0].dev -- 2.44.0
[PATCH net-next 0/6] selftests: net: page_poll allocation error injection
Add a test for exercising driver memory allocation failure paths. page pool is a bit tricky to inject errors into at the page allocator level because of the bulk alloc and recycling, so add explicit error injection support "in front" of the caches. Add a test to exercise that using only the standard APIs. This is the first useful test for the new tests with an endpoint. There's no point testing netdevsim here, so this is also the first HW-only test in Python. I'm not super happy with the traffic generation using iperf3, my initial approach was to use mausezahn. But it turned out to be 5x slower in terms of PPS. Hopefully this is good enough for now. Jakub Kicinski (6): net: page_pool: support error injection selftests: drv-net-hw: support using Python from net hw tests selftests: net: py: extract tool logic selftests: net: py: avoid all ports < 10k selftests: drv-net: support generating iperf3 load selftests: drv-net-hw: add test for memory allocation failures with page pool net/core/page_pool.c | 2 + tools/testing/selftests/Makefile | 2 +- .../testing/selftests/drivers/net/hw/Makefile | 2 + .../drivers/net/hw/lib/py/__init__.py | 16 +++ .../selftests/drivers/net/hw/pp_alloc_fail.py | 129 ++ .../selftests/drivers/net/lib/py/__init__.py | 1 + .../selftests/drivers/net/lib/py/env.py | 10 +- .../selftests/drivers/net/lib/py/load.py | 41 ++ tools/testing/selftests/net/lib/py/ksft.py| 4 + tools/testing/selftests/net/lib/py/utils.py | 14 +- 10 files changed, 214 insertions(+), 7 deletions(-) create mode 100644 tools/testing/selftests/drivers/net/hw/lib/py/__init__.py create mode 100755 tools/testing/selftests/drivers/net/hw/pp_alloc_fail.py create mode 100644 tools/testing/selftests/drivers/net/lib/py/load.py -- 2.44.0
[PATCH net-next 1/6] net: page_pool: support error injection
Because of caching / recycling using the general page allocation failures to induce errors in page pool allocation is very hard. Add direct error injection support to page_pool_alloc_pages(). Signed-off-by: Jakub Kicinski --- CC: h...@kernel.org CC: ilias.apalodi...@linaro.org --- net/core/page_pool.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 273c24429bce..8bcc7014a61a 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -5,6 +5,7 @@ * Copyright (C) 2016 Red Hat, Inc. */ +#include #include #include #include @@ -550,6 +551,7 @@ struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp) return page; } EXPORT_SYMBOL(page_pool_alloc_pages); +ALLOW_ERROR_INJECTION(page_pool_alloc_pages, NULL); /* Calculate distance between two u32 values, valid if distance is below 2^(31) * https://en.wikipedia.org/wiki/Serial_number_arithmetic#General_Solution -- 2.44.0
Re: [Intel-wired-lan] [PATCH v2 5/6] cio: ensure the copied buf is NUL terminated
On Fri, 26 Apr 2024 12:10:35 +0200 Alexander Gordeev wrote: > On Wed, Apr 24, 2024 at 05:16:56PM +0200, Alexander Gordeev wrote: > > Applied, thanks! > > Hi Jakub, > > I just want to make sure you do not have plans to pull this patch > via the net tree, right? (I schedulled it for the s390 tree already). Yes, go for it. I picked 1, 2 and 6, no interest in the other 3 :)
Re: [PATCH v4 1/8] net: Remove the now superfluous sentinel elements from ctl_table array
On Fri, 26 Apr 2024 08:59:31 +0200 Joel Granados wrote: > Sorry about this. I pulled the trigger way too early. This is already > fixed in my v4. > > |^~ > > -- > > netdev FAQ tl;dr: > > - designate your patch to a tree - [PATCH net] or [PATCH net-next] > > - for fixes the Fixes: tag is required, regardless of the tree > > - don't post large series (> 15 patches), break them up > > - don't repost your patches within one 24h period I guess you didn't bother reading the tl;dr I put in here.. SMH.
Re: [PATCH v4 1/8] net: Remove the now superfluous sentinel elements from ctl_table array
On Thu, 25 Apr 2024 14:02:59 +0200 Joel Granados via B4 Relay wrote: > - for (i = 0; i < ARRAY_SIZE(mpls_table) - 1; i++) > + for (i = 0; i < tabel_size; i++) > table[i].data = (char *)net + (uintptr_t)table[i].data; > > net->mpls.ctl = register_net_sysctl_sz(net, "net/mpls", table, > -ARRAY_SIZE(mpls_table)); > +tabel_size); ../net/mpls/af_mpls.c: In function ‘mpls_net_init’: ../net/mpls/af_mpls.c:2676:25: error: ‘tabel_size’ undeclared (first use in this function); did you mean ‘table_size’? 2676 | for (i = 0; i < tabel_size; i++) | ^~ | table_size ../net/mpls/af_mpls.c:2676:25: note: each undeclared identifier is reported only once for each function it appears in ../net/mpls/af_mpls.c:2660:16: warning: unused variable ‘table_size’ [-Wunused-variable] 2660 | size_t table_size = ARRAY_SIZE(mpls_table); |^~ -- netdev FAQ tl;dr: - designate your patch to a tree - [PATCH net] or [PATCH net-next] - for fixes the Fixes: tag is required, regardless of the tree - don't post large series (> 15 patches), break them up - don't repost your patches within one 24h period pw-bot: cr
[PATCH net-next v2 3/3] selftests: drv-net: validate the environment
Throw a slightly more helpful exception when env variables are partially populated. Prior to this change we'd get a dictionary key exception somewhere later on. Reviewed-by: Willem de Bruijn Signed-off-by: Jakub Kicinski --- v2: - check "both or none" for addresses --- .../selftests/drivers/net/lib/py/env.py | 25 +++ 1 file changed, 25 insertions(+) diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py index 6f57bd5c0ed7..e2ab637e56dc 100644 --- a/tools/testing/selftests/drivers/net/lib/py/env.py +++ b/tools/testing/selftests/drivers/net/lib/py/env.py @@ -88,6 +88,7 @@ from .remote import Remote self._ns_peer = None if "NETIF" in self.env: +self._check_env() self.dev = ip("link show dev " + self.env['NETIF'], json=True)[0] self.v4 = self.env.get("LOCAL_V4") @@ -143,6 +144,30 @@ from .remote import Remote ip(f"-6 addr add dev {self._ns_peer.nsims[0].ifname} {self.nsim_v6_pfx}2/64 nodad", ns=self._netns) ip(f" link set dev {self._ns_peer.nsims[0].ifname} up", ns=self._netns) +def _check_env(self): +vars_needed = [ +["LOCAL_V4", "LOCAL_V6"], +["REMOTE_V4", "REMOTE_V6"], +["REMOTE_TYPE"], +["REMOTE_ARGS"] +] +missing = [] + +for choice in vars_needed: +for entry in choice: +if entry in self.env: +break +else: +missing.append(choice) +# Make sure v4 / v6 configs are symmetric +if ("LOCAL_V6" in self.env) != ("REMOTE_V6" in self.env): +missing.append(["LOCAL_V6", "REMOTE_V6"]) +if ("LOCAL_V4" in self.env) != ("REMOTE_V4" in self.env): +missing.append(["LOCAL_V4", "REMOTE_V4"]) +if missing: +raise Exception("Invalid environment, missing configuration:", missing, +"Please see tools/testing/selftests/drivers/net/README.rst") + def __enter__(self): return self -- 2.44.0
[PATCH net-next v2 1/3] selftests: drv-net: extend the README with more info and example
Add more info to the README. It's also now copied to GitHub for increased visibility: https://github.com/linux-netdev/nipa/wiki/Running-driver-tests Reviewed-by: Willem de Bruijn Signed-off-by: Jakub Kicinski --- .../testing/selftests/drivers/net/README.rst | 97 --- 1 file changed, 85 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/drivers/net/README.rst b/tools/testing/selftests/drivers/net/README.rst index 0cbab33dad1f..3b6a29e6564b 100644 --- a/tools/testing/selftests/drivers/net/README.rst +++ b/tools/testing/selftests/drivers/net/README.rst @@ -1,18 +1,42 @@ -Running tests -= +.. SPDX-License-Identifier: GPL-2.0 -Tests are executed within kselftest framework like any other tests. -By default tests execute against software drivers such as netdevsim. -All tests must support running against a real device (SW-only tests -should instead be placed in net/ or drivers/net/netdevsim, HW-only -tests in drivers/net/hw). +Running driver tests + -Set appropriate variables to point the tests at a real device. +Networking driver tests are executed within kselftest framework like any +other tests. They support testing both real device drivers and emulated / +software drivers (latter mostly to test the core parts of the stack). + +SW mode +~~~ + +By default, when no extra parameters are set or exported, tests execute +against software drivers such as netdevsim. No extra preparation is required +the software devices are created and destroyed as part of the test. +In this mode the tests are indistinguishable from other selftests and +(for example) can be run under ``virtme-ng`` like the core networking selftests. + +HW mode +~~~ + +Executing tests against a real device requires external preparation. +The netdevice against which tests will be run must exist, be running +(in UP state) and be configured with an IP address. + +Refer to list of :ref:`Variables` later in this file to set up running +the tests against a real device. + +Both modes required +~~~ + +All tests in drivers/net must support running both against a software device +and a real device. SW-only tests should instead be placed in net/ or +drivers/net/netdevsim, HW-only tests in drivers/net/hw. Variables = -Variables can be set in the environment or by creating a net.config +The variables can be set in the environment or by creating a net.config file in the same directory as this README file. Example:: $ NETIF=eth0 ./some_test.sh @@ -23,9 +47,9 @@ Variables can be set in the environment or by creating a net.config # Variable set in a file NETIF=eth0 -Please note that the config parser is very simple, if there are -any non-alphanumeric characters in the value it needs to be in -double quotes. +Local test (which don't require endpoint for sending / receiving traffic) +need only the ``NETIF`` variable. Remaining variables define the endpoint +and communication method. NETIF ~ @@ -61,3 +85,52 @@ Arguments used to construct the communication channel. for netns - name of the "remote" namespace for ssh - name/address of the remote host + +Example +=== + +Build the selftests:: + + # make -C tools/testing/selftests/ TARGETS="drivers/net drivers/net/hw" + +"Install" the tests and copy them over to the target machine:: + + # make -C tools/testing/selftests/ TARGETS="drivers/net drivers/net/hw" \ + install INSTALL_PATH=/tmp/ksft-net-drv + + # rsync -ra --delete /tmp/ksft-net-drv root@192.168.1.1:/root/ + +On the target machine, running the tests will use netdevsim by default:: + + [/root] # ./ksft-net-drv/run_kselftest.sh -t drivers/net:ping.py + TAP version 13 + 1..1 + # timeout set to 45 + # selftests: drivers/net: ping.py + # KTAP version 1 + # 1..3 + # ok 1 ping.test_v4 + # ok 2 ping.test_v6 + # ok 3 ping.test_tcp + # # Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0 + ok 1 selftests: drivers/net: ping.py + +Create a config with remote info:: + + [/root] # cat > ./ksft-net-drv/drivers/net/net.config <
[PATCH net-next v2 2/3] selftests: drv-net: reimplement the config parser
The shell lexer is not helping much, do very basic parsing manually. Reviewed-by: Willem de Bruijn Signed-off-by: Jakub Kicinski --- v2: - use split() to split the line on = --- .../selftests/drivers/net/lib/py/env.py | 26 ++- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py index a3db1bb1afeb..6f57bd5c0ed7 100644 --- a/tools/testing/selftests/drivers/net/lib/py/env.py +++ b/tools/testing/selftests/drivers/net/lib/py/env.py @@ -1,7 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 import os -import shlex from pathlib import Path from lib.py import KsftSkipEx from lib.py import cmd, ip @@ -16,17 +15,20 @@ from .remote import Remote if not (src_dir / "net.config").exists(): return env -lexer = shlex.shlex(open((src_dir / "net.config").as_posix(), 'r').read()) -k = None -for token in lexer: -if k is None: -k = token -env[k] = "" -elif token == "=": -pass -else: -env[k] = token -k = None +with open((src_dir / "net.config").as_posix(), 'r') as fp: +for line in fp.readlines(): +full_file = line +# Strip comments +pos = line.find("#") +if pos >= 0: +line = line[:pos] +line = line.strip() +if not line: +continue +pair = line.split('=', maxsplit=1) +if len(pair) != 2: +raise Exception("Can't parse configuration line:", full_file) +env[pair[0]] = pair[1] return env -- 2.44.0
[PATCH net-next v2 0/3] selftests: drv-net: round some sharp edges
I had to explain how to run the driver tests twice already. Improve the README so we can just point to it. Improve the config validation. v2: - use split() in patch 2 - more validation in last patch v1: https://lore.kernel.org/r/20240424221444.4194069-1-k...@kernel.org/ Jakub Kicinski (3): selftests: drv-net: extend the README with more info and example selftests: drv-net: reimplement the config parser selftests: drv-net: validate the environment .../testing/selftests/drivers/net/README.rst | 97 --- .../selftests/drivers/net/lib/py/env.py | 51 +++--- 2 files changed, 124 insertions(+), 24 deletions(-) -- 2.44.0
Re: [PATCH net-next 0/4] selftests: drv-net: round some sharp edges
On Wed, 24 Apr 2024 21:45:49 -0400 Willem de Bruijn wrote: > Left two minor points comments inline, but nothing important. I'll respin, apply your suggestions and drop the unfortunate patch.
Re: [ovs-dev] selftests: openvswitch: Questions about possible enhancements
On Thu, 25 Apr 2024 19:57:19 +0100 Simon Horman wrote: > openvswitch.sh does not appear to have any dependencies on Open vSwitch > user-space. My understanding is that, rather, it makes use of > tools/testing/selftests/net/openvswitch/ovs-dpctl.py to talk to the Kernel > using Netlink (which is also what Open vSwitch user-space does). > > My brief testing indicates that for this the only dependencies > when running on Amazon Linux 2 are python3 and pyroute2. > > I think that it should be possible to port pmtu.sh to use ovs-dpctl.py. > This would require some enhancements to ovs-dpctl.py to handle adding the > tunnel vports (interfaces). > > As an aside, to run the Open vSwitch tests in pmtu.sh the openvswitch > kernel module is needed. So I think it would make sense to add > CONFIG_OPENVSWITCH to tools/testing/selftests/net/config. > > That would mean that tools/testing/selftests/net/config also has all > the requirements to run openvswitch.sh. If so, we probably wouldn't need to > add tools/testing/selftests/net/openvswitch/config or otherwise do anything > special to configure the kernel for openvswitch.sh. That sounds great, for simplicity we could move the ovs files down to the .../net/ directory. It'd be cool to not have to do that, and let us separate tests more clearly into directories. But right now every directory has its own runner so there's a high price to pay for a primarily aesthetic gain :( ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev