Re: [PATCH net] net: bridge: Trigger host query on v6 addr valid
From: Huang, Joseph <hidden>
Date: 2025-09-15 22:41:22
Also in:
bridge, lkml
On 9/13/2025 2:23 PM, Ido Schimmel wrote: > 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 [off-list ref] >> --- >> 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(). > It seems that inet6addr_notifier_call_chain() can be called when the address is still tentative, which means br_ip6_multicast_alloc_query() is still going to fail (br_ip6_multicast_alloc_query() calls ipv6_dev_get_saddr(), which calls __ipv6_dev_get_saddr(), which does not consider tentative source addresses). What the bridge needs really is a notification after DAD is completed, but I couldn't find such notification. Or did you mean reusing the same notification inet6addr_notifier_call_chain() but with a new event after DAD is completed? Thanks, Joseph