On 11/09/18 02:22, Grygorii Strashko wrote:
>
>
> On 09/10/2018 04:18 PM, Stephen Hemminger wrote:
>> On Mon, 10 Sep 2018 13:16:01 +0300
>> Nikolay Aleksandrov <nikolay@xxxxxxxxxxxxxxxxxxx> 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 TI CPSW hardware we have following definition for similar FDB/ALE entries
> (AM437x TRM - 15.3.2.7.1.4 Unicast Address Table Entry):
>
> Block (BLOCK) – The block bit indicates that a packet with a matching source or
> destination address should be dropped (block the address).
> 0 – Address is not blocked.
> 1 – Drop a packet with a matching source or destination address (secure must be zero)
> If block and secure are both set, then they no longer mean block and secure.
> When both are set, the block and secure bits indicate that the packet is
> a unicast supervisory (super) packet and they determine the unicast forward state test criteria.
> If both bits are set then the packet is forwarded if the receive port is in
> the Forwarding/Blocking/Learning state.
> If both bits are not set then the packet is forwarded if the receive port is in
> the Forwarding state.
>
> Secure (SECURE) - This bit indicates that a packet with a matching source address
> should be dropped if the received port number is not equal to the table entry port_number.
> 0 – Received port number is a don’t care.
> 1 – Drop the packet if the received port is not the secure port for the source
> address and do not update the address (block must be zero)
>
> Updating Process:
> if ((source address found) and (receive port number != port_number) and (secure or block))
> then do not update address
>
> "sticky" would mean SECURE+BLOCK = supervisory (super) packet
>
That could work. but we haven't really made any arrangements w.r.t. to current implementations.
So if IUC secure+block should be:
- block:
1 – Drop a packet with a matching source or destination address (secure must be zero)
If block and secure are both set, then they no longer mean block and secure.
- secure:
1 – Drop the packet if the received port is not the secure port for the source
address and do not update the address (block must be zero)
These could mean a very different config based on their description. Feel free to add.
>>
>>>
>>> Signed-off-by: Nikolay Aleksandrov <nikolay@xxxxxxxxxxxxxxxxxxx>
>>> ---
>>> 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.
Sure, I've added a similar "fix" on my bridge todo list which is getting very long by the way,
but it is not the point of this patch.
