Re: [Bridge] [PATCH v5 net-next 6/6] selftests: forwarding: add test of MAC-Auth Bypass to locked port tests
On 2022-09-28 10:46, Ido Schimmel wrote: "master" means manipulate the FDB of the master device. Therefore, the replace command manipulates the FDB of br0. "self" (which is the default [1]) means manipulate the FDB of the device itself. In case of br0 it means manipulate the FDB of the bridge device. For physical devices it usually translates to manipulating the unicast address filter list. Hi Ido, can you check the selftests of the v6 I have sent out using the iproute2-next I have also sent?
Re: [Bridge] [PATCH v6 net-next 0/9] Extend locked port feature with FDB locked flag (MAC-Auth/MAB)
On Thu, 29 Sep 2022 18:37:09 +0200 net...@kapio-technology.com wrote:
> On 2022-09-29 18:10, Jakub Kicinski wrote:
> > On Wed, 28 Sep 2022 17:02:47 +0200 Hans Schultz wrote:
> >> From: "Hans J. Schultz"
> >>
> >> 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.
> >
> > Try a allmodconfig build on latest net-next, seems broken.
>
> I have all different switch drivers enabled and I see no compile
> warnings or errors.
Just do what I told you - rebase on net-next, allmodconfig.
> I guess I will get a robot update if that is the
> case but please be specific as to what does not build.
The maintainers simply don't have time to hold everyone by the hand.
Sometimes I wish it was still okay to yell at people who post code
which does not build. Oh well.
../drivers/net/dsa/qca/qca8k-common.c:810:5: error: conflicting types for
‘qca8k_port_fdb_del’
int qca8k_port_fdb_del(struct dsa_switch *ds, int port,
^~
In file included from ../drivers/net/dsa/qca/qca8k-common.c:13:
../drivers/net/dsa/qca/qca8k.h:483:5: note: previous declaration of
‘qca8k_port_fdb_del’ was here
int qca8k_port_fdb_del(struct dsa_switch *ds, int port,
^~
../drivers/net/dsa/qca/qca8k-common.c: In function ‘qca8k_port_fdb_del’:
../drivers/net/dsa/qca/qca8k-common.c:818:6: error: ‘fdb_flags’ undeclared
(first use in this function); did you mean ‘tsq_flags’?
if (fdb_flags)
^
tsq_flags
../drivers/net/dsa/qca/qca8k-common.c:818:6: note: each undeclared identifier
is reported only once for each function it appears in
make[5]: *** [../scripts/Makefile.build:249:
drivers/net/dsa/qca/qca8k-common.o] Error 1
make[5]: *** Waiting for unfinished jobs
make[4]: *** [../scripts/Makefile.build:465: drivers/net/dsa/qca] Error 2
make[4]: *** Waiting for unfinished jobs
../drivers/net/dsa/sja1105/sja1105_main.c: In function ‘sja1105_fast_age’:
../drivers/net/dsa/sja1105/sja1105_main.c:1941:61: error: incompatible type for
argument 5 of ‘sja1105_fdb_del’
rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
^~
../drivers/net/dsa/sja1105/sja1105_main.c:1831:11: note: expected ‘u16’ {aka
‘short unsigned int’} but argument is of type ‘struct dsa_db’
u16 fdb_flags, struct dsa_db db)
^
../drivers/net/dsa/sja1105/sja1105_main.c:1941:8: error: too few arguments to
function ‘sja1105_fdb_del’
rc = sja1105_fdb_del(ds, port, macaddr, l2_lookup.vlanid, db);
^~~
../drivers/net/dsa/sja1105/sja1105_main.c:1829:12: note: declared here
static int sja1105_fdb_del(struct dsa_switch *ds, int port,
^~~
../drivers/net/dsa/sja1105/sja1105_main.c: In function ‘sja1105_mdb_del’:
../drivers/net/dsa/sja1105/sja1105_main.c:1962:56: error: incompatible type for
argument 5 of ‘sja1105_fdb_del’
return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid, db);
^~
../drivers/net/dsa/sja1105/sja1105_main.c:1831:11: note: expected ‘u16’ {aka
‘short unsigned int’} but argument is of type ‘struct dsa_db’
u16 fdb_flags, struct dsa_db db)
^
../drivers/net/dsa/sja1105/sja1105_main.c:1962:9: error: too few arguments to
function ‘sja1105_fdb_del’
return sja1105_fdb_del(ds, port, mdb->addr, mdb->vid, db);
^~~
../drivers/net/dsa/sja1105/sja1105_main.c:1829:12: note: declared here
static int sja1105_fdb_del(struct dsa_switch *ds, int port,
^~~
../drivers/net/dsa/sja1105/sja1105_main.c:1963:1: error: control reaches end of
non-void function [-Werror=return-type]
}
^
cc1: some warnings being treated as errors
make[5]: *** [../scripts/Makefile.build:249:
drivers/net/dsa/sja1105/sja1105_main.o] Error 1
make[5]: *** Waiting for unfinished jobs
make[4]: *** [../scripts/Makefile.build:465: drivers/net/dsa/sja1105] Error 2
make[3]: *** [../scripts/Makefile.build:465: drivers/net/dsa] Error 2
make[3]: *** Waiting for unfinished jobs
make[2]: *** [../scripts/Makefile.build:465: drivers/net] Error 2
make[1]: *** [/home/kicinski/linux/Makefile:1852: drivers] Error 2
make[1]: *** Waiting for unfinished jobs
make: *** [Makefile:222: __sub-make] Error 2
Re: [Bridge] [PATCH v6 net-next 0/9] Extend locked port feature with FDB locked flag (MAC-Auth/MAB)
On 2022-09-29 18:10, Jakub Kicinski wrote: On Wed, 28 Sep 2022 17:02:47 +0200 Hans Schultz wrote: From: "Hans J. Schultz" 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. Try a allmodconfig build on latest net-next, seems broken. I have all different switch drivers enabled and I see no compile warnings or errors. I guess I will get a robot update if that is the case, but please be specific as to what does not build.
Re: [Bridge] [PATCH v6 net-next 9/9] selftests: forwarding: add test of MAC-Auth Bypass to locked port tests
On Thu, 29 Sep 2022 18:17:40 +0200 net...@kapio-technology.com wrote: > > If you were trying to repost just the broken patches - that's not gonna > > work :( > > Sorry, I do not understand what 'broken' patches you are referring to? > > I think that the locked port tests should be working? Ignore it then. v6 does not build, see my other reply.
Re: [Bridge] [PATCH v6 net-next 9/9] selftests: forwarding: add test of MAC-Auth Bypass to locked port tests
On 2022-09-29 18:11, Jakub Kicinski wrote: On Wed, 28 Sep 2022 19:49:04 +0200 Hans Schultz wrote: From: "Hans J. Schultz" Verify that the MAC-Auth mechanism works by adding a FDB entry with the locked flag set, denying access until the FDB entry is replaced with a FDB entry without the locked flag set. Add test of blackhole fdb entries, verifying that there is no forwarding to a blackhole entry from any port, and that the blackhole entry can be replaced. Also add a test that verifies that sticky FDB entries cannot roam (this is not needed for now, but should in general be present anyhow for future applications). If you were trying to repost just the broken patches - that's not gonna work :( Sorry, I do not understand what 'broken' patches you are referring to? I think that the locked port tests should be working?
Re: [Bridge] [PATCH iproute2-next 2/2] bridge: fdb: enable FDB blackhole feature
On 2022-09-29 17:43, Stephen Hemminger wrote:
On Thu, 29 Sep 2022 17:21:37 +0200
Hans Schultz wrote:
@@ -493,6 +496,8 @@ static int fdb_modify(int cmd, int flags, int
argc, char **argv)
req.ndm.ndm_flags |= NTF_EXT_LEARNED;
} else if (matches(*argv, "sticky") == 0) {
req.ndm.ndm_flags |= NTF_STICKY;
+ } else if (matches(*argv, "blackhole") == 0) {
+ ext_flags |= NTF_EXT_BLACKHOLE;
} else {
if (strcmp(*argv, "to") == 0)
NEXT_ARG();
The parsing of flags is weird here, most of the flags are compared with
strcmp()
but some use matches().. I should have used strcmp() all the time; but
at the
time did not realize what kind of confusion matches() can cause.
Maybe just change all of them then, and then how about using strncmp()
and maybe also strnlen() instead?
Re: [Bridge] [PATCH v6 net-next 9/9] selftests: forwarding: add test of MAC-Auth Bypass to locked port tests
On Wed, 28 Sep 2022 19:49:04 +0200 Hans Schultz wrote: > From: "Hans J. Schultz" > > Verify that the MAC-Auth mechanism works by adding a FDB entry with the > locked flag set, denying access until the FDB entry is replaced with a > FDB entry without the locked flag set. > > Add test of blackhole fdb entries, verifying that there is no forwarding > to a blackhole entry from any port, and that the blackhole entry can be > replaced. > > Also add a test that verifies that sticky FDB entries cannot roam (this > is not needed for now, but should in general be present anyhow for future > applications). If you were trying to repost just the broken patches - that's not gonna work :(
Re: [Bridge] [PATCH v6 net-next 0/9] Extend locked port feature with FDB locked flag (MAC-Auth/MAB)
On Wed, 28 Sep 2022 17:02:47 +0200 Hans Schultz wrote: > From: "Hans J. Schultz" > > 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. Try a allmodconfig build on latest net-next, seems broken.
Re: [Bridge] [PATCH iproute2-next 2/2] bridge: fdb: enable FDB blackhole feature
On Thu, 29 Sep 2022 17:21:37 +0200
Hans Schultz wrote:
>
> @@ -493,6 +496,8 @@ static int fdb_modify(int cmd, int flags, int argc, char
> **argv)
> req.ndm.ndm_flags |= NTF_EXT_LEARNED;
> } else if (matches(*argv, "sticky") == 0) {
> req.ndm.ndm_flags |= NTF_STICKY;
> + } else if (matches(*argv, "blackhole") == 0) {
> + ext_flags |= NTF_EXT_BLACKHOLE;
> } else {
> if (strcmp(*argv, "to") == 0)
> NEXT_ARG();
The parsing of flags is weird here, most of the flags are compared with strcmp()
but some use matches().. I should have used strcmp() all the time; but at the
time did not realize what kind of confusion matches() can cause.
[Bridge] [PATCH iproute2-next 1/2] bridge: link: enable MacAuth/MAB feature
The MAB feature can be enabled on a locked port with the command:
bridge link set dev mab on
Signed-off-by: Hans Schultz
---
bridge/fdb.c | 17 +++--
bridge/link.c | 21 ++---
include/uapi/linux/if_link.h | 1 +
include/uapi/linux/neighbour.h | 7 ++-
ip/iplink_bridge_slave.c | 16 +---
man/man8/bridge.8 | 10 ++
man/man8/ip-link.8.in | 8
7 files changed, 71 insertions(+), 9 deletions(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index 5f71bde0..0fbe9bd3 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -93,7 +93,7 @@ static int state_a2n(unsigned int *s, const char *arg)
return 0;
}
-static void fdb_print_flags(FILE *fp, unsigned int flags)
+static void fdb_print_flags(FILE *fp, unsigned int flags, __u8 ext_flags)
{
open_json_array(PRINT_JSON,
is_json_context() ? "flags" : "");
@@ -116,6 +116,9 @@ static void fdb_print_flags(FILE *fp, unsigned int flags)
if (flags & NTF_STICKY)
print_string(PRINT_ANY, NULL, "%s ", "sticky");
+ if (ext_flags & NTF_EXT_LOCKED)
+ print_string(PRINT_ANY, NULL, "%s ", "locked");
+
close_json_array(PRINT_JSON, NULL);
}
@@ -144,6 +147,7 @@ int print_fdb(struct nlmsghdr *n, void *arg)
struct ndmsg *r = NLMSG_DATA(n);
int len = n->nlmsg_len;
struct rtattr *tb[NDA_MAX+1];
+ __u32 ext_flags = 0;
__u16 vid = 0;
if (n->nlmsg_type != RTM_NEWNEIGH && n->nlmsg_type != RTM_DELNEIGH) {
@@ -170,6 +174,9 @@ int print_fdb(struct nlmsghdr *n, void *arg)
parse_rtattr(tb, NDA_MAX, NDA_RTA(r),
n->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
+ if (tb[NDA_FLAGS_EXT])
+ ext_flags = rta_getattr_u32(tb[NDA_FLAGS_EXT]);
+
if (tb[NDA_VLAN])
vid = rta_getattr_u16(tb[NDA_VLAN]);
@@ -266,7 +273,7 @@ int print_fdb(struct nlmsghdr *n, void *arg)
if (show_stats && tb[NDA_CACHEINFO])
fdb_print_stats(fp, RTA_DATA(tb[NDA_CACHEINFO]));
- fdb_print_flags(fp, r->ndm_flags);
+ fdb_print_flags(fp, r->ndm_flags, ext_flags);
if (tb[NDA_MASTER])
@@ -414,6 +421,7 @@ static int fdb_modify(int cmd, int flags, int argc, char
**argv)
char *endptr;
short vid = -1;
__u32 nhid = 0;
+ __u32 ext_flags = 0;
while (argc > 0) {
if (strcmp(*argv, "dev") == 0) {
@@ -527,6 +535,11 @@ static int fdb_modify(int cmd, int flags, int argc, char
**argv)
if (dst_ok)
addattr_l(, sizeof(req), NDA_DST, , dst.bytelen);
+ if (ext_flags &&
+ addattr_l(, sizeof(req), NDA_FLAGS_EXT, _flags,
+ sizeof(ext_flags)) < 0)
+ return -1;
+
if (vid >= 0)
addattr16(, sizeof(req), NDA_VLAN, vid);
if (nhid > 0)
diff --git a/bridge/link.c b/bridge/link.c
index 3810fa04..dd69d7c3 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -181,9 +181,14 @@ static void print_protinfo(FILE *fp, struct rtattr *attr)
if (prtb[IFLA_BRPORT_ISOLATED])
print_on_off(PRINT_ANY, "isolated", "isolated %s ",
rta_getattr_u8(prtb[IFLA_BRPORT_ISOLATED]));
- if (prtb[IFLA_BRPORT_LOCKED])
- print_on_off(PRINT_ANY, "locked", "locked %s ",
-rta_getattr_u8(prtb[IFLA_BRPORT_LOCKED]));
+ if (prtb[IFLA_BRPORT_LOCKED]) {
+ if (prtb[IFLA_BRPORT_MAB] &&
rta_getattr_u8(prtb[IFLA_BRPORT_MAB]))
+ print_on_off(PRINT_ANY, "locked mab", "locked
mab %s ",
+
rta_getattr_u8(prtb[IFLA_BRPORT_LOCKED]));
+ else
+ print_on_off(PRINT_ANY, "locked", "locked %s ",
+
rta_getattr_u8(prtb[IFLA_BRPORT_LOCKED]));
+ }
} else
print_stp_state(rta_getattr_u8(attr));
}
@@ -281,6 +286,7 @@ static void usage(void)
" [ vlan_tunnel {on | off} ]\n"
" [ isolated {on | off} ]\n"
" [ locked {on | off} ]\n"
+ " [ mab {on | off} ]\n"
" [ hwmode {vepa | veb} ]\n"
" [ backup_port DEVICE ] [
nobackup_port ]\n"
" [ self ] [ master ]\n"
@@ -312,6 +318,7 @@ static int brlink_modify(int argc, char **argv)
__s8 bcast_flood = -1;
__s8 mcast_to_unicast = -1;
__s8 locked = -1;
+ __s8 macauth = -1;
__s8 isolated = -1;
__s8 hairpin = -1;
[Bridge] [PATCH iproute2-next 2/2] bridge: fdb: enable FDB blackhole feature
Block traffic to a specific host with the command:
bridge fdb add vlan dev br0 blackhole
The blackhole FDB entries can be added, deleted and replaced with
ordinary FDB entries.
Signed-off-by: Hans Schultz
---
bridge/fdb.c | 7 ++-
include/uapi/linux/neighbour.h | 4
man/man8/bridge.8 | 6 ++
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index 0fbe9bd3..2160f1c2 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -38,7 +38,7 @@ static void usage(void)
fprintf(stderr,
"Usage: bridge fdb { add | append | del | replace } ADDR dev
DEV\n"
" [ self ] [ master ] [ use ] [ router ] [
extern_learn ]\n"
- " [ sticky ] [ local | static | dynamic ] [ vlan
VID ]\n"
+ " [ sticky ] [ local | static | dynamic ]
[blackhole] [ vlan VID ]\n"
" { [ dst IPADDR ] [ port PORT] [ vni VNI ] | [
nhid NHID ] }\n"
" [ via DEV ] [ src_vni VNI ]\n"
" bridge fdb [ show [ br BRDEV ] [ brport DEV ] [ vlan
VID ]\n"
@@ -116,6 +116,9 @@ static void fdb_print_flags(FILE *fp, unsigned int flags,
__u8 ext_flags)
if (flags & NTF_STICKY)
print_string(PRINT_ANY, NULL, "%s ", "sticky");
+ if (ext_flags & NTF_EXT_BLACKHOLE)
+ print_string(PRINT_ANY, NULL, "%s ", "blackhole");
+
if (ext_flags & NTF_EXT_LOCKED)
print_string(PRINT_ANY, NULL, "%s ", "locked");
@@ -493,6 +496,8 @@ static int fdb_modify(int cmd, int flags, int argc, char
**argv)
req.ndm.ndm_flags |= NTF_EXT_LEARNED;
} else if (matches(*argv, "sticky") == 0) {
req.ndm.ndm_flags |= NTF_STICKY;
+ } else if (matches(*argv, "blackhole") == 0) {
+ ext_flags |= NTF_EXT_BLACKHOLE;
} else {
if (strcmp(*argv, "to") == 0)
NEXT_ARG();
diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index 4dda051b..cc7d540e 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -54,6 +54,7 @@ enum {
/* Extended flags under NDA_FLAGS_EXT: */
#define NTF_EXT_MANAGED(1 << 0)
#define NTF_EXT_LOCKED (1 << 1)
+#define NTF_EXT_BLACKHOLE (1 << 2)
/*
* Neighbor Cache Entry States.
@@ -91,6 +92,9 @@ enum {
* NTF_EXT_LOCKED flagged FDB entries are placeholder entries used with the
* locked port feature, that ensures that an entry exists while at the same
* time dropping packets on ingress with src MAC and VID matching the entry.
+ *
+ * NTF_EXT_BLACKHOLE flagged FDB entries ensure that no forwarding is allowed
+ * from any port to the destination MAC, VID pair associated with it.
*/
struct nda_cacheinfo {
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 40250477..af2e7db2 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -699,6 +699,12 @@ controller learnt dynamic entry. Kernel will not age such
an entry.
- this entry will not change its port due to learning.
.sp
+.B blackhole
+- this is an entry that denies all forwarding from any port to a destination
+matching the entry. It can be added by userspace, but the flag is mostly set
+from a hardware driver.
+.sp
+
.in -8
The next command line parameters apply only
when the specified device
--
2.34.1
