Add an intermediate state for clients behind a locked port to allow for
possible opening of the port for said clients. This feature corresponds
to the Mac-Auth and MAC Authentication Bypass (MAB) named features. The
latter defined by Cisco.
Signed-off-by: Hans Schultz <[email protected]>
---
include/uapi/linux/neighbour.h | 1 +
net/bridge/br_fdb.c | 6 ++++++
net/bridge/br_input.c | 11 ++++++++++-
net/bridge/br_private.h | 3 ++-
4 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index db05fb55055e..83115a592d58 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -208,6 +208,7 @@ enum {
NFEA_UNSPEC,
NFEA_ACTIVITY_NOTIFY,
NFEA_DONT_REFRESH,
+ NFEA_LOCKED,
__NFEA_MAX
};
#define NFEA_MAX (__NFEA_MAX - 1)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 6ccda68bd473..396dcf3084cf 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
struct nda_cacheinfo ci;
struct nlmsghdr *nlh;
struct ndmsg *ndm;
+ u8 ext_flags = 0;
nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags);
if (nlh == NULL)
@@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
ndm->ndm_flags |= NTF_EXT_LEARNED;
if (test_bit(BR_FDB_STICKY, &fdb->flags))
ndm->ndm_flags |= NTF_STICKY;
+ if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags))
+ ext_flags |= 1 << NFEA_LOCKED;
if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
goto nla_put_failure;
if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
goto nla_put_failure;
+ if (nla_put_u8(skb, NDA_FDB_EXT_ATTRS, ext_flags))
+ goto nla_put_failure;
+
ci.ndm_used = jiffies_to_clock_t(now - fdb->used);
ci.ndm_confirmed = 0;
ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated);
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index e0c13fcc50ed..897908484b18 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -75,6 +75,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
struct net_bridge_mcast *brmctx;
struct net_bridge_vlan *vlan;
struct net_bridge *br;
+ unsigned long flags = 0;
u16 vid = 0;
u8 state;
@@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
- test_bit(BR_FDB_LOCAL, &fdb_src->flags))
+ test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
+ if (!fdb_src) {
+ set_bit(BR_FDB_ENTRY_LOCKED, &flags);
+ br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags);
+ }
goto drop;
+ } else {
+ if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags))
+ goto drop;
+ }
}
nbp_switchdev_frame_mark(p, skb);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 48bc61ebc211..f5a0b68c4857 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -248,7 +248,8 @@ enum {
BR_FDB_ADDED_BY_EXT_LEARN,
BR_FDB_OFFLOADED,
BR_FDB_NOTIFY,
- BR_FDB_NOTIFY_INACTIVE
+ BR_FDB_NOTIFY_INACTIVE,
+ BR_FDB_ENTRY_LOCKED,
};
struct net_bridge_fdb_key {
--
2.30.2
On tor, mar 10, 2022 at 17:57, Nikolay Aleksandrov <[email protected]> wrote:
> On 10/03/2022 17:38, Hans Schultz wrote:
>> On tor, mar 10, 2022 at 16:42, Nikolay Aleksandrov <[email protected]> wrote:
>>> On 10/03/2022 16:23, Hans Schultz wrote:
>>>> Add an intermediate state for clients behind a locked port to allow for
>>>> possible opening of the port for said clients. This feature corresponds
>>>> to the Mac-Auth and MAC Authentication Bypass (MAB) named features. The
>>>> latter defined by Cisco.
>>>>
>>>> Signed-off-by: Hans Schultz <[email protected]>
>>>> ---
>>>> include/uapi/linux/neighbour.h | 1 +
>>>> net/bridge/br_fdb.c | 6 ++++++
>>>> net/bridge/br_input.c | 11 ++++++++++-
>>>> net/bridge/br_private.h | 3 ++-
>>>> 4 files changed, 19 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>>>> index db05fb55055e..83115a592d58 100644
>>>> --- a/include/uapi/linux/neighbour.h
>>>> +++ b/include/uapi/linux/neighbour.h
>>>> @@ -208,6 +208,7 @@ enum {
>>>> NFEA_UNSPEC,
>>>> NFEA_ACTIVITY_NOTIFY,
>>>> NFEA_DONT_REFRESH,
>>>> + NFEA_LOCKED,
>>>> __NFEA_MAX
>>>> };
>>>
>>> Hmm, can you use NDA_FLAGS_EXT instead ?
>>> That should simplify things and reduce the nl size.
>>>
>>
>> I am using NDA_FDB_EXT_ATTRS. NFEA_LOCKED is just the
>> flag as the other flags section is full wrt the normal flags, but maybe it
>> doesn't fit in that section?
>>
>
> Actually wait a second, this is completely wrong use of NDA_FDB_EXT_ATTRS.
> That is a nested attribute, so the code below is wrong. More below..
>
>> I will just note that iproute2 support for parsing nested attributes
>> does not work, thus the BR_FDB_NOTIFY section (lines 150-165) are
>> obsolete with respect to iproute2 as it is now. I cannot rule out that
>> someone has some other tool that can handle this BR_FDB_NOTIFY, but I
>> could not make iproute2 as it stands handle nested attributes. And of
>> course there is no handling of NDA_FDB_EXT_ATTRS in iproute2 now.
>> >>> #define NFEA_MAX (__NFEA_MAX - 1)
>>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>>> index 6ccda68bd473..396dcf3084cf 100644
>>>> --- a/net/bridge/br_fdb.c
>>>> +++ b/net/bridge/br_fdb.c
>>>> @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>> struct nda_cacheinfo ci;
>>>> struct nlmsghdr *nlh;
>>>> struct ndmsg *ndm;
>>>> + u8 ext_flags = 0;
>>>>
>>>> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags);
>>>> if (nlh == NULL)
>>>> @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>> ndm->ndm_flags |= NTF_EXT_LEARNED;
>>>> if (test_bit(BR_FDB_STICKY, &fdb->flags))
>>>> ndm->ndm_flags |= NTF_STICKY;
>>>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags))
>>>> + ext_flags |= 1 << NFEA_LOCKED;
>>>>
>>>> if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>>> goto nla_put_failure;
>>>> if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
>>>> goto nla_put_failure;
>>>> + if (nla_put_u8(skb, NDA_FDB_EXT_ATTRS, ext_flags))
>>>> + goto nla_put_failure;
>>>> +
>
> This is wrong. NDA_FDB_EXT_ATTRS is a nested attribute, you can't use it as a u8.
> You need to have this structure:
> [ NDA_FDB_EXT_ATTRS ]
> ` [ NFEA_LOCKED ]
>
> But that's why I asked if you could use the NDA_FLAGS_EXT attribute. You can see
> the logic from the neigh code.
Ahh yes, NDA_FLAGS_EXT was not there in the 5.15.x kernel I have
originally being making the patches in.
I hope that the handling of nested attributes has been fixed in
iproute2. ;-)
>
> Also note that you need to account for the new attribute's size in fdb_nlmsg_size().
>
>
>>>> ci.ndm_used = jiffies_to_clock_t(now - fdb->used);
>>>> ci.ndm_confirmed = 0;
>>>> ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated);
>>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>>>> index e0c13fcc50ed..897908484b18 100644
>>>> --- a/net/bridge/br_input.c
>>>> +++ b/net/bridge/br_input.c
>>>> @@ -75,6 +75,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>>> struct net_bridge_mcast *brmctx;
>>>> struct net_bridge_vlan *vlan;
>>>> struct net_bridge *br;
>>>> + unsigned long flags = 0;
>>>
>>> Please move this below...
>>>
>>>> u16 vid = 0;
>>>> u8 state;
>>>>
>>>> @@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>>> br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>>>>
>>>> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
>>>> - test_bit(BR_FDB_LOCAL, &fdb_src->flags))
>>>> + test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
>>>> + if (!fdb_src) {
>>>
>>> ... here where it's only used.
>>>
>>
>> Forgot that one. Shall do!
>>
>>>> + set_bit(BR_FDB_ENTRY_LOCKED, &flags);
>>>> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags);
>>>> + }
>>>> goto drop;
>>>> + } else {
>>>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags))
>>>> + goto drop;
>>>> + }
>>>> }
>>>>
>>>> nbp_switchdev_frame_mark(p, skb);
>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>> index 48bc61ebc211..f5a0b68c4857 100644
>>>> --- a/net/bridge/br_private.h
>>>> +++ b/net/bridge/br_private.h
>>>> @@ -248,7 +248,8 @@ enum {
>>>> BR_FDB_ADDED_BY_EXT_LEARN,
>>>> BR_FDB_OFFLOADED,
>>>> BR_FDB_NOTIFY,
>>>> - BR_FDB_NOTIFY_INACTIVE
>>>> + BR_FDB_NOTIFY_INACTIVE,
>>>> + BR_FDB_ENTRY_LOCKED,
>>>> };
>>>>
>>>> struct net_bridge_fdb_key {
On tor, mar 10, 2022 at 16:42, Nikolay Aleksandrov <[email protected]> wrote:
> On 10/03/2022 16:23, Hans Schultz wrote:
>> Add an intermediate state for clients behind a locked port to allow for
>> possible opening of the port for said clients. This feature corresponds
>> to the Mac-Auth and MAC Authentication Bypass (MAB) named features. The
>> latter defined by Cisco.
>>
>> Signed-off-by: Hans Schultz <[email protected]>
>> ---
>> include/uapi/linux/neighbour.h | 1 +
>> net/bridge/br_fdb.c | 6 ++++++
>> net/bridge/br_input.c | 11 ++++++++++-
>> net/bridge/br_private.h | 3 ++-
>> 4 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>> index db05fb55055e..83115a592d58 100644
>> --- a/include/uapi/linux/neighbour.h
>> +++ b/include/uapi/linux/neighbour.h
>> @@ -208,6 +208,7 @@ enum {
>> NFEA_UNSPEC,
>> NFEA_ACTIVITY_NOTIFY,
>> NFEA_DONT_REFRESH,
>> + NFEA_LOCKED,
>> __NFEA_MAX
>> };
>
> Hmm, can you use NDA_FLAGS_EXT instead ?
> That should simplify things and reduce the nl size.
>
I am using NDA_FDB_EXT_ATTRS. NFEA_LOCKED is just the
flag as the other flags section is full wrt the normal flags, but maybe it
doesn't fit in that section?
I will just note that iproute2 support for parsing nested attributes
does not work, thus the BR_FDB_NOTIFY section (lines 150-165) are
obsolete with respect to iproute2 as it is now. I cannot rule out that
someone has some other tool that can handle this BR_FDB_NOTIFY, but I
could not make iproute2 as it stands handle nested attributes. And of
course there is no handling of NDA_FDB_EXT_ATTRS in iproute2 now.
>> #define NFEA_MAX (__NFEA_MAX - 1)
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 6ccda68bd473..396dcf3084cf 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>> struct nda_cacheinfo ci;
>> struct nlmsghdr *nlh;
>> struct ndmsg *ndm;
>> + u8 ext_flags = 0;
>>
>> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags);
>> if (nlh == NULL)
>> @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>> ndm->ndm_flags |= NTF_EXT_LEARNED;
>> if (test_bit(BR_FDB_STICKY, &fdb->flags))
>> ndm->ndm_flags |= NTF_STICKY;
>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags))
>> + ext_flags |= 1 << NFEA_LOCKED;
>>
>> if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>> goto nla_put_failure;
>> if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
>> goto nla_put_failure;
>> + if (nla_put_u8(skb, NDA_FDB_EXT_ATTRS, ext_flags))
>> + goto nla_put_failure;
>> +
>> ci.ndm_used = jiffies_to_clock_t(now - fdb->used);
>> ci.ndm_confirmed = 0;
>> ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated);
>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>> index e0c13fcc50ed..897908484b18 100644
>> --- a/net/bridge/br_input.c
>> +++ b/net/bridge/br_input.c
>> @@ -75,6 +75,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>> struct net_bridge_mcast *brmctx;
>> struct net_bridge_vlan *vlan;
>> struct net_bridge *br;
>> + unsigned long flags = 0;
>
> Please move this below...
>
>> u16 vid = 0;
>> u8 state;
>>
>> @@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>> br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>>
>> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
>> - test_bit(BR_FDB_LOCAL, &fdb_src->flags))
>> + test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
>> + if (!fdb_src) {
>
> ... here where it's only used.
>
Forgot that one. Shall do!
>> + set_bit(BR_FDB_ENTRY_LOCKED, &flags);
>> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags);
>> + }
>> goto drop;
>> + } else {
>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags))
>> + goto drop;
>> + }
>> }
>>
>> nbp_switchdev_frame_mark(p, skb);
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 48bc61ebc211..f5a0b68c4857 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -248,7 +248,8 @@ enum {
>> BR_FDB_ADDED_BY_EXT_LEARN,
>> BR_FDB_OFFLOADED,
>> BR_FDB_NOTIFY,
>> - BR_FDB_NOTIFY_INACTIVE
>> + BR_FDB_NOTIFY_INACTIVE,
>> + BR_FDB_ENTRY_LOCKED,
>> };
>>
>> struct net_bridge_fdb_key {
On 10/03/2022 16:23, Hans Schultz wrote:
> Add an intermediate state for clients behind a locked port to allow for
> possible opening of the port for said clients. This feature corresponds
> to the Mac-Auth and MAC Authentication Bypass (MAB) named features. The
> latter defined by Cisco.
>
> Signed-off-by: Hans Schultz <[email protected]>
> ---
> include/uapi/linux/neighbour.h | 1 +
> net/bridge/br_fdb.c | 6 ++++++
> net/bridge/br_input.c | 11 ++++++++++-
> net/bridge/br_private.h | 3 ++-
> 4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
> index db05fb55055e..83115a592d58 100644
> --- a/include/uapi/linux/neighbour.h
> +++ b/include/uapi/linux/neighbour.h
> @@ -208,6 +208,7 @@ enum {
> NFEA_UNSPEC,
> NFEA_ACTIVITY_NOTIFY,
> NFEA_DONT_REFRESH,
> + NFEA_LOCKED,
> __NFEA_MAX
> };
Hmm, can you use NDA_FLAGS_EXT instead ?
That should simplify things and reduce the nl size.
> #define NFEA_MAX (__NFEA_MAX - 1)
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 6ccda68bd473..396dcf3084cf 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
> struct nda_cacheinfo ci;
> struct nlmsghdr *nlh;
> struct ndmsg *ndm;
> + u8 ext_flags = 0;
>
> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags);
> if (nlh == NULL)
> @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
> ndm->ndm_flags |= NTF_EXT_LEARNED;
> if (test_bit(BR_FDB_STICKY, &fdb->flags))
> ndm->ndm_flags |= NTF_STICKY;
> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags))
> + ext_flags |= 1 << NFEA_LOCKED;
>
> if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
> goto nla_put_failure;
> if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
> goto nla_put_failure;
> + if (nla_put_u8(skb, NDA_FDB_EXT_ATTRS, ext_flags))
> + goto nla_put_failure;
> +
> ci.ndm_used = jiffies_to_clock_t(now - fdb->used);
> ci.ndm_confirmed = 0;
> ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated);
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index e0c13fcc50ed..897908484b18 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -75,6 +75,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> struct net_bridge_mcast *brmctx;
> struct net_bridge_vlan *vlan;
> struct net_bridge *br;
> + unsigned long flags = 0;
Please move this below...
> u16 vid = 0;
> u8 state;
>
> @@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>
> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
> - test_bit(BR_FDB_LOCAL, &fdb_src->flags))
> + test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
> + if (!fdb_src) {
... here where it's only used.
> + set_bit(BR_FDB_ENTRY_LOCKED, &flags);
> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags);
> + }
> goto drop;
> + } else {
> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags))
> + goto drop;
> + }
> }
>
> nbp_switchdev_frame_mark(p, skb);
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 48bc61ebc211..f5a0b68c4857 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -248,7 +248,8 @@ enum {
> BR_FDB_ADDED_BY_EXT_LEARN,
> BR_FDB_OFFLOADED,
> BR_FDB_NOTIFY,
> - BR_FDB_NOTIFY_INACTIVE
> + BR_FDB_NOTIFY_INACTIVE,
> + BR_FDB_ENTRY_LOCKED,
> };
>
> struct net_bridge_fdb_key {
On 10/03/2022 18:11, Hans Schultz wrote:
> On tor, mar 10, 2022 at 17:57, Nikolay Aleksandrov <[email protected]> wrote:
>> On 10/03/2022 17:38, Hans Schultz wrote:
>>> On tor, mar 10, 2022 at 16:42, Nikolay Aleksandrov <[email protected]> wrote:
>>>> On 10/03/2022 16:23, Hans Schultz wrote:
>>>>> Add an intermediate state for clients behind a locked port to allow for
>>>>> possible opening of the port for said clients. This feature corresponds
>>>>> to the Mac-Auth and MAC Authentication Bypass (MAB) named features. The
>>>>> latter defined by Cisco.
>>>>>
>>>>> Signed-off-by: Hans Schultz <[email protected]>
>>>>> ---
>>>>> include/uapi/linux/neighbour.h | 1 +
>>>>> net/bridge/br_fdb.c | 6 ++++++
>>>>> net/bridge/br_input.c | 11 ++++++++++-
>>>>> net/bridge/br_private.h | 3 ++-
>>>>> 4 files changed, 19 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>>>>> index db05fb55055e..83115a592d58 100644
>>>>> --- a/include/uapi/linux/neighbour.h
>>>>> +++ b/include/uapi/linux/neighbour.h
>>>>> @@ -208,6 +208,7 @@ enum {
>>>>> NFEA_UNSPEC,
>>>>> NFEA_ACTIVITY_NOTIFY,
>>>>> NFEA_DONT_REFRESH,
>>>>> + NFEA_LOCKED,
>>>>> __NFEA_MAX
>>>>> };
>>>>
>>>> Hmm, can you use NDA_FLAGS_EXT instead ?
>>>> That should simplify things and reduce the nl size.
>>>>
>>>
>>> I am using NDA_FDB_EXT_ATTRS. NFEA_LOCKED is just the
>>> flag as the other flags section is full wrt the normal flags, but maybe it
>>> doesn't fit in that section?
>>>
>>
>> Actually wait a second, this is completely wrong use of NDA_FDB_EXT_ATTRS.
>> That is a nested attribute, so the code below is wrong. More below..
>>
>>> I will just note that iproute2 support for parsing nested attributes
>>> does not work, thus the BR_FDB_NOTIFY section (lines 150-165) are
>>> obsolete with respect to iproute2 as it is now. I cannot rule out that
>>> someone has some other tool that can handle this BR_FDB_NOTIFY, but I
>>> could not make iproute2 as it stands handle nested attributes. And of
>>> course there is no handling of NDA_FDB_EXT_ATTRS in iproute2 now.
>>>>>> #define NFEA_MAX (__NFEA_MAX - 1)
>>>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>>>> index 6ccda68bd473..396dcf3084cf 100644
>>>>> --- a/net/bridge/br_fdb.c
>>>>> +++ b/net/bridge/br_fdb.c
>>>>> @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>>> struct nda_cacheinfo ci;
>>>>> struct nlmsghdr *nlh;
>>>>> struct ndmsg *ndm;
>>>>> + u8 ext_flags = 0;
>>>>>
>>>>> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags);
>>>>> if (nlh == NULL)
>>>>> @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>>> ndm->ndm_flags |= NTF_EXT_LEARNED;
>>>>> if (test_bit(BR_FDB_STICKY, &fdb->flags))
>>>>> ndm->ndm_flags |= NTF_STICKY;
>>>>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags))
>>>>> + ext_flags |= 1 << NFEA_LOCKED;
>>>>>
>>>>> if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>>>> goto nla_put_failure;
>>>>> if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
>>>>> goto nla_put_failure;
>>>>> + if (nla_put_u8(skb, NDA_FDB_EXT_ATTRS, ext_flags))
>>>>> + goto nla_put_failure;
>>>>> +
>>
>> This is wrong. NDA_FDB_EXT_ATTRS is a nested attribute, you can't use it as a u8.
>> You need to have this structure:
>> [ NDA_FDB_EXT_ATTRS ]
>> ` [ NFEA_LOCKED ]
>>
>> But that's why I asked if you could use the NDA_FLAGS_EXT attribute. You can see
>> the logic from the neigh code.
>
> Ahh yes, NDA_FLAGS_EXT was not there in the 5.15.x kernel I have
> originally being making the patches in.
>
> I hope that the handling of nested attributes has been fixed in
> iproute2. ;-)
>
It hasn't been broken, I'm guessing you're having issues with the nested bit being set.
Check NLA_F_NESTED and NLA_TYPE_MASK.
>>
>> Also note that you need to account for the new attribute's size in fdb_nlmsg_size().
>>
>>
>>>>> ci.ndm_used = jiffies_to_clock_t(now - fdb->used);
>>>>> ci.ndm_confirmed = 0;
>>>>> ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated);
>>>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>>>>> index e0c13fcc50ed..897908484b18 100644
>>>>> --- a/net/bridge/br_input.c
>>>>> +++ b/net/bridge/br_input.c
>>>>> @@ -75,6 +75,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>>>> struct net_bridge_mcast *brmctx;
>>>>> struct net_bridge_vlan *vlan;
>>>>> struct net_bridge *br;
>>>>> + unsigned long flags = 0;
>>>>
>>>> Please move this below...
>>>>
>>>>> u16 vid = 0;
>>>>> u8 state;
>>>>>
>>>>> @@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>>>> br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>>>>>
>>>>> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
>>>>> - test_bit(BR_FDB_LOCAL, &fdb_src->flags))
>>>>> + test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
>>>>> + if (!fdb_src) {
>>>>
>>>> ... here where it's only used.
>>>>
>>>
>>> Forgot that one. Shall do!
>>>
>>>>> + set_bit(BR_FDB_ENTRY_LOCKED, &flags);
>>>>> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags);
>>>>> + }
>>>>> goto drop;
>>>>> + } else {
>>>>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags))
>>>>> + goto drop;
>>>>> + }
>>>>> }
>>>>>
>>>>> nbp_switchdev_frame_mark(p, skb);
>>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>>> index 48bc61ebc211..f5a0b68c4857 100644
>>>>> --- a/net/bridge/br_private.h
>>>>> +++ b/net/bridge/br_private.h
>>>>> @@ -248,7 +248,8 @@ enum {
>>>>> BR_FDB_ADDED_BY_EXT_LEARN,
>>>>> BR_FDB_OFFLOADED,
>>>>> BR_FDB_NOTIFY,
>>>>> - BR_FDB_NOTIFY_INACTIVE
>>>>> + BR_FDB_NOTIFY_INACTIVE,
>>>>> + BR_FDB_ENTRY_LOCKED,
>>>>> };
>>>>>
>>>>> struct net_bridge_fdb_key {
On tor, mar 10, 2022 at 18:14, Nikolay Aleksandrov <[email protected]> wrote:
> On 10/03/2022 18:11, Hans Schultz wrote:
>> On tor, mar 10, 2022 at 17:57, Nikolay Aleksandrov <[email protected]> wrote:
>>> On 10/03/2022 17:38, Hans Schultz wrote:
>>>> On tor, mar 10, 2022 at 16:42, Nikolay Aleksandrov <[email protected]> wrote:
>>>>> On 10/03/2022 16:23, Hans Schultz wrote:
>>>>>> Add an intermediate state for clients behind a locked port to allow for
>>>>>> possible opening of the port for said clients. This feature corresponds
>>>>>> to the Mac-Auth and MAC Authentication Bypass (MAB) named features. The
>>>>>> latter defined by Cisco.
>>>>>>
>>>>>> Signed-off-by: Hans Schultz <[email protected]>
>>>>>> ---
>>>>>> include/uapi/linux/neighbour.h | 1 +
>>>>>> net/bridge/br_fdb.c | 6 ++++++
>>>>>> net/bridge/br_input.c | 11 ++++++++++-
>>>>>> net/bridge/br_private.h | 3 ++-
>>>>>> 4 files changed, 19 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>>>>>> index db05fb55055e..83115a592d58 100644
>>>>>> --- a/include/uapi/linux/neighbour.h
>>>>>> +++ b/include/uapi/linux/neighbour.h
>>>>>> @@ -208,6 +208,7 @@ enum {
>>>>>> NFEA_UNSPEC,
>>>>>> NFEA_ACTIVITY_NOTIFY,
>>>>>> NFEA_DONT_REFRESH,
>>>>>> + NFEA_LOCKED,
>>>>>> __NFEA_MAX
>>>>>> };
>>>>>
>>>>> Hmm, can you use NDA_FLAGS_EXT instead ?
>>>>> That should simplify things and reduce the nl size.
>>>>>
>>>>
>>>> I am using NDA_FDB_EXT_ATTRS. NFEA_LOCKED is just the
>>>> flag as the other flags section is full wrt the normal flags, but maybe it
>>>> doesn't fit in that section?
>>>>
>>>
>>> Actually wait a second, this is completely wrong use of NDA_FDB_EXT_ATTRS.
>>> That is a nested attribute, so the code below is wrong. More below..
>>>
>>>> I will just note that iproute2 support for parsing nested attributes
>>>> does not work, thus the BR_FDB_NOTIFY section (lines 150-165) are
>>>> obsolete with respect to iproute2 as it is now. I cannot rule out that
>>>> someone has some other tool that can handle this BR_FDB_NOTIFY, but I
>>>> could not make iproute2 as it stands handle nested attributes. And of
>>>> course there is no handling of NDA_FDB_EXT_ATTRS in iproute2 now.
>>>>>>> #define NFEA_MAX (__NFEA_MAX - 1)
>>>>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>>>>> index 6ccda68bd473..396dcf3084cf 100644
>>>>>> --- a/net/bridge/br_fdb.c
>>>>>> +++ b/net/bridge/br_fdb.c
>>>>>> @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>>>> struct nda_cacheinfo ci;
>>>>>> struct nlmsghdr *nlh;
>>>>>> struct ndmsg *ndm;
>>>>>> + u8 ext_flags = 0;
>>>>>>
>>>>>> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags);
>>>>>> if (nlh == NULL)
>>>>>> @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>>>>> ndm->ndm_flags |= NTF_EXT_LEARNED;
>>>>>> if (test_bit(BR_FDB_STICKY, &fdb->flags))
>>>>>> ndm->ndm_flags |= NTF_STICKY;
>>>>>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags))
>>>>>> + ext_flags |= 1 << NFEA_LOCKED;
>>>>>>
>>>>>> if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>>>>> goto nla_put_failure;
>>>>>> if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
>>>>>> goto nla_put_failure;
>>>>>> + if (nla_put_u8(skb, NDA_FDB_EXT_ATTRS, ext_flags))
>>>>>> + goto nla_put_failure;
>>>>>> +
>>>
>>> This is wrong. NDA_FDB_EXT_ATTRS is a nested attribute, you can't use it as a u8.
>>> You need to have this structure:
>>> [ NDA_FDB_EXT_ATTRS ]
>>> ` [ NFEA_LOCKED ]
>>>
>>> But that's why I asked if you could use the NDA_FLAGS_EXT attribute. You can see
>>> the logic from the neigh code.
>>
>> Ahh yes, NDA_FLAGS_EXT was not there in the 5.15.x kernel I have
>> originally being making the patches in.
>>
>> I hope that the handling of nested attributes has been fixed in
>> iproute2. ;-)
>>
>
> It hasn't been broken, I'm guessing you're having issues with the nested bit being set.
> Check NLA_F_NESTED and NLA_TYPE_MASK.
>
Hmmm, then I wonder why I could not make the same code as in the said
lines (150-165) in br_fdb.c give any parsed attributes in iproute2 under
tb[NDA_FDB_EXT_ATTR].
Did I miss something, or are those lines incorrect?
>>>
>>> Also note that you need to account for the new attribute's size in fdb_nlmsg_size().
>>>
>>>
>>>>>> ci.ndm_used = jiffies_to_clock_t(now - fdb->used);
>>>>>> ci.ndm_confirmed = 0;
>>>>>> ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated);
>>>>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>>>>>> index e0c13fcc50ed..897908484b18 100644
>>>>>> --- a/net/bridge/br_input.c
>>>>>> +++ b/net/bridge/br_input.c
>>>>>> @@ -75,6 +75,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>>>>> struct net_bridge_mcast *brmctx;
>>>>>> struct net_bridge_vlan *vlan;
>>>>>> struct net_bridge *br;
>>>>>> + unsigned long flags = 0;
>>>>>
>>>>> Please move this below...
>>>>>
>>>>>> u16 vid = 0;
>>>>>> u8 state;
>>>>>>
>>>>>> @@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>>>>> br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>>>>>>
>>>>>> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
>>>>>> - test_bit(BR_FDB_LOCAL, &fdb_src->flags))
>>>>>> + test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
>>>>>> + if (!fdb_src) {
>>>>>
>>>>> ... here where it's only used.
>>>>>
>>>>
>>>> Forgot that one. Shall do!
>>>>
>>>>>> + set_bit(BR_FDB_ENTRY_LOCKED, &flags);
>>>>>> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags);
>>>>>> + }
>>>>>> goto drop;
>>>>>> + } else {
>>>>>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags))
>>>>>> + goto drop;
>>>>>> + }
>>>>>> }
>>>>>>
>>>>>> nbp_switchdev_frame_mark(p, skb);
>>>>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>>>>> index 48bc61ebc211..f5a0b68c4857 100644
>>>>>> --- a/net/bridge/br_private.h
>>>>>> +++ b/net/bridge/br_private.h
>>>>>> @@ -248,7 +248,8 @@ enum {
>>>>>> BR_FDB_ADDED_BY_EXT_LEARN,
>>>>>> BR_FDB_OFFLOADED,
>>>>>> BR_FDB_NOTIFY,
>>>>>> - BR_FDB_NOTIFY_INACTIVE
>>>>>> + BR_FDB_NOTIFY_INACTIVE,
>>>>>> + BR_FDB_ENTRY_LOCKED,
>>>>>> };
>>>>>>
>>>>>> struct net_bridge_fdb_key {
On 10/03/2022 17:38, Hans Schultz wrote:
> On tor, mar 10, 2022 at 16:42, Nikolay Aleksandrov <[email protected]> wrote:
>> On 10/03/2022 16:23, Hans Schultz wrote:
>>> Add an intermediate state for clients behind a locked port to allow for
>>> possible opening of the port for said clients. This feature corresponds
>>> to the Mac-Auth and MAC Authentication Bypass (MAB) named features. The
>>> latter defined by Cisco.
>>>
>>> Signed-off-by: Hans Schultz <[email protected]>
>>> ---
>>> include/uapi/linux/neighbour.h | 1 +
>>> net/bridge/br_fdb.c | 6 ++++++
>>> net/bridge/br_input.c | 11 ++++++++++-
>>> net/bridge/br_private.h | 3 ++-
>>> 4 files changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>>> index db05fb55055e..83115a592d58 100644
>>> --- a/include/uapi/linux/neighbour.h
>>> +++ b/include/uapi/linux/neighbour.h
>>> @@ -208,6 +208,7 @@ enum {
>>> NFEA_UNSPEC,
>>> NFEA_ACTIVITY_NOTIFY,
>>> NFEA_DONT_REFRESH,
>>> + NFEA_LOCKED,
>>> __NFEA_MAX
>>> };
>>
>> Hmm, can you use NDA_FLAGS_EXT instead ?
>> That should simplify things and reduce the nl size.
>>
>
> I am using NDA_FDB_EXT_ATTRS. NFEA_LOCKED is just the
> flag as the other flags section is full wrt the normal flags, but maybe it
> doesn't fit in that section?
>
Actually wait a second, this is completely wrong use of NDA_FDB_EXT_ATTRS.
That is a nested attribute, so the code below is wrong. More below..
> I will just note that iproute2 support for parsing nested attributes
> does not work, thus the BR_FDB_NOTIFY section (lines 150-165) are
> obsolete with respect to iproute2 as it is now. I cannot rule out that
> someone has some other tool that can handle this BR_FDB_NOTIFY, but I
> could not make iproute2 as it stands handle nested attributes. And of
> course there is no handling of NDA_FDB_EXT_ATTRS in iproute2 now.
> >>> #define NFEA_MAX (__NFEA_MAX - 1)
>>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>>> index 6ccda68bd473..396dcf3084cf 100644
>>> --- a/net/bridge/br_fdb.c
>>> +++ b/net/bridge/br_fdb.c
>>> @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>> struct nda_cacheinfo ci;
>>> struct nlmsghdr *nlh;
>>> struct ndmsg *ndm;
>>> + u8 ext_flags = 0;
>>>
>>> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags);
>>> if (nlh == NULL)
>>> @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>> ndm->ndm_flags |= NTF_EXT_LEARNED;
>>> if (test_bit(BR_FDB_STICKY, &fdb->flags))
>>> ndm->ndm_flags |= NTF_STICKY;
>>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags))
>>> + ext_flags |= 1 << NFEA_LOCKED;
>>>
>>> if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>> goto nla_put_failure;
>>> if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
>>> goto nla_put_failure;
>>> + if (nla_put_u8(skb, NDA_FDB_EXT_ATTRS, ext_flags))
>>> + goto nla_put_failure;
>>> +
This is wrong. NDA_FDB_EXT_ATTRS is a nested attribute, you can't use it as a u8.
You need to have this structure:
[ NDA_FDB_EXT_ATTRS ]
` [ NFEA_LOCKED ]
But that's why I asked if you could use the NDA_FLAGS_EXT attribute. You can see
the logic from the neigh code.
Also note that you need to account for the new attribute's size in fdb_nlmsg_size().
>>> ci.ndm_used = jiffies_to_clock_t(now - fdb->used);
>>> ci.ndm_confirmed = 0;
>>> ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated);
>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>>> index e0c13fcc50ed..897908484b18 100644
>>> --- a/net/bridge/br_input.c
>>> +++ b/net/bridge/br_input.c
>>> @@ -75,6 +75,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>> struct net_bridge_mcast *brmctx;
>>> struct net_bridge_vlan *vlan;
>>> struct net_bridge *br;
>>> + unsigned long flags = 0;
>>
>> Please move this below...
>>
>>> u16 vid = 0;
>>> u8 state;
>>>
>>> @@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>>> br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>>>
>>> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
>>> - test_bit(BR_FDB_LOCAL, &fdb_src->flags))
>>> + test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
>>> + if (!fdb_src) {
>>
>> ... here where it's only used.
>>
>
> Forgot that one. Shall do!
>
>>> + set_bit(BR_FDB_ENTRY_LOCKED, &flags);
>>> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags);
>>> + }
>>> goto drop;
>>> + } else {
>>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags))
>>> + goto drop;
>>> + }
>>> }
>>>
>>> nbp_switchdev_frame_mark(p, skb);
>>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>>> index 48bc61ebc211..f5a0b68c4857 100644
>>> --- a/net/bridge/br_private.h
>>> +++ b/net/bridge/br_private.h
>>> @@ -248,7 +248,8 @@ enum {
>>> BR_FDB_ADDED_BY_EXT_LEARN,
>>> BR_FDB_OFFLOADED,
>>> BR_FDB_NOTIFY,
>>> - BR_FDB_NOTIFY_INACTIVE
>>> + BR_FDB_NOTIFY_INACTIVE,
>>> + BR_FDB_ENTRY_LOCKED,
>>> };
>>>
>>> struct net_bridge_fdb_key {
On Tue, Mar 15, 2022 at 09:48:52AM +0100, Hans Schultz wrote:
> On m?n, mar 14, 2022 at 17:30, Ido Schimmel <[email protected]> wrote:
> > On Thu, Mar 10, 2022 at 03:23:18PM +0100, Hans Schultz wrote:
> >> @@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> >> br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
> >>
> >> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
> >> - test_bit(BR_FDB_LOCAL, &fdb_src->flags))
> >> + test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
> >> + if (!fdb_src) {
> >> + set_bit(BR_FDB_ENTRY_LOCKED, &flags);
> >
> > This flag is read-only for user space, right? That is, the kernel needs
> > to reject it during netlink policy validation.
> >
>
> Yes, the flag is only readable from user space, unless there is a wish
> to change that.
OK, so please spell it out in the commit message so that it is clear the
flag can only be set by the kernel.
On Thu, Mar 10, 2022 at 03:23:18PM +0100, Hans Schultz wrote:
> Add an intermediate state for clients behind a locked port to allow for
> possible opening of the port for said clients. This feature corresponds
> to the Mac-Auth and MAC Authentication Bypass (MAB) named features. The
> latter defined by Cisco.
>
> Signed-off-by: Hans Schultz <[email protected]>
> ---
> include/uapi/linux/neighbour.h | 1 +
> net/bridge/br_fdb.c | 6 ++++++
> net/bridge/br_input.c | 11 ++++++++++-
> net/bridge/br_private.h | 3 ++-
> 4 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
> index db05fb55055e..83115a592d58 100644
> --- a/include/uapi/linux/neighbour.h
> +++ b/include/uapi/linux/neighbour.h
> @@ -208,6 +208,7 @@ enum {
> NFEA_UNSPEC,
> NFEA_ACTIVITY_NOTIFY,
> NFEA_DONT_REFRESH,
> + NFEA_LOCKED,
> __NFEA_MAX
> };
> #define NFEA_MAX (__NFEA_MAX - 1)
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 6ccda68bd473..396dcf3084cf 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
> struct nda_cacheinfo ci;
> struct nlmsghdr *nlh;
> struct ndmsg *ndm;
> + u8 ext_flags = 0;
>
> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags);
> if (nlh == NULL)
> @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
> ndm->ndm_flags |= NTF_EXT_LEARNED;
> if (test_bit(BR_FDB_STICKY, &fdb->flags))
> ndm->ndm_flags |= NTF_STICKY;
> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags))
> + ext_flags |= 1 << NFEA_LOCKED;
>
> if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
> goto nla_put_failure;
> if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
> goto nla_put_failure;
> + if (nla_put_u8(skb, NDA_FDB_EXT_ATTRS, ext_flags))
> + goto nla_put_failure;
> +
> ci.ndm_used = jiffies_to_clock_t(now - fdb->used);
> ci.ndm_confirmed = 0;
> ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated);
> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
> index e0c13fcc50ed..897908484b18 100644
> --- a/net/bridge/br_input.c
> +++ b/net/bridge/br_input.c
> @@ -75,6 +75,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> struct net_bridge_mcast *brmctx;
> struct net_bridge_vlan *vlan;
> struct net_bridge *br;
> + unsigned long flags = 0;
> u16 vid = 0;
> u8 state;
>
> @@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
> br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>
> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
> - test_bit(BR_FDB_LOCAL, &fdb_src->flags))
> + test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
> + if (!fdb_src) {
> + set_bit(BR_FDB_ENTRY_LOCKED, &flags);
This flag is read-only for user space, right? That is, the kernel needs
to reject it during netlink policy validation.
> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags);
> + }
> goto drop;
> + } else {
IIUC, we get here in case there is a non-local FDB entry with the SA
that points to our port. Can you write it as:
if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
test_bit(BR_FDB_LOCAL, &fdb_src->flags) ||
test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags)) {
if (!fdb_src) {
...
}
goto drop;
}
> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags))
> + goto drop;
> + }
> }
>
> nbp_switchdev_frame_mark(p, skb);
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 48bc61ebc211..f5a0b68c4857 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -248,7 +248,8 @@ enum {
> BR_FDB_ADDED_BY_EXT_LEARN,
> BR_FDB_OFFLOADED,
> BR_FDB_NOTIFY,
> - BR_FDB_NOTIFY_INACTIVE
> + BR_FDB_NOTIFY_INACTIVE,
> + BR_FDB_ENTRY_LOCKED,
> };
>
> struct net_bridge_fdb_key {
> --
> 2.30.2
>
On mån, mar 14, 2022 at 17:30, Ido Schimmel <[email protected]> wrote:
> On Thu, Mar 10, 2022 at 03:23:18PM +0100, Hans Schultz wrote:
>> Add an intermediate state for clients behind a locked port to allow for
>> possible opening of the port for said clients. This feature corresponds
>> to the Mac-Auth and MAC Authentication Bypass (MAB) named features. The
>> latter defined by Cisco.
>>
>> Signed-off-by: Hans Schultz <[email protected]>
>> ---
>> include/uapi/linux/neighbour.h | 1 +
>> net/bridge/br_fdb.c | 6 ++++++
>> net/bridge/br_input.c | 11 ++++++++++-
>> net/bridge/br_private.h | 3 ++-
>> 4 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>> index db05fb55055e..83115a592d58 100644
>> --- a/include/uapi/linux/neighbour.h
>> +++ b/include/uapi/linux/neighbour.h
>> @@ -208,6 +208,7 @@ enum {
>> NFEA_UNSPEC,
>> NFEA_ACTIVITY_NOTIFY,
>> NFEA_DONT_REFRESH,
>> + NFEA_LOCKED,
>> __NFEA_MAX
>> };
>> #define NFEA_MAX (__NFEA_MAX - 1)
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 6ccda68bd473..396dcf3084cf 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -105,6 +105,7 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>> struct nda_cacheinfo ci;
>> struct nlmsghdr *nlh;
>> struct ndmsg *ndm;
>> + u8 ext_flags = 0;
>>
>> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags);
>> if (nlh == NULL)
>> @@ -125,11 +126,16 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>> ndm->ndm_flags |= NTF_EXT_LEARNED;
>> if (test_bit(BR_FDB_STICKY, &fdb->flags))
>> ndm->ndm_flags |= NTF_STICKY;
>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb->flags))
>> + ext_flags |= 1 << NFEA_LOCKED;
>>
>> if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>> goto nla_put_failure;
>> if (nla_put_u32(skb, NDA_MASTER, br->dev->ifindex))
>> goto nla_put_failure;
>> + if (nla_put_u8(skb, NDA_FDB_EXT_ATTRS, ext_flags))
>> + goto nla_put_failure;
>> +
>> ci.ndm_used = jiffies_to_clock_t(now - fdb->used);
>> ci.ndm_confirmed = 0;
>> ci.ndm_updated = jiffies_to_clock_t(now - fdb->updated);
>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
>> index e0c13fcc50ed..897908484b18 100644
>> --- a/net/bridge/br_input.c
>> +++ b/net/bridge/br_input.c
>> @@ -75,6 +75,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>> struct net_bridge_mcast *brmctx;
>> struct net_bridge_vlan *vlan;
>> struct net_bridge *br;
>> + unsigned long flags = 0;
>> u16 vid = 0;
>> u8 state;
>>
>> @@ -94,8 +95,16 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
>> br_fdb_find_rcu(br, eth_hdr(skb)->h_source, vid);
>>
>> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
>> - test_bit(BR_FDB_LOCAL, &fdb_src->flags))
>> + test_bit(BR_FDB_LOCAL, &fdb_src->flags)) {
>> + if (!fdb_src) {
>> + set_bit(BR_FDB_ENTRY_LOCKED, &flags);
>
> This flag is read-only for user space, right? That is, the kernel needs
> to reject it during netlink policy validation.
>
Yes, the flag is only readable from user space, unless there is a wish
to change that.
>> + br_fdb_update(br, p, eth_hdr(skb)->h_source, vid, flags);
>> + }
>> goto drop;
>> + } else {
>
> IIUC, we get here in case there is a non-local FDB entry with the SA
> that points to our port. Can you write it as:
>
Yes, looks like that's more optimal. :)
> if (!fdb_src || READ_ONCE(fdb_src->dst) != p ||
> test_bit(BR_FDB_LOCAL, &fdb_src->flags) ||
> test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags)) {
> if (!fdb_src) {
> ...
> }
> goto drop;
> }
>
>> + if (test_bit(BR_FDB_ENTRY_LOCKED, &fdb_src->flags))
>> + goto drop;
>> + }
>> }
>>
>> nbp_switchdev_frame_mark(p, skb);
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 48bc61ebc211..f5a0b68c4857 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -248,7 +248,8 @@ enum {
>> BR_FDB_ADDED_BY_EXT_LEARN,
>> BR_FDB_OFFLOADED,
>> BR_FDB_NOTIFY,
>> - BR_FDB_NOTIFY_INACTIVE
>> + BR_FDB_NOTIFY_INACTIVE,
>> + BR_FDB_ENTRY_LOCKED,
>> };
>>
>> struct net_bridge_fdb_key {
>> --
>> 2.30.2
>>