Re: [net-next,v2,05/14] net: bridge: mcast: track active state, foreign IGMP/MLD querier disappearance

Jakub Kicinski <[email protected]>
Newsgroups gmane.linux.network.bridge,gmane.linux.network,gmane.linux.kernel
Message-ID <[email protected]>
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
---
net: bridge: mcast: track active state, foreign IGMP/MLD querier disappearance

This change ensures that the new multicast active state variable is unset
again after a foreign IGMP/MLD querier has disappeared (default: 255
seconds). If no new, other IGMP/MLD querier took over then we can't
reliably receive IGMP/MLD reports anymore and in turn can't ensure the
completeness of our MDB anymore either.

> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index d5c623dce7eb..0fc29875db9c 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1826,6 +1826,10 @@ static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx,
>  	br_multicast_start_querier(brmctx, query);
>
>  out:
> +	/* another IGMP/MLD querier disappeared, set multicast state to inactive
> +	 * if our own querier is disabled, too
> +	 */
> +	br_multicast_update_active(brmctx);
>  	spin_unlock(&brmctx->br->multicast_lock);
>  }

This isn't a bug, but I noticed a couple of unaddressed review comments
from Ido Schimmel's feedback on v1 back in May 2025 that don't appear to
have been incorporated into this v2 series.

In __br_multicast_querier_exists() in net/bridge/br_private.h around line
1164, the 'is_ipv6' bool parameter is marked const. Since this is a
pass-by-value parameter, the const qualifier doesn't provide any benefit.
Should it be removed?

Reference: https://lore.kernel.org/all/aDNPxdmDPYwasUEM@shredder/

Also in br_multicast_querier_exists() in net/bridge/br_private.h around
line 1182, the function takes a struct ethhdr *eth parameter but only uses
eth->h_proto. Would it make more sense to either make the 'eth' parameter
const, or just pass the EtherType (__be16) directly instead of the entire
struct pointer?

Reference: https://lore.kernel.org/all/aDNPxdmDPYwasUEM@shredder/
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.