Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] net: Convert %pK back to %p
From: Kees Cook @ 2026-07-09 16:18 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: linux-atm-general, linux-can, linux-sctp, netdev, David S. Miller,
	Eric Dumazet, Herbert Xu, Jakub Kicinski, Kuniyuki Iwashima,
	Marc Kleine-Budde, Marcelo Ricardo Leitner, Neal Cardwell,
	Oliver Hartkopp, Paolo Abeni, Remi Denis-Courmont, Simon Horman,
	Steffen Klassert, Willem de Bruijn, Xin Long, Petr Mladek,
	Thomas Weißschuh
In-Reply-To: <20260706073824.xixrLxoD@linutronix.de>

On Mon, Jul 06, 2026 at 09:38:24AM +0200, Sebastian Andrzej Siewior wrote:
> This is a revert of commit 71338aa7d050c ("net: convert %p usage to
> %pK") which is from 2011. Back then the default behaviour for %p was to
> print the pointer. The %pK modifier was introduced to be able to control
> the behaviour of specific pointer output without changing the behaviour
> of %p for everyone. It was dedicated to avoid leaking pointers via
> /proc.

Given the policy on bare %p, and that there are so few in this list (15
files), how about review those that can just simply be removed or
switched to %pS, etc:
https://docs.kernel.org/process/deprecated.html#p-format-specifier

-- 
Kees Cook

^ permalink raw reply

* Re: [PATCH net-next v8 2/3] net: airoha: fix ETS QoS stats counter underflow and cross-channel corruption
From: Simon Horman @ 2026-07-09 16:14 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Alexander Lobakin, linux-arm-kernel, linux-mediatek,
	netdev
In-Reply-To: <20260703-airoha-ethtool-priv_flags-v8-2-015ba5ac89ee@kernel.org>

On Fri, Jul 03, 2026 at 11:19:26AM +0200, Lorenzo Bianconi wrote:
> airoha_qdma_get_tx_ets_stats() has two bugs:
> - The hardware counters read via airoha_qdma_rr() are 32-bit values
>   but are stored in u64 locals and subtracted from u64 baselines. When
>   a 32-bit hardware counter wraps around, the subtraction produces a
>   large underflow value passed to _bstats_update().
> - The baseline counters (cpu_tx_packets, fwd_tx_packets) are stored as
>   single per-device fields, but airoha_qdma_get_tx_ets_stats() is
>   called with different channel values (0-3). Each call reads a
>   different channel's hardware counter but overwrites the same
>   baseline, corrupting the delta computation for other channels.
> 
> Fix both by:
> - Narrowing the counter locals and baselines to u32 so that 32-bit
>   unsigned subtraction handles wrap-around naturally.
> - Grouping the baselines into a per-channel qos_stats array so each
>   channel tracks its own previous counter value independently.
> - Splitting the delta addition into two statements so the first u32
>   delta is widened to u64 on assignment and the second is added in
>   u64 arithmetic, preventing overflow when both deltas are large.
> 
> Fixes: 20bf7d07c956 ("net: airoha: Add sched ETS offload support")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply

* Re: [PATCH net-next v8 1/3] net: airoha: rename airoha_priv_flags to airoha_dev_flags
From: Simon Horman @ 2026-07-09 16:14 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Alexander Lobakin, linux-arm-kernel, linux-mediatek,
	netdev
In-Reply-To: <20260703-airoha-ethtool-priv_flags-v8-1-015ba5ac89ee@kernel.org>

On Fri, Jul 03, 2026 at 11:19:25AM +0200, Lorenzo Bianconi wrote:
> Rename the airoha_priv_flags enum to airoha_dev_flags and the
> AIROHA_PRIV_F_WAN flag to AIROHA_DEV_F_WAN. The "priv_flags" naming
> dates back to an earlier design that used ethtool private flags; since
> this series switched to tc qdisc offload for LAN/WAN configuration,
> align the naming to reflect that these are per-device flags rather than
> ethtool private flags. No functional change.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply

* [PATCH net-next] selftests: net: update wait mechanism for relocating netdevsim to a new ns
From: Thaison Phan @ 2026-07-09 16:08 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Shuah Khan
  Cc: Fengyuan Gong, Stan Iliev, Simon Horman, Willem de Bruijn,
	Petr Machata, netdev, linux-kselftest, linux-kernel, Thaison Phan

When netdevsim devices are reloaded into a non-default network namespace
via 'devlink dev reload', the host's udev daemon does not receive the
subsequent device creation events since it is generally associated to the
default network namespace, so the host udevd queue never officially clears.
As a result, running 'udevadm settle' blocks indefinitely and eventually
times out.

Fixed this by bypassing 'udevadm settle' when a namespace ('ns') is
provided by utilizing the namespace-aware 'self.wait_for_netdevs()'
helper to poll the namespace's sysfs until the interfaces are visible.
For host-only configurations (where 'ns' is None), preserve the original
'udevadm settle' behavior to ensure udev renaming rules are completed.

Also fixed minor bug in wait_for_devs() where wrong exception was caught
from if get_ifnames() failed. Now the if get_ifnames() results in a
command failure exception, wait_for_devs() will catch the exception and
set an empty list for the interface names rather than the exception
propogating upward.

Tested that tools/testing/selftests/drivers/net/ping.py and
tools/testing/selftests/drivers/net/psp.py still pass when using
software network devices in a QEMU x86_64 environment.

Signed-off-by: Thaison Phan <thaisonphan@google.com>
---
 tools/testing/selftests/net/lib/py/nsim.py | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/lib/py/nsim.py b/tools/testing/selftests/net/lib/py/nsim.py
index 7c640ed64c0b..a01491c2d1b6 100644
--- a/tools/testing/selftests/net/lib/py/nsim.py
+++ b/tools/testing/selftests/net/lib/py/nsim.py
@@ -6,7 +6,7 @@ import os
 import random
 import re
 import time
-from .utils import cmd, ip
+from .utils import cmd, ip, CmdExitFailure
 
 
 class NetdevSim:
@@ -76,8 +76,11 @@ class NetdevSimDev:
         if ns:
             cmd(f"devlink dev reload netdevsim/netdevsim{addr} netns {ns.name}")
             self.ns = ns
-
-        cmd("udevadm settle", ns=self.ns)
+            # udev daemon is not guaranteed to get notified of netdevs being created
+            # when in a new netns, so poll until we see the netdevs appear.
+            self.wait_for_netdevs(port_count)
+        else:
+            cmd("udevadm settle", ns=self.ns)
         ifnames = self.get_ifnames()
 
         self.dfs_dir = "/sys/kernel/debug/netdevsim/netdevsim%u/" % addr
@@ -116,7 +119,7 @@ class NetdevSimDev:
         while True:
             try:
                 ifnames = self.get_ifnames()
-            except FileNotFoundError as e:
+            except CmdExitFailure:
                 ifnames = []
             if len(ifnames) == port_count:
                 break
-- 
2.55.0.795.g602f6c329a-goog


^ permalink raw reply related

* Re: [PATCH net v3] tun/tap & vhost-net: make qdisc backpressure opt-in via IFF_BACKPRESSURE
From: Brett Sheffield @ 2026-07-09 16:06 UTC (permalink / raw)
  To: Simon Schippers
  Cc: Willem de Bruijn, Jason Wang, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Michael S . Tsirkin, netdev,
	Simon Horman, Jonathan Corbet, Shuah Khan, Andrew Lunn,
	Tim Gebauer, linux-doc, linux-kernel
In-Reply-To: <20260709095511.168235-1-simon.schippers@tu-dortmund.de>

On 2026-07-09 11:55, Simon Schippers wrote:
> Commit 1d6e569b7d0c ("tun/tap & vhost-net: avoid ptr_ring tail-drop
> when a qdisc is present") did not show a relevant performance regression
> in my testing but on Brett Sheffield's librecast testbed it shows a
> significant performance drop in a IPv6 multicast testcase. The regression
> can be pinpointed when multiple iperf3 TCP threads are sending. For 8
> threads the performance dropped from 13.5 Gbit/s to 9.13 Gbit/s. This is
> the reason why this patch makes the qdisc backpressure behavior opt-in.
> 
> One option to accomplish the opt-in would be to set the default qdisc to
> noqueue at init. However this may also break userspace as users might
> have chosen a custom qdisc even though most of the qdiscs did nothing
> for tun/tap in the past due to missing backpressure...
> 
> This is the reason why in this patch, the flag IFF_BACKPRESSURE is
> introduced instead which is required to enable the backpressure logic.
> This means the stopping logic in tun_net_xmit() and the waking logic in
> __tun_wake_queue() are skipped if the flag is disabled. Setting
> IFF_BACKPRESSURE makes an attached qdisc effective by stopping the queue
> instead of tail-dropping when the internal ring is full.
> 
> To avoid a possible stall due to disabling IFF_BACKPRESSURE, the new
> helper tun_force_wake_queue() is implemented. The helper safely wakes the
> respective netdev queue and resets cons_cnt while the consumer_lock and
> the producer_lock of the ring are held. The helper is run in tun_attach()
> when a queue (re)attaches, in tun_set_iff() for attached tfiles, and
> in tun_queue_resize().
> 
> The documentation in tuntap.rst is updated accordingly.
> 
> Fixes: 1d6e569b7d0c ("tun/tap & vhost-net: avoid ptr_ring tail-drop when a qdisc is present")
> Reported-by: Brett Sheffield <brett@librecast.net>
> Closes: https://lore.kernel.org/netdev/akVnoOYQOrt8k-Gu@karahi.librecast.net/T/#u
> Signed-off-by: Simon Schippers <simon.schippers@tu-dortmund.de>

Tested v3 patch applied to 7.2.0-rc2.  OK.

Tested-by: Brett A C Sheffield <bacs@librecast.net>

> ---
> V2 -> V3:
> - As suggested by MST: Clarify in tuntap.rst and the UAPI header what
>   enabling IFF_BACKPRESSURE opts into: an attached qdisc becomes effective
>   instead of the driver tail-dropping when the internal ring is full.
> - Avoid lines over 75 characters.
> - Update comment in tun_net_xmit() to include IFF_BACKPRESSURE.
> - Brett: Update in commit message that the referenced tests were TCP.
> 
> V1 -> V2:
> - Sashiko: Ensure detached queues are woken on re-attach by calling the
>   new tun_force_wake_queue() helper from tun_attach(), and reuse it
>   across the existing wake paths.
> - Specify the failing test case in the commit message.
> 
> V1: https://lore.kernel.org/netdev/20260704112058.95421-1-simon.schippers@tu-dortmund.de/T/#u
> V2: https://lore.kernel.org/netdev/20260706094242.115992-1-simon.schippers@tu-dortmund.de/T/#u
> 
>  Documentation/networking/tuntap.rst | 22 +++++++++++++
>  drivers/net/tun.c                   | 51 ++++++++++++++++++++---------
>  include/uapi/linux/if_tun.h         |  4 +++
>  tools/include/uapi/linux/if_tun.h   |  1 +
>  4 files changed, 62 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/networking/tuntap.rst b/Documentation/networking/tuntap.rst
> index 4d7087f727be..5921a924c2ae 100644
> --- a/Documentation/networking/tuntap.rst
> +++ b/Documentation/networking/tuntap.rst
> @@ -206,6 +206,28 @@ enable is true we enable it, otherwise we disable it::
>        return ioctl(fd, TUNSETQUEUE, (void *)&ifr);
>    }
>  
> +3.4 qdisc backpressure
> +----------------------
> +
> +Starting with Linux 7.2, IFF_BACKPRESSURE can be set to enable qdisc
> +backpressure. Without it, TX drops occur when the internal ring buffer
> +is full, so any attached qdisc is effectively bypassed and applications
> +only learn about congestion through those drops.
> +
> +With it, the kernel stops instead, letting the qdisc hold and schedule
> +packets, so its AQM, shaping and fairness actually apply. This helps
> +protocols like TCP, which cut throughput in reaction to packet drops.
> +With IFF_BACKPRESSURE, drops then only occur as a rare race. Backpressure
> +requires a qdisc to be attached and has no effect with noqueue.
> +
> +The txqueuelen can be reduced alongside this flag to further shift
> +buffering into the qdisc and reduce bufferbloat, but comes at possible
> +performance cost.
> +
> +When running multiple network streams in parallel through a single
> +TUN/TAP queue, the flag may reduce performance due to the extra overhead
> +of the backpressure mechanism.
> +
>  Universal TUN/TAP device driver Frequently Asked Question
>  =========================================================
>  
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index ffbe6f13fb1f..5941e8f302ea 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -98,7 +98,8 @@ static void tun_default_link_ksettings(struct net_device *dev,
>  #define TUN_FASYNC	IFF_ATTACH_QUEUE
>  
>  #define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
> -		      IFF_MULTI_QUEUE | IFF_NAPI | IFF_NAPI_FRAGS)
> +		      IFF_MULTI_QUEUE | IFF_NAPI | IFF_NAPI_FRAGS | \
> +		      IFF_BACKPRESSURE)
>  
>  #define GOODCOPY_LEN 128
>  
> @@ -694,6 +695,20 @@ static void tun_detach_all(struct net_device *dev)
>  		module_put(THIS_MODULE);
>  }
>  
> +static void tun_force_wake_queue(struct tun_struct *tun,
> +				 struct tun_file *tfile)
> +{
> +	/* Ensure that the producer can not stop the
> +	 * queue concurrently by taking locks.
> +	 */
> +	spin_lock_bh(&tfile->tx_ring.consumer_lock);
> +	spin_lock(&tfile->tx_ring.producer_lock);
> +	netif_wake_subqueue(tun->dev, tfile->queue_index);
> +	tfile->cons_cnt = 0;
> +	spin_unlock(&tfile->tx_ring.producer_lock);
> +	spin_unlock_bh(&tfile->tx_ring.consumer_lock);
> +}
> +
>  static int tun_attach(struct tun_struct *tun, struct file *file,
>  		      bool skip_filter, bool napi, bool napi_frags,
>  		      bool publish_tun)
> @@ -737,11 +752,9 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
>  		goto out;
>  	}
>  
> -	spin_lock(&tfile->tx_ring.consumer_lock);
> -	tfile->cons_cnt = 0;
> -	spin_unlock(&tfile->tx_ring.consumer_lock);
>  	tfile->queue_index = tun->numqueues;
>  	tfile->socket.sk->sk_shutdown &= ~RCV_SHUTDOWN;
> +	tun_force_wake_queue(tun, tfile);
>  
>  	if (tfile->detached) {
>  		/* Re-attach detached tfile, updating XDP queue_index */
> @@ -1077,7 +1090,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>  
>  	spin_lock(&tfile->tx_ring.producer_lock);
>  	ret = __ptr_ring_produce(&tfile->tx_ring, skb);
> -	if (!qdisc_txq_has_no_queue(queue) &&
> +	if ((tun->flags & IFF_BACKPRESSURE) &&
> +	    !qdisc_txq_has_no_queue(queue) &&
>  	    __ptr_ring_check_produce(&tfile->tx_ring) == -ENOSPC) {
>  		netif_tx_stop_queue(queue);
>  		/* Paired with smp_mb() in __tun_wake_queue() */
> @@ -1088,8 +1102,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>  	spin_unlock(&tfile->tx_ring.producer_lock);
>  
>  	if (ret) {
> -		/* This should be a rare case if a qdisc is present, but
> -		 * can happen due to lltx.
> +		/* This should be a rare case if IFF_BACKPRESSURE is enabled and
> +		 * a qdisc is present, but can happen due to lltx.
>  		 * Since skb_tx_timestamp(), skb_orphan(),
>  		 * run_ebpf_filter() and pskb_trim() could have tinkered
>  		 * with the SKB, returning NETDEV_TX_BUSY is unsafe and
> @@ -2151,8 +2165,12 @@ static ssize_t tun_put_user(struct tun_struct *tun,
>  static void __tun_wake_queue(struct tun_struct *tun,
>  			     struct tun_file *tfile, int consumed)
>  {
> -	struct netdev_queue *txq = netdev_get_tx_queue(tun->dev,
> -						tfile->queue_index);
> +	struct netdev_queue *txq;
> +
> +	if (!(tun->flags & IFF_BACKPRESSURE))
> +		return;
> +
> +	txq = netdev_get_tx_queue(tun->dev, tfile->queue_index);
>  
>  	/* Paired with smp_mb__after_atomic() in tun_net_xmit() */
>  	smp_mb();
> @@ -2764,7 +2782,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>  	struct tun_struct *tun;
>  	struct tun_file *tfile = file->private_data;
>  	struct net_device *dev;
> -	int err;
> +	int err, i;
>  
>  	if (tfile->detached)
>  		return -EINVAL;
> @@ -2893,8 +2911,12 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>  	/* Make sure persistent devices do not get stuck in
>  	 * xoff state.
>  	 */
> -	if (netif_running(tun->dev))
> -		netif_tx_wake_all_queues(tun->dev);
> +	if (netif_running(tun->dev)) {
> +		for (i = 0; i < tun->numqueues; i++) {
> +			tfile = rtnl_dereference(tun->tfiles[i]);
> +			tun_force_wake_queue(tun, tfile);
> +		}
> +	}
>  
>  	strscpy(ifr->ifr_name, tun->dev->name);
>  	return 0;
> @@ -3693,10 +3715,7 @@ static int tun_queue_resize(struct tun_struct *tun)
>  	if (!ret) {
>  		for (i = 0; i < tun->numqueues; i++) {
>  			tfile = rtnl_dereference(tun->tfiles[i]);
> -			spin_lock(&tfile->tx_ring.consumer_lock);
> -			netif_wake_subqueue(tun->dev, tfile->queue_index);
> -			tfile->cons_cnt = 0;
> -			spin_unlock(&tfile->tx_ring.consumer_lock);
> +			tun_force_wake_queue(tun, tfile);
>  		}
>  	}
>  
> diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> index 79d53c7a1ebd..a0ddc50a7534 100644
> --- a/include/uapi/linux/if_tun.h
> +++ b/include/uapi/linux/if_tun.h
> @@ -69,6 +69,10 @@
>  #define IFF_NAPI_FRAGS	0x0020
>  /* Used in TUNSETIFF to bring up tun/tap without carrier */
>  #define IFF_NO_CARRIER	0x0040
> +/* Stop the queue instead of dropping when the internal ring is full, so an
> + * attached qdisc applies backpressure instead of being bypassed.
> + */
> +#define IFF_BACKPRESSURE	0x0080
>  #define IFF_NO_PI	0x1000
>  /* This flag has no real effect */
>  #define IFF_ONE_QUEUE	0x2000
> diff --git a/tools/include/uapi/linux/if_tun.h b/tools/include/uapi/linux/if_tun.h
> index 2ec07de1d73b..97b670f5bc0a 100644
> --- a/tools/include/uapi/linux/if_tun.h
> +++ b/tools/include/uapi/linux/if_tun.h
> @@ -67,6 +67,7 @@
>  #define IFF_TAP		0x0002
>  #define IFF_NAPI	0x0010
>  #define IFF_NAPI_FRAGS	0x0020
> +#define IFF_BACKPRESSURE	0x0080
>  #define IFF_NO_PI	0x1000
>  /* This flag has no real effect */
>  #define IFF_ONE_QUEUE	0x2000
> -- 
> 2.43.0
> 

-- 
Brett Sheffield (he/him)
Librecast - Decentralising the Internet with Multicast
https://librecast.net/
https://blog.brettsheffield.com/

^ permalink raw reply

* Re: [PATCH v2] docs: packaging: fix brackets
From: Randy Dunlap @ 2026-07-09 16:02 UTC (permalink / raw)
  To: Manuel Ebner, Vladimir Oltean, Jonathan Corbet, Shuah Khan,
	Jacob Keller, Jakub Kicinski
  Cc: Andrew Lunn, Vladimir Oltean, netdev, linux-doc, linux-kernel
In-Reply-To: <20260709141012.400011-2-manuelebner@mailbox.org>



On 7/9/26 7:10 AM, Manuel Ebner wrote:
> Add two ')' to nested functions in code block.
> 
> Fixes: a9ad2a8dfb43 ("lib: packing: document recently added APIs")
> Signed-off-by: Manuel Ebner <manuelebner@mailbox.org>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Subject is now incorrect; should be packing:

Acked-by: Randy Dunlap <rdunlap@infradead.org>

> ---
> [v2]
>  add Fixes tag.
>  add Reviewed-by.
> [v1]
>  https://lore.kernel.org/all/20260709121427.391749-2-manuelebner@mailbox.org/
> ---
>  Documentation/core-api/packing.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/core-api/packing.rst b/Documentation/core-api/packing.rst
> index f68f1e08fef9..cff1a262efce 100644
> --- a/Documentation/core-api/packing.rst
> +++ b/Documentation/core-api/packing.rst
> @@ -330,7 +330,7 @@ Here is an example of how to use the fields APIs:
>  
>     void unpack_your_data(const packed_buf_t *buf, struct data *unpacked)
>     {
> -           BUILD_BUG_ON(sizeof(*buf) != SIZE;
> +           BUILD_BUG_ON(sizeof(*buf) != SIZE);
>  
>             unpack_fields(buf, sizeof(*buf), unpacked, fields,
>                           QUIRK_LITTLE_ENDIAN);
> @@ -338,7 +338,7 @@ Here is an example of how to use the fields APIs:
>  
>     void pack_your_data(const struct data *unpacked, packed_buf_t *buf)
>     {
> -           BUILD_BUG_ON(sizeof(*buf) != SIZE;
> +           BUILD_BUG_ON(sizeof(*buf) != SIZE);
>  
>             pack_fields(buf, sizeof(*buf), unpacked, fields,
>                         QUIRK_LITTLE_ENDIAN);

-- 
~Randy

^ permalink raw reply

* [PATCH iwl-next] ice: fix uninitialized response buffer in ice_acl_create_tbl()
From: Marcin Szycik @ 2026-07-09 14:59 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: netdev, anthony.l.nguyen, Marcin Szycik, kernel test robot

Buffer is uninitialized in error path. Move assignment before goto. This
buffer is only used for response, so its content doesn't matter.

Fixes: d80ee68614c3 ("ice: initialize ACL table")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202607022231.FWjSU75X-lkp@intel.com
Signed-off-by: Marcin Szycik <marcin.szycik@linux.intel.com>
---
Tony, please squash this with the offending commit. It's not applied
yet, so hash is invalid, but I don't want to resend a ~5000 line
patchset just to move one line.
---
 drivers/net/ethernet/intel/ice/ice_acl_ctrl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_acl_ctrl.c b/drivers/net/ethernet/intel/ice/ice_acl_ctrl.c
index a76762e3aa46..4ea08bd9784c 100644
--- a/drivers/net/ethernet/intel/ice/ice_acl_ctrl.c
+++ b/drivers/net/ethernet/intel/ice/ice_acl_ctrl.c
@@ -354,14 +354,14 @@ int ice_acl_create_tbl(struct ice_hw *hw, struct ice_acl_tbl_params *params)
 		return -ENOMEM;
 	}
 
+	resp_buf = &tbl_alloc.buf.resp_buf;
+
 	tbl = kzalloc_obj(*tbl);
 	if (!tbl) {
 		err = -ENOMEM;
 		goto err_dealloc_tbl;
 	}
 
-	resp_buf = &tbl_alloc.buf.resp_buf;
-
 	/* Retrieve information of the allocated table */
 	tbl->id = alloc_id;
 	tbl->first_tcam = resp_buf->ops.table.first_tcam;
-- 
2.49.0


^ permalink raw reply related

* Re: [PATCH net] rds: Fix inet6_addr_lst NULL dereference when IPv6 is disabled
From: Ilia Gavrilov @ 2026-07-09 15:51 UTC (permalink / raw)
  To: Allison Henderson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Ka-Cheong Poon, Santosh Shilimkar,
	netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	rds-devel@oss.oracle.com, linux-kernel@vger.kernel.org,
	lvc-project@linuxtesting.org
In-Reply-To: <a724cf77f2338776f44b465a25009c22a0ee7c43.camel@kernel.org>

On 7/9/26 6:41 PM, Allison Henderson wrote:
> On Wed, 2026-07-08 at 11:59 +0000, Ilia Gavrilov wrote:
>> When booting with the 'ipv6.disable=1' parameter, inet6_addr_lst
>> is never initialized because inet6_init() exits before addrconf_init()
>> is called to initialize it. An attempt to bind an RDS socket to
>> an ipv6 address results in a crash in __ipv6_chk_addr_and_flags()
> 
> Hello Ilia,
> 
> Thanks for the catch.  Some comments below:
> 
>>
>> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
>> RIP: 0010:__ipv6_chk_addr_and_flags+0x1df/0x7e0
>> Call Trace:
>>  <TASK>
>>  ipv6_chk_addr+0x3b/0x50
>>  rds_tcp_laddr_check+0x155/0x3b0 [rds_tcp]
>>  rds_trans_get_preferred+0x15d/0x2d0 [rds]
>>  ? trace_hardirqs_on+0x2d/0x110
>>  rds_bind+0x1433/0x1d60 [rds]
>>  ? rds_remove_bound+0xd50/0xd50 [rds]
>>  ? aa_af_perm+0x250/0x250
>>  ? __might_fault+0xde/0x190
>>  ? __sys_bind+0x1dc/0x210
>>  __sys_bind+0x1dc/0x210
>>  ? __ia32_sys_socketpair+0x100/0x100
>>  ? restore_fpregs_from_fpstate+0x53/0x100
>>  __x64_sys_bind+0x73/0xb0
>>  ? syscall_enter_from_user_mode+0x1c/0x50
>>  do_syscall_64+0x34/0x80
>>  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>> RIP: 0033:0x7f47f8269ea9
>>  </TASK>
>>
>> The following code reproduces the issue:
>>
>> struct sockaddr_in6 addr;
>> s = socket(PF_RDS, SOCK_SEQPACKET, 0);
>>
>> memset(&addr, 0, sizeof(addr));
>> inet_pton(AF_INET6, ADDRESS, &addr.sin6_addr);
>> addr.sin6_family = AF_INET6;
>> addr.sin6_port = htons(PORT);
>>
>> bind(s, &addr, sizeof(addr);
> nit: missing paren here
> 
>>
>> Found by InfoTeCS on behalf of Linux Verification Center
>> (linuxtesting.org) with Syzkaller.
>>
>> Fixes: eee2fa6ab322 ("rds: Changing IP address internal representation to struct in6_addr")
>> Fixes: 1e2b44e78eea ("rds: Enable RDS IPv6 support")
>> Signed-off-by: Ilia Gavrilov <Ilia.Gavrilov@infotecs.ru>
>> ---
>>  net/rds/ib.c  | 4 ++++
>>  net/rds/tcp.c | 8 +++++---
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/rds/ib.c b/net/rds/ib.c
>> index 39f87272e071..8f9cf491984f 100644
>> --- a/net/rds/ib.c
>> +++ b/net/rds/ib.c
>> @@ -429,6 +429,10 @@ static int rds_ib_laddr_check_cm(struct net *net, const struct in6_addr *addr,
>>  		sa = (struct sockaddr *)&sin;
>>  	} else {
>>  #if IS_ENABLED(CONFIG_IPV6)
>> +		if (!ipv6_mod_enabled()) {
>> +			ret = -EADDRNOTAVAIL;
>> +			goto out;
>> +		}
>>  		memset(&sin6, 0, sizeof(sin6));
>>  		sin6.sin6_family = AF_INET6;
>>  		sin6.sin6_addr = *addr;
>> diff --git a/net/rds/tcp.c b/net/rds/tcp.c
>> index a1de114d5e2e..955d92277d5a 100644
>> --- a/net/rds/tcp.c
>> +++ b/net/rds/tcp.c
>> @@ -366,9 +366,11 @@ int rds_tcp_laddr_check(struct net *net, const struct in6_addr *addr,
>>  		rcu_read_unlock();
>>  	}
>>  #if IS_ENABLED(CONFIG_IPV6)
>> -	ret = ipv6_chk_addr(net, addr, dev, 0);
>> -	if (ret)
>> -		return 0;
>> +	if (ipv6_mod_enabled()) {
>> +		ret = ipv6_chk_addr(net, addr, dev, 0);
>> +		if (ret)
>> +			return 0;
>> +	}
> 
> There's another ipv6_chk_addr() in __rds_find_ifindex() with the same issue that affects inbound link-local IPv6
> connects.  Can you add a similar guard there too?  Then I think that should cover all the points of exposure.  Thanks
> for working on this.
> 
> Allison
> 

Thanks for the review, I'll add the guard in the next patch version.

>>  #endif
>>  	return -EADDRNOTAVAIL;
>>  }
> 


^ permalink raw reply

* Re: [PATCH net-next v11 2/2] net: mana: force full-page RX buffers via ethtool private flag
From: Dipayaan Roy @ 2026-07-09 15:48 UTC (permalink / raw)
  To: Simon Horman, Paolo Abeni
  Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
	kuba, pabeni, leon, longli, kotaranov, shradhagupta, ssengar,
	ernis, shirazsaleem, linux-hyperv, netdev, linux-kernel,
	linux-rdma, stephen, jacob.e.keller, dipayanroy, leitao, kees,
	john.fastabend, hawk, bpf, daniel, ast, sdf, yury.norov,
	pavan.chebbi
In-Reply-To: <20260708155741.1509815-2-horms@kernel.org>

On Wed, Jul 08, 2026 at 04:57:42PM +0100, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/
> ---
> net: mana: force full-page RX buffers via ethtool private flag
> 
> This adds an ethtool private flag "full-page-rx" that forces one RX buffer
> per page to avoid a page_pool fragment path throughput regression on some
> 4K PAGE_SIZE ARM64 platforms. Toggling the flag while the port is up
> triggers a mana_detach() + mana_attach() reconfiguration to apply the new
> buffer layout.
> 
> A few questions inline.
> 
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > index 7438ea6b3f26..38f50b4eef87 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> 
> [ ... mana_use_single_rxbuf_per_page() and mana_get_rxbuf_cfg() hunks
>       unrelated to the comments below ... ]
> 
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> > index 3531abfdaf43..24f6ce63b031 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> 
> [ ... ]
> 
> > @@ -611,6 +630,88 @@ static int mana_get_link_ksettings(struct net_device *ndev,
> >  	return 0;
> >  }
> >  
> > +static u32 mana_get_priv_flags(struct net_device *ndev)
> > +{
> > +	struct mana_port_context *apc = netdev_priv(ndev);
> > +
> > +	return apc->priv_flags;
> > +}
> > +
> > +static int mana_set_priv_flags(struct net_device *ndev, u32 priv_flags)
> > +{
> > +	struct mana_port_context *apc = netdev_priv(ndev);
> > +	u32 changed = apc->priv_flags ^ priv_flags;
> > +	u32 old_priv_flags = apc->priv_flags;
> > +	bool schedule_port_reset = false;
> > +	int err = 0;
> > +
> > +	if (!changed)
> > +		return 0;
> > +
> > +	/* Reject unknown bits */
> > +	if (priv_flags & ~GENMASK(MANA_PRIV_FLAG_MAX - 1, 0))
> > +		return -EINVAL;
> > +
> > +	if (changed & BIT(MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF)) {
> > +		apc->priv_flags = priv_flags;
> 
> [Low]
> Is the storage of priv_flags intentionally nested inside the per-flag
> branch?  With only one flag today this is fine, but when a second bit is
> added to enum mana_priv_flag_bits, a user toggling only the new bit
> passes the !changed and unknown-bits checks, the outer if is skipped,
> apc->priv_flags is never updated, and mana_set_priv_flags() returns 0
> while mana_get_priv_flags() keeps reporting the old state.
> 
> Would it be cleaner to store priv_flags unconditionally after validation
> and use per-flag if (changed & BIT(...)) blocks only for the
> flag-specific reconfiguration?
> 

Hi Simon, Paolo,

Agreed on this future concern,
but also we donot plan to have another mana driver private flag.


> > +
> > +		if (!apc->port_is_up) {
> > +			/* Port is down, flag updated to apply on next up
> > +			 * so just return.
> > +			 */
> > +			return 0;
> > +		}
> > +
> > +		/* Pre-allocate buffers to prevent failure in mana_attach
> > +		 * later
> > +		 */
> > +		err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
> 
> [Low]
> When the MTU is already jumbo (mtu + MANA_RXBUF_PAD > PAGE_SIZE / 2) or
> XDP is attached, mana_use_single_rxbuf_per_page() already returns true
> regardless of the new priv flag:
> 
> 	if (apc->priv_flags & BIT(MANA_PRIV_FLAG_USE_FULL_PAGE_RXBUF))
> 		return true;
> 
> 	/* For xdp and jumbo frames make sure only one packet fits per page. */
> 	if (mtu + MANA_RXBUF_PAD > PAGE_SIZE / 2 || mana_xdp_get(apc))
> 		return true;
> 
> In that case mana_get_rxbuf_cfg() produces identical output whether the
> new flag is on or off, yet toggling the flag here always performs a full
> mana_pre_alloc_rxbufs() + mana_detach() + mana_attach() cycle and flaps
> the link.
> 
> Would a pre/post comparison of mana_get_rxbuf_cfg() output be worth
> adding to skip the reconfiguration when nothing actually changes?
Good point will add a pre and post comparison as well to skip the
detach/attach when already in that configuration.

> 
> [High]
> mana_set_priv_flags() drives the same detach/attach sequence used by
> mana_set_channels() and mana_per_port_queue_reset_work_handler(), but
> does not take apc->vport_mutex and set apc->channel_changing = true
> across the window.
> 
> The struct comment on channel_changing spells out the invariant:
> 
> 	/* Set by mana_set_channels() under vport_mutex to block RDMA
> 	 * from grabbing the vport during the detach/attach window.
> 	 * Checked by mana_cfg_vport() when called from the RDMA path.
> 	 */
> 	bool channel_changing;
> 
> Without it, after mana_detach() uncfg's the vport an RDMA client can
> call mana_cfg_vport() and bump vport_use_count, and then mana_attach()
> will fail with -EBUSY when it re-cfg's the vport.
> 
> There is also no equivalent of the '!port_is_up && vport_use_count'
> early -EBUSY check that mana_set_channels() performs, so is the flag
> allowed to be changed silently while an RDMA client is using the vport?
> 
Agreed. Will add the same vport_mutex + channel_changing pattern used by
mana_set_channels() and mana_per_port_queue_reset_work_handler() to
block RDMA from grabbing the vport during the detach/attach window.

> > +		if (err) {
> > +			netdev_err(ndev,
> > +				   "Insufficient memory for new allocations\n");
> > +			apc->priv_flags = old_priv_flags;
> > +			return err;
> > +		}
> > +
> > +		err = mana_detach(ndev, false);
> 
> [High]
> mana_detach() begins with ASSERT_RTNL(), and so does mana_attach()
> called below.  Is RTNL guaranteed to be held on the SET priv-flags path?
> 
> mana registers net_shaper_ops in mana_devops, which makes it an
> ops-locked driver, so netdev_need_ops_lock() is true and the ethtool
> core acquires rtnl only when the driver opts in via
> ETHTOOL_OP_NEEDS_RTNL_* for that specific command.
> 
> Looking at mana_ethtool_ops.op_needs_rtnl:
> 
> 	.op_needs_rtnl		= ETHTOOL_OP_NEEDS_RTNL_SCHANNELS |
> 				  ETHTOOL_OP_NEEDS_RTNL_SRINGPARAM |
> 				  ETHTOOL_OP_NEEDS_RTNL_GLINK,
> 
> ETHTOOL_OP_NEEDS_RTNL_SPFLAGS is not listed, so ASSERT_RTNL() in the
> detach/attach helpers below will WARN, and the code races with
> RTNL-serialized mutators (mana_open, mana_close, mana_change_mtu,
> mana_tx_timeout) that read and write apc->port_is_up, apc->rxqs,
> apc->tx_qp and apc->port_st_save.
> 
> Should ETHTOOL_OP_NEEDS_RTNL_SPFLAGS be added to op_needs_rtnl, matching
> what is already done for SET channels?
> 
Agreed, Will add ETHTOOL_OP_NEEDS_RTNL_SPFLAGS to .op_needs_rtnl.
> > +		if (err) {
> > +			netdev_err(ndev, "mana_detach failed: %d\n", err);
> > +			apc->priv_flags = old_priv_flags;
> > +
> > +			/* Port is in an inconsistent state. Restore
> > +			 * 'port_is_up' so that queue reset work handler
> > +			 * can properly detach and re-attach.
> > +			 */
> > +			apc->port_is_up = true;
> > +			schedule_port_reset = true;
> > +			goto out;
> > +		}
> > +
> > +		err = mana_attach(ndev);
> > +		if (err) {
> > +			netdev_err(ndev, "mana_attach failed: %d\n", err);
> > +			apc->priv_flags = old_priv_flags;
> > +
> > +			/* Restore 'port_is_up' so the reset work handler
> > +			 * can properly detach/attach. Without this,
> > +			 * the handler sees port_is_up=false and skips
> > +			 * queue allocation, leaving the port dead.
> > +			 */
> > +			apc->port_is_up = true;
> > +			schedule_port_reset = true;
> 
> [High]
> On this branch, mana_detach() has already succeeded and run
> mana_cleanup_port_context(), so apc->rxqs is NULL and queues are torn
> down.  Setting apc->port_is_up = true here between now and when
> queue_reset_work runs opens a window where callers gate on port_is_up
> as their "safe to access rxqs/tx_qp" predicate.
> 
> For example, mana_get_ethtool_stats() does:
> 
> 	if (!apc->port_is_up)
> 		return;
> 	...
> 	rxq = apc->rxqs[q];
> 	...
> 
> A concurrent ethtool -S invocation during that window will pass the
> port_is_up gate and dereference apc->rxqs[q]->stats on a NULL rxqs.
> 
> Is the port_is_up restore actually needed for the reset work handler's
> mana_detach() call?  On the reset path, mana_detach() takes its early
> return when !netif_device_present(ndev):
> 
> 	if (!from_close && !netif_device_present(ndev))
> 		return 0;
> 
> That early return does not touch apc->port_st_save, so the saved state
> from the earlier successful mana_detach() should already be intact for
> the follow-up mana_attach().
> 
> There is also no smp_wmb() paired with this write, unlike the pattern
> used inside mana_detach()/mana_attach() proper.
> 

For the mana_attach() failure path: agreed, will remove port_is_up =
true. After a successful mana_detach(ndev, false), netif_device_detach()
has already been called so the reset handler's mana_detach() takes the
early return via !netif_device_present(), preserving port_st_save for
the subsequent mana_attach().

For the mana_detach() failure path: keeping port_is_up = true is
intentional. Here netif_device_detach() was never reached, so the reset
handler's mana_detach() would re-enter fully and overwrite port_st_save
from port_is_up, if left as false, mana_attach() would skip queue
allocation and leave the port dead.


> > +		}
> > +	}
> > +
> > +out:
> > +	mana_pre_dealloc_rxbufs(apc);
> > +
> > +	if (schedule_port_reset)
> > +		queue_work(apc->ac->per_port_queue_reset_wq,
> > +			   &apc->queue_reset_work);
> > +
> > +	return err;
> > +}
> > +
> 
> [ ... remaining hunks unrelated to the comments above ... ]

Thanks for the review comments, I will address them in the next
version.


Regards
Dipayaan Roy

^ permalink raw reply

* Re: [PATCH net] rds: Fix inet6_addr_lst NULL dereference when IPv6 is disabled
From: Allison Henderson @ 2026-07-09 15:41 UTC (permalink / raw)
  To: Ilia Gavrilov
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Ka-Cheong Poon, Santosh Shilimkar,
	netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	rds-devel@oss.oracle.com, linux-kernel@vger.kernel.org,
	lvc-project@linuxtesting.org
In-Reply-To: <20260708115922.2226279-1-Ilia.Gavrilov@infotecs.ru>

On Wed, 2026-07-08 at 11:59 +0000, Ilia Gavrilov wrote:
> When booting with the 'ipv6.disable=1' parameter, inet6_addr_lst
> is never initialized because inet6_init() exits before addrconf_init()
> is called to initialize it. An attempt to bind an RDS socket to
> an ipv6 address results in a crash in __ipv6_chk_addr_and_flags()

Hello Ilia,

Thanks for the catch.  Some comments below:

> 
> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> RIP: 0010:__ipv6_chk_addr_and_flags+0x1df/0x7e0
> Call Trace:
>  <TASK>
>  ipv6_chk_addr+0x3b/0x50
>  rds_tcp_laddr_check+0x155/0x3b0 [rds_tcp]
>  rds_trans_get_preferred+0x15d/0x2d0 [rds]
>  ? trace_hardirqs_on+0x2d/0x110
>  rds_bind+0x1433/0x1d60 [rds]
>  ? rds_remove_bound+0xd50/0xd50 [rds]
>  ? aa_af_perm+0x250/0x250
>  ? __might_fault+0xde/0x190
>  ? __sys_bind+0x1dc/0x210
>  __sys_bind+0x1dc/0x210
>  ? __ia32_sys_socketpair+0x100/0x100
>  ? restore_fpregs_from_fpstate+0x53/0x100
>  __x64_sys_bind+0x73/0xb0
>  ? syscall_enter_from_user_mode+0x1c/0x50
>  do_syscall_64+0x34/0x80
>  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
> RIP: 0033:0x7f47f8269ea9
>  </TASK>
> 
> The following code reproduces the issue:
> 
> struct sockaddr_in6 addr;
> s = socket(PF_RDS, SOCK_SEQPACKET, 0);
> 
> memset(&addr, 0, sizeof(addr));
> inet_pton(AF_INET6, ADDRESS, &addr.sin6_addr);
> addr.sin6_family = AF_INET6;
> addr.sin6_port = htons(PORT);
> 
> bind(s, &addr, sizeof(addr);
nit: missing paren here

> 
> Found by InfoTeCS on behalf of Linux Verification Center
> (linuxtesting.org) with Syzkaller.
> 
> Fixes: eee2fa6ab322 ("rds: Changing IP address internal representation to struct in6_addr")
> Fixes: 1e2b44e78eea ("rds: Enable RDS IPv6 support")
> Signed-off-by: Ilia Gavrilov <Ilia.Gavrilov@infotecs.ru>
> ---
>  net/rds/ib.c  | 4 ++++
>  net/rds/tcp.c | 8 +++++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/net/rds/ib.c b/net/rds/ib.c
> index 39f87272e071..8f9cf491984f 100644
> --- a/net/rds/ib.c
> +++ b/net/rds/ib.c
> @@ -429,6 +429,10 @@ static int rds_ib_laddr_check_cm(struct net *net, const struct in6_addr *addr,
>  		sa = (struct sockaddr *)&sin;
>  	} else {
>  #if IS_ENABLED(CONFIG_IPV6)
> +		if (!ipv6_mod_enabled()) {
> +			ret = -EADDRNOTAVAIL;
> +			goto out;
> +		}
>  		memset(&sin6, 0, sizeof(sin6));
>  		sin6.sin6_family = AF_INET6;
>  		sin6.sin6_addr = *addr;
> diff --git a/net/rds/tcp.c b/net/rds/tcp.c
> index a1de114d5e2e..955d92277d5a 100644
> --- a/net/rds/tcp.c
> +++ b/net/rds/tcp.c
> @@ -366,9 +366,11 @@ int rds_tcp_laddr_check(struct net *net, const struct in6_addr *addr,
>  		rcu_read_unlock();
>  	}
>  #if IS_ENABLED(CONFIG_IPV6)
> -	ret = ipv6_chk_addr(net, addr, dev, 0);
> -	if (ret)
> -		return 0;
> +	if (ipv6_mod_enabled()) {
> +		ret = ipv6_chk_addr(net, addr, dev, 0);
> +		if (ret)
> +			return 0;
> +	}

There's another ipv6_chk_addr() in __rds_find_ifindex() with the same issue that affects inbound link-local IPv6
connects.  Can you add a similar guard there too?  Then I think that should cover all the points of exposure.  Thanks
for working on this.

Allison

>  #endif
>  	return -EADDRNOTAVAIL;
>  }


^ permalink raw reply

* Re: [GIT PULL] Networking for v7.2-rc3
From: pr-tracker-bot @ 2026-07-09 15:37 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: torvalds, kuba, davem, netdev, linux-kernel
In-Reply-To: <20260709134751.242764-1-pabeni@redhat.com>

The pull request you sent on Thu,  9 Jul 2026 15:47:51 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git net-7.2-rc3

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/2c7c88a412aa6d09cd04b414211b4ef8553b5309

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

^ permalink raw reply

* Re: Please backport bridge multicast exponential field encoding fix series to 6.1.y/6.6.y/6.12.y/6.18.y/7.0.y
From: Ujjal Roy @ 2026-07-09 15:21 UTC (permalink / raw)
  To: Greg KH
  Cc: Linux Stable, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Nikolay Aleksandrov, Ido Schimmel,
	David Ahern, Shuah Khan, Andy Roulin, Yong Wang, Petr Machata,
	Ujjal Roy, bridge, netdev, linux-kernel, linux-kselftest
In-Reply-To: <CAE2MWkn3L7V3x8i0F-soGLxsBBo_Umgs1pJ3FwCw1OW7=U55zg@mail.gmail.com>

On Thu, Jul 9, 2026 at 8:43 PM Ujjal Roy <royujjal@gmail.com> wrote:
>
> On Thu, Jul 9, 2026 at 7:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Jul 09, 2026 at 06:35:04PM +0530, Ujjal Roy wrote:
> > > On Thu, Jul 9, 2026 at 6:23 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Thu, Jul 09, 2026 at 06:12:40PM +0530, Ujjal Roy wrote:
> > > > > On Thu, Jul 9, 2026 at 4:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > > >
> > > > > > On Thu, Jul 09, 2026 at 10:13:27AM +0000, Ujjal Roy wrote:
> > > > > > > Hi Greg,
> > > > > > >
> > > > > > > Please consider backporting the following bridge multicast fix series to 6.1.y, 6.6.y, 6.12.y, 6.18.y and 7.0.y.
> > > > > > >
> > > > > > > 726fa7da2d8c ("ipv4: igmp: get rid of IGMPV3_{QQIC,MRC} and simplify calculation")
> > > > > > > 12cfb4ecc471 ("ipv6: mld: rename mldv2_mrc() and add mldv2_qqi()")
> > > > > > > 95bfd196f0dc ("ipv4: igmp: encode multicast exponential fields")
> > > > > > > e51560f4220a ("ipv6: mld: encode multicast exponential fields")
> > > > > > > 529dbe762de0 ("selftests: net: bridge: add MRC and QQIC field encoding tests")
> > > > > >
> > > > > > Why is any of this needed in older kernels?
> > > > > >
> > > > > > And 7.0.y is long end-of-life.
> > > > > >
> > > > > > And why, if this does fix issues, was it not tagged for stable to start
> > > > > > with?
> > > > > >
> > > > > > thanks,
> > > > > >
> > > > > > greg k-h
> > > > >
> > > > > I already explained this in the email thread, "Please backport bridge
> > > > > multicast exponential field encoding fix series to stable kernels".
> > > >
> > > > Sorry, but that's not here (remember, some of us get 1000+ emails a
> > > > day.)
> > > >
> > > > Please explain why patches need to be backported when asking for them to
> > > > be backported.
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > Sorry for breaking the thread. I understand your point, I will
> > > maintain this in the future.
> > > How should I send the patchset that addresses the conflicts on 5.10.y
> > > and 7.1.y? Shall I send the conflicts patchset as a series via a
> > > different thread or how? I've never done this before, so I'm asking.
> > >
> > > Here is the explanation for why the patches need to be backported:
> > >
> > > History: The multicast stack currently supports decoding of IGMPv3 and
> > > MLDv2 exponential timer field encodings, but lacks the corresponding
> > > encoding logic when generating multicast query packets. As a result,
> > > query intervals and response codes exceeding the linear encoding range
> > > can be transmitted incorrectly. This can cause multicast queriers and
> > > listeners to interpret different timing values, resulting in protocol
> > > interoperability issues, membership timeouts, and premature multicast
> > > group expiration.
> > >
> > > Testing: The series adds the missing encoding support for both IGMPv3
> > > and MLDv2 and includes selftests that validate the behavior.
> > > I backported the series to v6.6.123.2 and verified the accompanying
> > > selftests. The selftests fail on the unpatched kernel and pass after
> > > applying the series, demonstrating both the bug and the effectiveness
> > > of the fix.
> > >
> > > Given that this is a protocol correctness issue affecting multicast
> > > query generation, please consider backporting the complete series to
> > > all applicable stable kernels.
> > >
> >
> > But this really seems like a new feature being added, it's not fixing a
> > regression of something that previously worked, right?
> >
> > WHy can't people just update to the latest kernel release to get this if
> > they need it for their environments?
> >
> > thanks,
> >
> > greg k-h
>
> This is a corner case when people set the query timer value higher
> than 128. People usually use the default value and don't change it, so
> they may not encounter this issue. But I found it when I changed the
> value during some extensive validation of protocol timeouts.
>
> If one host doesn't have this fix, clients will observe premature
> multicast group expiration. For example, a set-top box channel might
> disconnect early.
>
> But we can ignore this until few more people request this fix for older kernels.

Last two patches actually fix the timeout issue. Below one fixes QQIC
encoding BUG for IPv4.

95bfd196f0dc ("ipv4: igmp: encode multicast exponential fields")
@@ -780,11 +780,9 @@ static struct sk_buff
*br_ip4_multicast_alloc_query(struct net_bridge *br,
        case 3:
                ihv3 = igmpv3_query_hdr(skb);
                ihv3->type = IGMP_HOST_MEMBERSHIP_QUERY;
-               ihv3->code = (group ? br->multicast_last_member_interval :
-                                     br->multicast_query_response_interval) /
-                            (HZ / IGMP_TIMER_SCALE);
+               ihv3->code = igmpv3_mrc(mrt / (HZ / IGMP_TIMER_SCALE));
                ihv3->group = group;
-               ihv3->qqic = br->multicast_query_interval / HZ;
+               ihv3->qqic = igmpv3_qqic(br->multicast_query_interval
/ HZ);  // This was not encoding earlier

^ permalink raw reply

* Re: [PATCH RESEND 1/1] macvlan: allow source mode devices along with passthru
From: Simon Horman @ 2026-07-09 15:18 UTC (permalink / raw)
  To: Thomas Martitz
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel
In-Reply-To: <4ffe7fe8-fdbb-42a1-b2b1-d11a351b2e16@fritz.com>

On Thu, Jul 09, 2026 at 10:49:36AM +0200, Thomas Martitz wrote:
> Hi Simon,
> 
> Am 08.07.26 um 18:43 schrieb Simon Horman:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > Full review at: https://sashiko.dev
> 
> Thanks for feedback. The remarks are valid, I agree. I also begin to better
> understand the macvlan code so I'm a bit more confident now about which of
> the existing "macvlan_passthru" checks need treatment. I'll send an updated
> patch.
> 
> General question though: The initial posting triggered an AI review (I see
> that now, I didn't know about https://sashiko.dev before). But it did not
> trigger an email. Can you explain why? Because I accidentally dropped the RFC
> tag from the mail subject?

Hi Thomas,

If I understand correctly you are asking why the sashiko.dev review
wasn't automatically emailed to you.

If so, I believe configuration of that feature is on a per ML basis,
and it is not enabled for netdev.

In principle the sashiko review(s) show up in patchwork
(although, perhaps not completely reliably). I would
suggest checking there.

...

^ permalink raw reply

* RE: [PATCH net] can: esd_usb: kill anchored URBs before freeing netdevs
From: Jagielski, Jedrzej @ 2026-07-09 15:17 UTC (permalink / raw)
  To: Fan Wu, linux-can@vger.kernel.org
  Cc: frank.jungclaus@esd.eu, socketcan@esd.eu, mkl@pengutronix.de,
	mailhol@kernel.org, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org
In-Reply-To: <20260709104620.133765-1-fanwu01@zju.edu.cn>

From: Fan Wu <fanwu01@zju.edu.cn> 
Sent: Thursday, July 9, 2026 12:46 PM

>esd_usb_disconnect() frees each CAN netdev with free_candev() inside
>its per-netdev loop and only calls unlink_all_urbs(dev) afterwards.
>The per-netdev private data (struct esd_usb_net_priv) is embedded in
>the net_device allocation returned by alloc_candev(), so once
>free_candev() has run, dev->nets[i] points to freed memory.
>unlink_all_urbs() then dereferences the freed dev->nets[i] to kill the
>per-netdev TX anchor (usb_kill_anchored_urbs(&priv->tx_submitted)),
>clear active_tx_jobs, and reset priv->tx_contexts[].
>
>Reorder the teardown so the anchored URBs are killed before the netdevs
>are freed, matching other CAN/USB drivers in the same directory such as
>ems_usb, usb_8dev and mcba_usb, which unregister, then unlink, then
>free: unregister the netdevs first (which stops their TX queues), call
>unlink_all_urbs(dev) once, then free the netdevs.
>
>This issue was found by an in-house static analysis tool.
>
>Fixes: 96d8e90382dc336b5de401164597edfdc2e8d9f1 ("can: Add driver for esd CAN-USB/2 device")

Hi Fan,

no need to put whole SHA, 12 first chars is enough




^ permalink raw reply

* Re: [PATCH] wanxl: Remove pci_map_single_debug()
From: Simon Horman @ 2026-07-09 15:14 UTC (permalink / raw)
  To: Bence Csokas
  Cc: Krzysztof Halasa, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
In-Reply-To: <20260703-wanxl-cleanup-v1-1-d5deb23fa941@arm.com>

On Fri, Jul 03, 2026 at 10:21:42AM +0200, Bence Csokas wrote:
> Since commit 24dd377a76b0 ("wan: wanxl: switch from 'pci_' to 'dma_' API")
> this has been dead code anyways. The pci_map_single() function it attempts
> to redefine has been removed in 7968778914e5 ("PCI: Remove the deprecated
> "pci-dma-compat.h" API").
> 
> Signed-off-by: Bence Csokas <bence.csokas@arm.com>

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply

* Re: Please backport bridge multicast exponential field encoding fix series to 6.1.y/6.6.y/6.12.y/6.18.y/7.0.y
From: Ujjal Roy @ 2026-07-09 15:13 UTC (permalink / raw)
  To: Greg KH
  Cc: Linux Stable, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Nikolay Aleksandrov, Ido Schimmel,
	David Ahern, Shuah Khan, Andy Roulin, Yong Wang, Petr Machata,
	Ujjal Roy, bridge, netdev, linux-kernel, linux-kselftest
In-Reply-To: <2026070948-lively-exchange-a458@gregkh>

On Thu, Jul 9, 2026 at 7:52 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jul 09, 2026 at 06:35:04PM +0530, Ujjal Roy wrote:
> > On Thu, Jul 9, 2026 at 6:23 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Jul 09, 2026 at 06:12:40PM +0530, Ujjal Roy wrote:
> > > > On Thu, Jul 9, 2026 at 4:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > On Thu, Jul 09, 2026 at 10:13:27AM +0000, Ujjal Roy wrote:
> > > > > > Hi Greg,
> > > > > >
> > > > > > Please consider backporting the following bridge multicast fix series to 6.1.y, 6.6.y, 6.12.y, 6.18.y and 7.0.y.
> > > > > >
> > > > > > 726fa7da2d8c ("ipv4: igmp: get rid of IGMPV3_{QQIC,MRC} and simplify calculation")
> > > > > > 12cfb4ecc471 ("ipv6: mld: rename mldv2_mrc() and add mldv2_qqi()")
> > > > > > 95bfd196f0dc ("ipv4: igmp: encode multicast exponential fields")
> > > > > > e51560f4220a ("ipv6: mld: encode multicast exponential fields")
> > > > > > 529dbe762de0 ("selftests: net: bridge: add MRC and QQIC field encoding tests")
> > > > >
> > > > > Why is any of this needed in older kernels?
> > > > >
> > > > > And 7.0.y is long end-of-life.
> > > > >
> > > > > And why, if this does fix issues, was it not tagged for stable to start
> > > > > with?
> > > > >
> > > > > thanks,
> > > > >
> > > > > greg k-h
> > > >
> > > > I already explained this in the email thread, "Please backport bridge
> > > > multicast exponential field encoding fix series to stable kernels".
> > >
> > > Sorry, but that's not here (remember, some of us get 1000+ emails a
> > > day.)
> > >
> > > Please explain why patches need to be backported when asking for them to
> > > be backported.
> > >
> > > thanks,
> > >
> > > greg k-h
> >
> > Sorry for breaking the thread. I understand your point, I will
> > maintain this in the future.
> > How should I send the patchset that addresses the conflicts on 5.10.y
> > and 7.1.y? Shall I send the conflicts patchset as a series via a
> > different thread or how? I've never done this before, so I'm asking.
> >
> > Here is the explanation for why the patches need to be backported:
> >
> > History: The multicast stack currently supports decoding of IGMPv3 and
> > MLDv2 exponential timer field encodings, but lacks the corresponding
> > encoding logic when generating multicast query packets. As a result,
> > query intervals and response codes exceeding the linear encoding range
> > can be transmitted incorrectly. This can cause multicast queriers and
> > listeners to interpret different timing values, resulting in protocol
> > interoperability issues, membership timeouts, and premature multicast
> > group expiration.
> >
> > Testing: The series adds the missing encoding support for both IGMPv3
> > and MLDv2 and includes selftests that validate the behavior.
> > I backported the series to v6.6.123.2 and verified the accompanying
> > selftests. The selftests fail on the unpatched kernel and pass after
> > applying the series, demonstrating both the bug and the effectiveness
> > of the fix.
> >
> > Given that this is a protocol correctness issue affecting multicast
> > query generation, please consider backporting the complete series to
> > all applicable stable kernels.
> >
>
> But this really seems like a new feature being added, it's not fixing a
> regression of something that previously worked, right?
>
> WHy can't people just update to the latest kernel release to get this if
> they need it for their environments?
>
> thanks,
>
> greg k-h

This is a corner case when people set the query timer value higher
than 128. People usually use the default value and don't change it, so
they may not encounter this issue. But I found it when I changed the
value during some extensive validation of protocol timeouts.

If one host doesn't have this fix, clients will observe premature
multicast group expiration. For example, a set-top box channel might
disconnect early.

But we can ignore this until few more people request this fix for older kernels.

^ permalink raw reply

* Re: [PATCH net-next v4 2/2] net: dsa: realtek: rtl8365mb: add HSGMII support for RTL8367S
From: Mieczyslaw Nalewaj @ 2026-07-09 15:02 UTC (permalink / raw)
  To: Johan Alvarado
  Cc: linusw, alsi, andrew, olteanv, kuba, davem, edumazet, pabeni,
	linux, luizluca, maxime.chevallier, netdev, linux-kernel
In-Reply-To: <0100019f45cc4fe2-2464b68a-dfcf-4e93-9bfc-8445370d8e3e-000000@email.amazonses.com>

Hi Johan

On 7/9/2026 9:34 AM, Johan Alvarado wrote:
> Hi Mieczyslaw,
> 
> On 7/3/2026 5:12 PM, Mieczyslaw Nalewaj wrote:
>> Johan, looks like you forgot to include the scheduler bandwidth bits
>> for the CPU port. Without this, HSGMII will still be capped at the
>> old SGMII rate limits. Something like:
> 
> Good catch. I had dropped these on the assumption that the vendor
> driver never touches them, but your mail made me look again: the
> "Max rate" block in dal_rtl8367c_switch_init() programs exactly these
> registers for EXT_PORT0 - the port 6 ingress and egress rate limiters
> are raised to 0x7FFFF via rtl8367c_setAsicPortIngressBandwidth() and
> rtl8367c_setAsicPortEgressRate(), followed by a raw write of 0x0007
> to 0x03fa (LINE_RATE_HSG_H). I had missed it because those writes go
> through the indexed register macros and a raw address rather than the
> PORT6_* register names.
> 
> On my board this was masked by the bootloader: U-Boot runs the vendor
> switch init (rtk_switch_init() from the ipq5018 gmac driver) and
> leaves the limits raised, but the kernel driver then issues a full
> chip reset in rtl8365mb_setup(), which puts these registers back to
> their reset defaults. With the ingress/egress limiters back at their
> defaults that would indeed cap HSGMII at roughly 1 Gbps, and I could
> not have seen it in testing because my downstream ports are 1G only
> (as noted in the cover letter, 2.5G line rate is unverified on my
> hardware).
> 
> One caveat I want to be transparent about: for the same reason I
> cannot verify the cap itself, and the vendor documentation leaves
> some room for doubt. The rtl8367c_setAsicPortIngressBandwidth()
> comment describes the bandwidth value 0x1FFFF as "disable", and
> that is exactly the reset default - I read back both limiters as
> 0x1FFFF (LSBs at 0xFFFF, MSBs at 1) on my RTL8367S after the
> driver's chip reset - so the limiter may in fact be disabled out
> of reset (though that note looks like stale text from earlier
> 17-bit-granularity chips, given this family's maximum is 0x7FFFF).
> 
> Raising the limiters is safe under either reading: if the default
> really is a ~1 Gbps limit, the write removes it, and if it means
> "disabled", the new value (0x7FFFF * 8 Kbps, about 4.19 Gbps, well
> above the 2.5G line rate) never engages.
> 
> While I cannot push more than 1 Gbps, I did verify that these
> registers really are the live rate limiters for the SerDes CPU
> port, by going the other way: on a build with the setup-time
> writes described below already in place, I lowered the limiters at
> runtime (via regmap debugfs) to 12500 * 8 Kbps = 100 Mbps
> and ran iperf3 across the CPU port. Throughput clamps from ~460 to
> ~95 Mbps with the ingress limiter lowered (CPU-originated traffic)
> and from ~600 to ~96 Mbps with the egress limiter lowered
> (CPU-bound traffic); restoring the maximum returns both directions
> to baseline. The ~95/100 ratio is the expected TCP goodput for a
> 100 Mbps wire-rate limit, so both limiters demonstrably meter this
> port at the documented 8 Kbps granularity. The only thing this
> cannot settle is the meaning of the 0x1FFFF default itself
> (~1.048 Gbps cap vs. disabled), since any value at or above the
> traffic I can generate behaves identically. If you have an
> RTL8367S setup that can push more than 1 Gbps across the HSGMII
> CPU port, a before/after measurement would be a very welcome
> confirmation of the cap.
> 
> So I'll add the writes in v5, following the vendor init: raise the
> port 6 ingress/egress limiters to their maximum once at setup time,
> rather than per interface mode. Two differences from your snippet:
> the vendor init writes the full 19-bit rate value, i.e. the LSB
> halves too (0x00cf and 0x0398 set to 0xFFFF), so I'll program both
> halves rather than only the *_CTRL1 registers, to avoid depending on
> the reset defaults of the LSB registers. And LINE_RATE_HSG_H needs no
> new write at all: the driver's common init jam table has always
> written 0x03fa = 0x0007 in rtl8365mb_switch_init(), so that one is
> already at its maximum on every probe (which also matches the 7 you
> read back for it). I have this running on my board already: with the
> setup-time writes in place both limiters read back 0x7FFFF after
> probe, and the HSGMII link comes up and forwards as before.
> 
> As an aside, a per-interface-mode variant of these writes does exist
> in the vendor code, but only for other chip families: in
> rtl8367c_setAsicPortExtMode(), the chip id 0x0652/0x6368 branch
> programs its rate registers (0x0130/0x039f/0x03fa) on every mode
> change, with 7s for EXT_HSGMII and lower values otherwise, and the
> 0x0801/0x6511 branch writes 0x00d0/0x0399/0x03fa = 7 only for
> EXT_HSGMII (which is where the pattern in your snippet comes from).
> For the 0x6367 family the ext-mode path never touches them, and the
> vendor only programs them once, unconditionally, in switch_init -
> which is another reason I prefer the one-time setup write over doing
> it from the PCS path.
> 
> For completeness, the same vendor block also touches three related
> bits: rtl8367c_setAsicPortIngressBandwidth() additionally clears the
> port 6 ingress metering pre-IFG bit and sets the "flow control when
> ingress rate exceeded" bit (PORT6_MISC_CFG, 0x00ce bits 10/11), and
> switch_init follows the egress rate write with
> rtl8367c_setAsicPortEgressRateIfg(ENABLED), which sets the global
> "egress rate accounting includes IFG" bit (SCHEDULE_WFQ_CTRL 0x0300
> bit 0). With the limiters raised to maximum the meter never engages,
> so neither the exceed action nor the IFG accounting has any
> observable effect. I plan to leave those three bits at their reset
> defaults and program only the rate values and LINE_RATE_HSG_H, to
> keep the driver's footprint on the QoS block minimal - but I'm happy
> to mirror the vendor init exactly if you'd rather not deviate from
> it.
> 
>> One more thing while we're on this: I checked the equivalent
>> registers for RGMII on the RTL8367S, and they come out to 1, 1, 7
>> respectively (INGRESSBW_PORT6_RATE_CTRL1, PORT6_EGRESSBW_CTRL1,
>> LINE_RATE_HSG_H). For correctness these should be set to those
>> values in the RGMII path as well, rather than left at whatever
>> reset/default state they're currently in.
> 
> The 1/1 you read back for the rate limiter MSBs match the reset
> defaults I see on my RTL8367S as well, and the 7 for LINE_RATE_HSG_H
> is the value the common init jam table writes on every probe, as
> above. Since the driver hard-resets the chip and then replays the
> init jam on every probe, those values are already guaranteed in
> the RGMII case, so writing them again from the RGMII path would be
> a no-op. These fields are also the MSBs of the per-port QoS rate
> limiters rather than link-mode configuration, so I would prefer to
> touch them as little as possible from the link paths. My plan for
> v5 is therefore the single write at setup time described above,
> which covers both SGMII and HSGMII and has no functional effect on
> RGMII either way: both the default value (~1.048 Gbps, if it acts
> as a limit at all) and the raised maximum sit above the 1G RGMII
> line rate, so the meter never engages there. Let me know if you
> see a problem with that.
> 
> Best regards,
> Johan

Performance degradation without setting these registers can only be
observed in HSGMII mode with simultaneous load on multiple LAN ports.

This was noticed during work on adding support for the Mercusys MR85X
router with DSA to OpenWrt: https://github.com/openwrt/openwrt/pull/19445#issuecomment-4505613294

Regards

^ permalink raw reply

* RE: [PATCH net-next v7 08/15] net: ethernet: oa_tc6: Support for hardware timestamp
From: Selvamani Rajagopal @ 2026-07-09 15:02 UTC (permalink / raw)
  To: Vadim Fedorenko, Andrew Lunn, Piergiorgio Beruto, Heiner Kallweit,
	Russell King, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, Parthiban Veerasooran, Richard Cochran,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Simon Horman,
	Jonathan Corbet, Shuah Khan
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-doc@vger.kernel.org, Jerry Ray
In-Reply-To: <4a847a6b-7837-4044-8a87-0ddac47725f8@linux.dev>

> -----Original Message-----
> From: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> Sent: Thursday, July 9, 2026 3:45 AM
> Subject: Re: [PATCH net-next v7 08/15] net: ethernet: oa_tc6: Support for hardware
> timestamp
> 
> 
> > +
> > + if (cmd == SIOCSHWTSTAMP || cmd == SIOCGHWTSTAMP)
> > + return oa_tc6_tstamp_ioctl(tc6, rq, cmd);
> 
> ioctl interface for HW timestamp configuration is deprecated, kernel
> code was recently cleaned up to aviod these ioctl commands in favor of
> ndo_hwtstamp_get/ndo_hwtstamp_set callbacks. New drivers must not use
> deprecated ioctl commands.
> 

I see. Sure. Will remove it. 

^ permalink raw reply

* Re: [PATCH net v2 2/2] net: fman: add error cleanup path in fman_probe
From: Simon Horman @ 2026-07-09 14:56 UTC (permalink / raw)
  To: zhaojinming
  Cc: Simon Horman, madalin.bucur, sean.anderson, netdev, andrew+netdev,
	davem, edumazet, kuba, pabeni, igal.liberman, linux-kernel
In-Reply-To: <20260703074324.907294-2-zhaojinming@uniontech.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
net: fman: add error cleanup path in fman_probe

This commit introduces a new error cleanup path in fman_probe() to prevent
memory leaks when fman_init() or devm_request_irq() fail. It adds
fman_muram_finish() and fman_free_resources() to properly tear down
allocations and states.

> diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
> index 6947f3bc7c87e..752c0df0e17c5 100644
> --- a/drivers/net/ethernet/freescale/fman/fman.c
> +++ b/drivers/net/ethernet/freescale/fman/fman.c

[ ... ]

> +static void fman_free_resources(struct fman *fman, struct device *dev,
> +				bool irq_registered)
> +{
> +	/* Free IRQs first while fman is still valid */
> +	if (irq_registered) {
> +		if (fman->dts_params.err_irq != 0)
> +			devm_free_irq(dev, fman->dts_params.err_irq, fman);
> +		devm_free_irq(dev, fman->dts_params.irq, fman);
> +	}
> +
> +	kfree(fman->keygen);
> +	free_init_resources(fman);

[Severity: High]
If fman_init() fails internally (for example, in dma_init() or
keygen_init()), it calls free_init_resources(fman) on its own failure path.
However, it does not clear fman->fifo_offset or fman->cam_offset.

When fman_probe() catches this error, it jumps to err_no_irq and calls
fman_free_resources(), which will invoke free_init_resources(fman) a second
time.

Does this cause a double-free of the MURAM resources?

[Severity: High]
This is a pre-existing issue, but if fman_muram_alloc() fails during
fman_init(), it returns an error code (such as -ENOMEM) which is stored
directly in fman->fifo_offset.

Since the error code is non-zero, free_init_resources() will evaluate
if (fman->fifo_offset) as true and pass the error pointer to
fman_muram_free_mem().

Does attempting to free this pseudo-random error code cause corruption in
the gen_pool allocator?

> +	kfree(fman->cfg);
> +	fman_muram_finish(fman->muram);
> +	kfree(fman->state);
> +	kfree(fman);
> +}

[ ... ]

> @@ -2844,7 +2863,7 @@ static int fman_probe(struct platform_device *of_dev)
>  	if (err < 0) {
>  		dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
>  			__func__, fman->dts_params.irq, err);
> -		return err;
> +		goto err_no_irq;
>  	}
>  
>  	if (fman->dts_params.err_irq != 0) {

[Severity: High]
If devm_request_irq() fails here, fman_init() has already succeeded. At the
end of fman_init(), the FMan hardware is fully enabled via enable(fman, cfg).

Jumping to err_no_irq calls fman_free_resources(), which destroys the
structures and unmaps the MURAM memory. However, there doesn't appear to be
any call to stop or disable the active hardware.

Will the hardware continue to operate and perform DMA to the freed MURAM
addresses, potentially causing memory corruption?

^ permalink raw reply

* Re: [PATCH net] tipc: guard against empty buffer list in tipc_node_xmit()
From: Weiming Shi @ 2026-07-09 14:56 UTC (permalink / raw)
  To: Tung Quang Nguyen
  Cc: Xiang Mei, netdev@vger.kernel.org,
	tipc-discussion@lists.sourceforge.net, Jon Maloy, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, David S . Miller, Simon Horman
In-Reply-To: <GV1P189MB1988683F0940A1DC37094256C6FE2@GV1P189MB1988.EURP189.PROD.OUTLOOK.COM>

Tung Quang Nguyen <tung.quang.nguyen@est.tech> 于2026年7月9日周四 14:33写道:
>
> >Subject: [PATCH net] tipc: guard against empty buffer list in tipc_node_xmit()
> >
> >tipc_node_xmit() dispatches a buffer list either to the bearer path via
> >tipc_link_xmit() or, when the destination node lives in a sibling network
> >namespace on the same host (n->peer_net set), to tipc_lxc_xmit(). The bearer
> >path returns early on an empty list, but tipc_node_xmit() does not, and
> >tipc_lxc_xmit() dereferences the first buffer without checking:
> >
> >       struct tipc_msg *hdr = buf_msg(skb_peek(list));
> >
> >named_distribute() can hand tipc_node_xmit() an empty list. It bails out early
> >when named_prepare_buf() fails its GFP_ATOMIC allocation, leaving the queue
> >empty, and tipc_named_node_up() then calls tipc_node_xmit() on it
> >unconditionally. On the intra-host container path skb_peek() returns NULL and
> >msg_user() reads through it.
> >
> >The TIPC configuration ops are flagged GENL_UNS_ADMIN_PERM, so an
> >unprivileged user can reach this via
> >unshare(CLONE_NEWUSER|CLONE_NEWNET).
> >
> >  Oops: general protection fault, probably for non-canonical address
> >0xdffffc000000001b
> >  KASAN: null-ptr-deref in range [0x00000000000000d8-0x00000000000000df]
> >  CPU: 0 Comm: ksoftirqd/0
> >  RIP: 0010:tipc_lxc_xmit (net/tipc/msg.h:202 net/tipc/node.c:1629)
> >  Call Trace:
> >   tipc_node_xmit (net/tipc/node.c:1721)
> >   tipc_named_node_up (net/tipc/name_distr.c:223)
> >   tipc_node_write_unlock (net/tipc/node.c:428)
> >   tipc_rcv (net/tipc/node.c:2189)
> >   tipc_l2_rcv_msg (net/tipc/bearer.c:670)
> >
> >Return early from tipc_node_xmit() when the list is empty.
> >
> >Fixes: f73b12812a3d ("tipc: improve throughput between nodes in netns")
> >Reported-by: Xiang Mei <xmei5@asu.edu>
> >Assisted-by: Claude:claude-opus-4-8
> >Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> >---
> > net/tipc/node.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> >diff --git a/net/tipc/node.c b/net/tipc/node.c index
> >8e4ef2630ae4..cd05269e5335 100644
> >--- a/net/tipc/node.c
> >+++ b/net/tipc/node.c
> >@@ -1695,6 +1695,9 @@ int tipc_node_xmit(struct net *net, struct
> >sk_buff_head *list,
> >       int bearer_id;
> >       int rc;
> >
> >+      if (skb_queue_empty(list))
> >+              return 0;
> >+
>
> Most callers of tipc_node_xmit() prepares准备 a non-empty list before calling except tipc_named_node_up().
> So, it is not optimal to add this check in tipc_node_xmit().
> I can see this issue needs to be addressed as part of your existing commit:
> https://patchwork.kernel.org/project/netdevbpf/patch/20260706163024.1205930-2-bestswngs@gmail.com/
>
> Please update above patch.
>
> >       if (in_own_node(net, dnode)) {
> >               tipc_loopback_trace(net, list);
> >               spin_lock_init(&list->lock);
> >--
> >2.43.0
> >
>

Thanks for the review. I've updated the patches accordingly. I
accidentally changed the subject line in v3. It should have stayed the
same as v2:

v3: https://lore.kernel.org/all/20260709144718.64535-2-bestswngs@gmail.com/

^ permalink raw reply

* Re: [PATCH net v2 1/2] net: fman: move IRQ registration after init to prevent NULL deref and UAF
From: Simon Horman @ 2026-07-09 14:52 UTC (permalink / raw)
  To: zhaojinming
  Cc: Simon Horman, madalin.bucur, sean.anderson, netdev, andrew+netdev,
	davem, edumazet, kuba, pabeni, igal.liberman, linux-kernel
In-Reply-To: <20260703074324.907294-1-zhaojinming@uniontech.com>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev

Simon says:

I think we can treat the pre-existing issue flagged below in the context of
possible follow-up. But I'm wondering if you could take a look over
the first issue.

---
net: fman: move IRQ registration after init to prevent NULL deref and UAF

This commit moves IRQ registration in the fman driver from read_dts_node() to
fman_probe() after fman_init() has completed. The intention is to avoid a NULL
dereference or use-after-free caused by shared interrupts firing before the
driver state is fully initialized.

Link: https://lore.kernel.org/netdev/20260626162323.GE1310988@horms.kernel.org/

> diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
> index 299bab043175b..6947f3bc7c87e 100644
> --- a/drivers/net/ethernet/freescale/fman/fman.c
> +++ b/drivers/net/ethernet/freescale/fman/fman.c

[ ... ]

> @@ -2848,6 +2829,35 @@ static int fman_probe(struct platform_device *of_dev)
>  		return -EINVAL;
>  	}
>  
> +	/* Register IRQ handlers only after initialization is complete.
> +	 * This prevents two issues:
> +	 * 1) Pre-init NULL dereference: is_init_done(NULL) returns true,
> +	 *    so a shared-IRQ spurious firing before fpm_regs is set would
> +	 *    dereference NULL.
> +	 * 2) Use-after-free on probe failure: fman was kzalloc'd (not devm),
> +	 *    so on error paths kfree(fman) ran before devm_free_irq, leaving
> +	 *    a window where the handler could fire with a freed dev_id.
> +	 * By registering here, both problems are eliminated.
> +	 */
> +	err = devm_request_irq(dev, fman->dts_params.irq, fman_irq,
> +			       IRQF_SHARED, "fman", fman);

Simon says:

It seems to me that one possible way to address the raced flagged
below would be to register the handler earlier, but have a flag
so that that the handler is effectively an noop until initialisation
is complete.

[Severity: High]
Can this race condition lead to an unhandled interrupt storm and disable the
shared IRQ line?

By moving the IRQ registration here, the handler is registered after
fman_init() has completed. The fman_init() function configures the hardware
and enables its interrupts (e.g. via fpm_init() and enable()). Because the
interrupt is shared (IRQF_SHARED), the IRQ line might already be unmasked by
another device.

If the FMan hardware asserts an interrupt in the window between fman_init()
completing and this handler being registered, the shared IRQ will fire. The
other device's handler will return IRQ_NONE, and since the FMan handler is
absent, no one will clear the FMan interrupt.

This interrupt storm would cause the core IRQ subsystem to permanently
disable the shared IRQ line.

[Severity: High]
This is a pre-existing issue, but does the driver leak dynamically allocated
resources when unbound?

The FMan driver allocates several non-devm-managed resources (e.g., fman
via kzalloc_obj, fman->state, fman->cfg, and MURAM allocations). However,
the platform_driver structure completely lacks a .remove callback.

If the device is unbound via sysfs or the module is unloaded, the core
driver subsystem tears down devm resources like IRQs, but the dynamically
allocated FMan structures and MURAM pools appear to be permanently leaked.

[Severity: High]
This is a pre-existing issue, but can child devices (MACs) probe before the
FMan parent initialization is complete, leading to permanent probe failures?

In fman_probe(), read_dts_node() is called first, which calls
of_platform_populate() to create child MAC devices before the FMan parent
has finished initializing:

fman_probe() {
    fman = read_dts_node(of_dev); /* Calls of_platform_populate() */
    ...
    err = fman_config(fman);
    ...
    if (fman_init(fman) != 0) {
    ...
    dev_set_drvdata(dev, fman);
}

If the MAC driver is already registered when of_platform_populate() is
called, the MAC devices will probe synchronously. Their probe routine
calls fman_bind(), which reads the parent's driver data via
dev_get_drvdata().

Since dev_set_drvdata() is not called until the very end of fman_probe(),
fman_bind() would return NULL, causing the MAC probe to fail with -ENODEV.

> +	if (err < 0) {
> +		dev_err(dev, "%s: irq %d allocation failed (error = %d)\n",
> +			__func__, fman->dts_params.irq, err);
> +		return err;
> +	}

^ permalink raw reply

* Re: [PATCH iproute2-next v5 0/2] devlink: support u64-array devlink parameters
From: patchwork-bot+netdevbpf @ 2026-07-09 14:50 UTC (permalink / raw)
  To: Ratheesh Kannoth
  Cc: stephen, dsahern, kuba, linux-kernel, netdev, andrew+netdev,
	edumazet, pabeni, jiri
In-Reply-To: <20260709035016.911607-1-rkannoth@marvell.com>

Hello:

This series was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:

On Thu, 9 Jul 2026 09:20:14 +0530 you wrote:
> The kernel gained support for devlink parameters of type
> DEVLINK_VAR_ATTR_TYPE_U64_ARRAY.  These parameters carry a variable-length
> list of u64 values encoded as multiple DEVLINK_ATTR_PARAM_VALUE_DATA
> attributes.  This is used by drivers that need to expose ordered lists of
> configuration values, such as the Marvell CN20K npc_srch_order parameter.
> 
> This series updates the devlink tool to handle the new UAPI and adds
> show/set support for u64-array parameters on both device and port params.
> 
> [...]

Here is the summary with links:
  - [v5,iproute2-next,1/2] devlink: use DEVLINK_VAR_ATTR_TYPE_* in param show/set
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=32e2a8832a53
  - [v5,iproute2-next,2/2] devlink: support u64-array values in devlink param show/set
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=37c491d82ceb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply

* [PATCH bpf-next v11 5/5] selftests/bpf: add bpf_icmp_send no route test
From: Mahe Tardy @ 2026-07-09 14:49 UTC (permalink / raw)
  To: bpf
  Cc: andrii, ast, daniel, john.fastabend, jordan, martin.lau,
	yonghong.song, emil, netdev, edumazet, kuba, pabeni, davem, horms,
	sdf.kernel, Mahe Tardy
In-Reply-To: <20260709144900.245904-1-mahe.tardy@gmail.com>

For normal live cgroup_skb paths, the skb should already be routed. The
exception is for test run via BPF_PROG_TEST_RUN with packets created
via bpf_prog_test_run_skb. Those lack dst route and thus the icmp_send
would quietly fail by returning early.

This test exercises this and makes sure the kfunc returns -ENETUNREACH.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Reviewed-by: Jordan Rife <jordan@jrife.io>
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
 .../bpf/prog_tests/icmp_send_kfunc.c          | 49 +++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
index bb532aa0d158..9318d4bc7ce8 100644
--- a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
+++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
@@ -169,6 +169,49 @@ static void run_icmp_test(struct icmp_send *skel, int af, const char *ip,
 	}
 }

+static void run_icmp_no_route_test(struct icmp_send *skel, int af)
+{
+	union {
+		struct ipv4_packet v4;
+		struct ipv6_packet v6;
+	} pkt;
+	DECLARE_LIBBPF_OPTS(bpf_test_run_opts, opts,
+		.data_in = &pkt,
+	);
+	int err;
+
+	switch (af) {
+	case AF_INET:
+		pkt.v4 = pkt_v4;
+		pkt.v4.iph.version = 4;
+		pkt.v4.iph.daddr = htonl(INADDR_LOOPBACK);
+		pkt.v4.tcp.dest = htons(80);
+		opts.data_size_in = sizeof(pkt.v4);
+		skel->bss->unreach_type = ICMP_DEST_UNREACH;
+		break;
+	case AF_INET6:
+		pkt.v6 = pkt_v6;
+		pkt.v6.iph.version = 6;
+		pkt.v6.iph.daddr = in6addr_loopback;
+		pkt.v6.tcp.dest = htons(80);
+		opts.data_size_in = sizeof(pkt.v6);
+		skel->bss->unreach_type = ICMPV6_DEST_UNREACH;
+		break;
+	default:
+		ASSERT_FAIL("af_not_supported");
+		return;
+	}
+
+	skel->bss->server_port = 80;
+	skel->data->kfunc_ret = KFUNC_RET_UNSET;
+
+	err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.egress), &opts);
+	if (!ASSERT_OK(err, "test_run"))
+		return;
+
+	ASSERT_EQ(skel->data->kfunc_ret, -ENETUNREACH, "kfunc_ret_no_route");
+}
+
 void test_icmp_send_unreach_cgroup(void)
 {
 	struct icmp_send *skel;
@@ -193,6 +236,12 @@ void test_icmp_send_unreach_cgroup(void)
 	if (test__start_subtest("ipv6"))
 		run_icmp_test(skel, AF_INET6, "::1", ICMPV6_REJECT_ROUTE);

+	if (test__start_subtest("no_route_ipv4"))
+		run_icmp_no_route_test(skel, AF_INET);
+
+	if (test__start_subtest("no_route_ipv6"))
+		run_icmp_no_route_test(skel, AF_INET6);
+
 cleanup:
 	icmp_send__destroy(skel);
 	if (cgroup_fd >= 0)
--
2.34.1


^ permalink raw reply related

* [PATCH bpf-next v11 4/5] selftests/bpf: add bpf_icmp_send recursion test
From: Mahe Tardy @ 2026-07-09 14:48 UTC (permalink / raw)
  To: bpf
  Cc: andrii, ast, daniel, john.fastabend, jordan, martin.lau,
	yonghong.song, emil, netdev, edumazet, kuba, pabeni, davem, horms,
	sdf.kernel, Mahe Tardy
In-Reply-To: <20260709144900.245904-1-mahe.tardy@gmail.com>

This test is similar to test_icmp_send_unreach_cgroup but checks that,
in case of recursion, meaning that the BPF program calling the kfunc was
re-triggered by the icmp_send done by the kfunc, the kfunc will stop
early and return -EBUSY.

The test attaches to the root cgroup to ensure the ICMP packet generated
by the kfunc re-triggers the BPF program.

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Reviewed-by: Jordan Rife <jordan@jrife.io>
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
 .../bpf/prog_tests/icmp_send_kfunc.c          | 46 ++++++++++++++++
 tools/testing/selftests/bpf/progs/icmp_send.c | 55 +++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
index bbb3c3d4509c..bb532aa0d158 100644
--- a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
+++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
@@ -1,8 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <test_progs.h>
 #include <network_helpers.h>
+#include <cgroup_helpers.h>
 #include <linux/errqueue.h>
 #include <poll.h>
+#include <unistd.h>
 #include "icmp_send.skel.h"

 #define TIMEOUT_MS 1000
@@ -10,6 +12,7 @@
 #define ICMP_DEST_UNREACH 3
 #define ICMPV6_DEST_UNREACH 1

+#define ICMP_HOST_UNREACH 1
 #define ICMP_FRAG_NEEDED 4
 #define NR_ICMP_UNREACH 15
 #define ICMPV6_REJECT_ROUTE 6
@@ -195,3 +198,46 @@ void test_icmp_send_unreach_cgroup(void)
 	if (cgroup_fd >= 0)
 		close(cgroup_fd);
 }
+
+void test_icmp_send_unreach_recursion(void)
+{
+	struct icmp_send *skel;
+	int cgroup_fd = -1;
+	int err;
+
+	err = setup_cgroup_environment();
+	if (!ASSERT_OK(err, "setup_cgroup_environment"))
+		return;
+
+	skel = icmp_send__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		goto cleanup;
+
+	cgroup_fd = get_root_cgroup();
+	if (!ASSERT_OK_FD(cgroup_fd, "get_root_cgroup"))
+		goto cleanup;
+
+	skel->data->target_pid = getpid();
+	skel->links.recursion =
+		bpf_program__attach_cgroup(skel->progs.recursion, cgroup_fd);
+	if (!ASSERT_OK_PTR(skel->links.recursion, "prog_attach_cgroup"))
+		goto cleanup;
+
+	trigger_prog_read_icmp_errqueue(skel, ICMP_HOST_UNREACH, AF_INET,
+					"127.0.0.1");
+
+	/*
+	 * Because there's recursion involved, the first call will return at
+	 * index 1 since it will return the second, and the second call will
+	 * return at index 0 since it will return the first.
+	 */
+	ASSERT_EQ(skel->bss->rec_count, 2, "rec_count");
+	ASSERT_EQ(skel->data->rec_kfunc_rets[0], -EBUSY, "kfunc_rets[0]");
+	ASSERT_EQ(skel->data->rec_kfunc_rets[1], 0, "kfunc_rets[1]");
+
+cleanup:
+	icmp_send__destroy(skel);
+	if (cgroup_fd >= 0)
+		close(cgroup_fd);
+	cleanup_cgroup_environment();
+}
diff --git a/tools/testing/selftests/bpf/progs/icmp_send.c b/tools/testing/selftests/bpf/progs/icmp_send.c
index 6e1ba539eeb0..c642ccdf9fd5 100644
--- a/tools/testing/selftests/bpf/progs/icmp_send.c
+++ b/tools/testing/selftests/bpf/progs/icmp_send.c
@@ -12,6 +12,10 @@ __u16 server_port = 0;
 int unreach_type = 0;
 int unreach_code = 0;
 int kfunc_ret = -1;
+int target_pid = -1;
+
+unsigned int rec_count = 0;
+int rec_kfunc_rets[] = { -1, -1 };

 SEC("cgroup_skb/egress")
 int egress(struct __sk_buff *skb)
@@ -65,4 +69,55 @@ int egress(struct __sk_buff *skb)
 	return SK_DROP;
 }

+SEC("cgroup_skb/egress")
+int recursion(struct __sk_buff *skb)
+{
+	void *data = (void *)(long)skb->data;
+	void *data_end = (void *)(long)skb->data_end;
+	struct icmphdr *icmph;
+	struct tcphdr *tcph;
+	struct iphdr *iph;
+	int ret;
+
+	if ((bpf_get_current_pid_tgid() >> 32) != target_pid)
+		return SK_PASS;
+
+	iph = data;
+	if ((void *)(iph + 1) > data_end || iph->version != 4)
+		return SK_PASS;
+
+	if (iph->daddr != bpf_htonl(SERVER_IP))
+		return SK_PASS;
+
+	if (iph->protocol == IPPROTO_TCP) {
+		tcph = (void *)iph + iph->ihl * 4;
+		if ((void *)(tcph + 1) > data_end ||
+		    tcph->dest != bpf_htons(server_port))
+			return SK_PASS;
+	} else if (iph->protocol == IPPROTO_ICMP) {
+		icmph = (void *)iph + iph->ihl * 4;
+		if ((void *)(icmph + 1) > data_end ||
+		    icmph->type != unreach_type || icmph->code != unreach_code)
+			return SK_PASS;
+	} else {
+		return SK_PASS;
+	}
+
+	/*
+	 * This call will provoke a recursion: the ICMP packet generated by the
+	 * kfunc will re-trigger this program since we are in the root cgroup in
+	 * which the kernel ICMP socket belongs. However when re-entering the
+	 * kfunc, it should return EBUSY.
+	 */
+	ret = bpf_icmp_send(skb, unreach_type, unreach_code);
+	rec_kfunc_rets[rec_count & 1] = ret;
+	__sync_fetch_and_add(&rec_count, 1);
+
+	/* Let the first ICMP error message pass */
+	if (iph->protocol == IPPROTO_ICMP)
+		return SK_PASS;
+
+	return SK_DROP;
+}
+
 char LICENSE[] SEC("license") = "Dual BSD/GPL";
--
2.34.1


^ permalink raw reply related

* [PATCH bpf-next v11 3/5] selftests/bpf: add bpf_icmp_send kfunc cgroup_skb IPv6 tests
From: Mahe Tardy @ 2026-07-09 14:48 UTC (permalink / raw)
  To: bpf
  Cc: andrii, ast, daniel, john.fastabend, jordan, martin.lau,
	yonghong.song, emil, netdev, edumazet, kuba, pabeni, davem, horms,
	sdf.kernel, Mahe Tardy
In-Reply-To: <20260709144900.245904-1-mahe.tardy@gmail.com>

This test extends the existing cgroup_skb tests with IPv6 support.

Note that we need to set IPV6_RECVERR on the socket for IPv6 in
connect_to_fd_nonblock otherwise the error will be ignored even if we
are in the middle of the TCP handshake. See in
net/ipv6/datagram.c:ipv6_icmp_error for more details.

Reviewed-by: Jordan Rife <jordan@jrife.io>
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
 .../bpf/prog_tests/icmp_send_kfunc.c          | 91 +++++++++++++------
 tools/testing/selftests/bpf/progs/icmp_send.c | 48 ++++++++--
 2 files changed, 101 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
index b8a98c90053e..bbb3c3d4509c 100644
--- a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
+++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
@@ -8,9 +8,11 @@
 #define TIMEOUT_MS 1000

 #define ICMP_DEST_UNREACH 3
+#define ICMPV6_DEST_UNREACH 1

 #define ICMP_FRAG_NEEDED 4
 #define NR_ICMP_UNREACH 15
+#define ICMPV6_REJECT_ROUTE 6

 #define KFUNC_RET_UNSET -1

@@ -18,7 +20,7 @@ static int connect_to_fd_nonblock(int server_fd)
 {
 	struct sockaddr_storage addr;
 	socklen_t len = sizeof(addr);
-	int fd, err;
+	int fd, err, on = 1;

 	if (getsockname(server_fd, (struct sockaddr *)&addr, &len))
 		return -1;
@@ -27,6 +29,12 @@ static int connect_to_fd_nonblock(int server_fd)
 	if (fd < 0)
 		return -1;

+	if (addr.ss_family == AF_INET6 &&
+	    setsockopt(fd, IPPROTO_IPV6, IPV6_RECVERR, &on, sizeof(on)) < 0) {
+		close(fd);
+		return -1;
+	}
+
 	err = connect(fd, (struct sockaddr *)&addr, len);
 	if (err < 0 && errno != EINPROGRESS) {
 		close(fd);
@@ -36,8 +44,14 @@ static int connect_to_fd_nonblock(int server_fd)
 	return fd;
 }

-static void read_icmp_errqueue(int sockfd, int expected_code)
+static void read_icmp_errqueue(int sockfd, int expected_code, int af)
 {
+	int expected_ee_type = (af == AF_INET) ? ICMP_DEST_UNREACH :
+						 ICMPV6_DEST_UNREACH;
+	int expected_origin = (af == AF_INET) ? SO_EE_ORIGIN_ICMP :
+						SO_EE_ORIGIN_ICMP6;
+	int expected_level = (af == AF_INET) ? IPPROTO_IP : IPPROTO_IPV6;
+	int expected_type = (af == AF_INET) ? IP_RECVERR : IPV6_RECVERR;
 	struct sock_extended_err *sock_err;
 	char ctrl_buf[512];
 	struct msghdr msg = {
@@ -63,38 +77,43 @@ static void read_icmp_errqueue(int sockfd, int expected_code)
 		return;

 	for (; cm; cm = CMSG_NXTHDR(&msg, cm)) {
-		if (cm->cmsg_level != IPPROTO_IP || cm->cmsg_type != IP_RECVERR)
+		if (cm->cmsg_level != expected_level ||
+		    cm->cmsg_type != expected_type)
 			continue;

 		sock_err = (struct sock_extended_err *)CMSG_DATA(cm);

-		if (!ASSERT_EQ(sock_err->ee_origin, SO_EE_ORIGIN_ICMP,
-			       "sock_err_origin_icmp"))
+		if (!ASSERT_EQ(sock_err->ee_origin, expected_origin,
+			       "sock_err_origin"))
 			return;
-		if (!ASSERT_EQ(sock_err->ee_type, ICMP_DEST_UNREACH,
+		if (!ASSERT_EQ(sock_err->ee_type, expected_ee_type,
 			       "sock_err_type_dest_unreach"))
 			return;
 		ASSERT_EQ(sock_err->ee_code, expected_code, "sock_err_code");
 		return;
 	}

-	ASSERT_FAIL("no IP_RECVERR control message found");
+	ASSERT_FAIL("no IP_RECVERR/IPV6_RECVERR control message found");
 }

-static bool valid_unreach_code(int code)
+static bool valid_unreach_code(int code, int af)
 {
 	if (code < 0)
 		return false;

-	return code <= NR_ICMP_UNREACH && code != ICMP_FRAG_NEEDED;
+	if (af == AF_INET)
+		return code <= NR_ICMP_UNREACH && code != ICMP_FRAG_NEEDED;
+
+	return code <= ICMPV6_REJECT_ROUTE;
 }

-static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code)
+static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code,
+					    int af, const char *ip)
 {
 	int srv_fd = -1, client_fd = -1;
 	int port;

-	srv_fd = start_server(AF_INET, SOCK_STREAM, "127.0.0.1", 0, TIMEOUT_MS);
+	srv_fd = start_server(af, SOCK_STREAM, ip, 0, TIMEOUT_MS);
 	if (!ASSERT_OK_FD(srv_fd, "start_server"))
 		return;

@@ -105,6 +124,8 @@ static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code)
 	}

 	skel->bss->server_port = ntohs(port);
+	skel->bss->unreach_type = (af == AF_INET) ? ICMP_DEST_UNREACH :
+						    ICMPV6_DEST_UNREACH;
 	skel->bss->unreach_code = code;
 	skel->data->kfunc_ret = KFUNC_RET_UNSET;

@@ -114,13 +135,37 @@ static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code)
 		return;
 	}

-	if (valid_unreach_code(code))
-		read_icmp_errqueue(client_fd, code);
+	if (valid_unreach_code(code, af))
+		read_icmp_errqueue(client_fd, code, af);

 	close(client_fd);
 	close(srv_fd);
 }

+static void run_icmp_test(struct icmp_send *skel, int af, const char *ip,
+			  int max_code)
+{
+	for (int code = 0; code <= max_code; code++) {
+		if (af == AF_INET && code == ICMP_FRAG_NEEDED)
+			continue;
+
+		trigger_prog_read_icmp_errqueue(skel, code, af, ip);
+		ASSERT_EQ(skel->data->kfunc_ret, 0, "kfunc_ret");
+	}
+
+	/* Test invalid codes */
+	trigger_prog_read_icmp_errqueue(skel, -1, af, ip);
+	ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+
+	trigger_prog_read_icmp_errqueue(skel, max_code + 1, af, ip);
+	ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+
+	if (af == AF_INET) {
+		trigger_prog_read_icmp_errqueue(skel, ICMP_FRAG_NEEDED, af, ip);
+		ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+	}
+}
+
 void test_icmp_send_unreach_cgroup(void)
 {
 	struct icmp_send *skel;
@@ -139,23 +184,11 @@ void test_icmp_send_unreach_cgroup(void)
 	if (!ASSERT_OK_PTR(skel->links.egress, "prog_attach_cgroup"))
 		goto cleanup;

-	for (int code = 0; code <= NR_ICMP_UNREACH; code++) {
-		if (code == ICMP_FRAG_NEEDED)
-			continue;
-
-		trigger_prog_read_icmp_errqueue(skel, code);
-		ASSERT_EQ(skel->data->kfunc_ret, 0, "kfunc_ret");
-	}
-
-	/* Test invalid codes */
-	trigger_prog_read_icmp_errqueue(skel, -1);
-	ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+	if (test__start_subtest("ipv4"))
+		run_icmp_test(skel, AF_INET, "127.0.0.1", NR_ICMP_UNREACH);

-	trigger_prog_read_icmp_errqueue(skel, NR_ICMP_UNREACH + 1);
-	ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
-
-	trigger_prog_read_icmp_errqueue(skel, ICMP_FRAG_NEEDED);
-	ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+	if (test__start_subtest("ipv6"))
+		run_icmp_test(skel, AF_INET6, "::1", ICMPV6_REJECT_ROUTE);

 cleanup:
 	icmp_send__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/icmp_send.c b/tools/testing/selftests/bpf/progs/icmp_send.c
index 6d0be0a9afe1..6e1ba539eeb0 100644
--- a/tools/testing/selftests/bpf/progs/icmp_send.c
+++ b/tools/testing/selftests/bpf/progs/icmp_send.c
@@ -5,10 +5,11 @@

 /* 127.0.0.1 in host byte order */
 #define SERVER_IP 0x7F000001
-
-#define ICMP_DEST_UNREACH 3
+/* ::1 in host byte order (last 32-bit word) */
+#define SERVER_IP6_LO 0x00000001

 __u16 server_port = 0;
+int unreach_type = 0;
 int unreach_code = 0;
 int kfunc_ret = -1;

@@ -18,19 +19,48 @@ int egress(struct __sk_buff *skb)
 	void *data = (void *)(long)skb->data;
 	void *data_end = (void *)(long)skb->data_end;
 	struct iphdr *iph;
+	struct ipv6hdr *ip6h;
 	struct tcphdr *tcph;
+	__u8 version;

-	iph = data;
-	if ((void *)(iph + 1) > data_end || iph->version != 4 ||
-	    iph->protocol != IPPROTO_TCP || iph->daddr != bpf_htonl(SERVER_IP))
+	if (data + 1 > data_end)
 		return SK_PASS;

-	tcph = (void *)iph + iph->ihl * 4;
-	if ((void *)(tcph + 1) > data_end ||
-	    tcph->dest != bpf_htons(server_port))
+	version = (*((__u8 *)data)) >> 4;
+
+	if (version == 4) {
+		iph = data;
+		if ((void *)(iph + 1) > data_end ||
+		    iph->protocol != IPPROTO_TCP ||
+		    iph->daddr != bpf_htonl(SERVER_IP))
+			return SK_PASS;
+
+		tcph = (void *)iph + iph->ihl * 4;
+		if ((void *)(tcph + 1) > data_end ||
+		    tcph->dest != bpf_htons(server_port))
+			return SK_PASS;
+
+	} else if (version == 6) {
+		ip6h = data;
+		if ((void *)(ip6h + 1) > data_end ||
+		    ip6h->nexthdr != IPPROTO_TCP)
+			return SK_PASS;
+
+		if (ip6h->daddr.in6_u.u6_addr32[0] != 0 ||
+		    ip6h->daddr.in6_u.u6_addr32[1] != 0 ||
+		    ip6h->daddr.in6_u.u6_addr32[2] != 0 ||
+		    ip6h->daddr.in6_u.u6_addr32[3] != bpf_htonl(SERVER_IP6_LO))
+			return SK_PASS;
+
+		tcph = (void *)(ip6h + 1);
+		if ((void *)(tcph + 1) > data_end ||
+		    tcph->dest != bpf_htons(server_port))
+			return SK_PASS;
+	} else {
 		return SK_PASS;
+	}

-	kfunc_ret = bpf_icmp_send(skb, ICMP_DEST_UNREACH, unreach_code);
+	kfunc_ret = bpf_icmp_send(skb, unreach_type, unreach_code);

 	return SK_DROP;
 }
--
2.34.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox