Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] net: emac: mal: replace devm_request_irq with request_irq to fix probe error race
From: Andrew Lunn @ 2026-07-03 16:38 UTC (permalink / raw)
  To: Rosen Penev
  Cc: netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, open list
In-Reply-To: <20260702235045.1326384-1-rosenp@gmail.com>

On Thu, Jul 02, 2026 at 04:50:45PM -0700, Rosen Penev wrote:
> devm_request_irq() is a managed resource: the IRQ is not freed until
> devres_release_all() runs after the probe function returns.  In the
> probe error path, free_netdev(mal->dummy_dev) and dcr_unmap() execute
> while the IRQ is still live.  If the shared IRQ fires during cleanup,
> the handler accesses unmapped DCR registers (crash) or the already-
> freed dummy_dev (use-after-free).
> 
> Switch to plain request_irq() with per-IRQ error labels that tear down
> only the IRQs that were successfully registered, and add the matching
> free_irq() calls in mal_remove().
> 
> Fixes: 14f59154ff0b ("net: ibm: emac: mal: use devm for request_irq")
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

You seemed to of sent the same patch within 24 hours. Please don't do
that.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

Send a self NACK to the broken version, wait 24 hours, and send v2.

    Andrew

---
pw-bot: cr

^ permalink raw reply

* Re: [PATCH net] selftests/tc-testing: Add tests that force multiq and taprio to enqueue to child's gso_skb
From: Paolo Abeni @ 2026-07-03 16:37 UTC (permalink / raw)
  To: Victor Nogueira
  Cc: netdev, hexlabsecurity, pctammela, davem, edumazet, kuba, jhs,
	jiri
In-Reply-To: <20260630153651.249752-1-victor@mojatatu.com>

On 6/30/26 5:36 PM, Victor Nogueira wrote:
> Add test cases to reproduce scenarios fixed recently [1] where
> multiqueue and taprio forced their children into enqueueing an skb to
> gso_skb (during peek), but failed to dequeue from gso_skb because they
> called the child's dequeue callback directly. This causes a desync in the
> child's qlen/backlog and results in an eventual null-ptr-deref (with a
> qfq or dualpi2 child).
> 
> Test cases are the following:
> 
> - Force multiq to dequeue from its child's gso_skb with qfq leaf (fb6c)
> - Force multiq to dequeue from its child's gso_skb with dualpi2 leaf (1922)
> - Force taprio to dequeue from its child's gso_skb with qfq leaf (476f)
> - Force taprio to dequeue from its child's gso_skb with dualpi2 leaf (0235)
> 
> [1] https://lore.kernel.org/netdev/20260625-b4-disp-31bcb279-v1-0-85c40b83c529@proton.me/
> 
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>

Since the fix has been propagated to net-next, this is net-next
material, right? no need to repost, just ack/nack, thanks!

/P


^ permalink raw reply

* Re: [PATCH net] net: emac: mal: fix W1C write race in ICINTSTAT clearing
From: Andrew Lunn @ 2026-07-03 16:36 UTC (permalink / raw)
  To: David Gibson
  Cc: Rosen Penev, netdev, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Jeff Garzik, open list
In-Reply-To: <akcnQEiJYfAtPqN1@zatzit>

On Fri, Jul 03, 2026 at 01:06:40PM +1000, David Gibson wrote:
> On Thu, Jul 02, 2026 at 04:49:23PM -0700, Rosen Penev wrote:
> > The ICINTSTAT register is write-1-to-clear (W1C).  The read-modify-write
> > pattern in both mal_txeob() and mal_rxeob() can lose interrupts: if a bit
> > that should not be cleared is already asserted when mfdcri() reads the
> > register, it is included in the read value, retained by the bitwise OR, and
> > then written back as 1 - inadvertently clearing a pending but unhandled
> > interrupt.
> > 
> > Fix by writing only the specific bit to clear (ICINTSTAT_ICTX for TXEOB,
> > ICINTSTAT_ICRX for RXEOB).  W1C semantics guarantee that writing 0 to the
> > other bits has no effect.
> 
> Wow, it's a long time since I thought about the MAL.
> 
> > Fixes: 1d3bb996481e ("Device tree aware EMAC driver")
> 
> This doesn't appear correct.  The lines in question were added by
> fbcc4bacee30c ("ibm_newemac: MAL support for PowerPC 405EZ")
> 
> > Assisted-by: opencode:big-pickle
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> 
> Assuming ICINTSTAT is indeed a W1C register (or "read/clear" as I
> believe they were termed in the 405 documentation) the change looks
> correct.  However, I no longer have access to the documentation that
> would let me verify that.  I would absolutely not trust an LLM to know
> if that's the case, since it's a fairly arbitrary and specific detail
> of an obscure CPU.

I agree. If this is pure LLM, we need some form of verification.

  Andrew

^ permalink raw reply

* Re: [PATCH ipsec] xfrm: espintcp: fix UAF during close
From: Breno Leitao @ 2026-07-03 16:36 UTC (permalink / raw)
  To: Sabrina Dubroca
  Cc: netdev, Steffen Klassert, Herbert Xu, stable, zdi-disclosures
In-Reply-To: <50e2ab4348eb8177581058f0152394cfae6a8d27.1783071494.git.sd@queasysnail.net>

Hello Sabrina,

On Fri, Jul 03, 2026 at 04:21:12PM +0200, Sabrina Dubroca wrote:
> diff --git a/net/xfrm/espintcp.c b/net/xfrm/espintcp.c
> index 374e1b964438..f09b5dd85db8 100644
> --- a/net/xfrm/espintcp.c
> +++ b/net/xfrm/espintcp.c
> @@ -517,6 +517,8 @@ static void espintcp_close(struct sock *sk, long timeout)
>  	sk->sk_prot = &tcp_prot;
>  	barrier();
>  
> +	synchronize_rcu();

I've got the impression netdev usually prefers synchronize_net() instead
of synchornize_rcu(). Is there any reason for synchronize_net() not
being used here?

Also, given you have a explicit synchronize_rcu() here, should the
barrier() above be dropped?

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH iwl-net v3 2/2] ice: preserve uplink DFLT Rx rule on switchdev release
From: Marcin Szycik @ 2026-07-03 16:36 UTC (permalink / raw)
  To: Petr Oros, netdev
  Cc: Ivan Vecera, Alice Michael, Przemek Kitszel, Eric Dumazet,
	linux-kernel, Martyna Szapar-Mudlaw, Andrew Lunn, Tony Nguyen,
	Simon Horman, intel-wired-lan, Jacob Keller, Jakub Kicinski,
	Paolo Abeni, David S. Miller
In-Reply-To: <20260701133601.2118382-3-poros@redhat.com>



On 01.07.2026 15:36, Petr Oros wrote:
> When the uplink PF is promiscuous, ice_vsi_sync_fltr() installs an
> ICE_SW_LKUP_DFLT catch-all Rx rule on the uplink VSI. Entering switchdev
> re-affirms it through the idempotent ice_set_dflt_vsi(), but
> ice_eswitch_release_env() removed both the Rx and Tx DFLT rules
> unconditionally on teardown. That clobbered a promisc-owned Rx rule: it
> disappeared while IFF_PROMISC was still set and the sync path was not
> retriggered, leaving the uplink without the catch-all the netdev
> requested.
> 
> Skip the Rx DFLT removal when the uplink is promiscuous, both in
> ice_eswitch_release_env() and the err_def_tx unwind of
> ice_eswitch_setup_env(); the Tx leg, owned by switchdev, is still removed.
> Test the live netdev->flags, the same value ena_rx_filtering() ->
> ice_cfg_vlan_pruning() above already keys on, so the preserved rule and
> the pruning state stay consistent, including for a promisc change made
> while switchdev ran (which never reached the gated filter sync).
> 
> Fixes: 5c07be96d8b3 ("ice: Avoid setting default Rx VSI twice in switchdev setup")
> Signed-off-by: Petr Oros <poros@redhat.com>

Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com>

> ---
> v3:
> - Corrected the Fixes tag from 1a1c40df2e80 ("ice: set and release
>   switchdev environment") to 5c07be96d8b3 ("ice: Avoid setting default
>   Rx VSI twice in switchdev setup"), the commit that made
>   ice_eswitch_setup_env() use the idempotent ice_set_dflt_vsi(); before
>   it a pre-existing promisc DFLT rule made setup fail with -EEXIST so the
>   release path was never reached. No code change.
> 
> v2: https://lore.kernel.org/all/20260622113428.2565255-3-poros@redhat.com/
> v1: https://lore.kernel.org/all/deef5756e534ef06c12d910c5305d3fd205d30a0.1781786935.git.poros@redhat.com/
> ---
>  drivers/net/ethernet/intel/ice/ice_eswitch.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_eswitch.c b/drivers/net/ethernet/intel/ice/ice_eswitch.c
> index c30e27bbfe6e25..07e2016fb9481f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_eswitch.c
> +++ b/drivers/net/ethernet/intel/ice/ice_eswitch.c
> @@ -66,8 +66,10 @@ static int ice_eswitch_setup_env(struct ice_pf *pf)
>  	ice_cfg_dflt_vsi(uplink_vsi->port_info, uplink_vsi->idx, false,
>  			 ICE_FLTR_TX);
>  err_def_tx:
> -	ice_cfg_dflt_vsi(uplink_vsi->port_info, uplink_vsi->idx, false,
> -			 ICE_FLTR_RX);
> +	/* keep the Rx DFLT rule if the uplink is promiscuous (see release_env) */
> +	if (!(uplink_vsi->netdev->flags & IFF_PROMISC))
> +		ice_cfg_dflt_vsi(uplink_vsi->port_info, uplink_vsi->idx,
> +				 false, ICE_FLTR_RX);
>  err_def_rx:
>  	ice_vsi_del_vlan_zero(uplink_vsi);
>  err_vlan_zero:
> @@ -276,8 +278,16 @@ static void ice_eswitch_release_env(struct ice_pf *pf)
>  	vlan_ops->ena_rx_filtering(uplink_vsi);
>  	ice_cfg_dflt_vsi(uplink_vsi->port_info, uplink_vsi->idx, false,
>  			 ICE_FLTR_TX);
> -	ice_cfg_dflt_vsi(uplink_vsi->port_info, uplink_vsi->idx, false,
> -			 ICE_FLTR_RX);
> +
> +	/* Keep the Rx DFLT rule if the uplink is promiscuous; it must outlive
> +	 * the session. Test the live netdev->flags, the same value
> +	 * ena_rx_filtering() -> ice_cfg_vlan_pruning() above keys its decision
> +	 * on, so the preserved DFLT rule and the pruning state stay consistent.
> +	 */
> +	if (!(uplink_vsi->netdev->flags & IFF_PROMISC))
> +		ice_cfg_dflt_vsi(uplink_vsi->port_info, uplink_vsi->idx,
> +				 false, ICE_FLTR_RX);
> +
>  	ice_fltr_add_mac_and_broadcast(uplink_vsi,
>  				       uplink_vsi->port_info->mac.perm_addr,
>  				       ICE_FWD_TO_VSI);


^ permalink raw reply

* Re: [PATCH net-next v3 01/15] net: macb: drop "consistent" from alloc/free function names
From: Conor Dooley @ 2026-07-03 16:34 UTC (permalink / raw)
  To: Théo Lebrun
  Cc: Conor Dooley, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Richard Cochran, Russell King,
	netdev, linux-kernel, Nicolas Ferre, Claudiu Beznea,
	Paolo Valerio, Nicolai Buchwitz, Vladimir Kondratiev,
	Gregory CLEMENT, Benoît Monin, Tawfik Bayouk,
	Thomas Petazzoni, Maxime Chevallier
In-Reply-To: <DJP2W90JJK0G.1DBR4RMJKJO2Z@bootlin.com>

[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]

On Fri, Jul 03, 2026 at 06:32:08PM +0200, Théo Lebrun wrote:
> Hello Conor,
> 
> On Fri Jul 3, 2026 at 1:28 PM CEST, Conor Dooley wrote:
> > On Wed, Jul 01, 2026 at 05:59:04PM +0200, Théo Lebrun wrote:
> >> Since commit 4df95131ea80 ("net/macb: change RX path for GEM") those
> >> functions have not been only allocating or freeing consistent memory
> >> mappings.
> >> 
> >> Rename from macb_alloc_consistent() to macb_alloc() and
> >>        from macb_free_consistent()  to macb_free().
> >
> > What does "consistent" even mean? Is it intended to be analogous to
> > coherent?
> 
> That's my guess. We used to have pci_alloc_consistent(), being an alias
> to dma_alloc_coherent().
> 
> This alias has been present since forever (since Git):
> 
>    git show 1da177e4c3f4:include/asm-generic/pci-dma-compat.h
> 
> The alias got removed in commit 7968778914e5 ("PCI: Remove the
> deprecated "pci-dma-compat.h" API", 2022-03-09).

Right, I did figure given the commit you cited as being the end of it
that it was something like that.
Acked-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH iwl-net v3 1/2] ice: skip per-VLAN promisc rules when default VSI Rx rule is set
From: Marcin Szycik @ 2026-07-03 16:34 UTC (permalink / raw)
  To: Petr Oros, netdev
  Cc: Ivan Vecera, Alice Michael, Przemek Kitszel, Eric Dumazet,
	linux-kernel, Martyna Szapar-Mudlaw, Andrew Lunn, Tony Nguyen,
	Simon Horman, intel-wired-lan, Jacob Keller, Jakub Kicinski,
	Paolo Abeni, David S. Miller
In-Reply-To: <20260701133601.2118382-2-poros@redhat.com>



On 01.07.2026 15:36, Petr Oros wrote:
> When an ice port in a vlan-filtering bridge goes promiscuous (typical for
> bond slaves), the driver installs a per-VLAN ICE_SW_LKUP_PROMISC_VLAN rule
> for every VID on top of the broad ICE_SW_LKUP_DFLT VSI Rx rule. Each rule
> consumes one of the ~32K Flow Lookup Unit (FLU) entries the device shares
> across PFs, so a wide trunk (vid 2-4094) over several PFs overruns the
> pool: firmware rejects further Add Switch Rules with ENOSPC (AQ 0x10) and
> the DFLT Rx rule itself fails to install:
> 
>   ice 0000:5c:00.1: Failed to set VSI 14 as the default forwarding
>                     VSI, error -5
>   ice 0000:5c:00.1 ens1f1: Error -5 setting default VSI 14 Rx rule
> 
> Once a switch context is overrun the retries can also come back as ENOENT
> (AQ 0x2), which has misled triage toward a perceived recipe binding defect
> rather than a capacity issue.
> 
> The DFLT rule already catches every packet on the port regardless of VLAN
> tag, so the per-VLAN promisc expansion is redundant while it is installed.
> Skip it at the two sites that drive it, ice_set_promisc() and
> ice_vlan_rx_add_vid(), keyed on ice_is_vsi_dflt_vsi() rather than the
> netdev IFF_PROMISC flag so a failed or LAG-suppressed DFLT install still
> falls back to the per-VLAN rules.
> 
> IFF_ALLMULTI and IFF_PROMISC can reach ice_vsi_sync_fltr() in separate
> passes (a bridge join sets them through separate calls), so the allmulti
> pass may expand the per-VID rules before the DFLT rule exists. Drop those
> now-redundant rules right after ice_set_dflt_vsi() installs the DFLT rule;
> ice_vsi_exit_dflt_promisc() reinstates them when promisc is cleared.
> 
> ice_vsi_sync_fltr() subscribed multicast promiscuity only inside the
> "default VSI not yet in use" branch, so a promiscuous VSI that finds the
> default VSI rule already present (owned by another VSI, or preserved
> across a switchdev session) ended up in unicast promisc with no multicast
> subscription. Issue ice_set_promisc(ICE_MCAST_PROMISC_BITS) whenever the
> netdev is promiscuous; it is idempotent and returns 0 if the rule is
> already present.
> 
> Fixes: 1273f89578f2 ("ice: Fix broken IFF_ALLMULTI handling")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
> v3:
> - Dropped the two vid=0 ICE_SW_LKUP_PROMISC <-> ICE_SW_LKUP_PROMISC_VLAN
>   recipe-swap guards in ice_vlan_rx_add_vid() and ice_vlan_rx_kill_vid();
>   each swap is net-zero and guarding the demote stranded the vid=0 rule
>   in ICE_SW_LKUP_PROMISC_VLAN when the last VLAN was removed under the
>   DFLT rule. Reported by review.
> - Drop the now-redundant per-VID multicast promisc rules right after
>   ice_set_dflt_vsi(). A bridge join raises IFF_ALLMULTI and IFF_PROMISC
>   in separate sync passes, so the allmulti pass expands the per-VID rules
>   before the DFLT rule exists; the cleanup keeps them from lingering and
>   exhausting the FLU pool. ice_vsi_exit_dflt_promisc() reinstates them on
>   promisc off. Reported by review.
> - Issue ice_set_promisc(ICE_MCAST_PROMISC_BITS) whenever the netdev is
>   promiscuous, not only when this VSI installs the default VSI rule, so
>   multicast promisc is not lost when the rule is already in use (owned by
>   another VSI, or preserved across a switchdev session). Reported by
>   review.
> - Hoisted the combined VLAN promisc mask in ice_clear_promisc() into a
>   local for alignment. Dropped Aleksandr's Reviewed-by since the code
>   changed.
> 
> v2: https://lore.kernel.org/all/20260622113428.2565255-2-poros@redhat.com/
> v1: https://lore.kernel.org/all/89efbea9831175e6f57e9fe8557f7a0e48e050b7.1781786935.git.poros@redhat.com/
> ---
>  drivers/net/ethernet/intel/ice/ice_main.c | 111 ++++++++++++++++++----
>  1 file changed, 90 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index b43d420ece99ca..a84de6cf6eb078 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -274,7 +274,8 @@ static int ice_set_promisc(struct ice_vsi *vsi, u8 promisc_m)
>  	if (vsi->type != ICE_VSI_PF)
>  		return 0;
>  
> -	if (ice_vsi_has_non_zero_vlans(vsi)) {
> +	/* skip per-VID expansion; the DFLT Rx rule already covers every VID */
> +	if (ice_vsi_has_non_zero_vlans(vsi) && !ice_is_vsi_dflt_vsi(vsi)) {
>  		promisc_m |= (ICE_PROMISC_VLAN_RX | ICE_PROMISC_VLAN_TX);
>  		status = ice_fltr_set_vlan_vsi_promisc(&vsi->back->hw, vsi,
>  						       promisc_m);
> @@ -304,9 +305,20 @@ static int ice_clear_promisc(struct ice_vsi *vsi, u8 promisc_m)
>  		return 0;
>  
>  	if (ice_vsi_has_non_zero_vlans(vsi)) {
> -		promisc_m |= (ICE_PROMISC_VLAN_RX | ICE_PROMISC_VLAN_TX);
> +		u8 vlan_promisc_m = promisc_m | ICE_PROMISC_VLAN_RX |
> +				    ICE_PROMISC_VLAN_TX;
> +		int vid0_status;
> +
> +		/* set time used either recipe (per-VID PROMISC_VLAN, or vid=0

I find this sentence hard to understand - did you mean "ice_set_promisc() used
either recipe..."?

> +		 * PROMISC via the ice_set_promisc() else branch), so clear
> +		 * both; clearing an absent rule succeeds

What do you mean by this? Both will return -EEXIST if rule is absent. There can
also be other errors.

> +		 */
>  		status = ice_fltr_clear_vlan_vsi_promisc(&vsi->back->hw, vsi,
> -							 promisc_m);
> +							 vlan_promisc_m);
> +		vid0_status = ice_fltr_clear_vsi_promisc(&vsi->back->hw,
> +							 vsi->idx, promisc_m, 0);
> +		if (status == 0)
> +			status = vid0_status;
>  	} else {
>  		status = ice_fltr_clear_vsi_promisc(&vsi->back->hw, vsi->idx,
>  						    promisc_m, 0);
> @@ -317,6 +329,59 @@ static int ice_clear_promisc(struct ice_vsi *vsi, u8 promisc_m)
>  	return status;
>  }
>  
> +/**
> + * ice_vsi_exit_dflt_promisc - drop the default VSI Rx rule on promisc off
> + * @vsi: the VSI leaving promiscuous mode
> + *
> + * For an IFF_ALLMULTI VSI with VLANs the per-VID multicast rules are
> + * reinstated before the default rule is cleared so coverage never lapses;
> + * the then redundant vid=0 rule is dropped best-effort. The callees log
> + * their own failures, so error returns are not re-logged here.
> + *
> + * Return: 0 on success, negative on error with the default rule left in place.
> + */
> +static int ice_vsi_exit_dflt_promisc(struct ice_vsi *vsi)
> +{
> +	struct ice_vsi_vlan_ops *vlan_ops = ice_get_compat_vsi_vlan_ops(vsi);
> +	struct net_device *netdev = vsi->netdev;
> +	struct ice_hw *hw = &vsi->back->hw;
> +	bool restore_mc;
> +	int err;
> +
> +	restore_mc = (vsi->current_netdev_flags & IFF_ALLMULTI) &&
> +		     ice_vsi_has_non_zero_vlans(vsi);
> +
> +	if (restore_mc) {
> +		err = ice_fltr_set_vlan_vsi_promisc(hw, vsi,
> +						    ICE_MCAST_VLAN_PROMISC_BITS);
> +		if (err && err != -EEXIST)
> +			return err;
> +	}
> +
> +	err = ice_clear_dflt_vsi(vsi);
> +	if (err)
> +		return err;
> +
> +	if (netdev->features & NETIF_F_HW_VLAN_CTAG_FILTER)
> +		vlan_ops->ena_rx_filtering(vsi);
> +
> +	if (restore_mc)
> +		ice_fltr_clear_vsi_promisc(hw, vsi->idx, ICE_MCAST_PROMISC_BITS,
> +					   0);
> +
> +	return 0;
> +}
> +
> +/* Drop the per-VID multicast promisc rules, redundant once the default
> + * VSI Rx rule covers every VID. A no-op when the VSI has no VLANs.
> + */
> +static void ice_vsi_clear_vlan_mc_promisc(struct ice_vsi *vsi)
> +{
> +	if (ice_vsi_has_non_zero_vlans(vsi))

Nit: could flip condition to decrease indent level.

> +		ice_fltr_clear_vlan_vsi_promisc(&vsi->back->hw, vsi,
> +						ICE_MCAST_VLAN_PROMISC_BITS);

Error code ignored, not sure if intentionally.

> +}
> +
>  /**
>   * ice_vsi_sync_fltr - Update the VSI filter list to the HW
>   * @vsi: ptr to the VSI
> @@ -429,30 +494,35 @@ static int ice_vsi_sync_fltr(struct ice_vsi *vsi)
>  				err = 0;
>  				vlan_ops->dis_rx_filtering(vsi);
>  
> -				/* promiscuous mode implies allmulticast so
> -				 * that VSIs that are in promiscuous mode are
> -				 * subscribed to multicast packets coming to
> -				 * the port
> +				/* DFLT now covers every VID; drop the per-VID
> +				 * multicast promisc rules a prior IFF_ALLMULTI
> +				 * pass may have installed (separate passes on a
> +				 * bridge join) so they do not linger and exhaust
> +				 * the FLU pool. exit_dflt_promisc() reinstates

Please use the full function name.

> +				 * them on promisc off.
>  				 */
> -				err = ice_set_promisc(vsi,
> -						      ICE_MCAST_PROMISC_BITS);
> -				if (err)
> -					goto out_promisc;
> +				ice_vsi_clear_vlan_mc_promisc(vsi);
>  			}
> +
> +			/* Promiscuous mode implies allmulticast. Subscribe
> +			 * the VSI to all multicast even when the default VSI
> +			 * rule is already in use and the block above is
> +			 * skipped (it may be owned by another VSI, or
> +			 * preserved across a switchdev session); the unicast
> +			 * catch-all does not cover the multicast subscription.
> +			 */
> +			err = ice_set_promisc(vsi, ICE_MCAST_PROMISC_BITS);
> +			if (err)
> +				goto out_promisc;
>  		} else {
>  			/* Clear Rx filter to remove traffic from wire */
>  			if (ice_is_vsi_dflt_vsi(vsi)) {
> -				err = ice_clear_dflt_vsi(vsi);
> +				err = ice_vsi_exit_dflt_promisc(vsi);
>  				if (err) {
> -					netdev_err(netdev, "Error %d clearing default VSI %i Rx rule\n",
> -						   err, vsi->vsi_num);
>  					vsi->current_netdev_flags |=
>  						IFF_PROMISC;
>  					goto out_promisc;
>  				}
> -				if (vsi->netdev->features &
> -				    NETIF_F_HW_VLAN_CTAG_FILTER)
> -					vlan_ops->ena_rx_filtering(vsi);
>  			}
>  
>  			/* disable allmulti here, but only if allmulti is not
> @@ -3676,10 +3746,9 @@ int ice_vlan_rx_add_vid(struct net_device *netdev, __be16 proto, u16 vid)
>  	while (test_and_set_bit(ICE_CFG_BUSY, vsi->state))
>  		usleep_range(1000, 2000);
>  
> -	/* Add multicast promisc rule for the VLAN ID to be added if
> -	 * all-multicast is currently enabled.
> -	 */
> -	if (vsi->current_netdev_flags & IFF_ALLMULTI) {
> +	/* skip the per-VID rule when the DFLT Rx rule already covers this VID */
> +	if ((vsi->current_netdev_flags & IFF_ALLMULTI) &&
> +	    !ice_is_vsi_dflt_vsi(vsi)) {
>  		ret = ice_fltr_set_vsi_promisc(&vsi->back->hw, vsi->idx,
>  					       ICE_MCAST_VLAN_PROMISC_BITS,
>  					       vid);
Thanks,
Marcin

^ permalink raw reply

* Re: [PATCH net v2] ppp: defer channel free to an RCU grace period to fix pppol2tp RX UAF
From: Breno Leitao @ 2026-07-03 16:32 UTC (permalink / raw)
  To: Qingfang Deng
  Cc: Norbert Szetei, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Sebastian Andrzej Siewior, Taegu Ha,
	Kees Cook, linux-ppp, linux-kernel, Guillaume Nault, netdev
In-Reply-To: <de2616b3-6edf-4255-ba77-0674e225ab27@linux.dev>

On Fri, Jul 03, 2026 at 03:27:00PM +0800, Qingfang Deng wrote:
> Hi,
> 
> On 2026/7/2 2:12, Norbert Szetei wrote:
> > +/* Purge after the grace period: a late ppp_input() may still queue an
> > + * skb on pch->file.rq before the last RCU reader drains.
> > + */
> > +static void ppp_release_channel_free(struct rcu_head *rcu)
> > +{
> > +	struct channel *pch = container_of(rcu, struct channel, rcu);
> > +
> > +	skb_queue_purge(&pch->file.xq);
> > +	skb_queue_purge(&pch->file.rq);
> > +	kfree(pch);
> > +}
> > +
> >   /*
> >    * Drop a reference to a ppp channel and free its memory if the refcount reaches
> >    * zero.
> > @@ -3581,9 +3594,7 @@ static void ppp_release_channel(struct channel *pch)
> >   		pr_err("ppp: destroying undead channel %p !\n", pch);
> >   		return;
> >   	}
> > -	skb_queue_purge(&pch->file.xq);
> > -	skb_queue_purge(&pch->file.rq);
> > -	kfree(pch);
> > +	call_rcu(&pch->rcu, ppp_release_channel_free);
> >   }
> >   static void __exit ppp_cleanup(void)
> 
> AI-review found an issue: https://sashiko.dev/#/patchset/D9C0245B-608B-4884-8A09-F55BA4A9F948%40doyensec.com
> 
> An rcu_barrier() call is needed at the end of ppp_cleanup().

I was initially unclear why rcu_barrier() would be necessary on a kfree path,
but it appears to be required during module unload to ensure that
ppp_release_channel_free() completes before the module's struct rcu_head is
destroyed. Is that the correct understanding?

^ permalink raw reply

* Re: [PATCH net-next v3 01/15] net: macb: drop "consistent" from alloc/free function names
From: Théo Lebrun @ 2026-07-03 16:32 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Conor Dooley, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Richard Cochran, Russell King,
	netdev, linux-kernel, Nicolas Ferre, Claudiu Beznea,
	Paolo Valerio, Nicolai Buchwitz, Vladimir Kondratiev,
	Gregory CLEMENT, Benoît Monin, Tawfik Bayouk,
	Thomas Petazzoni, Maxime Chevallier
In-Reply-To: <20260703-stucco-broadly-d9393f789868@spud>

Hello Conor,

On Fri Jul 3, 2026 at 1:28 PM CEST, Conor Dooley wrote:
> On Wed, Jul 01, 2026 at 05:59:04PM +0200, Théo Lebrun wrote:
>> Since commit 4df95131ea80 ("net/macb: change RX path for GEM") those
>> functions have not been only allocating or freeing consistent memory
>> mappings.
>> 
>> Rename from macb_alloc_consistent() to macb_alloc() and
>>        from macb_free_consistent()  to macb_free().
>
> What does "consistent" even mean? Is it intended to be analogous to
> coherent?

That's my guess. We used to have pci_alloc_consistent(), being an alias
to dma_alloc_coherent().

This alias has been present since forever (since Git):

   git show 1da177e4c3f4:include/asm-generic/pci-dma-compat.h

The alias got removed in commit 7968778914e5 ("PCI: Remove the
deprecated "pci-dma-compat.h" API", 2022-03-09).

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


^ permalink raw reply

* Re: [PATCH net-next v9 4/4] net: phy: realtek: load firmware for RTL8261C_CG
From: Andrew Lunn @ 2026-07-03 16:30 UTC (permalink / raw)
  To: javen
  Cc: hkallweit1, linux, davem, edumazet, kuba, pabeni, freddy_gu, nb,
	maxime.chevallier, netdev, linux-kernel, daniel, vladimir.oltean
In-Reply-To: <20260703071330.1707-5-javen_xu@realsil.com.cn>

On Fri, Jul 03, 2026 at 03:13:30PM +0800, javen wrote:
> From: Javen Xu <javen_xu@realsil.com.cn>
> 
> This patch adds support for loading firmware. Download some parameters
> for RTL8261C_CG.
> 
> Signed-off-by: Javen Xu <javen_xu@realsil.com.cn>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCHv2 3/4] mmc: sdhci-esdhc-mcf: do not use readl()/writel() on ColdFire
From: Adrian Hunter @ 2026-07-03 16:29 UTC (permalink / raw)
  To: Greg Ungerer, linux-m68k
  Cc: linux-kernel, arnd, wei.fang, frank.li, shenwei.wang, imx, netdev,
	nico, adureghello, ulfh, linux-mmc, linux-can, linux-spi, olteanv
In-Reply-To: <20260609142139.1563360-5-gerg@linux-m68k.org>

On 09/06/2026 17:13, Greg Ungerer wrote:
> The implementation of the readX() and writeX() family of IO access
> functions is non-standard on ColdFire platforms. They check the supplied
> IO address and will return either big or little endian results based on
> that check. This is non-standard, they are expected to always return
> little-endian byte ordered data. Unfortunately this behavior also means
> that ioreadX()/iowroteX() and their big-endian counter parts
> ioreadXbe()/iowriteXbe() are wrong. This is now in the process of being
> cleaned up and fixed.
> 
> Change the use of the readX() and writeX() access functions in this driver
> to use the recently defined specific ColdFire internal SoC hardware IO
> access functions mcf_read8()/mcf_read16()/mcf_read32() and
> mcf_write8()/mcf_write16()/mcf_write32().
> 
> There is no functional change to the driver. Though it does have the
> effect of making the IO access slightly more efficient, since there is
> no longer a need to do the address check at every register access.
> 
> Acked-by: Angelo Dureghello <adureghello@baylibre.com>
> Tested-by: Angelo Dureghello <adureghello@baylibre.com>
> Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
> v2: moved from RFC to PATCH
> 
>  drivers/mmc/host/sdhci-esdhc-mcf.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-mcf.c b/drivers/mmc/host/sdhci-esdhc-mcf.c
> index 375fce5639d7..6853521e8b2c 100644
> --- a/drivers/mmc/host/sdhci-esdhc-mcf.c
> +++ b/drivers/mmc/host/sdhci-esdhc-mcf.c
> @@ -55,7 +55,7 @@ static inline void esdhc_clrset_be(struct sdhci_host *host,
>  	if (reg == SDHCI_HOST_CONTROL)
>  		val |= ESDHC_PROCTL_D3CD;
>  
> -	writel((readl(base) & ~mask) | val, base);
> +	mcf_write32((mcf_read32(base) & ~mask) | val, base);
>  }
>  
>  /*
> @@ -71,7 +71,7 @@ static void esdhc_mcf_writeb_be(struct sdhci_host *host, u8 val, int reg)
>  	if (reg == SDHCI_HOST_CONTROL) {
>  		u32 host_ctrl = ESDHC_DEFAULT_HOST_CONTROL;
>  		u8 dma_bits = (val & SDHCI_CTRL_DMA_MASK) >> 3;
> -		u8 tmp = readb(host->ioaddr + SDHCI_HOST_CONTROL + 1);
> +		u8 tmp = mcf_read8(host->ioaddr + SDHCI_HOST_CONTROL + 1);
>  
>  		tmp &= ~0x03;
>  		tmp |= dma_bits;
> @@ -82,12 +82,12 @@ static void esdhc_mcf_writeb_be(struct sdhci_host *host, u8 val, int reg)
>  		 */
>  		host_ctrl |= val;
>  		host_ctrl |= (dma_bits << 8);
> -		writel(host_ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
> +		mcf_write32(host_ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
>  
>  		return;
>  	}
>  
> -	writel((readl(base) & mask) | (val << shift), base);
> +	mcf_write32((mcf_read32(base) & mask) | (val << shift), base);
>  }
>  
>  static void esdhc_mcf_writew_be(struct sdhci_host *host, u16 val, int reg)
> @@ -110,24 +110,24 @@ static void esdhc_mcf_writew_be(struct sdhci_host *host, u16 val, int reg)
>  		 * As for the fsl driver,
>  		 * we have to set the mode in a single write here.
>  		 */
> -		writel(val << 16 | mcf_data->aside,
> +		mcf_write32(val << 16 | mcf_data->aside,
>  		       host->ioaddr + SDHCI_TRANSFER_MODE);
>  		return;
>  	}
>  
> -	writel((readl(base) & mask) | (val << shift), base);
> +	mcf_write32((mcf_read32(base) & mask) | (val << shift), base);
>  }
>  
>  static void esdhc_mcf_writel_be(struct sdhci_host *host, u32 val, int reg)
>  {
> -	writel(val, host->ioaddr + reg);
> +	mcf_write32(val, host->ioaddr + reg);
>  }
>  
>  static u8 esdhc_mcf_readb_be(struct sdhci_host *host, int reg)
>  {
>  	if (reg == SDHCI_HOST_CONTROL) {
>  		u8 __iomem *base = host->ioaddr + (reg & ~3);
> -		u16 val = readw(base + 2);
> +		u16 val = mcf_read16(base + 2);
>  		u8 dma_bits = (val >> 5) & SDHCI_CTRL_DMA_MASK;
>  		u8 host_ctrl = val & 0xff;
>  
> @@ -137,7 +137,7 @@ static u8 esdhc_mcf_readb_be(struct sdhci_host *host, int reg)
>  		return host_ctrl;
>  	}
>  
> -	return readb(host->ioaddr + (reg ^ 0x3));
> +	return mcf_read8(host->ioaddr + (reg ^ 0x3));
>  }
>  
>  static u16 esdhc_mcf_readw_be(struct sdhci_host *host, int reg)
> @@ -149,14 +149,14 @@ static u16 esdhc_mcf_readw_be(struct sdhci_host *host, int reg)
>  	if (reg == SDHCI_HOST_VERSION)
>  		reg -= 2;
>  
> -	return readw(host->ioaddr + (reg ^ 0x2));
> +	return mcf_read16(host->ioaddr + (reg ^ 0x2));
>  }
>  
>  static u32 esdhc_mcf_readl_be(struct sdhci_host *host, int reg)
>  {
>  	u32 val;
>  
> -	val = readl(host->ioaddr + reg);
> +	val = mcf_read32(host->ioaddr + reg);
>  
>  	/*
>  	 * RM (25.3.9) sd pin clock must never exceed 25Mhz.
> @@ -245,7 +245,7 @@ static void esdhc_mcf_pltfm_set_clock(struct sdhci_host *host,
>  	 * fvco = fsys * outdvi1 + 1
>  	 * fshdc = fvco / outdiv3 + 1
>  	 */
> -	temp = readl(pll_dr);
> +	temp = mcf_read32(pll_dr);
>  	fsys = pltfm_host->clock;
>  	fvco = fsys * ((temp & 0x1f) + 1);
>  	fesdhc = fvco / (((temp >> 10) & 0x1f) + 1);


^ permalink raw reply

* Re: [PATCH net-next v7 08/11] udp: Set length in UDP header to 0 for big GSO packets
From: Alice Mikityanska @ 2026-07-03 16:28 UTC (permalink / raw)
  To: Paolo Abeni, Daniel Borkmann, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Xin Long, Willem de Bruijn, Willem de Bruijn,
	David Ahern, Nikolay Aleksandrov
  Cc: Shuah Khan, Stanislav Fomichev, Andrew Lunn, Simon Horman,
	Florian Westphal, netdev, Alice Mikityanska
In-Reply-To: <617cfbfe-0b42-4ce8-a283-5c99da26bb10@redhat.com>

On Sun, Jun 14, 2026, at 16:19, Paolo Abeni wrote:
> On 6/11/26 9:29 PM, Alice Mikityanska wrote:
>> diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
>> index dcff7fb16ff6..32525a051a6f 100644
>> --- a/net/ipv6/ip6_udp_tunnel.c
>> +++ b/net/ipv6/ip6_udp_tunnel.c
>> @@ -93,7 +93,7 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
>>  	uh->dest = dst_port;
>>  	uh->source = src_port;
>>  
>> -	udp_set_len_short(uh, skb->len);
>> +	udp_set_len(uh, skb->len);
>
> Both Sashikos noted the above breaks GSO csum, as the following
> udp_set_csum() will use skb->len to compute the csum partial.

I think the existing convention is to actually use skb->len even when
it's bigger than 64k. For example, see [1]. In order to keep this
working, I kept using skb->len for the pseudoheader checksum. I think I
made some fixups in one of previous iterations, because some places
missed using skb->len.

Regarding Sashiko's comment, I've already replied to it when it
commented the same thing on v3. These packets don't do to
__udp_gso_segment, they go to skb_udp_tunnel_segment, which calls
__skb_udp_tunnel_segment and adjusts the checksum with skb->len [1].

Please correct me if I'm missing some cases, but it looks correct
to me as is.

[1]: https://elixir.bootlin.com/linux/v7.1.2/source/net/ipv4/udp_offload.c#L192-L202

> I think it would be useful to consolidate udp_set_len() and
> udp_set_csum() in a single helper - say udp_set_csum_len():
>
> void udp_set_csum_len(bool nocheck, struct sk_buff *skb,
> 		      __be32 saddr, __be32 daddr, int len)
> {
> 	struct udphdr *uh = udp_hdr(skb);
>
>	
> 	uh->len = htons(len);
> 	if (nocheck) {
> 		uh->check = 0;
> 	} else if (skb_is_gso(skb)) {
> 		uh->len = len < GRO_LEGACY_MAX_SIZE ? htons(len) : 0;
> 		uh->check = ~udp_v4_check(len, saddr, daddr, 0);
> 	} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> 		uh->check = 0;
> 		uh->check = udp_v4_check(len, saddr, daddr,
> 	// ...
>
> so that csum and len are always consistent, the 'set' logic is symmetric
> with udp_get_len(), and no duplicate checks are needed.
>
> /P

^ permalink raw reply

* Re: [RFC] connectat()/bindat() or an alternative design
From: John Ericson @ 2026-07-03 16:17 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Cong Wang, Li Chen, Andy Lutomirski, Jens Axboe, network dev,
	linux-fsdevel, Sergei Zimmerman
In-Reply-To: <20260703-teamgeist-ganoven-kundig-eff19aa92e38@brauner>

On Fri, Jul 3, 2026, at 9:35 AM, Christian Brauner wrote:
> Please stop sending a bunch of disparate patch series that all do
> slightly related or overlapping things and point back at each other.
> 
> It's completely impossible to follow for anyone what's going on without
> chasing down discussion state across multiple subsystems. The net people
> have zero insight onto the fs struct discussions and it's completely
> pointless to try and design all of this based on thin air. Nothing is
> locked-in yet. This is not how this goes.
> 
> This frantic pushing of various features doesn't scale. It is requesting
> costly review time for multiple RFC series

I'm sorry for the confusion and the demands on your review time from
concurrently spreading my various Capsicum-like ideas across too many
threads. I can hereafter make things more single-threaded, as I'll
propose below. I hope that helps.

Just so we are all on the same page, at the moment I believe I have
created two RFC discussion threads, and submitted one RFC patch series:

- discussion thread: [RFC] Null Namespaces
  URL: https://lore.kernel.org/all/a49ce818-f38d-41b0-bbf7-80b8aad998b1@app.fastmail.com/
  Current status:
    - You (Christian) are writing nullfs/failfs FDs.
    - I am submitting nothing else (e.g. the actual null namespaces)
      until that is done.

- discussion thread: [RFC] connectat()/bindat() or an alternative design
  (this one)
  URL: https://lore.kernel.org/all/b1af80fc-a57c-408d-bdfe-fa6bae26eaca@app.fastmail.com/
  Current status: Working on one patch:
    - Patch Series: [RFC PATCH 0/3] coredump, net: fix layer violation
      with direct connection
      URL: https://lore.kernel.org/all/20260703073948.2541875-1-John.Ericson@Obsidian.Systems/
    - I am not submitting any UAPI changes until that is done.
  (Note: This thread had some code in the first message, but I never
  meant for that to be a formal submission, just a discussion aid, and
  the conversation moved on from that original design anyways.)

To make things maximally single-threaded, I could pause working on
"coredump, net: fix layer violation" (i.e. preparing a v2 based on what
you requested in your review) until you are done implementing
nullfs/failfs FDs. I am actually quite happy to do that if you want; I
should spend less time on the computer the next few days anyways, so the
timing is very good for that.

Either way, I can also henceforth not refer to yet-unwritten and
-unmerged patches like nullfs/failfs FDs in the motivation for other
things.

John

P.S. There is also my non-RFC bugfix series:

[PATCH net] af_unix: fix listen() succeeding on sockets in the wrong state
URL: https://lore.kernel.org/all/20260703081416.2583118-1-John.Ericson@Obsidian.Systems/

but I don't believe you were referring to that one, since I think
bugfixes are supposed to be submitted separately and immediately. Do
correct me if I am wrong.

^ permalink raw reply

* bpf, sockmap: FIONREAD returns 0 for TCP sockets in a sockmap without a verdict program
From: Mattia Meleleo @ 2026-07-03 16:10 UTC (permalink / raw)
  To: bpf; +Cc: netdev, john.fastabend@gmail.com, jakub@cloudflare.com,
	jiayuan.chen

Hi,

in OpenTelemetry eBPF instrumentation we use a sockhash to track
outgoing TCP sockets for trace context propagation. Sockets are added
from a sock_ops program, and the map only has an sk_msg program
attached on the egress side - there is no ingress verdict program.

Since commit 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap"),
ioctl(FIONREAD) returns 0 for these sockets even though read() returns
data. tcp_bpf_ioctl() answers from psock->msg_tot_len, which only
accounts for bytes in ingress_msg. Without a verdict program, data
never lands there: it stays in sk_receive_queue and is read through
the tcp_bpf_recvmsg() fallback, so FIONREAD always reports 0.

The commit message explains that sk_receive_queue is intentionally not
counted because a verdict program may redirect or drop its contents.
Without a verdict program, however, all of that data is readable, and
applications that use FIONREAD to size their reads (nginx, Java, .NET)
hang or truncate transfers once their sockets are in the map.

Observed on mainline and on 6.12.75+, 6.6.128+, 6.18.14+.

Reproducer: https://github.com/mmat11/fionread-repro

  pre-insert     FIONREAD=4096 (expect 4096) OK
  in-sockhash    FIONREAD=0 (expect 4096) BROKEN
  post-delete    FIONREAD=4096 (expect 4096) OK

Is this intended for psocks without a verdict program? If not, would
falling back to tcp_inq() (plus msg_tot_len) in that case be an
acceptable fix? Happy to send a patch and test it.

Thanks,
Mattia

^ permalink raw reply

* Re: [PATCH net] gve: fix Rx queue stall on alloc failure
From: Maciej Fijalkowski @ 2026-07-03 16:05 UTC (permalink / raw)
  To: Eddie Phillips
  Cc: Harshitha Ramamurthy, netdev, joshwash, andrew+netdev, davem,
	edumazet, kuba, pabeni, ast, daniel, hawk, john.fastabend, bpf,
	sdf, willemb, jordanrhee, nktgrg, maolson, jacob.e.keller,
	thostet, csully, bcf, linux-kernel, stable
In-Reply-To: <CAPBb8HmE6q0VPa5PooFP3VFF27GU3B4622Xww6MHRT-9i4zTxA@mail.gmail.com>

On Fri, Jul 03, 2026 at 01:03:20AM -0700, Eddie Phillips wrote:
> > I think this deserves to be pulled out of the timer logic?
> 
> If by this you mean pull the stats into a separate patch, I agree.

Hi Eddie,

instead of forming a response at the top of the mail, please have your
answers inlined; it is preferred way of communication on mailing lists.

> 
> > - couldn't you detect this case within napi poll loop?
> 
> It can only be detected after attempting to refill the queue and finding
> that we are still below the critical threshold.
> 
> > - if not, does it have to be per-q timer? wouldn't one global per pf timer
> >   satisfy your needs?
> 
> There are a few ways a global timer could be implemented,
>  - The global timer could queue napi for *all* queues, which would
> result in a lot of unnecessary work.
>  - The global timer could iterate over each queue and try to detect
> the critical low buffer condition, however this would require
> introducing synchronization between the timer and the napis, which
> would introduce expensive locking into the hot path.
>  - The global timer could be paired with a bitmap that stores which
> queues need to be serviced.

bitmap would probably do the job but i won't insist here tho.

One more question/idea:
Before arming the starvation timer, could we first try to make a smaller batch
of already-posted buffers visible to HW?

It seems the HW can accept RX buffer tail doorbell updates at a granularity
lower than the normal `GVE_RX_BUF_THRESH_DQO` batching threshold, apparently as
low as 8 descriptors. If that is the case, could we first use this as an
emergency low-watermark path: when refill posts at least 8 descriptors but does
not reach the normal 32-descriptor threshold, ring the doorbell immediately and
only arm the starvation timer if even that lower threshold cannot be reached?

> 
> A `struct timer_list` is only 40 bytes, so the current implemention is
> not expensive. Though a global timer is valid, it's not strictly better.
> 
> That said, I agree that we can clean up the structure—I will move the
> timer state from the individual RX rings to the `gve_priv` structure.
> 
> On Wed, Jul 1, 2026 at 6:22 AM Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > On Wed, Jul 01, 2026 at 12:53:41AM +0000, Harshitha Ramamurthy wrote:
> > > From: Eddie Phillips <eddiephillips@google.com>
> > >
> > > When the system is under extreme memory pressure, page allocations can
> > > fail during the Rx buffer refill loop. If the number of buffers posted
> > > to hardware falls below a critical low threshold and the refill loop
> > > exits due to allocation failures, the queue can stall:
> > >
> > > 1. The device drops incoming packets because there are no descriptors.
> > > 2. Since no packets are processed, no Rx completions are generated.
> > > 3. Because no completions occur, NAPI is never scheduled, preventing
> > >    the refill loop from running again even after memory is freed.
> > >
> > > This results in a permanent queue stall.
> > >
> > > Resolve this by introducing a starvation recovery timer for each Rx queue.
> > > If the number of buffers posted to hardware falls below a critical low
> > > threshold, start a timer to periodically reschedule NAPI. Once NAPI runs
> > > and successfully refills the queue above the threshold, the timer is
> > > not rescheduled.
> > >
> > > Also add a new ethtool statistic "rx_critical_low_bufs" to track the
> > > number of times the starvation recovery timer is triggered.
> >
> > I think this deserves to be pulled out of the timer logic?
> >
> > Two questions tho:
> > - couldn't you detect this case within napi poll loop?
> > - if not, does it have to be per-q timer? wouldn't one global per pf timer
> >   satisfy your needs?
> >
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: 9b8dd5e5ea48 ("gve: DQO: Add RX path")
> > > Reviewed-by: Jordan Rhee <jordanrhee@google.com>
> > > Signed-off-by: Eddie Phillips <eddiephillips@google.com>
> > > Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
> > > ---
> > >  drivers/net/ethernet/google/gve/gve.h         |  4 ++++
> > >  drivers/net/ethernet/google/gve/gve_ethtool.c | 14 +++++++++++++-
> > >  drivers/net/ethernet/google/gve/gve_rx_dqo.c  | 32 ++++++++++++++++++++++++++++++++
> > >  3 files changed, 49 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
> > > index 2f7bd330..8378bef2 100644
> > > --- a/drivers/net/ethernet/google/gve/gve.h
> > > +++ b/drivers/net/ethernet/google/gve/gve.h
> > > @@ -13,6 +13,7 @@
> > >  #include <linux/netdevice.h>
> > >  #include <linux/net_tstamp.h>
> > >  #include <linux/pci.h>
> > > +#include <linux/timer.h>
> > >  #include <linux/ptp_clock_kernel.h>
> > >  #include <linux/u64_stats_sync.h>
> > >  #include <net/page_pool/helpers.h>
> > > @@ -41,6 +42,7 @@
> > >
> > >  /* Interval to schedule a stats report update, 20000ms. */
> > >  #define GVE_STATS_REPORT_TIMER_PERIOD        20000
> > > +#define GVE_RX_NAPI_RESCHED_MS 20 /* msecs */
> > >
> > >  /* Numbers of NIC tx/rx stats in stats report. */
> > >  #define NIC_TX_STATS_REPORT_NUM      0
> > > @@ -318,6 +320,7 @@ struct gve_rx_ring {
> > >       u64 rx_copied_pkt; /* free-running total number of copied packets */
> > >       u64 rx_skb_alloc_fail; /* free-running count of skb alloc fails */
> > >       u64 rx_buf_alloc_fail; /* free-running count of buffer alloc fails */
> > > +     u64 rx_critical_low_bufs; /* count of critical low buffer events */
> > >       u64 rx_desc_err_dropped_pkt; /* free-running count of packets dropped by descriptor error */
> > >       /* free-running count of unsplit packets due to header buffer overflow or hdr_len is 0 */
> > >       u64 rx_hsplit_unsplit_pkt;
> > > @@ -334,6 +337,7 @@ struct gve_rx_ring {
> > >       struct gve_queue_resources *q_resources; /* head and tail pointer idx */
> > >       dma_addr_t q_resources_bus; /* dma address for the queue resources */
> > >       struct u64_stats_sync statss; /* sync stats for 32bit archs */
> > > +     struct timer_list starvation_timer; /* for queue starvation recovery */
> > >
> > >       struct gve_rx_ctx ctx; /* Info for packet currently being processed in this ring. */
> > >
> > > diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
> > > index a0e0472b..71b6efbf 100644
> > > --- a/drivers/net/ethernet/google/gve/gve_ethtool.c
> > > +++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
> > > @@ -46,6 +46,7 @@ static const char gve_gstrings_main_stats[][ETH_GSTRING_LEN] = {
> > >       "rx_hsplit_unsplit_pkt",
> > >       "interface_up_cnt", "interface_down_cnt", "reset_cnt",
> > >       "page_alloc_fail", "dma_mapping_error", "stats_report_trigger_cnt",
> > > +     "rx_critical_low_bufs",
> > >  };
> > >
> > >  static const char gve_gstrings_rx_stats[][ETH_GSTRING_LEN] = {
> > > @@ -58,6 +59,7 @@ static const char gve_gstrings_rx_stats[][ETH_GSTRING_LEN] = {
> > >       "rx_xdp_aborted[%u]", "rx_xdp_drop[%u]", "rx_xdp_pass[%u]",
> > >       "rx_xdp_tx[%u]", "rx_xdp_redirect[%u]",
> > >       "rx_xdp_tx_errors[%u]", "rx_xdp_redirect_errors[%u]", "rx_xdp_alloc_fails[%u]",
> > > +     "rx_critical_low_bufs[%u]",
> > >  };
> > >
> > >  static const char gve_gstrings_tx_stats[][ETH_GSTRING_LEN] = {
> > > @@ -151,12 +153,14 @@ gve_get_ethtool_stats(struct net_device *netdev,
> > >  {
> > >       u64 tmp_rx_pkts, tmp_rx_hsplit_pkt, tmp_rx_bytes, tmp_rx_hsplit_bytes,
> > >               tmp_rx_skb_alloc_fail, tmp_rx_buf_alloc_fail,
> > > +             tmp_rx_critical_low_bufs,
> > >               tmp_rx_desc_err_dropped_pkt, tmp_rx_hsplit_unsplit_pkt,
> > >               tmp_tx_pkts, tmp_tx_bytes,
> > >               tmp_xdp_tx_errors, tmp_xdp_redirect_errors;
> > >       u64 rx_buf_alloc_fail, rx_desc_err_dropped_pkt, rx_hsplit_unsplit_pkt,
> > >               rx_pkts, rx_hsplit_pkt, rx_skb_alloc_fail, rx_bytes, tx_pkts, tx_bytes,
> > > -             tx_dropped, xdp_tx_errors, xdp_redirect_errors;
> > > +             rx_critical_low_bufs, tx_dropped, xdp_tx_errors,
> > > +             xdp_redirect_errors;
> > >       int rx_base_stats_idx, max_rx_stats_idx, max_tx_stats_idx;
> > >       int stats_idx, stats_region_len, nic_stats_len;
> > >       struct stats *report_stats;
> > > @@ -197,6 +201,7 @@ gve_get_ethtool_stats(struct net_device *netdev,
> > >
> > >       for (rx_pkts = 0, rx_bytes = 0, rx_hsplit_pkt = 0,
> > >            rx_skb_alloc_fail = 0, rx_buf_alloc_fail = 0,
> > > +          rx_critical_low_bufs = 0,
> > >            rx_desc_err_dropped_pkt = 0, rx_hsplit_unsplit_pkt = 0,
> > >            xdp_tx_errors = 0, xdp_redirect_errors = 0,
> > >            ring = 0;
> > > @@ -212,6 +217,8 @@ gve_get_ethtool_stats(struct net_device *netdev,
> > >                               tmp_rx_bytes = rx->rbytes;
> > >                               tmp_rx_skb_alloc_fail = rx->rx_skb_alloc_fail;
> > >                               tmp_rx_buf_alloc_fail = rx->rx_buf_alloc_fail;
> > > +                             tmp_rx_critical_low_bufs =
> > > +                                     rx->rx_critical_low_bufs;
> > >                               tmp_rx_desc_err_dropped_pkt =
> > >                                       rx->rx_desc_err_dropped_pkt;
> > >                               tmp_rx_hsplit_unsplit_pkt =
> > > @@ -226,6 +233,7 @@ gve_get_ethtool_stats(struct net_device *netdev,
> > >                       rx_bytes += tmp_rx_bytes;
> > >                       rx_skb_alloc_fail += tmp_rx_skb_alloc_fail;
> > >                       rx_buf_alloc_fail += tmp_rx_buf_alloc_fail;
> > > +                     rx_critical_low_bufs += tmp_rx_critical_low_bufs;
> > >                       rx_desc_err_dropped_pkt += tmp_rx_desc_err_dropped_pkt;
> > >                       rx_hsplit_unsplit_pkt += tmp_rx_hsplit_unsplit_pkt;
> > >                       xdp_tx_errors += tmp_xdp_tx_errors;
> > > @@ -269,6 +277,7 @@ gve_get_ethtool_stats(struct net_device *netdev,
> > >       data[i++] = priv->page_alloc_fail;
> > >       data[i++] = priv->dma_mapping_error;
> > >       data[i++] = priv->stats_report_trigger_cnt;
> > > +     data[i++] = rx_critical_low_bufs;
> > >       i = GVE_MAIN_STATS_LEN;
> > >
> > >       rx_base_stats_idx = 0;
> > > @@ -337,6 +346,8 @@ gve_get_ethtool_stats(struct net_device *netdev,
> > >                               tmp_rx_hsplit_bytes = rx->rx_hsplit_bytes;
> > >                               tmp_rx_skb_alloc_fail = rx->rx_skb_alloc_fail;
> > >                               tmp_rx_buf_alloc_fail = rx->rx_buf_alloc_fail;
> > > +                             tmp_rx_critical_low_bufs =
> > > +                                     rx->rx_critical_low_bufs;
> > >                               tmp_rx_desc_err_dropped_pkt =
> > >                                       rx->rx_desc_err_dropped_pkt;
> > >                               tmp_xdp_tx_errors = rx->xdp_tx_errors;
> > > @@ -381,6 +392,7 @@ gve_get_ethtool_stats(struct net_device *netdev,
> > >                       } while (u64_stats_fetch_retry(&priv->rx[ring].statss,
> > >                                                      start));
> > >                       i += GVE_XDP_ACTIONS + 3; /* XDP rx counters */
> > > +                     data[i++] = tmp_rx_critical_low_bufs;
> > >               }
> > >       } else {
> > >               i += priv->rx_cfg.num_queues * NUM_GVE_RX_CNTS;
> > > diff --git a/drivers/net/ethernet/google/gve/gve_rx_dqo.c b/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> > > index 02cba280..303db4fa 100644
> > > --- a/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> > > +++ b/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> > > @@ -18,6 +18,16 @@
> > >  #include <net/tcp.h>
> > >  #include <net/xdp_sock_drv.h>
> > >
> > > +static void gve_rx_starvation_timer(struct timer_list *t)
> > > +{
> > > +     struct gve_rx_ring *rx = timer_container_of(rx, t, starvation_timer);
> > > +     struct gve_priv *priv = rx->gve;
> > > +     struct gve_notify_block *block;
> > > +
> > > +     block = &priv->ntfy_blocks[rx->ntfy_id];
> > > +     napi_schedule(&block->napi);
> > > +}
> > > +
> > >  static void gve_rx_free_hdr_bufs(struct gve_priv *priv, struct gve_rx_ring *rx)
> > >  {
> > >       struct device *hdev = &priv->pdev->dev;
> > > @@ -120,6 +130,7 @@ void gve_rx_stop_ring_dqo(struct gve_priv *priv, int idx)
> > >
> > >       if (rx->dqo.page_pool)
> > >               page_pool_disable_direct_recycling(rx->dqo.page_pool);
> > > +     timer_delete_sync(&rx->starvation_timer);
> > >       gve_remove_napi(priv, ntfy_idx);
> > >       gve_rx_remove_from_block(priv, idx);
> > >       gve_rx_reset_ring_dqo(priv, idx);
> > > @@ -136,6 +147,8 @@ void gve_rx_free_ring_dqo(struct gve_priv *priv, struct gve_rx_ring *rx,
> > >       u32 qpl_id;
> > >       int i;
> > >
> > > +     timer_shutdown_sync(&rx->starvation_timer);
> > > +
> > >       completion_queue_slots = rx->dqo.complq.mask + 1;
> > >       buffer_queue_slots = rx->dqo.bufq.mask + 1;
> > >
> > > @@ -232,6 +245,7 @@ int gve_rx_alloc_ring_dqo(struct gve_priv *priv,
> > >       rx->gve = priv;
> > >       rx->q_num = idx;
> > >       rx->packet_buffer_size = cfg->packet_buffer_size;
> > > +     timer_setup(&rx->starvation_timer, gve_rx_starvation_timer, 0);
> > >
> > >       if (cfg->xdp) {
> > >               rx->packet_buffer_truesize = GVE_XDP_RX_BUFFER_SIZE_DQO;
> > > @@ -365,6 +379,7 @@ void gve_rx_post_buffers_dqo(struct gve_rx_ring *rx)
> > >       struct gve_rx_compl_queue_dqo *complq = &rx->dqo.complq;
> > >       struct gve_rx_buf_queue_dqo *bufq = &rx->dqo.bufq;
> > >       struct gve_priv *priv = rx->gve;
> > > +     u32 num_bufs_avail_to_hw;
> > >       u32 num_avail_slots;
> > >       u32 num_full_slots;
> > >       u32 num_posted = 0;
> > > @@ -400,6 +415,23 @@ void gve_rx_post_buffers_dqo(struct gve_rx_ring *rx)
> > >       }
> > >
> > >       rx->fill_cnt += num_posted;
> > > +
> > > +     /* If the queue has fewer than GVE_RX_BUF_THRESH_DQO descriptors
> > > +      * visible to the hardware, and no doorbell was written, the hardware
> > > +      * is in danger of starving and cannot trigger interrupts. Start the
> > > +      * timer to periodically reschedule NAPI and recover from starvation.
> > > +      */
> > > +     num_bufs_avail_to_hw =
> > > +             ((bufq->tail & ~(GVE_RX_BUF_THRESH_DQO - 1)) -
> > > +              bufq->head) & bufq->mask;
> > > +
> > > +     if (num_bufs_avail_to_hw < GVE_RX_BUF_THRESH_DQO) {
> > > +             u64_stats_update_begin(&rx->statss);
> > > +             rx->rx_critical_low_bufs++;
> > > +             u64_stats_update_end(&rx->statss);
> > > +             mod_timer(&rx->starvation_timer,
> > > +                       jiffies + msecs_to_jiffies(GVE_RX_NAPI_RESCHED_MS));
> > > +     }
> > >  }
> > >
> > >  static void gve_rx_skb_csum(struct sk_buff *skb,
> > > --
> > > 2.55.0.rc2.803.g1fd1e6609c-goog
> > >
> > >

^ permalink raw reply

* Re: [PATCH net v2] ppp: defer channel free to an RCU grace period to fix pppol2tp RX UAF
From: Guillaume Nault @ 2026-07-03 16:05 UTC (permalink / raw)
  To: Qingfang Deng
  Cc: Norbert Szetei, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Sebastian Andrzej Siewior,
	Breno Leitao, Taegu Ha, Kees Cook, linux-ppp, linux-kernel,
	netdev
In-Reply-To: <166370f4-0b8c-4af4-9fb7-6967828a99bc@linux.dev>

On Thu, Jul 02, 2026 at 04:19:02PM +0800, Qingfang Deng wrote:
> Add: Guillaume
> 
> On 2026/7/2 at 2:12, Norbert Szetei wrote:
> > pppol2tp_recv() runs in the L2TP UDP-encap softirq RX path:
> > 
> >   l2tp_udp_encap_recv() -> l2tp_recv_common() -> pppol2tp_recv()
> >     -> ppp_input(&po->chan)

Hi Qingfang,

Thanks for Cc-ing me. I haven't had time to look at this problem yet,
and I'll be offline next week. So not sure if I'll get the possibility
to provide any feedback to this patch in time.

> > It runs under rcu_read_lock() holding only an l2tp_session reference and
> > takes NO reference on the internal PPP channel (struct channel,
> > chan->ppp) that ppp_input() dereferences.
> > 
> > The pppox socket is SOCK_RCU_FREE, so 'po' and the embedded ppp_channel
> > are RCU-safe.  But the internal struct channel is a separate allocation
> > that ppp_release_channel() frees with a plain kfree():
> > 
> >   close(data socket) -> pppol2tp_release() -> pppox_unbind_sock()
> >     -> ppp_unregister_channel() -> ppp_release_channel() -> kfree(pch)
> > 
> > For a channel that is bound (PPPIOCGCHAN) but not attached to a ppp unit
> > (no PPPIOCCONNECT, pch->ppp == NULL) and not bridged, teardown skips
> > both ppp_disconnect_channel()'s synchronize_net() and
> > ppp_unbridge_channels()'s synchronize_rcu(), so the kfree() has no grace
> > period.  rcu_read_lock() in pppol2tp_recv() does not protect against a
> > plain kfree(), so an in-flight ppp_input() on one CPU can dereference
> > the channel just freed by close() on another CPU.
> > 
> > The bug is reachable by an unprivileged user.
> > 
> > Defer the channel free to an RCU callback via call_rcu() so the grace
> > period fences any in-flight ppp_input(). The disconnect and unbridge
> > teardown paths already fence with synchronize_net()/synchronize_rcu();
> > call_rcu() does the same here without stalling the close() path.
> > 
> > Fixes: ee40fb2e1eb5 ("l2tp: protect sock pointer of struct pppol2tp_session with RCU")
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Norbert Szetei <norbert@doyensec.com>
> > ---
> > v2:
> > - Moved skb_queue_purge() to a dedicated RCU callback to prevent leaking
> >    skbs added by an in-flight ppp_input() during the grace period (Sebastian).
> > - Retained call_rcu() to avoid introducing synchronous multi-millisecond
> >    latency into the teardown path.
> > v1: https://lore.kernel.org/netdev/C954A7EA-AA98-4E3C-80B5-42C34B3183A3@doyensec.com/
> > 
> >   drivers/net/ppp/ppp_generic.c | 17 ++++++++++++++---
> >   1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
> > index 57c68efa5ff8..2d57de77780f 100644
> > --- a/drivers/net/ppp/ppp_generic.c
> > +++ b/drivers/net/ppp/ppp_generic.c
> > @@ -184,6 +184,7 @@ struct channel {
> >   	struct list_head clist;		/* link in list of channels per unit */
> >   	spinlock_t	upl;		/* protects `ppp' and 'bridge' */
> >   	struct channel __rcu *bridge;	/* "bridged" ppp channel */
> > +	struct rcu_head rcu;		/* for RCU-deferred free of the channel */
> >   #ifdef CONFIG_PPP_MULTILINK
> >   	u8		avail;		/* flag used in multilink stuff */
> >   	u8		had_frag;	/* >= 1 fragments have been sent */
> > @@ -3562,6 +3563,18 @@ ppp_disconnect_channel(struct channel *pch)
> >   	return err;
> >   }
> > +/* Purge after the grace period: a late ppp_input() may still queue an
> > + * skb on pch->file.rq before the last RCU reader drains.
> > + */
> > +static void ppp_release_channel_free(struct rcu_head *rcu)
> > +{
> > +	struct channel *pch = container_of(rcu, struct channel, rcu);
> > +
> > +	skb_queue_purge(&pch->file.xq);
> > +	skb_queue_purge(&pch->file.rq);
> > +	kfree(pch);
> > +}
> > +
> >   /*
> >    * Drop a reference to a ppp channel and free its memory if the refcount reaches
> >    * zero.
> > @@ -3581,9 +3594,7 @@ static void ppp_release_channel(struct channel *pch)
> >   		pr_err("ppp: destroying undead channel %p !\n", pch);
> >   		return;
> >   	}
> > -	skb_queue_purge(&pch->file.xq);
> > -	skb_queue_purge(&pch->file.rq);
> > -	kfree(pch);
> > +	call_rcu(&pch->rcu, ppp_release_channel_free);
> >   }
> >   static void __exit ppp_cleanup(void)
> 
> Reviewed-by: Qingfang Deng <qingfang.deng@linux.dev>
> 
> FYI, I attempted to merge the two channel structs and AI-review found a UAF
> [1], so this patch addresses the issue.
> 
> [1] https://lore.kernel.org/netdev/590d7931-02b0-45d6-8f43-ef909c9bde89@redhat.com/
> 
> Best regards,
> 
> Qingfang
> 
> 


^ permalink raw reply

* Re: [PATCH net-next] tun: no longer rely on RTNL in tun_fill_info()
From: Willem de Bruijn @ 2026-07-03 15:43 UTC (permalink / raw)
  To: Kuniyuki Iwashima, Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Andrew Lunn, netdev, eric.dumazet
In-Reply-To: <CAAVpQUC1UKqqUdQaP+ud_uGkhByxooMvCs1H7i7q=OzQFP7trA@mail.gmail.com>

Kuniyuki Iwashima wrote:
> On Wed, Jul 1, 2026 at 5:51 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > Update tun_fill_info() to read device configuration fields (flags, owner,
> > group, numqueues, numdisabled) locklessly using READ_ONCE().
> >
> > Annotate all writes to these fields in the control paths with WRITE_ONCE()
> > to prevent data races, as these fields can be modified concurrently via
> > ioctls (TUNSETPERSIST, TUNSETOWNER, TUNSETGROUP, TUNSETIFF) or queue
> > attaching/detaching.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support
From: Maxime Chevallier @ 2026-07-03 15:41 UTC (permalink / raw)
  To: Luke Howard, Vladimir Oltean, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Vivien Didelot, Gregory CLEMENT,
	Andrew Lunn, Richard Cochran
  Cc: Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel
In-Reply-To: <20260703-mv88e6xxx-ptp-fixes-v1-2-0138581889a9@padl.com>

Hi Luke,

On 7/3/26 08:41, Luke Howard wrote:
> mv88e6xxx switches can support embedding PTP timestamps directly
> in the frame, either as a trailer or at a configurable offset
> (typically the reserved bytes in the PTP header).
> 
> Add support for this on the 88E6341 and 88E6352 switches, being
> those on which I was able to verify this. Other switch chips may
> also work. The arrival timestamp offsets are relative to the PTP
> common header and will work for both L2 and L3 PTP packets; the
> respective headers are skipped by the number of bytes set in the
> ETJump and IPJump registers, which are initialized to sensible
> defaults and are VLAN tag-aware.
> 
> (Note: the 6352 datasheet incorrectly states that ETJump and
> IPJump are initialized to zero. They are initialized to 12 and
> 2 respectively; this is corrected in the 6341 data sheet.)
> 
> Signed-off-by: Luke Howard <lukeh@padl.com>

[...]

> +static bool parse_embedded_ts(unsigned int arr_ts_mode,
> +			      struct sk_buff *skb, u64 *ns)
> +{
> +	struct ptp_header *hdr;
> +
> +	*ns = 0;
> +
> +	/* APPEND means the switch appended the time stamp as a 4-byte trailer
> +	 * (not all switches support this). Any other non-zero value is the byte
> +	 * offset past the start of the PTP common header at which the switch
> +	 * overwrote the time stamp in place (e.g. the reserved header bytes).
> +	 */
> +	if (arr_ts_mode == MV88E6XXX_PTP_ARR_TS_MODE_APPEND && skb->len >= 4) {
> +		*ns = (u64)get_unaligned_be32(&skb->data[skb->len - 4]);

One thing is that you need to make sure you're dealing with a linear skb
when accessing the trailer, which can be done by calling skb_linearize(skb).

Then you should use skb_tail_pointer(skb) to get to the trailer.

You can take as examples the DSA taggers that pull the tag from the tail, such as
the tag_trailer [1], tag_ksz, tag_rtl8_4, etc. 

And finally you may have to use the chksum variants of the skb_trim such as
pskb_trim_rcsum(), like done in the above-mentionned taggers.

[1] : https://elixir.bootlin.com/linux/v7.1.2/source/net/dsa/tag_trailer.c#L28

Thanks,

Maxime

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: use direct ATU hash on 88E6141/6341
From: Andrew Lunn @ 2026-07-03 15:40 UTC (permalink / raw)
  To: Luke Howard
  Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Cedric Jehasse,
	Kieran Tyrrell, Max Holtmann, Max Hunter, Christoph Mellauner,
	Simon Gapp, netdev, linux-kernel
In-Reply-To: <20260703-mv88e6x41-fixes-v1-2-fbd3a1bf8965@padl.com>

On Fri, Jul 03, 2026 at 04:42:56PM +1000, Luke Howard wrote:
> The default 88E6341/88E6141 ATU hash algorithm appears to result
> in frequent collisions, evicting permanent registrations (including
> the broadcast address) out of the ATU.

Is there any documentation about how the 88E6341/88E6141 hashing
algorithm is different to all the other chips? How is it special?

> This workaround disables hasing in the ATU control register. It may

hashing.

> have a performance impact (the data sheet notes this is for testing
> only), but it also enables correctness, at least in local testing.

How do you define correctness? Are you using a well defined test?

Why is the devlink parameter not sufficient.

    Andrew

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: write the ATU FID register on 88E6141/88E6341
From: Andrew Lunn @ 2026-07-03 15:31 UTC (permalink / raw)
  To: Luke Howard
  Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Cedric Jehasse,
	Kieran Tyrrell, Max Holtmann, Max Hunter, Christoph Mellauner,
	Simon Gapp, netdev, linux-kernel
In-Reply-To: <20260703-mv88e6x41-fixes-v1-1-fbd3a1bf8965@padl.com>

On Fri, Jul 03, 2026 at 04:42:55PM +1000, Luke Howard wrote:
> The existing code assumed the 88E6141/88E6341 did not have a dedicated
> ATU FID register because of its database count (256), instead taking
> the legacy path which resulted in the FID register never being set.
> 
> This resulted in every FDB entry being loaded into FID 0, breaking
> VLAN aware bridging.
> 
> Fixes: a75961d0ebfd ("net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341")

Please submit fixes to the net tree.

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

> @@ -131,6 +131,7 @@ struct mv88e6xxx_info {
>  	u16 prod_num;
>  	const char *name;
>  	unsigned int num_databases;
> +	bool atu_fid_reg;
>  	unsigned int num_macs;
>  	unsigned int num_ports;
>  	unsigned int num_internal_phys;

Please think about padding. The current structure layout is not great,
invalid_port_mask should be somewhere else, but please don't make it
worse. Also, this structure has some reasonable comments. You have the
chance to comment that the number of FIBs does not imply if there is a
dedicated register, there are examples with 255 and a dedicated
register.

> diff --git a/drivers/net/dsa/mv88e6xxx/global1_atu.c b/drivers/net/dsa/mv88e6xxx/global1_atu.c
> index c47f068f56b32..aa5adc78607ca 100644
> --- a/drivers/net/dsa/mv88e6xxx/global1_atu.c
> +++ b/drivers/net/dsa/mv88e6xxx/global1_atu.c
> @@ -135,7 +135,7 @@ static int mv88e6xxx_g1_atu_op(struct mv88e6xxx_chip *chip, u16 fid, u16 op)
>  	int err;
>  
>  	/* FID bits are dispatched all around gradually as more are supported */
> -	if (mv88e6xxx_num_databases(chip) > 256) {
> +	if (mv88e6xxx_num_databases(chip) > 256 || chip->info->atu_fid_reg) {

So currently, > 256 implies a dedicated register.  But do we need both
> 255 and chip->info->atu_fid_reg? I would probably set atu_fid_reg
true for all devices which have a dedicated register.

    Andrew

---
pw-bot: cr

^ permalink raw reply

* Re: [PATCH net-next 1/2] net: dsa: mv88e6xxx: use ARRIVAL1 counter for all peer delay messages
From: Andrew Lunn @ 2026-07-03 15:14 UTC (permalink / raw)
  To: Luke Howard
  Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Richard Cochran,
	Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel
In-Reply-To: <20260703-mv88e6xxx-ptp-fixes-v1-1-0138581889a9@padl.com>

On Fri, Jul 03, 2026 at 04:41:58PM +1000, Luke Howard wrote:
> mv88e6xxx switches have two arrival timestamp counters for timestamping
> PTP event messages. This permits more than one arriving event message's
> timestamp to be captured. This is useful for the case where event
> messages from a grandmaster arrive at the same time as PDelayReq/PDelayResp
> messages from a peer.
> 
> Previously only PDelayResp messages were assigned to the second arrival
> counter; this patch does so for PDelayReq messages as well.
> 
> Signed-off-by: Luke Howard <lukeh@padl.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: embedded PTP timestamp support
From: Andrew Lunn @ 2026-07-03 15:13 UTC (permalink / raw)
  To: Luke Howard
  Cc: Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Vivien Didelot, Gregory CLEMENT, Richard Cochran,
	Cedric Jehasse, Kieran Tyrrell, Max Holtmann, Max Hunter,
	Christoph Mellauner, Simon Gapp, netdev, linux-kernel
In-Reply-To: <20260703-mv88e6xxx-ptp-fixes-v1-2-0138581889a9@padl.com>

> +	/* Arrival Time Stamp Mode (ArrTSMode); see the ArrTSMode encoding in
> +	 * hwtstamp.h. Zero (the default) leaves arrival time stamps in the
> +	 * switch registers; non-zero embeds them in the frame, either appended
> +	 * as a trailer or overwritten at that byte offset past the start of the
> +	 * PTP common header.

So how do you indicate trailer?

> +++ b/drivers/net/dsa/mv88e6xxx/hwtstamp.c
> @@ -8,6 +8,8 @@
>   *      Erik Hons <erik.hons@ni.com>
>   *      Brandon Streiff <brandon.streiff@ni.com>
>   *      Dane Wagner <dane.wagner@ni.com>
> + *
> + * Copyright (c) 2025 PADL Software Pty Ltd

It is unusual to add a copyright for the passed.

    Andrew

---
pw-bot: cr

^ permalink raw reply

* Re: [PATCH net-next v4 2/2] net: dsa: realtek: rtl8365mb: add HSGMII support for RTL8367S
From: Mieczyslaw Nalewaj @ 2026-07-03 15:12 UTC (permalink / raw)
  To: Johan Alvarado, linusw, alsi, andrew, olteanv, kuba, davem,
	edumazet, pabeni, linux
  Cc: luizluca, maxime.chevallier, netdev, linux-kernel
In-Reply-To: <0100019f24962e49-10163ad6-22fd-4528-b8f3-75703cfd80dc-000000@email.amazonses.com>

On 7/2/2026 10:47 PM, Johan Alvarado wrote:
> In addition to SGMII, the RTL8367S SerDes also supports HSGMII, which
> carries 2.5 Gbps with the same signaling as SGMII at 2.5x clock rate.
> The chip info table already declares HSGMII as a supported interface
> mode for external interface 1.
> 
> Extend the SerDes PCS to handle HSGMII, which phylink represents as
> 2500base-x:
> 
>  - Select the HSGMII SerDes tuning parameters and external interface
>    mode, and mux the SerDes to MAC8 in HSGMII mode, from pcs_config()
>    according to the interface. The parameters are again lifted from the
>    GPL-licensed Realtek rtl8367c vendor driver, and again only cover
>    the tuning variant for a non-zero chip option, so the mode is gated
>    on the option probed at setup.

[...]

> @@ -1264,6 +1279,16 @@ static int rtl8365mb_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
>  	if (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED)
>  		return -EOPNOTSUPP;
>  
> +	if (interface == PHY_INTERFACE_MODE_2500BASEX) {
> +		sds_jam = rtl8365mb_sds_jam_hsgmii;
> +		sds_jam_size = ARRAY_SIZE(rtl8365mb_sds_jam_hsgmii);
> +		mode = RTL8365MB_EXT_PORT_MODE_HSGMII;
> +	} else {
> +		sds_jam = rtl8365mb_sds_jam_sgmii;
> +		sds_jam_size = ARRAY_SIZE(rtl8365mb_sds_jam_sgmii);
> +		mode = RTL8365MB_EXT_PORT_MODE_SGMII;
> +	}
> +

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:

#define RTL8365MB_REG_INGRESSBW_PORT6_RATE_CTRL1	0x00d0
#define  RTL8365MB_INGRESSBW_PORT6_RATE_CTRL1_INGRESSBW_RATE16_MASK GENMASK(2, 0)

#define RTL8365MB_REG_PORT6_EGRESSBW_CTRL1		0x0399
#define  RTL8365MB_PORT6_EGRESSBW_CTRL1_MASK		GENMASK(2, 0)

#define RTL8365MB_REG_LINE_RATE_HSG_H			0x03fa
#define  RTL8365MB_LINE_RATE_HSG_H_MASK			GENMASK(2, 0)

[...]

		/* Allow full 2.5G on HSGMII CPU port: set scheduler
		 * bandwidth limits to max (0x7). Fixed-link init-only;
		 * no runtime SGMII reconfiguration is expected here.
		 */
		ret = regmap_write(priv->map,
				   RTL8365MB_REG_INGRESSBW_PORT6_RATE_CTRL1,
				   FIELD_PREP(RTL8365MB_INGRESSBW_PORT6_RATE_CTRL1_INGRESSBW_RATE16_MASK,
					      7));
		if (ret)
			return ret;

		ret = regmap_write(priv->map,
				   RTL8365MB_REG_PORT6_EGRESSBW_CTRL1,
				   FIELD_PREP(RTL8365MB_PORT6_EGRESSBW_CTRL1_MASK,
					      7));
		if (ret)
			return ret;

		ret = regmap_write(priv->map,
				   RTL8365MB_REG_LINE_RATE_HSG_H,
				   FIELD_PREP(RTL8365MB_LINE_RATE_HSG_H_MASK,
					      7));
		if (ret)
			return ret;

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.


^ permalink raw reply

* Re: [PATCH v9 01/14] firmware: qcom: Add a generic PAS service
From: Konrad Dybcio @ 2026-07-03 15:06 UTC (permalink / raw)
  To: Sumit Garg, andersson, konradybcio
  Cc: linux-arm-msm, devicetree, dri-devel, freedreno, linux-media,
	netdev, linux-wireless, ath12k, linux-remoteproc, robh, krzk+dt,
	conor+dt, robin.clark, sean, akhilpo, lumag, abhinav.kumar,
	jesszhan0024, marijn.suijten, airlied, simona, vikash.garodia,
	bod, mchehab, elder, andrew+netdev, davem, edumazet, kuba, pabeni,
	jjohnson, mathieu.poirier, trilokkumar.soni, mukesh.ojha,
	pavan.kondeti, jorge.ramirez, tonyh, vignesh.viswanathan,
	srinivas.kandagatla, amirreza.zarrabi, jenswi, op-tee, apurupa,
	skare, linux-kernel, Sumit Garg, Harshal Dev
In-Reply-To: <20260702115835.167602-2-sumit.garg@kernel.org>

On 7/2/26 1:58 PM, Sumit Garg wrote:
> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
> 
> Qcom platforms has the legacy of using non-standard SCM calls
> splintered over the various kernel drivers. These SCM calls aren't
> compliant with the standard SMC calling conventions which is a
> prerequisite to enable migration to the FF-A specifications from Arm.
> 
> OP-TEE as an alternative trusted OS to Qualcomm TEE (QTEE) can't
> support these non-standard SCM calls. And even for newer architectures
> using S-EL2 with Hafnium support, QTEE won't be able to support SCM
> calls either with FF-A requirements coming in. And with both OP-TEE
> and QTEE drivers well integrated in the TEE subsystem, it makes further
> sense to reuse the TEE bus client drivers infrastructure.
> 
> The added benefit of TEE bus infrastructure is that there is support
> for discoverable/enumerable services. With that client drivers don't
> have to manually invoke a special SCM call to know the service status.
> 
> So enable the generic Peripheral Authentication Service (PAS) provided
> by the firmware. It acts as the common layer with different TZ
> backends plugged in whether it's an SCM implementation or a proper
> TEE bus based PAS service implementation.
> 
> Reviewed-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> Tested-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> # Lemans
> Reviewed-by: Harshal Dev <harshal.dev@oss.qualcomm.com>
> Tested-by: Vignesh Viswanathan <vignesh.viswanathan@oss.qualcomm.com> # IPQ9650
> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

^ permalink raw reply

* Re: [PATCH net-next 0/4] net: usb: move exported code to usbnet
From: Andrew Lunn @ 2026-07-03 14:59 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, shaoxul, netdev,
	linux-usb, linux-kernel
In-Reply-To: <c3d3c7a0-5d47-4e73-a46b-2fd610f4cd59@suse.com>

On Fri, Jul 03, 2026 at 12:16:20PM +0200, Oliver Neukum wrote:
> On 02.07.26 20:15, Andrew Lunn wrote:
> > On Thu, Jul 02, 2026 at 04:25:29PM +0200, Oliver Neukum wrote:
> > > Some drivers are reusing common code originating in other drivers.
> > > This means that two drivers need to be loaded for one device.
> > 
> > Maybe consider using 'framework' or 'library', rather than driver,
> > when referring to the shared code?
> > 
> > I tend to think of a driver as the leaf node which probes based on
> > enumeration of a bus. But usbnet.c itself is never probed.
> 
> Yes, I also think of a leaf node when a "driver" is referred to.
> That is the very point. Currently you need _two_ drivers and
> usbnet for some devices.
> Should I reformulate? If so, how exactly?

Maybe give examples? List the two drivers used for one device?
cdc_ether.c is being used both as a driver and a library? The
EXPORT_SYMBOL_GPL() make this clear, a driver would normally not need
exports. The patchset starts to make cdc_ether.c a pure driver, moving
some of the shared code into the usbnet library.

Just to be clear, i think what you are doing is correct, i just had
problems understanding the commit message.

     Andrew


^ permalink raw reply


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