Re: [PATCH net-next] net: bridge: add support for sticky fdb entries

[Roopa Prabhu <roopa@cumulusnetworks.com>]

On Mon, Sep 10, 2018 at 2:18 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Mon, 10 Sep 2018 13:16:01 +0300
> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>
>> Add support for entries which are "sticky", i.e. will not change their port
>> if they show up from a different one. A new ndm flag is introduced for that
>> purpose - NTF_STICKY. We allow to set it only to non-local entries.
>
> Is there a name for this in other network switch API's?


In all switch ASIC's static fdb entries implicitly don't move.
Since the kernel does not follow the same for static fdb entries, we
want a new flag to
explicitly make the mac moves not possible.

As nikolay says, primary request for this came from an E-VPN rfc. It
uses the name 'sticky'...
hence the name 'sticky'  https://tools.ietf.org/html/rfc7432#section-15.2



>
>>
>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> ---
>> I'll send the selftest for sticky with the iproute2 patch if this one is
>> accepted. We've had multiple requests to support such flag and now it's
>> also needed for some eVPN and clag setups.
>>
>>  include/uapi/linux/neighbour.h |  1 +
>>  net/bridge/br_fdb.c            | 19 ++++++++++++++++---
>>  net/bridge/br_private.h        |  1 +
>>  3 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
>> index 904db6148476..998155444e0d 100644
>> --- a/include/uapi/linux/neighbour.h
>> +++ b/include/uapi/linux/neighbour.h
>> @@ -43,6 +43,7 @@ enum {
>>  #define NTF_PROXY    0x08    /* == ATF_PUBL */
>>  #define NTF_EXT_LEARNED      0x10
>>  #define NTF_OFFLOADED   0x20
>> +#define NTF_STICKY   0x40
>>  #define NTF_ROUTER   0x80
>>
>>  /*
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 502f66349530..26569ed06a4d 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -584,7 +584,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
>>                       unsigned long now = jiffies;
>>
>>                       /* fastpath: update of existing entry */
>> -                     if (unlikely(source != fdb->dst)) {
>> +                     if (unlikely(source != fdb->dst && !fdb->is_sticky)) {
>>                               fdb->dst = source;
>>                               fdb_modified = true;
>>                               /* Take over HW learned entry */
>> @@ -656,6 +656,8 @@ static int fdb_fill_info(struct sk_buff *skb, const struct net_bridge *br,
>>               ndm->ndm_flags |= NTF_OFFLOADED;
>>       if (fdb->added_by_external_learn)
>>               ndm->ndm_flags |= NTF_EXT_LEARNED;
>> +     if (fdb->is_sticky)
>> +             ndm->ndm_flags |= NTF_STICKY;
>>
>>       if (nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->key.addr))
>>               goto nla_put_failure;
>> @@ -772,7 +774,8 @@ int br_fdb_dump(struct sk_buff *skb,
>>
>>  /* Update (create or replace) forwarding database entry */
>>  static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
>> -                      const __u8 *addr, __u16 state, __u16 flags, __u16 vid)
>> +                      const u8 *addr, u16 state, u16 flags, u16 vid,
>> +                      u8 is_sticky)
>
> Why not change the API to take a full ndm flags, someone is sure to add more later.