linux-kernel - Re: [PATCH v2 net] net: bridge: Do not offload IGMP/MLD messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aHdF-1uIp75pqfSG@shredder>
Date: Wed, 16 Jul 2025 09:26:03 +0300
From: Ido Schimmel <idosch@...dia.com>
To: Joseph Huang <Joseph.Huang@...min.com>
Cc: netdev@...r.kernel.org, Joseph Huang <joseph.huang.2024@...il.com>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Vladimir Oltean <vladimir.oltean@....com>,
	Tobias Waldekranz <tobias@...dekranz.com>,
	Florian Fainelli <f.fainelli@...il.com>, bridge@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 net] net: bridge: Do not offload IGMP/MLD messages

On Mon, Jul 14, 2025 at 11:01:00AM -0400, Joseph Huang wrote:
> Do not offload IGMP/MLD messages as it could lead to IGMP/MLD Reports
> being unintentionally flooded to Hosts. Instead, let the bridge decide
> where to send these IGMP/MLD messages.
> 
> Consider the case where the local host is sending out reports in response
> to a remote querier like the following:
> 
>        mcast-listener-process (IP_ADD_MEMBERSHIP)
>           \
>           br0
>          /   \
>       swp1   swp2
>         |     |
>   QUERIER     SOME-OTHER-HOST
> 
> In the above setup, br0 will want to br_forward() reports for
> mcast-listener-process's group(s) via swp1 to QUERIER; but since the
> source hwdom is 0, the report is eligible for tx offloading, and is
> flooded by hardware to both swp1 and swp2, reaching SOME-OTHER-HOST as
> well. (Example and illustration provided by Tobias.)
> 
> Fixes: 472111920f1c ("net: bridge: switchdev: allow the TX data plane forwarding to be offloaded")
> Signed-off-by: Joseph Huang <Joseph.Huang@...min.com>

I don't have personal experience with this offload, but it makes sense
to not offload the replication of control packets to the underlying
device and instead let the CPU handle it. These shouldn't be sent at an
high rate anyway.

> ---
> v1: https://lore.kernel.org/netdev/20250701193639.836027-1-Joseph.Huang@garmin.com/
> v2: Updated commit message.
> ---
>  net/bridge/br_switchdev.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index 95d7355a0407..757c34bf5931 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -18,7 +18,8 @@ static bool nbp_switchdev_can_offload_tx_fwd(const struct net_bridge_port *p,
>  		return false;
>  
>  	return (p->flags & BR_TX_FWD_OFFLOAD) &&
> -	       (p->hwdom != BR_INPUT_SKB_CB(skb)->src_hwdom);
> +	       (p->hwdom != BR_INPUT_SKB_CB(skb)->src_hwdom) &&
> +	       !br_multicast_igmp_type(skb);

I think you can just early return if the packet is IGMP/MLD. Something
like:

diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 95d7355a0407..9a910cf0256e 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -17,6 +17,9 @@ static bool nbp_switchdev_can_offload_tx_fwd(const struct net_bridge_port *p,
 	if (!static_branch_unlikely(&br_switchdev_tx_fwd_offload))
 		return false;
 
+	if (br_multicast_igmp_type(skb))
+		return false;
+
 	return (p->flags & BR_TX_FWD_OFFLOAD) &&
 	       (p->hwdom != BR_INPUT_SKB_CB(skb)->src_hwdom);
 }