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.

Fixes: 472111920f1c ("net: bridge: switchdev: allow the TX data plane forwarding to be offloaded")
Signed-off-by: Joseph Huang <Joseph.Huang@garmin.com>
---
 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);
 }
 
 bool br_switchdev_frame_uses_tx_fwd_offload(struct sk_buff *skb)
-- 
2.49.0

Hi Joseph,

On Tue, Jul 01, 2025 at 03:36:38PM -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.
> 
> Fixes: 472111920f1c ("net: bridge: switchdev: allow the TX data plane forwarding to be offloaded")
> Signed-off-by: Joseph Huang <Joseph.Huang@garmin.com>
> ---
>  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);
>  }
> 
>  bool br_switchdev_frame_uses_tx_fwd_offload(struct sk_buff *skb)
> --
> 2.49.0
>

Can you please incorporate the extra clarifications made to Tobias in
the commit message? They provide valuable background.

Also, keeping in mind I have no experience with IGMP/MLD snooping:
aren't there IGMP/MLD messages which should be delivered to all hosts?
Are you looking for BR_INPUT_SKB_CB_MROUTERS_ONLY(skb) as a substitute
to br_multicast_igmp_type() instead?

On 7/7/2025 9:00 AM, Vladimir Oltean wrote:
> Hi Joseph,
> 
> On Tue, Jul 01, 2025 at 03:36:38PM -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.
>> 
>> Fixes: 472111920f1c ("net: bridge: switchdev: allow the TX data plane forwarding to be offloaded")
>> Signed-off-by: Joseph Huang <Joseph.Huang@garmin.com>
>> ---
>>  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);
>>  }
>> 
>>  bool br_switchdev_frame_uses_tx_fwd_offload(struct sk_buff *skb)
>> --
>> 2.49.0
>>
> 
> Can you please incorporate the extra clarifications made to Tobias in
> the commit message? They provide valuable background.

Will do.

> 
> Also, keeping in mind I have no experience with IGMP/MLD snooping:
> aren't there IGMP/MLD messages which should be delivered to all hosts?
> Are you looking for BR_INPUT_SKB_CB_MROUTERS_ONLY(skb) as a substitute
> to br_multicast_igmp_type() instead?
> 

Yes, there are messages which should be delivered to all hosts, but if 
we offload these messages, we're at the mercy of the multicast_flood 
setting on each port and whether or not 224.0.0.1/ff02::1 are "known" 
multicast groups. (There's a discrepancy between the bridge and DSA 
subsystem on how 224.0.0.1/ff02::1 groups are handled.)

Group Specific Queries might be the only ones which are safe to offload 
regardless of what multicast_flood setting is, but I don't think the 
bridge has a flag specifically for those.

Thanks,
Joseph

On tis, jul 01, 2025 at 15:36, Joseph Huang <Joseph.Huang@garmin.com> 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.

Hi Joseph,

Do I understand the situation correctly that this is the case where the
local host is sending out reports in response to a remote querier?

       mcast-listener-process (IP_ADD_MEMBERSHIP)
          \
          br0
         /   \
      swp1   swp2
        |     |
  QUERIER     SOME-OTHER-HOST

So 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?

> Fixes: 472111920f1c ("net: bridge: switchdev: allow the TX data plane forwarding to be offloaded")
> Signed-off-by: Joseph Huang <Joseph.Huang@garmin.com>
> ---
>  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);
>  }
>  
>  bool br_switchdev_frame_uses_tx_fwd_offload(struct sk_buff *skb)
> -- 
> 2.49.0

On 7/2/2025 4:41 AM, Tobias Waldekranz wrote:
> On tis, jul 01, 2025 at 15:36, Joseph Huang <Joseph.Huang@garmin.com> 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.
> 
> Hi Joseph,
> 
> Do I understand the situation correctly that this is the case where the
> local host is sending out reports in response to a remote querier?
> 
>          mcast-listener-process (IP_ADD_MEMBERSHIP)
>             \
>             br0
>            /   \
>         swp1   swp2
>           |     |
>     QUERIER     SOME-OTHER-HOST
> 
> So 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?

That's exactly what's happening with my setup.

Also, IIRC, the querier port (a.k.a. mrouter port) is not offloaded to 
the switch (at least not for DSA switches). So depending on the 
multicast_flood setting on the (querier) port, the Reports may not even 
reach the querier if they are tx offloaded.

Thanks,
Joseph