[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aMW2lvRboW_oPyyP@shredder>
Date: Sat, 13 Sep 2025 21:23:18 +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>,
"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>,
Andrew Lunn <andrew+netdev@...n.ch>,
Nikolay Aleksandrov <razor@...ckwall.org>,
David Ahern <dsahern@...nel.org>,
Stanislav Fomichev <sdf@...ichev.me>,
Kuniyuki Iwashima <kuniyu@...gle.com>,
Ahmed Zaki <ahmed.zaki@...el.com>,
Alexander Lobakin <aleksander.lobakin@...el.com>,
linux-kernel@...r.kernel.org, bridge@...ts.linux.dev
Subject: Re: [PATCH net] net: bridge: Trigger host query on v6 addr valid
On Fri, Sep 12, 2025 at 06:39:30PM -0400, Joseph Huang wrote:
> Trigger the bridge to (re)start sending out Queries to the Host once
> IPv6 address becomes valid.
>
> In current implementation, once the bridge (interface) is brought up,
> the bridge will start trying to send v4 and v6 Queries to the Host
> immediately. However, at that time most likely the IPv6 address of
> the bridge interface is not valid yet, and thus the send (actually
> the alloc) operation will fail. So the first v6 Startup Query is
> always missed.
>
> This caused a ripple effect on the timing of Querier Election. In
> current implementation, :: always wins the election. In order for
> the "real" election to take place, the bridge would have to first
> select itself (this happens when a v6 Query is successfully sent
> to the Host), and then do the real address comparison when the next
> Query is received. In worst cast scenario, the bridge would have to
> wait for [Startup Query Interval] seconds (for the second Query to
> be sent to the Host) plus [Query Interval] seconds (for the real
> Querier to send the next Query) before it can recognize the real
> Querier.
>
> This patch adds a new notification NETDEV_NEWADDR when IPv6 address
> becomes valid. When the bridge receives the notification, it will
> restart the Startup Queries (much like how the bridge handles port
> NETDEV_CHANGE events today).
>
> Signed-off-by: Joseph Huang <Joseph.Huang@...min.com>
> ---
> include/linux/netdevice.h | 1 +
> net/bridge/br.c | 5 +++++
> net/bridge/br_multicast.c | 16 ++++++++++++++++
> net/bridge/br_private.h | 1 +
> net/core/dev.c | 10 +++++-----
> net/ipv6/addrconf.c | 3 +++
> 6 files changed, 31 insertions(+), 5 deletions(-)
A few comments:
1. The confidentiality footer needs to be removed.
2. Patches targeted at net need to have a Fixes tag. If you cannot
identify a commit before which this worked correctly (i.e., it's not a
regression), then target the patch at net-next instead.
3. The commit message needs to describe the user visible changes. My
understanding is as follows: When the bridge is brought administratively
up it will try to send a General Query which requires an IPv6 link-local
address to be configured on the bridge device. Because of DAD, such an
address might not exist right away, which means that the first General
Query will be sent after "mcast_startup_query_interval" seconds.
During this time the bridge will be unaware of multicast listeners that
joined before the creation of the bridge. Therefore, the bridge will
either unnecessarily flood multicast traffic to all the bridge ports or
just to those marked as router ports.
The patch aims to reduce this time period and send a General Query as
soon as the bridge is assigned an IPv6 link-local address.
4. Use imperative mood:
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
5. There is already a notification chain that notifies about addition /
deletion of IPv6 addresses. See register_inet6addr_notifier().
6. Please extend bridge_mld.sh with a test case in a separate patch. You
can look at xstats to see if queries were sent or not. See for example:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aea45363e29dd16050e6ce333ce0d3696ac3b5a9
Thanks
Powered by blists - more mailing lists