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]
Date:   Tue, 01 Mar 2022 23:43:41 +0100
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>,
        Mattias Forsblad <mattias.forsblad+netdev@...il.com>
Subject: Re: [PATCH 1/3] net: bridge: Implement bridge flag local_receive

On 1 March 2022 13:31:02 CET, Mattias Forsblad <mattias.forsblad@...il.com> wrote:
>This patch implements the bridge flag local_receive. When this
>flag is cleared packets received on bridge ports will not be forwarded up.
>This makes is possible to only forward traffic between the port members
>of the bridge.
>
>Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@...il.com>
>---
> include/linux/if_bridge.h      |  6 ++++++
> include/net/switchdev.h        |  2 ++
> include/uapi/linux/if_bridge.h |  1 +
> include/uapi/linux/if_link.h   |  1 +
> net/bridge/br.c                | 18 ++++++++++++++++++
> net/bridge/br_device.c         |  1 +
> net/bridge/br_input.c          |  3 +++
> net/bridge/br_ioctl.c          |  1 +
> net/bridge/br_netlink.c        | 14 +++++++++++++-
> net/bridge/br_private.h        |  2 ++
> net/bridge/br_sysfs_br.c       | 23 +++++++++++++++++++++++
> net/bridge/br_vlan.c           |  8 ++++++++
> 12 files changed, 79 insertions(+), 1 deletion(-)
>
>diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
>index 3aae023a9353..e6b77d18c1d2 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_local_receive_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_local_receive_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/net/switchdev.h b/include/net/switchdev.h
>index 3e424d40fae3..aec5c1f9b5c7 100644
>--- a/include/net/switchdev.h
>+++ b/include/net/switchdev.h
>@@ -25,6 +25,7 @@ enum switchdev_attr_id {
> 	SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME,
> 	SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING,
> 	SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL,
>+	SWITCHDEV_ATTR_ID_BRIDGE_LOCAL_RECEIVE,
> 	SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED,
> 	SWITCHDEV_ATTR_ID_BRIDGE_MROUTER,
> 	SWITCHDEV_ATTR_ID_MRP_PORT_ROLE,
>@@ -50,6 +51,7 @@ struct switchdev_attr {
> 		u16 vlan_protocol;			/* BRIDGE_VLAN_PROTOCOL */
> 		bool mc_disabled;			/* MC_DISABLED */
> 		u8 mrp_port_role;			/* MRP_PORT_ROLE */
>+		bool local_receive;			/* BRIDGE_LOCAL_RECEIVE */
> 	} u;
> };
> 
>diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>index 2711c3522010..fc889b5ccd69 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 local_receive;
> };
> 
> struct __port_info {
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index e315e53125f4..bb7c25e1c89c 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -482,6 +482,7 @@ enum {
> 	IFLA_BR_VLAN_STATS_PER_PORT,
> 	IFLA_BR_MULTI_BOOLOPT,
> 	IFLA_BR_MCAST_QUERIER_STATE,
>+	IFLA_BR_LOCAL_RECEIVE,

Please use the boolopt api for new boolean options
We're trying to limit the nl options expansion as the bridge is the
largest user.

> 	__IFLA_BR_MAX,
> };
> 
>diff --git a/net/bridge/br.c b/net/bridge/br.c
>index b1dea3febeea..ff7eb4f269ec 100644
>--- a/net/bridge/br.c
>+++ b/net/bridge/br.c
>@@ -325,6 +325,24 @@ void br_boolopt_multi_get(const struct net_bridge *br,
> 	bm->optmask = GENMASK((BR_BOOLOPT_MAX - 1), 0);
> }
> 
>+int br_local_receive_change(struct net_bridge *p,
>+			    bool local_receive)
>+{
>+	struct switchdev_attr attr = {
>+		.orig_dev = p->dev,
>+		.id = SWITCHDEV_ATTR_ID_BRIDGE_LOCAL_RECEIVE,
>+		.flags = SWITCHDEV_F_DEFER,
>+		.u.local_receive = local_receive,
>+	};
>+	int ret;
>+
>+	ret = switchdev_port_attr_set(p->dev, &attr, NULL);
>+	if (!ret)
>+		br_opt_toggle(p, BROPT_LOCAL_RECEIVE, local_receive);
>+
>+	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..7cd9c5880d18 100644
>--- a/net/bridge/br_device.c
>+++ b/net/bridge/br_device.c
>@@ -524,6 +524,7 @@ 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_LOCAL_RECEIVE, 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..5864b61157d3 100644
>--- a/net/bridge/br_input.c
>+++ b/net/bridge/br_input.c
>@@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> 		break;
> 	}
> 
>+	if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE))
>+		local_rcv = false;
>+

this affects the whole fast path, it can be better localized to make sure
it will not affect all use cases

> 	if (dst) {
> 		unsigned long now = jiffies;
> 
>diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c
>index f213ed108361..abe538129290 100644
>--- a/net/bridge/br_ioctl.c
>+++ b/net/bridge/br_ioctl.c
>@@ -177,6 +177,7 @@ int br_dev_siocdevprivate(struct net_device *dev, struct ifreq *rq,
> 		b.topology_change = br->topology_change;
> 		b.topology_change_detected = br->topology_change_detected;
> 		b.root_port = br->root_port;
>+		b.local_receive = br_opt_get(br, BROPT_LOCAL_RECEIVE) ? 1 : 0;

ioctl is not being extended anymore, please drop it

> 
> 		b.stp_enabled = (br->stp_enabled != BR_NO_STP);
> 		b.ageing_time = jiffies_to_clock_t(br->ageing_time);
>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>index 7d4432ca9a20..5e7f99950195 100644
>--- a/net/bridge/br_netlink.c
>+++ b/net/bridge/br_netlink.c
>@@ -1163,6 +1163,7 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = {
> 	[IFLA_BR_MCAST_IGMP_VERSION] = { .type = NLA_U8 },
> 	[IFLA_BR_MCAST_MLD_VERSION] = { .type = NLA_U8 },
> 	[IFLA_BR_VLAN_STATS_PER_PORT] = { .type = NLA_U8 },
>+	[IFLA_BR_LOCAL_RECEIVE] = { .type = NLA_U8 },
> 	[IFLA_BR_MULTI_BOOLOPT] =
> 		NLA_POLICY_EXACT_LEN(sizeof(struct br_boolopt_multi)),
> };
>@@ -1434,6 +1435,14 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
> 			return err;
> 	}
> 
>+	if (data[IFLA_BR_LOCAL_RECEIVE]) {
>+		u8 val = nla_get_u8(data[IFLA_BR_LOCAL_RECEIVE]);
>+
>+		err = br_local_receive_change(br, !!val);
>+		if (err)
>+			return err;
>+	}
>+
> 	return 0;
> }
> 
>@@ -1514,6 +1523,7 @@ static size_t br_get_size(const struct net_device *brdev)
> 	       nla_total_size(sizeof(u8)) +     /* IFLA_BR_NF_CALL_ARPTABLES */
> #endif
> 	       nla_total_size(sizeof(struct br_boolopt_multi)) + /* IFLA_BR_MULTI_BOOLOPT */
>+	       nla_total_size(sizeof(u8)) +     /* IFLA_BR_LOCAL_RECEIVE */
> 	       0;
> }
> 
>@@ -1527,6 +1537,7 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev)
> 	u32 stp_enabled = br->stp_enabled;
> 	u16 priority = (br->bridge_id.prio[0] << 8) | br->bridge_id.prio[1];
> 	u8 vlan_enabled = br_vlan_enabled(br->dev);
>+	u8 local_receive = br_opt_get(br, BROPT_LOCAL_RECEIVE) ? 1 : 0;
> 	struct br_boolopt_multi bm;
> 	u64 clockval;
> 
>@@ -1563,7 +1574,8 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev)
> 	    nla_put_u8(skb, IFLA_BR_TOPOLOGY_CHANGE_DETECTED,
> 		       br->topology_change_detected) ||
> 	    nla_put(skb, IFLA_BR_GROUP_ADDR, ETH_ALEN, br->group_addr) ||
>-	    nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm))
>+	    nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm) ||
>+	    nla_put_u8(skb, IFLA_BR_LOCAL_RECEIVE, local_receive))
> 		return -EMSGSIZE;
> 
> #ifdef CONFIG_BRIDGE_VLAN_FILTERING
>diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>index 48bc61ebc211..01fa5426094b 100644
>--- a/net/bridge/br_private.h
>+++ b/net/bridge/br_private.h
>@@ -445,6 +445,7 @@ enum net_bridge_opts {
> 	BROPT_NO_LL_LEARN,
> 	BROPT_VLAN_BRIDGE_BINDING,
> 	BROPT_MCAST_VLAN_SNOOPING_ENABLED,
>+	BROPT_LOCAL_RECEIVE,
> };
> 
> struct net_bridge {
>@@ -720,6 +721,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_local_receive_change(struct net_bridge *p, bool local_receive);
> 
> /* br_device.c */
> void br_dev_setup(struct net_device *dev);
>diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
>index 3f7ca88c2aa3..9af0c2ba929c 100644
>--- a/net/bridge/br_sysfs_br.c
>+++ b/net/bridge/br_sysfs_br.c
>@@ -84,6 +84,28 @@ static ssize_t forward_delay_store(struct device *d,
> }
> static DEVICE_ATTR_RW(forward_delay);
> 
>+static ssize_t local_receive_show(struct device *d,
>+				  struct device_attribute *attr, char *buf)
>+{
>+	struct net_bridge *br = to_bridge(d);
>+
>+	return sprintf(buf, "%u\n", br_opt_get(br, BROPT_LOCAL_RECEIVE));
>+}
>+
>+static int set_local_receive(struct net_bridge *br, unsigned long val,
>+			     struct netlink_ext_ack *extack)
>+{
>+	return br_local_receive_change(br, !!val);
>+}
>+
>+static ssize_t local_receive_store(struct device *d,
>+				   struct device_attribute *attr,
>+				   const char *buf, size_t len)
>+{
>+	return store_bridge_parm(d, buf, len, set_local_receive);
>+}
>+static DEVICE_ATTR_RW(local_receive);
>+

Drop sysfs too, netlink only

> static ssize_t hello_time_show(struct device *d, struct device_attribute *attr,
> 			       char *buf)
> {
>@@ -950,6 +972,7 @@ static struct attribute *bridge_attrs[] = {
> 	&dev_attr_group_addr.attr,
> 	&dev_attr_flush.attr,
> 	&dev_attr_no_linklocal_learn.attr,
>+	&dev_attr_local_receive.attr,
> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> 	&dev_attr_multicast_router.attr,
> 	&dev_attr_multicast_snooping.attr,
>diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
>index 7557e90b60e1..57dd14d5e360 100644
>--- a/net/bridge/br_vlan.c
>+++ b/net/bridge/br_vlan.c
>@@ -905,6 +905,14 @@ bool br_vlan_enabled(const struct net_device *dev)
> }
> EXPORT_SYMBOL_GPL(br_vlan_enabled);
> 
>+bool br_local_receive_enabled(const struct net_device *dev)
>+{
>+	struct net_bridge *br = netdev_priv(dev);
>+
>+	return br_opt_get(br, BROPT_LOCAL_RECEIVE);
>+}
>+EXPORT_SYMBOL_GPL(br_local_receive_enabled);
>+

What the hell is this doing in br_vlan.c???

> int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto)
> {
> 	struct net_bridge *br = netdev_priv(dev);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ