Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v8 01/12] lsm: Add LSM hook security_unix_find
From: Paul Moore @ 2026-03-27 17:55 UTC (permalink / raw)
  To: Günther Noack
  Cc: Mickaël Salaün, John Johansen, James Morris,
	Serge E . Hallyn, Tingmao Wang, Justin Suess,
	linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
	Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
	Alyssa Ross, Jann Horn, Tahera Fahimi, Sebastian Andrzej Siewior,
	Kuniyuki Iwashima, Georgia Garcia, Simon Horman, netdev,
	Alexander Viro, Christian Brauner
In-Reply-To: <20260327164838.38231-2-gnoack3000@gmail.com>

On Fri, Mar 27, 2026 at 12:49 PM Günther Noack <gnoack3000@gmail.com> wrote:
>
> From: Justin Suess <utilityemal77@gmail.com>
>
> Add an LSM hook security_unix_find.
>
> This hook is called to check the path of a named UNIX socket before a
> connection is initiated. The peer socket may be inspected as well.
>
> Why existing hooks are unsuitable:
>
> Existing socket hooks, security_unix_stream_connect(),
> security_unix_may_send(), and security_socket_connect() don't provide
> TOCTOU-free / namespace independent access to the paths of sockets.
>
> (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> This requires another path lookup. A change in the path between the
> two lookups will cause a TOCTOU bug.
>
> (2) We cannot use the struct path from the listening socket, because it
> may be bound to a path in a different namespace than the caller,
> resulting in a path that cannot be referenced at policy creation time.
>
> Consumers of the hook wishing to reference @other are responsible
> for acquiring the unix_state_lock and checking for the SOCK_DEAD flag
> therein, ensuring the socket hasn't died since lookup.
>
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Tingmao Wang <m@maowtm.org>
> Cc: Mickaël Salaün <mic@digikod.net>
> Cc: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
>  include/linux/lsm_hook_defs.h |  5 +++++
>  include/linux/security.h      | 11 +++++++++++
>  net/unix/af_unix.c            | 10 +++++++---
>  security/security.c           | 20 ++++++++++++++++++++
>  4 files changed, 43 insertions(+), 3 deletions(-)

This patch doesn't look like it changed significantly in this
revision, is there a reason you dropped the tags from Georgia and I?

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH net] ipv6: icmp: clear skb2->cb[] in ip6_err_gen_icmpv6_unreach()
From: Ido Schimmel @ 2026-03-27 17:54 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	David Ahern, netdev, eric.dumazet, Oskar Kjos
In-Reply-To: <20260326202608.2976021-1-edumazet@google.com>

On Thu, Mar 26, 2026 at 08:26:08PM +0000, Eric Dumazet wrote:
> Sashiko AI-review observed:
> 
>   In ip6_err_gen_icmpv6_unreach(), the skb is an outer IPv4 ICMP error packet
>   where its cb contains an IPv4 inet_skb_parm. When skb is cloned into skb2
>   and passed to icmp6_send(), it uses IP6CB(skb2).
> 
>   IP6CB interprets the IPv4 inet_skb_parm as an inet6_skb_parm. The cipso
>   offset in inet_skb_parm.opt directly overlaps with dsthao in inet6_skb_parm
>   at offset 18.
> 
>   If an attacker sends a forged ICMPv4 error with a CIPSO IP option, dsthao
>   would be a non-zero offset. Inside icmp6_send(), mip6_addr_swap() is called
>   and uses ipv6_find_tlv(skb, opt->dsthao, IPV6_TLV_HAO).
> 
>   This would scan the inner, attacker-controlled IPv6 packet starting at that
>   offset, potentially returning a fake TLV without checking if the remaining
>   packet length can hold the full 18-byte struct ipv6_destopt_hao.
> 
>   Could mip6_addr_swap() then perform a 16-byte swap that extends past the end
>   of the packet data into skb_shared_info?
> 
>   Should the cb array also be cleared in ip6_err_gen_icmpv6_unreach() and
>   ip6ip6_err() to prevent this?
> 
> This patch implements the first suggestion.
> 
> I am not sure if ip6ip6_err() needs to be changed.
> A separate patch would be better anyway.
> 
> Fixes: ca15a078bd90 ("sit: generate icmpv6 error when receiving icmpv4 error")
> Reported-by: Ido Schimmel <idosch@nvidia.com>
> Closes: https://sashiko.dev/#/patchset/20260326155138.2429480-1-edumazet%40google.com
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Ido Schimmel <idosch@nvidia.com>
> Cc: Oskar Kjos <oskar.kjos@hotmail.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

^ permalink raw reply

* Re: [net-next PATCH v2 00/13] net: renesas: rswitch: R-Car S4 add VLAN aware switching
From: Jakub Kicinski @ 2026-03-27 17:36 UTC (permalink / raw)
  To: Michael Dege
  Cc: Yoshihiro Shimoda, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, netdev, linux-renesas-soc, linux-kernel
In-Reply-To: <20260327-rswitch_add_vlans-v2-0-d7f4358ca57a@renesas.com>

On Fri, 27 Mar 2026 09:27:30 +0100 Michael Dege wrote:
> Hello!
> 
> The current R-Car S4 rswitch driver only supports basic HW offloading
> for L2 switching/bridgeing. This patchset extends the driver with
> support for VLAN aware switching.
> 
> 1. For every port mode change from configuration to operation and
>    vice-versa you need to change to disabled mode first. The
>    functions rswitch_<port>_change_mode now take care of this. 
> 
> 2. In upcomming changes for adding vlan support struct net_device
>    will not be available in all cases, therefore use struct device
>    instead.
> 
> 3. Fix typo in register define macro and remove duplicate macro.
> 
> 4. Add register definitions needed fo vlan support.
> 
> 5. Add exception path for packets with unknown destitination MAC
>    addresses.
> 
> 6. Make the helper functions rswitch_reg_wait(),
>    rswitch_etha_change_mode() and rswitch_etha_change_mode()
>    available to the whole driver.
> 
> 7. Add basic start-up time initialization needed to support VLANs.
> 
> 8. Update ETHA and GWCA port HW initializations.
> 
> 9. Clean up is_rdev() rswitch_device checking.
> 
> 10. Provide struct rswitch_private to notifiers.
> 
> 11. Add handler for FDB notifications to configure bridge MAC address
>     to GWCA registers and update static MAC table entry.
> 
> 12. Add vlan support to L2 HW bridge.

coccicheck reports:

drivers/net/ethernet/renesas/rswitch_main.c:1590:11-66: duplicated argument to & or |
@@ -182,0 +184,3 @@
drivers/net/ethernet/renesas/rswitch_l2.c:350:5-8: WARNING: Unsigned expression compared with zero: err < 0
drivers/net/ethernet/renesas/rswitch_l2.c:380:5-8: WARNING: Unsigned expression compared with zero: err < 0
drivers/net/ethernet/renesas/rswitch_l2.c:465:5-8: WARNING: Unsigned expression compared with zero: err < 0

reminder: please do not post a new version of patches on netdev earlier
than a full 24h after previous posting.

^ permalink raw reply

* RE: [Intel-wired-lan] [PATCH v4 net-next 6/8] ice: implement CPI support for E825C
From: Loktionov, Aleksandr @ 2026-03-27 17:12 UTC (permalink / raw)
  To: Nitka, Grzegorz, netdev@vger.kernel.org
  Cc: Vecera, Ivan, vadim.fedorenko@linux.dev, kuba@kernel.org,
	jiri@resnulli.us, edumazet@google.com, Kitszel, Przemyslaw,
	richardcochran@gmail.com, donald.hunter@gmail.com,
	linux-kernel@vger.kernel.org, Kubalewski, Arkadiusz,
	andrew+netdev@lunn.ch, intel-wired-lan@lists.osuosl.org,
	horms@kernel.org, Prathosh.Satish@microchip.com,
	Nguyen, Anthony L, pabeni@redhat.com, davem@davemloft.net
In-Reply-To: <20260326162832.3135857-7-grzegorz.nitka@intel.com>



> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Grzegorz Nitka
> Sent: Thursday, March 26, 2026 5:29 PM
> To: netdev@vger.kernel.org
> Cc: Vecera, Ivan <ivecera@redhat.com>; vadim.fedorenko@linux.dev;
> kuba@kernel.org; jiri@resnulli.us; edumazet@google.com; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; richardcochran@gmail.com;
> donald.hunter@gmail.com; linux-kernel@vger.kernel.org; Kubalewski,
> Arkadiusz <arkadiusz.kubalewski@intel.com>; andrew+netdev@lunn.ch;
> intel-wired-lan@lists.osuosl.org; horms@kernel.org;
> Prathosh.Satish@microchip.com; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; pabeni@redhat.com; davem@davemloft.net
> Subject: [Intel-wired-lan] [PATCH v4 net-next 6/8] ice: implement
> CPI support for E825C
> 
> Add full CPI (Converged PHY Interface) command handling required for
> E825C devices. The CPI interface allows the driver to interact with
> PHY-side control logic through the LM/PHY command registers,
> including enabling/disabling/selection of PHY reference clock.
> 
> This patch introduces:
>  - a new CPI subsystem (ice_cpi.c / ice_cpi.h) implementing the CPI
>    request/acknowledge state machine, including REQ/ACK protocol,
>    command execution, and response handling
>  - helper functions for reading/writing PHY registers over Sideband
>    Queue
>  - CPI command execution API (ice_cpi_exec) and a helper for
> enabling or
>    disabling Tx reference clocks (CPI 0xF1 opcode 'Config PHY
> clocking')
>  - addition of the non-posted write opcode (wr_np) to SBQ
>  - Makefile integration to build CPI support together with the PTP
> stack
> 
> This provides the infrastructure necessary to support PHY-side
> configuration flows on E825C and is required for advanced link
> control and Tx reference clock management.
> 
> Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Signed-off-by: Grzegorz Nitka <grzegorz.nitka@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/Makefile      |   2 +-
>  drivers/net/ethernet/intel/ice/ice_cpi.c     | 347
> +++++++++++++++++++
>  drivers/net/ethernet/intel/ice/ice_cpi.h     |  69 ++++
>  drivers/net/ethernet/intel/ice/ice_sbq_cmd.h |   5 +-
>  4 files changed, 420 insertions(+), 3 deletions(-)  create mode
> 100644 drivers/net/ethernet/intel/ice/ice_cpi.c
>  create mode 100644 drivers/net/ethernet/intel/ice/ice_cpi.h
> 
> diff --git a/drivers/net/ethernet/intel/ice/Makefile
> b/drivers/net/ethernet/intel/ice/Makefile
> index 5b2c666496e7..38db476ab2ec 100644
> --- a/drivers/net/ethernet/intel/ice/Makefile
> +++ b/drivers/net/ethernet/intel/ice/Makefile
> @@ -54,7 +54,7 @@ ice-$(CONFIG_PCI_IOV) +=	\
>  	ice_vf_mbx.o		\
>  	ice_vf_vsi_vlan_ops.o	\
>  	ice_vf_lib.o

...

> +
> +union cpi_reg_phy_cmd_data {
> +	struct {
> +		u16 data;
> +		u16 opcode : 8;
> +		u16 portlane : 3;
> +		u16 reserved_13_11: 3;
> +		u16 error : 1;
> +		u16 ack : 1;
> +	} __packed field;
> +	u32 val;
> +};
Why not use FIELD_GET()/FIELD_PREP() with GENMASK()? Bitfields are compiled differently on LE nd BE CPUs.

> +
> +union cpi_reg_lm_cmd_data {
> +	struct {
> +		u16 data;
> +		u16 opcode : 8;
> +		u16 portlane : 3;
> +		u16 reserved_12_11: 2;
> +		u16 get_set : 1;
> +		u16 cpi_reset : 1;
> +		u16 cpi_req : 1;
> +	} __packed field;
> +	u32 val;
> +};
Why not use FIELD_GET()/FIELD_PREP() with GENMASK()? Bitfields are compiled differently on LE nd BE CPUs.


...

> 
>  #define ICE_SBQ_MSG_FLAGS	0x40
> --
> 2.39.3


^ permalink raw reply

* [PATCH RFC net-next] net: stmmac: qcom-ethqos: set clk_csr
From: Russell King (Oracle) @ 2026-03-27 17:02 UTC (permalink / raw)
  To: Mohd Ayaan Anwar
  Cc: Alexandre Torgue, Andrew Lunn, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, linux-arm-kernel, linux-arm-msm,
	linux-stm32, netdev, Paolo Abeni

The clocks for qcom-ethqos return a rate of zero as firmware manages
their rate. According to hardware documentation, the clock which is
fed to the slave AHB interface can crange between 50 and 100MHz.

Currently, stmmac uses an undefined divisor value. Instead, use
STMMAC_CSR_60_100M which will mean we meet IEEE 802.3 specification
since this will generate between a 1.19MHz and 2.38MHz MDC clock for
this range. Add a comment describing this.

Link: https://lore.kernel.org/r/acGhQ0oui+dVRdLY@oss.qualcomm.com
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---

This likely needs the qcom-ethqos 15 patch cleanup series.

I think this is what's needed to fix the MDC clocking issue. Please
review and test. Thanks.

 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index ad3a983d2a08..ac7d6d3e205a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -764,6 +764,12 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
 	qcom_ethqos_set_sgmii_loopback(ethqos, true);
 	ethqos_set_func_clk_en(ethqos);
 
+	/* The clocks are controlled by firmware, so we don't know for certain
+	 * what clock rate is being used. Hardware documentation mentions that
+	 * the AHB slave clock will be in the range of 50 to 100MHz, which
+	 * equates to a MDC between 1.19 and 2.38MHz.
+	 */
+	plat_dat->clk_csr = STMMAC_CSR_60_100M;
 	plat_dat->bsp_priv = ethqos;
 	plat_dat->set_clk_tx_rate = ethqos_set_clk_tx_rate;
 	plat_dat->dump_debug_regs = rgmii_dump;
-- 
2.47.3


^ permalink raw reply related

* Re: [PATCH net v2 1/3] bonding: 3ad: fix carrier when no valid slaves
From: Jay Vosburgh @ 2026-03-27 16:56 UTC (permalink / raw)
  To: Louis Scalbert
  Cc: netdev, andrew+netdev, edumazet, kuba, pabeni, fbl, andy,
	shemminger
In-Reply-To: <CAJ5u_OcpOGSPYK-EzYeW8LdpJrLd16fGn6zN6cvk+tCjCi+QxA@mail.gmail.com>

Louis Scalbert <louis.scalbert@6wind.com> wrote:

>Hi everyone,
>
>Le mer. 25 mars 2026 à 14:44, Louis Scalbert
><louis.scalbert@6wind.com> a écrit :
>>
>> When an 802.3ad (LACP) bonding interface has no slaves in the
>> collecting/distributing state, the bonding master may still report
>> carrier as up. In this situation, no slave is actually able to transmit
>> or receive traffic.
>>
>I’ve just realized that the issue description is not entirely accurate.
>
>Bonding links that are not in LACP Collecting/Distributing state are
>excluded from traffic
>distribution in favor of those that are. However, if no links are in
>Collecting/Distributing state,
>the kernel considers all links as valid for distributing traffic.
>
>> As a result, upper-layer daemons consider the interface operational
>> while traffic is effectively blackholed.
>
>There are situations where traffic is not blackholed.
>
>For example, a Linux machine A is connected to machine B using two
>aggregated links.
>On A, LACP is configured, but not on B. As a result, LACP will not
>reach the Collecting/Distributing
>state since B does not send LACP frames. However, the bonding
>interface will still be up on both
>sides, and traffic can be exchanged.

	Bonding does this because, in the absense of a LACP partner, the
interfaces in the bond will become Individual links, each of which will
become its own aggregator, one of which will be chosen to be active.

	This is in compliance with the standard, (802.1AX-2014, 6.1.1.j)
and is intended to permit some communication between a system that is
LACP-enabled and a system that is not.  The common example I'm familiar
with is a host that PXE boots over a network, connected to a
LACP-enabled switch.  During early boot, the PXE environment does not
perform LACP, but still needs to communicate with the switch.

>This behavior is, of course, not compliant with the LACP (802.3ad)
>standard, but applying this fix
>as-is could introduce regressions in some setups. Under normal
>conditions, having no Collecting /
>Distributing links means that no interfaces are operational for
>handling traffic - that is precisely the
>purpose of using LACP.

	FWIW, generally speaking we've settled on IEEE 802.1AX-2014 as
the version of the standard to which the bonding implementation should
be conformant.  There is a more recent version, -2020, but it is a
significant change from the prior versions.

>I will publish a new version with a knob to enforce compliance with
>the LACP standard. It will be
>disabled by default for backward compatibility.
>
>What do you think of "lacp_strict" or "lacp_enforce" ?

	I'm not convinced that we are in violation of the standard with
the present behavior.  That said, if such a switch is necessary, I'd
vote for "lacp_strict".

	-J

>>
>> Fix this by asserting carrier only when at least 'min_links' slaves are
>> in the collecting/distributing state (or collecting only if the
>> coupled_control default behavior is disabled).
>>
>> Fixes: 655f8919d549 ("bonding: add min links parameter to 802.3ad")
>> Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
>> ---
>>  drivers/net/bonding/bond_3ad.c | 24 ++++++++++++++++++++++--
>>  1 file changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index af7f74cfdc08..6d3613755d45 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -745,6 +745,22 @@ static void __set_agg_ports_ready(struct aggregator *aggregator, int val)
>>         }
>>  }
>>
>> +static int __agg_valid_ports(struct aggregator *agg)
>> +{
>> +       struct port *port;
>> +       int valid = 0;
>> +
>> +       for (port = agg->lag_ports; port;
>> +            port = port->next_port_in_aggregator) {
>> +               if (port->actor_oper_port_state & LACP_STATE_COLLECTING &&
>> +                   (!port->slave->bond->params.coupled_control ||
>> +                    port->actor_oper_port_state & LACP_STATE_DISTRIBUTING))
>> +                       valid++;
>> +       }
>> +
>> +       return valid;
>> +}
>> +
>>  static int __agg_active_ports(struct aggregator *agg)
>>  {
>>         struct port *port;
>> @@ -2120,6 +2136,7 @@ static void ad_enable_collecting_distributing(struct port *port,
>>                           port->actor_port_number,
>>                           port->aggregator->aggregator_identifier);
>>                 __enable_port(port);
>> +               bond_3ad_set_carrier(port->slave->bond);
>>                 /* Slave array needs update */
>>                 *update_slave_arr = true;
>>                 /* Should notify peers if possible */
>> @@ -2141,6 +2158,7 @@ static void ad_disable_collecting_distributing(struct port *port,
>>                           port->actor_port_number,
>>                           port->aggregator->aggregator_identifier);
>>                 __disable_port(port);
>> +               bond_3ad_set_carrier(port->slave->bond);
>>                 /* Slave array needs an update */
>>                 *update_slave_arr = true;
>>         }
>> @@ -2819,8 +2837,10 @@ int bond_3ad_set_carrier(struct bonding *bond)
>>         }
>>         active = __get_active_agg(&(SLAVE_AD_INFO(first_slave)->aggregator));
>>         if (active) {
>> -               /* are enough slaves available to consider link up? */
>> -               if (__agg_active_ports(active) < bond->params.min_links) {
>> +               /* are enough slaves in collecting (and distributing) state to consider
>> +                * link up?
>> +                */
>> +               if (__agg_valid_ports(active) < bond->params.min_links) {
>>                         if (netif_carrier_ok(bond->dev)) {
>>                                 netif_carrier_off(bond->dev);
>>                                 goto out;
>> --
>> 2.39.2
>>

---
	-Jay Vosburgh, jv@jvosburgh.net

^ permalink raw reply

* [PATCH v8 01/12] lsm: Add LSM hook security_unix_find
From: Günther Noack @ 2026-03-27 16:48 UTC (permalink / raw)
  To: Mickaël Salaün, John Johansen, Paul Moore, James Morris,
	Serge E . Hallyn
  Cc: Günther Noack, Tingmao Wang, Justin Suess,
	linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
	Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
	Alyssa Ross, Jann Horn, Tahera Fahimi, Sebastian Andrzej Siewior,
	Kuniyuki Iwashima, Georgia Garcia, Simon Horman, netdev,
	Alexander Viro, Christian Brauner
In-Reply-To: <20260327164838.38231-1-gnoack3000@gmail.com>

From: Justin Suess <utilityemal77@gmail.com>

Add an LSM hook security_unix_find.

This hook is called to check the path of a named UNIX socket before a
connection is initiated. The peer socket may be inspected as well.

Why existing hooks are unsuitable:

Existing socket hooks, security_unix_stream_connect(),
security_unix_may_send(), and security_socket_connect() don't provide
TOCTOU-free / namespace independent access to the paths of sockets.

(1) We cannot resolve the path from the struct sockaddr in existing hooks.
This requires another path lookup. A change in the path between the
two lookups will cause a TOCTOU bug.

(2) We cannot use the struct path from the listening socket, because it
may be bound to a path in a different namespace than the caller,
resulting in a path that cannot be referenced at policy creation time.

Consumers of the hook wishing to reference @other are responsible
for acquiring the unix_state_lock and checking for the SOCK_DEAD flag
therein, ensuring the socket hasn't died since lookup.

Cc: Günther Noack <gnoack3000@gmail.com>
Cc: Tingmao Wang <m@maowtm.org>
Cc: Mickaël Salaün <mic@digikod.net>
Cc: Paul Moore <paul@paul-moore.com>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
Signed-off-by: Günther Noack <gnoack3000@gmail.com>
---
 include/linux/lsm_hook_defs.h |  5 +++++
 include/linux/security.h      | 11 +++++++++++
 net/unix/af_unix.c            | 10 +++++++---
 security/security.c           | 20 ++++++++++++++++++++
 4 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 8c42b4bde09c..7a0fd3dbfa29 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -317,6 +317,11 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
 LSM_HOOK(int, 0, watch_key, struct key *key)
 #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
 
+#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
+LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
+	 int flags)
+#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
+
 #ifdef CONFIG_SECURITY_NETWORK
 LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
 	 struct sock *newsk)
diff --git a/include/linux/security.h b/include/linux/security.h
index 83a646d72f6f..99a33d8eb28d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
 }
 #endif	/* CONFIG_SECURITY_NETWORK */
 
+#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
+
+int security_unix_find(const struct path *path, struct sock *other, int flags);
+
+#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
+static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
+{
+	return 0;
+}
+#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
+
 #ifdef CONFIG_SECURITY_INFINIBAND
 int security_ib_pkey_access(void *sec, u64 subnet_prefix, u16 pkey);
 int security_ib_endport_manage_subnet(void *sec, const char *name, u8 port_num);
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 3756a93dc63a..5ef3c2e31757 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1231,11 +1231,15 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
 		goto path_put;
 
 	err = -EPROTOTYPE;
-	if (sk->sk_type == type)
-		touch_atime(&path);
-	else
+	if (sk->sk_type != type)
 		goto sock_put;
 
+	err = security_unix_find(&path, sk, flags);
+	if (err)
+		goto sock_put;
+
+	touch_atime(&path);
+
 	path_put(&path);
 
 	return sk;
diff --git a/security/security.c b/security/security.c
index 67af9228c4e9..28ccea205874 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
 
 #endif	/* CONFIG_SECURITY_NETWORK */
 
+#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
+/**
+ * security_unix_find() - Check if a named AF_UNIX socket can connect
+ * @path: path of the socket being connected to
+ * @other: peer sock
+ * @flags: flags associated with the socket
+ *
+ * This hook is called to check permissions before connecting to a named
+ * AF_UNIX socket. The caller does not hold any locks on @other.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_unix_find(const struct path *path, struct sock *other, int flags)
+{
+	return call_int_hook(unix_find, path, other, flags);
+}
+EXPORT_SYMBOL(security_unix_find);
+
+#endif	/* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
+
 #ifdef CONFIG_SECURITY_INFINIBAND
 /**
  * security_ib_pkey_access() - Check if access to an IB pkey is allowed
-- 
2.53.0


^ permalink raw reply related

* [PATCH v8 00/12] landlock: UNIX connect() control by pathname and scope
From: Günther Noack @ 2026-03-27 16:48 UTC (permalink / raw)
  To: Mickaël Salaün, John Johansen, Paul Moore, James Morris,
	Serge E . Hallyn
  Cc: Günther Noack, linux-security-module, Tingmao Wang,
	Justin Suess, Samasth Norway Ananda, Matthieu Buffet,
	Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
	Alyssa Ross, Jann Horn, Tahera Fahimi, Sebastian Andrzej Siewior,
	Kuniyuki Iwashima, Georgia Garcia, Simon Horman, netdev,
	Alexander Viro, Christian Brauner

Hello!

This patch set introduces a filesystem-based Landlock restriction
mechanism for connecting to UNIX domain sockets (or addressing them
with sendmsg(2)).  It introduces the filesystem access right
LANDLOCK_ACCESS_FS_RESOLVE_UNIX.

For the connection-oriented SOCK_STREAM and SOCK_SEQPACKET type
sockets, the access right makes the connect(2) operation fail with
EACCES, if denied.

SOCK_DGRAM-type UNIX sockets can be used both with connect(2), or by
passing an explicit recipient address with every sendmsg(2)
invocation.  In the latter case, the Landlock check is done when an
explicit recipient address is passed to sendmsg(2) and can make
sendmsg(2) return EACCES.  When UNIX datagram sockets are connected
with connect(2), a fixed recipient address is associated with the
socket and the check happens during connect(2) and may return EACCES.

When LANDLOCK_ACCESS_FS_RESOLVE_UNIX is handled within a Landlock
domain, this domain will only allow connect(2) and sendmsg(2) to
server sockets that were created within the same domain.  Or, to
phrase it the other way around: Unless it is allow-listed with a
LANDLOCK_PATH_BENEATH rule, the newly created domain denies connect(2)
and sendmsg(2) actions that are directed *outwards* of that domain.
In that regard, LANDLOCK_ACCESS_FS_RESOLVE_UNIX has the same semantics
as one of the "scoped" access rights.

== Motivation

Currently, landlocked processes can connect to named UNIX sockets
through the BSD socket API described in unix(7), by invoking socket(2)
followed by connect(2) with a suitable struct sockname_un holding the
socket's filename.  This is a surprising gap in Landlock's sandboxing
capabilities for users (e.g. in [1]) and it can be used to escape a
sandbox when a Unix service offers command execution (various such
scenarios were listed by Tingmao Wang in [2]).

The original feature request is at [4].

== Alternatives and Related Work

=== Alternative: Use existing LSM hooks

We have carefully and seriously considered the use of existing LSM
hooks, but still came to the conclusion that a new LSM hook is better
suited in this case:

The existing hooks security_unix_stream_connect(),
security_unix_may_send() and security_socket_connect() do not give
access to the resolved filesystem path.

* Resolving the filesystem path in the struct sockaddr_un again within
  a Landlock would produce a TOCTOU race, so this is not an option.
* We would therefore need to wire through the resolved struct path
  from unix_find_bsd() to one of the existing LSM hooks which get
  called later.  This would be a more substantial change to af_unix.c.

The struct path that is available in the listening-side struct sock is
can be read through the existing hooks, but it is not an option to use
this information: As the listening socket may have been bound from
within a different namespace, the path that was used for that can is
in the general case not meaningful for a sandboxed process.  In
particular, it is not possible to use this path (or prefixes thereof)
when constructing a sandbox policy in the client-side process.

Paul Moore also chimed in in support of adding a new hook, with the
rationale that the simplest change to the LSM hook interface has
traditionally proven to be the most robust. [11]

More details are on the Github issue at [6] and on the LKML at [9].

In a the discussion of the V2 review, started by Christian Brauner
[10], we have further explored the approach of reusing the existing
LSM hooks but still ended up leaning on the side of introducing a new
hook, with Paul Moore and me (gnoack) arguing for that option.

Further insights about the LSM hook were shared in the V3 review by
Tingmao Wang [12], who spotted additional requirements due to the two
approaches being merged into one patch set.  The summary of that
discussion is in [13].

=== Related work: Scope Control for Pathname Unix Sockets

The motivation for this patch is the same as in Tingmao Wang's patch
set for "scoped" control for pathname Unix sockets [2], originally
proposed in the Github feature request [5].

In [14], we have settled on the decision to merge the two patch sets
into this one, whose primary way of controlling connect(2) is
LANDLOCK_ACCESS_FS_RESOLVE_UNIX, but where this flag additionally has
the semantics of only restricting this unix(7) IPC *outwards* of the
created Landlock domain, in line with the logic that exists for the
existing "scoped" flags already.

By having LANDLOCK_ACCESS_FS_RESOLVE_UNIX implement "scoping"
semantics, we can avoid introducing two separate interacting flags for
now, but we retain the option of introducing
LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET at a later point in time, should
such a flag be needed to express additional rules.

== Credits

The feature was originally suggested by Jann Horn in [7].

Tingmao Wang and Demi Marie Obenour have taken the initiative to
revive this discussion again in [1], [4] and [5].

Tingmao Wang has sent the patch set for the scoped access control for
pathname Unix sockets [2] and has contributed substantial insights
during the code review, shaping the form of the LSM hook and agreeing
to merge the pathname and scoped-flag patch sets.

Justin Suess has sent the patch for the LSM hook in [8] and
subsequently through this patch set.

Christian Brauner and Paul Moore have contributed to the design of the
new LSM hook, discussing the tradeoffs in [10].

Sebastian Andrzej Siewior and Kuniyuki Iwashima have helped with
locking questions in the networking subsystem. [15] [16]

Ryan Sullivan has started on an initial implementation and has brought
up relevant discussion points on the Github issue at [4].

As maintainer of Landlock, Mickaël Salaün has done the main review so
far and particularly pointed out ways in which the UNIX connect()
patch sets interact with each other and what we need to look for with
regards to UAPI consistency as Landlock evolves.

[1] https://lore.kernel.org/landlock/515ff0f4-2ab3-46de-8d1e-5c66a93c6ede@gmail.com/
[2] Tingmao Wang's "Implement scope control for pathname Unix sockets"
    https://lore.kernel.org/all/cover.1767115163.git.m@maowtm.org/
[3] https://lore.kernel.org/all/20251230.bcae69888454@gnoack.org/
[4] Github issue for FS-based control for named Unix sockets:
    https://github.com/landlock-lsm/linux/issues/36
[5] Github issue for scope-based restriction of named Unix sockets:
    https://github.com/landlock-lsm/linux/issues/51
[6] https://github.com/landlock-lsm/linux/issues/36#issuecomment-2950632277
[7] https://lore.kernel.org/linux-security-module/CAG48ez3NvVnonOqKH4oRwRqbSOLO0p9djBqgvxVwn6gtGQBPcw@mail.gmail.com/
[8] Patch for the LSM hook:
    https://lore.kernel.org/all/20251231213314.2979118-1-utilityemal77@gmail.com/
[9] https://lore.kernel.org/all/20260108.64bd7391e1ae@gnoack.org/
[10] https://lore.kernel.org/all/20260113-kerngesund-etage-86de4a21da24@brauner/
[11] https://lore.kernel.org/all/CAHC9VhQHZCe0LMx4xzSo-h1SWY489U4frKYnxu4YVrcJN3x7nA@mail.gmail.com/
[12] https://lore.kernel.org/all/e6b6b069-384c-4c45-a56b-fa54b26bc72a@maowtm.org/
[13] https://lore.kernel.org/all/aYMenaSmBkAsFowd@google.com/
[14] https://lore.kernel.org/all/20260205.Kiech3gupee1@digikod.net/
[15] https://lore.kernel.org/all/20260310151907.VYySCtJp@linutronix.de/
[16] https://lore.kernel.org/all/CAAVpQUC95mSjX1vRK===pubHofcYqbkNE7goYKiu6vha5GYAFw@mail.gmail.com/

---

== Patch set history

V1: https://lore.kernel.org/all/20260101134102.25938-1-gnoack3000@gmail.com/
V2: https://lore.kernel.org/all/20260110143300.71048-2-gnoack3000@gmail.com/
V3: https://lore.kernel.org/all/20260119203457.97676-2-gnoack3000@gmail.com/
V4: https://lore.kernel.org/all/20260208231017.114343-1-gnoack3000@gmail.com/
V5: https://lore.kernel.org/all/20260215105158.28132-1-gnoack3000@gmail.com/
V6: https://lore.kernel.org/all/20260315222150.121952-1-gnoack3000@gmail.com/
V7: https://lore.kernel.org/all/20260323165654.193957-1-gnoack3000@gmail.com/

Changes in V8:

* Added a fix for the m68k GCC 15 compiler failure
* Reorder BUILD_BUG_ON commit to be directly after the implementation commit
* Documentation: Use the current month (March 2026) in uapi doc comment

Changes in V7:

* Implementation:
  * LSM hook: Small header file layout restructurings, typos (Justin)
  * Hold unix_state_lock across the usage of the other Landlock domain
    This prevents a UAF; spotted by Mickaël and Sebastian in code review 🏆
  * Ignore the return value from landlock_init_layer_masks() -
    it is not needed in this specific case
  * Add a BUILD_BUG_ON to unmask_scoped_access() like in
    domain_is_scoped().  Revise the BUILD_BUG_ON logic and bring both
    implementations in line.
* Documentation and commentary:
  * Mention access_mask_t change from u16 to u32 in commit message
  * Improve documentation for unmask_scoped_access()
  * Various typos and smaller documentation fixes, caught in code review
  * Add ABI version remark to landlock.h
  * Add comment to explain why returning 0 on SOCK_DEAD is correct -
    This is not obvious in the code and requires understanding the callers
  * In kernel docs, use "case 6 ... 8" in a place

Changes in V6:

* Implementation:
  * Move the LSM hook call after the check that checks for the other
    end's matching socket type.  (Justin Suess)
  * Lock with unix_state_lock() and check SOCK_DEAD.
  * Remove unnecessary layer_access_masks_empty() call (and its
    implementation).
* Documentation:
  * Squash docs with design rationale into main implementation commit,
    and cross-referece it from the header docs.
  * Clarify that denials result in EACCES and that this is consistent
    with other filesystem access rights.
* Minor:
  * Use mem_is_zero() in is_layer_masks_allowed() (minor cleanup)
  * Omit unnecessary __attribute__((fallthrough)) usages
  * Remove comment at the end of a line in a place.
* Selftests:
  * sun_path population fixes
  * coredump test: Set EUID to 0 (needed for UML-based selftests)
    Link[1]: https://lore.kernel.org/all/20260218.ohth8theu8Yi@digikod.net/

Changes in V5:

This change primarily adds tests, changing the testing approach for
the main test to use the scoped_domains fixture as in Tingmao's patch
set [2], and adding tests for the audit and coredump cases.

* Selftests:
  * Replaced the main selftest with one based on scoped_domains
  * Added audit test
  * Added test for the coredump case
  * Added a follow-up commit that simplifies ruleset enforcement
* Kernel code:
  * Mark coredump check as unlikely (per Justin's review)
  * Drop check for socket type (per Mickaël's review)

Changes in V4:

Since this version, this patch set subsumes the scoping semantics from
Tingmao Wang's "Scope Control" patch set [2], per discussion with
Tingmao Wang and Mickaël Salaün in [14] and in the thread leading up
to it.

Now, LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET only restricts connect(2) and
sendmsg(2) *outwards* of the domain where it is restricted, *with the
same semantics as a "scoped" flag*.

 * Implement a layer-mask based version of domain_is_scoped():
   unmask_scoped_access().  Rationale: domain_is_scoped() returns
   early, which we can't do in the layer masks based variant.  The two
   variants are similar enough.
 * LSM hook: Replace 'type' argument with 'sk' argument,
   per discussion in [12] and [13].
 * Bump ABI version to 9 (pessimistically assuming that we won't make
   it for 7.0)
 * Documentation fixes in header file and in Documentation/
 * selftests: more test variants, now also parameterizing whether the
   server socket gets created within the Landlock domain or before that
 * selftests: use EXPECT_EQ() for test cleanup

Changes in V3:
 * LSM hook: rename it to security_unix_find() (Justin Suess)
   (resolving the previously open question about the LSM hook name)
   Related discussions:
   https://lore.kernel.org/all/20260112.Wufar9coosoo@digikod.net/
   https://lore.kernel.org/all/CAHC9VhSRiHwLEWfFkQdPEwgB4AXKbXzw_+3u=9hPpvUTnu02Bg@mail.gmail.com/
 * Reunite the three UNIX resolving access rights back into one
   (resolving the previously open question about the access right
   structuring) Related discussion:
   https://lore.kernel.org/all/20260112.Wufar9coosoo@digikod.net/)
 * Sample tool: Add new UNIX lookup access rights to ACCESS_FILE

Changes in V2:
 * Send Justin Suess's LSM hook patch together with the Landlock
   implementation
 * LSM hook: Pass type and flags parameters to the hook, to make the
   access right more generally usable across LSMs, per suggestion from
   Paul Moore (Implemented by Justin)
 * Split the access right into the three types of UNIX domain sockets:
   SOCK_STREAM, SOCK_DGRAM and SOCK_SEQPACKET.
 * selftests: More exhaustive tests.
 * Removed a minor commit from V1 which adds a missing close(fd) to a
   test (it is already in the mic-next branch)


Günther Noack (11):
  landlock: Use mem_is_zero() in is_layer_masks_allowed()
  landlock: Replace union access_masks_all with helper functions
  landlock: Control pathname UNIX domain socket resolution by path
  landlock: Clarify BUILD_BUG_ON check in scoping logic
  samples/landlock: Add support for named UNIX domain socket
    restrictions
  selftests/landlock: Replace access_fs_16 with ACCESS_ALL in fs_test
  selftests/landlock: Test LANDLOCK_ACCESS_FS_RESOLVE_UNIX
  selftests/landlock: Audit test for LANDLOCK_ACCESS_FS_RESOLVE_UNIX
  selftests/landlock: Check that coredump sockets stay unrestricted
  selftests/landlock: fs_test: Simplify ruleset creation and enforcement
  landlock: Document FS access right for pathname UNIX sockets

Justin Suess (1):
  lsm: Add LSM hook security_unix_find

 Documentation/security/landlock.rst          |   42 +-
 Documentation/userspace-api/landlock.rst     |   16 +-
 include/linux/lsm_hook_defs.h                |    5 +
 include/linux/security.h                     |   11 +
 include/uapi/linux/landlock.h                |   21 +
 net/unix/af_unix.c                           |   10 +-
 samples/landlock/sandboxer.c                 |   12 +-
 security/landlock/access.h                   |   23 +-
 security/landlock/audit.c                    |    1 +
 security/landlock/cred.h                     |   10 +-
 security/landlock/fs.c                       |  135 +-
 security/landlock/limits.h                   |    2 +-
 security/landlock/ruleset.h                  |   13 +-
 security/landlock/syscalls.c                 |    2 +-
 security/landlock/task.c                     |    9 +-
 security/security.c                          |   20 +
 tools/testing/selftests/landlock/base_test.c |    2 +-
 tools/testing/selftests/landlock/fs_test.c   | 1345 ++++++++++--------
 18 files changed, 1025 insertions(+), 654 deletions(-)

Range-diff against v7:
 -:  ------------ >  1:  dec515e7d7bf lsm: Add LSM hook security_unix_find
 -:  ------------ >  2:  aaa334660a52 landlock: Use mem_is_zero() in is_layer_masks_allowed()
 -:  ------------ >  3:  79dd5036482d landlock: Replace union access_masks_all with helper functions
 1:  4d455134c5d9 !  4:  b39e336967de landlock: Control pathname UNIX domain socket resolution by path
    @@ Commit message
         Signed-off-by: Günther Noack <gnoack3000@gmail.com>
     
      ## Documentation/security/landlock.rst ##
    +@@ Documentation/security/landlock.rst: Landlock LSM: kernel documentation
    + ==================================
    + 
    + :Author: Mickaël Salaün
    +-:Date: September 2025
    ++:Date: March 2026
    + 
    + Landlock's goal is to create scoped access-control (i.e. sandboxing).  To
    + harden a whole system, this feature should be available to any process,
     @@ Documentation/security/landlock.rst: this is required to keep access controls consistent over the whole system, and
      this avoids unattended bypasses through file descriptor passing (i.e. confused
      deputy attack).
    @@ Documentation/security/landlock.rst: this is required to keep access controls co
      =====
      
     
    + ## Documentation/userspace-api/landlock.rst ##
    +@@ Documentation/userspace-api/landlock.rst: Landlock: unprivileged access control
    + =====================================
    + 
    + :Author: Mickaël Salaün
    +-:Date: January 2026
    ++:Date: March 2026
    + 
    + The goal of Landlock is to enable restriction of ambient rights (e.g. global
    + filesystem or network access) for a set of processes.  Because Landlock
    +
      ## include/uapi/linux/landlock.h ##
     @@ include/uapi/linux/landlock.h: struct landlock_net_port_attr {
       *
 3:  00c90045f470 =  5:  3c8678a4a482 landlock: Clarify BUILD_BUG_ON check in scoping logic
 2:  70354dfcefcb =  6:  d0c8731fb751 samples/landlock: Add support for named UNIX domain socket restrictions
 4:  b8e6477b2f9a =  7:  49fa86251f1b selftests/landlock: Replace access_fs_16 with ACCESS_ALL in fs_test
 5:  66554c5b5b9c =  8:  69b368c23083 selftests/landlock: Test LANDLOCK_ACCESS_FS_RESOLVE_UNIX
 6:  fab3d3d71215 =  9:  7306a1da2947 selftests/landlock: Audit test for LANDLOCK_ACCESS_FS_RESOLVE_UNIX
 7:  4e8a07344278 = 10:  e63e98c44e58 selftests/landlock: Check that coredump sockets stay unrestricted
 8:  fcaef2882dbd = 11:  497bcce69260 selftests/landlock: fs_test: Simplify ruleset creation and enforcement
 9:  362a0e8f84a0 = 12:  c4716fc8e131 landlock: Document FS access right for pathname UNIX sockets
-- 
2.53.0


^ permalink raw reply

* Re: [PATCH net v3] xfrm: clear trailing padding in build_polexpire()
From: Breno Leitao @ 2026-03-27 16:46 UTC (permalink / raw)
  To: Yasuaki Torimaru
  Cc: netdev, steffen.klassert, herbert, davem, edumazet, kuba, pabeni,
	horms, stable
In-Reply-To: <20260326055801.897013-1-yasuakitorimaru@gmail.com>

On Thu, Mar 26, 2026 at 02:58:00PM +0900, Yasuaki Torimaru wrote:
> build_expire() clears the trailing padding bytes of struct
> xfrm_user_expire after setting the hard field via memset_after(),
> but the analogous function build_polexpire() does not do this for
> struct xfrm_user_polexpire.
> 
> The padding bytes after the __u8 hard field are left
> uninitialized from the heap allocation, and are then sent to
> userspace via netlink multicast to XFRMNLGRP_EXPIRE listeners,
> leaking kernel heap memory contents.
> 
> Add the missing memset_after() call, matching build_expire().
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yasuaki Torimaru <yasuakitorimaru@gmail.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

^ permalink raw reply

* Re: [PATCH net v2 1/3] bonding: 3ad: fix carrier when no valid slaves
From: Mahesh Bandewar (महेश बंडेवार) @ 2026-03-27 16:37 UTC (permalink / raw)
  To: Louis Scalbert
  Cc: netdev, andrew+netdev, jv, edumazet, kuba, pabeni, fbl, andy,
	shemminger
In-Reply-To: <CAJ5u_OcpOGSPYK-EzYeW8LdpJrLd16fGn6zN6cvk+tCjCi+QxA@mail.gmail.com>

On Fri, Mar 27, 2026 at 9:22 AM Louis Scalbert <louis.scalbert@6wind.com> wrote:
>
> Hi everyone,
>
> Le mer. 25 mars 2026 à 14:44, Louis Scalbert
> <louis.scalbert@6wind.com> a écrit :
> >
> > When an 802.3ad (LACP) bonding interface has no slaves in the
> > collecting/distributing state, the bonding master may still report
> > carrier as up. In this situation, no slave is actually able to transmit
> > or receive traffic.
> >
> I’ve just realized that the issue description is not entirely accurate.
>
> Bonding links that are not in LACP Collecting/Distributing state are
> excluded from traffic
> distribution in favor of those that are. However, if no links are in
> Collecting/Distributing state,
> the kernel considers all links as valid for distributing traffic.
>
> > As a result, upper-layer daemons consider the interface operational
> > while traffic is effectively blackholed.
>
> There are situations where traffic is not blackholed.
>
> For example, a Linux machine A is connected to machine B using two
> aggregated links.
> On A, LACP is configured, but not on B. As a result, LACP will not
> reach the Collecting/Distributing
> state since B does not send LACP frames. However, the bonding
> interface will still be up on both
> sides, and traffic can be exchanged.
>
> This behavior is, of course, not compliant with the LACP (802.3ad)
> standard, but applying this fix
> as-is could introduce regressions in some setups. Under normal
> conditions, having no Collecting /
> Distributing links means that no interfaces are operational for
> handling traffic - that is precisely the
> purpose of using LACP.
>
The legacy behavior is to "maintain connectivity" by selecting a
functioning aggregator, as long as there are no link events. It's
crucial to maintain this legacy behavior.

> I will publish a new version with a knob to enforce compliance with
> the LACP standard. It will be
> disabled by default for backward compatibility.
>
> What do you think of "lacp_strict" or "lacp_enforce" ?
>
I don't have any specific preference for the name, but the default
value must comply with the legacy behavior.

> >
> > Fix this by asserting carrier only when at least 'min_links' slaves are
> > in the collecting/distributing state (or collecting only if the
> > coupled_control default behavior is disabled).
> >
> > Fixes: 655f8919d549 ("bonding: add min links parameter to 802.3ad")
> > Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
> > ---
> >  drivers/net/bonding/bond_3ad.c | 24 ++++++++++++++++++++++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> > index af7f74cfdc08..6d3613755d45 100644
> > --- a/drivers/net/bonding/bond_3ad.c
> > +++ b/drivers/net/bonding/bond_3ad.c
> > @@ -745,6 +745,22 @@ static void __set_agg_ports_ready(struct aggregator *aggregator, int val)
> >         }
> >  }
> >
> > +static int __agg_valid_ports(struct aggregator *agg)
> > +{
> > +       struct port *port;
> > +       int valid = 0;
> > +
> > +       for (port = agg->lag_ports; port;
> > +            port = port->next_port_in_aggregator) {
> > +               if (port->actor_oper_port_state & LACP_STATE_COLLECTING &&
> > +                   (!port->slave->bond->params.coupled_control ||
> > +                    port->actor_oper_port_state & LACP_STATE_DISTRIBUTING))
> > +                       valid++;
> > +       }
> > +
> > +       return valid;
> > +}
> > +
> >  static int __agg_active_ports(struct aggregator *agg)
> >  {
> >         struct port *port;
> > @@ -2120,6 +2136,7 @@ static void ad_enable_collecting_distributing(struct port *port,
> >                           port->actor_port_number,
> >                           port->aggregator->aggregator_identifier);
> >                 __enable_port(port);
> > +               bond_3ad_set_carrier(port->slave->bond);
> >                 /* Slave array needs update */
> >                 *update_slave_arr = true;
> >                 /* Should notify peers if possible */
> > @@ -2141,6 +2158,7 @@ static void ad_disable_collecting_distributing(struct port *port,
> >                           port->actor_port_number,
> >                           port->aggregator->aggregator_identifier);
> >                 __disable_port(port);
> > +               bond_3ad_set_carrier(port->slave->bond);
> >                 /* Slave array needs an update */
> >                 *update_slave_arr = true;
> >         }
> > @@ -2819,8 +2837,10 @@ int bond_3ad_set_carrier(struct bonding *bond)
> >         }
> >         active = __get_active_agg(&(SLAVE_AD_INFO(first_slave)->aggregator));
> >         if (active) {
> > -               /* are enough slaves available to consider link up? */
> > -               if (__agg_active_ports(active) < bond->params.min_links) {
> > +               /* are enough slaves in collecting (and distributing) state to consider
> > +                * link up?
> > +                */
> > +               if (__agg_valid_ports(active) < bond->params.min_links) {
> >                         if (netif_carrier_ok(bond->dev)) {
> >                                 netif_carrier_off(bond->dev);
> >                                 goto out;
> > --
> > 2.39.2
> >
>

^ permalink raw reply

* RE: [PATCH net-next 5/5] dpll: zl3073x: add ref-sync pair support
From: Prathosh.Satish @ 2026-03-27 16:24 UTC (permalink / raw)
  To: ivecera, netdev
  Cc: arkadiusz.kubalewski, jiri, mschmidt, poros, horms,
	vadim.fedorenko, linux-kernel, conor+dt, krzk+dt, robh,
	devicetree, pvaanane
In-Reply-To: <20260319174826.7623-6-ivecera@redhat.com>

Reviewed-by: prathosh.satish@microchip.com

-----Original Message-----
From: Ivan Vecera <ivecera@redhat.com> 
Sent: Thursday, March 19, 2026 5:48 PM
To: netdev@vger.kernel.org
Cc: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>; Jiri Pirko <jiri@resnulli.us>; Michal Schmidt <mschmidt@redhat.com>; Petr Oros <poros@redhat.com>; Prathosh Satish - M66066 <Prathosh.Satish@microchip.com>; Simon Horman <horms@kernel.org>; Vadim Fedorenko <vadim.fedorenko@linux.dev>; linux-kernel@vger.kernel.org; Conor Dooley <conor+dt@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; devicetree@vger.kernel.org; Pasi Vaananen <pvaanane@redhat.com>
Subject: [PATCH net-next 5/5] dpll: zl3073x: add ref-sync pair support

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Add support for ref-sync pair registration using the 'ref-sync-sources'
phandle property from device tree. A ref-sync pair consists of a clock reference and a low-frequency sync signal where the DPLL locks to the clock reference but phase-aligns to the sync reference.

The implementation:
- Stores fwnode handle in zl3073x_dpll_pin during pin registration
- Adds ref_sync_get/set callbacks to read and write the sync control
  mode and pair registers
- Validates ref-sync frequency constraints: sync signal must be 8 kHz
  or less, clock reference must be 1 kHz or more and higher than sync
- Excludes sync source from automatic reference selection by setting
  its priority to NONE on connect; on disconnect the priority is left
  as NONE and the user must explicitly make the pin selectable again
- Iterates ref-sync-sources phandles to register declared pairings
  via dpll_pin_ref_sync_pair_add()

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 207 +++++++++++++++++++++++++++++++++++-
 1 file changed, 206 insertions(+), 1 deletion(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c index 276f0a92db0b1..8010e2635f641 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/netlink.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/sprintf.h>

@@ -30,6 +31,7 @@
  * @dpll: DPLL the pin is registered to
  * @dpll_pin: pointer to registered dpll_pin
  * @tracker: tracking object for the acquired reference
+ * @fwnode: firmware node handle
  * @label: package label
  * @dir: pin direction
  * @id: pin id
@@ -45,6 +47,7 @@ struct zl3073x_dpll_pin {
        struct zl3073x_dpll     *dpll;
        struct dpll_pin         *dpll_pin;
        dpll_tracker            tracker;
+       struct fwnode_handle    *fwnode;
        char                    label[8];
        enum dpll_pin_direction dir;
        u8                      id;
@@ -184,6 +187,109 @@ zl3073x_dpll_input_pin_esync_set(const struct dpll_pin *dpll_pin,
        return zl3073x_ref_state_set(zldev, ref_id, &ref);  }

+static int
+zl3073x_dpll_input_pin_ref_sync_get(const struct dpll_pin *dpll_pin,
+                                   void *pin_priv,
+                                   const struct dpll_pin *ref_sync_pin,
+                                   void *ref_sync_pin_priv,
+                                   enum dpll_pin_state *state,
+                                   struct netlink_ext_ack *extack) {
+       struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
+       struct zl3073x_dpll_pin *pin = pin_priv;
+       struct zl3073x_dpll *zldpll = pin->dpll;
+       struct zl3073x_dev *zldev = zldpll->dev;
+       const struct zl3073x_ref *ref;
+       u8 ref_id, mode, pair;
+
+       ref_id = zl3073x_input_pin_ref_get(pin->id);
+       ref = zl3073x_ref_state_get(zldev, ref_id);
+       mode = zl3073x_ref_sync_mode_get(ref);
+       pair = zl3073x_ref_sync_pair_get(ref);
+
+       if (mode == ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR &&
+           pair == zl3073x_input_pin_ref_get(sync_pin->id))
+               *state = DPLL_PIN_STATE_CONNECTED;
+       else
+               *state = DPLL_PIN_STATE_DISCONNECTED;
+
+       return 0;
+}
+
+static int
+zl3073x_dpll_input_pin_ref_sync_set(const struct dpll_pin *dpll_pin,
+                                   void *pin_priv,
+                                   const struct dpll_pin *ref_sync_pin,
+                                   void *ref_sync_pin_priv,
+                                   const enum dpll_pin_state state,
+                                   struct netlink_ext_ack *extack) {
+       struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
+       struct zl3073x_dpll_pin *pin = pin_priv;
+       struct zl3073x_dpll *zldpll = pin->dpll;
+       struct zl3073x_dev *zldev = zldpll->dev;
+       u8 mode, ref_id, sync_ref_id;
+       struct zl3073x_chan chan;
+       struct zl3073x_ref ref;
+       int rc;
+
+       ref_id = zl3073x_input_pin_ref_get(pin->id);
+       sync_ref_id = zl3073x_input_pin_ref_get(sync_pin->id);
+       ref = *zl3073x_ref_state_get(zldev, ref_id);
+
+       if (state == DPLL_PIN_STATE_CONNECTED) {
+               const struct zl3073x_ref *sync_ref;
+               u32 ref_freq, sync_freq;
+
+               sync_ref = zl3073x_ref_state_get(zldev, sync_ref_id);
+               ref_freq = zl3073x_ref_freq_get(&ref);
+               sync_freq = zl3073x_ref_freq_get(sync_ref);
+
+               /* Sync signal must be 8 kHz or less and clock reference
+                * must be 1 kHz or more and higher than the sync signal.
+                */
+               if (sync_freq > 8000) {
+                       NL_SET_ERR_MSG(extack,
+                                      "sync frequency must be 8 kHz or less");
+                       return -EINVAL;
+               }
+               if (ref_freq < 1000) {
+                       NL_SET_ERR_MSG(extack,
+                                      "clock frequency must be 1 kHz or more");
+                       return -EINVAL;
+               }
+               if (ref_freq <= sync_freq) {
+                       NL_SET_ERR_MSG(extack,
+                                      "clock frequency must be higher than sync frequency");
+                       return -EINVAL;
+               }
+
+               zl3073x_ref_sync_pair_set(&ref, sync_ref_id);
+               mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR;
+       } else {
+               mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR_OFF;
+       }
+
+       zl3073x_ref_sync_mode_set(&ref, mode);
+
+       rc = zl3073x_ref_state_set(zldev, ref_id, &ref);
+       if (rc)
+               return rc;
+
+       /* Exclude sync source from automatic reference selection by setting
+        * its priority to NONE. On disconnect the priority is left as NONE
+        * and the user must explicitly make the pin selectable again.
+        */
+       if (state == DPLL_PIN_STATE_CONNECTED) {
+               chan = *zl3073x_chan_state_get(zldev, zldpll->id);
+               zl3073x_chan_ref_prio_set(&chan, sync_ref_id,
+                                         ZL_DPLL_REF_PRIO_NONE);
+               return zl3073x_chan_state_set(zldev, zldpll->id, &chan);
+       }
+
+       return 0;
+}
+
 static int
 zl3073x_dpll_input_pin_ffo_get(const struct dpll_pin *dpll_pin, void *pin_priv,
                               const struct dpll_device *dpll, void *dpll_priv, @@ -1100,6 +1206,8 @@ static const struct dpll_pin_ops zl3073x_dpll_input_pin_ops = {
        .phase_adjust_set = zl3073x_dpll_input_pin_phase_adjust_set,
        .prio_get = zl3073x_dpll_input_pin_prio_get,
        .prio_set = zl3073x_dpll_input_pin_prio_set,
+       .ref_sync_get = zl3073x_dpll_input_pin_ref_sync_get,
+       .ref_sync_set = zl3073x_dpll_input_pin_ref_sync_set,
        .state_on_dpll_get = zl3073x_dpll_input_pin_state_on_dpll_get,
        .state_on_dpll_set = zl3073x_dpll_input_pin_state_on_dpll_set,
 };
@@ -1190,8 +1298,11 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
        if (IS_ERR(props))
                return PTR_ERR(props);

-       /* Save package label, esync capability and phase adjust granularity */
+       /* Save package label, fwnode, esync capability and phase adjust
+        * granularity.
+        */
        strscpy(pin->label, props->package_label);
+       pin->fwnode = fwnode_handle_get(props->fwnode);
        pin->esync_control = props->esync_control;
        pin->phase_gran = props->dpll_props.phase_gran;

@@ -1236,6 +1347,8 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
        dpll_pin_put(pin->dpll_pin, &pin->tracker);
        pin->dpll_pin = NULL;
 err_pin_get:
+       fwnode_handle_put(pin->fwnode);
+       pin->fwnode = NULL;
        zl3073x_pin_props_put(props);

        return rc;
@@ -1265,6 +1378,9 @@ zl3073x_dpll_pin_unregister(struct zl3073x_dpll_pin *pin)

        dpll_pin_put(pin->dpll_pin, &pin->tracker);
        pin->dpll_pin = NULL;
+
+       fwnode_handle_put(pin->fwnode);
+       pin->fwnode = NULL;
 }

 /**
@@ -1735,6 +1851,88 @@ zl3073x_dpll_free(struct zl3073x_dpll *zldpll)
        kfree(zldpll);
 }

+/**
+ * zl3073x_dpll_ref_sync_pair_register - register ref_sync pairs for a 
+pin
+ * @pin: pointer to zl3073x_dpll_pin structure
+ *
+ * Iterates 'ref-sync-sources' phandles in the pin's firmware node and
+ * registers each declared pairing.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_dpll_ref_sync_pair_register(struct zl3073x_dpll_pin *pin) {
+       struct zl3073x_dev *zldev = pin->dpll->dev;
+       struct fwnode_handle *fwnode;
+       struct dpll_pin *sync_pin;
+       dpll_tracker tracker;
+       int n, rc;
+
+       for (n = 0; ; n++) {
+               /* Get n'th ref-sync source */
+               fwnode = fwnode_find_reference(pin->fwnode, "ref-sync-sources",
+                                              n);
+               if (IS_ERR(fwnode)) {
+                       rc = PTR_ERR(fwnode);
+                       break;
+               }
+
+               /* Find associated dpll pin */
+               sync_pin = fwnode_dpll_pin_find(fwnode, &tracker);
+               fwnode_handle_put(fwnode);
+               if (!sync_pin) {
+                       dev_warn(zldev->dev, "%s: ref-sync source %d not found",
+                                pin->label, n);
+                       continue;
+               }
+
+               /* Register new ref-sync pair */
+               rc = dpll_pin_ref_sync_pair_add(pin->dpll_pin, sync_pin);
+               dpll_pin_put(sync_pin, &tracker);
+
+               /* -EBUSY means pairing already exists from another DPLL's
+                * registration.
+                */
+               if (rc && rc != -EBUSY) {
+                       dev_err(zldev->dev,
+                               "%s: failed to add ref-sync source %d: %pe",
+                               pin->label, n, ERR_PTR(rc));
+                       break;
+               }
+       }
+
+       return rc != -ENOENT ? rc : 0;
+}
+
+/**
+ * zl3073x_dpll_ref_sync_pairs_register - register ref_sync pairs for a 
+DPLL
+ * @zldpll: pointer to zl3073x_dpll structure
+ *
+ * Iterates all registered input pins of the given DPLL and establishes
+ * ref_sync pairings declared by 'ref-sync-sources' phandles in the
+ * device tree.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_dpll_ref_sync_pairs_register(struct zl3073x_dpll *zldpll) {
+       struct zl3073x_dpll_pin *pin;
+       int rc;
+
+       list_for_each_entry(pin, &zldpll->pins, list) {
+               if (!zl3073x_dpll_is_input_pin(pin) || !pin->fwnode)
+                       continue;
+
+               rc = zl3073x_dpll_ref_sync_pair_register(pin);
+               if (rc)
+                       return rc;
+       }
+
+       return 0;
+}
+
 /**
  * zl3073x_dpll_register - register DPLL device and all its pins
  * @zldpll: pointer to zl3073x_dpll structure @@ -1758,6 +1956,13 @@ zl3073x_dpll_register(struct zl3073x_dpll *zldpll)
                return rc;
        }

+       rc = zl3073x_dpll_ref_sync_pairs_register(zldpll);
+       if (rc) {
+               zl3073x_dpll_pins_unregister(zldpll);
+               zl3073x_dpll_device_unregister(zldpll);
+               return rc;
+       }
+
        return 0;
 }

--
2.52.0


^ permalink raw reply

* Re: [PATCH net-next] net: phylink: allow PHYs to be attached in 802.3z inband mode
From: Russell King (Oracle) @ 2026-03-27 16:17 UTC (permalink / raw)
  To: Maxime Chevallier
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	netdev, Paolo Abeni, Alexis Lothore
In-Reply-To: <a5d7aedb-9770-492f-b528-0a28a652279b@bootlin.com>

On Fri, Mar 27, 2026 at 04:44:23PM +0100, Maxime Chevallier wrote:
> Hi Russell,
> 
> On 27/03/2026 13:56, Russell King (Oracle) wrote:
> > Now that we have proper decision making for inband mode support which
> > makes it a "best efforts" feature based on the capabilities of the PHY
> > and PCS, we can relax whether we expect and permit a PHY to be
> > attached. This is especially true for the 2500BASE-X case which some
> > PHYs use without inband on their host side interface for 2.5G speeds,
> > but use inband for slower speeds switching to SGMII on their host side
> > interface.
> > 
> > We already have such a case for some qcom-ethqos setups, although
> > qcom-ethqos overrides phylink's inband settings by accessing the PCS
> > directly at the moment. This should allow qcom-ethqos to transition to
> > defaulting to inband when 2500BASE-X or SGMII is specified in its DTS.
> > 
> > Allow PHYs to be attached when inband mode has been specified, which
> > will be necessary to allow inband mode to be used on qcom-ethqos.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> >  drivers/net/phy/phylink.c | 8 ++------
> >  1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> > index 087ac63f9193..1c178038e6f1 100644
> > --- a/drivers/net/phy/phylink.c
> > +++ b/drivers/net/phy/phylink.c
> > @@ -1965,9 +1965,7 @@ EXPORT_SYMBOL_GPL(phylink_destroy);
> >   */
> >  bool phylink_expects_phy(struct phylink *pl)
> >  {
> > -	if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
> > -	    (pl->cfg_link_an_mode == MLO_AN_INBAND &&
> > -	     phy_interface_mode_is_8023z(pl->link_interface)))
> > +	if (pl->cfg_link_an_mode == MLO_AN_FIXED)
> >  		return false;
> >  	return true;
> >  }
> > @@ -2206,9 +2204,7 @@ static int phylink_attach_phy(struct phylink *pl, struct phy_device *phy,
> >  {
> >  	u32 flags = 0;
> >  
> > -	if (WARN_ON(pl->cfg_link_an_mode == MLO_AN_FIXED ||
> > -		    (pl->cfg_link_an_mode == MLO_AN_INBAND &&
> > -		     phy_interface_mode_is_8023z(interface) && !pl->sfp_bus)))
> > +	if (WARN_ON(pl->cfg_link_an_mode == MLO_AN_FIXED))
> >  		return -EINVAL;
> >  
> >  	if (pl->phydev)
> 
> So I've tested that on my socfpga setup, and I now get this when trying to
> bring an interface attached to an SFP up :
> 
>   socfpga-dwmac ff702000.ethernet eth1: no phy found
> 
> This wasn't showing before because our DT has :
> 
>   &gmac1 {
>     status = "okay";
>     phy-mode = "1000base-x";
>     managed = "in-band-status";
>     sfp = <&sfp>;
>   };
> 
> It used to kinda work because the problem was hidden with the use of "1000base-x"
> as the PHY mode.
> 
> Now with this patch, using "sgmii" or "1000base-x" as the phy-mode doesn't
> do any difference, as expected.
> 
> The real issue IMO is rather how stmmac_init_phy() behaves, it's guarded by:
> 
>   if (!phylink_expects_phy(priv->phylink))
>           return 0;
> 
> and then tries to find a PHY, failing if there's none. We should probably add
> an extra check there to say "if we have a PCS, then it's OK not to have a PHY" ?

I had a feeling that we'd run into this, because stmmac is somewhat
"special" (for all the wrong reasons.)

phylink has, for a long time, had "we must have a PHY" when it operates
in MLO_AN_PHY mode, "we must not have a PHY" when it operates in
MLO_AN_FIXED mode, and "we may or may not have a PHY" when operating in
inband depending on the PHY interface mode.

stmmac basically doesn't support the latter - it assumes that if a PHY
node is not present, then it needs to fall back to the PHY addressed
by priv->plat->phy_addr, and if that's not specified, then it errors
out. phylink_expects_phy() was added to try and workaround this.

I suspect that if you specify:

   &gmac1 {
     status = "okay";
     phy-mode = "sgmii";
     managed = "in-band-status";
     sfp = <&sfp>;
   };

without this patch, you'll run into the same problem - while phylink
would be perfectly happy, stmmac decides "we must have a PHY!" because
phylink_expects_phy() will return true for this.

As I've recently said, I think we're more or less boxed in between a
rock and a hard place because of the hacks that qcom-ethqos introduced
with its PCS support. At the moment, I am failing to see any path where
the stmmac PCS can be programmed for inband according to phylink's
wishes and qcom-ethqos can be made to work without its hacks.

This all ultimately comes back to the half-hearted phylink conversion
of stmmac that was done behind my back without my review. Had the
conversion been done fully, and kept up with the phylink changes for
PCS support, then we probably wouldn't be in this situation today.

Unfortunately, the driver is going to keep breaking all the time that
it abuses phylink, and at the moment I see no way to stop this sodding
driver abusing phylink.

Can we not just delete this bloody driver and be done with it? :D

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* Re: [PATCH net v2 1/3] bonding: 3ad: fix carrier when no valid slaves
From: Louis Scalbert @ 2026-03-27 16:16 UTC (permalink / raw)
  To: netdev; +Cc: andrew+netdev, jv, edumazet, kuba, pabeni, fbl, andy, shemminger
In-Reply-To: <20260325134439.3048615-2-louis.scalbert@6wind.com>

Hi everyone,

Le mer. 25 mars 2026 à 14:44, Louis Scalbert
<louis.scalbert@6wind.com> a écrit :
>
> When an 802.3ad (LACP) bonding interface has no slaves in the
> collecting/distributing state, the bonding master may still report
> carrier as up. In this situation, no slave is actually able to transmit
> or receive traffic.
>
I’ve just realized that the issue description is not entirely accurate.

Bonding links that are not in LACP Collecting/Distributing state are
excluded from traffic
distribution in favor of those that are. However, if no links are in
Collecting/Distributing state,
the kernel considers all links as valid for distributing traffic.

> As a result, upper-layer daemons consider the interface operational
> while traffic is effectively blackholed.

There are situations where traffic is not blackholed.

For example, a Linux machine A is connected to machine B using two
aggregated links.
On A, LACP is configured, but not on B. As a result, LACP will not
reach the Collecting/Distributing
state since B does not send LACP frames. However, the bonding
interface will still be up on both
sides, and traffic can be exchanged.

This behavior is, of course, not compliant with the LACP (802.3ad)
standard, but applying this fix
as-is could introduce regressions in some setups. Under normal
conditions, having no Collecting /
Distributing links means that no interfaces are operational for
handling traffic - that is precisely the
purpose of using LACP.

I will publish a new version with a knob to enforce compliance with
the LACP standard. It will be
disabled by default for backward compatibility.

What do you think of "lacp_strict" or "lacp_enforce" ?

>
> Fix this by asserting carrier only when at least 'min_links' slaves are
> in the collecting/distributing state (or collecting only if the
> coupled_control default behavior is disabled).
>
> Fixes: 655f8919d549 ("bonding: add min links parameter to 802.3ad")
> Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
> ---
>  drivers/net/bonding/bond_3ad.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index af7f74cfdc08..6d3613755d45 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -745,6 +745,22 @@ static void __set_agg_ports_ready(struct aggregator *aggregator, int val)
>         }
>  }
>
> +static int __agg_valid_ports(struct aggregator *agg)
> +{
> +       struct port *port;
> +       int valid = 0;
> +
> +       for (port = agg->lag_ports; port;
> +            port = port->next_port_in_aggregator) {
> +               if (port->actor_oper_port_state & LACP_STATE_COLLECTING &&
> +                   (!port->slave->bond->params.coupled_control ||
> +                    port->actor_oper_port_state & LACP_STATE_DISTRIBUTING))
> +                       valid++;
> +       }
> +
> +       return valid;
> +}
> +
>  static int __agg_active_ports(struct aggregator *agg)
>  {
>         struct port *port;
> @@ -2120,6 +2136,7 @@ static void ad_enable_collecting_distributing(struct port *port,
>                           port->actor_port_number,
>                           port->aggregator->aggregator_identifier);
>                 __enable_port(port);
> +               bond_3ad_set_carrier(port->slave->bond);
>                 /* Slave array needs update */
>                 *update_slave_arr = true;
>                 /* Should notify peers if possible */
> @@ -2141,6 +2158,7 @@ static void ad_disable_collecting_distributing(struct port *port,
>                           port->actor_port_number,
>                           port->aggregator->aggregator_identifier);
>                 __disable_port(port);
> +               bond_3ad_set_carrier(port->slave->bond);
>                 /* Slave array needs an update */
>                 *update_slave_arr = true;
>         }
> @@ -2819,8 +2837,10 @@ int bond_3ad_set_carrier(struct bonding *bond)
>         }
>         active = __get_active_agg(&(SLAVE_AD_INFO(first_slave)->aggregator));
>         if (active) {
> -               /* are enough slaves available to consider link up? */
> -               if (__agg_active_ports(active) < bond->params.min_links) {
> +               /* are enough slaves in collecting (and distributing) state to consider
> +                * link up?
> +                */
> +               if (__agg_valid_ports(active) < bond->params.min_links) {
>                         if (netif_carrier_ok(bond->dev)) {
>                                 netif_carrier_off(bond->dev);
>                                 goto out;
> --
> 2.39.2
>

^ permalink raw reply

* Re: [PATCH net-next v5] net: pppoe: implement GRO/GSO support
From: Xin Long @ 2026-03-27 16:10 UTC (permalink / raw)
  To: Qingfang Deng
  Cc: Jakub Kicinski, Felix Fietkau, linux-ppp, Andrew Lunn,
	David S. Miller, Eric Dumazet, Paolo Abeni, David Ahern,
	Simon Horman, netdev, linux-kernel, Richard Gobert
In-Reply-To: <CALW65jYQt9-EQ0tKkgPRGMdf4Ra2oq4BT4XkCPu0zaRH=rzviQ@mail.gmail.com>

On Fri, Mar 27, 2026 at 10:43 AM Qingfang Deng <dqfext@gmail.com> wrote:
>
> Hi, Xin Long,
>
> On Fri, Mar 27, 2026 at 9:23 PM Xin Long <lucien.xin@gmail.com> wrote:
> >
> > On Thu, Mar 26, 2026 at 9:49 PM Qingfang Deng <dqfext@gmail.com> wrote:
> > >
> > > In my v6 patch, I set phdr->length to 0xFFFF. Is that okay?
> > >
> > > https://lore.kernel.org/netdev/20260326081127.61229-1-dqfext@gmail.com/
> >
> > I think you need to check how the PPPoE stack handles this value. If
> > it recalculates the actual length using skb->len when phdr->length ==
> > 0xFFFF, then this approach might be fine.
> >
> > Also, how does the PPPoE stack distinguish between 0xFFFF being the
> > actual payload length versus it being used as a marker to indicate a
> > jumbo packet (similar to BIG TCP)?
>
> In the receive function 'pppoe_rcv()': if a skb's actual payload
> length is less than phdr->length, it will be dropped; if greater than
> phdr->length, it will be trimmed to match phdr->length. After that,
> the phdr is popped and the skb is passed to the generic PPP layer.
> There is also a fastpath in Netfilter flowtable for IPv4/IPv6 that
> bypasses pppoe_rcv(): it does not check phdr->length at all, and only
> relies on the length field in the network header.
>
> In the v2 version of this patch, phdr->length is not updated in
> gro_complete(), so an additional check 'skb_is_gso()' is added to the
> function to avoid trimming a GRO skb. Then Richard Gobert suggested
> that the length field needs to be updated, and if the updated field
> matches the actual length, the 'skb_is_gso()' is not necessary, but
> only if the length fits in the u16 field.
I guess you have to look back to pppoe_rcv() for this, and double check
with Richard Gobert.

The phdr->length trick for jumbo packets might also affect how tcpdump
or tshark parses PPPoE packets.

^ permalink raw reply

* Re: [PATCH] rtnetlink: add missing netlink_ns_capable() check for peer netns
From: Kuniyuki Iwashima @ 2026-03-27 16:03 UTC (permalink / raw)
  To: nickgarlis; +Cc: kuba, netdev
In-Reply-To: <CA+jwDRk_pg6jLsg_nZ+DwHKcLk-TjbhS5o8aveOD5ns=yxUYCg@mail.gmail.com>

From: Nikolaos Gkarlis <nickgarlis@gmail.com>
Date: Fri, 27 Mar 2026 09:55:31 +0100
> Hi Jakub,
> 
> I've created a crude reproduction script using some AI. If that helps,
> you can find it here:
> https://gist.githubusercontent.com/nickgarlis/5a5815d438aa19b409806712f35f2cb1/raw/c7e2dd0902f1f53f5750f7bbd3ba39ff0b21e43e/inject_iface.c
> 
> > AI assistant here suggests this is not the valid Fixes tag,
> > which seems true on the surface. This commit was fixing
> > cleanup, IDK how it could break input validation?
> 
> I went back to check and ran the test against 7a0ea70da56e (the commit
> prior) and it does seem like it was broken then too. I think I must have
> messed up my testing process when I initially submitted the patch.
> 
> It also seems like the bug was always there or at least - almost always
> - since Iassume it wasn't a problem before unprivileged namespaces
> were introduced ?
> 
> In that case, should the tags point to the introduction of the
> attributes (like shown below) or point to the introduction of
> the namespaces ?
> 
> Fixes: 0eb87b02a705 ("veth: Set VETH_INFO_PEER to veth_link_ops.peer_type.")
> Fixes: 6b84e558e95d ("vxcan: Set VXCAN_INFO_PEER to vxcan_link_ops.peer_type.")
> Fixes: fefd5d082172 ("netkit: Set IFLA_NETKIT_PEER_INFO to
> netkit_link_ops.peer_type.")

No, these commits just factorised the common part.

veth does not have the userns check from

commit 81adee47dfb608df3ad0b91d230fb3cef75f0060
Author: Eric W. Biederman <ebiederm@aristanetworks.com>
Date:   Sun Nov 8 08:53:51 2009

    net: Support specifying the network namespace upon device creation.


and vxcan and netkit does not have the check from Day 0.

^ permalink raw reply

* Re: [PATCH net-next v2 2/4] net: hsr: fix VLAN add unwind on slave errors
From: Luka Gejak @ 2026-03-27 16:03 UTC (permalink / raw)
  To: Felix Maurer
  Cc: davem, edumazet, kuba, pabeni, netdev, horms, liuhangbin,
	linux-kernel
In-Reply-To: <acZvYp_SYvypHWgk@thinkpad>

On March 27, 2026 12:52:02 PM GMT+01:00, Felix Maurer <fmaurer@redhat.com> wrote:
>Thank you for updating this patch!
>
>On Thu, Mar 26, 2026 at 04:47:13PM +0100, luka.gejak@linux.dev wrote:
>> From: Luka Gejak <luka.gejak@linux.dev>
>>
>> When vlan_vid_add() fails for a secondary slave, the error path calls
>> vlan_vid_del() on the failing port instead of the peer slave that had
>> already succeeded. This results in asymmetric VLAN state across the HSR
>> pair.
>>
>> Fix this by switching to a centralized unwind path that removes the VID
>> from any slave device that was already programmed.
>>
>> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
>> ---
>>  net/hsr/hsr_device.c | 46 ++++++++++++++++++++++++++------------------
>>  1 file changed, 27 insertions(+), 19 deletions(-)
>>
>> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
>> index 5c3eca2235ce..75c491279df8 100644
>> --- a/net/hsr/hsr_device.c
>> +++ b/net/hsr/hsr_device.c
>> @@ -532,8 +532,8 @@ static void hsr_change_rx_flags(struct net_device *dev, int change)
>>  static int hsr_ndo_vlan_rx_add_vid(struct net_device *dev,
>>  				   __be16 proto, u16 vid)
>>  {
>> -	bool is_slave_a_added = false;
>> -	bool is_slave_b_added = false;
>> +	struct net_device *slave_a_dev = NULL;
>> +	struct net_device *slave_b_dev = NULL;
>>  	struct hsr_port *port;
>>  	struct hsr_priv *hsr;
>>  	int ret = 0;
>> @@ -546,29 +546,28 @@ static int hsr_ndo_vlan_rx_add_vid(struct net_device *dev,
>>  			continue;
>>
>>  		ret = vlan_vid_add(port->dev, proto, vid);
>> -		switch (port->type) {
>> -		case HSR_PT_SLAVE_A:
>> -			if (ret) {
>> -				/* clean up Slave-B */
>> +		if (ret) {
>> +			switch (port->type) {
>> +			case HSR_PT_SLAVE_A:
>>  				netdev_err(dev, "add vid failed for Slave-A\n");
>> -				if (is_slave_b_added)
>> -					vlan_vid_del(port->dev, proto, vid);
>> -				return ret;
>> +				break;
>> +			case HSR_PT_SLAVE_B:
>> +				netdev_err(dev, "add vid failed for Slave-B\n");
>> +				break;
>> +			default:
>> +				break;
>>  			}
>>
>> -			is_slave_a_added = true;
>> +			goto unwind;
>> +		}
>> +
>> +		switch (port->type) {
>> +		case HSR_PT_SLAVE_A:
>> +			slave_a_dev = port->dev;
>>  			break;
>>
>
>nit: superflous empty line (it's inconsistent with the other case
>blocks)
>
>>  		case HSR_PT_SLAVE_B:
>> -			if (ret) {
>> -				/* clean up Slave-A */
>> -				netdev_err(dev, "add vid failed for Slave-B\n");
>> -				if (is_slave_a_added)
>> -					vlan_vid_del(port->dev, proto, vid);
>> -				return ret;
>> -			}
>> -
>> -			is_slave_b_added = true;
>> +			slave_b_dev = port->dev;
>>  			break;
>>  		default:
>>  			break;
>
>I think this would look cleaner with the good and the bad paths in the
>same switch-case, but that may be a matter of (my) taste. What do you
>think?
>
>Thanks,
>   Felix
>

Hi Felix,
Thanks for taking a look at v2.
>nit: superflous empty line (it's inconsistent with the other case 
>blocks)
Good catch. I'll drop the extra newline in v3.
>I think this would look cleaner with the good and the bad paths in 
>the same switch-case, but that may be a matter of (my) taste. What do
> you think?
Very well. I will take your preference into consideration and will 
make mentioned changes in v3. However I am not currently home and will
send v3 once I come home in 1-2 days.
Best regards,
Luka Gejak

^ permalink raw reply

* Re: [PATCH 2/4] ipv6: mld: rename mldv2_mrc() and add mldv2_qqi()
From: kernel test robot @ 2026-03-27 15:49 UTC (permalink / raw)
  To: Ujjal Roy, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Nikolay Aleksandrov, Ido Schimmel,
	David Ahern
  Cc: oe-kbuild-all, Ujjal Roy, bridge, netdev, linux-kernel
In-Reply-To: <20260326150742.50289-3-royujjal@gmail.com>

Hi Ujjal,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]
[also build test WARNING on net/main klassert-ipsec/master linus/master v7.0-rc5 next-20260326]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ujjal-Roy/ipv4-igmp-get-rid-of-IGMPV3_-QQIC-MRC-and-simplify-calculation/20260327-083159
base:   net-next/main
patch link:    https://lore.kernel.org/r/20260326150742.50289-3-royujjal%40gmail.com
patch subject: [PATCH 2/4] ipv6: mld: rename mldv2_mrc() and add mldv2_qqi()
config: x86_64-randconfig-121-20260327 (https://download.01.org/0day-ci/archive/20260327/202603272318.WLQHm8jS-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260327/202603272318.WLQHm8jS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603272318.WLQHm8jS-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   net/ipv6/mcast.c: note: in included file:
   include/net/mld.h:32:43: sparse: sparse: array of flexible structures
>> include/net/mld.h:147:30: sparse: sparse: cast to restricted __be16

vim +147 include/net/mld.h

    29	
    30	struct mld2_report {
    31		struct icmp6hdr		mld2r_hdr;
  > 32		struct mld2_grec	mld2r_grec[];
    33	};
    34	
    35	#define mld2r_type		mld2r_hdr.icmp6_type
    36	#define mld2r_resv1		mld2r_hdr.icmp6_code
    37	#define mld2r_cksum		mld2r_hdr.icmp6_cksum
    38	#define mld2r_resv2		mld2r_hdr.icmp6_dataun.un_data16[0]
    39	#define mld2r_ngrec		mld2r_hdr.icmp6_dataun.un_data16[1]
    40	
    41	/* MLDv2 Query */
    42	struct mld2_query {
    43		struct icmp6hdr		mld2q_hdr;
    44		struct in6_addr		mld2q_mca;
    45	#if defined(__LITTLE_ENDIAN_BITFIELD)
    46		__u8			mld2q_qrv:3,
    47					mld2q_suppress:1,
    48					mld2q_resv2:4;
    49	#elif defined(__BIG_ENDIAN_BITFIELD)
    50		__u8			mld2q_resv2:4,
    51					mld2q_suppress:1,
    52					mld2q_qrv:3;
    53	#else
    54	#error "Please fix <asm/byteorder.h>"
    55	#endif
    56		__u8			mld2q_qqic;
    57		__be16			mld2q_nsrcs;
    58		struct in6_addr		mld2q_srcs[];
    59	};
    60	
    61	#define mld2q_type		mld2q_hdr.icmp6_type
    62	#define mld2q_code		mld2q_hdr.icmp6_code
    63	#define mld2q_cksum		mld2q_hdr.icmp6_cksum
    64	#define mld2q_mrc		mld2q_hdr.icmp6_maxdelay
    65	#define mld2q_resv1		mld2q_hdr.icmp6_dataun.un_data16[1]
    66	
    67	/* RFC3810, 5.1.3. Maximum Response Code:
    68	 *
    69	 * If Maximum Response Code >= 32768, Maximum Response Code represents a
    70	 * floating-point value as follows:
    71	 *
    72	 *  0 1 2 3 4 5 6 7 8 9 A B C D E F
    73	 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    74	 * |1| exp |          mant         |
    75	 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    76	 */
    77	#define MLDV2_MRC_EXP(value)	(((value) >> 12) & 0x0007)
    78	#define MLDV2_MRC_MAN(value)	((value) & 0x0fff)
    79	
    80	/* RFC3810, 5.1.9. QQIC (Querier's Query Interval Code):
    81	 *
    82	 * If QQIC >= 128, QQIC represents a floating-point value as follows:
    83	 *
    84	 *  0 1 2 3 4 5 6 7
    85	 * +-+-+-+-+-+-+-+-+
    86	 * |1| exp | mant  |
    87	 * +-+-+-+-+-+-+-+-+
    88	 */
    89	#define MLDV2_QQIC_EXP(value)	(((value) >> 4) & 0x07)
    90	#define MLDV2_QQIC_MAN(value)	((value) & 0x0f)
    91	
    92	#define MLD_QQIC_MIN_THRESHOLD	128
    93	#define MLD_MRC_MIN_THRESHOLD	32768UL
    94	#define MLDV1_MRD_MAX_COMPAT	(MLD_MRC_MIN_THRESHOLD - 1)
    95	
    96	#define MLD_MAX_QUEUE		8
    97	#define MLD_MAX_SKBS		32
    98	
    99	/* V2 exponential field decoding */
   100	
   101	/* Calculate Maximum Response Delay from Maximum Response Code
   102	 *
   103	 * RFC3810, 5.1.3. defines the decoding formula:
   104	 *      0 1 2 3 4 5 6 7 8 9 A B C D E F
   105	 *     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   106	 *     |1| exp |          mant         |
   107	 *     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   108	 * Maximum Response Delay = (mant | 0x1000) << (exp+3)
   109	 */
   110	static inline unsigned long mldv2_mrd(const struct mld2_query *mlh2)
   111	{
   112		/* RFC3810, relevant sections:
   113		 *  - 5.1.3. Maximum Response Code
   114		 *  - 9.3. Query Response Interval
   115		 */
   116		unsigned long mc_mrc = ntohs(mlh2->mld2q_mrc);
   117	
   118		if (mc_mrc < MLD_MRC_MIN_THRESHOLD) {
   119			return mc_mrc;
   120		} else {
   121			unsigned long mc_man, mc_exp;
   122	
   123			mc_exp = MLDV2_MRC_EXP(mc_mrc);
   124			mc_man = MLDV2_MRC_MAN(mc_mrc);
   125	
   126			return ((mc_man | 0x1000) << (mc_exp + 3));
   127		}
   128	}
   129	
   130	/* Calculate Querier's Query Interval from Querier's Query Interval Code
   131	 *
   132	 * RFC3810, 5.1.9. defines the decoding formula:
   133	 *      0 1 2 3 4 5 6 7
   134	 *     +-+-+-+-+-+-+-+-+
   135	 *     |1| exp | mant  |
   136	 *     +-+-+-+-+-+-+-+-+
   137	 * QQI = (mant | 0x10) << (exp + 3)
   138	 */
   139	static inline unsigned long mldv2_qqi(const struct mld2_query *mlh2)
   140	{
   141		/* RFC3810, relevant sections:
   142		 *  - 5.1.9. QQIC (Querier's Query Interval Code)
   143		 *  - 9.2. Query Interval
   144		 *  - 9.12. Older Version Querier Present Timeout
   145		 *    (the [Query Interval] in the last Query received)
   146		 */
 > 147		unsigned long qqic = ntohs(mlh2->mld2q_qqic);
   148	
   149		if (qqic < MLD_QQIC_MIN_THRESHOLD) {
   150			return qqic;
   151		} else {
   152			unsigned long mc_man, mc_exp;
   153	
   154			mc_exp = MLDV2_QQIC_EXP(qqic);
   155			mc_man = MLDV2_QQIC_MAN(qqic);
   156	
   157			return ((mc_man | 0x10) << (mc_exp + 3));
   158		}
   159	}
   160	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH net-next] net: phylink: allow PHYs to be attached in 802.3z inband mode
From: Maxime Chevallier @ 2026-03-27 15:44 UTC (permalink / raw)
  To: Russell King (Oracle), Andrew Lunn
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, netdev,
	Paolo Abeni, Alexis Lothore
In-Reply-To: <E1w66jW-0000000E3qS-23fW@rmk-PC.armlinux.org.uk>

Hi Russell,

On 27/03/2026 13:56, Russell King (Oracle) wrote:
> Now that we have proper decision making for inband mode support which
> makes it a "best efforts" feature based on the capabilities of the PHY
> and PCS, we can relax whether we expect and permit a PHY to be
> attached. This is especially true for the 2500BASE-X case which some
> PHYs use without inband on their host side interface for 2.5G speeds,
> but use inband for slower speeds switching to SGMII on their host side
> interface.
> 
> We already have such a case for some qcom-ethqos setups, although
> qcom-ethqos overrides phylink's inband settings by accessing the PCS
> directly at the moment. This should allow qcom-ethqos to transition to
> defaulting to inband when 2500BASE-X or SGMII is specified in its DTS.
> 
> Allow PHYs to be attached when inband mode has been specified, which
> will be necessary to allow inband mode to be used on qcom-ethqos.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  drivers/net/phy/phylink.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 087ac63f9193..1c178038e6f1 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -1965,9 +1965,7 @@ EXPORT_SYMBOL_GPL(phylink_destroy);
>   */
>  bool phylink_expects_phy(struct phylink *pl)
>  {
> -	if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
> -	    (pl->cfg_link_an_mode == MLO_AN_INBAND &&
> -	     phy_interface_mode_is_8023z(pl->link_interface)))
> +	if (pl->cfg_link_an_mode == MLO_AN_FIXED)
>  		return false;
>  	return true;
>  }
> @@ -2206,9 +2204,7 @@ static int phylink_attach_phy(struct phylink *pl, struct phy_device *phy,
>  {
>  	u32 flags = 0;
>  
> -	if (WARN_ON(pl->cfg_link_an_mode == MLO_AN_FIXED ||
> -		    (pl->cfg_link_an_mode == MLO_AN_INBAND &&
> -		     phy_interface_mode_is_8023z(interface) && !pl->sfp_bus)))
> +	if (WARN_ON(pl->cfg_link_an_mode == MLO_AN_FIXED))
>  		return -EINVAL;
>  
>  	if (pl->phydev)

So I've tested that on my socfpga setup, and I now get this when trying to
bring an interface attached to an SFP up :

  socfpga-dwmac ff702000.ethernet eth1: no phy found

This wasn't showing before because our DT has :

  &gmac1 {
    status = "okay";
    phy-mode = "1000base-x";
    managed = "in-band-status";
    sfp = <&sfp>;
  };

It used to kinda work because the problem was hidden with the use of "1000base-x"
as the PHY mode.

Now with this patch, using "sgmii" or "1000base-x" as the phy-mode doesn't
do any difference, as expected.

The real issue IMO is rather how stmmac_init_phy() behaves, it's guarded by:

  if (!phylink_expects_phy(priv->phylink))
          return 0;

and then tries to find a PHY, failing if there's none. We should probably add
an extra check there to say "if we have a PCS, then it's OK not to have a PHY" ?

Thanks,

Maxime

^ permalink raw reply

* Re: [PATCH net-next 4/4] net: dsa: initial support for MT7628 embedded switch
From: Joris Vaisvila @ 2026-03-27 15:39 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, horms, pabeni, kuba, edumazet, davem, olteanv, devicetree,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
In-Reply-To: <e6d91e11-fe19-48c0-886f-e1c0c92b094d@lunn.ch>

Thanks for the review, will fix all of these in v2. 

^ permalink raw reply

* Re: [PATCH net-next 0/5] veth: add Byte Queue Limits (BQL) support
From: Jonas Köppeler @ 2026-03-27 15:37 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, jhs, jiri,
	kernel-team, Chris Arges, Mike Freemon
In-Reply-To: <7f00346d-5dc6-421b-8d61-75c1c3898c30@kernel.org>

On 3/27/26 13:49, Jesper Dangaard Brouer wrote:
>
>
> On 27/03/2026 10.50, Toke Høiland-Jørgensen wrote:
>> hawk@kernel.org writes:
>>
>>> From: Jesper Dangaard Brouer <hawk@kernel.org>
>>>
>>> This series adds BQL (Byte Queue Limits) to the veth driver, reducing
>>> latency by dynamically limiting in-flight bytes in the ptr_ring and
>>> moving buffering into the qdisc where AQM algorithms can act on it.
>>>
>>> Problem:
>>>    veth's 256-entry ptr_ring acts as a "dark buffer" -- packets queued
>>>    there are invisible to the qdisc's AQM.  Under load, the ring fills
>>>    completely (DRV_XOFF backpressure), adding up to 256 packets of
>>>    unmanaged latency before the qdisc even sees congestion.
>>>
>>> Solution:
>>>    BQL (STACK_XOFF) dynamically limits in-flight bytes, stopping the
>>>    queue before the ring fills.  This keeps the ring shallow and pushes
>>>    excess packets into the qdisc, where sojourn-based AQM can measure
>>>    and drop them.
>>
>> So one question here: Is *Byte* queue limits really the right thing for
>> veth? As you mention above, the ptr_ring is sized in a number of
>> packets. On a physical NIC, accounting bytes makes sense because there's
>> a fixed line rate, so bytes turn directly into latency.
>>
>> But on a veth device, the stack processing is per packet, and most
>> processing takes the same amount of time regardless of the size of the
>> packet (e.g., netfilter rules that operate on the skb only).
>>
>> So my worry would be that when you're accounting in bytes, if there's a
>> mix of big and small packets, you'd end up with the BQL algorithm
>> scaling to a "too large" value, which would allow a lot of small packets
>> to be queued up, adding extra latency (or even overflowing the ring
>> buffer if the ratio is large enough).
>>
>> Have you run any such experiments? 
>
> Thank for bring this up.
> Yes, we have considered this (and agree).
>
> Jonas is conduction some experiments.
> I will let Jonas answer?
Hi,

I used the provided selftest, modified so that the payload size alternates
between 1400 bytes and sizeof(struct pkt_hdr) = 24 bytes every 5000 packets.

The receiver was slowed down using 10K iptables rules. I could confirm that
the receive queue filled up to ~66 packets, whereas the BQL limit is around
2884 bytes, corresponding to approximately 2 x 1400-byte packets.

I compared two accounting strategies: using skb->len vs. a fixed size of 1.

Ping results over 5 runs using skb->len accounting:

   rtt min/avg/max/mdev = 0.636/2.784/ 9.543/1.735 ms
   rtt min/avg/max/mdev = 0.629/2.947/10.587/1.927 ms
   rtt min/avg/max/mdev = 0.587/2.966/11.625/1.963 ms
   rtt min/avg/max/mdev = 0.589/3.006/10.694/1.979 ms

Ping results over 5 runs using fixed size (1) accounting:

   rtt min/avg/max/mdev = 0.587/2.446/6.261/1.065 ms
   rtt min/avg/max/mdev = 0.641/2.339/6.008/0.950 ms
   rtt min/avg/max/mdev = 0.688/2.527/5.506/1.086 ms
   rtt min/avg/max/mdev = 0.596/2.411/5.228/1.041 ms

The avg and max RTT are consistently lower with the fixed-size accounting.
This suggests that the excess buffered packets contribute to some latency.

Jonas
>
> > And have you tried just accounting> the queue in packets, so instead 
> of:
>>
>> +        netdev_tx_sent_queue(txq, skb->len);
>>
>> you'd just do:
>>
>> +        netdev_tx_sent_queue(txq, 1);
>
> I've been playing with using 1000 instead of 1, as that seems to work
> better with the DQL algorithm[1].
>
> --Jesper
>
> [1] 
> https://medium.com/@tom_84912/byte-queue-limits-the-unauthorized-biography-61adc5730b83
>
>

^ permalink raw reply

* Re: [PATCH 3/3] net: dsa: microchip: implement KSZ87xx Module 3 low-loss cable errata
From: Bastien Curutchet @ 2026-03-27 15:38 UTC (permalink / raw)
  To: Marek Vasut, Vladimir Oltean, Fidelio Lawson
  Cc: Woojung Huh, UNGLinuxDriver, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Marek Vasut, Maxime Chevallier,
	netdev, devicetree, linux-kernel, Fidelio Lawson
In-Reply-To: <80e21085-84b9-4ebd-9a60-1d7acfc1a56b@nabladev.com>

Hi Marek,

On 3/27/26 3:55 PM, Marek Vasut wrote:
> On 3/27/26 3:47 PM, Bastien Curutchet wrote:
> 
> Hello Bastien,
> 
>>> FYI, the driver is in a restructuring process. The ksz88xx_switch_ops
>>> will be split out of the common ksz_switch_ops. This will conflict with
>>> your series, so you should rebase on top of that much larger set.
>>> You can coordinate with Bastien Curutchet to see what is the status:
>>> https://lore.kernel.org/netdev/20260313153849.qkfzv5c2u6fepjku@skbuf
>>
>> Indeed, I'm currently polishing a first iteration for it. I'll send it 
>> ASAP.
> I would argue that this low-loss cable errata is a bugfix, so maybe that 
> should have a priority over restructuring ?

Yes, I'm fine with that.

These three patches aren't big so I shouldn't encounter much difficulty 
to fix the conflicts on my side.


Best regards,
Bastien

^ permalink raw reply

* RE: [PATCH net-next 1/5] dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()
From: Prathosh.Satish @ 2026-03-27 15:35 UTC (permalink / raw)
  To: horms, ivecera
  Cc: devicetree, vadim.fedorenko, jiri, netdev, pvaanane,
	arkadiusz.kubalewski, robh, linux-kernel, conor+dt, krzk+dt,
	mschmidt, poros
In-Reply-To: <20260321090108.GP74886@horms.kernel.org>

Reviewed-by: prathosh.satish@microchip.com

-----Original Message-----
From: Simon Horman <horms@kernel.org> 
Sent: Saturday, March 21, 2026 9:01 AM
To: Ivan Vecera <ivecera@redhat.com>
Cc: devicetree@vger.kernel.org; vadim.fedorenko@linux.dev; jiri@resnulli.us; netdev@vger.kernel.org; pvaanane@redhat.com; arkadiusz.kubalewski@intel.com; robh@kernel.org; linux-kernel@vger.kernel.org; conor+dt@kernel.org; krzk+dt@kernel.org; Prathosh Satish - M66066 <Prathosh.Satish@microchip.com>; mschmidt@redhat.com; poros@redhat.com
Subject: Re: [PATCH net-next 1/5] dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On Fri, Mar 20, 2026 at 06:31:36PM +0100, Ivan Vecera wrote:
> Hi Simon,
>
> On 3/20/26 6:17 PM, 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.
> >
> > For local reproduction steps see 
> > https://netdev-ai.bots.linux.dev/ai-local.html
> > ---
> > dpll: zl3073x: clean up esync get/set and use zl3073x_out_is_ndiv()
> >
> > Return -EOPNOTSUPP early in esync_get callbacks when esync is not 
> > supported instead of conditionally populating the range at the end.
> > This simplifies the control flow by removing the finish label/goto 
> > in the output variant and the conditional range assignment in both 
> > input and output variants.
> >
> > Replace open-coded N-div signal format switch statements with
> > zl3073x_out_is_ndiv() helper in esync_get, esync_set and 
> > frequency_set callbacks.
> >
> > > diff --git a/drivers/dpll/zl3073x/dpll.c 
> > > b/drivers/dpll/zl3073x/dpll.c index a29f606318f6..79ef62d69a32 
> > > 100644
> > > --- a/drivers/dpll/zl3073x/dpll.c
> > > +++ b/drivers/dpll/zl3073x/dpll.c
> > > @@ -131,6 +131,12 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >           ref_id = zl3073x_input_pin_ref_get(pin->id);
> > >           ref = zl3073x_ref_state_get(zldev, ref_id);
> > >
> > > + if (!pin->esync_control || zl3073x_ref_freq_get(ref) == 1)
> > > +         return -EOPNOTSUPP;
> > > +
> > > + esync->range = esync_freq_ranges; range_num = 
> > > + esync->ARRAY_SIZE(esync_freq_ranges);
> > > +
> > >           switch (FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl)) {
> > >           case ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75:
> > >                   esync->freq = ref->esync_n_div == 
> > > ZL_REF_ESYNC_DIV_1HZ ? 1 : 0; @@ -142,17 +148,6 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >                   break;
> > >           }
> > >
> > > - /* If the pin supports esync control expose its range but only
> > > -  * if the current reference frequency is > 1 Hz.
> > > -  */
> > > - if (pin->esync_control && zl3073x_ref_freq_get(ref) > 1) {
> > > -         esync->range = esync_freq_ranges;
> > > -         esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> > > - } else {
> > > -         esync->range = NULL;
> > > -         esync->range_num = 0;
> > > - }
> > > -
> > >           return 0;
> > >   }
> >
> > The refactoring changes the behavior when the reference frequency is 0 Hz.
> >
> > The old code checked 'zl3073x_ref_freq_get(ref) > 1' which excluded 
> > both
> > freq==0 and freq==1 from having the esync range exposed. The new 
> > code checks
> > 'zl3073x_ref_freq_get(ref) == 1' which only excludes freq==1.
> >
> > When freq==0 (which can happen when freq_base, freq_mult, or 
> > freq_ratio_m is 0), the old code would return 0 with 
> > esync->range=NULL and
> > esync->range_num=0, while the new code returns 0 with 
> > esync->range=esync_freq_ranges and esync->range_num=1.
> >
> > The removed comment documented the original intent: "if the current 
> > reference frequency is > 1 Hz". Should the new check be 'zl3073x_ref_freq_get(ref) <= 1'
> > to preserve the original behavior for 0 Hz frequencies?
> >
> > >
> > > @@ -582,8 +577,8 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >           struct zl3073x_dpll_pin *pin = pin_priv;
> > >           const struct zl3073x_synth *synth;
> > >           const struct zl3073x_out *out;
> > > + u32 synth_freq, out_freq;
> > >           u8 clock_type, out_id;
> > > - u32 synth_freq;
> > >
> > >           out_id = zl3073x_output_pin_out_get(pin->id);
> > >           out = zl3073x_out_state_get(zldev, out_id); @@ -592,17 
> > > +587,19 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >            * for N-division is also used for the esync divider so both cannot
> > >            * be used.
> > >            */
> > > - switch (zl3073x_out_signal_format_get(out)) {
> > > - case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV:
> > > - case ZL_OUTPUT_MODE_SIGNAL_FORMAT_2_NDIV_INV:
> > > + if (zl3073x_out_is_ndiv(out))
> > >                   return -EOPNOTSUPP;
> > > - default:
> > > -         break;
> > > - }
> > >
> > >           /* Get attached synth frequency */
> > >           synth = zl3073x_synth_state_get(zldev, zl3073x_out_synth_get(out));
> > >           synth_freq = zl3073x_synth_freq_get(synth);
> > > + out_freq = synth_freq / out->div;
> > > +
> > > + if (!pin->esync_control || out_freq == 1)
> > > +         return -EOPNOTSUPP;
> > > +
> > > + esync->range = esync_freq_ranges; range_num = 
> > > + esync->ARRAY_SIZE(esync_freq_ranges);
> > >
> > >           clock_type = FIELD_GET(ZL_OUTPUT_MODE_CLOCK_TYPE, out->mode);
> > >           if (clock_type != ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC) { @@ 
> > > -610,11 +607,11 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >                   esync->freq = 0;
> > >                   esync->pulse = 0;
> > >
> > > -         goto finish;
> > > +         return 0;
> > >           }
> > >
> > >           /* Compute esync frequency */
> > > - esync->freq = synth_freq / out->div / out->esync_n_period;
> > > + esync->freq = out_freq / out->esync_n_period;
> > >
> > >           /* By comparing the esync_pulse_width to the half of the pulse width
> > >            * the esync pulse percentage can be determined.
> > > @@ -623,18 +620,6 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
> > >            */
> > >           esync->pulse = (50 * out->esync_n_width) / out->div;
> > >
> > > -finish:
> > > - /* Set supported esync ranges if the pin supports esync control 
> > > and
> > > -  * if the output frequency is > 1 Hz.
> > > -  */
> > > - if (pin->esync_control && (synth_freq / out->div) > 1) {
> > > -         esync->range = esync_freq_ranges;
> > > -         esync->range_num = ARRAY_SIZE(esync_freq_ranges);
> > > - } else {
> > > -         esync->range = NULL;
> > > -         esync->range_num = 0;
> > > - }
> > > -
> > >           return 0;
> > >   }
> >
> > Similar to the input variant, this changes the behavior for 0 Hz 
> > output frequencies.
> >
> > The old code used '(synth_freq / out->div) > 1' to decide whether to 
> > expose the esync range, which excluded both out_freq==0 and 
> > out_freq==1. The new code uses 'out_freq == 1' for the early 
> > -EOPNOTSUPP return, only excluding out_freq==1.
> >
> > When out_freq==0 (which can happen when synth_freq is 0 or 
> > synth_freq <
> > out->div), the old code returned 0 with esync->range=NULL and
> > esync->range_num=0, while the new code returns 0 with 
> > esync->range=esync_freq_ranges and esync->range_num=1.
> >
> > The removed comment documented the intent: "if the output frequency 
> > is > 1 Hz". Should this also use 'out_freq <= 1' to maintain 
> > consistency with the original behavior?
> >
> > [ ... ]
> >
>
> The frequency for both input and output pins cannot be zero... Cannot 
> be set to 0 and device never returns configured frequency to be 0.
>
> I can modify the lines to be:
>
> if (!pin->esync_control || out_freq <= 1)
>       return -EOPNOTSUPP;
>
> but it is needless - but let me know.

Thanks Ivan,

If it is needless then there is no need to handle it in the code.
Sorry for not understanding that before forwarding on the AI generated review.

^ permalink raw reply

* RE: [PATCH net-next 2/5] dpll: zl3073x: use FIELD_MODIFY() for clear-and-set patterns
From: Prathosh.Satish @ 2026-03-27 15:35 UTC (permalink / raw)
  To: ivecera, netdev
  Cc: arkadiusz.kubalewski, jiri, mschmidt, poros, horms,
	vadim.fedorenko, linux-kernel, conor+dt, krzk+dt, robh,
	devicetree, pvaanane
In-Reply-To: <20260319174826.7623-3-ivecera@redhat.com>

Reviewed-by: prathosh.satish@microchip.com

-----Original Message-----
From: Ivan Vecera <ivecera@redhat.com> 
Sent: Thursday, March 19, 2026 5:48 PM
To: netdev@vger.kernel.org
Cc: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>; Jiri Pirko <jiri@resnulli.us>; Michal Schmidt <mschmidt@redhat.com>; Petr Oros <poros@redhat.com>; Prathosh Satish - M66066 <Prathosh.Satish@microchip.com>; Simon Horman <horms@kernel.org>; Vadim Fedorenko <vadim.fedorenko@linux.dev>; linux-kernel@vger.kernel.org; Conor Dooley <conor+dt@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Rob Herring <robh@kernel.org>; devicetree@vger.kernel.org; Pasi Vaananen <pvaanane@redhat.com>
Subject: [PATCH net-next 2/5] dpll: zl3073x: use FIELD_MODIFY() for clear-and-set patterns

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Replace open-coded clear-and-set bitfield operations with FIELD_MODIFY().

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/chan.h  | 17 ++++++-----------  drivers/dpll/zl3073x/core.c  |  3 +--  drivers/dpll/zl3073x/flash.c |  3 +--
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h index e0f02d3432086..481da2133202b 100644
--- a/drivers/dpll/zl3073x/chan.h
+++ b/drivers/dpll/zl3073x/chan.h
@@ -66,8 +66,7 @@ static inline u8 zl3073x_chan_ref_get(const struct zl3073x_chan *chan)
  */
 static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)  {
-       chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_MODE;
-       chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_MODE, mode);
+       FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_MODE, &chan->mode_refsel, 
+ mode);
 }

 /**
@@ -77,8 +76,7 @@ static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
  */
 static inline void zl3073x_chan_ref_set(struct zl3073x_chan *chan, u8 ref)  {
-       chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_REF;
-       chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
+       FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_REF, &chan->mode_refsel, ref);
 }

 /**
@@ -110,13 +108,10 @@ zl3073x_chan_ref_prio_set(struct zl3073x_chan *chan, u8 ref, u8 prio)  {
        u8 *val = &chan->ref_prio[ref / 2];

-       if (!(ref & 1)) {
-               *val &= ~ZL_DPLL_REF_PRIO_REF_P;
-               *val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_P, prio);
-       } else {
-               *val &= ~ZL_DPLL_REF_PRIO_REF_N;
-               *val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_N, prio);
-       }
+       if (!(ref & 1))
+               FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_P, val, prio);
+       else
+               FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_N, val, prio);
 }

 /**
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c index 6363002d48d46..7eebfc1ad1019 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -743,8 +743,7 @@ int zl3073x_dev_phase_avg_factor_set(struct zl3073x_dev *zldev, u8 factor)
        value = (factor + 1) & 0x0f;

        /* Update phase measurement control register */
-       dpll_meas_ctrl &= ~ZL_DPLL_MEAS_CTRL_AVG_FACTOR;
-       dpll_meas_ctrl |= FIELD_PREP(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, value);
+       FIELD_MODIFY(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, &dpll_meas_ctrl, 
+ value);
        rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MEAS_CTRL, dpll_meas_ctrl);
        if (rc)
                return rc;
diff --git a/drivers/dpll/zl3073x/flash.c b/drivers/dpll/zl3073x/flash.c index 83452a77e3e98..f85535c8ad246 100644
--- a/drivers/dpll/zl3073x/flash.c
+++ b/drivers/dpll/zl3073x/flash.c
@@ -194,8 +194,7 @@ zl3073x_flash_cmd_wait(struct zl3073x_dev *zldev, u32 operation,
        if (rc)
                return rc;

-       value &= ~ZL_WRITE_FLASH_OP;
-       value |= FIELD_PREP(ZL_WRITE_FLASH_OP, operation);
+       FIELD_MODIFY(ZL_WRITE_FLASH_OP, &value, operation);

        rc = zl3073x_write_u8(zldev, ZL_REG_WRITE_FLASH, value);
        if (rc)
--
2.52.0


^ permalink raw reply

* [PATCH net v7] net: use skb_header_pointer() for TCPv4 GSO frag_off check
From: Guoyu Su @ 2026-03-27 15:35 UTC (permalink / raw)
  To: edumazet, davem, kuba, pabeni
  Cc: willemdebruijn.kernel, netdev, horms, linux-kernel,
	syzkaller-bugs, syzbot+1543a7d954d9c6d00407, Guoyu Su
In-Reply-To: <willemdebruijn.kernel.19f0cde3c6457@gmail.com>

Syzbot reported a KMSAN uninit-value warning in gso_features_check()
called from netif_skb_features() [1].

gso_features_check() reads iph->frag_off to decide whether to clear
mangleid_features. Accessing the IPv4 header via ip_hdr()/inner_ip_hdr()
can rely on skb header offsets that are not always safe for direct
dereference on packets injected from PF_PACKET paths.

Use skb_header_pointer() for the TCPv4 frag_off check so the header read
is robust whether data is already linear or needs copying.

[1] https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407

Link: https://lore.kernel.org/netdev/willemdebruijn.kernel.1a9f35039caab@gmail.com/
Fixes: cbc53e08a793 ("GSO: Add GSO type for fixed IPv4 ID")
Reported-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1543a7d954d9c6d00407
Tested-by: syzbot+1543a7d954d9c6d00407@syzkaller.appspotmail.com
Signed-off-by: Guoyu Su <yss2813483011xxl@gmail.com>
---
v7:
 - Drop revision-diff wording from commit message body.
 - Minor style: keep variable definitions contiguous in gso_features_check().
 - No functional change from v6.

v6: https://lore.kernel.org/netdev/20260326121813.457049-1-yss2813483011xxl@gmail.com/
v5: https://lore.kernel.org/netdev/20260320141459.9691-1-yss2813483011xxl@gmail.com/
v4: https://lore.kernel.org/netdev/20260319005421.14908-1-yss2813483011xxl@gmail.com/
v3: https://lore.kernel.org/netdev/20260312104351.185370-1-yss2813483011xxl@gmail.com/
v2: https://lore.kernel.org/netdev/20260308083319.1255118-1-yss2813483011xxl@gmail.com/
v1: https://lore.kernel.org/netdev/20260307162905.3697050-1-yss2813483011xxl@gmail.com/

 net/core/dev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index fc5557062414..831129f2a69b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3821,10 +3821,15 @@ static netdev_features_t gso_features_check(const struct sk_buff *skb,
 	 * segmentation-offloads.rst).
 	 */
 	if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4) {
-		struct iphdr *iph = skb->encapsulation ?
-				    inner_ip_hdr(skb) : ip_hdr(skb);
+		const struct iphdr *iph;
+		struct iphdr _iph;
+		int nhoff = skb->encapsulation ?
+			    skb_inner_network_offset(skb) :
+			    skb_network_offset(skb);
 
-		if (!(iph->frag_off & htons(IP_DF)))
+		iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph);
+
+		if (!iph || !(iph->frag_off & htons(IP_DF)))
 			features &= ~dev->mangleid_features;
 	}
 
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH net-next 1/4] dt-bindings: net: dsa: add MT7628 ESW
From: Joris Vaisvila @ 2026-03-27 15:35 UTC (permalink / raw)
  To: Daniel Golle
  Cc: netdev, horms, pabeni, kuba, edumazet, davem, olteanv,
	Andrew Lunn, devicetree, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
In-Reply-To: <acZqvZfYXR_4sMlT@makrotopia.org>

On Fri, Mar 27, 2026 at 11:32:13AM +0000, Daniel Golle wrote:
> > [...]
> > On the hardware I'm testing on, it works fine with the port set to
> > "internal" or "rgmii". Would it make more sense to set "internal" then? 
> 
> "internal" then. It's a single-die SoC, the switch sharing the same
> memory space, clocking domain, ... with all the rest of the SoC makes
> it very unlikely that RGMII would be used as an on-die connection
> type. (unlike eg. MT7621 or MT7623A which are using multiple dies in
> the same package, and actually RGMII or TRGMII to connect the
> MDIO-managed switch part to the main SoC, see "MCM" / "multi-chip
> module" in the mt7530 driver...)

Makes sense, will change CPU port to internal in v2. 

Thanks,
Joris

^ 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