All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@idosch.org>
To: Nikolay Aleksandrov <razor@blackwall.org>
Cc: netdev@vger.kernel.org, bridge@lists.linux-foundation.org,
	davem@davemloft.net, kuba@kernel.org, roopa@nvidia.com
Subject: Re: [Bridge] [PATCH net-next 0/6] net: bridge: add flush filtering support
Date: Mon, 11 Apr 2022 10:47:33 +0300	[thread overview]
Message-ID: <YlPdFS//hYbBSAkT@shredder> (raw)
In-Reply-To: <20220409105857.803667-1-razor@blackwall.org>

On Sat, Apr 09, 2022 at 01:58:51PM +0300, Nikolay Aleksandrov wrote:
> Hi,
> This patch-set adds support to specify filtering conditions for a flush
> operation. Initially only FDB flush filtering is added, later MDB
> support will be added as well. Some user-space applications need a way
> to delete only a specific set of entries, e.g. mlag implementations need
> a way to flush only dynamic entries excluding externally learned ones
> or only externally learned ones without static entries etc. Also apps
> usually want to target only a specific vlan or port/vlan combination.
> The current 2 flush operations (per port and bridge-wide) are not
> extensible and cannot provide such filtering, so a new bridge af
> attribute is added (IFLA_BRIDGE_FLUSH) which contains the filtering
> information for each object type which has to be flushed.
> An example structure for fdbs:
>      [ IFLA_BRIDGE_FLUSH ]
>       `[ BRIDGE_FDB_FLUSH ]
>         `[ FDB_FLUSH_NDM_STATE ]
>         `[ FDB_FLUSH_NDM_FLAGS ]
> 
> I decided against embedding these into the old flush attributes for
> multiple reasons - proper error handling on unsupported attributes,
> older kernels silently flushing all, need for a second mechanism to
> signal that the attribute should be parsed (e.g. using boolopts),
> special treatment for permanent entries.
> 
> Examples:
> $ bridge fdb flush dev bridge vlan 100 static
> < flush all static entries on vlan 100 >
> $ bridge fdb flush dev bridge vlan 1 dynamic
> < flush all dynamic entries on vlan 1 >
> $ bridge fdb flush dev bridge port ens16 vlan 1 dynamic
> < flush all dynamic entries on port ens16 and vlan 1 >
> $ bridge fdb flush dev bridge nooffloaded nopermanent
> < flush all non-offloaded and non-permanent entries >
> $ bridge fdb flush dev bridge static noextern_learn
> < flush all static entries which are not externally learned >
> $ bridge fdb flush dev bridge permanent
> < flush all permanent entries >

IIUC, the new IFLA_BRIDGE_FLUSH attribute is supposed to be passed in
RTM_SETLINK messages, but the current 'bridge fdb' commands all
correspond to RTM_{NEW,DEL,GET}NEIGH messages. To continue following
this pattern, did you consider turning the above examples to the
following?

$ ip link set dev bridge type bridge fdb_flush vlan 100 static
$ ip link set dev bridge type bridge fdb_flush vlan 1 dynamic
$ ip link set dev ens16 type bridge_slave fdb_flush vlan 1 dynamic
$ ip link set dev bridge type bridge fdb_flush nooffloaded nopermanent
$ ip link set dev bridge type bridge fdb_flush static noextern_learn
$ ip link set dev bridge type bridge fdb_flush permanent

It's not critical, but I like the correspondence between iproute2
commands and the underlying netlink messages.

> 
> Note that all flags have their negated version (static vs nostatic etc)
> and there are some tricky cases to handle like "static" which in flag
> terms means fdbs that have NUD_NOARP but *not* NUD_PERMANENT, so the
> mask matches on both but we need only NUD_NOARP to be set. That's
> because permanent entries have both set so we can't just match on
> NUD_NOARP. Also note that this flush operation doesn't treat permanent
> entries in a special way (fdb_delete vs fdb_delete_local), it will
> delete them regardless if any port is using them. We can extend the api
> with a flag to do that if needed in the future.
> 
> Patches in this set:
>  1. adds the new IFLA_BRIDGE_FLUSH bridge af attribute
>  2. adds a basic structure to describe an fdb flush filter
>  3. adds fdb netlink flush call via BRIDGE_FDB_FLUSH attribute
>  4 - 6. add support for specifying various fdb fields to filter
> 
> Patch-sets (in order):
>  - Initial flush infra and fdb flush filtering (this set)
>  - iproute2 support
>  - selftests
> 
> Future work:
>  - mdb flush support
> 
> Thanks,
>  Nik
> 
> Nikolay Aleksandrov (6):
>   net: bridge: add a generic flush operation
>   net: bridge: fdb: add support for fine-grained flushing
>   net: bridge: fdb: add new nl attribute-based flush call
>   net: bridge: fdb: add support for flush filtering based on ndm flags
>     and state
>   net: bridge: fdb: add support for flush filtering based on ifindex
>   net: bridge: fdb: add support for flush filtering based on vlan id
> 
>  include/uapi/linux/if_bridge.h |  22 ++++++
>  net/bridge/br_fdb.c            | 128 +++++++++++++++++++++++++++++++--
>  net/bridge/br_netlink.c        |  59 ++++++++++++++-
>  net/bridge/br_private.h        |  12 +++-
>  net/bridge/br_sysfs_br.c       |   6 +-
>  5 files changed, 215 insertions(+), 12 deletions(-)
> 
> -- 
> 2.35.1
> 

WARNING: multiple messages have this Message-ID (diff)
From: Ido Schimmel <idosch@idosch.org>
To: Nikolay Aleksandrov <razor@blackwall.org>
Cc: netdev@vger.kernel.org, roopa@nvidia.com, kuba@kernel.org,
	davem@davemloft.net, bridge@lists.linux-foundation.org
Subject: Re: [PATCH net-next 0/6] net: bridge: add flush filtering support
Date: Mon, 11 Apr 2022 10:47:33 +0300	[thread overview]
Message-ID: <YlPdFS//hYbBSAkT@shredder> (raw)
In-Reply-To: <20220409105857.803667-1-razor@blackwall.org>

On Sat, Apr 09, 2022 at 01:58:51PM +0300, Nikolay Aleksandrov wrote:
> Hi,
> This patch-set adds support to specify filtering conditions for a flush
> operation. Initially only FDB flush filtering is added, later MDB
> support will be added as well. Some user-space applications need a way
> to delete only a specific set of entries, e.g. mlag implementations need
> a way to flush only dynamic entries excluding externally learned ones
> or only externally learned ones without static entries etc. Also apps
> usually want to target only a specific vlan or port/vlan combination.
> The current 2 flush operations (per port and bridge-wide) are not
> extensible and cannot provide such filtering, so a new bridge af
> attribute is added (IFLA_BRIDGE_FLUSH) which contains the filtering
> information for each object type which has to be flushed.
> An example structure for fdbs:
>      [ IFLA_BRIDGE_FLUSH ]
>       `[ BRIDGE_FDB_FLUSH ]
>         `[ FDB_FLUSH_NDM_STATE ]
>         `[ FDB_FLUSH_NDM_FLAGS ]
> 
> I decided against embedding these into the old flush attributes for
> multiple reasons - proper error handling on unsupported attributes,
> older kernels silently flushing all, need for a second mechanism to
> signal that the attribute should be parsed (e.g. using boolopts),
> special treatment for permanent entries.
> 
> Examples:
> $ bridge fdb flush dev bridge vlan 100 static
> < flush all static entries on vlan 100 >
> $ bridge fdb flush dev bridge vlan 1 dynamic
> < flush all dynamic entries on vlan 1 >
> $ bridge fdb flush dev bridge port ens16 vlan 1 dynamic
> < flush all dynamic entries on port ens16 and vlan 1 >
> $ bridge fdb flush dev bridge nooffloaded nopermanent
> < flush all non-offloaded and non-permanent entries >
> $ bridge fdb flush dev bridge static noextern_learn
> < flush all static entries which are not externally learned >
> $ bridge fdb flush dev bridge permanent
> < flush all permanent entries >

IIUC, the new IFLA_BRIDGE_FLUSH attribute is supposed to be passed in
RTM_SETLINK messages, but the current 'bridge fdb' commands all
correspond to RTM_{NEW,DEL,GET}NEIGH messages. To continue following
this pattern, did you consider turning the above examples to the
following?

$ ip link set dev bridge type bridge fdb_flush vlan 100 static
$ ip link set dev bridge type bridge fdb_flush vlan 1 dynamic
$ ip link set dev ens16 type bridge_slave fdb_flush vlan 1 dynamic
$ ip link set dev bridge type bridge fdb_flush nooffloaded nopermanent
$ ip link set dev bridge type bridge fdb_flush static noextern_learn
$ ip link set dev bridge type bridge fdb_flush permanent

It's not critical, but I like the correspondence between iproute2
commands and the underlying netlink messages.

> 
> Note that all flags have their negated version (static vs nostatic etc)
> and there are some tricky cases to handle like "static" which in flag
> terms means fdbs that have NUD_NOARP but *not* NUD_PERMANENT, so the
> mask matches on both but we need only NUD_NOARP to be set. That's
> because permanent entries have both set so we can't just match on
> NUD_NOARP. Also note that this flush operation doesn't treat permanent
> entries in a special way (fdb_delete vs fdb_delete_local), it will
> delete them regardless if any port is using them. We can extend the api
> with a flag to do that if needed in the future.
> 
> Patches in this set:
>  1. adds the new IFLA_BRIDGE_FLUSH bridge af attribute
>  2. adds a basic structure to describe an fdb flush filter
>  3. adds fdb netlink flush call via BRIDGE_FDB_FLUSH attribute
>  4 - 6. add support for specifying various fdb fields to filter
> 
> Patch-sets (in order):
>  - Initial flush infra and fdb flush filtering (this set)
>  - iproute2 support
>  - selftests
> 
> Future work:
>  - mdb flush support
> 
> Thanks,
>  Nik
> 
> Nikolay Aleksandrov (6):
>   net: bridge: add a generic flush operation
>   net: bridge: fdb: add support for fine-grained flushing
>   net: bridge: fdb: add new nl attribute-based flush call
>   net: bridge: fdb: add support for flush filtering based on ndm flags
>     and state
>   net: bridge: fdb: add support for flush filtering based on ifindex
>   net: bridge: fdb: add support for flush filtering based on vlan id
> 
>  include/uapi/linux/if_bridge.h |  22 ++++++
>  net/bridge/br_fdb.c            | 128 +++++++++++++++++++++++++++++++--
>  net/bridge/br_netlink.c        |  59 ++++++++++++++-
>  net/bridge/br_private.h        |  12 +++-
>  net/bridge/br_sysfs_br.c       |   6 +-
>  5 files changed, 215 insertions(+), 12 deletions(-)
> 
> -- 
> 2.35.1
> 

  parent reply	other threads:[~2022-04-11  7:47 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-09 10:58 [Bridge] [PATCH net-next 0/6] net: bridge: add flush filtering support Nikolay Aleksandrov
2022-04-09 10:58 ` Nikolay Aleksandrov
2022-04-09 10:58 ` [Bridge] [PATCH net-next 1/6] net: bridge: add a generic flush operation Nikolay Aleksandrov
2022-04-09 10:58   ` Nikolay Aleksandrov
2022-04-09 10:58 ` [Bridge] [PATCH net-next 2/6] net: bridge: fdb: add support for fine-grained flushing Nikolay Aleksandrov
2022-04-09 10:58   ` Nikolay Aleksandrov
2022-04-11  8:20   ` [Bridge] " Ido Schimmel
2022-04-11  8:20     ` Ido Schimmel
2022-04-11  8:54     ` [Bridge] " Nikolay Aleksandrov
2022-04-11  8:54       ` Nikolay Aleksandrov
2022-04-09 10:58 ` [Bridge] [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call Nikolay Aleksandrov
2022-04-09 10:58   ` Nikolay Aleksandrov
2022-04-11  8:33   ` [Bridge] " Ido Schimmel
2022-04-11  8:33     ` Ido Schimmel
2022-04-11  9:01     ` [Bridge] " Nikolay Aleksandrov
2022-04-11  9:01       ` Nikolay Aleksandrov
2022-04-11  8:41   ` [Bridge] " Ido Schimmel
2022-04-11  8:41     ` Ido Schimmel
2022-04-11  9:05     ` [Bridge] " Nikolay Aleksandrov
2022-04-11  9:05       ` Nikolay Aleksandrov
2022-04-09 10:58 ` [Bridge] [PATCH net-next 4/6] net: bridge: fdb: add support for flush filtering based on ndm flags and state Nikolay Aleksandrov
2022-04-09 10:58   ` Nikolay Aleksandrov
2022-04-11  8:47   ` [Bridge] " Ido Schimmel
2022-04-11  8:47     ` Ido Schimmel
2022-04-11  9:07     ` [Bridge] " Nikolay Aleksandrov
2022-04-11  9:07       ` Nikolay Aleksandrov
2022-04-09 10:58 ` [Bridge] [PATCH net-next 5/6] net: bridge: fdb: add support for flush filtering based on ifindex Nikolay Aleksandrov
2022-04-09 10:58   ` Nikolay Aleksandrov
2022-04-11  8:57   ` [Bridge] " Ido Schimmel
2022-04-11  8:57     ` Ido Schimmel
2022-04-11  9:03     ` [Bridge] " Nikolay Aleksandrov
2022-04-11  9:03       ` Nikolay Aleksandrov
2022-04-09 10:58 ` [Bridge] [PATCH net-next 6/6] net: bridge: fdb: add support for flush filtering based on vlan id Nikolay Aleksandrov
2022-04-09 10:58   ` Nikolay Aleksandrov
2022-04-09 12:36 ` [Bridge] [PATCH net-next 0/6] net: bridge: add flush filtering support Nikolay Aleksandrov
2022-04-09 12:36   ` Nikolay Aleksandrov
2022-04-10 20:43 ` [Bridge] " Nikolay Aleksandrov
2022-04-10 20:43   ` Nikolay Aleksandrov
2022-04-11  7:47 ` Ido Schimmel [this message]
2022-04-11  7:47   ` Ido Schimmel
2022-04-11  8:53   ` [Bridge] " Nikolay Aleksandrov
2022-04-11  8:53     ` Nikolay Aleksandrov
2022-04-11  8:54   ` [Bridge] " Ido Schimmel
2022-04-11  8:54     ` Ido Schimmel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YlPdFS//hYbBSAkT@shredder \
    --to=idosch@idosch.org \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=razor@blackwall.org \
    --cc=roopa@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.