linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode
       [not found] <20220209130538.533699-1-schultz.hans+netdev@gmail.com>
@ 2022-02-09 13:05 ` Hans Schultz
  2022-02-10  8:30   ` Nikolay Aleksandrov
  2022-02-09 13:05 ` [PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag Hans Schultz
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Hans Schultz @ 2022-02-09 13:05 UTC (permalink / raw)
  To: davem, kuba
  Cc: netdev, Hans Schultz, Roopa Prabhu, Nikolay Aleksandrov,
	linux-kernel, bridge

In a 802.1X scenario, clients connected to a bridge port shall not
be allowed to have traffic forwarded until fully authenticated.
A static fdb entry of the clients MAC address for the bridge port
unlocks the client and allows bidirectional communication.

This scenario is facilitated with setting the bridge port in locked
mode, which is also supported by various switchcore chipsets.

Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
---
 include/linux/if_bridge.h    |  1 +
 include/uapi/linux/if_link.h |  1 +
 net/bridge/br_input.c        | 10 +++++++++-
 net/bridge/br_netlink.c      |  6 +++++-
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index 509e18c7e740..3aae023a9353 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -58,6 +58,7 @@ struct br_ip_list {
 #define BR_MRP_LOST_CONT	BIT(18)
 #define BR_MRP_LOST_IN_CONT	BIT(19)
 #define BR_TX_FWD_OFFLOAD	BIT(20)
+#define BR_PORT_LOCKED		BIT(21)
 
 #define BR_DEFAULT_AGEING_TIME	(300 * HZ)
 
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 6218f93f5c1a..a45cc0a1f415 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -537,6 +537,7 @@ enum {
 	IFLA_BRPORT_MRP_IN_OPEN,
 	IFLA_BRPORT_MCAST_EHT_HOSTS_LIMIT,
 	IFLA_BRPORT_MCAST_EHT_HOSTS_CNT,
+	IFLA_BRPORT_LOCKED,
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index b50382f957c1..469e3adbce07 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -69,6 +69,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
 	struct net_bridge_port *p = br_port_get_rcu(skb->dev);
 	enum br_pkt_type pkt_type = BR_PKT_UNICAST;
 	struct net_bridge_fdb_entry *dst = NULL;
+	struct net_bridge_fdb_entry *fdb_entry;
 	struct net_bridge_mcast_port *pmctx;
 	struct net_bridge_mdb_entry *mdst;
 	bool local_rcv, mcast_hit = false;
@@ -81,6 +82,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
 	if (!p || p->state == BR_STATE_DISABLED)
 		goto drop;
 
+	br = p->br;
+
 	brmctx = &p->br->multicast_ctx;
 	pmctx = &p->multicast_ctx;
 	state = p->state;
@@ -88,10 +91,15 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
 				&state, &vlan))
 		goto out;
 
+	if (p->flags & BR_PORT_LOCKED) {
+		fdb_entry = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
+		if (!(fdb_entry && fdb_entry->dst == p))
+			goto drop;
+	}
+
 	nbp_switchdev_frame_mark(p, skb);
 
 	/* insert into forwarding database after filtering to avoid spoofing */
-	br = p->br;
 	if (p->flags & BR_LEARNING)
 		br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, 0);
 
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 2ff83d84230d..7d4432ca9a20 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -184,6 +184,7 @@ static inline size_t br_port_info_size(void)
 		+ nla_total_size(1)	/* IFLA_BRPORT_VLAN_TUNNEL */
 		+ nla_total_size(1)	/* IFLA_BRPORT_NEIGH_SUPPRESS */
 		+ nla_total_size(1)	/* IFLA_BRPORT_ISOLATED */
+		+ nla_total_size(1)	/* IFLA_BRPORT_LOCKED */
 		+ nla_total_size(sizeof(struct ifla_bridge_id))	/* IFLA_BRPORT_ROOT_ID */
 		+ nla_total_size(sizeof(struct ifla_bridge_id))	/* IFLA_BRPORT_BRIDGE_ID */
 		+ nla_total_size(sizeof(u16))	/* IFLA_BRPORT_DESIGNATED_PORT */
@@ -269,7 +270,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
 							  BR_MRP_LOST_CONT)) ||
 	    nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN,
 		       !!(p->flags & BR_MRP_LOST_IN_CONT)) ||
-	    nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)))
+	    nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)) ||
+	    nla_put_u8(skb, IFLA_BRPORT_LOCKED, !!(p->flags & BR_PORT_LOCKED)))
 		return -EMSGSIZE;
 
 	timerval = br_timer_value(&p->message_age_timer);
@@ -827,6 +829,7 @@ static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
 	[IFLA_BRPORT_GROUP_FWD_MASK] = { .type = NLA_U16 },
 	[IFLA_BRPORT_NEIGH_SUPPRESS] = { .type = NLA_U8 },
 	[IFLA_BRPORT_ISOLATED]	= { .type = NLA_U8 },
+	[IFLA_BRPORT_LOCKED] = { .type = NLA_U8 },
 	[IFLA_BRPORT_BACKUP_PORT] = { .type = NLA_U32 },
 	[IFLA_BRPORT_MCAST_EHT_HOSTS_LIMIT] = { .type = NLA_U32 },
 };
@@ -893,6 +896,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[],
 	br_set_port_flag(p, tb, IFLA_BRPORT_VLAN_TUNNEL, BR_VLAN_TUNNEL);
 	br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS, BR_NEIGH_SUPPRESS);
 	br_set_port_flag(p, tb, IFLA_BRPORT_ISOLATED, BR_ISOLATED);
+	br_set_port_flag(p, tb, IFLA_BRPORT_LOCKED, BR_PORT_LOCKED);
 
 	changed_mask = old_flags ^ p->flags;
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag
       [not found] <20220209130538.533699-1-schultz.hans+netdev@gmail.com>
  2022-02-09 13:05 ` [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode Hans Schultz
@ 2022-02-09 13:05 ` Hans Schultz
  2022-02-10  8:30   ` Nikolay Aleksandrov
  2022-02-09 13:05 ` [PATCH net-next v2 3/5] net: dsa: Add support for offloaded " Hans Schultz
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Hans Schultz @ 2022-02-09 13:05 UTC (permalink / raw)
  To: davem, kuba
  Cc: netdev, Hans Schultz, Roopa Prabhu, Nikolay Aleksandrov, bridge,
	linux-kernel

Various switchcores support setting ports in locked mode, so that
clients behind locked ports cannot send traffic through the port
unless a fdb entry is added with the clients MAC address.

Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
---
 net/bridge/br_switchdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index f8fbaaa7c501..bf549fc22556 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -72,7 +72,7 @@ bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p,
 
 /* Flags that can be offloaded to hardware */
 #define BR_PORT_FLAGS_HW_OFFLOAD (BR_LEARNING | BR_FLOOD | \
-				  BR_MCAST_FLOOD | BR_BCAST_FLOOD)
+				  BR_MCAST_FLOOD | BR_BCAST_FLOOD | BR_PORT_LOCKED)
 
 int br_switchdev_set_port_flag(struct net_bridge_port *p,
 			       unsigned long flags,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 3/5] net: dsa: Add support for offloaded locked port flag
       [not found] <20220209130538.533699-1-schultz.hans+netdev@gmail.com>
  2022-02-09 13:05 ` [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode Hans Schultz
  2022-02-09 13:05 ` [PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag Hans Schultz
@ 2022-02-09 13:05 ` Hans Schultz
  2022-02-10 17:09   ` Vladimir Oltean
  2022-02-09 13:05 ` [PATCH net-next v2 4/5] net: dsa: mv88e6xxx: Add support for bridge port locked mode Hans Schultz
  2022-02-09 13:05 ` [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels Hans Schultz
  4 siblings, 1 reply; 11+ messages in thread
From: Hans Schultz @ 2022-02-09 13:05 UTC (permalink / raw)
  To: davem, kuba
  Cc: netdev, Hans Schultz, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, linux-kernel

Among the switchcores that support this feature is the Marvell
mv88e6xxx family.

Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
---
 net/dsa/port.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dsa/port.c b/net/dsa/port.c
index bd78192e0e47..01ed22ed74a1 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -176,7 +176,7 @@ static int dsa_port_inherit_brport_flags(struct dsa_port *dp,
 					 struct netlink_ext_ack *extack)
 {
 	const unsigned long mask = BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
-				   BR_BCAST_FLOOD;
+				   BR_BCAST_FLOOD | BR_PORT_LOCKED;
 	struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
 	int flag, err;
 
@@ -200,7 +200,7 @@ static void dsa_port_clear_brport_flags(struct dsa_port *dp)
 {
 	const unsigned long val = BR_FLOOD | BR_MCAST_FLOOD | BR_BCAST_FLOOD;
 	const unsigned long mask = BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
-				   BR_BCAST_FLOOD;
+				   BR_BCAST_FLOOD | BR_PORT_LOCKED;
 	int flag, err;
 
 	for_each_set_bit(flag, &mask, 32) {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 4/5] net: dsa: mv88e6xxx: Add support for bridge port locked mode
       [not found] <20220209130538.533699-1-schultz.hans+netdev@gmail.com>
                   ` (2 preceding siblings ...)
  2022-02-09 13:05 ` [PATCH net-next v2 3/5] net: dsa: Add support for offloaded " Hans Schultz
@ 2022-02-09 13:05 ` Hans Schultz
  2022-02-09 13:05 ` [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels Hans Schultz
  4 siblings, 0 replies; 11+ messages in thread
From: Hans Schultz @ 2022-02-09 13:05 UTC (permalink / raw)
  To: davem, kuba
  Cc: netdev, Hans Schultz, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, Vladimir Oltean, linux-kernel

Supporting bridge ports in locked mode using the drop on lock
feature in Marvell mv88e6xxx switchcores is described in the
'88E6096/88E6097/88E6097F Datasheet', sections 4.4.6, 4.4.7 and
5.1.2.1 (Drop on Lock).

This feature is implemented here facilitated by the locked port flag.

Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c |  9 ++++++++-
 drivers/net/dsa/mv88e6xxx/port.c | 33 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/port.h |  9 ++++++++-
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 58ca684d73f7..eed3713b97ae 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -5881,7 +5881,7 @@ static int mv88e6xxx_port_pre_bridge_flags(struct dsa_switch *ds, int port,
 	const struct mv88e6xxx_ops *ops;
 
 	if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
-			   BR_BCAST_FLOOD))
+			   BR_BCAST_FLOOD | BR_PORT_LOCKED))
 		return -EINVAL;
 
 	ops = chip->info->ops;
@@ -5939,6 +5939,13 @@ static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
 			goto out;
 	}
 
+	if (flags.mask & BR_PORT_LOCKED) {
+		bool locked = !!(flags.val & BR_PORT_LOCKED);
+
+		err = mv88e6xxx_port_set_lock(chip, port, locked);
+		if (err)
+			goto out;
+	}
 out:
 	mv88e6xxx_reg_unlock(chip);
 
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index ab41619a809b..46b7381899a0 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -1234,6 +1234,39 @@ int mv88e6xxx_port_set_mirror(struct mv88e6xxx_chip *chip, int port,
 	return err;
 }
 
+int mv88e6xxx_port_set_lock(struct mv88e6xxx_chip *chip, int port,
+			    bool locked)
+{
+	u16 reg;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_CTL0, &reg);
+	if (err)
+		return err;
+
+	reg &= ~MV88E6XXX_PORT_CTL0_SA_FILT_MASK;
+	if (locked)
+		reg |= MV88E6XXX_PORT_CTL0_SA_FILT_DROP_ON_LOCK;
+
+	err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL0, reg);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_ASSOC_VECTOR, &reg);
+	if (err)
+		return err;
+
+	reg &= ~MV88E6XXX_PORT_ASSOC_VECTOR_LOCKED_PORT;
+	if (locked)
+		reg |= MV88E6XXX_PORT_ASSOC_VECTOR_LOCKED_PORT;
+
+	err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_ASSOC_VECTOR, reg);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip *chip, int port,
 				  u16 mode)
 {
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 03382b66f800..3f70557f0d48 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -142,7 +142,11 @@
 /* Offset 0x04: Port Control Register */
 #define MV88E6XXX_PORT_CTL0					0x04
 #define MV88E6XXX_PORT_CTL0_USE_CORE_TAG			0x8000
-#define MV88E6XXX_PORT_CTL0_DROP_ON_LOCK			0x4000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_MASK			0xc000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_DISABLED			0x0000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_DROP_ON_LOCK		0x4000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_DROP_ON_UNLOCK		0x8000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_DROP_ON_CPU		0xc000
 #define MV88E6XXX_PORT_CTL0_EGRESS_MODE_MASK			0x3000
 #define MV88E6XXX_PORT_CTL0_EGRESS_MODE_UNMODIFIED		0x0000
 #define MV88E6XXX_PORT_CTL0_EGRESS_MODE_UNTAGGED		0x1000
@@ -365,6 +369,9 @@ int mv88e6xxx_port_set_fid(struct mv88e6xxx_chip *chip, int port, u16 fid);
 int mv88e6xxx_port_get_pvid(struct mv88e6xxx_chip *chip, int port, u16 *pvid);
 int mv88e6xxx_port_set_pvid(struct mv88e6xxx_chip *chip, int port, u16 pvid);
 
+int mv88e6xxx_port_set_lock(struct mv88e6xxx_chip *chip, int port,
+			    bool locked);
+
 int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip *chip, int port,
 				  u16 mode);
 int mv88e6095_port_tag_remap(struct mv88e6xxx_chip *chip, int port);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels
       [not found] <20220209130538.533699-1-schultz.hans+netdev@gmail.com>
                   ` (3 preceding siblings ...)
  2022-02-09 13:05 ` [PATCH net-next v2 4/5] net: dsa: mv88e6xxx: Add support for bridge port locked mode Hans Schultz
@ 2022-02-09 13:05 ` Hans Schultz
  2022-02-10  8:31   ` Nikolay Aleksandrov
  4 siblings, 1 reply; 11+ messages in thread
From: Hans Schultz @ 2022-02-09 13:05 UTC (permalink / raw)
  To: davem, kuba
  Cc: netdev, Hans Schultz, Hans Schultz, Roopa Prabhu,
	Nikolay Aleksandrov, bridge, linux-kernel

From: Hans Schultz <schultz.hans+lkml@gmail.com>

As the locked mode feature is in the hot path of the bridge modules
reception of packets, it needs to be refactored to use jump labels
for optimization.

Signed-off-by: Hans Schultz <schultz.hans+lkml@gmail.com>
Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
---
 net/bridge/br_input.c   | 22 ++++++++++++++++++----
 net/bridge/br_netlink.c |  6 ++++++
 net/bridge/br_private.h |  2 ++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 469e3adbce07..6fc428d6bac5 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -23,6 +23,18 @@
 #include "br_private.h"
 #include "br_private_tunnel.h"
 
+static struct static_key_false br_input_locked_port_feature;
+
+void br_input_locked_port_add(void)
+{
+	static_branch_inc(&br_input_locked_port_feature);
+}
+
+void br_input_locked_port_remove(void)
+{
+	static_branch_dec(&br_input_locked_port_feature);
+}
+
 static int
 br_netif_receive_skb(struct net *net, struct sock *sk, struct sk_buff *skb)
 {
@@ -91,10 +103,12 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
 				&state, &vlan))
 		goto out;
 
-	if (p->flags & BR_PORT_LOCKED) {
-		fdb_entry = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
-		if (!(fdb_entry && fdb_entry->dst == p))
-			goto drop;
+	if (static_branch_unlikely(&br_input_locked_port_feature)) {
+		if (p->flags & BR_PORT_LOCKED) {
+			fdb_entry = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
+			if (!(fdb_entry && fdb_entry->dst == p))
+				goto drop;
+		}
 	}
 
 	nbp_switchdev_frame_mark(p, skb);
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 7d4432ca9a20..e3dbe9fed75c 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -860,6 +860,7 @@ static int br_set_port_state(struct net_bridge_port *p, u8 state)
 static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
 			     int attrtype, unsigned long mask)
 {
+	bool locked = p->flags & BR_PORT_LOCKED;
 	if (!tb[attrtype])
 		return;
 
@@ -867,6 +868,11 @@ static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
 		p->flags |= mask;
 	else
 		p->flags &= ~mask;
+
+	if ((p->flags & BR_PORT_LOCKED) && !locked)
+		br_input_locked_port_add();
+	if (!(p->flags & BR_PORT_LOCKED) && locked)
+		br_input_locked_port_remove();
 }
 
 /* Process bridge protocol info on port */
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 2661dda1a92b..0ec3ef897978 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -832,6 +832,8 @@ void br_manage_promisc(struct net_bridge *br);
 int nbp_backup_change(struct net_bridge_port *p, struct net_device *backup_dev);
 
 /* br_input.c */
+void br_input_locked_port_add(void);
+void br_input_locked_port_remove(void);
 int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb);
 rx_handler_func_t *br_get_rx_handler(const struct net_device *dev);
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode
  2022-02-09 13:05 ` [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode Hans Schultz
@ 2022-02-10  8:30   ` Nikolay Aleksandrov
  2022-02-10 10:20     ` Ido Schimmel
  0 siblings, 1 reply; 11+ messages in thread
From: Nikolay Aleksandrov @ 2022-02-10  8:30 UTC (permalink / raw)
  To: Hans Schultz, davem, kuba
  Cc: netdev, Hans Schultz, Roopa Prabhu, Nikolay Aleksandrov,
	linux-kernel, bridge

On 09/02/2022 15:05, Hans Schultz wrote:
> In a 802.1X scenario, clients connected to a bridge port shall not
> be allowed to have traffic forwarded until fully authenticated.
> A static fdb entry of the clients MAC address for the bridge port
> unlocks the client and allows bidirectional communication.
> 
> This scenario is facilitated with setting the bridge port in locked
> mode, which is also supported by various switchcore chipsets.
> 
> Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
> ---

Hi,
I'm writing from my private email because for some reason I'm not receiving the full
patch-set in my nvidia mail, a few comments below..

>  include/linux/if_bridge.h    |  1 +
>  include/uapi/linux/if_link.h |  1 +
>  net/bridge/br_input.c        | 10 +++++++++-
>  net/bridge/br_netlink.c      |  6 +++++-
>  4 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> index 509e18c7e740..3aae023a9353 100644
> --- a/include/linux/if_bridge.h
> +++ b/include/linux/if_bridge.h
> @@ -58,6 +58,7 @@ struct br_ip_list {
>  #define BR_MRP_LOST_CONT	BIT(18)
>  #define BR_MRP_LOST_IN_CONT	BIT(19)
>  #define BR_TX_FWD_OFFLOAD	BIT(20)
> +#define BR_PORT_LOCKED		BIT(21)
>  
>  #define BR_DEFAULT_AGEING_TIME	(300 * HZ)
>  
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 6218f93f5c1a..a45cc0a1f415 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -537,6 +537,7 @@ enum {
>  	IFLA_BRPORT_MRP_IN_OPEN,
>  	IFLA_BRPORT_MCAST_EHT_HOSTS_LIMIT,
>  	IFLA_BRPORT_MCAST_EHT_HOSTS_CNT,
> +	IFLA_BRPORT_LOCKED,
>  	__IFLA_BRPORT_MAX
>  };
>  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index b50382f957c1..469e3adbce07 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -69,6 +69,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  	struct net_bridge_port *p = br_port_get_rcu(skb->dev);
>  	enum br_pkt_type pkt_type = BR_PKT_UNICAST;
>  	struct net_bridge_fdb_entry *dst = NULL;
> +	struct net_bridge_fdb_entry *fdb_entry;

move fdb_entry below to where it is used

>  	struct net_bridge_mcast_port *pmctx;
>  	struct net_bridge_mdb_entry *mdst;
>  	bool local_rcv, mcast_hit = false;
> @@ -81,6 +82,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  	if (!p || p->state == BR_STATE_DISABLED)
>  		goto drop;
>  
> +	br = p->br;
> +

please drop the extra new line

>  	brmctx = &p->br->multicast_ctx;
>  	pmctx = &p->multicast_ctx;
>  	state = p->state;
> @@ -88,10 +91,15 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  				&state, &vlan))
>  		goto out;
>  
> +	if (p->flags & BR_PORT_LOCKED) {

fdb_entry should be defined in this scope only, and please rename it to something
like fdb_src or just "src" as we already have "dst".

> +		fdb_entry = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
> +		if (!(fdb_entry && fdb_entry->dst == p))

if (!fdb_entry || READ_ONCE(fdb_entry->dst) != p

> +			goto drop;
> +	}
> +
>  	nbp_switchdev_frame_mark(p, skb);
>  
>  	/* insert into forwarding database after filtering to avoid spoofing */
> -	br = p->br;
>  	if (p->flags & BR_LEARNING)
>  		br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, 0);
>  
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 2ff83d84230d..7d4432ca9a20 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -184,6 +184,7 @@ static inline size_t br_port_info_size(void)
>  		+ nla_total_size(1)	/* IFLA_BRPORT_VLAN_TUNNEL */
>  		+ nla_total_size(1)	/* IFLA_BRPORT_NEIGH_SUPPRESS */
>  		+ nla_total_size(1)	/* IFLA_BRPORT_ISOLATED */
> +		+ nla_total_size(1)	/* IFLA_BRPORT_LOCKED */
>  		+ nla_total_size(sizeof(struct ifla_bridge_id))	/* IFLA_BRPORT_ROOT_ID */
>  		+ nla_total_size(sizeof(struct ifla_bridge_id))	/* IFLA_BRPORT_BRIDGE_ID */
>  		+ nla_total_size(sizeof(u16))	/* IFLA_BRPORT_DESIGNATED_PORT */
> @@ -269,7 +270,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
>  							  BR_MRP_LOST_CONT)) ||
>  	    nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN,
>  		       !!(p->flags & BR_MRP_LOST_IN_CONT)) ||
> -	    nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)))
> +	    nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)) ||
> +	    nla_put_u8(skb, IFLA_BRPORT_LOCKED, !!(p->flags & BR_PORT_LOCKED)))
>  		return -EMSGSIZE;
>  
>  	timerval = br_timer_value(&p->message_age_timer);
> @@ -827,6 +829,7 @@ static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
>  	[IFLA_BRPORT_GROUP_FWD_MASK] = { .type = NLA_U16 },
>  	[IFLA_BRPORT_NEIGH_SUPPRESS] = { .type = NLA_U8 },
>  	[IFLA_BRPORT_ISOLATED]	= { .type = NLA_U8 },
> +	[IFLA_BRPORT_LOCKED] = { .type = NLA_U8 },
>  	[IFLA_BRPORT_BACKUP_PORT] = { .type = NLA_U32 },
>  	[IFLA_BRPORT_MCAST_EHT_HOSTS_LIMIT] = { .type = NLA_U32 },
>  };
> @@ -893,6 +896,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[],
>  	br_set_port_flag(p, tb, IFLA_BRPORT_VLAN_TUNNEL, BR_VLAN_TUNNEL);
>  	br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS, BR_NEIGH_SUPPRESS);
>  	br_set_port_flag(p, tb, IFLA_BRPORT_ISOLATED, BR_ISOLATED);
> +	br_set_port_flag(p, tb, IFLA_BRPORT_LOCKED, BR_PORT_LOCKED);
>  
>  	changed_mask = old_flags ^ p->flags;
>  

Thanks,
 Nik

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag
  2022-02-09 13:05 ` [PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag Hans Schultz
@ 2022-02-10  8:30   ` Nikolay Aleksandrov
  0 siblings, 0 replies; 11+ messages in thread
From: Nikolay Aleksandrov @ 2022-02-10  8:30 UTC (permalink / raw)
  To: Hans Schultz, davem, kuba
  Cc: netdev, Hans Schultz, Roopa Prabhu, Nikolay Aleksandrov, bridge,
	linux-kernel

On 09/02/2022 15:05, Hans Schultz wrote:
> Various switchcores support setting ports in locked mode, so that
> clients behind locked ports cannot send traffic through the port
> unless a fdb entry is added with the clients MAC address.
> 
> Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
> ---
>  net/bridge/br_switchdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index f8fbaaa7c501..bf549fc22556 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -72,7 +72,7 @@ bool nbp_switchdev_allowed_egress(const struct net_bridge_port *p,
>  
>  /* Flags that can be offloaded to hardware */
>  #define BR_PORT_FLAGS_HW_OFFLOAD (BR_LEARNING | BR_FLOOD | \
> -				  BR_MCAST_FLOOD | BR_BCAST_FLOOD)
> +				  BR_MCAST_FLOOD | BR_BCAST_FLOOD | BR_PORT_LOCKED)
>  
>  int br_switchdev_set_port_flag(struct net_bridge_port *p,
>  			       unsigned long flags,

Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels
  2022-02-09 13:05 ` [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels Hans Schultz
@ 2022-02-10  8:31   ` Nikolay Aleksandrov
  2022-02-10  9:22     ` Hans Schultz
  0 siblings, 1 reply; 11+ messages in thread
From: Nikolay Aleksandrov @ 2022-02-10  8:31 UTC (permalink / raw)
  To: Hans Schultz, davem, kuba
  Cc: netdev, Hans Schultz, Hans Schultz, Roopa Prabhu,
	Nikolay Aleksandrov, bridge, linux-kernel

On 09/02/2022 15:05, Hans Schultz wrote:
> From: Hans Schultz <schultz.hans+lkml@gmail.com>
> 
> As the locked mode feature is in the hot path of the bridge modules
> reception of packets, it needs to be refactored to use jump labels
> for optimization.
> 
> Signed-off-by: Hans Schultz <schultz.hans+lkml@gmail.com>
> Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
> ---

Why two (almost) identical sign-offs?

Also, as Ido mentioned, please fold this patch into patch 01.

>  net/bridge/br_input.c   | 22 ++++++++++++++++++----
>  net/bridge/br_netlink.c |  6 ++++++
>  net/bridge/br_private.h |  2 ++
>  3 files changed, 26 insertions(+), 4 deletions(-)
> 
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index 469e3adbce07..6fc428d6bac5 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -23,6 +23,18 @@
>  #include "br_private.h"
>  #include "br_private_tunnel.h"
>  
> +static struct static_key_false br_input_locked_port_feature;
> +
> +void br_input_locked_port_add(void)
> +{
> +	static_branch_inc(&br_input_locked_port_feature);
> +}
> +
> +void br_input_locked_port_remove(void)
> +{
> +	static_branch_dec(&br_input_locked_port_feature);
> +}
> +
>  static int
>  br_netif_receive_skb(struct net *net, struct sock *sk, struct sk_buff *skb)
>  {
> @@ -91,10 +103,12 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>  				&state, &vlan))
>  		goto out;
>  
> -	if (p->flags & BR_PORT_LOCKED) {
> -		fdb_entry = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
> -		if (!(fdb_entry && fdb_entry->dst == p))
> -			goto drop;
> +	if (static_branch_unlikely(&br_input_locked_port_feature)) {
> +		if (p->flags & BR_PORT_LOCKED) {
> +			fdb_entry = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
> +			if (!(fdb_entry && fdb_entry->dst == p))
> +				goto drop;
> +		}
>  	}
>  
>  	nbp_switchdev_frame_mark(p, skb);
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 7d4432ca9a20..e3dbe9fed75c 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -860,6 +860,7 @@ static int br_set_port_state(struct net_bridge_port *p, u8 state)
>  static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
>  			     int attrtype, unsigned long mask)
>  {
> +	bool locked = p->flags & BR_PORT_LOCKED;
>  	if (!tb[attrtype])
>  		return;
>  
> @@ -867,6 +868,11 @@ static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
>  		p->flags |= mask;
>  	else
>  		p->flags &= ~mask;
> +
> +	if ((p->flags & BR_PORT_LOCKED) && !locked)
> +		br_input_locked_port_add();
> +	if (!(p->flags & BR_PORT_LOCKED) && locked)
> +		br_input_locked_port_remove();
>  }
>  
>  /* Process bridge protocol info on port */
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 2661dda1a92b..0ec3ef897978 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -832,6 +832,8 @@ void br_manage_promisc(struct net_bridge *br);
>  int nbp_backup_change(struct net_bridge_port *p, struct net_device *backup_dev);
>  
>  /* br_input.c */
> +void br_input_locked_port_add(void);
> +void br_input_locked_port_remove(void);
>  int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb);
>  rx_handler_func_t *br_get_rx_handler(const struct net_device *dev);
>  


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels
  2022-02-10  8:31   ` Nikolay Aleksandrov
@ 2022-02-10  9:22     ` Hans Schultz
  0 siblings, 0 replies; 11+ messages in thread
From: Hans Schultz @ 2022-02-10  9:22 UTC (permalink / raw)
  To: Nikolay Aleksandrov, Hans Schultz, davem, kuba
  Cc: netdev, Hans Schultz, Roopa Prabhu, Nikolay Aleksandrov, bridge,
	linux-kernel

On tor, feb 10, 2022 at 10:31, Nikolay Aleksandrov <razor@blackwall.org> wrote:
> On 09/02/2022 15:05, Hans Schultz wrote:
>> From: Hans Schultz <schultz.hans+lkml@gmail.com>
>> 
>> As the locked mode feature is in the hot path of the bridge modules
>> reception of packets, it needs to be refactored to use jump labels
>> for optimization.
>> 
>> Signed-off-by: Hans Schultz <schultz.hans+lkml@gmail.com>
>> Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
>> ---
>
> Why two (almost) identical sign-offs?

Ups, a mistake...

>
> Also, as Ido mentioned, please fold this patch into patch 01.
>
>>  net/bridge/br_input.c   | 22 ++++++++++++++++++----
>>  net/bridge/br_netlink.c |  6 ++++++
>>  net/bridge/br_private.h |  2 ++
>>  3 files changed, 26 insertions(+), 4 deletions(-)
>> 
>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>> index 469e3adbce07..6fc428d6bac5 100644
>> --- a/net/bridge/br_input.c
>> +++ b/net/bridge/br_input.c
>> @@ -23,6 +23,18 @@
>>  #include "br_private.h"
>>  #include "br_private_tunnel.h"
>>  
>> +static struct static_key_false br_input_locked_port_feature;
>> +
>> +void br_input_locked_port_add(void)
>> +{
>> +	static_branch_inc(&br_input_locked_port_feature);
>> +}
>> +
>> +void br_input_locked_port_remove(void)
>> +{
>> +	static_branch_dec(&br_input_locked_port_feature);
>> +}
>> +
>>  static int
>>  br_netif_receive_skb(struct net *net, struct sock *sk, struct sk_buff *skb)
>>  {
>> @@ -91,10 +103,12 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>  				&state, &vlan))
>>  		goto out;
>>  
>> -	if (p->flags & BR_PORT_LOCKED) {
>> -		fdb_entry = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>> -		if (!(fdb_entry && fdb_entry->dst == p))
>> -			goto drop;
>> +	if (static_branch_unlikely(&br_input_locked_port_feature)) {
>> +		if (p->flags & BR_PORT_LOCKED) {
>> +			fdb_entry = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>> +			if (!(fdb_entry && fdb_entry->dst == p))
>> +				goto drop;
>> +		}
>>  	}
>>  
>>  	nbp_switchdev_frame_mark(p, skb);
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 7d4432ca9a20..e3dbe9fed75c 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -860,6 +860,7 @@ static int br_set_port_state(struct net_bridge_port *p, u8 state)
>>  static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
>>  			     int attrtype, unsigned long mask)
>>  {
>> +	bool locked = p->flags & BR_PORT_LOCKED;
>>  	if (!tb[attrtype])
>>  		return;
>>  
>> @@ -867,6 +868,11 @@ static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
>>  		p->flags |= mask;
>>  	else
>>  		p->flags &= ~mask;
>> +
>> +	if ((p->flags & BR_PORT_LOCKED) && !locked)
>> +		br_input_locked_port_add();
>> +	if (!(p->flags & BR_PORT_LOCKED) && locked)
>> +		br_input_locked_port_remove();
>>  }
>>  
>>  /* Process bridge protocol info on port */
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 2661dda1a92b..0ec3ef897978 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -832,6 +832,8 @@ void br_manage_promisc(struct net_bridge *br);
>>  int nbp_backup_change(struct net_bridge_port *p, struct net_device *backup_dev);
>>  
>>  /* br_input.c */
>> +void br_input_locked_port_add(void);
>> +void br_input_locked_port_remove(void);
>>  int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb);
>>  rx_handler_func_t *br_get_rx_handler(const struct net_device *dev);
>>  

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode
  2022-02-10  8:30   ` Nikolay Aleksandrov
@ 2022-02-10 10:20     ` Ido Schimmel
  0 siblings, 0 replies; 11+ messages in thread
From: Ido Schimmel @ 2022-02-10 10:20 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: Hans Schultz, davem, kuba, netdev, Hans Schultz, Roopa Prabhu,
	Nikolay Aleksandrov, linux-kernel, bridge

On Thu, Feb 10, 2022 at 10:30:01AM +0200, Nikolay Aleksandrov wrote:
> On 09/02/2022 15:05, Hans Schultz wrote:
> > In a 802.1X scenario, clients connected to a bridge port shall not
> > be allowed to have traffic forwarded until fully authenticated.
> > A static fdb entry of the clients MAC address for the bridge port
> > unlocks the client and allows bidirectional communication.
> > 
> > This scenario is facilitated with setting the bridge port in locked
> > mode, which is also supported by various switchcore chipsets.
> > 
> > Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
> > ---
> 
> Hi,
> I'm writing from my private email because for some reason I'm not receiving the full
> patch-set in my nvidia mail, a few comments below..
> 
> >  include/linux/if_bridge.h    |  1 +
> >  include/uapi/linux/if_link.h |  1 +
> >  net/bridge/br_input.c        | 10 +++++++++-
> >  net/bridge/br_netlink.c      |  6 +++++-
> >  4 files changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
> > index 509e18c7e740..3aae023a9353 100644
> > --- a/include/linux/if_bridge.h
> > +++ b/include/linux/if_bridge.h
> > @@ -58,6 +58,7 @@ struct br_ip_list {
> >  #define BR_MRP_LOST_CONT	BIT(18)
> >  #define BR_MRP_LOST_IN_CONT	BIT(19)
> >  #define BR_TX_FWD_OFFLOAD	BIT(20)
> > +#define BR_PORT_LOCKED		BIT(21)
> >  
> >  #define BR_DEFAULT_AGEING_TIME	(300 * HZ)
> >  
> > diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> > index 6218f93f5c1a..a45cc0a1f415 100644
> > --- a/include/uapi/linux/if_link.h
> > +++ b/include/uapi/linux/if_link.h
> > @@ -537,6 +537,7 @@ enum {
> >  	IFLA_BRPORT_MRP_IN_OPEN,
> >  	IFLA_BRPORT_MCAST_EHT_HOSTS_LIMIT,
> >  	IFLA_BRPORT_MCAST_EHT_HOSTS_CNT,
> > +	IFLA_BRPORT_LOCKED,
> >  	__IFLA_BRPORT_MAX
> >  };
> >  #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
> > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> > index b50382f957c1..469e3adbce07 100644
> > --- a/net/bridge/br_input.c
> > +++ b/net/bridge/br_input.c
> > @@ -69,6 +69,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> >  	struct net_bridge_port *p = br_port_get_rcu(skb->dev);
> >  	enum br_pkt_type pkt_type = BR_PKT_UNICAST;
> >  	struct net_bridge_fdb_entry *dst = NULL;
> > +	struct net_bridge_fdb_entry *fdb_entry;
> 
> move fdb_entry below to where it is used
> 
> >  	struct net_bridge_mcast_port *pmctx;
> >  	struct net_bridge_mdb_entry *mdst;
> >  	bool local_rcv, mcast_hit = false;
> > @@ -81,6 +82,8 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> >  	if (!p || p->state == BR_STATE_DISABLED)
> >  		goto drop;
> >  
> > +	br = p->br;
> > +
> 
> please drop the extra new line
> 
> >  	brmctx = &p->br->multicast_ctx;
> >  	pmctx = &p->multicast_ctx;
> >  	state = p->state;
> > @@ -88,10 +91,15 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> >  				&state, &vlan))
> >  		goto out;
> >  
> > +	if (p->flags & BR_PORT_LOCKED) {
> 
> fdb_entry should be defined in this scope only, and please rename it to something
> like fdb_src or just "src" as we already have "dst".
> 
> > +		fdb_entry = br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
> > +		if (!(fdb_entry && fdb_entry->dst == p))
> 
> if (!fdb_entry || READ_ONCE(fdb_entry->dst) != p

I think we should also check that entry does not have 'BR_FDB_LOCAL'
set. These entries point at the bridge ports themselves, but do not
actually represent hosts behind the ports. Since they are automatically
populated, a malicious host can craft packets with SMAC of the bridge
port and bypass the check.

Assuming the above is true (didn't test), would be good to add a test
case for it in the selftest.

> 
> > +			goto drop;
> > +	}
> > +
> >  	nbp_switchdev_frame_mark(p, skb);
> >  
> >  	/* insert into forwarding database after filtering to avoid spoofing */
> > -	br = p->br;
> >  	if (p->flags & BR_LEARNING)
> >  		br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, 0);
> >  
> > diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> > index 2ff83d84230d..7d4432ca9a20 100644
> > --- a/net/bridge/br_netlink.c
> > +++ b/net/bridge/br_netlink.c
> > @@ -184,6 +184,7 @@ static inline size_t br_port_info_size(void)
> >  		+ nla_total_size(1)	/* IFLA_BRPORT_VLAN_TUNNEL */
> >  		+ nla_total_size(1)	/* IFLA_BRPORT_NEIGH_SUPPRESS */
> >  		+ nla_total_size(1)	/* IFLA_BRPORT_ISOLATED */
> > +		+ nla_total_size(1)	/* IFLA_BRPORT_LOCKED */
> >  		+ nla_total_size(sizeof(struct ifla_bridge_id))	/* IFLA_BRPORT_ROOT_ID */
> >  		+ nla_total_size(sizeof(struct ifla_bridge_id))	/* IFLA_BRPORT_BRIDGE_ID */
> >  		+ nla_total_size(sizeof(u16))	/* IFLA_BRPORT_DESIGNATED_PORT */
> > @@ -269,7 +270,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
> >  							  BR_MRP_LOST_CONT)) ||
> >  	    nla_put_u8(skb, IFLA_BRPORT_MRP_IN_OPEN,
> >  		       !!(p->flags & BR_MRP_LOST_IN_CONT)) ||
> > -	    nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)))
> > +	    nla_put_u8(skb, IFLA_BRPORT_ISOLATED, !!(p->flags & BR_ISOLATED)) ||
> > +	    nla_put_u8(skb, IFLA_BRPORT_LOCKED, !!(p->flags & BR_PORT_LOCKED)))
> >  		return -EMSGSIZE;
> >  
> >  	timerval = br_timer_value(&p->message_age_timer);
> > @@ -827,6 +829,7 @@ static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
> >  	[IFLA_BRPORT_GROUP_FWD_MASK] = { .type = NLA_U16 },
> >  	[IFLA_BRPORT_NEIGH_SUPPRESS] = { .type = NLA_U8 },
> >  	[IFLA_BRPORT_ISOLATED]	= { .type = NLA_U8 },
> > +	[IFLA_BRPORT_LOCKED] = { .type = NLA_U8 },
> >  	[IFLA_BRPORT_BACKUP_PORT] = { .type = NLA_U32 },
> >  	[IFLA_BRPORT_MCAST_EHT_HOSTS_LIMIT] = { .type = NLA_U32 },
> >  };
> > @@ -893,6 +896,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[],
> >  	br_set_port_flag(p, tb, IFLA_BRPORT_VLAN_TUNNEL, BR_VLAN_TUNNEL);
> >  	br_set_port_flag(p, tb, IFLA_BRPORT_NEIGH_SUPPRESS, BR_NEIGH_SUPPRESS);
> >  	br_set_port_flag(p, tb, IFLA_BRPORT_ISOLATED, BR_ISOLATED);
> > +	br_set_port_flag(p, tb, IFLA_BRPORT_LOCKED, BR_PORT_LOCKED);
> >  
> >  	changed_mask = old_flags ^ p->flags;
> >  
> 
> Thanks,
>  Nik

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next v2 3/5] net: dsa: Add support for offloaded locked port flag
  2022-02-09 13:05 ` [PATCH net-next v2 3/5] net: dsa: Add support for offloaded " Hans Schultz
@ 2022-02-10 17:09   ` Vladimir Oltean
  0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Oltean @ 2022-02-10 17:09 UTC (permalink / raw)
  To: Hans Schultz
  Cc: davem, kuba, netdev, Hans Schultz, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, linux-kernel

Hello,

Next time you send a patch version, if you're going to copy me on a
patch, can you please copy me on all of them? I have a problem with not
receiving emails in real time from netdev@vger.kernel.org, and
refreshing patchwork to see if anything has been said on the other
patches is pretty out of hand. I don't have enough information to
comment just on the DSA bits.

Thanks.

On Wed, Feb 09, 2022 at 02:05:35PM +0100, Hans Schultz wrote:
> Among the switchcores that support this feature is the Marvell
> mv88e6xxx family.
> 
> Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
> ---
>  net/dsa/port.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index bd78192e0e47..01ed22ed74a1 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -176,7 +176,7 @@ static int dsa_port_inherit_brport_flags(struct dsa_port *dp,
>  					 struct netlink_ext_ack *extack)
>  {
>  	const unsigned long mask = BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
> -				   BR_BCAST_FLOOD;
> +				   BR_BCAST_FLOOD | BR_PORT_LOCKED;
>  	struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
>  	int flag, err;
>  
> @@ -200,7 +200,7 @@ static void dsa_port_clear_brport_flags(struct dsa_port *dp)
>  {
>  	const unsigned long val = BR_FLOOD | BR_MCAST_FLOOD | BR_BCAST_FLOOD;
>  	const unsigned long mask = BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
> -				   BR_BCAST_FLOOD;
> +				   BR_BCAST_FLOOD | BR_PORT_LOCKED;
>  	int flag, err;
>  
>  	for_each_set_bit(flag, &mask, 32) {
> -- 
> 2.30.2
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-02-10 17:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220209130538.533699-1-schultz.hans+netdev@gmail.com>
2022-02-09 13:05 ` [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode Hans Schultz
2022-02-10  8:30   ` Nikolay Aleksandrov
2022-02-10 10:20     ` Ido Schimmel
2022-02-09 13:05 ` [PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag Hans Schultz
2022-02-10  8:30   ` Nikolay Aleksandrov
2022-02-09 13:05 ` [PATCH net-next v2 3/5] net: dsa: Add support for offloaded " Hans Schultz
2022-02-10 17:09   ` Vladimir Oltean
2022-02-09 13:05 ` [PATCH net-next v2 4/5] net: dsa: mv88e6xxx: Add support for bridge port locked mode Hans Schultz
2022-02-09 13:05 ` [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels Hans Schultz
2022-02-10  8:31   ` Nikolay Aleksandrov
2022-02-10  9:22     ` Hans Schultz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).