Re: [PATCH net v2] bridge: mcast: fix disabled snooping after long uptime
On 27/01/2024 19:50, Linus Lüssing wrote:
> The original idea of the delay_time check was to not apply multicast
> snooping too early when an MLD querier appears. And to instead wait at
> least for MLD reports to arrive before switching from flooding to group
> based, MLD snooped forwarding, to avoid temporary packet loss.
>
> However in a batman-adv mesh network it was noticed that after 248 days of
> uptime 32bit MIPS based devices would start to signal that they had
> stopped applying multicast snooping due to missing queriers - even though
> they were the elected querier and still sending MLD queries themselves.
>
> While time_is_before_jiffies() generally is safe against jiffies
> wrap-arounds, like the code comments in jiffies.h explain, it won't
> be able to track a difference larger than ULONG_MAX/2. With a 32bit
> large jiffies and one jiffies tick every 10ms (CONFIG_HZ=100) on these MIPS
> devices running OpenWrt this would result in a difference larger than
> ULONG_MAX/2 after 248 (= 2^32/100/60/60/24/2) days and
> time_is_before_jiffies() would then start to return false instead of
> true. Leading to multicast snooping not being applied to multicast
> packets anymore.
>
> Fix this issue by using a proper timer_list object which won't have this
> ULONG_MAX/2 difference limitation.
>
> Fixes: b00589af3b04 ("bridge: disable snooping if there is no querier")
> Signed-off-by: Linus Lüssing
> ---
> Changelog v2:
> * removed "inline" from br_multicast_query_delay_expired()
>
> net/bridge/br_multicast.c | 20 +++-----
> net/bridge/br_private.h | 4 ++--
> 2 files changed, 17 insertions(+), 7 deletions(-)
>
Acked-by: Nikolay Aleksandrov
Re: [PATCH v3 net-next] net: make use of helper netif_is_bridge_master()
The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software.--- Begin Message --- On 16/10/2021 14:21, Kyungrok Chung wrote: > Make use of netdev helper functions to improve code readability. > Replace 'dev->priv_flags & IFF_EBRIDGE' with netif_is_bridge_master(dev). > > Signed-off-by: Kyungrok Chung > --- > > v1->v2: > - Apply fixes to batman-adv, core too. > > v2->v3: > - Fix wrong logic. > > net/batman-adv/multicast.c | 2 +- > net/bridge/br.c | 4 ++-- > net/bridge/br_fdb.c | 6 +++--- > net/bridge/br_if.c | 2 +- > net/bridge/br_ioctl.c | 2 +- > net/bridge/br_mdb.c | 4 ++-- > net/bridge/br_netfilter_hooks.c | 2 +- > net/bridge/br_netlink.c | 4 ++-- > net/core/rtnetlink.c | 2 +- > 9 files changed, 14 insertions(+), 14 deletions(-) > LGTM, Reviewed-by: Nikolay Aleksandrov --- End Message ---
Re: [net-next v2 10/11] net: bridge: mcast: add ip4+ip6 mcast router timers to mdb netlink
On 09/05/2021 22:45, Linus Lüssing wrote:
> Now that we have split the multicast router state into two, one for IPv4
> and one for IPv6, also add individual timers to the mdb netlink router
> port dump. Leaving the old timer attribute for backwards compatibility.
>
> Signed-off-by: Linus Lüssing
> ---
> include/uapi/linux/if_bridge.h | 2 ++
> net/bridge/br_mdb.c| 8 +++-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
Acked-by: Nikolay Aleksandrov
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 13d59c5..6b56a75 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -627,6 +627,8 @@ enum {
> MDBA_ROUTER_PATTR_UNSPEC,
> MDBA_ROUTER_PATTR_TIMER,
> MDBA_ROUTER_PATTR_TYPE,
> + MDBA_ROUTER_PATTR_INET_TIMER,
> + MDBA_ROUTER_PATTR_INET6_TIMER,
> __MDBA_ROUTER_PATTR_MAX
> };
> #define MDBA_ROUTER_PATTR_MAX (__MDBA_ROUTER_PATTR_MAX - 1)
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index 3c608da..2cdd9b6 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -79,7 +79,13 @@ static int br_rports_fill_info(struct sk_buff *skb, struct
> netlink_callback *cb,
> nla_put_u32(skb, MDBA_ROUTER_PATTR_TIMER,
> max(ip4_timer, ip6_timer)) ||
> nla_put_u8(skb, MDBA_ROUTER_PATTR_TYPE,
> -p->multicast_router)) {
> +p->multicast_router) ||
> + (have_ip4_mc_rtr &&
> + nla_put_u32(skb, MDBA_ROUTER_PATTR_INET_TIMER,
> + ip4_timer)) ||
> + (have_ip6_mc_rtr &&
> + nla_put_u32(skb, MDBA_ROUTER_PATTR_INET6_TIMER,
> + ip6_timer))) {
> nla_nest_cancel(skb, port_nest);
> goto fail;
> }
>
Re: [net-next v2 09/11] net: bridge: mcast: split multicast router state for IPv4 and IPv6
On 11/05/2021 12:29, Nikolay Aleksandrov wrote:
> On 09/05/2021 22:45, Linus Lüssing wrote:
>> A multicast router for IPv4 does not imply that the same host also is a
>> multicast router for IPv6 and vice versa.
>>
>> To reduce multicast traffic when a host is only a multicast router for
>> one of these two protocol families, keep router state for IPv4 and IPv6
>> separately. Similar to how querier state is kept separately.
>>
>> For backwards compatibility for netlink and switchdev notifications
>> these two will still only notify if a port switched from either no
>> IPv4/IPv6 multicast router to any IPv4/IPv6 multicast router or the
>> other way round. However a full netlink MDB router dump will now also
>> include a multicast router timeout for both IPv4 and IPv6.
>>
>> Signed-off-by: Linus Lüssing
>> ---
>> net/bridge/br_forward.c | 8 ++
>> net/bridge/br_mdb.c | 10 ++
>> net/bridge/br_multicast.c | 197 ++
>> net/bridge/br_private.h | 6 +-
>> 4 files changed, 201 insertions(+), 20 deletions(-)
[snip]
>> +#else
>> +static inline void br_ip6_multicast_add_router(struct net_bridge *br,
>> + struct net_bridge_port *port)
>> +{
>> +}
>
> Actually that goes for multicast_add_router, too.
>
err, my bad - multicast_add_router is fine as is, sorry about that
> I'm saying all this because soon I'll be adding per-vlan multicast router
> support
> and these will be reusable there without any modification if they can take
> any list.
> Also it'll be easier to maintain one set of functions instead of multiple
> identical ones.
>
Re: [net-next v2 11/11] net: bridge: mcast: export multicast router presence adjacent to a port
On 09/05/2021 22:45, Linus Lüssing wrote:
> To properly support routable multicast addresses in batman-adv in a
> group-aware way, a batman-adv node needs to know if it serves multicast
> routers.
>
> This adds a function to the bridge to export this so that batman-adv
> can then make full use of the Multicast Router Discovery capability of
> the bridge.
>
> Signed-off-by: Linus Lüssing
> ---
> net/bridge/br_multicast.c | 58 +++
> 1 file changed, 58 insertions(+)
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index b625fd6..e963de5 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -4061,6 +4061,64 @@ unlock:
> }
> EXPORT_SYMBOL_GPL(br_multicast_has_querier_adjacent);
>
> +/**
> + * br_multicast_has_router_adjacent - Checks for a router behind a bridge
> port
> + * @dev: The bridge port adjacent to which to check for a multicast router
> + * @proto: The protocol family to check for: IGMP -> ETH_P_IP, MLD ->
> ETH_P_IPV6
> + *
> + * Checks whether the given interface has a bridge on top and if so returns
> + * true if a multicast router is behind one of the other ports of this
> + * bridge. Otherwise returns false.
> + */
> +bool br_multicast_has_router_adjacent(struct net_device *dev, int proto)
> +{
> + struct net_bridge_port *port, *p;
> + bool ret = false;
> +
> + rcu_read_lock();
> + if (!netif_is_bridge_port(dev))
> + goto unlock;
> +
> + port = br_port_get_rcu(dev);
You can combine both of netif_is_bridge_port and br_port_get_rcu() checks and
use
br_port_get_check_rcu(). Then you can also drop the port->br check.
> + if (!port || !port->br)
> + goto unlock;
> +
> + switch (proto) {
> + case ETH_P_IP:
> + hlist_for_each_entry_rcu(p, >br->ip4_mc_router_list,
> + ip4_rlist) {
> + if (p == port)
> + continue;
> +
> + ret = true;
> + goto unlock;
> + }
> + break;
> +#if IS_ENABLED(CONFIG_IPV6)
> + case ETH_P_IPV6:
> + hlist_for_each_entry_rcu(p, >br->ip6_mc_router_list,
> + ip6_rlist) {
> + if (p == port)
> + continue;
> +
> + ret = true;
> + goto unlock;
> + }
> + break;
> +#endif
> + default:
> + /* when compiled without IPv6 support, be conservative and
> + * always assume presence of an IPv6 multicast router
> + */
> + ret = true;
> + }
> +
> +unlock:
> + rcu_read_unlock();
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(br_multicast_has_router_adjacent);
> +
> static void br_mcast_stats_add(struct bridge_mcast_stats __percpu *stats,
> const struct sk_buff *skb, u8 type, u8 dir)
> {
>
Re: [net-next v2 09/11] net: bridge: mcast: split multicast router state for IPv4 and IPv6
On 09/05/2021 22:45, Linus Lüssing wrote:
> A multicast router for IPv4 does not imply that the same host also is a
> multicast router for IPv6 and vice versa.
>
> To reduce multicast traffic when a host is only a multicast router for
> one of these two protocol families, keep router state for IPv4 and IPv6
> separately. Similar to how querier state is kept separately.
>
> For backwards compatibility for netlink and switchdev notifications
> these two will still only notify if a port switched from either no
> IPv4/IPv6 multicast router to any IPv4/IPv6 multicast router or the
> other way round. However a full netlink MDB router dump will now also
> include a multicast router timeout for both IPv4 and IPv6.
>
> Signed-off-by: Linus Lüssing
> ---
> net/bridge/br_forward.c | 8 ++
> net/bridge/br_mdb.c | 10 ++
> net/bridge/br_multicast.c | 197 ++
> net/bridge/br_private.h | 6 +-
> 4 files changed, 201 insertions(+), 20 deletions(-)
>
> diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> index b5ec4f9..31a02c5 100644
> --- a/net/bridge/br_forward.c
> +++ b/net/bridge/br_forward.c
> @@ -266,11 +266,19 @@ static void maybe_deliver_addr(struct net_bridge_port
> *p, struct sk_buff *skb,
>
> static inline struct hlist_node *
> br_multicast_get_first_rport_node(struct net_bridge *b, struct sk_buff *skb)
> {
> +#if IS_ENABLED(CONFIG_IPV6)
> + if (skb->protocol == htons(ETH_P_IPV6))
> + return rcu_dereference(hlist_first_rcu(>ip6_mc_router_list));
> +#endif
> return rcu_dereference(hlist_first_rcu(>ip4_mc_router_list));
> }
>
> static inline struct net_bridge_port *
> br_multicast_rport_from_node(struct hlist_node *rp, struct sk_buff *skb) {
> +#if IS_ENABLED(CONFIG_IPV6)
> + if (skb->protocol == htons(ETH_P_IPV6))
> + return hlist_entry_safe(rp, struct net_bridge_port, ip6_rlist);
> +#endif
> return hlist_entry_safe(rp, struct net_bridge_port, ip4_rlist);
> }
>
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index 6937d3b..3c608da 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -18,7 +18,12 @@
>
> static inline bool br_rports_have_mc_router(struct net_bridge *br)
> {
> +#if IS_ENABLED(CONFIG_IPV6)
> + return !hlist_empty(>ip4_mc_router_list) ||
> +!hlist_empty(>ip6_mc_router_list);
> +#else
> return !hlist_empty(>ip4_mc_router_list);
> +#endif
> }
>
> static inline bool
> @@ -31,8 +36,13 @@ br_ip4_rports_get_timer(struct net_bridge_port *port,
> unsigned long *timer)
> static inline bool
> br_ip6_rports_get_timer(struct net_bridge_port *port, unsigned long *timer)
> {
> +#if IS_ENABLED(CONFIG_IPV6)
> + *timer = br_timer_value(>ip6_mc_router_timer);
> + return !hlist_unhashed(>ip6_rlist);
> +#else
> *timer = 0;
> return false;
> +#endif
> }
>
> static int br_rports_fill_info(struct sk_buff *skb, struct netlink_callback
> *cb,
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 39854d5..b625fd6 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -63,6 +63,8 @@ static void br_multicast_port_group_rexmit(struct
> timer_list *t);
> static void
> br_multicast_rport_del_notify(struct net_bridge_port *p, bool deleted);
> #if IS_ENABLED(CONFIG_IPV6)
> +static void br_ip6_multicast_add_router(struct net_bridge *br,
> + struct net_bridge_port *port);
> static void br_ip6_multicast_leave_group(struct net_bridge *br,
>struct net_bridge_port *port,
>const struct in6_addr *group,
> @@ -1369,6 +1371,15 @@ static inline bool br_ip4_multicast_rport_del(struct
> net_bridge_port *p)
> return br_multicast_rport_del(>ip4_rlist);
> }
>
> +static inline bool br_ip6_multicast_rport_del(struct net_bridge_port *p)
> +{
> +#if IS_ENABLED(CONFIG_IPV6)
> + return br_multicast_rport_del(>ip6_rlist);
> +#else
> + return false;
> +#endif
> +}
> +
> static void br_multicast_router_expired(struct net_bridge_port *port,
> struct timer_list *t,
> struct hlist_node *rlist)
> @@ -1395,6 +1406,15 @@ static void br_ip4_multicast_router_expired(struct
> timer_list *t)
> br_multicast_router_expired(port, t, >ip4_rlist);
> }
>
> +#if IS_ENABLED(CONFIG_IPV6)
> +static void br_ip6_multicast_router_expired(struct timer_list *t)
> +{
> + struct net_bridge_port *port = from_timer(port, t, ip6_mc_router_timer);
> +
> + br_multicast_router_expired(port, t, >ip6_rlist);
> +}
> +#endif
> +
> static void br_mc_router_state_change(struct net_bridge *p,
> bool is_mc_router)
> {
> @@ -1430,6 +1450,15 @@ static inline void
> br_ip4_multicast_local_router_expired(struct timer_list *t)
> br_multicast_local_router_expired(br, t);
>
Re: [net-next v2 08/11] net: bridge: mcast: split router port del+notify for mcast router split
On 09/05/2021 22:45, Linus Lüssing wrote:
> In preparation for the upcoming split of multicast router state into
> their IPv4 and IPv6 variants split router port deletion and notification
> into two functions. When we disable a port for instance later we want to
> only send one notification to switchdev and netlink for compatibility
> and want to avoid sending one for IPv4 and one for IPv6. For that the
> split is needed.
>
> Signed-off-by: Linus Lüssing
> ---
> net/bridge/br_multicast.c | 40 ++-
> 1 file changed, 31 insertions(+), 9 deletions(-)
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 839d21b..39854d5 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -60,7 +60,8 @@ static void br_ip4_multicast_leave_group(struct net_bridge
> *br,
>const unsigned char *src);
> static void br_multicast_port_group_rexmit(struct timer_list *t);
>
> -static void __del_port_router(struct net_bridge_port *p);
> +static void
> +br_multicast_rport_del_notify(struct net_bridge_port *p, bool deleted);
> #if IS_ENABLED(CONFIG_IPV6)
> static void br_ip6_multicast_leave_group(struct net_bridge *br,
>struct net_bridge_port *port,
> @@ -1354,11 +1355,26 @@ static int br_ip6_multicast_add_group(struct
> net_bridge *br,
> }
> #endif
>
> +static bool br_multicast_rport_del(struct hlist_node *rlist)
> +{
> + if (hlist_unhashed(rlist))
> + return false;
> +
> + hlist_del_init_rcu(rlist);
> + return true;
> +}
> +
> +static inline bool br_ip4_multicast_rport_del(struct net_bridge_port *p)
> +{
> + return br_multicast_rport_del(>ip4_rlist);
> +}
> +
Same comment about inline in .c files, either drop the inline or move it to
br_private.h
For functions that are not critical for performance(fast-path) I'd just drop
the inline
in the .c files and leave them there.
> static void br_multicast_router_expired(struct net_bridge_port *port,
> struct timer_list *t,
> struct hlist_node *rlist)
> {
> struct net_bridge *br = port->br;
> + bool del;
>
> spin_lock(>multicast_lock);
> if (port->multicast_router == MDB_RTR_TYPE_DISABLED ||
> @@ -1366,7 +1382,8 @@ static void br_multicast_router_expired(struct
> net_bridge_port *port,
> timer_pending(t))
> goto out;
>
> - __del_port_router(port);
> + del = br_multicast_rport_del(rlist);
> + br_multicast_rport_del_notify(port, del);
> out:
> spin_unlock(>multicast_lock);
> }
> @@ -1706,19 +1723,20 @@ void br_multicast_disable_port(struct net_bridge_port
> *port)
> struct net_bridge *br = port->br;
> struct net_bridge_port_group *pg;
> struct hlist_node *n;
> + bool del = false;
>
> spin_lock(>multicast_lock);
> hlist_for_each_entry_safe(pg, n, >mglist, mglist)
> if (!(pg->flags & MDB_PG_FLAGS_PERMANENT))
> br_multicast_find_del_pg(br, pg);
>
> - __del_port_router(port);
> -
> + del |= br_ip4_multicast_rport_del(port);
> del_timer(>ip4_mc_router_timer);
> del_timer(>ip4_own_query.timer);
> #if IS_ENABLED(CONFIG_IPV6)
> del_timer(>ip6_own_query.timer);
> #endif
> + br_multicast_rport_del_notify(port, del);
> spin_unlock(>multicast_lock);
> }
>
> @@ -3508,11 +3526,12 @@ int br_multicast_set_router(struct net_bridge *br,
> unsigned long val)
> return err;
> }
>
> -static void __del_port_router(struct net_bridge_port *p)
> +static void
> +br_multicast_rport_del_notify(struct net_bridge_port *p, bool deleted)
> {
> - if (hlist_unhashed(>ip4_rlist))
> + if (!deleted)
> return;
> - hlist_del_init_rcu(>ip4_rlist);
> +
> br_rtr_notify(p->br->dev, p, RTM_DELMDB);
> br_port_mc_router_state_change(p, false);
>
> @@ -3526,6 +3545,7 @@ int br_multicast_set_port_router(struct net_bridge_port
> *p, unsigned long val)
> struct net_bridge *br = p->br;
> unsigned long now = jiffies;
> int err = -EINVAL;
> + bool del = false;
>
> spin_lock(>multicast_lock);
> if (p->multicast_router == val) {
> @@ -3539,12 +3559,14 @@ int br_multicast_set_port_router(struct
> net_bridge_port *p, unsigned long val)
> switch (val) {
> case MDB_RTR_TYPE_DISABLED:
> p->multicast_router = MDB_RTR_TYPE_DISABLED;
> - __del_port_router(p);
> + del |= br_ip4_multicast_rport_del(p);
> del_timer(>ip4_mc_router_timer);
> + br_multicast_rport_del_notify(p, del);
> break;
> case MDB_RTR_TYPE_TEMP_QUERY:
> p->multicast_router = MDB_RTR_TYPE_TEMP_QUERY;
> - __del_port_router(p);
> + del |= br_ip4_multicast_rport_del(p);
> +
Re: [net-next v2 06/11] net: bridge: mcast: prepare expiry functions for mcast router split
On 09/05/2021 22:45, Linus Lüssing wrote:
> In preparation for the upcoming split of multicast router state into
> their IPv4 and IPv6 variants move the protocol specific timer access to
> an ip4 wrapper function.
>
> Signed-off-by: Linus Lüssing
> ---
> net/bridge/br_multicast.c | 31 ++-
> 1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 048b5b9..6c844b2 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1354,16 +1354,16 @@ static int br_ip6_multicast_add_group(struct
> net_bridge *br,
> }
> #endif
>
> -static void br_multicast_router_expired(struct timer_list *t)
> +static void br_multicast_router_expired(struct net_bridge_port *port,
> + struct timer_list *t,
> + struct hlist_node *rlist)
> {
> - struct net_bridge_port *port =
> - from_timer(port, t, ip4_mc_router_timer);
> struct net_bridge *br = port->br;
>
> spin_lock(>multicast_lock);
> if (port->multicast_router == MDB_RTR_TYPE_DISABLED ||
> port->multicast_router == MDB_RTR_TYPE_PERM ||
> - timer_pending(>ip4_mc_router_timer))
> + timer_pending(t))
> goto out;
>
> __del_port_router(port);
> @@ -1371,6 +1371,13 @@ out:
> spin_unlock(>multicast_lock);
> }
>
> +static void br_ip4_multicast_router_expired(struct timer_list *t)
> +{
> + struct net_bridge_port *port = from_timer(port, t, ip4_mc_router_timer);
> +
> + br_multicast_router_expired(port, t, >ip4_rlist);
> +}
> +
> static void br_mc_router_state_change(struct net_bridge *p,
> bool is_mc_router)
> {
> @@ -1384,10 +1391,9 @@ static void br_mc_router_state_change(struct
> net_bridge *p,
> switchdev_port_attr_set(p->dev, , NULL);
> }
>
> -static void br_multicast_local_router_expired(struct timer_list *t)
> +static void br_multicast_local_router_expired(struct net_bridge *br,
> + struct timer_list *timer)
> {
> - struct net_bridge *br = from_timer(br, t, ip4_mc_router_timer);
> -
> spin_lock(>multicast_lock);
> if (br->multicast_router == MDB_RTR_TYPE_DISABLED ||
> br->multicast_router == MDB_RTR_TYPE_PERM ||
> @@ -1400,6 +1406,13 @@ out:
> spin_unlock(>multicast_lock);
> }
>
> +static inline void br_ip4_multicast_local_router_expired(struct timer_list
> *t)
> +{
> + struct net_bridge *br = from_timer(br, t, ip4_mc_router_timer);
> +
> + br_multicast_local_router_expired(br, t);
> +}
> +
Same comment about inlines in .c files, please move them to br_private.h or
drop the inline
> static void br_multicast_querier_expired(struct net_bridge *br,
>struct bridge_mcast_own_query *query)
> {
> @@ -1615,7 +1628,7 @@ int br_multicast_add_port(struct net_bridge_port *port)
> port->multicast_eht_hosts_limit = BR_MCAST_DEFAULT_EHT_HOSTS_LIMIT;
>
> timer_setup(>ip4_mc_router_timer,
> - br_multicast_router_expired, 0);
> + br_ip4_multicast_router_expired, 0);
> timer_setup(>ip4_own_query.timer,
> br_ip4_multicast_port_query_expired, 0);
> #if IS_ENABLED(CONFIG_IPV6)
> @@ -3319,7 +3332,7 @@ void br_multicast_init(struct net_bridge *br)
>
> spin_lock_init(>multicast_lock);
> timer_setup(>ip4_mc_router_timer,
> - br_multicast_local_router_expired, 0);
> + br_ip4_multicast_local_router_expired, 0);
> timer_setup(>ip4_other_query.timer,
> br_ip4_multicast_querier_expired, 0);
> timer_setup(>ip4_own_query.timer,
>
Re: [net-next v2 05/11] net: bridge: mcast: prepare is-router function for mcast router split
On 09/05/2021 22:45, Linus Lüssing wrote:
> In preparation for the upcoming split of multicast router state into
> their IPv4 and IPv6 variants make br_multicast_is_router() protocol
> family aware.
>
> Note that for now br_ip6_multicast_is_router() uses the currently still
> common ip4_mc_router_timer for now. It will be renamed to
> ip6_mc_router_timer later when the split is performed.
>
> While at it also renames the "1" and "2" constants in
> br_multicast_is_router() to the MDB_RTR_TYPE_TEMP_QUERY and
> MDB_RTR_TYPE_PERM enums.
>
> Signed-off-by: Linus Lüssing
> ---
> net/bridge/br_input.c | 2 +-
> net/bridge/br_multicast.c | 5 +++--
> net/bridge/br_private.h | 36
> 3 files changed, 36 insertions(+), 7 deletions(-)
>
Minor comment below, but either way:
Acked-by: Nikolay Aleksandrov
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 8875e95..1f50630 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -132,7 +132,7 @@ int br_handle_frame_finish(struct net *net, struct sock
> *sk, struct sk_buff *skb
> if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
> br_multicast_querier_exists(br, eth_hdr(skb), mdst)) {
> if ((mdst && mdst->host_joined) ||
> - br_multicast_is_router(br)) {
> + br_multicast_is_router(br, skb)) {
> local_rcv = true;
> br->dev->stats.multicast++;
> }
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 7edbbc9..048b5b9 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1391,7 +1391,8 @@ static void br_multicast_local_router_expired(struct
> timer_list *t)
> spin_lock(>multicast_lock);
> if (br->multicast_router == MDB_RTR_TYPE_DISABLED ||
> br->multicast_router == MDB_RTR_TYPE_PERM ||
> - timer_pending(>ip4_mc_router_timer))
> + br_ip4_multicast_is_router(br) ||
> + br_ip6_multicast_is_router(br))
> goto out;
>
> br_mc_router_state_change(br, false);
> @@ -3622,7 +3623,7 @@ bool br_multicast_router(const struct net_device *dev)
> bool is_router;
>
> spin_lock_bh(>multicast_lock);
> - is_router = br_multicast_is_router(br);
> + is_router = br_multicast_is_router(br, NULL);
> spin_unlock_bh(>multicast_lock);
> return is_router;
> }
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 26e91d2..ac5ca5b 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -864,11 +864,39 @@ static inline bool br_group_is_l2(const struct br_ip
> *group)
> #define mlock_dereference(X, br) \
> rcu_dereference_protected(X, lockdep_is_held(>multicast_lock))
>
> -static inline bool br_multicast_is_router(struct net_bridge *br)
> +static inline bool br_ip4_multicast_is_router(struct net_bridge *br)
> {
> - return br->multicast_router == 2 ||
> -(br->multicast_router == 1 &&
> - timer_pending(>ip4_mc_router_timer));
> + return timer_pending(>ip4_mc_router_timer);
> +}
> +
> +static inline bool br_ip6_multicast_is_router(struct net_bridge *br)
> +{
> +#if IS_ENABLED(CONFIG_IPV6)
> + return timer_pending(>ip4_mc_router_timer);
> +#else
> + return false;
> +#endif
> +}
> +
> +static inline bool
> +br_multicast_is_router(struct net_bridge *br, struct sk_buff *skb)
> +{
> + if (br->multicast_router == MDB_RTR_TYPE_PERM)
> + return true;
> +
> + if (br->multicast_router == MDB_RTR_TYPE_TEMP_QUERY) {
> + if (skb) {
> + if (skb->protocol == htons(ETH_P_IP))
> + return br_ip4_multicast_is_router(br);
> + else if (skb->protocol == htons(ETH_P_IPV6))
> + return br_ip6_multicast_is_router(br);
> + } else {
> + return br_ip4_multicast_is_router(br) ||
> +br_ip6_multicast_is_router(br);
> + }
> + }
Personally I'd prefer using a switch statement for br->multicast_router.
> +
> + return false;
> }
>
> static inline bool
>
Re: [net-next v2 04/11] net: bridge: mcast: prepare query reception for mcast router split
On 09/05/2021 22:45, Linus Lüssing wrote:
> In preparation for the upcoming split of multicast router state into
> their IPv4 and IPv6 variants and as the br_multicast_mark_router() will
> be split for that remove the select querier wrapper and instead add
> ip4 and ip6 variants for br_multicast_query_received().
>
> Signed-off-by: Linus Lüssing
> ---
> net/bridge/br_multicast.c | 53 ---
> 1 file changed, 27 insertions(+), 26 deletions(-)
>
Acked-by: Nikolay Aleksandrov
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 6fe93a3..7edbbc9 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -2615,22 +2615,6 @@ update:
> }
> #endif
>
> -static bool br_multicast_select_querier(struct net_bridge *br,
> - struct net_bridge_port *port,
> - struct br_ip *saddr)
> -{
> - switch (saddr->proto) {
> - case htons(ETH_P_IP):
> - return br_ip4_multicast_select_querier(br, port,
> saddr->src.ip4);
> -#if IS_ENABLED(CONFIG_IPV6)
> - case htons(ETH_P_IPV6):
> - return br_ip6_multicast_select_querier(br, port,
> >src.ip6);
> -#endif
> - }
> -
> - return false;
> -}
> -
> static void
> br_multicast_update_query_timer(struct net_bridge *br,
> struct bridge_mcast_other_query *query,
> @@ -2708,19 +2692,36 @@ static void br_multicast_mark_router(struct
> net_bridge *br,
> now + br->multicast_querier_interval);
> }
>
> -static void br_multicast_query_received(struct net_bridge *br,
> - struct net_bridge_port *port,
> - struct bridge_mcast_other_query *query,
> - struct br_ip *saddr,
> - unsigned long max_delay)
> +static void
> +br_ip4_multicast_query_received(struct net_bridge *br,
> + struct net_bridge_port *port,
> + struct bridge_mcast_other_query *query,
> + struct br_ip *saddr,
> + unsigned long max_delay)
> {
> - if (!br_multicast_select_querier(br, port, saddr))
> + if (!br_ip4_multicast_select_querier(br, port, saddr->src.ip4))
> return;
>
> br_multicast_update_query_timer(br, query, max_delay);
> br_multicast_mark_router(br, port);
> }
>
> +#if IS_ENABLED(CONFIG_IPV6)
> +static void
> +br_ip6_multicast_query_received(struct net_bridge *br,
> + struct net_bridge_port *port,
> + struct bridge_mcast_other_query *query,
> + struct br_ip *saddr,
> + unsigned long max_delay)
> +{
> + if (!br_ip6_multicast_select_querier(br, port, >src.ip6))
> + return;
> +
> + br_multicast_update_query_timer(br, query, max_delay);
> + br_multicast_mark_router(br, port);
> +}
> +#endif
> +
> static void br_ip4_multicast_query(struct net_bridge *br,
> struct net_bridge_port *port,
> struct sk_buff *skb,
> @@ -2768,8 +2769,8 @@ static void br_ip4_multicast_query(struct net_bridge
> *br,
> saddr.proto = htons(ETH_P_IP);
> saddr.src.ip4 = iph->saddr;
>
> - br_multicast_query_received(br, port, >ip4_other_query,
> - , max_delay);
> + br_ip4_multicast_query_received(br, port, >ip4_other_query,
> + , max_delay);
> goto out;
> }
>
> @@ -2856,8 +2857,8 @@ static int br_ip6_multicast_query(struct net_bridge *br,
> saddr.proto = htons(ETH_P_IPV6);
> saddr.src.ip6 = ipv6_hdr(skb)->saddr;
>
> - br_multicast_query_received(br, port, >ip6_other_query,
> - , max_delay);
> + br_ip6_multicast_query_received(br, port, >ip6_other_query,
> + , max_delay);
> goto out;
> } else if (!group) {
> goto out;
>
Re: [net-next v2 03/11] net: bridge: mcast: prepare mdb netlink for mcast router split
On 09/05/2021 22:45, Linus Lüssing wrote:
> In preparation for the upcoming split of multicast router state into
> their IPv4 and IPv6 variants and to avoid IPv6 #ifdef clutter later add
> some inline functions for the protocol specific parts in the mdb router
> netlink code. Also the we need iterate over the port instead of router
> list to be able put one router port entry with both the IPv4 and IPv6
> multicast router info later.
>
> Signed-off-by: Linus Lüssing
> ---
> net/bridge/br_mdb.c | 39 ++-
> 1 file changed, 34 insertions(+), 5 deletions(-)
>
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index d61def8..6937d3b 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -16,29 +16,58 @@
>
> #include "br_private.h"
>
> +static inline bool br_rports_have_mc_router(struct net_bridge *br)
> +{
> + return !hlist_empty(>ip4_mc_router_list);
> +}
> +
> +static inline bool
> +br_ip4_rports_get_timer(struct net_bridge_port *port, unsigned long *timer)
> +{
> + *timer = br_timer_value(>ip4_mc_router_timer);
> + return !hlist_unhashed(>ip4_rlist);
> +}
> +
> +static inline bool
> +br_ip6_rports_get_timer(struct net_bridge_port *port, unsigned long *timer)
> +{
> + *timer = 0;
> + return false;
> +}
> +
Same comment about inlines in .c files, please move them to br_private.h
> static int br_rports_fill_info(struct sk_buff *skb, struct netlink_callback
> *cb,
> struct net_device *dev)
> {
> struct net_bridge *br = netdev_priv(dev);
> - struct net_bridge_port *p;
> + bool have_ip4_mc_rtr, have_ip6_mc_rtr;
> + unsigned long ip4_timer, ip6_timer;
> struct nlattr *nest, *port_nest;
> + struct net_bridge_port *p;
>
> - if (!br->multicast_router || hlist_empty(>ip4_mc_router_list))
> + if (!br->multicast_router)
> + return 0;
> +
> + if (!br_rports_have_mc_router(br))
> return 0;
>
> nest = nla_nest_start_noflag(skb, MDBA_ROUTER);
> if (nest == NULL)
> return -EMSGSIZE;
>
> - hlist_for_each_entry_rcu(p, >ip4_mc_router_list, ip4_rlist) {
> - if (!p)
> + list_for_each_entry_rcu(p, >port_list, list) {
> + have_ip4_mc_rtr = br_ip4_rports_get_timer(p, _timer);
> + have_ip6_mc_rtr = br_ip6_rports_get_timer(p, _timer);
> +
> + if (!have_ip4_mc_rtr && !have_ip6_mc_rtr)
> continue;
> +
> port_nest = nla_nest_start_noflag(skb, MDBA_ROUTER_PORT);
> if (!port_nest)
> goto fail;
> +
> if (nla_put_nohdr(skb, sizeof(u32), >dev->ifindex) ||
> nla_put_u32(skb, MDBA_ROUTER_PATTR_TIMER,
> - br_timer_value(>ip4_mc_router_timer)) ||
> + max(ip4_timer, ip6_timer)) ||
> nla_put_u8(skb, MDBA_ROUTER_PATTR_TYPE,
> p->multicast_router)) {
> nla_nest_cancel(skb, port_nest);
>
Re: [net-next v2 02/11] net: bridge: mcast: add wrappers for router node retrieval
On 09/05/2021 22:45, Linus Lüssing wrote:
> In preparation for the upcoming split of multicast router state into
> their IPv4 and IPv6 variants and to avoid IPv6 #ifdef clutter later add
> two wrapper functions for router node retrieval in the payload
> forwarding code.
>
> Signed-off-by: Linus Lüssing
> ---
> net/bridge/br_forward.c | 15 +--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> index 3b67184..b5ec4f9 100644
> --- a/net/bridge/br_forward.c
> +++ b/net/bridge/br_forward.c
> @@ -264,6 +264,16 @@ static void maybe_deliver_addr(struct net_bridge_port
> *p, struct sk_buff *skb,
> __br_forward(p, skb, local_orig);
> }
>
> +static inline struct hlist_node *
> +br_multicast_get_first_rport_node(struct net_bridge *b, struct sk_buff *skb)
> {
> + return rcu_dereference(hlist_first_rcu(>ip4_mc_router_list));
> +}
> +
> +static inline struct net_bridge_port *
> +br_multicast_rport_from_node(struct hlist_node *rp, struct sk_buff *skb) {
> + return hlist_entry_safe(rp, struct net_bridge_port, ip4_rlist);
> +}
> +
Inline functions in .c files are not allowed, please move these to br_private.h
> /* called with rcu_read_lock */
> void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
> struct sk_buff *skb,
> @@ -276,7 +286,8 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
> bool allow_mode_include = true;
> struct hlist_node *rp;
>
> - rp = rcu_dereference(hlist_first_rcu(>router_list));
> + rp = br_multicast_get_first_rport_node(br, skb);
> +
> if (mdst) {
> p = rcu_dereference(mdst->ports);
> if (br_multicast_should_handle_mode(br, mdst->addr.proto) &&
> @@ -290,7 +301,7 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
> struct net_bridge_port *port, *lport, *rport;
>
> lport = p ? p->key.port : NULL;
> - rport = hlist_entry_safe(rp, struct net_bridge_port, ip4_rlist);
> + rport = br_multicast_rport_from_node(rp, skb);
>
> if ((unsigned long)lport > (unsigned long)rport) {
> port = lport;
>
Re: [net-next v2 01/11] net: bridge: mcast: rename multicast router lists and timers
On 09/05/2021 22:44, Linus Lüssing wrote:
> In preparation for the upcoming split of multicast router state into
> their IPv4 and IPv6 variants, rename the affected variable to the IPv4
> version first to avoid some renames in later commits.
>
> Signed-off-by: Linus Lüssing
> ---
> net/bridge/br_forward.c | 2 +-
> net/bridge/br_mdb.c | 6 ++---
> net/bridge/br_multicast.c | 48 +++
> net/bridge/br_private.h | 10
> 4 files changed, 33 insertions(+), 33 deletions(-)
>
Acked-by: Nikolay Aleksandrov
> diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> index 6e9b049..3b67184 100644
> --- a/net/bridge/br_forward.c
> +++ b/net/bridge/br_forward.c
> @@ -290,7 +290,7 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
> struct net_bridge_port *port, *lport, *rport;
>
> lport = p ? p->key.port : NULL;
> - rport = hlist_entry_safe(rp, struct net_bridge_port, rlist);
> + rport = hlist_entry_safe(rp, struct net_bridge_port, ip4_rlist);
>
> if ((unsigned long)lport > (unsigned long)rport) {
> port = lport;
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index 95fa4af..d61def8 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -23,14 +23,14 @@ static int br_rports_fill_info(struct sk_buff *skb,
> struct netlink_callback *cb,
> struct net_bridge_port *p;
> struct nlattr *nest, *port_nest;
>
> - if (!br->multicast_router || hlist_empty(>router_list))
> + if (!br->multicast_router || hlist_empty(>ip4_mc_router_list))
> return 0;
>
> nest = nla_nest_start_noflag(skb, MDBA_ROUTER);
> if (nest == NULL)
> return -EMSGSIZE;
>
> - hlist_for_each_entry_rcu(p, >router_list, rlist) {
> + hlist_for_each_entry_rcu(p, >ip4_mc_router_list, ip4_rlist) {
> if (!p)
> continue;
> port_nest = nla_nest_start_noflag(skb, MDBA_ROUTER_PORT);
> @@ -38,7 +38,7 @@ static int br_rports_fill_info(struct sk_buff *skb, struct
> netlink_callback *cb,
> goto fail;
> if (nla_put_nohdr(skb, sizeof(u32), >dev->ifindex) ||
> nla_put_u32(skb, MDBA_ROUTER_PATTR_TIMER,
> - br_timer_value(>multicast_router_timer)) ||
> + br_timer_value(>ip4_mc_router_timer)) ||
> nla_put_u8(skb, MDBA_ROUTER_PATTR_TYPE,
> p->multicast_router)) {
> nla_nest_cancel(skb, port_nest);
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 226bb05..6fe93a3 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1357,13 +1357,13 @@ static int br_ip6_multicast_add_group(struct
> net_bridge *br,
> static void br_multicast_router_expired(struct timer_list *t)
> {
> struct net_bridge_port *port =
> - from_timer(port, t, multicast_router_timer);
> + from_timer(port, t, ip4_mc_router_timer);
> struct net_bridge *br = port->br;
>
> spin_lock(>multicast_lock);
> if (port->multicast_router == MDB_RTR_TYPE_DISABLED ||
> port->multicast_router == MDB_RTR_TYPE_PERM ||
> - timer_pending(>multicast_router_timer))
> + timer_pending(>ip4_mc_router_timer))
> goto out;
>
> __del_port_router(port);
> @@ -1386,12 +1386,12 @@ static void br_mc_router_state_change(struct
> net_bridge *p,
>
> static void br_multicast_local_router_expired(struct timer_list *t)
> {
> - struct net_bridge *br = from_timer(br, t, multicast_router_timer);
> + struct net_bridge *br = from_timer(br, t, ip4_mc_router_timer);
>
> spin_lock(>multicast_lock);
> if (br->multicast_router == MDB_RTR_TYPE_DISABLED ||
> br->multicast_router == MDB_RTR_TYPE_PERM ||
> - timer_pending(>multicast_router_timer))
> + timer_pending(>ip4_mc_router_timer))
> goto out;
>
> br_mc_router_state_change(br, false);
> @@ -1613,7 +1613,7 @@ int br_multicast_add_port(struct net_bridge_port *port)
> port->multicast_router = MDB_RTR_TYPE_TEMP_QUERY;
> port->multicast_eht_hosts_limit = BR_MCAST_DEFAULT_EHT_HOSTS_LIMIT;
>
> - timer_setup(>multicast_router_timer,
> + timer_setup(>ip4_mc_router_timer,
> br_multicast_router_expired, 0);
> timer_setup(>ip4_own_query.timer,
&g
Re: [PATCH net-next 1/2] net: bridge: mcast: split multicast router state for IPv4 and IPv6
On 25/04/2021 19:00, Linus Lüssing wrote:
> A multicast router for IPv4 does not imply that the same host also is a
> multicast router for IPv6 and vice versa.
>
> To reduce multicast traffic when a host is only a multicast router for
> one of these two protocol families, keep router state for IPv4 and IPv6
> separately. Similar to how querier state is kept separately.
>
> For backwards compatibility for netlink and switchdev notifications
> these two will still only notify if a port switched from either no
> IPv4/IPv6 multicast router to any IPv4/IPv6 multicast router or the
> other way round. However a full netlink MDB router dump will now also
> include a multicast router timeout for both IPv4 and IPv6.
>
> Signed-off-by: Linus Lüssing
> ---
> include/uapi/linux/if_bridge.h | 2 +
> net/bridge/br_forward.c| 22 ++-
> net/bridge/br_input.c | 2 +-
> net/bridge/br_mdb.c| 38 +++-
> net/bridge/br_multicast.c | 341 +
> net/bridge/br_private.h| 48 -
> 6 files changed, 352 insertions(+), 101 deletions(-)
>
Hi Linus,
A few comments below, in general I like the change.
I'd really like it if this patch could be broken up, up to you I'll review it
either way. :)
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 13d59c51ef5b..6b56a7549531 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -627,6 +627,8 @@ enum {
> MDBA_ROUTER_PATTR_UNSPEC,
> MDBA_ROUTER_PATTR_TIMER,
> MDBA_ROUTER_PATTR_TYPE,
> + MDBA_ROUTER_PATTR_INET_TIMER,
> + MDBA_ROUTER_PATTR_INET6_TIMER,
> __MDBA_ROUTER_PATTR_MAX
> };
> #define MDBA_ROUTER_PATTR_MAX (__MDBA_ROUTER_PATTR_MAX - 1)
> diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> index 6e9b049ae521..897fafc83cd0 100644
> --- a/net/bridge/br_forward.c
> +++ b/net/bridge/br_forward.c
> @@ -275,8 +275,19 @@ void br_multicast_flood(struct net_bridge_mdb_entry
> *mdst,
> struct net_bridge_port_group *p;
> bool allow_mode_include = true;
> struct hlist_node *rp;
> +#if IS_ENABLED(CONFIG_IPV6)
> + bool is_ipv6 = false;
> +
> + if (skb->protocol == htons(ETH_P_IPV6)) {
> + is_ipv6 = true;
> + rp = rcu_dereference(hlist_first_rcu(>ip6_mc_router_list));
> + } else {
> +#else
> + if (1) {
> +#endif
> + rp = rcu_dereference(hlist_first_rcu(>ip4_mc_router_list));
> + }
>
> - rp = rcu_dereference(hlist_first_rcu(>router_list));
> if (mdst) {
> p = rcu_dereference(mdst->ports);
> if (br_multicast_should_handle_mode(br, mdst->addr.proto) &&
> @@ -290,7 +301,14 @@ void br_multicast_flood(struct net_bridge_mdb_entry
> *mdst,
> struct net_bridge_port *port, *lport, *rport;
>
> lport = p ? p->key.port : NULL;
> - rport = hlist_entry_safe(rp, struct net_bridge_port, rlist);
> +#if IS_ENABLED(CONFIG_IPV6)
> + if (is_ipv6)
> + rport = hlist_entry_safe(rp, struct net_bridge_port,
> + ip6_rlist);
> + else
> +#endif
> + rport = hlist_entry_safe(rp, struct net_bridge_port,
> + ip4_rlist);
>
Please add a pointer to the proper list and retrieve it before this,
preferrably also
add a helper to do that.
> if ((unsigned long)lport > (unsigned long)rport) {
> port = lport;
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 8875e953ac53..1f506309efa8 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -132,7 +132,7 @@ int br_handle_frame_finish(struct net *net, struct sock
> *sk, struct sk_buff *skb
> if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) &&
> br_multicast_querier_exists(br, eth_hdr(skb), mdst)) {
> if ((mdst && mdst->host_joined) ||
> - br_multicast_is_router(br)) {
> + br_multicast_is_router(br, skb)) {
> local_rcv = true;
> br->dev->stats.multicast++;
> }
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index 95fa4af0e8dd..2fce1a895a70 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -22,25 +22,53 @@ static int br_rports_fill_info(struct sk_buff *skb,
> struct netlink_callback *cb,
> struct net_bridge *br = netdev_priv(dev);
> struct net_bridge_port *p;
> struct nlattr *nest, *port_nest;
> + bool have_ip4_mc_rtr, have_ip6_mc_rtr = false;
> + unsigned long ip4_timer, ip6_timer = 0;
>
Reverse xmas tree, feel free to fix the current ones as well. :)
> - if (!br->multicast_router || hlist_empty(>router_list))
> + if (!br->multicast_router)
> return 0;
[PATCH net-next v2 06/16] net: bridge: mcast: rename br_ip's u member to dst
From: Nikolay Aleksandrov
Since now we have src in br_ip, u no longer makes sense so rename
it to dst. No functional changes.
v2: fix build with CONFIG_BATMAN_ADV_MCAST
CC: Marek Lindner
CC: Simon Wunderlich
CC: Antonio Quartulli
CC: Sven Eckelmann
CC: b.a.t.m.a.n@lists.open-mesh.org
Signed-off-by: Nikolay Aleksandrov
---
include/linux/if_bridge.h | 2 +-
net/batman-adv/multicast.c | 14 +++---
net/bridge/br_mdb.c| 16
net/bridge/br_multicast.c | 26 +-
4 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 4fb9c4954f3a..556caed00258 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -25,7 +25,7 @@ struct br_ip {
#if IS_ENABLED(CONFIG_IPV6)
struct in6_addr ip6;
#endif
- } u;
+ } dst;
__be16 proto;
__u16 vid;
};
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index 1622c3f5898f..7dda0f7b3d96 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -220,7 +220,7 @@ static u8 batadv_mcast_mla_rtr_flags_bridge_get(struct
batadv_priv *bat_priv,
* address here, only IPv6 ones
*/
if (br_ip_entry->addr.proto == htons(ETH_P_IPV6) &&
- ipv6_addr_is_ll_all_routers(_ip_entry->addr.u.ip6))
+ ipv6_addr_is_ll_all_routers(_ip_entry->addr.dst.ip6))
flags &= ~BATADV_MCAST_WANT_NO_RTR6;
list_del(_ip_entry->list);
@@ -561,10 +561,10 @@ batadv_mcast_mla_softif_get(struct net_device *dev,
static void batadv_mcast_mla_br_addr_cpy(char *dst, const struct br_ip *src)
{
if (src->proto == htons(ETH_P_IP))
- ip_eth_mc_map(src->u.ip4, dst);
+ ip_eth_mc_map(src->dst.ip4, dst);
#if IS_ENABLED(CONFIG_IPV6)
else if (src->proto == htons(ETH_P_IPV6))
- ipv6_eth_mc_map(>u.ip6, dst);
+ ipv6_eth_mc_map(>dst.ip6, dst);
#endif
else
eth_zero_addr(dst);
@@ -608,11 +608,11 @@ static int batadv_mcast_mla_bridge_get(struct net_device
*dev,
continue;
if (tvlv_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES &&
- ipv4_is_local_multicast(br_ip_entry->addr.u.ip4))
+ ipv4_is_local_multicast(br_ip_entry->addr.dst.ip4))
continue;
if (!(tvlv_flags & BATADV_MCAST_WANT_NO_RTR4) &&
- !ipv4_is_local_multicast(br_ip_entry->addr.u.ip4))
+ !ipv4_is_local_multicast(br_ip_entry->addr.dst.ip4))
continue;
}
@@ -622,11 +622,11 @@ static int batadv_mcast_mla_bridge_get(struct net_device
*dev,
continue;
if (tvlv_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES &&
- ipv6_addr_is_ll_all_nodes(_ip_entry->addr.u.ip6))
+
ipv6_addr_is_ll_all_nodes(_ip_entry->addr.dst.ip6))
continue;
if (!(tvlv_flags & BATADV_MCAST_WANT_NO_RTR6) &&
- IPV6_ADDR_MC_SCOPE(_ip_entry->addr.u.ip6) >
+ IPV6_ADDR_MC_SCOPE(_ip_entry->addr.dst.ip6) >
IPV6_ADDR_SCOPE_LINKLOCAL)
continue;
}
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 269ffd2e549b..a1ff0a372185 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -70,10 +70,10 @@ static void __mdb_entry_to_br_ip(struct br_mdb_entry
*entry, struct br_ip *ip)
ip->vid = entry->vid;
ip->proto = entry->addr.proto;
if (ip->proto == htons(ETH_P_IP))
- ip->u.ip4 = entry->addr.u.ip4;
+ ip->dst.ip4 = entry->addr.u.ip4;
#if IS_ENABLED(CONFIG_IPV6)
else
- ip->u.ip6 = entry->addr.u.ip6;
+ ip->dst.ip6 = entry->addr.u.ip6;
#endif
}
@@ -158,10 +158,10 @@ static int __mdb_fill_info(struct sk_buff *skb,
e.ifindex = ifindex;
e.vid = mp->addr.vid;
if (mp->addr.proto == htons(ETH_P_IP))
- e.addr.u.ip4 = mp->addr.u.ip4;
+ e.addr.u.ip4 = mp->addr.dst.ip4;
#if IS_ENABLED(CONFIG_IPV6)
if (mp->addr.proto == htons(ETH_P_IPV6))
- e.addr.u.ip6 = mp->addr.u.ip6;
+ e.addr.u.ip6 = mp->addr.dst.ip6;
#endif
e.addr.proto = mp->addr.proto;
nest_ent = nla_nest_start_noflag(skb,
@@ -474,10 +474,10 @@ static void br_mdb_switchdev_host_port(struct net_devi
Re: [PATCH net-next 06/16] net: bridge: mcast: rename br_ip's u member to dst
On Mon, 2020-09-21 at 21:30 +0800, kernel test robot wrote: > Hi Nikolay, > > I love your patch! Yet something to improve: > > [auto build test ERROR on net-next/master] > > url: > https://github.com/0day-ci/linux/commits/Nikolay-Aleksandrov/net-bridge-mcast-IGMPv3-MLDv2-fast-path-part-2/20200921-185933 > base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git > 3cec0369905d086a56a7515f3449982403057599 > config: riscv-allyesconfig (attached as .config) > compiler: riscv64-linux-gcc (GCC) 9.3.0 > reproduce (this is a W=1 build): > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross > ARCH=riscv > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot > > All errors (new ones prefixed by >>): > >net/batman-adv/multicast.c: In function 'batadv_mcast_mla_br_addr_cpy': > > > net/batman-adv/multicast.c:564:20: error: 'const struct br_ip' has no > > > member named 'u' > 564 | ip_eth_mc_map(src->u.ip4, dst); > |^~ >net/batman-adv/multicast.c:567:23: error: 'const struct br_ip' has no > member named 'u' > 567 | ipv6_eth_mc_map(>u.ip6, dst); > | ^~ > Hrm, I'm pretty sure I tested batman, but apparently I missed CONFIG_BATMAN_ADV_MCAST. I'll fix it up and send v2 after some time to give people the chance to comment on the rest of the set. Thanks!
[PATCH net-next 06/16] net: bridge: mcast: rename br_ip's u member to dst
From: Nikolay Aleksandrov
Since now we have src in br_ip, u no longer makes sense so rename
it to dst. No functional changes.
CC: Marek Lindner
CC: Simon Wunderlich
CC: Antonio Quartulli
CC: b.a.t.m.a.n@lists.open-mesh.org
Signed-off-by: Nikolay Aleksandrov
---
include/linux/if_bridge.h | 2 +-
net/batman-adv/multicast.c | 10 +-
net/bridge/br_mdb.c| 16
net/bridge/br_multicast.c | 26 +-
4 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 4fb9c4954f3a..556caed00258 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -25,7 +25,7 @@ struct br_ip {
#if IS_ENABLED(CONFIG_IPV6)
struct in6_addr ip6;
#endif
- } u;
+ } dst;
__be16 proto;
__u16 vid;
};
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index 1622c3f5898f..2317c0d69b58 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -220,7 +220,7 @@ static u8 batadv_mcast_mla_rtr_flags_bridge_get(struct
batadv_priv *bat_priv,
* address here, only IPv6 ones
*/
if (br_ip_entry->addr.proto == htons(ETH_P_IPV6) &&
- ipv6_addr_is_ll_all_routers(_ip_entry->addr.u.ip6))
+ ipv6_addr_is_ll_all_routers(_ip_entry->addr.dst.ip6))
flags &= ~BATADV_MCAST_WANT_NO_RTR6;
list_del(_ip_entry->list);
@@ -608,11 +608,11 @@ static int batadv_mcast_mla_bridge_get(struct net_device
*dev,
continue;
if (tvlv_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES &&
- ipv4_is_local_multicast(br_ip_entry->addr.u.ip4))
+ ipv4_is_local_multicast(br_ip_entry->addr.dst.ip4))
continue;
if (!(tvlv_flags & BATADV_MCAST_WANT_NO_RTR4) &&
- !ipv4_is_local_multicast(br_ip_entry->addr.u.ip4))
+ !ipv4_is_local_multicast(br_ip_entry->addr.dst.ip4))
continue;
}
@@ -622,11 +622,11 @@ static int batadv_mcast_mla_bridge_get(struct net_device
*dev,
continue;
if (tvlv_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES &&
- ipv6_addr_is_ll_all_nodes(_ip_entry->addr.u.ip6))
+
ipv6_addr_is_ll_all_nodes(_ip_entry->addr.dst.ip6))
continue;
if (!(tvlv_flags & BATADV_MCAST_WANT_NO_RTR6) &&
- IPV6_ADDR_MC_SCOPE(_ip_entry->addr.u.ip6) >
+ IPV6_ADDR_MC_SCOPE(_ip_entry->addr.dst.ip6) >
IPV6_ADDR_SCOPE_LINKLOCAL)
continue;
}
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 269ffd2e549b..a1ff0a372185 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -70,10 +70,10 @@ static void __mdb_entry_to_br_ip(struct br_mdb_entry
*entry, struct br_ip *ip)
ip->vid = entry->vid;
ip->proto = entry->addr.proto;
if (ip->proto == htons(ETH_P_IP))
- ip->u.ip4 = entry->addr.u.ip4;
+ ip->dst.ip4 = entry->addr.u.ip4;
#if IS_ENABLED(CONFIG_IPV6)
else
- ip->u.ip6 = entry->addr.u.ip6;
+ ip->dst.ip6 = entry->addr.u.ip6;
#endif
}
@@ -158,10 +158,10 @@ static int __mdb_fill_info(struct sk_buff *skb,
e.ifindex = ifindex;
e.vid = mp->addr.vid;
if (mp->addr.proto == htons(ETH_P_IP))
- e.addr.u.ip4 = mp->addr.u.ip4;
+ e.addr.u.ip4 = mp->addr.dst.ip4;
#if IS_ENABLED(CONFIG_IPV6)
if (mp->addr.proto == htons(ETH_P_IPV6))
- e.addr.u.ip6 = mp->addr.u.ip6;
+ e.addr.u.ip6 = mp->addr.dst.ip6;
#endif
e.addr.proto = mp->addr.proto;
nest_ent = nla_nest_start_noflag(skb,
@@ -474,10 +474,10 @@ static void br_mdb_switchdev_host_port(struct net_device
*dev,
};
if (mp->addr.proto == htons(ETH_P_IP))
- ip_eth_mc_map(mp->addr.u.ip4, mdb.addr);
+ ip_eth_mc_map(mp->addr.dst.ip4, mdb.addr);
#if IS_ENABLED(CONFIG_IPV6)
else
- ipv6_eth_mc_map(>addr.u.ip6, mdb.addr);
+ ipv6_eth_mc_map(>addr.dst.ip6, mdb.addr);
#endif
mdb.obj.orig_dev = dev;
@@ -520,10 +520,10 @@ void br_mdb_notify(struct net_device *dev,
if (pg) {
if (mp->addr.proto == htons(ETH_P_IP))
- ip_eth_mc_map(mp->addr.u.ip4, mdb
Re: [RFC net-next] net: dsa: add support for MC_DISABLED attribute
On 30/06/2019 19:56, Linus Lüssing wrote:
> On Sat, Jun 29, 2019 at 07:29:45PM +0300, Ido Schimmel wrote:
>> I would like to avoid having drivers take the querier state into account
>> as it will only complicate things further.
>
> I absolutely share your pain. Initially in the early prototypes of
> multicast awareness in batman-adv we did not consider the querier state.
> And doing so later did indeed complicate the code a good bit in batman-adv
> (together with the IGMP/MLD suppression issues). I would have loved to
> avoid that.
>
>
>> Is there anything we can do about it? Enable the bridge querier if no
>> other querier was detected? Commit c5c23260594c ("bridge: Add
>> multicast_querier toggle and disable queries by default") disabled
>> queries by default, but I'm only suggesting to turn them on if no other
>> querier was detected on the link. Do you think it's still a problem?
>
> As soon as you start becoming the querier, you will not be able to reliably
> detect anymore whether you are the only querier candidate.
>
> If any random Linux host using a bridge device were potentially becoming
> a querier, that would cause quite some trouble when this host is
> behind some bad, bottleneck connection. This host will receive
> all multicast traffic, not just IGMP/MLD reports. And with a
> congested connection and then unreliable IGMP/MLD, multicast would
> become unreliable overall in this domain. So it's important that
> your querier is not running in the "dark, remote, dusty closet" of
> your network (topologically speaking).
>
+1
We definitely don't want random hosts becoming queriers
>> On Sun, Jun 23, 2019 at 10:44:27AM +0300, Ido Schimmel wrote:
>>> See commit b00589af3b04 ("bridge: disable snooping if there is no
>>> querier"). I think that's unfortunate behavior that we need because
>>> multicast snooping is enabled by default. If it weren't enabled by
>>> default, then anyone enabling it would also make sure there's a querier
>>> in the network.
>
> I do not quite understand that point. In a way, that's what we
> have right now, isn't it? By default it's disabled, because by
> default there is no querier on the link. So anyone wanting to use
> multicast snooping will need to make sure there's a querier in the
> network.
>
Indeed, also you could create the bridge with explicit mcast parameters if you
need
different behaviour on start. Unfortunately I think you'll have to handle
the querier state.
>
> Overall I think the querier (election) mechanism in the standards could
> need an update. While the lowest-address first might have
> worked well back then, in uniform, fully wired networks where the
> position of the querier did not matter, this is not a good
> solution anymore in networks involving wireless, dynamic connections.
> Especially in wireless mesh networks this is a bit of an issue for
> us. Ideally, the querier mechanism were dismissed in favour of simply
> unsolicited, periodic IGMP/MLD reports...
>
> But of course, updating IETF standards is no solution for now.
>
> While more complicated, it would not be impossible to consider the
> querier state, would it? I mean you probably already need to
> consider the case of a user disabling multicast snooping during
> runtime, right? So similarly, you could react to appearing or
> disappearing queriers?
>
> Cheers, Linus
>
Thanks,
Nik
Re: [B.A.T.M.A.N.] [PATCH net-next 0/4] bridge: implement Multicast Router Discovery (RFC4286)
On 12/21/18 5:15 PM, Linus Lüssing wrote:
> Hi,
>
> This patchset adds initial Multicast Router Discovery support to
> the Linux bridge (RFC4286). With MRD it is possible to detect multicast
> routers and mark bridge ports and forward multicast packets to such routers
> accordingly.
>
> So far, multicast routers are detected via IGMP/MLD queries and PIM
> messages in the Linux bridge. As there is only one active, selected
> querier at a time RFC4541 ("Considerations for Internet Group Management
> Protocol (IGMP) and Multicast Listener Discovery (MLD) Snooping
> Switches") section 2.1.1.a) recommends snooping Multicast Router
> Advertisements as provided by MRD (RFC4286).
>
>
> The first two patches are refactoring some existing code which is reused
> for parsing the Multicast Router Advertisements later in the fourth
> patch. The third patch lets the bridge join the all-snoopers multicast
> address to be able to reliably receive the Multicast Router
> Advertisements.
>
>
> What is not implemented yet from RFC4286 yet:
>
> * Sending Multicast Router Solicitations:
> -> RFC4286: "[...] may be sent when [...] an interface is
> (re-)initialized [or] MRD is enabled"
> * Snooping Multicast Router Terminations:
> -> currently this only relies on our own timeouts
> * Adjusting timeouts with the values provided in the announcements
>
>
> Regards, Linus
>
>
>
Hi Linus,
Nice work, unfortunately net-next is currenty closed. Anyway I'll
review the patches in detail after the holidays so if there's
anything it can be adjusted for when net-next opens up.
Thanks,
Nik
