Re: [Intel-wired-lan] [PATCH iwl-net v5] ice: Do not get coalesce settings while in reset

2024-06-10 Thread Jakub Kicinski
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

2024-06-05 Thread Jakub Kicinski
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

2024-06-05 Thread Jakub Kicinski
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

2024-06-05 Thread Jakub Kicinski
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

2024-06-03 Thread Jakub Kicinski
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

2024-06-03 Thread Jakub Kicinski
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()

2024-06-03 Thread Jakub Kicinski
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

2024-06-01 Thread Jakub Kicinski
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

2024-05-30 Thread Jakub Kicinski
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

2024-05-30 Thread Jakub Kicinski
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()

2024-05-29 Thread Jakub Kicinski
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

2024-05-29 Thread Jakub Kicinski
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

2024-05-28 Thread Jakub Kicinski
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.

2024-05-28 Thread Jakub Kicinski
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

2024-05-28 Thread Jakub Kicinski
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

2024-05-28 Thread Jakub Kicinski
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

2024-05-28 Thread Jakub Kicinski
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

2024-05-28 Thread Jakub Kicinski
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

2024-05-23 Thread Jakub Kicinski
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

2024-05-22 Thread Jakub Kicinski
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

2024-05-17 Thread Jakub Kicinski
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

2024-05-17 Thread Jakub Kicinski
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

2024-05-16 Thread Jakub Kicinski
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

2024-05-16 Thread Jakub Kicinski
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

2024-05-16 Thread Jakub Kicinski
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

2024-05-15 Thread Jakub Kicinski
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

2024-05-14 Thread Jakub Kicinski
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

2024-05-14 Thread Jakub Kicinski
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

2024-05-14 Thread Jakub Kicinski
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

2024-05-13 Thread Jakub Kicinski
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

2024-05-13 Thread Jakub Kicinski
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

2024-05-13 Thread Jakub Kicinski
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

2024-05-13 Thread Jakub Kicinski
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

2024-05-10 Thread Jakub Kicinski
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

2024-05-10 Thread Jakub Kicinski
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

2024-05-10 Thread Jakub Kicinski
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

2024-05-09 Thread Jakub Kicinski
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

2024-05-09 Thread Jakub Kicinski
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

2024-05-09 Thread Jakub Kicinski
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

2024-05-09 Thread Jakub Kicinski
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

2024-05-09 Thread Jakub Kicinski
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

2024-05-09 Thread Jakub Kicinski
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.

2024-05-09 Thread Jakub Kicinski
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

2024-05-09 Thread Jakub Kicinski
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

2024-05-09 Thread Jakub Kicinski
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

2024-05-08 Thread Jakub Kicinski
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

2024-05-08 Thread Jakub Kicinski
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

2024-05-08 Thread Jakub Kicinski
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

2024-05-08 Thread Jakub Kicinski
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

2024-05-08 Thread Jakub Kicinski
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)

2024-05-08 Thread Jakub Kicinski
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

2024-05-07 Thread Jakub Kicinski
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

2024-05-07 Thread Jakub Kicinski
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_

2024-05-07 Thread Jakub Kicinski
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

2024-05-06 Thread Jakub Kicinski
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

2024-05-06 Thread Jakub Kicinski
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)

2024-05-06 Thread Jakub Kicinski
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

2024-05-06 Thread Jakub Kicinski
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)

2024-05-06 Thread Jakub Kicinski
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

2024-05-06 Thread Jakub Kicinski
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

2024-05-03 Thread Jakub Kicinski
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

2024-05-03 Thread Jakub Kicinski
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

2024-05-03 Thread Jakub Kicinski
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

2024-05-03 Thread Jakub Kicinski
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

2024-05-03 Thread Jakub Kicinski
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

2024-05-01 Thread Jakub Kicinski
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

2024-05-01 Thread Jakub Kicinski
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

2024-04-30 Thread Jakub Kicinski
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

2024-04-30 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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)

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-29 Thread Jakub Kicinski
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

2024-04-26 Thread Jakub Kicinski
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

2024-04-26 Thread Jakub Kicinski
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

2024-04-26 Thread Jakub Kicinski
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

2024-04-26 Thread Jakub Kicinski
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

2024-04-26 Thread Jakub Kicinski
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

2024-04-26 Thread Jakub Kicinski
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

2024-04-26 Thread Jakub Kicinski
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

2024-04-26 Thread Jakub Kicinski
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

2024-04-26 Thread Jakub Kicinski
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

2024-04-25 Thread Jakub Kicinski
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

2024-04-25 Thread Jakub Kicinski
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

2024-04-25 Thread Jakub Kicinski
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

2024-04-25 Thread Jakub Kicinski
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

2024-04-25 Thread Jakub Kicinski
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

2024-04-25 Thread Jakub Kicinski
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

2024-04-25 Thread Jakub Kicinski
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


  1   2   3   4   5   6   7   8   9   10   >