[PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Hans Schultz]

This series starts by adding support for SA filtering to the bridge,
which is then allowed to be offloaded to switchdev devices. Furthermore
an offloading implementation is supplied for the mv88e6xxx driver.

Public Local Area Networks are often deployed such that there is a
risk of unauthorized or unattended clients getting access to the LAN.
To prevent such access we introduce SA filtering, such that ports
designated as secure ports are set in locked mode, so that only
authorized source MAC addresses are given access by adding them to
the bridges forwarding database. Incoming packets with source MAC
addresses that are not in the forwarding database of the bridge are
discarded. It is then the task of user space daemons to populate the
bridge's forwarding database with static entries of authorized entities.

The most common approach is to use the IEEE 802.1X protocol to take
care of the authorization of allowed users to gain access by opening
for the source address of the authorized host.

With the current use of the bridge parameter in hostapd, there is
a limitation in using this for IEEE 802.1X port authentication. It
depends on hostapd attaching the port on which it has a successful
authentication to the bridge, but that only allows for a single
authentication per port. This patch set allows for the use of
IEEE 802.1X port authentication in a more general network context with
multiple 802.1X aware hosts behind a single port as depicted, which is
a commonly used commercial use-case, as it is only the number of
available entries in the forwarding database that limits the number of
authenticated clients.

      +--------------------------------+
      |                                |
      |      Bridge/Authenticator      |
      |                                |
      +-------------+------------------+
       802.1X port  |
                    |
                    |
             +------+-------+
             |              |
             |  Hub/Switch  |
             |              |
             +-+----------+-+
               |          |
            +--+--+    +--+--+
            |     |    |     |
    Hosts   |  a  |    |  b  |   . . .
            |     |    |     |
            +-----+    +-----+

The 802.1X standard involves three different components, a Supplicant
(Host), an Authenticator (Network Access Point) and an Authentication
Server which is typically a Radius server. This patch set thus enables
the bridge module together with an authenticator application to serve
as an Authenticator on designated ports.


For the bridge to become an IEEE 802.1X Authenticator, a solution using
hostapd with the bridge driver can be found at
https://github.com/westermo/hostapd/tree/bridge_driver .


The relevant components work transparently in relation to if it is the
bridge module or the offloaded switchcore case that is in use.

Hans Schultz (5):
  net: bridge: Add support for bridge port in locked mode
  net: bridge: Add support for offloading of locked port flag
  net: dsa: Add support for offloaded locked port flag
  net: dsa: mv88e6xxx: Add support for bridge port locked mode
  net: bridge: Refactor bridge port in locked mode to use jump labels

 drivers/net/dsa/mv88e6xxx/chip.c |  9 ++++++++-
 drivers/net/dsa/mv88e6xxx/port.c | 33 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/port.h |  9 ++++++++-
 include/linux/if_bridge.h        |  1 +
 include/uapi/linux/if_link.h     |  1 +
 net/bridge/br_input.c            | 24 ++++++++++++++++++++++-
 net/bridge/br_netlink.c          | 12 +++++++++++-
 net/bridge/br_private.h          |  2 ++
 net/bridge/br_switchdev.c        |  2 +-
 net/dsa/port.c                   |  4 ++--
 10 files changed, 90 insertions(+), 7 deletions(-)

-- 
2.30.2

[PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode

[Hans Schultz]

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

[Bridge] [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode

[Hans Schultz]

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

[PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag

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

[Bridge] [PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag

[Hans Schultz]

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

[PATCH net-next v2 3/5] net: dsa: Add support for offloaded locked port flag

[Hans Schultz]

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

[PATCH net-next v2 4/5] net: dsa: mv88e6xxx: Add support for bridge port locked mode

[Hans Schultz]

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

[PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels

[Hans Schultz]

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

[Bridge] [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels

[Hans Schultz]

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

Re: [PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Ido Schimmel]

On Wed, Feb 09, 2022 at 02:05:32PM +0100, Hans Schultz wrote:
> This series starts by adding support for SA filtering to the bridge,
> which is then allowed to be offloaded to switchdev devices. Furthermore
> an offloading implementation is supplied for the mv88e6xxx driver.

[...]

> Hans Schultz (5):
>   net: bridge: Add support for bridge port in locked mode
>   net: bridge: Add support for offloading of locked port flag
>   net: dsa: Add support for offloaded locked port flag
>   net: dsa: mv88e6xxx: Add support for bridge port locked mode
>   net: bridge: Refactor bridge port in locked mode to use jump labels

I think it is a bit weird to add a static key for this option when other
options (e.g., learning) don't use one. If you have data that proves
it's critical, then at least add it in patch #1 where the new option is
introduced.

Please add a selftest under tools/testing/selftests/net/forwarding/. It
should allow you to test both the SW data path with veth pairs and the
offloaded data path with loopbacks. See tools/testing/selftests/net/forwarding/README

Re: [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode

[Nikolay Aleksandrov]

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

Re: [Bridge] [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode

[Nikolay Aleksandrov]

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

Re: [PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag

[Nikolay Aleksandrov]

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>

Re: [Bridge] [PATCH net-next v2 2/5] net: bridge: Add support for offloading of locked port flag

[Nikolay Aleksandrov]

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>

Re: [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels

[Nikolay Aleksandrov]

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);
>  

Re: [Bridge] [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels

[Nikolay Aleksandrov]

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);
>  

Re: [PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Hans Schultz]

On ons, feb 09, 2022 at 18:31, Ido Schimmel <idosch@idosch.org> wrote:
> On Wed, Feb 09, 2022 at 02:05:32PM +0100, Hans Schultz wrote:
>> This series starts by adding support for SA filtering to the bridge,
>> which is then allowed to be offloaded to switchdev devices. Furthermore
>> an offloading implementation is supplied for the mv88e6xxx driver.
>
> [...]
>
>> Hans Schultz (5):
>>   net: bridge: Add support for bridge port in locked mode
>>   net: bridge: Add support for offloading of locked port flag
>>   net: dsa: Add support for offloaded locked port flag
>>   net: dsa: mv88e6xxx: Add support for bridge port locked mode
>>   net: bridge: Refactor bridge port in locked mode to use jump labels
>
> I think it is a bit weird to add a static key for this option when other
> options (e.g., learning) don't use one. If you have data that proves
> it's critical, then at least add it in patch #1 where the new option is
> introduced.

Do you suggest that I drop patch #5 as I don't have data that it is
critical?

>
> Please add a selftest under tools/testing/selftests/net/forwarding/. It
> should allow you to test both the SW data path with veth pairs and the
> offloaded data path with loopbacks. See tools/testing/selftests/net/forwarding/README

I will do that.

Re: [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels

[Hans Schultz]

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);
>>  

Re: [Bridge] [PATCH net-next v2 5/5] net: bridge: Refactor bridge port in locked mode to use jump labels

[Hans Schultz]

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);
>>  

Re: [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode

[Ido Schimmel]

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

Re: [Bridge] [PATCH net-next v2 1/5] net: bridge: Add support for bridge port in locked mode

[Ido Schimmel]

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

Re: [PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Ido Schimmel]

On Thu, Feb 10, 2022 at 10:07:09AM +0100, Hans Schultz wrote:
> On ons, feb 09, 2022 at 18:31, Ido Schimmel <idosch@idosch.org> wrote:
> > On Wed, Feb 09, 2022 at 02:05:32PM +0100, Hans Schultz wrote:
> >> This series starts by adding support for SA filtering to the bridge,
> >> which is then allowed to be offloaded to switchdev devices. Furthermore
> >> an offloading implementation is supplied for the mv88e6xxx driver.
> >
> > [...]
> >
> >> Hans Schultz (5):
> >>   net: bridge: Add support for bridge port in locked mode
> >>   net: bridge: Add support for offloading of locked port flag
> >>   net: dsa: Add support for offloaded locked port flag
> >>   net: dsa: mv88e6xxx: Add support for bridge port locked mode
> >>   net: bridge: Refactor bridge port in locked mode to use jump labels
> >
> > I think it is a bit weird to add a static key for this option when other
> > options (e.g., learning) don't use one. If you have data that proves
> > it's critical, then at least add it in patch #1 where the new option is
> > introduced.
> 
> Do you suggest that I drop patch #5 as I don't have data that it is
> critical?

Yes. That way all the bridge options are consistent. In the future, if
someone reports a performance degradation (unlikely), all the options
can be converted to use a static key.

> 
> >
> > Please add a selftest under tools/testing/selftests/net/forwarding/. It
> > should allow you to test both the SW data path with veth pairs and the
> > offloaded data path with loopbacks. See tools/testing/selftests/net/forwarding/README
> 
> I will do that.

Thanks!

Re: [PATCH net-next v2 3/5] net: dsa: Add support for offloaded locked port flag

[Vladimir Oltean]

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
> 

Re: [PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Florian Fainelli]

On 2/9/22 5:05 AM, Hans Schultz wrote:
> This series starts by adding support for SA filtering to the bridge,
> which is then allowed to be offloaded to switchdev devices. Furthermore
> an offloading implementation is supplied for the mv88e6xxx driver.
> 
> Public Local Area Networks are often deployed such that there is a
> risk of unauthorized or unattended clients getting access to the LAN.
> To prevent such access we introduce SA filtering, such that ports
> designated as secure ports are set in locked mode, so that only
> authorized source MAC addresses are given access by adding them to
> the bridges forwarding database. Incoming packets with source MAC
> addresses that are not in the forwarding database of the bridge are
> discarded. It is then the task of user space daemons to populate the
> bridge's forwarding database with static entries of authorized entities.
> 
> The most common approach is to use the IEEE 802.1X protocol to take
> care of the authorization of allowed users to gain access by opening
> for the source address of the authorized host.
> 
> With the current use of the bridge parameter in hostapd, there is
> a limitation in using this for IEEE 802.1X port authentication. It
> depends on hostapd attaching the port on which it has a successful
> authentication to the bridge, but that only allows for a single
> authentication per port. This patch set allows for the use of
> IEEE 802.1X port authentication in a more general network context with
> multiple 802.1X aware hosts behind a single port as depicted, which is
> a commonly used commercial use-case, as it is only the number of
> available entries in the forwarding database that limits the number of
> authenticated clients.
> 
>       +--------------------------------+
>       |                                |
>       |      Bridge/Authenticator      |
>       |                                |
>       +-------------+------------------+
>        802.1X port  |
>                     |
>                     |
>              +------+-------+
>              |              |
>              |  Hub/Switch  |
>              |              |
>              +-+----------+-+
>                |          |
>             +--+--+    +--+--+
>             |     |    |     |
>     Hosts   |  a  |    |  b  |   . . .
>             |     |    |     |
>             +-----+    +-----+
> 
> The 802.1X standard involves three different components, a Supplicant
> (Host), an Authenticator (Network Access Point) and an Authentication
> Server which is typically a Radius server. This patch set thus enables
> the bridge module together with an authenticator application to serve
> as an Authenticator on designated ports.
> 
> 
> For the bridge to become an IEEE 802.1X Authenticator, a solution using
> hostapd with the bridge driver can be found at
> https://github.com/westermo/hostapd/tree/bridge_driver .
> 
> 
> The relevant components work transparently in relation to if it is the
> bridge module or the offloaded switchcore case that is in use.


It would help for future submissions if you create an union of the
people to copy for *all* patches. What I typically do is:

git format-patch *.patch
./scripts/get_maintainers.pl *.patch > cclist
git send-email *.patch --cc-cmd=cclist.sh
cat cclist.sh
#!/bin/sh
cat $(dirname $0)/cclist

or any smarter way to do that. Now on the actual changes themselves.

It makes sense that we are using the bridge layer to support 802.1x but
this leaves behind the case of standalone ports which people might want
to use. Once we cover the standalone ports there is also a question of
what to do in the case of a regular Ethernet NIC, and whether it can
actually support 802.1x properly given their MAC filtering capabilities
(my guess is that most cannot without breaking communication with other
stations connecting through the same port).

Looking at what Broadcom Roboswitch support, the model you propose
should be working, it makes me wonder if we need to go a bit beyond what
can be configurable besides blocked/not-blocked and have different
levels of strictness. These switches do the following on a per-port
basis you can:

- set the EAP destination MAC address if different than 01:80:C2:00:00:03

- enable EAP frame with destination MAC address specified

- set the EAP block mode:
	0 - disabled
	1 - only the frames with the EAP reserved multicast address will be
forwarded, otherwise frames will be dropped
	3 - only the frames with the EAP reserved multicast address will be
forwarded. Forwarding verifies that each egress port is enabled for EAP
+ BLK_MODE
- set the EAP mode:
	0 - disabled
	2 - extended mode: check MAC SA, port, drop if unknown
	3 - simplified mode: check SA, port, trap to management if SA is unknown

We have a number of vectors that can be used to accept specific MAC SA
addresses.

Then we have a global register that allows us to configure whether we
want to allow ARP, DHCP, BPDU, RMC, RARP or the frames with the
specified destination IP address/masks being specified. I would assume
that the two registers allowing us to specify a destination IP/subnet
might be used to park unauthenticated stations to a "guest" VLAN maybe?

So with that said, it looks like we may not need a method beyond just
setting the port state. In your case, it sounds like you would program
the mv88e6xxx switch's ATU with the MAC addresses learned from the
bridge via the standard FDB learning notification?

In the case of Broadcom switches, I suppose the same should be done,
however instead of programming the main FDB, we would have to program
the multi-port vector when the port is in blocked state. This becomes a
switch driver detail.

I would like other maintainers of switch drivers to chime in to know
whether microchip, Qualcomm/Atheros and Mediatek have similar features
or not.

Does that make sense?

Time to dust off my freeradius settings to test this out, it's been
nearly 15 years since I last did this, time to see if EAP-TTLS or
EAP-PEAP are more streamlined on the client side :)

Re: [PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Vladimir Oltean]

On Fri, Feb 11, 2022 at 10:11:38AM -0800, Florian Fainelli wrote:
> It makes sense that we are using the bridge layer to support 802.1x but
> this leaves behind the case of standalone ports which people might want
> to use. Once we cover the standalone ports there is also a question of
> what to do in the case of a regular Ethernet NIC, and whether it can
> actually support 802.1x properly given their MAC filtering capabilities

The hardware implementations I'm aware of heavily revolve around what is
fundamentally the bridging service. But I've downloaded the 802.1X-2010
standard and there's nothing limiting it to a bridge. It can also apply
to a router, or an end point station. Not sure where that leaves us,
though. A more generic concept would be a MAC SA filter, which is kept
in sync with the bridge FDB by a user space daemon in case the bridge is
used on a port. But in whichever way I think about it, I don't see a
unified way of modeling this MAC SA filter in a way that lends itself
easily to switchdev offloading. On a standalone NIC, it would probably
make more sense for it to be an eBPF program. But for a switch, you just
expand simple information like "drop anything that wasn't learned" into
"drop anything that isn't this, that or the other", and the switch
driver has to parse all that just to figure out "oh, so you mean drop
anything that wasn't learned? well I have a bit for that".

Either we try to find a unified solution, or we let user space do
different things depending on whether a bridge is in use or not.

> (my guess is that most cannot without breaking communication with other
> stations connecting through the same port).

If the hardware support requires dropping packets with unknown MAC SA,
then yes, probably. DSA standalone ports disable MAC SA learning and
operate with empty FDBs on them these days. So we'd cut off all traffic
on that port if we don't change something in this model. And even if we
change something, we also need to take FDB isolation into account, using
a separate FID for authenticated MACs, with no forwarding other than to
the CPU. I'm probably thinking too far ahead though.

> Looking at what Broadcom Roboswitch support, the model you propose
> should be working, it makes me wonder if we need to go a bit beyond what
> can be configurable besides blocked/not-blocked and have different
> levels of strictness. These switches do the following on a per-port
> basis you can:
> 
> - set the EAP destination MAC address if different than 01:80:C2:00:00:03
> 
> - enable EAP frame with destination MAC address specified
> 
> - set the EAP block mode:
> 	0 - disabled
> 	1 - only the frames with the EAP reserved multicast address will be
> forwarded, otherwise frames will be dropped
> 	3 - only the frames with the EAP reserved multicast address will be
> forwarded. Forwarding verifies that each egress port is enabled for EAP
> + BLK_MODE

What's the difference between 1 and 3? At 1, when you say that EAP
reserved multicast packets will be forwarded, do you actually mean they
will be trapped to CPU?

> - set the EAP mode:
> 	0 - disabled
> 	2 - extended mode: check MAC SA, port, drop if unknown
> 	3 - simplified mode: check SA, port, trap to management if SA is unknown
> 
> We have a number of vectors that can be used to accept specific MAC SA
> addresses.
> 
> Then we have a global register that allows us to configure whether we
> want to allow ARP, DHCP, BPDU, RMC, RARP or the frames with the
> specified destination IP address/masks being specified. I would assume
> that the two registers allowing us to specify a destination IP/subnet
> might be used to park unauthenticated stations to a "guest" VLAN maybe?
> 
> So with that said, it looks like we may not need a method beyond just
> setting the port state. In your case, it sounds like you would program
> the mv88e6xxx switch's ATU with the MAC addresses learned from the
> bridge via the standard FDB learning notification?
> 
> In the case of Broadcom switches, I suppose the same should be done,
> however instead of programming the main FDB, we would have to program
> the multi-port vector when the port is in blocked state. This becomes a
> switch driver detail.
> 
> I would like other maintainers of switch drivers to chime in to know
> whether microchip, Qualcomm/Atheros and Mediatek have similar features
> or not.
> 
> Does that make sense?
> 
> Time to dust off my freeradius settings to test this out, it's been
> nearly 15 years since I last did this, time to see if EAP-TTLS or
> EAP-PEAP are more streamlined on the client side :)

The Microchip Ocelot switch family (and most probably others too) has 2
kinds of support for 802.1X access control.

- Port-based, this essentially revolves around putting a port in the
  BLOCKING STP state. IEEE 802.1X uses MAC address 01-80-C2-00-00-03, so
  that will continue to be trapped to the CPU regardless of STP state.

- MAC-based, this essentially revolves around "learn frames" (frames
  with unknown MAC SA). There are 2 sub-groups here, solution 1 is
  secure CPU-based learning, where learn frames are trapped to the CPU,
  and solution 2 is no-learning, where learn frames are just dropped.
  In both cases, the host must program the FDB to allow that MAC SA to
  be forwarded to other ports. I'm thinking that the no-learning method
  is what would resemble the closest this new BR_PORT_LOCKED flag.
  The secure learning procedure is probably useful when you want to
  bypass the MAC authentication process for a dumb device like a printer
  that doesn't speak 802.1X but you want it to be nonetheless whitelisted.

With the difference that MAC-based authentication allows you to have
more than 1 MAC SA behind an 802.1X switch port (like another switch),
and port-based authentication doesn't have that kind of flexibility.

Of course, there's also the TCAM where a custom policy could be enabled
per MAC SA.

The NXP SJA1105P/Q/R/S and SJA1110 of course have to be snowflakes here
as well. There is a "DRPNOLEARN" bit (drop frames which couldn't be
learned), but it's global, not per port, and it doesn't affect frames
which weren't learned due to learning being disabled on a port - just
due to the FDB space reserved for dynamic entries getting depleted.
I could probably hack around that by reserving zero space for dynamic
FDB entries, but again, that would be a setting per switch, not per port.
There's just something about this switch that makes my skin itch :)
This switch family also supports "port enforcement" per FDB entry, where
the switch blocks station migrations/spoofing attempts by dropping a
given MAC SA if the FDB has that address pointing towards a different
port than it was received on. Not sure whether this is something useful
in the context of 802.1X. Looking at Ocelot switches again, I see the
definition of a "learn frame" is actually expanded to cover these kind
of frames too (not just unknown MAC SA). So all that's described above
for learn frames should apply for spoofed traffic too.

Re: [PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Jakub Kicinski]

On Wed,  9 Feb 2022 14:05:32 +0100 Hans Schultz wrote:
> The most common approach is to use the IEEE 802.1X protocol to take
> care of the authorization of allowed users to gain access by opening
> for the source address of the authorized host.

noob question - this is 802.1x without crypto? I'm trying to understand
the system you're describing.

Re: [PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Tobias Waldekranz]

On Fri, Feb 11, 2022 at 10:11, Florian Fainelli <f.fainelli@gmail.com> wrote:
> It makes sense that we are using the bridge layer to support 802.1x but
> this leaves behind the case of standalone ports which people might want
> to use. Once we cover the standalone ports there is also a question of
> what to do in the case of a regular Ethernet NIC, and whether it can
> actually support 802.1x properly given their MAC filtering capabilities
> (my guess is that most cannot without breaking communication with other
> stations connecting through the same port).

The only interface I can think of that would be generic enough to
support all cases is TC offloading. I.e. you could program a matchall
rule drop rule, and then later insert exceptions for authenticated
clients before the drop rule.

It feels awfully cumbersome for drivers to have to figure this out
though.

> Looking at what Broadcom Roboswitch support, the model you propose
> should be working, it makes me wonder if we need to go a bit beyond what
> can be configurable besides blocked/not-blocked and have different
> levels of strictness. These switches do the following on a per-port
> basis you can:
>
> - set the EAP destination MAC address if different than 01:80:C2:00:00:03
>
> - enable EAP frame with destination MAC address specified
>
> - set the EAP block mode:
> 	0 - disabled
> 	1 - only the frames with the EAP reserved multicast address will be
> forwarded, otherwise frames will be dropped
> 	3 - only the frames with the EAP reserved multicast address will be
> forwarded. Forwarding verifies that each egress port is enabled for EAP
> + BLK_MODE
> - set the EAP mode:
> 	0 - disabled
> 	2 - extended mode: check MAC SA, port, drop if unknown
> 	3 - simplified mode: check SA, port, trap to management if SA is unknown
>
> We have a number of vectors that can be used to accept specific MAC SA
> addresses.
>
> Then we have a global register that allows us to configure whether we
> want to allow ARP, DHCP, BPDU, RMC, RARP or the frames with the
> specified destination IP address/masks being specified. I would assume
> that the two registers allowing us to specify a destination IP/subnet
> might be used to park unauthenticated stations to a "guest" VLAN maybe?
>
> So with that said, it looks like we may not need a method beyond just
> setting the port state. In your case, it sounds like you would program
> the mv88e6xxx switch's ATU with the MAC addresses learned from the
> bridge via the standard FDB learning notification?

The idea is that hostapd (or whatever authenticator daemon you're
running) would do the equivalent of:

    bridge fdb add <sa-of-station> [vid X] dev <port> permanent

(Vladimir can correct the flags that I have undoubtedly gotten wrong,
but you get the idea :))

Would this approach not work on a Roboswitch?

In the future, we also want to support things like MAB (mac
authentication bypass), i.e. allowing for some vetting of legacy
devices. This is a bit more complicated, because it requires a
notification from the HW about which exact address that was seen.

Our idea is to add a flag to FDB entries ("locked"), that would indicate
that a dynamic entry has been learned on a locked port. The entry must
therefore never be used to forward traffic. Its addition to the FDB will
trigger an event though, which can be noticed by a daemon that can
perform the necessary verifications. If the station is allowed, the
deamon will clear the locked flag on the FDB entry, allowing forwarding.

In a software setting, this entry will be added by the bridge when a new
SA is detected on a locked port. For switchdevs, the drivers could use
the existing way of notifying the bridge of new entries that it has
learned, except that the locked flag would be set on the entry.

So on Roboswitch, for example, I imagine you could:

- Use EAP mode 3 to trap the frames to the CPU
- Add a hardware FDB entry with a null-destination to block any further
  traps for that SA.
- Generate a locked FDB entry notification to the bridge (and by
  extension to userspace).

This will let you enforce policies like "only allow stations with OUI
02:de:ad on swp3".

> In the case of Broadcom switches, I suppose the same should be done,
> however instead of programming the main FDB, we would have to program
> the multi-port vector when the port is in blocked state. This becomes a
> switch driver detail.
>
> I would like other maintainers of switch drivers to chime in to know
> whether microchip, Qualcomm/Atheros and Mediatek have similar features
> or not.
>
> Does that make sense?
>
> Time to dust off my freeradius settings to test this out, it's been
> nearly 15 years since I last did this, time to see if EAP-TTLS or
> EAP-PEAP are more streamlined on the client side :)

Re: [PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Hans Schultz]

On fre, feb 11, 2022 at 14:59, Jakub Kicinski <kuba@kernel.org> wrote:
> On Wed,  9 Feb 2022 14:05:32 +0100 Hans Schultz wrote:
>> The most common approach is to use the IEEE 802.1X protocol to take
>> care of the authorization of allowed users to gain access by opening
>> for the source address of the authorized host.
>
> noob question - this is 802.1x without crypto? I'm trying to understand
> the system you're describing.

No, user space will take care of authentication, f.ex. hostapd, so in a
typical setup the supplicant and the authentication daemon will take
care of all crypto related stuff in their communication.
So the authentication daemon will open the port for the authenticated
supplicant. See the cover letter.

Re: [PATCH net-next v2 0/5] Add support for locked bridge ports (for 802.1X)

[Jakub Kicinski]

On Mon, 14 Feb 2022 09:58:12 +0100 Hans Schultz wrote:
> On fre, feb 11, 2022 at 14:59, Jakub Kicinski <kuba@kernel.org> wrote:
> > On Wed,  9 Feb 2022 14:05:32 +0100 Hans Schultz wrote:  
> >> The most common approach is to use the IEEE 802.1X protocol to take
> >> care of the authorization of allowed users to gain access by opening
> >> for the source address of the authorized host.  
> >
> > noob question - this is 802.1x without crypto? I'm trying to understand
> > the system you're describing.  
> 
> No, user space will take care of authentication, f.ex. hostapd, so in a
> typical setup the supplicant and the authentication daemon will take
> care of all crypto related stuff in their communication.
> So the authentication daemon will open the port for the authenticated
> supplicant.

To be clear - I'm talking about wire crypto after all the communication
with the control plane and after the connection the port is opened. Not
crypto in whatever authentication method gets used. Does the device get
the keys somehow from user space?

> See the cover letter.

Which part of it?