2022-07-07 15:56:36

by Hans Schultz

[permalink] [raw]
Subject: [PATCH v4 net-next 0/6] Extend locked port feature with FDB locked flag (MAC-Auth/MAB)

This patch set extends the locked port feature for devices
that are behind a locked port, but do not have the ability to
authorize themselves as a supplicant using IEEE 802.1X.
Such devices can be printers, meters or anything related to
fixed installations. Instead of 802.1X authorization, devices
can get access based on their MAC addresses being whitelisted.

For an authorization daemon to detect that a device is trying
to get access through a locked port, the bridge will add the
MAC address of the device to the FDB with a locked flag to it.
Thus the authorization daemon can catch the FDB add event and
check if the MAC address is in the whitelist and if so replace
the FDB entry without the locked flag enabled, and thus open
the port for the device.

This feature is known as MAC-Auth or MAC Authentication Bypass
(MAB) in Cisco terminology, where the full MAB concept involves
additional Cisco infrastructure for authorization. There is no
real authentication process, as the MAC address of the device
is the only input the authorization daemon, in the general
case, has to base the decision if to unlock the port or not.

With this patch set, an implementation of the offloaded case is
supplied for the mv88e6xxx driver. When a packet ingresses on
a locked port, an ATU miss violation event will occur. When
handling such ATU miss violation interrupts, the MAC address of
the device is added to the FDB with a zero destination port
vector (DPV) and the MAC address is communicated through the
switchdev layer to the bridge, so that a FDB entry with the
locked flag enabled can be added.

Log:
v3: Added timers and lists in the driver (mv88e6xxx)
to keep track of and remove locked entries.

v4: Leave out enforcing a limit to the number of
locked entries in the bridge.
Removed the timers in the driver and use the
worker only. Add locked FDB flag to all drivers
using port_fdb_add() from the dsa api and let
all drivers ignore entries with this flag set.
Change how to get the ageing timeout of locked
entries. See global1_atu.c and switchdev.c.
Use struct mv88e6xxx_port for locked entries
variables instead of struct dsa_port.

Hans Schultz (6):
net: bridge: add locked entry fdb flag to extend locked port feature
net: switchdev: add support for offloading of fdb locked flag
drivers: net: dsa: add locked fdb entry flag to drivers
net: dsa: mv88e6xxx: allow reading FID when handling ATU violations
net: dsa: mv88e6xxx: mac-auth/MAB implementation
selftests: forwarding: add test of MAC-Auth Bypass to locked port
tests

drivers/net/dsa/b53/b53_common.c | 5 +
drivers/net/dsa/b53/b53_priv.h | 1 +
drivers/net/dsa/hirschmann/hellcreek.c | 5 +
drivers/net/dsa/lan9303-core.c | 5 +
drivers/net/dsa/lantiq_gswip.c | 5 +
drivers/net/dsa/microchip/ksz9477.c | 5 +
drivers/net/dsa/mt7530.c | 5 +
drivers/net/dsa/mv88e6xxx/Makefile | 1 +
drivers/net/dsa/mv88e6xxx/chip.c | 54 +++-
drivers/net/dsa/mv88e6xxx/chip.h | 15 +
drivers/net/dsa/mv88e6xxx/global1.h | 1 +
drivers/net/dsa/mv88e6xxx/global1_atu.c | 32 +-
drivers/net/dsa/mv88e6xxx/port.c | 30 +-
drivers/net/dsa/mv88e6xxx/port.h | 2 +
drivers/net/dsa/mv88e6xxx/switchdev.c | 280 ++++++++++++++++++
drivers/net/dsa/mv88e6xxx/switchdev.h | 37 +++
drivers/net/dsa/ocelot/felix.c | 5 +
drivers/net/dsa/qca8k.c | 5 +
drivers/net/dsa/sja1105/sja1105_main.c | 5 +
include/net/dsa.h | 7 +
include/net/switchdev.h | 1 +
include/uapi/linux/neighbour.h | 1 +
net/bridge/br.c | 3 +-
net/bridge/br_fdb.c | 19 +-
net/bridge/br_input.c | 10 +-
net/bridge/br_private.h | 5 +-
net/bridge/br_switchdev.c | 1 +
net/dsa/dsa_priv.h | 4 +-
net/dsa/port.c | 7 +-
net/dsa/slave.c | 4 +-
net/dsa/switch.c | 10 +-
.../net/forwarding/bridge_locked_port.sh | 30 +-
32 files changed, 566 insertions(+), 34 deletions(-)
create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.c
create mode 100644 drivers/net/dsa/mv88e6xxx/switchdev.h

--
2.30.2


2022-07-07 16:26:20

by Hans Schultz

[permalink] [raw]
Subject: [PATCH v4 net-next 2/6] net: switchdev: add support for offloading of fdb locked flag

Used for Mac-auth/MAB feature in the offloaded case.
Send flag through switchdev to driver.

Signed-off-by: Hans Schultz <[email protected]>
---
include/net/dsa.h | 6 ++++++
include/net/switchdev.h | 1 +
net/bridge/br.c | 3 ++-
net/bridge/br_fdb.c | 7 +++++--
net/bridge/br_private.h | 2 +-
net/bridge/br_switchdev.c | 1 +
net/dsa/dsa_priv.h | 4 +++-
net/dsa/port.c | 7 ++++++-
net/dsa/slave.c | 4 +++-
net/dsa/switch.c | 6 +++---
10 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 14f07275852b..a5a843b2d67d 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -330,6 +330,12 @@ struct dsa_port {
/* List of VLANs that CPU and DSA ports are members of. */
struct mutex vlans_lock;
struct list_head vlans;
+
+ /* List and maintenance of locked ATU entries */
+ struct mutex locked_entries_list_lock;
+ struct list_head atu_locked_entries_list;
+ atomic_t atu_locked_entry_cnt;
+ struct delayed_work atu_work;
};

/* TODO: ideally DSA ports would have a single dp->link_dp member,
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index aa0171d5786d..9f83c835ee62 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -245,6 +245,7 @@ struct switchdev_notifier_fdb_info {
u16 vid;
u8 added_by_user:1,
is_local:1,
+ is_locked:1,
offloaded:1;
};

diff --git a/net/bridge/br.c b/net/bridge/br.c
index 96e91d69a9a8..fe0a4741fcda 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -166,7 +166,8 @@ static int br_switchdev_event(struct notifier_block *unused,
case SWITCHDEV_FDB_ADD_TO_BRIDGE:
fdb_info = ptr;
err = br_fdb_external_learn_add(br, p, fdb_info->addr,
- fdb_info->vid, false);
+ fdb_info->vid, false,
+ fdb_info->is_locked);
if (err) {
err = notifier_from_errno(err);
break;
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index ee9064a536ae..32ebb18050b9 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -1136,7 +1136,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
"FDB entry towards bridge must be permanent");
return -EINVAL;
}
- err = br_fdb_external_learn_add(br, p, addr, vid, true);
+ err = br_fdb_external_learn_add(br, p, addr, vid, true, false);
} else {
spin_lock_bh(&br->hash_lock);
err = fdb_add_entry(br, p, addr, ndm, nlh_flags, vid, nfea_tb);
@@ -1366,7 +1366,7 @@ void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p)

int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
const unsigned char *addr, u16 vid,
- bool swdev_notify)
+ bool swdev_notify, bool locked)
{
struct net_bridge_fdb_entry *fdb;
bool modified = false;
@@ -1386,6 +1386,9 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
if (!p)
flags |= BIT(BR_FDB_LOCAL);

+ if (locked)
+ flags |= BIT(BR_FDB_ENTRY_LOCKED);
+
fdb = fdb_create(br, p, addr, vid, flags);
if (!fdb) {
err = -ENOMEM;
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 47a3598d25c8..9082451b4d40 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -811,7 +811,7 @@ int br_fdb_sync_static(struct net_bridge *br, struct net_bridge_port *p);
void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p);
int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
const unsigned char *addr, u16 vid,
- bool swdev_notify);
+ bool swdev_notify, bool locked);
int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p,
const unsigned char *addr, u16 vid,
bool swdev_notify);
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index 8f3d76c751dd..85e566b856e1 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -136,6 +136,7 @@ static void br_switchdev_fdb_populate(struct net_bridge *br,
item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags);
item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags);
+ item->is_locked = test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags);
item->info.dev = (!p || item->is_local) ? br->dev : p->dev;
item->info.ctx = ctx;
}
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index d9722e49864b..42f47a94b0f0 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -65,6 +65,7 @@ struct dsa_notifier_fdb_info {
const struct dsa_port *dp;
const unsigned char *addr;
u16 vid;
+ bool is_locked;
struct dsa_db db;
};

@@ -131,6 +132,7 @@ struct dsa_switchdev_event_work {
unsigned char addr[ETH_ALEN];
u16 vid;
bool host_addr;
+ bool is_locked;
};

enum dsa_standalone_event {
@@ -232,7 +234,7 @@ int dsa_port_vlan_msti(struct dsa_port *dp,
const struct switchdev_vlan_msti *msti);
int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu);
int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
- u16 vid);
+ u16 vid, bool is_locked);
int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
u16 vid);
int dsa_port_standalone_host_fdb_add(struct dsa_port *dp,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 3738f2d40a0b..8bdac9aabe5d 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -35,6 +35,7 @@ static void dsa_port_notify_bridge_fdb_flush(const struct dsa_port *dp, u16 vid)
struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
struct switchdev_notifier_fdb_info info = {
.vid = vid,
+ .is_locked = false,
};

/* When the port becomes standalone it has already left the bridge.
@@ -950,12 +951,13 @@ int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu)
}

int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
- u16 vid)
+ u16 vid, bool is_locked)
{
struct dsa_notifier_fdb_info info = {
.dp = dp,
.addr = addr,
.vid = vid,
+ .is_locked = is_locked,
.db = {
.type = DSA_DB_BRIDGE,
.bridge = *dp->bridge,
@@ -979,6 +981,7 @@ int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
.dp = dp,
.addr = addr,
.vid = vid,
+ .is_locked = false,
.db = {
.type = DSA_DB_BRIDGE,
.bridge = *dp->bridge,
@@ -999,6 +1002,7 @@ static int dsa_port_host_fdb_add(struct dsa_port *dp,
.dp = dp,
.addr = addr,
.vid = vid,
+ .is_locked = false,
.db = db,
};

@@ -1050,6 +1054,7 @@ static int dsa_port_host_fdb_del(struct dsa_port *dp,
.dp = dp,
.addr = addr,
.vid = vid,
+ .is_locked = false,
.db = db,
};

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 801a5d445833..905b15e4eab9 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -2784,6 +2784,7 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work)
container_of(work, struct dsa_switchdev_event_work, work);
const unsigned char *addr = switchdev_work->addr;
struct net_device *dev = switchdev_work->dev;
+ bool is_locked = switchdev_work->is_locked;
u16 vid = switchdev_work->vid;
struct dsa_switch *ds;
struct dsa_port *dp;
@@ -2799,7 +2800,7 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work)
else if (dp->lag)
err = dsa_port_lag_fdb_add(dp, addr, vid);
else
- err = dsa_port_fdb_add(dp, addr, vid);
+ err = dsa_port_fdb_add(dp, addr, vid, is_locked);
if (err) {
dev_err(ds->dev,
"port %d failed to add %pM vid %d to fdb: %d\n",
@@ -2907,6 +2908,7 @@ static int dsa_slave_fdb_event(struct net_device *dev,
ether_addr_copy(switchdev_work->addr, fdb_info->addr);
switchdev_work->vid = fdb_info->vid;
switchdev_work->host_addr = host_addr;
+ switchdev_work->is_locked = fdb_info->is_locked;

dsa_schedule_work(&switchdev_work->work);

diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 2b56218fc57c..32b1e7ac6373 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -234,7 +234,7 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp,
}

static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
- u16 vid, struct dsa_db db)
+ u16 vid, bool is_locked, struct dsa_db db)
{
struct dsa_switch *ds = dp->ds;
struct dsa_mac_addr *a;
@@ -398,7 +398,7 @@ static int dsa_switch_host_fdb_add(struct dsa_switch *ds,
dsa_switch_for_each_port(dp, ds) {
if (dsa_port_host_address_match(dp, info->dp)) {
err = dsa_port_do_fdb_add(dp, info->addr, info->vid,
- info->db);
+ false, info->db);
if (err)
break;
}
@@ -437,7 +437,7 @@ static int dsa_switch_fdb_add(struct dsa_switch *ds,
if (!ds->ops->port_fdb_add)
return -EOPNOTSUPP;

- return dsa_port_do_fdb_add(dp, info->addr, info->vid, info->db);
+ return dsa_port_do_fdb_add(dp, info->addr, info->vid, info->is_locked, info->db);
}

static int dsa_switch_fdb_del(struct dsa_switch *ds,
--
2.30.2

2022-07-08 01:27:58

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH v4 net-next 0/6] Extend locked port feature with FDB locked flag (MAC-Auth/MAB)

On Thu, 7 Jul 2022 17:29:24 +0200 Hans Schultz wrote:
> [PATCH v4 net-next 0/6] Extend locked port feature with FDB locked flag (MAC-Auth/MAB)

Let's give it a day or two for feedback but the series does not apply
cleanly to net-next so a rebase & repost will be needed even if it's
otherwise perfect.

2022-07-08 08:58:49

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v4 net-next 2/6] net: switchdev: add support for offloading of fdb locked flag

On Thu, Jul 07, 2022 at 05:29:26PM +0200, Hans Schultz wrote:
> Used for Mac-auth/MAB feature in the offloaded case.
> Send flag through switchdev to driver.
>
> Signed-off-by: Hans Schultz <[email protected]>
> ---
> include/net/dsa.h | 6 ++++++
> include/net/switchdev.h | 1 +
> net/bridge/br.c | 3 ++-
> net/bridge/br_fdb.c | 7 +++++--
> net/bridge/br_private.h | 2 +-
> net/bridge/br_switchdev.c | 1 +
> net/dsa/dsa_priv.h | 4 +++-
> net/dsa/port.c | 7 ++++++-
> net/dsa/slave.c | 4 +++-
> net/dsa/switch.c | 6 +++---
> 10 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 14f07275852b..a5a843b2d67d 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -330,6 +330,12 @@ struct dsa_port {
> /* List of VLANs that CPU and DSA ports are members of. */
> struct mutex vlans_lock;
> struct list_head vlans;
> +
> + /* List and maintenance of locked ATU entries */
> + struct mutex locked_entries_list_lock;
> + struct list_head atu_locked_entries_list;
> + atomic_t atu_locked_entry_cnt;
> + struct delayed_work atu_work;

Leftovers from an old change, please drop from the patch.

> };
>
> /* TODO: ideally DSA ports would have a single dp->link_dp member,
> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> index aa0171d5786d..9f83c835ee62 100644
> --- a/include/net/switchdev.h
> +++ b/include/net/switchdev.h
> @@ -245,6 +245,7 @@ struct switchdev_notifier_fdb_info {
> u16 vid;
> u8 added_by_user:1,
> is_local:1,
> + is_locked:1,
> offloaded:1;
> };
>
> diff --git a/net/bridge/br.c b/net/bridge/br.c
> index 96e91d69a9a8..fe0a4741fcda 100644
> --- a/net/bridge/br.c
> +++ b/net/bridge/br.c
> @@ -166,7 +166,8 @@ static int br_switchdev_event(struct notifier_block *unused,
> case SWITCHDEV_FDB_ADD_TO_BRIDGE:
> fdb_info = ptr;
> err = br_fdb_external_learn_add(br, p, fdb_info->addr,
> - fdb_info->vid, false);
> + fdb_info->vid, false,
> + fdb_info->is_locked);
> if (err) {
> err = notifier_from_errno(err);
> break;
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index ee9064a536ae..32ebb18050b9 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -1136,7 +1136,7 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,
> "FDB entry towards bridge must be permanent");
> return -EINVAL;
> }
> - err = br_fdb_external_learn_add(br, p, addr, vid, true);
> + err = br_fdb_external_learn_add(br, p, addr, vid, true, false);
> } else {
> spin_lock_bh(&br->hash_lock);
> err = fdb_add_entry(br, p, addr, ndm, nlh_flags, vid, nfea_tb);
> @@ -1366,7 +1366,7 @@ void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p)
>
> int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
> const unsigned char *addr, u16 vid,
> - bool swdev_notify)
> + bool swdev_notify, bool locked)
> {
> struct net_bridge_fdb_entry *fdb;
> bool modified = false;
> @@ -1386,6 +1386,9 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
> if (!p)
> flags |= BIT(BR_FDB_LOCAL);
>
> + if (locked)
> + flags |= BIT(BR_FDB_ENTRY_LOCKED);
> +
> fdb = fdb_create(br, p, addr, vid, flags);
> if (!fdb) {
> err = -ENOMEM;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 47a3598d25c8..9082451b4d40 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -811,7 +811,7 @@ int br_fdb_sync_static(struct net_bridge *br, struct net_bridge_port *p);
> void br_fdb_unsync_static(struct net_bridge *br, struct net_bridge_port *p);
> int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
> const unsigned char *addr, u16 vid,
> - bool swdev_notify);
> + bool swdev_notify, bool locked);
> int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p,
> const unsigned char *addr, u16 vid,
> bool swdev_notify);
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index 8f3d76c751dd..85e566b856e1 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -136,6 +136,7 @@ static void br_switchdev_fdb_populate(struct net_bridge *br,
> item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
> item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags);
> item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags);
> + item->is_locked = test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags);
> item->info.dev = (!p || item->is_local) ? br->dev : p->dev;
> item->info.ctx = ctx;
> }
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index d9722e49864b..42f47a94b0f0 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -65,6 +65,7 @@ struct dsa_notifier_fdb_info {
> const struct dsa_port *dp;
> const unsigned char *addr;
> u16 vid;
> + bool is_locked;

drop

> struct dsa_db db;
> };
>
> @@ -131,6 +132,7 @@ struct dsa_switchdev_event_work {
> unsigned char addr[ETH_ALEN];
> u16 vid;
> bool host_addr;
> + bool is_locked;

drop

> };
>
> enum dsa_standalone_event {
> @@ -232,7 +234,7 @@ int dsa_port_vlan_msti(struct dsa_port *dp,
> const struct switchdev_vlan_msti *msti);
> int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu);
> int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
> - u16 vid);
> + u16 vid, bool is_locked);

drop

> int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
> u16 vid);
> int dsa_port_standalone_host_fdb_add(struct dsa_port *dp,
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 3738f2d40a0b..8bdac9aabe5d 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -35,6 +35,7 @@ static void dsa_port_notify_bridge_fdb_flush(const struct dsa_port *dp, u16 vid)
> struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
> struct switchdev_notifier_fdb_info info = {
> .vid = vid,
> + .is_locked = false,

drop

> };
>
> /* When the port becomes standalone it has already left the bridge.
> @@ -950,12 +951,13 @@ int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu)
> }
>
> int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
> - u16 vid)
> + u16 vid, bool is_locked)

drop

> {
> struct dsa_notifier_fdb_info info = {
> .dp = dp,
> .addr = addr,
> .vid = vid,
> + .is_locked = is_locked,

drop

> .db = {
> .type = DSA_DB_BRIDGE,
> .bridge = *dp->bridge,
> @@ -979,6 +981,7 @@ int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
> .dp = dp,
> .addr = addr,
> .vid = vid,
> + .is_locked = false,

drop

> .db = {
> .type = DSA_DB_BRIDGE,
> .bridge = *dp->bridge,
> @@ -999,6 +1002,7 @@ static int dsa_port_host_fdb_add(struct dsa_port *dp,
> .dp = dp,
> .addr = addr,
> .vid = vid,
> + .is_locked = false,

drop

> .db = db,
> };
>
> @@ -1050,6 +1054,7 @@ static int dsa_port_host_fdb_del(struct dsa_port *dp,
> .dp = dp,
> .addr = addr,
> .vid = vid,
> + .is_locked = false,

drop

> .db = db,
> };
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 801a5d445833..905b15e4eab9 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -2784,6 +2784,7 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work)
> container_of(work, struct dsa_switchdev_event_work, work);
> const unsigned char *addr = switchdev_work->addr;
> struct net_device *dev = switchdev_work->dev;
> + bool is_locked = switchdev_work->is_locked;

drop

> u16 vid = switchdev_work->vid;
> struct dsa_switch *ds;
> struct dsa_port *dp;
> @@ -2799,7 +2800,7 @@ static void dsa_slave_switchdev_event_work(struct work_struct *work)
> else if (dp->lag)
> err = dsa_port_lag_fdb_add(dp, addr, vid);
> else
> - err = dsa_port_fdb_add(dp, addr, vid);
> + err = dsa_port_fdb_add(dp, addr, vid, is_locked);

drop

> if (err) {
> dev_err(ds->dev,
> "port %d failed to add %pM vid %d to fdb: %d\n",
> @@ -2907,6 +2908,7 @@ static int dsa_slave_fdb_event(struct net_device *dev,
> ether_addr_copy(switchdev_work->addr, fdb_info->addr);
> switchdev_work->vid = fdb_info->vid;
> switchdev_work->host_addr = host_addr;
> + switchdev_work->is_locked = fdb_info->is_locked;

drop

>
> dsa_schedule_work(&switchdev_work->work);
>
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index 2b56218fc57c..32b1e7ac6373 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -234,7 +234,7 @@ static int dsa_port_do_mdb_del(struct dsa_port *dp,
> }
>
> static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned char *addr,
> - u16 vid, struct dsa_db db)
> + u16 vid, bool is_locked, struct dsa_db db)

drop

> {
> struct dsa_switch *ds = dp->ds;
> struct dsa_mac_addr *a;
> @@ -398,7 +398,7 @@ static int dsa_switch_host_fdb_add(struct dsa_switch *ds,
> dsa_switch_for_each_port(dp, ds) {
> if (dsa_port_host_address_match(dp, info->dp)) {
> err = dsa_port_do_fdb_add(dp, info->addr, info->vid,
> - info->db);
> + false, info->db);

drop

> if (err)
> break;
> }
> @@ -437,7 +437,7 @@ static int dsa_switch_fdb_add(struct dsa_switch *ds,
> if (!ds->ops->port_fdb_add)
> return -EOPNOTSUPP;
>
> - return dsa_port_do_fdb_add(dp, info->addr, info->vid, info->db);
> + return dsa_port_do_fdb_add(dp, info->addr, info->vid, info->is_locked, info->db);

drop

> }
>
> static int dsa_switch_fdb_del(struct dsa_switch *ds,
> --
> 2.30.2
>

2022-08-02 08:32:10

by Hans Schultz

[permalink] [raw]
Subject: Re: [PATCH v4 net-next 2/6] net: switchdev: add support for offloading of fdb locked flag

On 2022-07-08 10:54, Vladimir Oltean wrote:
>> };
>>
>> /* TODO: ideally DSA ports would have a single dp->link_dp member,
>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>> index aa0171d5786d..9f83c835ee62 100644
>> --- a/include/net/switchdev.h
>> +++ b/include/net/switchdev.h
>> @@ -245,6 +245,7 @@ struct switchdev_notifier_fdb_info {
>> u16 vid;
>> u8 added_by_user:1,
>> is_local:1,
>> + is_locked:1,
>> offloaded:1;
>> };
>>
>> diff --git a/net/bridge/br.c b/net/bridge/br.c
>> index 96e91d69a9a8..fe0a4741fcda 100644
>> --- a/net/bridge/br.c
>> +++ b/net/bridge/br.c
>> @@ -166,7 +166,8 @@ static int br_switchdev_event(struct
>> notifier_block *unused,
>> case SWITCHDEV_FDB_ADD_TO_BRIDGE:
>> fdb_info = ptr;
>> err = br_fdb_external_learn_add(br, p, fdb_info->addr,
>> - fdb_info->vid, false);
>> + fdb_info->vid, false,
>> + fdb_info->is_locked);
>> if (err) {
>> err = notifier_from_errno(err);
>> break;
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index ee9064a536ae..32ebb18050b9 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -1136,7 +1136,7 @@ static int __br_fdb_add(struct ndmsg *ndm,
>> struct net_bridge *br,
>> "FDB entry towards bridge must be permanent");
>> return -EINVAL;
>> }
>> - err = br_fdb_external_learn_add(br, p, addr, vid, true);
>> + err = br_fdb_external_learn_add(br, p, addr, vid, true, false);
>> } else {
>> spin_lock_bh(&br->hash_lock);
>> err = fdb_add_entry(br, p, addr, ndm, nlh_flags, vid, nfea_tb);
>> @@ -1366,7 +1366,7 @@ void br_fdb_unsync_static(struct net_bridge *br,
>> struct net_bridge_port *p)
>>
>> int br_fdb_external_learn_add(struct net_bridge *br, struct
>> net_bridge_port *p,
>> const unsigned char *addr, u16 vid,
>> - bool swdev_notify)
>> + bool swdev_notify, bool locked)
>> {
>> struct net_bridge_fdb_entry *fdb;
>> bool modified = false;
>> @@ -1386,6 +1386,9 @@ int br_fdb_external_learn_add(struct net_bridge
>> *br, struct net_bridge_port *p,
>> if (!p)
>> flags |= BIT(BR_FDB_LOCAL);
>>
>> + if (locked)
>> + flags |= BIT(BR_FDB_ENTRY_LOCKED);
>> +
>> fdb = fdb_create(br, p, addr, vid, flags);
>> if (!fdb) {
>> err = -ENOMEM;
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 47a3598d25c8..9082451b4d40 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -811,7 +811,7 @@ int br_fdb_sync_static(struct net_bridge *br,
>> struct net_bridge_port *p);
>> void br_fdb_unsync_static(struct net_bridge *br, struct
>> net_bridge_port *p);
>> int br_fdb_external_learn_add(struct net_bridge *br, struct
>> net_bridge_port *p,
>> const unsigned char *addr, u16 vid,
>> - bool swdev_notify);
>> + bool swdev_notify, bool locked);
>> int br_fdb_external_learn_del(struct net_bridge *br, struct
>> net_bridge_port *p,
>> const unsigned char *addr, u16 vid,
>> bool swdev_notify);
>> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
>> index 8f3d76c751dd..85e566b856e1 100644
>> --- a/net/bridge/br_switchdev.c
>> +++ b/net/bridge/br_switchdev.c
>> @@ -136,6 +136,7 @@ static void br_switchdev_fdb_populate(struct
>> net_bridge *br,
>> item->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
>> item->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags);
>> item->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags);
>> + item->is_locked = test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags);
>> item->info.dev = (!p || item->is_local) ? br->dev : p->dev;
>> item->info.ctx = ctx;
>> }
>> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
>> index d9722e49864b..42f47a94b0f0 100644
>> --- a/net/dsa/dsa_priv.h
>> +++ b/net/dsa/dsa_priv.h
>> @@ -65,6 +65,7 @@ struct dsa_notifier_fdb_info {
>> const struct dsa_port *dp;
>> const unsigned char *addr;
>> u16 vid;
>> + bool is_locked;
>
> drop
>

If this is dropped, the whole scheme will not work as userspace will not
get notifications of new locked entries from the driver.

2022-08-02 10:18:20

by Hans Schultz

[permalink] [raw]
Subject: Re: [PATCH v4 net-next 2/6] net: switchdev: add support for offloading of fdb locked flag

On 2022-07-08 10:54, Vladimir Oltean wrote:

>> struct dsa_db db;
>> };
>>
>> @@ -131,6 +132,7 @@ struct dsa_switchdev_event_work {
>> unsigned char addr[ETH_ALEN];
>> u16 vid;
>> bool host_addr;
>> + bool is_locked;
>
> drop
>
>> };
>>
>> enum dsa_standalone_event {
>> @@ -232,7 +234,7 @@ int dsa_port_vlan_msti(struct dsa_port *dp,
>> const struct switchdev_vlan_msti *msti);
>> int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu);
>> int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>> - u16 vid);
>> + u16 vid, bool is_locked);
>
> drop
>
>> int dsa_port_fdb_del(struct dsa_port *dp, const unsigned char *addr,
>> u16 vid);
>> int dsa_port_standalone_host_fdb_add(struct dsa_port *dp,
>> diff --git a/net/dsa/port.c b/net/dsa/port.c
>> index 3738f2d40a0b..8bdac9aabe5d 100644
>> --- a/net/dsa/port.c
>> +++ b/net/dsa/port.c
>> @@ -35,6 +35,7 @@ static void dsa_port_notify_bridge_fdb_flush(const
>> struct dsa_port *dp, u16 vid)
>> struct net_device *brport_dev = dsa_port_to_bridge_port(dp);
>> struct switchdev_notifier_fdb_info info = {
>> .vid = vid,
>> + .is_locked = false,
>
> drop
>
>> };
>>
>> /* When the port becomes standalone it has already left the bridge.
>> @@ -950,12 +951,13 @@ int dsa_port_mtu_change(struct dsa_port *dp, int
>> new_mtu)
>> }
>>
>> int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
>> - u16 vid)
>> + u16 vid, bool is_locked)
>
> drop
>
>> {
>> struct dsa_notifier_fdb_info info = {
>> .dp = dp,
>> .addr = addr,
>> .vid = vid,
>> + .is_locked = is_locked,
>
> drop
>
>> .db = {
>> .type = DSA_DB_BRIDGE,
>> .bridge = *dp->bridge,
>> @@ -979,6 +981,7 @@ int dsa_port_fdb_del(struct dsa_port *dp, const
>> unsigned char *addr,
>> .dp = dp,
>> .addr = addr,
>> .vid = vid,
>> + .is_locked = false,
>
> drop
>
>> .db = {
>> .type = DSA_DB_BRIDGE,
>> .bridge = *dp->bridge,
>> @@ -999,6 +1002,7 @@ static int dsa_port_host_fdb_add(struct dsa_port
>> *dp,
>> .dp = dp,
>> .addr = addr,
>> .vid = vid,
>> + .is_locked = false,
>
> drop
>
>> .db = db,
>> };
>>
>> @@ -1050,6 +1054,7 @@ static int dsa_port_host_fdb_del(struct dsa_port
>> *dp,
>> .dp = dp,
>> .addr = addr,
>> .vid = vid,
>> + .is_locked = false,
>
> drop
>
>> .db = db,
>> };
>>
>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
>> index 801a5d445833..905b15e4eab9 100644
>> --- a/net/dsa/slave.c
>> +++ b/net/dsa/slave.c
>> @@ -2784,6 +2784,7 @@ static void
>> dsa_slave_switchdev_event_work(struct work_struct *work)
>> container_of(work, struct dsa_switchdev_event_work, work);
>> const unsigned char *addr = switchdev_work->addr;
>> struct net_device *dev = switchdev_work->dev;
>> + bool is_locked = switchdev_work->is_locked;
>
> drop
>
>> u16 vid = switchdev_work->vid;
>> struct dsa_switch *ds;
>> struct dsa_port *dp;
>> @@ -2799,7 +2800,7 @@ static void
>> dsa_slave_switchdev_event_work(struct work_struct *work)
>> else if (dp->lag)
>> err = dsa_port_lag_fdb_add(dp, addr, vid);
>> else
>> - err = dsa_port_fdb_add(dp, addr, vid);
>> + err = dsa_port_fdb_add(dp, addr, vid, is_locked);
>
> drop
>
>> if (err) {
>> dev_err(ds->dev,
>> "port %d failed to add %pM vid %d to fdb: %d\n",
>> @@ -2907,6 +2908,7 @@ static int dsa_slave_fdb_event(struct net_device
>> *dev,
>> ether_addr_copy(switchdev_work->addr, fdb_info->addr);
>> switchdev_work->vid = fdb_info->vid;
>> switchdev_work->host_addr = host_addr;
>> + switchdev_work->is_locked = fdb_info->is_locked;
>
> drop
>
>>
>> dsa_schedule_work(&switchdev_work->work);
>>
>> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
>> index 2b56218fc57c..32b1e7ac6373 100644
>> --- a/net/dsa/switch.c
>> +++ b/net/dsa/switch.c
>> @@ -234,7 +234,7 @@ static int dsa_port_do_mdb_del(struct dsa_port
>> *dp,
>> }
>>
>> static int dsa_port_do_fdb_add(struct dsa_port *dp, const unsigned
>> char *addr,
>> - u16 vid, struct dsa_db db)
>> + u16 vid, bool is_locked, struct dsa_db db)
>
> drop
>
>> {
>> struct dsa_switch *ds = dp->ds;
>> struct dsa_mac_addr *a;
>> @@ -398,7 +398,7 @@ static int dsa_switch_host_fdb_add(struct
>> dsa_switch *ds,
>> dsa_switch_for_each_port(dp, ds) {
>> if (dsa_port_host_address_match(dp, info->dp)) {
>> err = dsa_port_do_fdb_add(dp, info->addr, info->vid,
>> - info->db);
>> + false, info->db);
>
> drop
>
>> if (err)
>> break;
>> }
>> @@ -437,7 +437,7 @@ static int dsa_switch_fdb_add(struct dsa_switch
>> *ds,
>> if (!ds->ops->port_fdb_add)
>> return -EOPNOTSUPP;
>>
>> - return dsa_port_do_fdb_add(dp, info->addr, info->vid, info->db);
>> + return dsa_port_do_fdb_add(dp, info->addr, info->vid,
>> info->is_locked, info->db);
>
> drop
>
>> }
>>
>> static int dsa_switch_fdb_del(struct dsa_switch *ds,
>> --
>> 2.30.2
>>

Hi Vladimir and Ido,

I can either ignore locked entries early or late in the dsa/switchdev
layers.

If I ignore early, I think it should be in br_switchdev_fdb_notify() in
net/bridge/br_switchdev.c.
If I ignore late, I would think that it should be jut before sending it
to the driver(s), e.g. in dsa_port_do_fdb_add() in net/dsa/switch.c.

There is of course pros and cons of both options, but if the flag is
never to be sent to the driver, then it should be ignored early.

If ignored late most of this patch should not be dropped.

2022-08-11 05:43:52

by Benjamin Poirier

[permalink] [raw]
Subject: Re: [PATCH v4 net-next 0/6] Extend locked port feature with FDB locked flag (MAC-Auth/MAB)

On 2022-07-07 17:29 +0200, Hans Schultz wrote:
> This patch set extends the locked port feature for devices
> that are behind a locked port, but do not have the ability to
> authorize themselves as a supplicant using IEEE 802.1X.
> Such devices can be printers, meters or anything related to
> fixed installations. Instead of 802.1X authorization, devices
> can get access based on their MAC addresses being whitelisted.
^

Please consider using the alternate vocabulary discussed in
Documentation/process/coding-style.rst ?4.