lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f2104e0e-45f2-1fe6-5cf9-ef3fa0f1475d@blackwall.org>
Date:   Thu, 17 Mar 2022 12:11:40 +0200
From:   Nikolay Aleksandrov <razor@...ckwall.org>
To:     Mattias Forsblad <mattias.forsblad@...il.com>,
        netdev@...r.kernel.org
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Roopa Prabhu <roopa@...dia.com>,
        Tobias Waldekranz <tobias@...dekranz.com>,
        Joachim Wiberg <troglobit@...il.com>
Subject: Re: [PATCH 2/5] net: bridge: Implement bridge flood flag

On 17/03/2022 08:50, Mattias Forsblad wrote:
> This patch implements the bridge flood flags. There are three different
> flags matching unicast, multicast and broadcast. When the corresponding
> flag is cleared packets received on bridge ports will not be flooded
> towards the bridge.
> This makes is possible to only forward selected traffic between the
> port members of the bridge.
> 
> Signed-off-by: Mattias Forsblad <mattias.forsblad@...il.com>
> ---
>  include/linux/if_bridge.h      |  6 +++++
>  include/uapi/linux/if_bridge.h |  9 ++++++-
>  net/bridge/br.c                | 45 ++++++++++++++++++++++++++++++++++
>  net/bridge/br_device.c         |  3 +++
>  net/bridge/br_input.c          | 23 ++++++++++++++---
>  net/bridge/br_private.h        |  4 +++
>  6 files changed, 85 insertions(+), 5 deletions(-)
> 
Please always CC bridge maintainers for bridge patches.

I almost missed this one. I'll add my reply to Joachim's notes
which are pretty spot on.

> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index 3aae023a9353..fa8e000a6fb9 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -157,6 +157,7 @@ static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid,
>  struct net_device *br_fdb_find_port(const struct net_device *br_dev,
>  				    const unsigned char *addr,
>  				    __u16 vid);
> +bool br_flood_enabled(const struct net_device *dev);
>  void br_fdb_clear_offload(const struct net_device *dev, u16 vid);
>  bool br_port_flag_is_set(const struct net_device *dev, unsigned long flag);
>  u8 br_port_get_stp_state(const struct net_device *dev);
> @@ -170,6 +171,11 @@ br_fdb_find_port(const struct net_device *br_dev,
>  	return NULL;
>  }
>  
> +static inline bool br_flood_enabled(const struct net_device *dev)
> +{
> +	return true;
> +}
> +
>  static inline void br_fdb_clear_offload(const struct net_device *dev, u16 vid)
>  {
>  }
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 2711c3522010..765ed70c9b28 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -72,6 +72,7 @@ struct __bridge_info {
>  	__u32 tcn_timer_value;
>  	__u32 topology_change_timer_value;
>  	__u32 gc_timer_value;
> +	__u8 flood;
>  };
>  
>  struct __port_info {
> @@ -752,13 +753,19 @@ struct br_mcast_stats {
>  /* bridge boolean options
>   * BR_BOOLOPT_NO_LL_LEARN - disable learning from link-local packets
>   * BR_BOOLOPT_MCAST_VLAN_SNOOPING - control vlan multicast snooping
> + * BR_BOOLOPT_FLOOD - control bridge flood flag
> + * BR_BOOLOPT_MCAST_FLOOD - control bridge multicast flood flag
> + * BR_BOOLOPT_BCAST_FLOOD - control bridge broadcast flood flag
>   *
>   * IMPORTANT: if adding a new option do not forget to handle
> - *            it in br_boolopt_toggle/get and bridge sysfs
> + *            it in br_boolopt_toggle/get
>   */
>  enum br_boolopt_id {
>  	BR_BOOLOPT_NO_LL_LEARN,
>  	BR_BOOLOPT_MCAST_VLAN_SNOOPING,
> +	BR_BOOLOPT_FLOOD,
> +	BR_BOOLOPT_MCAST_FLOOD,
> +	BR_BOOLOPT_BCAST_FLOOD,
>  	BR_BOOLOPT_MAX
>  };
>  
> diff --git a/net/bridge/br.c b/net/bridge/br.c
> index b1dea3febeea..63a17bed6c63 100644
> --- a/net/bridge/br.c
> +++ b/net/bridge/br.c
> @@ -265,6 +265,11 @@ int br_boolopt_toggle(struct net_bridge *br, enum br_boolopt_id opt, bool on,
>  	case BR_BOOLOPT_MCAST_VLAN_SNOOPING:
>  		err = br_multicast_toggle_vlan_snooping(br, on, extack);
>  		break;
> +	case BR_BOOLOPT_FLOOD:
> +	case BR_BOOLOPT_MCAST_FLOOD:
> +	case BR_BOOLOPT_BCAST_FLOOD:
> +		err = br_flood_toggle(br, opt, on);
> +		break;
>  	default:
>  		/* shouldn't be called with unsupported options */
>  		WARN_ON(1);
> @@ -281,6 +286,12 @@ int br_boolopt_get(const struct net_bridge *br, enum br_boolopt_id opt)
>  		return br_opt_get(br, BROPT_NO_LL_LEARN);
>  	case BR_BOOLOPT_MCAST_VLAN_SNOOPING:
>  		return br_opt_get(br, BROPT_MCAST_VLAN_SNOOPING_ENABLED);
> +	case BR_BOOLOPT_FLOOD:
> +		return br_opt_get(br, BROPT_FLOOD);
> +	case BR_BOOLOPT_MCAST_FLOOD:
> +		return br_opt_get(br, BROPT_MCAST_FLOOD);
> +	case BR_BOOLOPT_BCAST_FLOOD:
> +		return br_opt_get(br, BROPT_BCAST_FLOOD);
>  	default:
>  		/* shouldn't be called with unsupported options */
>  		WARN_ON(1);
> @@ -325,6 +336,40 @@ void br_boolopt_multi_get(const struct net_bridge *br,
>  	bm->optmask = GENMASK((BR_BOOLOPT_MAX - 1), 0);
>  }
>  
> +int br_flood_toggle(struct net_bridge *br, enum br_boolopt_id opt,
> +		    bool on)
> +{
> +	struct switchdev_attr attr = {
> +		.orig_dev = br->dev,
> +		.id = SWITCHDEV_ATTR_ID_BRIDGE_FLOOD,
> +		.flags = SWITCHDEV_F_DEFER,
> +	};
> +	enum net_bridge_opts bropt;
> +	int ret;
> +
> +	switch (opt) {
> +	case BR_BOOLOPT_FLOOD:
> +		bropt = BROPT_FLOOD;
> +		break;
> +	case BR_BOOLOPT_MCAST_FLOOD:
> +		bropt = BROPT_MCAST_FLOOD;
> +		break;
> +	case BR_BOOLOPT_BCAST_FLOOD:
> +		bropt = BROPT_BCAST_FLOOD;
> +		break;
> +	default:
> +		WARN_ON(1);
> +		return -EINVAL;
> +	}
> +	br_opt_toggle(br, bropt, on);
> +
> +	attr.u.brport_flags.mask = BIT(bropt);
> +	attr.u.brport_flags.val = on << bropt;
> +	ret = switchdev_port_attr_set(br->dev, &attr, NULL);
> +
> +	return ret;
> +}
> +
>  /* private bridge options, controlled by the kernel */
>  void br_opt_toggle(struct net_bridge *br, enum net_bridge_opts opt, bool on)
>  {
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index 8d6bab244c4a..fafaef9d4b3a 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -524,6 +524,9 @@ void br_dev_setup(struct net_device *dev)
>  	br->bridge_hello_time = br->hello_time = 2 * HZ;
>  	br->bridge_forward_delay = br->forward_delay = 15 * HZ;
>  	br->bridge_ageing_time = br->ageing_time = BR_DEFAULT_AGEING_TIME;
> +	br_opt_toggle(br, BROPT_FLOOD, true);
> +	br_opt_toggle(br, BROPT_MCAST_FLOOD, true);
> +	br_opt_toggle(br, BROPT_BCAST_FLOOD, true);
>  	dev->max_mtu = ETH_MAX_MTU;
>  
>  	br_netfilter_rtable_init(br);
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index e0c13fcc50ed..fcb0757bfdcc 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -109,11 +109,12 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  		/* by definition the broadcast is also a multicast address */
>  		if (is_broadcast_ether_addr(eth_hdr(skb)->h_dest)) {
>  			pkt_type = BR_PKT_BROADCAST;
> -			local_rcv = true;
> +			local_rcv = true && br_opt_get(br, BROPT_BCAST_FLOOD);
>  		} else {
>  			pkt_type = BR_PKT_MULTICAST;
> -			if (br_multicast_rcv(&brmctx, &pmctx, vlan, skb, vid))
> -				goto drop;
> +			if (br_opt_get(br, BROPT_MCAST_FLOOD))
> +				if (br_multicast_rcv(&brmctx, &pmctx, vlan, skb, vid))
> +					goto drop;
>  		}
>  	}
>  
> @@ -155,9 +156,13 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  			local_rcv = true;
>  			br->dev->stats.multicast++;
>  		}
> +		if (!br_opt_get(br, BROPT_MCAST_FLOOD))
> +			local_rcv = false;
>  		break;
>  	case BR_PKT_UNICAST:
>  		dst = br_fdb_find_rcu(br, eth_hdr(skb)->h_dest, vid);
> +		if (!br_opt_get(br, BROPT_FLOOD))
> +			local_rcv = false;
>  		break;
>  	default:
>  		break;
> @@ -166,7 +171,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  	if (dst) {
>  		unsigned long now = jiffies;
>  
> -		if (test_bit(BR_FDB_LOCAL, &dst->flags))
> +		if (test_bit(BR_FDB_LOCAL, &dst->flags) && local_rcv)
>  			return br_pass_frame_up(skb);
>  
>  		if (now != dst->used)
> @@ -190,6 +195,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  }
>  EXPORT_SYMBOL_GPL(br_handle_frame_finish);
>  
> +bool br_flood_enabled(const struct net_device *dev)
> +{
> +	struct net_bridge *br = netdev_priv(dev);
> +
> +	return !!(br_opt_get(br, BROPT_FLOOD) ||
> +		   br_opt_get(br, BROPT_MCAST_FLOOD) ||
> +		   br_opt_get(br, BROPT_BCAST_FLOOD));
> +}
> +EXPORT_SYMBOL_GPL(br_flood_enabled);
> +
>  static void __br_handle_local_finish(struct sk_buff *skb)
>  {
>  	struct net_bridge_port *p = br_port_get_rcu(skb->dev);
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 48bc61ebc211..cf88dce0b92b 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -445,6 +445,9 @@ enum net_bridge_opts {
>  	BROPT_NO_LL_LEARN,
>  	BROPT_VLAN_BRIDGE_BINDING,
>  	BROPT_MCAST_VLAN_SNOOPING_ENABLED,
> +	BROPT_FLOOD,
> +	BROPT_MCAST_FLOOD,
> +	BROPT_BCAST_FLOOD,
>  };
>  
>  struct net_bridge {
> @@ -720,6 +723,7 @@ int br_boolopt_multi_toggle(struct net_bridge *br,
>  void br_boolopt_multi_get(const struct net_bridge *br,
>  			  struct br_boolopt_multi *bm);
>  void br_opt_toggle(struct net_bridge *br, enum net_bridge_opts opt, bool on);
> +int br_flood_toggle(struct net_bridge *br, enum br_boolopt_id opt, bool on);
>  
>  /* br_device.c */
>  void br_dev_setup(struct net_device *dev);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ