This is the first step to track in dedicated, per protocol family variables if we can actively and safely use multicast snooping. To later use these in the fast/data path instead of performing all these checks for every packet and to later notify DSA/switchdev about this state. This toggles these new variables to true after a Maximum Response Delay (default: 10 seconds) if a new IGMP or MLD querier has appeared. This can be triggered either through receiving an IGMP/MLD query from another host or by a user enabling our own IGMP/MLD querier. This is the first of several requirements, similar to what br_multicast_querier_exists() already checks so far, to be able to reliably receive IGMP/MLD reports, which in turn are needed to build a complete multicast database. No functional change for the fast/data path yet. Signed-off-by: Linus Lüssing --- net/bridge/br_multicast.c | 91 +++++++++++++++++++++++++++++++++++++-- net/bridge/br_private.h | 2 + 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 22d12e545966..5cc713adcf9b 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1069,6 +1069,65 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge_mcast *brm return skb; } +static bool br_ip4_multicast_querier_exists(struct net_bridge_mcast *brmctx) +{ + return __br_multicast_querier_exists(brmctx, &brmctx->ip4_other_query, false); +} + +#if IS_ENABLED(CONFIG_IPV6) +static bool br_ip6_multicast_querier_exists(struct net_bridge_mcast *brmctx) +{ + return __br_multicast_querier_exists(brmctx, &brmctx->ip6_other_query, true); +} +#endif + +static void br_ip4_multicast_update_active(struct net_bridge_mcast *brmctx, + bool force_inactive) +{ + if (force_inactive) + brmctx->ip4_active = false; + else + brmctx->ip4_active = br_ip4_multicast_querier_exists(brmctx); +} + +static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx, + bool force_inactive) +{ +#if IS_ENABLED(CONFIG_IPV6) + if (force_inactive) + brmctx->ip6_active = false; + else + brmctx->ip6_active = br_ip6_multicast_querier_exists(brmctx); +#endif +} + +/** + * br_multicast_update_active() - update mcast active state + * @brmctx: the bridge multicast context to check + * + * This (potentially) updates the IPv4/IPv6 multicast active state. And by + * that enables or disables snooping of multicast payload traffic in fast + * path. + * + * The multicast active state is set, per protocol family, if: + * + * - an IGMP/MLD querier is present + * + * And is unset otherwise. + * + * This function should be called by anything that changes one of the + * above prerequisites. + */ +static void br_multicast_update_active(struct net_bridge_mcast *brmctx) +{ + bool force_inactive = false; + + lockdep_assert_held_once(&brmctx->br->multicast_lock); + + br_ip4_multicast_update_active(brmctx, force_inactive); + br_ip6_multicast_update_active(brmctx, force_inactive); +} + #if IS_ENABLED(CONFIG_IPV6) static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge_mcast *brmctx, struct net_bridge_mcast_port *pmctx, @@ -1762,10 +1821,34 @@ static void br_ip6_multicast_querier_expired(struct timer_list *t) } #endif -static void br_multicast_query_delay_expired(struct timer_list *t) +static void br_ip4_multicast_query_delay_expired(struct timer_list *t) { + struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t, + ip4_other_query.delay_timer); + + spin_lock(&brmctx->br->multicast_lock); + /* an own or other IGMP querier appeared some seconds ago and all + * reports should have arrived by now, maybe set multicast state to active + */ + br_multicast_update_active(brmctx); + spin_unlock(&brmctx->br->multicast_lock); } +#if IS_ENABLED(CONFIG_IPV6) +static void br_ip6_multicast_query_delay_expired(struct timer_list *t) +{ + struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t, + ip6_other_query.delay_timer); + + spin_lock(&brmctx->br->multicast_lock); + /* an own or other MLD querier appeared some seconds ago and all + * reports should have arrived, maybe set multicast state to active + */ + br_multicast_update_active(brmctx); + spin_unlock(&brmctx->br->multicast_lock); +} +#endif + static void br_multicast_select_own_querier(struct net_bridge_mcast *brmctx, struct br_ip *ip, struct sk_buff *skb) @@ -4112,11 +4195,13 @@ void br_multicast_ctx_init(struct net_bridge *br, brmctx->multicast_membership_interval = 260 * HZ; brmctx->ip4_querier.port_ifidx = 0; + brmctx->ip4_active = 0; seqcount_spinlock_init(&brmctx->ip4_querier.seq, &br->multicast_lock); brmctx->multicast_igmp_version = 2; #if IS_ENABLED(CONFIG_IPV6) brmctx->multicast_mld_version = 1; brmctx->ip6_querier.port_ifidx = 0; + brmctx->ip6_active = 0; seqcount_spinlock_init(&brmctx->ip6_querier.seq, &br->multicast_lock); #endif @@ -4125,7 +4210,7 @@ void br_multicast_ctx_init(struct net_bridge *br, timer_setup(&brmctx->ip4_other_query.timer, br_ip4_multicast_querier_expired, 0); timer_setup(&brmctx->ip4_other_query.delay_timer, - br_multicast_query_delay_expired, 0); + br_ip4_multicast_query_delay_expired, 0); timer_setup(&brmctx->ip4_own_query.timer, br_ip4_multicast_query_expired, 0); #if IS_ENABLED(CONFIG_IPV6) @@ -4134,7 +4219,7 @@ void br_multicast_ctx_init(struct net_bridge *br, timer_setup(&brmctx->ip6_other_query.timer, br_ip6_multicast_querier_expired, 0); timer_setup(&brmctx->ip6_other_query.delay_timer, - br_multicast_query_delay_expired, 0); + br_ip6_multicast_query_delay_expired, 0); timer_setup(&brmctx->ip6_own_query.timer, br_ip6_multicast_query_expired, 0); #endif diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 8de0904b9627..f83c24def595 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -160,12 +160,14 @@ struct net_bridge_mcast { struct bridge_mcast_other_query ip4_other_query; struct bridge_mcast_own_query ip4_own_query; struct bridge_mcast_querier ip4_querier; + bool ip4_active; #if IS_ENABLED(CONFIG_IPV6) struct hlist_head ip6_mc_router_list; struct timer_list ip6_mc_router_timer; struct bridge_mcast_other_query ip6_other_query; struct bridge_mcast_own_query ip6_own_query; struct bridge_mcast_querier ip6_querier; + bool ip6_active; #endif /* IS_ENABLED(CONFIG_IPV6) */ #endif /* CONFIG_BRIDGE_IGMP_SNOOPING */ }; -- 2.50.1 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. No functional change for the fast/data path yet. Signed-off-by: Linus Lüssing --- net/bridge/br_multicast.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 5cc713adcf9b..2e5b5281e484 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1800,6 +1800,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); } -- 2.50.1 If we are the only potential MLD querier but don't have an IPv6 link-local address configured on our bridge interface then we can't create a valid MLD query and in turn can't reliably receive MLD reports and can't build a complete MDB. Hence disable the new multicast active state variable then. Or reenable it if an IPv6 link-local address became available. No functional change for the fast/data path yet. Signed-off-by: Linus Lüssing --- net/bridge/br_multicast.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 2e5b5281e484..b689e62b9e74 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1112,6 +1112,7 @@ static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx, * The multicast active state is set, per protocol family, if: * * - an IGMP/MLD querier is present + * - for own IPv6 MLD querier: an IPv6 address is configured on the bridge * * And is unset otherwise. * @@ -1206,10 +1207,12 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge_mcast *brm &ip6h->daddr, 0, &ip6h->saddr)) { kfree_skb(skb); br_opt_toggle(brmctx->br, BROPT_HAS_IPV6_ADDR, false); + br_multicast_update_active(brmctx); return NULL; } br_opt_toggle(brmctx->br, BROPT_HAS_IPV6_ADDR, true); + br_multicast_update_active(brmctx); ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest); hopopt = (u8 *)(ip6h + 1); -- 2.50.1 This change ensures that the new multicast active state variable is immediately unset if our internal IGMP/MLD querier was elected and now disabled. If no IGMP/MLD querier exists on the link then we can't reliably receive IGMP/MLD reports and in turn can't ensure the completeness of our MDB anymore either. No functional change for the fast/data path yet. This is the last necessary check before using the new multicast active state variable in the fast/data path, too. Signed-off-by: Linus Lüssing --- net/bridge/br_multicast.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index b689e62b9e74..13840f8f2e5f 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -4849,6 +4849,7 @@ int br_multicast_set_querier(struct net_bridge_mcast *brmctx, unsigned long val) #endif unlock: + br_multicast_update_active(brmctx); spin_unlock_bh(&brmctx->br->multicast_lock); return 0; -- 2.50.1 Export the new ip{4,6}_active variables to netlink, to be able to check from userspace that they are updated as intended. Signed-off-by: Linus Lüssing --- include/uapi/linux/if_bridge.h | 2 ++ include/uapi/linux/if_link.h | 12 ++++++++++++ net/bridge/br_netlink.c | 10 +++++++++- net/bridge/br_vlan_options.c | 10 +++++++++- net/core/rtnetlink.c | 2 +- 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h index 73876c0e2bba..c4d92df8d374 100644 --- a/include/uapi/linux/if_bridge.h +++ b/include/uapi/linux/if_bridge.h @@ -584,6 +584,8 @@ enum { BRIDGE_VLANDB_GOPTS_MCAST_ROUTER_PORTS, BRIDGE_VLANDB_GOPTS_MCAST_QUERIER_STATE, BRIDGE_VLANDB_GOPTS_MSTI, + BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V4, + BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6, __BRIDGE_VLANDB_GOPTS_MAX }; #define BRIDGE_VLANDB_GOPTS_MAX (__BRIDGE_VLANDB_GOPTS_MAX - 1) diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 784ace3a519c..fcf2c42aa6c4 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -742,6 +742,16 @@ enum in6_addr_gen_mode { * @IFLA_BR_FDB_MAX_LEARNED * Set the number of max dynamically learned FDB entries for the current * bridge. + * + * @IFLA_BR_MCAST_ACTIVE_V4 + * Bridge IPv4 mcast active state, read only. + * + * 1 if an IGMP querier is present, 0 otherwise. + * + * @IFLA_BR_MCAST_ACTIVE_V6 + * Bridge IPv6 mcast active state, read only. + * + * 1 if an MLD querier is present, 0 otherwise. */ enum { IFLA_BR_UNSPEC, @@ -794,6 +804,8 @@ enum { IFLA_BR_MCAST_QUERIER_STATE, IFLA_BR_FDB_N_LEARNED, IFLA_BR_FDB_MAX_LEARNED, + IFLA_BR_MCAST_ACTIVE_V4, + IFLA_BR_MCAST_ACTIVE_V6, __IFLA_BR_MAX, }; diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 4e2d53b27221..5aa1721830d6 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -1264,7 +1264,9 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = { [IFLA_BR_VLAN_STATS_ENABLED] = { .type = NLA_U8 }, [IFLA_BR_MCAST_STATS_ENABLED] = { .type = NLA_U8 }, [IFLA_BR_MCAST_IGMP_VERSION] = { .type = NLA_U8 }, + [IFLA_BR_MCAST_ACTIVE_V4] = { .type = NLA_REJECT }, [IFLA_BR_MCAST_MLD_VERSION] = { .type = NLA_U8 }, + [IFLA_BR_MCAST_ACTIVE_V6] = { .type = NLA_REJECT }, [IFLA_BR_VLAN_STATS_PER_PORT] = { .type = NLA_U8 }, [IFLA_BR_MULTI_BOOLOPT] = NLA_POLICY_EXACT_LEN(sizeof(struct br_boolopt_multi)), @@ -1625,7 +1627,9 @@ static size_t br_get_size(const struct net_device *brdev) nla_total_size_64bit(sizeof(u64)) + /* IFLA_BR_MCAST_QUERY_RESPONSE_INTVL */ nla_total_size_64bit(sizeof(u64)) + /* IFLA_BR_MCAST_STARTUP_QUERY_INTVL */ nla_total_size(sizeof(u8)) + /* IFLA_BR_MCAST_IGMP_VERSION */ + nla_total_size(sizeof(u8)) + /* IFLA_BR_MCAST_ACTIVE_V4 */ nla_total_size(sizeof(u8)) + /* IFLA_BR_MCAST_MLD_VERSION */ + nla_total_size(sizeof(u8)) + /* IFLA_BR_MCAST_ACTIVE_V6 */ br_multicast_querier_state_size() + /* IFLA_BR_MCAST_QUERIER_STATE */ #endif #if IS_ENABLED(CONFIG_BRIDGE_NETFILTER) @@ -1717,12 +1721,16 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev) br->multicast_ctx.multicast_startup_query_count) || nla_put_u8(skb, IFLA_BR_MCAST_IGMP_VERSION, br->multicast_ctx.multicast_igmp_version) || + nla_put_u8(skb, IFLA_BR_MCAST_ACTIVE_V4, + br->multicast_ctx.ip4_active) || br_multicast_dump_querier_state(skb, &br->multicast_ctx, IFLA_BR_MCAST_QUERIER_STATE)) return -EMSGSIZE; #if IS_ENABLED(CONFIG_IPV6) if (nla_put_u8(skb, IFLA_BR_MCAST_MLD_VERSION, - br->multicast_ctx.multicast_mld_version)) + br->multicast_ctx.multicast_mld_version) || + nla_put_u8(skb, IFLA_BR_MCAST_ACTIVE_V6, + br->multicast_ctx.ip6_active)) return -EMSGSIZE; #endif clockval = jiffies_to_clock_t(br->multicast_ctx.multicast_last_member_interval); diff --git a/net/bridge/br_vlan_options.c b/net/bridge/br_vlan_options.c index 8fa89b04ee94..33f055d3a304 100644 --- a/net/bridge/br_vlan_options.c +++ b/net/bridge/br_vlan_options.c @@ -369,6 +369,8 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range, !!(v_opts->priv_flags & BR_VLFLAG_GLOBAL_MCAST_ENABLED)) || nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION, v_opts->br_mcast_ctx.multicast_igmp_version) || + nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V4, + v_opts->br_mcast_ctx.ip4_active) || nla_put_u32(skb, BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_CNT, v_opts->br_mcast_ctx.multicast_last_member_count) || nla_put_u32(skb, BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_CNT, @@ -423,7 +425,9 @@ bool br_vlan_global_opts_fill(struct sk_buff *skb, u16 vid, u16 vid_range, #if IS_ENABLED(CONFIG_IPV6) if (nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_MLD_VERSION, - v_opts->br_mcast_ctx.multicast_mld_version)) + v_opts->br_mcast_ctx.multicast_mld_version) || + nla_put_u8(skb, BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6, + v_opts->br_mcast_ctx.ip4_active)) goto out_err; #endif #endif @@ -448,7 +452,9 @@ static size_t rtnl_vlan_global_opts_nlmsg_size(const struct net_bridge_vlan *v) #ifdef CONFIG_BRIDGE_IGMP_SNOOPING + nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_SNOOPING */ + nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION */ + + nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V4 */ + nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_MLD_VERSION */ + + nla_total_size(sizeof(u8)) /* BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6 */ + nla_total_size(sizeof(u32)) /* BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_CNT */ + nla_total_size(sizeof(u32)) /* BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_CNT */ + nla_total_size(sizeof(u64)) /* BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_INTVL */ @@ -630,9 +636,11 @@ static const struct nla_policy br_vlan_db_gpol[BRIDGE_VLANDB_GOPTS_MAX + 1] = { [BRIDGE_VLANDB_GOPTS_RANGE] = { .type = NLA_U16 }, [BRIDGE_VLANDB_GOPTS_MCAST_SNOOPING] = { .type = NLA_U8 }, [BRIDGE_VLANDB_GOPTS_MCAST_MLD_VERSION] = { .type = NLA_U8 }, + [BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V6] = { .type = NLA_REJECT }, [BRIDGE_VLANDB_GOPTS_MCAST_QUERY_INTVL] = { .type = NLA_U64 }, [BRIDGE_VLANDB_GOPTS_MCAST_QUERIER] = { .type = NLA_U8 }, [BRIDGE_VLANDB_GOPTS_MCAST_IGMP_VERSION] = { .type = NLA_U8 }, + [BRIDGE_VLANDB_GOPTS_MCAST_ACTIVE_V4] = { .type = NLA_REJECT }, [BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_CNT] = { .type = NLA_U32 }, [BRIDGE_VLANDB_GOPTS_MCAST_STARTUP_QUERY_CNT] = { .type = NLA_U32 }, [BRIDGE_VLANDB_GOPTS_MCAST_LAST_MEMBER_INTVL] = { .type = NLA_U64 }, diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 094b085cff20..cb24370b719f 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -62,7 +62,7 @@ #include "dev.h" -#define RTNL_MAX_TYPE 50 +#define RTNL_MAX_TYPE 52 #define RTNL_SLAVE_MAX_TYPE 44 struct rtnl_link { -- 2.50.1 As the multicast active state variable is now always up to date and functionally equivalent to our manual, extensive checks in fast path we can just use this state variable in fast path, too. This allows to save some CPU cycles for every multicast packet in the fast/data path. Next to using brmctx->ip4_active / brmctx->ip6_active in fast path this mostly just moves some code around to not expose it via br_private.h anymore. While at it now also passing the ethernet protocol number directly, instead of a pointer into the ethernet header. Signed-off-by: Linus Lüssing --- net/bridge/br_device.c | 2 +- net/bridge/br_input.c | 2 +- net/bridge/br_multicast.c | 40 ++++++++++++++++++++++++++++++++++----- net/bridge/br_private.h | 38 ++++++++----------------------------- 4 files changed, 45 insertions(+), 37 deletions(-) diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index a818fdc22da9..3cdf1c17108b 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -102,7 +102,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev) mdst = br_mdb_entry_skb_get(brmctx, skb, vid); if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && - br_multicast_querier_exists(brmctx, eth_hdr(skb), mdst)) + br_multicast_snooping_active(brmctx, eth_hdr(skb)->h_proto, mdst)) br_multicast_flood(mdst, skb, brmctx, false, true); else br_flood(br, skb, BR_PKT_MULTICAST, false, true, vid); diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index 5f6ac9bf1527..dfd49b309683 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -187,7 +187,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb case BR_PKT_MULTICAST: mdst = br_mdb_entry_skb_get(brmctx, skb, vid); if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && - br_multicast_querier_exists(brmctx, eth_hdr(skb), mdst)) { + br_multicast_snooping_active(brmctx, eth_hdr(skb)->h_proto, mdst)) { if ((mdst && mdst->host_joined) || br_multicast_is_router(brmctx, skb) || br->dev->flags & IFF_ALLMULTI) { diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 13840f8f2e5f..54163c74b9a9 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1069,6 +1069,26 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge_mcast *brm return skb; } +static bool +__br_multicast_querier_exists(struct net_bridge_mcast *brmctx, + struct bridge_mcast_other_query *querier, + bool is_ipv6) +{ + bool own_querier_enabled; + + if (brmctx->multicast_querier) { + if (is_ipv6 && !br_opt_get(brmctx->br, BROPT_HAS_IPV6_ADDR)) + own_querier_enabled = false; + else + own_querier_enabled = true; + } else { + own_querier_enabled = false; + } + + return !timer_pending(&querier->delay_timer) && + (own_querier_enabled || timer_pending(&querier->timer)); +} + static bool br_ip4_multicast_querier_exists(struct net_bridge_mcast *brmctx) { return __br_multicast_querier_exists(brmctx, &brmctx->ip4_other_query, false); @@ -1081,6 +1101,20 @@ static bool br_ip6_multicast_querier_exists(struct net_bridge_mcast *brmctx) } #endif +static bool br_multicast_querier_exists(struct net_bridge_mcast *brmctx, int proto) +{ + switch (proto) { + case (ETH_P_IP): + return br_ip4_multicast_querier_exists(brmctx); +#if IS_ENABLED(CONFIG_IPV6) + case (ETH_P_IPV6): + return br_ip6_multicast_querier_exists(brmctx); +#endif + default: + return false; + } +} + static void br_ip4_multicast_update_active(struct net_bridge_mcast *brmctx, bool force_inactive) { @@ -5013,7 +5047,6 @@ bool br_multicast_has_querier_anywhere(struct net_device *dev, int proto) { struct net_bridge *br; struct net_bridge_port *port; - struct ethhdr eth; bool ret = false; rcu_read_lock(); @@ -5026,10 +5059,7 @@ bool br_multicast_has_querier_anywhere(struct net_device *dev, int proto) br = port->br; - memset(ð, 0, sizeof(eth)); - eth.h_proto = htons(proto); - - ret = br_multicast_querier_exists(&br->multicast_ctx, ð, NULL); + ret = br_multicast_querier_exists(&br->multicast_ctx, proto); unlock: rcu_read_unlock(); diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index f83c24def595..05650af596ab 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -1154,37 +1154,15 @@ br_multicast_is_router(struct net_bridge_mcast *brmctx, struct sk_buff *skb) } static inline bool -__br_multicast_querier_exists(struct net_bridge_mcast *brmctx, - struct bridge_mcast_other_query *querier, - const bool is_ipv6) +br_multicast_snooping_active(struct net_bridge_mcast *brmctx, __be16 eth_proto, + const struct net_bridge_mdb_entry *mdb) { - bool own_querier_enabled; - - if (brmctx->multicast_querier) { - if (is_ipv6 && !br_opt_get(brmctx->br, BROPT_HAS_IPV6_ADDR)) - own_querier_enabled = false; - else - own_querier_enabled = true; - } else { - own_querier_enabled = false; - } - - return !timer_pending(&querier->delay_timer) && - (own_querier_enabled || timer_pending(&querier->timer)); -} - -static inline bool br_multicast_querier_exists(struct net_bridge_mcast *brmctx, - struct ethhdr *eth, - const struct net_bridge_mdb_entry *mdb) -{ - switch (eth->h_proto) { + switch (eth_proto) { case (htons(ETH_P_IP)): - return __br_multicast_querier_exists(brmctx, - &brmctx->ip4_other_query, false); + return brmctx->ip4_active; #if IS_ENABLED(CONFIG_IPV6) case (htons(ETH_P_IPV6)): - return __br_multicast_querier_exists(brmctx, - &brmctx->ip6_other_query, true); + return brmctx->ip6_active; #endif default: return !!mdb && br_group_is_l2(&mdb->addr); @@ -1439,9 +1417,9 @@ static inline bool br_multicast_is_router(struct net_bridge_mcast *brmctx, return false; } -static inline bool br_multicast_querier_exists(struct net_bridge_mcast *brmctx, - struct ethhdr *eth, - const struct net_bridge_mdb_entry *mdb) +static inline bool +br_multicast_snooping_active(struct net_bridge_mcast *brmctx, __be16 eth_proto, + const struct net_bridge_mdb_entry *mdb) { return false; } -- 2.50.1 To be able to use the upcoming SWITCHDEV_ATTR_ID_BRIDGE_MC_ACTIVE as a potential replacement for SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED also check and toggle the active state if multicast snooping is enabled or disabled. So that MC_ACTIVE not only checks the querier state, but also if multicast snooping is enabled in general. No functional change for the fast/data path yet. Signed-off-by: Linus Lüssing --- include/uapi/linux/if_link.h | 6 ++++-- net/bridge/br_multicast.c | 21 +++++++++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index fcf2c42aa6c4..ef686ea17afe 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -746,12 +746,14 @@ enum in6_addr_gen_mode { * @IFLA_BR_MCAST_ACTIVE_V4 * Bridge IPv4 mcast active state, read only. * - * 1 if an IGMP querier is present, 0 otherwise. + * 1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an IGMP querier is present, + * 0 otherwise. * * @IFLA_BR_MCAST_ACTIVE_V6 * Bridge IPv6 mcast active state, read only. * - * 1 if an MLD querier is present, 0 otherwise. + * 1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an MLD querier is present, + * 0 otherwise. */ enum { IFLA_BR_UNSPEC, diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 54163c74b9a9..53720337a1e3 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1145,6 +1145,7 @@ static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx, * * The multicast active state is set, per protocol family, if: * + * - multicast snooping is enabled * - an IGMP/MLD querier is present * - for own IPv6 MLD querier: an IPv6 address is configured on the bridge * @@ -1159,6 +1160,13 @@ static void br_multicast_update_active(struct net_bridge_mcast *brmctx) lockdep_assert_held_once(&brmctx->br->multicast_lock); + if (!br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED)) + force_inactive = true; + + if (br_opt_get(brmctx->br, BROPT_MCAST_VLAN_SNOOPING_ENABLED) && + br_multicast_ctx_vlan_disabled(brmctx)) + force_inactive = true; + br_ip4_multicast_update_active(brmctx, force_inactive); br_ip6_multicast_update_active(brmctx, force_inactive); } @@ -1371,6 +1379,12 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx, return NULL; } +static void br_multicast_toggle_enabled(struct net_bridge *br, bool on) +{ + br_opt_toggle(br, BROPT_MULTICAST_ENABLED, on); + br_multicast_update_active(&br->multicast_ctx); +} + struct net_bridge_mdb_entry *br_multicast_new_group(struct net_bridge *br, struct br_ip *group) { @@ -1384,7 +1398,7 @@ struct net_bridge_mdb_entry *br_multicast_new_group(struct net_bridge *br, if (atomic_read(&br->mdb_hash_tbl.nelems) >= br->hash_max) { trace_br_mdb_full(br->dev, group); br_mc_disabled_update(br->dev, false, NULL); - br_opt_toggle(br, BROPT_MULTICAST_ENABLED, false); + br_multicast_toggle_enabled(br, false); return ERR_PTR(-E2BIG); } @@ -4452,6 +4466,7 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on) spin_lock_bh(&br->multicast_lock); vlan->priv_flags ^= BR_VLFLAG_MCAST_ENABLED; + br_multicast_update_active(&vlan->br_mcast_ctx); spin_unlock_bh(&br->multicast_lock); if (on) @@ -4472,6 +4487,7 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on) __br_multicast_enable_port_ctx(&vlan->port_mcast_ctx); else __br_multicast_disable_port_ctx(&vlan->port_mcast_ctx); + br_multicast_update_active(brmctx); spin_unlock_bh(&br->multicast_lock); } } @@ -4792,7 +4808,8 @@ int br_multicast_toggle(struct net_bridge *br, unsigned long val, if (err) goto unlock; - br_opt_toggle(br, BROPT_MULTICAST_ENABLED, !!val); + br_multicast_toggle_enabled(br, !!val); + if (!br_opt_get(br, BROPT_MULTICAST_ENABLED)) { change_snoopers = true; goto unlock; -- 2.50.1 This is mainly for switchdev and DSA later: To ensure that we switch to inactive before destroying a bridge interface. A switchdev/DSA driver might have allocated resources after we switched to an enabled multicast active state. This gives switchdev/DSA drivers a chance to free these resources again when we destroy the bridge (later). Putting it into the ndo_stop / bridge interface down part instead of the ndo_uninit / bridge destroy part though for a better semantic match. If the bridge interface is down / stopped then it is also inactive. No functional change for the fast/data path. Signed-off-by: Linus Lüssing --- include/uapi/linux/if_link.h | 8 ++++---- net/bridge/br_device.c | 3 +++ net/bridge/br_multicast.c | 26 ++++++++++++++++++++++---- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index ef686ea17afe..76d15a07344d 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -746,14 +746,14 @@ enum in6_addr_gen_mode { * @IFLA_BR_MCAST_ACTIVE_V4 * Bridge IPv4 mcast active state, read only. * - * 1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an IGMP querier is present, - * 0 otherwise. + * 1 if *IFLA_BR_MCAST_SNOOPING* is enabled, an IGMP querier is present + * and the bridge interface is up, 0 otherwise. * * @IFLA_BR_MCAST_ACTIVE_V6 * Bridge IPv6 mcast active state, read only. * - * 1 if *IFLA_BR_MCAST_SNOOPING* is enabled and an MLD querier is present, - * 0 otherwise. + * 1 if *IFLA_BR_MCAST_SNOOPING* is enabled, an MLD querier is present + * and the bridge interface is up, 0 otherwise. */ enum { IFLA_BR_UNSPEC, diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index 3cdf1c17108b..efb5d35c2dd4 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -168,7 +168,10 @@ static int br_dev_open(struct net_device *dev) netdev_update_features(dev); netif_start_queue(dev); br_stp_enable_bridge(br); + + spin_lock_bh(&br->multicast_lock); br_multicast_open(br); + spin_unlock_bh(&br->multicast_lock); if (br_opt_get(br, BROPT_MULTICAST_ENABLED)) br_multicast_join_snoopers(br); diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 53720337a1e3..09e23e4d8b74 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1145,6 +1145,7 @@ static void br_ip6_multicast_update_active(struct net_bridge_mcast *brmctx, * * The multicast active state is set, per protocol family, if: * + * - the bridge interface is up * - multicast snooping is enabled * - an IGMP/MLD querier is present * - for own IPv6 MLD querier: an IPv6 address is configured on the bridge @@ -1160,6 +1161,9 @@ static void br_multicast_update_active(struct net_bridge_mcast *brmctx) lockdep_assert_held_once(&brmctx->br->multicast_lock); + if (!netif_running(brmctx->br->dev)) + force_inactive = true; + if (!br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED)) force_inactive = true; @@ -4379,6 +4383,9 @@ static void __br_multicast_open(struct net_bridge_mcast *brmctx) #if IS_ENABLED(CONFIG_IPV6) __br_multicast_open_query(brmctx->br, &brmctx->ip6_own_query); #endif + + /* bridge interface is up, maybe set multicast state to active */ + br_multicast_update_active(brmctx); } void br_multicast_open(struct net_bridge *br) @@ -4417,6 +4424,11 @@ static void __br_multicast_stop(struct net_bridge_mcast *brmctx) timer_delete_sync(&brmctx->ip6_other_query.delay_timer); timer_delete_sync(&brmctx->ip6_own_query.timer); #endif + + spin_lock_bh(&brmctx->br->multicast_lock); + /* bridge interface is down, set multicast state to inactive */ + br_multicast_update_active(brmctx); + spin_unlock_bh(&brmctx->br->multicast_lock); } void br_multicast_update_vlan_mcast_ctx(struct net_bridge_vlan *v, u8 state) @@ -4469,10 +4481,13 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on) br_multicast_update_active(&vlan->br_mcast_ctx); spin_unlock_bh(&br->multicast_lock); - if (on) + if (on) { + spin_lock_bh(&br->multicast_lock); __br_multicast_open(&vlan->br_mcast_ctx); - else + spin_unlock_bh(&br->multicast_lock); + } else { __br_multicast_stop(&vlan->br_mcast_ctx); + } } else { struct net_bridge_mcast *brmctx; @@ -4534,10 +4549,13 @@ int br_multicast_toggle_vlan_snooping(struct net_bridge *br, bool on, br_opt_toggle(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED, on); /* disable/enable non-vlan mcast contexts based on vlan snooping */ - if (on) + if (on) { __br_multicast_stop(&br->multicast_ctx); - else + } else { + spin_lock_bh(&br->multicast_lock); __br_multicast_open(&br->multicast_ctx); + spin_unlock_bh(&br->multicast_lock); + } list_for_each_entry(p, &br->port_list, list) { if (on) br_multicast_disable_port_ctx(&p->multicast_ctx); -- 2.50.1 To avoid packetloss and as it is very hard from a user's perspective to debug multicast snooping related issues it is even more crucial to properly switch from an active to an inactive multicast snooping state than the other way around. Therefore adding a few kernel warnings if any of our assertions to be in an inactive state would fail. Signed-off-by: Linus Lüssing --- net/bridge/br_multicast.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c index 09e23e4d8b74..46161e707fc5 100644 --- a/net/bridge/br_multicast.c +++ b/net/bridge/br_multicast.c @@ -1383,10 +1383,29 @@ static struct sk_buff *br_multicast_alloc_query(struct net_bridge_mcast *brmctx, return NULL; } +static void br_ip4_multicast_assert_inactive(struct net_bridge_mcast *brmctx) +{ + WARN_ON(br_multicast_snooping_active(brmctx, htons(ETH_P_IP), NULL)); +} + +static void br_ip6_multicast_assert_inactive(struct net_bridge_mcast *brmctx) +{ + WARN_ON(br_multicast_snooping_active(brmctx, htons(ETH_P_IPV6), NULL)); +} + +static void br_multicast_assert_inactive(struct net_bridge_mcast *brmctx) +{ + br_ip4_multicast_assert_inactive(brmctx); + br_ip6_multicast_assert_inactive(brmctx); +} + static void br_multicast_toggle_enabled(struct net_bridge *br, bool on) { br_opt_toggle(br, BROPT_MULTICAST_ENABLED, on); br_multicast_update_active(&br->multicast_ctx); + + if (!on) + br_multicast_assert_inactive(&br->multicast_ctx); } struct net_bridge_mdb_entry *br_multicast_new_group(struct net_bridge *br, @@ -1846,7 +1865,6 @@ static void br_ip6_multicast_local_router_expired(struct timer_list *t) static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx, struct bridge_mcast_own_query *query) { - spin_lock(&brmctx->br->multicast_lock); if (!netif_running(brmctx->br->dev) || br_multicast_ctx_vlan_global_disabled(brmctx) || !br_opt_get(brmctx->br, BROPT_MULTICAST_ENABLED)) @@ -1859,7 +1877,6 @@ static void br_multicast_querier_expired(struct net_bridge_mcast *brmctx, * if our own querier is disabled, too */ br_multicast_update_active(brmctx); - spin_unlock(&brmctx->br->multicast_lock); } static void br_ip4_multicast_querier_expired(struct timer_list *t) @@ -1867,7 +1884,12 @@ static void br_ip4_multicast_querier_expired(struct timer_list *t) struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t, ip4_other_query.timer); + spin_lock(&brmctx->br->multicast_lock); br_multicast_querier_expired(brmctx, &brmctx->ip4_own_query); + + if (!brmctx->multicast_querier) + br_ip4_multicast_assert_inactive(brmctx); + spin_unlock(&brmctx->br->multicast_lock); } #if IS_ENABLED(CONFIG_IPV6) @@ -1876,7 +1898,12 @@ static void br_ip6_multicast_querier_expired(struct timer_list *t) struct net_bridge_mcast *brmctx = timer_container_of(brmctx, t, ip6_other_query.timer); + spin_lock(&brmctx->br->multicast_lock); br_multicast_querier_expired(brmctx, &brmctx->ip6_own_query); + + if (!brmctx->multicast_querier) + br_ip6_multicast_assert_inactive(brmctx); + spin_unlock(&brmctx->br->multicast_lock); } #endif @@ -4428,6 +4455,7 @@ static void __br_multicast_stop(struct net_bridge_mcast *brmctx) spin_lock_bh(&brmctx->br->multicast_lock); /* bridge interface is down, set multicast state to inactive */ br_multicast_update_active(brmctx); + br_multicast_assert_inactive(brmctx); spin_unlock_bh(&brmctx->br->multicast_lock); } @@ -4479,6 +4507,8 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on) spin_lock_bh(&br->multicast_lock); vlan->priv_flags ^= BR_VLFLAG_MCAST_ENABLED; br_multicast_update_active(&vlan->br_mcast_ctx); + if (!on) + br_multicast_assert_inactive(&vlan->br_mcast_ctx); spin_unlock_bh(&br->multicast_lock); if (on) { @@ -4503,6 +4533,8 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on) else __br_multicast_disable_port_ctx(&vlan->port_mcast_ctx); br_multicast_update_active(brmctx); + if (!on) + br_multicast_assert_inactive(brmctx); spin_unlock_bh(&br->multicast_lock); } } -- 2.50.1