| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 11 Sep 2018 09:23:31 +0300
From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To: Grygorii Strashko <grygorii.strashko@...com>,
Stephen Hemminger <stephen@...workplumber.org>
Cc: netdev@...r.kernel.org, roopa@...ulusnetworks.com,
davem@...emloft.net, bridge@...ts.linux-foundation.org
Subject: Re: [PATCH net-next] net: bridge: add support for sticky fdb entries
On 11/09/18 02:55, Nikolay Aleksandrov wrote:
> 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@...ulusnetworks.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 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.
>
Nevermind my comment, got it. To support them fully though we will need to have more options.
They should be achievable through firewall as well.
>>>
>>>>
>>>> Signed-off-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.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.
>
> 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.
>
I'll send v2 with your proposed change, indeed more people will probably make use of it in the
future. Thanks for the feedback.
Cheers,
Nik
Powered by blists - more mailing lists