[PATCH net-next 0/6] net: bridge: add flush filtering support

[PATCH net-next 0/6] net: bridge: add flush filtering support

[Nikolay Aleksandrov]

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 >

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

[Bridge] [PATCH net-next 0/6] net: bridge: add flush filtering support

[Nikolay Aleksandrov]

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 >

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

[PATCH net-next 1/6] net: bridge: add a generic flush operation

[Nikolay Aleksandrov]

Add a new bridge attribute (IFLA_BRIDGE_FLUSH) which will have embedded
attributes describing the object types that will be flushed. It will
allow fine-grained object flushing. Only a single flush attribute is
allowed per call since it can be a very load heavy operation. Also it is
allowed only with setlink command (similar to changelink flush). A nice
side-effect of using an af spec attribute is that it avoids making the
bridge link attribute options list longer.

An example structure for fdbs:
 [ IFLA_BRIDGE_FLUSH ]
  `[ BRIDGE_FDB_FLUSH ]
    `[ FDB_FLUSH_NDM_STATE ]
    `[ FDB_FLUSH_NDM_FLAGS ]

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h |  8 +++++++
 net/bridge/br_netlink.c        | 42 +++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index a86a7e7b811f..221a4256808f 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -123,6 +123,7 @@ enum {
 	IFLA_BRIDGE_MRP,
 	IFLA_BRIDGE_CFM,
 	IFLA_BRIDGE_MST,
+	IFLA_BRIDGE_FLUSH,
 	__IFLA_BRIDGE_MAX,
 };
 #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
@@ -802,4 +803,11 @@ enum {
 	__BRIDGE_QUERIER_MAX
 };
 #define BRIDGE_QUERIER_MAX (__BRIDGE_QUERIER_MAX - 1)
+
+/* embedded in IFLA_BRIDGE_FLUSH */
+enum {
+	BRIDGE_FLUSH_UNSPEC,
+	__BRIDGE_FLUSH_MAX
+};
+#define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
 #endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 200ad05b296f..fe2211d4c0c7 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -779,6 +779,34 @@ int br_process_vlan_info(struct net_bridge *br,
 	return err;
 }
 
+static const struct nla_policy br_flush_policy[BRIDGE_FLUSH_MAX + 1] = {
+	[BRIDGE_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
+};
+
+static int br_flush(struct net_bridge *br, int cmd,
+		    struct nlattr *flush_attr,
+		    struct netlink_ext_ack *extack)
+{
+	struct nlattr *flush_tb[BRIDGE_FLUSH_MAX + 1];
+	int err;
+
+	switch (cmd) {
+	case RTM_SETLINK:
+		break;
+	default:
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Bridge flush attribute is allowed only with RTM_SETLINK");
+		return -EINVAL;
+	}
+
+	err = nla_parse_nested(flush_tb, BRIDGE_FLUSH_MAX, flush_attr,
+			       br_flush_policy, extack);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static int br_afspec(struct net_bridge *br,
 		     struct net_bridge_port *p,
 		     struct nlattr *af_spec,
@@ -787,9 +815,10 @@ static int br_afspec(struct net_bridge *br,
 {
 	struct bridge_vlan_info *vinfo_curr = NULL;
 	struct bridge_vlan_info *vinfo_last = NULL;
-	struct nlattr *attr;
 	struct vtunnel_info tinfo_last = {};
 	struct vtunnel_info tinfo_curr = {};
+	bool flushed = false;
+	struct nlattr *attr;
 	int err = 0, rem;
 
 	nla_for_each_nested(attr, af_spec, rem) {
@@ -845,6 +874,17 @@ static int br_afspec(struct net_bridge *br,
 			if (err)
 				return err;
 			break;
+		case IFLA_BRIDGE_FLUSH:
+			if (flushed) {
+				NL_SET_ERR_MSG_MOD(extack,
+						   "Multiple bridge flush attributes are not allowed");
+				return -EINVAL;
+			}
+			err = br_flush(br, cmd, attr, extack);
+			if (err)
+				return err;
+			flushed = true;
+			break;
 		}
 	}
 
-- 
2.35.1

[Bridge] [PATCH net-next 1/6] net: bridge: add a generic flush operation

[Nikolay Aleksandrov]

Add a new bridge attribute (IFLA_BRIDGE_FLUSH) which will have embedded
attributes describing the object types that will be flushed. It will
allow fine-grained object flushing. Only a single flush attribute is
allowed per call since it can be a very load heavy operation. Also it is
allowed only with setlink command (similar to changelink flush). A nice
side-effect of using an af spec attribute is that it avoids making the
bridge link attribute options list longer.

An example structure for fdbs:
 [ IFLA_BRIDGE_FLUSH ]
  `[ BRIDGE_FDB_FLUSH ]
    `[ FDB_FLUSH_NDM_STATE ]
    `[ FDB_FLUSH_NDM_FLAGS ]

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h |  8 +++++++
 net/bridge/br_netlink.c        | 42 +++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index a86a7e7b811f..221a4256808f 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -123,6 +123,7 @@ enum {
 	IFLA_BRIDGE_MRP,
 	IFLA_BRIDGE_CFM,
 	IFLA_BRIDGE_MST,
+	IFLA_BRIDGE_FLUSH,
 	__IFLA_BRIDGE_MAX,
 };
 #define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
@@ -802,4 +803,11 @@ enum {
 	__BRIDGE_QUERIER_MAX
 };
 #define BRIDGE_QUERIER_MAX (__BRIDGE_QUERIER_MAX - 1)
+
+/* embedded in IFLA_BRIDGE_FLUSH */
+enum {
+	BRIDGE_FLUSH_UNSPEC,
+	__BRIDGE_FLUSH_MAX
+};
+#define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
 #endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 200ad05b296f..fe2211d4c0c7 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -779,6 +779,34 @@ int br_process_vlan_info(struct net_bridge *br,
 	return err;
 }
 
+static const struct nla_policy br_flush_policy[BRIDGE_FLUSH_MAX + 1] = {
+	[BRIDGE_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
+};
+
+static int br_flush(struct net_bridge *br, int cmd,
+		    struct nlattr *flush_attr,
+		    struct netlink_ext_ack *extack)
+{
+	struct nlattr *flush_tb[BRIDGE_FLUSH_MAX + 1];
+	int err;
+
+	switch (cmd) {
+	case RTM_SETLINK:
+		break;
+	default:
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Bridge flush attribute is allowed only with RTM_SETLINK");
+		return -EINVAL;
+	}
+
+	err = nla_parse_nested(flush_tb, BRIDGE_FLUSH_MAX, flush_attr,
+			       br_flush_policy, extack);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static int br_afspec(struct net_bridge *br,
 		     struct net_bridge_port *p,
 		     struct nlattr *af_spec,
@@ -787,9 +815,10 @@ static int br_afspec(struct net_bridge *br,
 {
 	struct bridge_vlan_info *vinfo_curr = NULL;
 	struct bridge_vlan_info *vinfo_last = NULL;
-	struct nlattr *attr;
 	struct vtunnel_info tinfo_last = {};
 	struct vtunnel_info tinfo_curr = {};
+	bool flushed = false;
+	struct nlattr *attr;
 	int err = 0, rem;
 
 	nla_for_each_nested(attr, af_spec, rem) {
@@ -845,6 +874,17 @@ static int br_afspec(struct net_bridge *br,
 			if (err)
 				return err;
 			break;
+		case IFLA_BRIDGE_FLUSH:
+			if (flushed) {
+				NL_SET_ERR_MSG_MOD(extack,
+						   "Multiple bridge flush attributes are not allowed");
+				return -EINVAL;
+			}
+			err = br_flush(br, cmd, attr, extack);
+			if (err)
+				return err;
+			flushed = true;
+			break;
 		}
 	}
 
-- 
2.35.1

[PATCH net-next 2/6] net: bridge: fdb: add support for fine-grained flushing

[Nikolay Aleksandrov]

Add the ability to specify exactly which fdbs to be flushed. They are
described by a new structure - net_bridge_fdb_flush_desc. Currently it
can match on port/bridge ifindex, vlan id and fdb flags. It is used to
describe the existing dynamic fdb flush operation.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_fdb.c      | 36 +++++++++++++++++++++++++++++-------
 net/bridge/br_netlink.c  |  9 +++++++--
 net/bridge/br_private.h  | 10 +++++++++-
 net/bridge/br_sysfs_br.c |  6 +++++-
 4 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 6ccda68bd473..4b0bf88c4121 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -558,18 +558,40 @@ void br_fdb_cleanup(struct work_struct *work)
 	mod_delayed_work(system_long_wq, &br->gc_work, work_delay);
 }
 
-/* Completely flush all dynamic entries in forwarding database.*/
-void br_fdb_flush(struct net_bridge *br)
+static bool __fdb_flush_matches(const struct net_bridge *br,
+				const struct net_bridge_fdb_entry *f,
+				const struct net_bridge_fdb_flush_desc *desc)
+{
+	const struct net_bridge_port *dst = READ_ONCE(f->dst);
+	int port_ifidx, br_ifidx = br->dev->ifindex;
+
+	port_ifidx = dst ? dst->dev->ifindex : 0;
+
+	return (!desc->vlan_id || desc->vlan_id == f->key.vlan_id) &&
+	       (!desc->port_ifindex ||
+		(desc->port_ifindex == port_ifidx ||
+		 (!dst && desc->port_ifindex == br_ifidx))) &&
+	       (!desc->flags_mask ||
+		((f->flags & desc->flags_mask) == desc->flags));
+}
+
+/* Flush forwarding database entries matching the description */
+void br_fdb_flush(struct net_bridge *br,
+		  const struct net_bridge_fdb_flush_desc *desc)
 {
 	struct net_bridge_fdb_entry *f;
-	struct hlist_node *tmp;
 
-	spin_lock_bh(&br->hash_lock);
-	hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
-		if (!test_bit(BR_FDB_STATIC, &f->flags))
+	rcu_read_lock();
+	hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
+		if (!__fdb_flush_matches(br, f, desc))
+			continue;
+
+		spin_lock_bh(&br->hash_lock);
+		if (!hlist_unhashed(&f->fdb_node))
 			fdb_delete(br, f, true);
+		spin_unlock_bh(&br->hash_lock);
 	}
-	spin_unlock_bh(&br->hash_lock);
+	rcu_read_unlock();
 }
 
 /* Flush all entries referring to a specific port.
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index fe2211d4c0c7..6e6dce6880c9 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1366,8 +1366,13 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
 		br_recalculate_fwd_mask(br);
 	}
 
-	if (data[IFLA_BR_FDB_FLUSH])
-		br_fdb_flush(br);
+	if (data[IFLA_BR_FDB_FLUSH]) {
+		struct net_bridge_fdb_flush_desc desc = {
+			.flags_mask = BR_FDB_STATIC
+		};
+
+		br_fdb_flush(br, &desc);
+	}
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	if (data[IFLA_BR_MCAST_ROUTER]) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6e62af2e07e9..e6930e9ee69d 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -274,6 +274,13 @@ struct net_bridge_fdb_entry {
 	struct rcu_head			rcu;
 };
 
+struct net_bridge_fdb_flush_desc {
+	unsigned long			flags;
+	unsigned long			flags_mask;
+	int				port_ifindex;
+	u16				vlan_id;
+};
+
 #define MDB_PG_FLAGS_PERMANENT	BIT(0)
 #define MDB_PG_FLAGS_OFFLOAD	BIT(1)
 #define MDB_PG_FLAGS_FAST_LEAVE	BIT(2)
@@ -759,7 +766,8 @@ int br_fdb_init(void);
 void br_fdb_fini(void);
 int br_fdb_hash_init(struct net_bridge *br);
 void br_fdb_hash_fini(struct net_bridge *br);
-void br_fdb_flush(struct net_bridge *br);
+void br_fdb_flush(struct net_bridge *br,
+		  const struct net_bridge_fdb_flush_desc *desc);
 void br_fdb_find_delete_local(struct net_bridge *br,
 			      const struct net_bridge_port *p,
 			      const unsigned char *addr, u16 vid);
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 3f7ca88c2aa3..612e367fff20 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -344,7 +344,11 @@ static DEVICE_ATTR_RW(group_addr);
 static int set_flush(struct net_bridge *br, unsigned long val,
 		     struct netlink_ext_ack *extack)
 {
-	br_fdb_flush(br);
+	struct net_bridge_fdb_flush_desc desc = {
+		.flags_mask = BR_FDB_STATIC
+	};
+
+	br_fdb_flush(br, &desc);
 	return 0;
 }
 
-- 
2.35.1

[Bridge] [PATCH net-next 2/6] net: bridge: fdb: add support for fine-grained flushing

[Nikolay Aleksandrov]

Add the ability to specify exactly which fdbs to be flushed. They are
described by a new structure - net_bridge_fdb_flush_desc. Currently it
can match on port/bridge ifindex, vlan id and fdb flags. It is used to
describe the existing dynamic fdb flush operation.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 net/bridge/br_fdb.c      | 36 +++++++++++++++++++++++++++++-------
 net/bridge/br_netlink.c  |  9 +++++++--
 net/bridge/br_private.h  | 10 +++++++++-
 net/bridge/br_sysfs_br.c |  6 +++++-
 4 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 6ccda68bd473..4b0bf88c4121 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -558,18 +558,40 @@ void br_fdb_cleanup(struct work_struct *work)
 	mod_delayed_work(system_long_wq, &br->gc_work, work_delay);
 }
 
-/* Completely flush all dynamic entries in forwarding database.*/
-void br_fdb_flush(struct net_bridge *br)
+static bool __fdb_flush_matches(const struct net_bridge *br,
+				const struct net_bridge_fdb_entry *f,
+				const struct net_bridge_fdb_flush_desc *desc)
+{
+	const struct net_bridge_port *dst = READ_ONCE(f->dst);
+	int port_ifidx, br_ifidx = br->dev->ifindex;
+
+	port_ifidx = dst ? dst->dev->ifindex : 0;
+
+	return (!desc->vlan_id || desc->vlan_id == f->key.vlan_id) &&
+	       (!desc->port_ifindex ||
+		(desc->port_ifindex == port_ifidx ||
+		 (!dst && desc->port_ifindex == br_ifidx))) &&
+	       (!desc->flags_mask ||
+		((f->flags & desc->flags_mask) == desc->flags));
+}
+
+/* Flush forwarding database entries matching the description */
+void br_fdb_flush(struct net_bridge *br,
+		  const struct net_bridge_fdb_flush_desc *desc)
 {
 	struct net_bridge_fdb_entry *f;
-	struct hlist_node *tmp;
 
-	spin_lock_bh(&br->hash_lock);
-	hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
-		if (!test_bit(BR_FDB_STATIC, &f->flags))
+	rcu_read_lock();
+	hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
+		if (!__fdb_flush_matches(br, f, desc))
+			continue;
+
+		spin_lock_bh(&br->hash_lock);
+		if (!hlist_unhashed(&f->fdb_node))
 			fdb_delete(br, f, true);
+		spin_unlock_bh(&br->hash_lock);
 	}
-	spin_unlock_bh(&br->hash_lock);
+	rcu_read_unlock();
 }
 
 /* Flush all entries referring to a specific port.
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index fe2211d4c0c7..6e6dce6880c9 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1366,8 +1366,13 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
 		br_recalculate_fwd_mask(br);
 	}
 
-	if (data[IFLA_BR_FDB_FLUSH])
-		br_fdb_flush(br);
+	if (data[IFLA_BR_FDB_FLUSH]) {
+		struct net_bridge_fdb_flush_desc desc = {
+			.flags_mask = BR_FDB_STATIC
+		};
+
+		br_fdb_flush(br, &desc);
+	}
 
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	if (data[IFLA_BR_MCAST_ROUTER]) {
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 6e62af2e07e9..e6930e9ee69d 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -274,6 +274,13 @@ struct net_bridge_fdb_entry {
 	struct rcu_head			rcu;
 };
 
+struct net_bridge_fdb_flush_desc {
+	unsigned long			flags;
+	unsigned long			flags_mask;
+	int				port_ifindex;
+	u16				vlan_id;
+};
+
 #define MDB_PG_FLAGS_PERMANENT	BIT(0)
 #define MDB_PG_FLAGS_OFFLOAD	BIT(1)
 #define MDB_PG_FLAGS_FAST_LEAVE	BIT(2)
@@ -759,7 +766,8 @@ int br_fdb_init(void);
 void br_fdb_fini(void);
 int br_fdb_hash_init(struct net_bridge *br);
 void br_fdb_hash_fini(struct net_bridge *br);
-void br_fdb_flush(struct net_bridge *br);
+void br_fdb_flush(struct net_bridge *br,
+		  const struct net_bridge_fdb_flush_desc *desc);
 void br_fdb_find_delete_local(struct net_bridge *br,
 			      const struct net_bridge_port *p,
 			      const unsigned char *addr, u16 vid);
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 3f7ca88c2aa3..612e367fff20 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -344,7 +344,11 @@ static DEVICE_ATTR_RW(group_addr);
 static int set_flush(struct net_bridge *br, unsigned long val,
 		     struct netlink_ext_ack *extack)
 {
-	br_fdb_flush(br);
+	struct net_bridge_fdb_flush_desc desc = {
+		.flags_mask = BR_FDB_STATIC
+	};
+
+	br_fdb_flush(br, &desc);
 	return 0;
 }
 
-- 
2.35.1

[PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Nikolay Aleksandrov]

Add a new fdb flush call which parses the embedded attributes in
BRIDGE_FLUSH_FDB and fills in the fdb flush descriptor to delete only
matching entries. Currently it's a complete flush, support for more
fine-grained filtering will be added in the following patches.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h |  8 ++++++++
 net/bridge/br_fdb.c            | 24 ++++++++++++++++++++++++
 net/bridge/br_netlink.c        |  8 ++++++++
 net/bridge/br_private.h        |  2 ++
 4 files changed, 42 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 221a4256808f..2f3799cf14b2 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -807,7 +807,15 @@ enum {
 /* embedded in IFLA_BRIDGE_FLUSH */
 enum {
 	BRIDGE_FLUSH_UNSPEC,
+	BRIDGE_FLUSH_FDB,
 	__BRIDGE_FLUSH_MAX
 };
 #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
+
+/* embedded in BRIDGE_FLUSH_FDB */
+enum {
+	FDB_FLUSH_UNSPEC,
+	__FDB_FLUSH_MAX
+};
+#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
 #endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 4b0bf88c4121..62f694a739e1 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -594,6 +594,30 @@ void br_fdb_flush(struct net_bridge *br,
 	rcu_read_unlock();
 }
 
+static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
+	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
+};
+
+int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
+			struct netlink_ext_ack *extack)
+{
+	struct nlattr *fdb_flush_tb[FDB_FLUSH_MAX + 1];
+	struct net_bridge_fdb_flush_desc desc = {};
+	int err;
+
+	err = nla_parse_nested(fdb_flush_tb, FDB_FLUSH_MAX, fdb_flush_attr,
+			       br_fdb_flush_policy, extack);
+	if (err)
+		return err;
+
+	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
+		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
+
+	br_fdb_flush(br, &desc);
+
+	return 0;
+}
+
 /* Flush all entries referring to a specific port.
  * if do_all is set also flush static entries
  * if vid is set delete all entries that match the vlan_id
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 6e6dce6880c9..bd2c91e5723d 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -781,6 +781,7 @@ int br_process_vlan_info(struct net_bridge *br,
 
 static const struct nla_policy br_flush_policy[BRIDGE_FLUSH_MAX + 1] = {
 	[BRIDGE_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
+	[BRIDGE_FLUSH_FDB]	= { .type = NLA_NESTED },
 };
 
 static int br_flush(struct net_bridge *br, int cmd,
@@ -804,6 +805,13 @@ static int br_flush(struct net_bridge *br, int cmd,
 	if (err)
 		return err;
 
+	if (flush_tb[BRIDGE_FLUSH_FDB]) {
+		err = br_fdb_flush_nlattr(br, flush_tb[BRIDGE_FLUSH_FDB],
+					  extack);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index e6930e9ee69d..c7ea531d30ef 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -768,6 +768,8 @@ int br_fdb_hash_init(struct net_bridge *br);
 void br_fdb_hash_fini(struct net_bridge *br);
 void br_fdb_flush(struct net_bridge *br,
 		  const struct net_bridge_fdb_flush_desc *desc);
+int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
+			struct netlink_ext_ack *extack);
 void br_fdb_find_delete_local(struct net_bridge *br,
 			      const struct net_bridge_port *p,
 			      const unsigned char *addr, u16 vid);
-- 
2.35.1

[Bridge] [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Nikolay Aleksandrov]

Add a new fdb flush call which parses the embedded attributes in
BRIDGE_FLUSH_FDB and fills in the fdb flush descriptor to delete only
matching entries. Currently it's a complete flush, support for more
fine-grained filtering will be added in the following patches.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h |  8 ++++++++
 net/bridge/br_fdb.c            | 24 ++++++++++++++++++++++++
 net/bridge/br_netlink.c        |  8 ++++++++
 net/bridge/br_private.h        |  2 ++
 4 files changed, 42 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 221a4256808f..2f3799cf14b2 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -807,7 +807,15 @@ enum {
 /* embedded in IFLA_BRIDGE_FLUSH */
 enum {
 	BRIDGE_FLUSH_UNSPEC,
+	BRIDGE_FLUSH_FDB,
 	__BRIDGE_FLUSH_MAX
 };
 #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
+
+/* embedded in BRIDGE_FLUSH_FDB */
+enum {
+	FDB_FLUSH_UNSPEC,
+	__FDB_FLUSH_MAX
+};
+#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
 #endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 4b0bf88c4121..62f694a739e1 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -594,6 +594,30 @@ void br_fdb_flush(struct net_bridge *br,
 	rcu_read_unlock();
 }
 
+static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
+	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
+};
+
+int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
+			struct netlink_ext_ack *extack)
+{
+	struct nlattr *fdb_flush_tb[FDB_FLUSH_MAX + 1];
+	struct net_bridge_fdb_flush_desc desc = {};
+	int err;
+
+	err = nla_parse_nested(fdb_flush_tb, FDB_FLUSH_MAX, fdb_flush_attr,
+			       br_fdb_flush_policy, extack);
+	if (err)
+		return err;
+
+	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
+		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
+
+	br_fdb_flush(br, &desc);
+
+	return 0;
+}
+
 /* Flush all entries referring to a specific port.
  * if do_all is set also flush static entries
  * if vid is set delete all entries that match the vlan_id
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 6e6dce6880c9..bd2c91e5723d 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -781,6 +781,7 @@ int br_process_vlan_info(struct net_bridge *br,
 
 static const struct nla_policy br_flush_policy[BRIDGE_FLUSH_MAX + 1] = {
 	[BRIDGE_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
+	[BRIDGE_FLUSH_FDB]	= { .type = NLA_NESTED },
 };
 
 static int br_flush(struct net_bridge *br, int cmd,
@@ -804,6 +805,13 @@ static int br_flush(struct net_bridge *br, int cmd,
 	if (err)
 		return err;
 
+	if (flush_tb[BRIDGE_FLUSH_FDB]) {
+		err = br_fdb_flush_nlattr(br, flush_tb[BRIDGE_FLUSH_FDB],
+					  extack);
+		if (err)
+			return err;
+	}
+
 	return 0;
 }
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index e6930e9ee69d..c7ea531d30ef 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -768,6 +768,8 @@ int br_fdb_hash_init(struct net_bridge *br);
 void br_fdb_hash_fini(struct net_bridge *br);
 void br_fdb_flush(struct net_bridge *br,
 		  const struct net_bridge_fdb_flush_desc *desc);
+int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
+			struct netlink_ext_ack *extack);
 void br_fdb_find_delete_local(struct net_bridge *br,
 			      const struct net_bridge_port *p,
 			      const unsigned char *addr, u16 vid);
-- 
2.35.1

[PATCH net-next 4/6] net: bridge: fdb: add support for flush filtering based on ndm flags and state

[Nikolay Aleksandrov]

Add support for fdb flush filtering based on ndm flags and state. The
new attributes allow users to specify a mask and value which are mapped
to bridge-specific flags. NTF_USE is used to represent added_by_user
flag since it sets it on fdb add and we don't have a 1:1 mapping for it.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h |  4 +++
 net/bridge/br_fdb.c            | 55 ++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 2f3799cf14b2..4638d7e39f2a 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -815,6 +815,10 @@ enum {
 /* embedded in BRIDGE_FLUSH_FDB */
 enum {
 	FDB_FLUSH_UNSPEC,
+	FDB_FLUSH_NDM_STATE,
+	FDB_FLUSH_NDM_STATE_MASK,
+	FDB_FLUSH_NDM_FLAGS,
+	FDB_FLUSH_NDM_FLAGS_MASK,
 	__FDB_FLUSH_MAX
 };
 #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 62f694a739e1..340a2ace1d5e 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -594,8 +594,40 @@ void br_fdb_flush(struct net_bridge *br,
 	rcu_read_unlock();
 }
 
+static unsigned long __ndm_state_to_fdb_flags(u16 ndm_state)
+{
+	unsigned long flags = 0;
+
+	if (ndm_state & NUD_PERMANENT)
+		__set_bit(BR_FDB_LOCAL, &flags);
+	if (ndm_state & NUD_NOARP)
+		__set_bit(BR_FDB_STATIC, &flags);
+
+	return flags;
+}
+
+static unsigned long __ndm_flags_to_fdb_flags(u16 ndm_flags)
+{
+	unsigned long flags = 0;
+
+	if (ndm_flags & NTF_USE)
+		__set_bit(BR_FDB_ADDED_BY_USER, &flags);
+	if (ndm_flags & NTF_EXT_LEARNED)
+		__set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &flags);
+	if (ndm_flags & NTF_OFFLOADED)
+		__set_bit(BR_FDB_OFFLOADED, &flags);
+	if (ndm_flags & NTF_STICKY)
+		__set_bit(BR_FDB_STICKY, &flags);
+
+	return flags;
+}
+
 static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
 	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
+	[FDB_FLUSH_NDM_STATE]	= { .type = NLA_U16 },
+	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
+	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
+	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
 };
 
 int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
@@ -610,6 +642,29 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
 	if (err)
 		return err;
 
+	if (fdb_flush_tb[FDB_FLUSH_NDM_STATE]) {
+		u16 ndm_state = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_STATE]);
+
+		desc.flags |= __ndm_state_to_fdb_flags(ndm_state);
+	}
+	if (fdb_flush_tb[FDB_FLUSH_NDM_STATE_MASK]) {
+		u16 ndm_state_mask;
+
+		ndm_state_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_STATE_MASK]);
+		desc.flags_mask |= __ndm_state_to_fdb_flags(ndm_state_mask);
+	}
+	if (fdb_flush_tb[FDB_FLUSH_NDM_FLAGS]) {
+		u16 ndm_flags = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS]);
+
+		desc.flags |= __ndm_flags_to_fdb_flags(ndm_flags);
+	}
+	if (fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]) {
+		u16 ndm_flags_mask;
+
+		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
+		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
+	}
+
 	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
 		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
 
-- 
2.35.1

[Bridge] [PATCH net-next 4/6] net: bridge: fdb: add support for flush filtering based on ndm flags and state

[Nikolay Aleksandrov]

Add support for fdb flush filtering based on ndm flags and state. The
new attributes allow users to specify a mask and value which are mapped
to bridge-specific flags. NTF_USE is used to represent added_by_user
flag since it sets it on fdb add and we don't have a 1:1 mapping for it.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h |  4 +++
 net/bridge/br_fdb.c            | 55 ++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 2f3799cf14b2..4638d7e39f2a 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -815,6 +815,10 @@ enum {
 /* embedded in BRIDGE_FLUSH_FDB */
 enum {
 	FDB_FLUSH_UNSPEC,
+	FDB_FLUSH_NDM_STATE,
+	FDB_FLUSH_NDM_STATE_MASK,
+	FDB_FLUSH_NDM_FLAGS,
+	FDB_FLUSH_NDM_FLAGS_MASK,
 	__FDB_FLUSH_MAX
 };
 #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 62f694a739e1..340a2ace1d5e 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -594,8 +594,40 @@ void br_fdb_flush(struct net_bridge *br,
 	rcu_read_unlock();
 }
 
+static unsigned long __ndm_state_to_fdb_flags(u16 ndm_state)
+{
+	unsigned long flags = 0;
+
+	if (ndm_state & NUD_PERMANENT)
+		__set_bit(BR_FDB_LOCAL, &flags);
+	if (ndm_state & NUD_NOARP)
+		__set_bit(BR_FDB_STATIC, &flags);
+
+	return flags;
+}
+
+static unsigned long __ndm_flags_to_fdb_flags(u16 ndm_flags)
+{
+	unsigned long flags = 0;
+
+	if (ndm_flags & NTF_USE)
+		__set_bit(BR_FDB_ADDED_BY_USER, &flags);
+	if (ndm_flags & NTF_EXT_LEARNED)
+		__set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &flags);
+	if (ndm_flags & NTF_OFFLOADED)
+		__set_bit(BR_FDB_OFFLOADED, &flags);
+	if (ndm_flags & NTF_STICKY)
+		__set_bit(BR_FDB_STICKY, &flags);
+
+	return flags;
+}
+
 static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
 	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
+	[FDB_FLUSH_NDM_STATE]	= { .type = NLA_U16 },
+	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
+	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
+	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
 };
 
 int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
@@ -610,6 +642,29 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
 	if (err)
 		return err;
 
+	if (fdb_flush_tb[FDB_FLUSH_NDM_STATE]) {
+		u16 ndm_state = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_STATE]);
+
+		desc.flags |= __ndm_state_to_fdb_flags(ndm_state);
+	}
+	if (fdb_flush_tb[FDB_FLUSH_NDM_STATE_MASK]) {
+		u16 ndm_state_mask;
+
+		ndm_state_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_STATE_MASK]);
+		desc.flags_mask |= __ndm_state_to_fdb_flags(ndm_state_mask);
+	}
+	if (fdb_flush_tb[FDB_FLUSH_NDM_FLAGS]) {
+		u16 ndm_flags = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS]);
+
+		desc.flags |= __ndm_flags_to_fdb_flags(ndm_flags);
+	}
+	if (fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]) {
+		u16 ndm_flags_mask;
+
+		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
+		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
+	}
+
 	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
 		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
 
-- 
2.35.1

[PATCH net-next 5/6] net: bridge: fdb: add support for flush filtering based on ifindex

[Nikolay Aleksandrov]

Add support for fdb flush filtering based on destination ifindex. The
ifindex must either match a port's device ifindex or the bridge's.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h | 1 +
 net/bridge/br_fdb.c            | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 4638d7e39f2a..67ee12586844 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -819,6 +819,7 @@ enum {
 	FDB_FLUSH_NDM_STATE_MASK,
 	FDB_FLUSH_NDM_FLAGS,
 	FDB_FLUSH_NDM_FLAGS_MASK,
+	FDB_FLUSH_PORT_IFINDEX,
 	__FDB_FLUSH_MAX
 };
 #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 340a2ace1d5e..53208adf7474 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -628,6 +628,7 @@ static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
 	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
+	[FDB_FLUSH_PORT_IFINDEX]	= { .type = NLA_S32 },
 };
 
 int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
@@ -664,6 +665,12 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
 		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
 		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
 	}
+	if (fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]) {
+		int port_ifidx;
+
+		port_ifidx = nla_get_u32(fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]);
+		desc.port_ifindex = port_ifidx;
+	}
 
 	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
 		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
-- 
2.35.1

[Bridge] [PATCH net-next 5/6] net: bridge: fdb: add support for flush filtering based on ifindex

[Nikolay Aleksandrov]

Add support for fdb flush filtering based on destination ifindex. The
ifindex must either match a port's device ifindex or the bridge's.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h | 1 +
 net/bridge/br_fdb.c            | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 4638d7e39f2a..67ee12586844 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -819,6 +819,7 @@ enum {
 	FDB_FLUSH_NDM_STATE_MASK,
 	FDB_FLUSH_NDM_FLAGS,
 	FDB_FLUSH_NDM_FLAGS_MASK,
+	FDB_FLUSH_PORT_IFINDEX,
 	__FDB_FLUSH_MAX
 };
 #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 340a2ace1d5e..53208adf7474 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -628,6 +628,7 @@ static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
 	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
+	[FDB_FLUSH_PORT_IFINDEX]	= { .type = NLA_S32 },
 };
 
 int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
@@ -664,6 +665,12 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
 		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
 		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
 	}
+	if (fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]) {
+		int port_ifidx;
+
+		port_ifidx = nla_get_u32(fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]);
+		desc.port_ifindex = port_ifidx;
+	}
 
 	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
 		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
-- 
2.35.1

[PATCH net-next 6/6] net: bridge: fdb: add support for flush filtering based on vlan id

[Nikolay Aleksandrov]

Add support for fdb flush filtering based on vlan id.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h | 1 +
 net/bridge/br_fdb.c            | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 67ee12586844..7f6730812916 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -820,6 +820,7 @@ enum {
 	FDB_FLUSH_NDM_FLAGS,
 	FDB_FLUSH_NDM_FLAGS_MASK,
 	FDB_FLUSH_PORT_IFINDEX,
+	FDB_FLUSH_VLAN_ID,
 	__FDB_FLUSH_MAX
 };
 #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 53208adf7474..bc8b5cbde8ed 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -626,6 +626,7 @@ static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
 	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
 	[FDB_FLUSH_NDM_STATE]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
+	[FDB_FLUSH_VLAN_ID]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
 	[FDB_FLUSH_PORT_IFINDEX]	= { .type = NLA_S32 },
@@ -671,6 +672,11 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
 		port_ifidx = nla_get_u32(fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]);
 		desc.port_ifindex = port_ifidx;
 	}
+	if (fdb_flush_tb[FDB_FLUSH_VLAN_ID]) {
+		desc.vlan_id = nla_get_u16(fdb_flush_tb[FDB_FLUSH_VLAN_ID]);
+		if (!br_vlan_valid_id(desc.vlan_id, extack))
+			return -EINVAL;
+	}
 
 	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
 		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
-- 
2.35.1

[Bridge] [PATCH net-next 6/6] net: bridge: fdb: add support for flush filtering based on vlan id

[Nikolay Aleksandrov]

Add support for fdb flush filtering based on vlan id.

Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 include/uapi/linux/if_bridge.h | 1 +
 net/bridge/br_fdb.c            | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 67ee12586844..7f6730812916 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -820,6 +820,7 @@ enum {
 	FDB_FLUSH_NDM_FLAGS,
 	FDB_FLUSH_NDM_FLAGS_MASK,
 	FDB_FLUSH_PORT_IFINDEX,
+	FDB_FLUSH_VLAN_ID,
 	__FDB_FLUSH_MAX
 };
 #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 53208adf7474..bc8b5cbde8ed 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -626,6 +626,7 @@ static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
 	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
 	[FDB_FLUSH_NDM_STATE]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
+	[FDB_FLUSH_VLAN_ID]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
 	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
 	[FDB_FLUSH_PORT_IFINDEX]	= { .type = NLA_S32 },
@@ -671,6 +672,11 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
 		port_ifidx = nla_get_u32(fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]);
 		desc.port_ifindex = port_ifidx;
 	}
+	if (fdb_flush_tb[FDB_FLUSH_VLAN_ID]) {
+		desc.vlan_id = nla_get_u16(fdb_flush_tb[FDB_FLUSH_VLAN_ID]);
+		if (!br_vlan_valid_id(desc.vlan_id, extack))
+			return -EINVAL;
+	}
 
 	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
 		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
-- 
2.35.1

Re: [PATCH net-next 0/6] net: bridge: add flush filtering support

[Nikolay Aleksandrov]

On 9 April 2022 13:58:51 EEST, Nikolay Aleksandrov <razor@blackwall.org> 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 >
>
>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(-)
>

Actually if you prefer I can send the selftests with this set, I'm used to sending them last
after the iproute2 support is finalised. :)

Cheers,
  Nik

Re: [Bridge] [PATCH net-next 0/6] net: bridge: add flush filtering support

[Nikolay Aleksandrov]

On 9 April 2022 13:58:51 EEST, Nikolay Aleksandrov <razor@blackwall.org> 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 >
>
>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(-)
>

Actually if you prefer I can send the selftests with this set, I'm used to sending them last
after the iproute2 support is finalised. :)

Cheers,
  Nik

Re: [PATCH net-next 0/6] net: bridge: add flush filtering support

[Nikolay Aleksandrov]

On 09/04/2022 13:58, 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 ]
> 
[snip]
> 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(-)
> 

Just FYI I plan to send v2 tomorrow with a few cleanups suggested by Ido.
Please don't apply this one, I'll wait for more feedback and will resubmit.

Thanks,
 Nik

Re: [Bridge] [PATCH net-next 0/6] net: bridge: add flush filtering support

[Nikolay Aleksandrov]

On 09/04/2022 13:58, 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 ]
> 
[snip]
> 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(-)
> 

Just FYI I plan to send v2 tomorrow with a few cleanups suggested by Ido.
Please don't apply this one, I'll wait for more feedback and will resubmit.

Thanks,
 Nik

Re: [PATCH net-next 0/6] net: bridge: add flush filtering support

[Ido Schimmel]

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
> 

Re: [Bridge] [PATCH net-next 0/6] net: bridge: add flush filtering support

[Ido Schimmel]

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
> 

Re: [PATCH net-next 2/6] net: bridge: fdb: add support for fine-grained flushing

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:53PM +0300, Nikolay Aleksandrov wrote:
> Add the ability to specify exactly which fdbs to be flushed. They are
> described by a new structure - net_bridge_fdb_flush_desc. Currently it
> can match on port/bridge ifindex, vlan id and fdb flags. It is used to
> describe the existing dynamic fdb flush operation.
> 
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  net/bridge/br_fdb.c      | 36 +++++++++++++++++++++++++++++-------
>  net/bridge/br_netlink.c  |  9 +++++++--
>  net/bridge/br_private.h  | 10 +++++++++-
>  net/bridge/br_sysfs_br.c |  6 +++++-
>  4 files changed, 50 insertions(+), 11 deletions(-)
> 
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 6ccda68bd473..4b0bf88c4121 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -558,18 +558,40 @@ void br_fdb_cleanup(struct work_struct *work)
>  	mod_delayed_work(system_long_wq, &br->gc_work, work_delay);
>  }
>  
> -/* Completely flush all dynamic entries in forwarding database.*/
> -void br_fdb_flush(struct net_bridge *br)
> +static bool __fdb_flush_matches(const struct net_bridge *br,
> +				const struct net_bridge_fdb_entry *f,
> +				const struct net_bridge_fdb_flush_desc *desc)
> +{
> +	const struct net_bridge_port *dst = READ_ONCE(f->dst);
> +	int port_ifidx, br_ifidx = br->dev->ifindex;
> +
> +	port_ifidx = dst ? dst->dev->ifindex : 0;
> +
> +	return (!desc->vlan_id || desc->vlan_id == f->key.vlan_id) &&
> +	       (!desc->port_ifindex ||
> +		(desc->port_ifindex == port_ifidx ||
> +		 (!dst && desc->port_ifindex == br_ifidx))) &&
> +	       (!desc->flags_mask ||
> +		((f->flags & desc->flags_mask) == desc->flags));

I find this easier to read:

port_ifidx = dst ? dst->dev->ifindex : br_ifidx;

if (desc->vlan_id && desc->vlan_id != f->key.vlan_id)
	return false;
if (desc->port_ifindex && desc->port_ifindex != port_ifidx)
	return false;
if (desc->flags_mask && (f->flags & desc->flags_mask) != desc->flags)
	return false;

return true;

> +}
> +
> +/* Flush forwarding database entries matching the description */
> +void br_fdb_flush(struct net_bridge *br,
> +		  const struct net_bridge_fdb_flush_desc *desc)
>  {
>  	struct net_bridge_fdb_entry *f;
> -	struct hlist_node *tmp;
>  
> -	spin_lock_bh(&br->hash_lock);
> -	hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
> -		if (!test_bit(BR_FDB_STATIC, &f->flags))
> +	rcu_read_lock();
> +	hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
> +		if (!__fdb_flush_matches(br, f, desc))
> +			continue;
> +
> +		spin_lock_bh(&br->hash_lock);
> +		if (!hlist_unhashed(&f->fdb_node))
>  			fdb_delete(br, f, true);
> +		spin_unlock_bh(&br->hash_lock);
>  	}
> -	spin_unlock_bh(&br->hash_lock);
> +	rcu_read_unlock();
>  }
>  
>  /* Flush all entries referring to a specific port.
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index fe2211d4c0c7..6e6dce6880c9 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -1366,8 +1366,13 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
>  		br_recalculate_fwd_mask(br);
>  	}
>  
> -	if (data[IFLA_BR_FDB_FLUSH])
> -		br_fdb_flush(br);
> +	if (data[IFLA_BR_FDB_FLUSH]) {
> +		struct net_bridge_fdb_flush_desc desc = {
> +			.flags_mask = BR_FDB_STATIC
> +		};
> +
> +		br_fdb_flush(br, &desc);

I wanted to ask why you are not doing the same for IFLA_BRPORT_FLUSH,
but then I read the implementation of br_fdb_delete_by_port() and
remembered the comment in the cover letter regarding fdb_delete vs
fdb_delete_local. Probably best to note it in the commit message

> +	}
>  
>  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
>  	if (data[IFLA_BR_MCAST_ROUTER]) {
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 6e62af2e07e9..e6930e9ee69d 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -274,6 +274,13 @@ struct net_bridge_fdb_entry {
>  	struct rcu_head			rcu;
>  };
>  
> +struct net_bridge_fdb_flush_desc {
> +	unsigned long			flags;
> +	unsigned long			flags_mask;
> +	int				port_ifindex;
> +	u16				vlan_id;
> +};
> +
>  #define MDB_PG_FLAGS_PERMANENT	BIT(0)
>  #define MDB_PG_FLAGS_OFFLOAD	BIT(1)
>  #define MDB_PG_FLAGS_FAST_LEAVE	BIT(2)
> @@ -759,7 +766,8 @@ int br_fdb_init(void);
>  void br_fdb_fini(void);
>  int br_fdb_hash_init(struct net_bridge *br);
>  void br_fdb_hash_fini(struct net_bridge *br);
> -void br_fdb_flush(struct net_bridge *br);
> +void br_fdb_flush(struct net_bridge *br,
> +		  const struct net_bridge_fdb_flush_desc *desc);
>  void br_fdb_find_delete_local(struct net_bridge *br,
>  			      const struct net_bridge_port *p,
>  			      const unsigned char *addr, u16 vid);
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index 3f7ca88c2aa3..612e367fff20 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -344,7 +344,11 @@ static DEVICE_ATTR_RW(group_addr);
>  static int set_flush(struct net_bridge *br, unsigned long val,
>  		     struct netlink_ext_ack *extack)
>  {
> -	br_fdb_flush(br);
> +	struct net_bridge_fdb_flush_desc desc = {
> +		.flags_mask = BR_FDB_STATIC
> +	};
> +
> +	br_fdb_flush(br, &desc);
>  	return 0;
>  }
>  
> -- 
> 2.35.1
> 

Re: [Bridge] [PATCH net-next 2/6] net: bridge: fdb: add support for fine-grained flushing

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:53PM +0300, Nikolay Aleksandrov wrote:
> Add the ability to specify exactly which fdbs to be flushed. They are
> described by a new structure - net_bridge_fdb_flush_desc. Currently it
> can match on port/bridge ifindex, vlan id and fdb flags. It is used to
> describe the existing dynamic fdb flush operation.
> 
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  net/bridge/br_fdb.c      | 36 +++++++++++++++++++++++++++++-------
>  net/bridge/br_netlink.c  |  9 +++++++--
>  net/bridge/br_private.h  | 10 +++++++++-
>  net/bridge/br_sysfs_br.c |  6 +++++-
>  4 files changed, 50 insertions(+), 11 deletions(-)
> 
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 6ccda68bd473..4b0bf88c4121 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -558,18 +558,40 @@ void br_fdb_cleanup(struct work_struct *work)
>  	mod_delayed_work(system_long_wq, &br->gc_work, work_delay);
>  }
>  
> -/* Completely flush all dynamic entries in forwarding database.*/
> -void br_fdb_flush(struct net_bridge *br)
> +static bool __fdb_flush_matches(const struct net_bridge *br,
> +				const struct net_bridge_fdb_entry *f,
> +				const struct net_bridge_fdb_flush_desc *desc)
> +{
> +	const struct net_bridge_port *dst = READ_ONCE(f->dst);
> +	int port_ifidx, br_ifidx = br->dev->ifindex;
> +
> +	port_ifidx = dst ? dst->dev->ifindex : 0;
> +
> +	return (!desc->vlan_id || desc->vlan_id == f->key.vlan_id) &&
> +	       (!desc->port_ifindex ||
> +		(desc->port_ifindex == port_ifidx ||
> +		 (!dst && desc->port_ifindex == br_ifidx))) &&
> +	       (!desc->flags_mask ||
> +		((f->flags & desc->flags_mask) == desc->flags));

I find this easier to read:

port_ifidx = dst ? dst->dev->ifindex : br_ifidx;

if (desc->vlan_id && desc->vlan_id != f->key.vlan_id)
	return false;
if (desc->port_ifindex && desc->port_ifindex != port_ifidx)
	return false;
if (desc->flags_mask && (f->flags & desc->flags_mask) != desc->flags)
	return false;

return true;

> +}
> +
> +/* Flush forwarding database entries matching the description */
> +void br_fdb_flush(struct net_bridge *br,
> +		  const struct net_bridge_fdb_flush_desc *desc)
>  {
>  	struct net_bridge_fdb_entry *f;
> -	struct hlist_node *tmp;
>  
> -	spin_lock_bh(&br->hash_lock);
> -	hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
> -		if (!test_bit(BR_FDB_STATIC, &f->flags))
> +	rcu_read_lock();
> +	hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
> +		if (!__fdb_flush_matches(br, f, desc))
> +			continue;
> +
> +		spin_lock_bh(&br->hash_lock);
> +		if (!hlist_unhashed(&f->fdb_node))
>  			fdb_delete(br, f, true);
> +		spin_unlock_bh(&br->hash_lock);
>  	}
> -	spin_unlock_bh(&br->hash_lock);
> +	rcu_read_unlock();
>  }
>  
>  /* Flush all entries referring to a specific port.
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index fe2211d4c0c7..6e6dce6880c9 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -1366,8 +1366,13 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
>  		br_recalculate_fwd_mask(br);
>  	}
>  
> -	if (data[IFLA_BR_FDB_FLUSH])
> -		br_fdb_flush(br);
> +	if (data[IFLA_BR_FDB_FLUSH]) {
> +		struct net_bridge_fdb_flush_desc desc = {
> +			.flags_mask = BR_FDB_STATIC
> +		};
> +
> +		br_fdb_flush(br, &desc);

I wanted to ask why you are not doing the same for IFLA_BRPORT_FLUSH,
but then I read the implementation of br_fdb_delete_by_port() and
remembered the comment in the cover letter regarding fdb_delete vs
fdb_delete_local. Probably best to note it in the commit message

> +	}
>  
>  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
>  	if (data[IFLA_BR_MCAST_ROUTER]) {
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 6e62af2e07e9..e6930e9ee69d 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -274,6 +274,13 @@ struct net_bridge_fdb_entry {
>  	struct rcu_head			rcu;
>  };
>  
> +struct net_bridge_fdb_flush_desc {
> +	unsigned long			flags;
> +	unsigned long			flags_mask;
> +	int				port_ifindex;
> +	u16				vlan_id;
> +};
> +
>  #define MDB_PG_FLAGS_PERMANENT	BIT(0)
>  #define MDB_PG_FLAGS_OFFLOAD	BIT(1)
>  #define MDB_PG_FLAGS_FAST_LEAVE	BIT(2)
> @@ -759,7 +766,8 @@ int br_fdb_init(void);
>  void br_fdb_fini(void);
>  int br_fdb_hash_init(struct net_bridge *br);
>  void br_fdb_hash_fini(struct net_bridge *br);
> -void br_fdb_flush(struct net_bridge *br);
> +void br_fdb_flush(struct net_bridge *br,
> +		  const struct net_bridge_fdb_flush_desc *desc);
>  void br_fdb_find_delete_local(struct net_bridge *br,
>  			      const struct net_bridge_port *p,
>  			      const unsigned char *addr, u16 vid);
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index 3f7ca88c2aa3..612e367fff20 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -344,7 +344,11 @@ static DEVICE_ATTR_RW(group_addr);
>  static int set_flush(struct net_bridge *br, unsigned long val,
>  		     struct netlink_ext_ack *extack)
>  {
> -	br_fdb_flush(br);
> +	struct net_bridge_fdb_flush_desc desc = {
> +		.flags_mask = BR_FDB_STATIC
> +	};
> +
> +	br_fdb_flush(br, &desc);
>  	return 0;
>  }
>  
> -- 
> 2.35.1
> 

Re: [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:54PM +0300, Nikolay Aleksandrov wrote:
> Add a new fdb flush call which parses the embedded attributes in
> BRIDGE_FLUSH_FDB and fills in the fdb flush descriptor to delete only
> matching entries. Currently it's a complete flush, support for more
> fine-grained filtering will be added in the following patches.
> 
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  include/uapi/linux/if_bridge.h |  8 ++++++++
>  net/bridge/br_fdb.c            | 24 ++++++++++++++++++++++++
>  net/bridge/br_netlink.c        |  8 ++++++++
>  net/bridge/br_private.h        |  2 ++
>  4 files changed, 42 insertions(+)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 221a4256808f..2f3799cf14b2 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -807,7 +807,15 @@ enum {
>  /* embedded in IFLA_BRIDGE_FLUSH */
>  enum {
>  	BRIDGE_FLUSH_UNSPEC,
> +	BRIDGE_FLUSH_FDB,
>  	__BRIDGE_FLUSH_MAX
>  };
>  #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
> +
> +/* embedded in BRIDGE_FLUSH_FDB */
> +enum {
> +	FDB_FLUSH_UNSPEC,
> +	__FDB_FLUSH_MAX
> +};
> +#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 4b0bf88c4121..62f694a739e1 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -594,6 +594,30 @@ void br_fdb_flush(struct net_bridge *br,
>  	rcu_read_unlock();
>  }
>  
> +static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
> +	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
> +};
> +
> +int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
> +			struct netlink_ext_ack *extack)
> +{
> +	struct nlattr *fdb_flush_tb[FDB_FLUSH_MAX + 1];
> +	struct net_bridge_fdb_flush_desc desc = {};
> +	int err;
> +
> +	err = nla_parse_nested(fdb_flush_tb, FDB_FLUSH_MAX, fdb_flush_attr,
> +			       br_fdb_flush_policy, extack);
> +	if (err)
> +		return err;
> +
> +	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
> +		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
> +
> +	br_fdb_flush(br, &desc);
> +
> +	return 0;
> +}
> +
>  /* Flush all entries referring to a specific port.
>   * if do_all is set also flush static entries
>   * if vid is set delete all entries that match the vlan_id
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 6e6dce6880c9..bd2c91e5723d 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -781,6 +781,7 @@ int br_process_vlan_info(struct net_bridge *br,
>  
>  static const struct nla_policy br_flush_policy[BRIDGE_FLUSH_MAX + 1] = {
>  	[BRIDGE_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
> +	[BRIDGE_FLUSH_FDB]	= { .type = NLA_NESTED },

In a previous submission [1] Jakub suggested using NLA_POLICY_NESTED()

[1] https://lore.kernel.org/netdev/20220224221447.6c7fa95d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/

>  };
>  
>  static int br_flush(struct net_bridge *br, int cmd,
> @@ -804,6 +805,13 @@ static int br_flush(struct net_bridge *br, int cmd,
>  	if (err)
>  		return err;
>  
> +	if (flush_tb[BRIDGE_FLUSH_FDB]) {
> +		err = br_fdb_flush_nlattr(br, flush_tb[BRIDGE_FLUSH_FDB],
> +					  extack);
> +		if (err)
> +			return err;
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index e6930e9ee69d..c7ea531d30ef 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -768,6 +768,8 @@ int br_fdb_hash_init(struct net_bridge *br);
>  void br_fdb_hash_fini(struct net_bridge *br);
>  void br_fdb_flush(struct net_bridge *br,
>  		  const struct net_bridge_fdb_flush_desc *desc);
> +int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
> +			struct netlink_ext_ack *extack);
>  void br_fdb_find_delete_local(struct net_bridge *br,
>  			      const struct net_bridge_port *p,
>  			      const unsigned char *addr, u16 vid);
> -- 
> 2.35.1
> 

Re: [Bridge] [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:54PM +0300, Nikolay Aleksandrov wrote:
> Add a new fdb flush call which parses the embedded attributes in
> BRIDGE_FLUSH_FDB and fills in the fdb flush descriptor to delete only
> matching entries. Currently it's a complete flush, support for more
> fine-grained filtering will be added in the following patches.
> 
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  include/uapi/linux/if_bridge.h |  8 ++++++++
>  net/bridge/br_fdb.c            | 24 ++++++++++++++++++++++++
>  net/bridge/br_netlink.c        |  8 ++++++++
>  net/bridge/br_private.h        |  2 ++
>  4 files changed, 42 insertions(+)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 221a4256808f..2f3799cf14b2 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -807,7 +807,15 @@ enum {
>  /* embedded in IFLA_BRIDGE_FLUSH */
>  enum {
>  	BRIDGE_FLUSH_UNSPEC,
> +	BRIDGE_FLUSH_FDB,
>  	__BRIDGE_FLUSH_MAX
>  };
>  #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
> +
> +/* embedded in BRIDGE_FLUSH_FDB */
> +enum {
> +	FDB_FLUSH_UNSPEC,
> +	__FDB_FLUSH_MAX
> +};
> +#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 4b0bf88c4121..62f694a739e1 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -594,6 +594,30 @@ void br_fdb_flush(struct net_bridge *br,
>  	rcu_read_unlock();
>  }
>  
> +static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
> +	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
> +};
> +
> +int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
> +			struct netlink_ext_ack *extack)
> +{
> +	struct nlattr *fdb_flush_tb[FDB_FLUSH_MAX + 1];
> +	struct net_bridge_fdb_flush_desc desc = {};
> +	int err;
> +
> +	err = nla_parse_nested(fdb_flush_tb, FDB_FLUSH_MAX, fdb_flush_attr,
> +			       br_fdb_flush_policy, extack);
> +	if (err)
> +		return err;
> +
> +	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
> +		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
> +
> +	br_fdb_flush(br, &desc);
> +
> +	return 0;
> +}
> +
>  /* Flush all entries referring to a specific port.
>   * if do_all is set also flush static entries
>   * if vid is set delete all entries that match the vlan_id
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 6e6dce6880c9..bd2c91e5723d 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -781,6 +781,7 @@ int br_process_vlan_info(struct net_bridge *br,
>  
>  static const struct nla_policy br_flush_policy[BRIDGE_FLUSH_MAX + 1] = {
>  	[BRIDGE_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
> +	[BRIDGE_FLUSH_FDB]	= { .type = NLA_NESTED },

In a previous submission [1] Jakub suggested using NLA_POLICY_NESTED()

[1] https://lore.kernel.org/netdev/20220224221447.6c7fa95d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/

>  };
>  
>  static int br_flush(struct net_bridge *br, int cmd,
> @@ -804,6 +805,13 @@ static int br_flush(struct net_bridge *br, int cmd,
>  	if (err)
>  		return err;
>  
> +	if (flush_tb[BRIDGE_FLUSH_FDB]) {
> +		err = br_fdb_flush_nlattr(br, flush_tb[BRIDGE_FLUSH_FDB],
> +					  extack);
> +		if (err)
> +			return err;
> +	}
> +
>  	return 0;
>  }
>  
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index e6930e9ee69d..c7ea531d30ef 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -768,6 +768,8 @@ int br_fdb_hash_init(struct net_bridge *br);
>  void br_fdb_hash_fini(struct net_bridge *br);
>  void br_fdb_flush(struct net_bridge *br,
>  		  const struct net_bridge_fdb_flush_desc *desc);
> +int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
> +			struct netlink_ext_ack *extack);
>  void br_fdb_find_delete_local(struct net_bridge *br,
>  			      const struct net_bridge_port *p,
>  			      const unsigned char *addr, u16 vid);
> -- 
> 2.35.1
> 

Re: [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:54PM +0300, Nikolay Aleksandrov wrote:
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 221a4256808f..2f3799cf14b2 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -807,7 +807,15 @@ enum {
>  /* embedded in IFLA_BRIDGE_FLUSH */
>  enum {
>  	BRIDGE_FLUSH_UNSPEC,
> +	BRIDGE_FLUSH_FDB,
>  	__BRIDGE_FLUSH_MAX
>  };
>  #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
> +
> +/* embedded in BRIDGE_FLUSH_FDB */
> +enum {
> +	FDB_FLUSH_UNSPEC,

BTW, is there a reason this is not called FLUSH_FDB_UNSPEC given it's
embedded in BRIDGE_FLUSH_FDB, which is embedded in IFLA_BRIDGE_FLUSH ?

Regardless, in the cover letter you have '[ BRIDGE_FDB_FLUSH ]', which
is actually BRIDGE_FLUSH_FDB. I only noticed it because the code didn't
match what I had in my notebook, which I copied from the cover letter :)

> +	__FDB_FLUSH_MAX
> +};
> +#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */

Re: [Bridge] [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:54PM +0300, Nikolay Aleksandrov wrote:
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 221a4256808f..2f3799cf14b2 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -807,7 +807,15 @@ enum {
>  /* embedded in IFLA_BRIDGE_FLUSH */
>  enum {
>  	BRIDGE_FLUSH_UNSPEC,
> +	BRIDGE_FLUSH_FDB,
>  	__BRIDGE_FLUSH_MAX
>  };
>  #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
> +
> +/* embedded in BRIDGE_FLUSH_FDB */
> +enum {
> +	FDB_FLUSH_UNSPEC,

BTW, is there a reason this is not called FLUSH_FDB_UNSPEC given it's
embedded in BRIDGE_FLUSH_FDB, which is embedded in IFLA_BRIDGE_FLUSH ?

Regardless, in the cover letter you have '[ BRIDGE_FDB_FLUSH ]', which
is actually BRIDGE_FLUSH_FDB. I only noticed it because the code didn't
match what I had in my notebook, which I copied from the cover letter :)

> +	__FDB_FLUSH_MAX
> +};
> +#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */

Re: [PATCH net-next 4/6] net: bridge: fdb: add support for flush filtering based on ndm flags and state

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:55PM +0300, Nikolay Aleksandrov wrote:
> Add support for fdb flush filtering based on ndm flags and state. The
> new attributes allow users to specify a mask and value which are mapped
> to bridge-specific flags. NTF_USE is used to represent added_by_user
> flag since it sets it on fdb add and we don't have a 1:1 mapping for it.
> 
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  include/uapi/linux/if_bridge.h |  4 +++
>  net/bridge/br_fdb.c            | 55 ++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 2f3799cf14b2..4638d7e39f2a 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -815,6 +815,10 @@ enum {
>  /* embedded in BRIDGE_FLUSH_FDB */
>  enum {
>  	FDB_FLUSH_UNSPEC,
> +	FDB_FLUSH_NDM_STATE,
> +	FDB_FLUSH_NDM_STATE_MASK,
> +	FDB_FLUSH_NDM_FLAGS,
> +	FDB_FLUSH_NDM_FLAGS_MASK,
>  	__FDB_FLUSH_MAX
>  };
>  #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 62f694a739e1..340a2ace1d5e 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -594,8 +594,40 @@ void br_fdb_flush(struct net_bridge *br,
>  	rcu_read_unlock();
>  }
>  
> +static unsigned long __ndm_state_to_fdb_flags(u16 ndm_state)
> +{
> +	unsigned long flags = 0;
> +
> +	if (ndm_state & NUD_PERMANENT)
> +		__set_bit(BR_FDB_LOCAL, &flags);
> +	if (ndm_state & NUD_NOARP)
> +		__set_bit(BR_FDB_STATIC, &flags);
> +
> +	return flags;
> +}
> +
> +static unsigned long __ndm_flags_to_fdb_flags(u16 ndm_flags)
> +{
> +	unsigned long flags = 0;
> +
> +	if (ndm_flags & NTF_USE)
> +		__set_bit(BR_FDB_ADDED_BY_USER, &flags);
> +	if (ndm_flags & NTF_EXT_LEARNED)
> +		__set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &flags);
> +	if (ndm_flags & NTF_OFFLOADED)
> +		__set_bit(BR_FDB_OFFLOADED, &flags);
> +	if (ndm_flags & NTF_STICKY)
> +		__set_bit(BR_FDB_STICKY, &flags);
> +
> +	return flags;
> +}
> +
>  static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>  	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
> +	[FDB_FLUSH_NDM_STATE]	= { .type = NLA_U16 },
> +	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
> +	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
> +	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },

Might be better to use NLA_POLICY_MASK(NLA_U16, mask) and reject
unsupported states / flags instead of just ignoring them?

>  };
>  
>  int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
> @@ -610,6 +642,29 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>  	if (err)
>  		return err;
>  
> +	if (fdb_flush_tb[FDB_FLUSH_NDM_STATE]) {
> +		u16 ndm_state = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_STATE]);
> +
> +		desc.flags |= __ndm_state_to_fdb_flags(ndm_state);
> +	}
> +	if (fdb_flush_tb[FDB_FLUSH_NDM_STATE_MASK]) {
> +		u16 ndm_state_mask;
> +
> +		ndm_state_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_STATE_MASK]);
> +		desc.flags_mask |= __ndm_state_to_fdb_flags(ndm_state_mask);
> +	}
> +	if (fdb_flush_tb[FDB_FLUSH_NDM_FLAGS]) {
> +		u16 ndm_flags = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS]);
> +
> +		desc.flags |= __ndm_flags_to_fdb_flags(ndm_flags);
> +	}
> +	if (fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]) {
> +		u16 ndm_flags_mask;
> +
> +		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
> +		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
> +	}
> +
>  	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
>  		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
>  
> -- 
> 2.35.1
> 

Re: [Bridge] [PATCH net-next 4/6] net: bridge: fdb: add support for flush filtering based on ndm flags and state

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:55PM +0300, Nikolay Aleksandrov wrote:
> Add support for fdb flush filtering based on ndm flags and state. The
> new attributes allow users to specify a mask and value which are mapped
> to bridge-specific flags. NTF_USE is used to represent added_by_user
> flag since it sets it on fdb add and we don't have a 1:1 mapping for it.
> 
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  include/uapi/linux/if_bridge.h |  4 +++
>  net/bridge/br_fdb.c            | 55 ++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 2f3799cf14b2..4638d7e39f2a 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -815,6 +815,10 @@ enum {
>  /* embedded in BRIDGE_FLUSH_FDB */
>  enum {
>  	FDB_FLUSH_UNSPEC,
> +	FDB_FLUSH_NDM_STATE,
> +	FDB_FLUSH_NDM_STATE_MASK,
> +	FDB_FLUSH_NDM_FLAGS,
> +	FDB_FLUSH_NDM_FLAGS_MASK,
>  	__FDB_FLUSH_MAX
>  };
>  #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 62f694a739e1..340a2ace1d5e 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -594,8 +594,40 @@ void br_fdb_flush(struct net_bridge *br,
>  	rcu_read_unlock();
>  }
>  
> +static unsigned long __ndm_state_to_fdb_flags(u16 ndm_state)
> +{
> +	unsigned long flags = 0;
> +
> +	if (ndm_state & NUD_PERMANENT)
> +		__set_bit(BR_FDB_LOCAL, &flags);
> +	if (ndm_state & NUD_NOARP)
> +		__set_bit(BR_FDB_STATIC, &flags);
> +
> +	return flags;
> +}
> +
> +static unsigned long __ndm_flags_to_fdb_flags(u16 ndm_flags)
> +{
> +	unsigned long flags = 0;
> +
> +	if (ndm_flags & NTF_USE)
> +		__set_bit(BR_FDB_ADDED_BY_USER, &flags);
> +	if (ndm_flags & NTF_EXT_LEARNED)
> +		__set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &flags);
> +	if (ndm_flags & NTF_OFFLOADED)
> +		__set_bit(BR_FDB_OFFLOADED, &flags);
> +	if (ndm_flags & NTF_STICKY)
> +		__set_bit(BR_FDB_STICKY, &flags);
> +
> +	return flags;
> +}
> +
>  static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>  	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
> +	[FDB_FLUSH_NDM_STATE]	= { .type = NLA_U16 },
> +	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
> +	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
> +	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },

Might be better to use NLA_POLICY_MASK(NLA_U16, mask) and reject
unsupported states / flags instead of just ignoring them?

>  };
>  
>  int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
> @@ -610,6 +642,29 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>  	if (err)
>  		return err;
>  
> +	if (fdb_flush_tb[FDB_FLUSH_NDM_STATE]) {
> +		u16 ndm_state = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_STATE]);
> +
> +		desc.flags |= __ndm_state_to_fdb_flags(ndm_state);
> +	}
> +	if (fdb_flush_tb[FDB_FLUSH_NDM_STATE_MASK]) {
> +		u16 ndm_state_mask;
> +
> +		ndm_state_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_STATE_MASK]);
> +		desc.flags_mask |= __ndm_state_to_fdb_flags(ndm_state_mask);
> +	}
> +	if (fdb_flush_tb[FDB_FLUSH_NDM_FLAGS]) {
> +		u16 ndm_flags = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS]);
> +
> +		desc.flags |= __ndm_flags_to_fdb_flags(ndm_flags);
> +	}
> +	if (fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]) {
> +		u16 ndm_flags_mask;
> +
> +		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
> +		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
> +	}
> +
>  	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
>  		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
>  
> -- 
> 2.35.1
> 

Re: [PATCH net-next 0/6] net: bridge: add flush filtering support

[Nikolay Aleksandrov]

On 11/04/2022 10:47, Ido Schimmel wrote:
> 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?
> 

Yes, I did think about that but when I think about ip link set, I think about
configuring the bridge, while flush is an action similar to dump/show. Also
it's a special setlink with bridge address family similar to bridge/link.c.
More below..

> $ 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.
> 

Generally I agree with you, but in this case I'd prefer to keep them in bridge/(fdb|mdb).c.
Semantically I think fdb/mdb actions should be done through "bridge fdb/mdb" sub-commands. All
of the flush options are fdb/mdb-specific attributes which already exist there and shouldn't
be exposed in ip/. I know there are counterexamples of actions being done through ip link
(e.g. current flush) but those exist due to historic reasons. Another thing is that if it
becomes an ip link subcommand I'll either have to move the bridge family setlink into ip/ or
I'd have to make it a bridge attribute (i.e. extend the bridge option attributes). I don't
like the duplication that has been happening recently (same options added to bridge link and
to ip link set type bridge_slave for example), let's try and keep ip link set for bridge/bridge_slave
configuration only. Although it's still a SETLINK just in the bridge AF, we do have actions being
done through it already. That being said I don't mind changing it to DELLINK given that it's a
special case.

>>
>> 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
>>

Re: [Bridge] [PATCH net-next 0/6] net: bridge: add flush filtering support

[Nikolay Aleksandrov]

On 11/04/2022 10:47, Ido Schimmel wrote:
> 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?
> 

Yes, I did think about that but when I think about ip link set, I think about
configuring the bridge, while flush is an action similar to dump/show. Also
it's a special setlink with bridge address family similar to bridge/link.c.
More below..

> $ 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.
> 

Generally I agree with you, but in this case I'd prefer to keep them in bridge/(fdb|mdb).c.
Semantically I think fdb/mdb actions should be done through "bridge fdb/mdb" sub-commands. All
of the flush options are fdb/mdb-specific attributes which already exist there and shouldn't
be exposed in ip/. I know there are counterexamples of actions being done through ip link
(e.g. current flush) but those exist due to historic reasons. Another thing is that if it
becomes an ip link subcommand I'll either have to move the bridge family setlink into ip/ or
I'd have to make it a bridge attribute (i.e. extend the bridge option attributes). I don't
like the duplication that has been happening recently (same options added to bridge link and
to ip link set type bridge_slave for example), let's try and keep ip link set for bridge/bridge_slave
configuration only. Although it's still a SETLINK just in the bridge AF, we do have actions being
done through it already. That being said I don't mind changing it to DELLINK given that it's a
special case.

>>
>> 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
>>

Re: [PATCH net-next 2/6] net: bridge: fdb: add support for fine-grained flushing

[Nikolay Aleksandrov]

On 11/04/2022 11:20, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:53PM +0300, Nikolay Aleksandrov wrote:
>> Add the ability to specify exactly which fdbs to be flushed. They are
>> described by a new structure - net_bridge_fdb_flush_desc. Currently it
>> can match on port/bridge ifindex, vlan id and fdb flags. It is used to
>> describe the existing dynamic fdb flush operation.
>>
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> ---
>>  net/bridge/br_fdb.c      | 36 +++++++++++++++++++++++++++++-------
>>  net/bridge/br_netlink.c  |  9 +++++++--
>>  net/bridge/br_private.h  | 10 +++++++++-
>>  net/bridge/br_sysfs_br.c |  6 +++++-
>>  4 files changed, 50 insertions(+), 11 deletions(-)
>>
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 6ccda68bd473..4b0bf88c4121 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -558,18 +558,40 @@ void br_fdb_cleanup(struct work_struct *work)
>>  	mod_delayed_work(system_long_wq, &br->gc_work, work_delay);
>>  }
>>  
>> -/* Completely flush all dynamic entries in forwarding database.*/
>> -void br_fdb_flush(struct net_bridge *br)
>> +static bool __fdb_flush_matches(const struct net_bridge *br,
>> +				const struct net_bridge_fdb_entry *f,
>> +				const struct net_bridge_fdb_flush_desc *desc)
>> +{
>> +	const struct net_bridge_port *dst = READ_ONCE(f->dst);
>> +	int port_ifidx, br_ifidx = br->dev->ifindex;
>> +
>> +	port_ifidx = dst ? dst->dev->ifindex : 0;
>> +
>> +	return (!desc->vlan_id || desc->vlan_id == f->key.vlan_id) &&
>> +	       (!desc->port_ifindex ||
>> +		(desc->port_ifindex == port_ifidx ||
>> +		 (!dst && desc->port_ifindex == br_ifidx))) &&
>> +	       (!desc->flags_mask ||
>> +		((f->flags & desc->flags_mask) == desc->flags));
> 
> I find this easier to read:
> 
> port_ifidx = dst ? dst->dev->ifindex : br_ifidx;
> 
> if (desc->vlan_id && desc->vlan_id != f->key.vlan_id)
> 	return false;
> if (desc->port_ifindex && desc->port_ifindex != port_ifidx)
> 	return false;
> if (desc->flags_mask && (f->flags & desc->flags_mask) != desc->flags)
> 	return false;
> 
> return true;
> 

Ack

>> +}
>> +
>> +/* Flush forwarding database entries matching the description */
>> +void br_fdb_flush(struct net_bridge *br,
>> +		  const struct net_bridge_fdb_flush_desc *desc)
>>  {
>>  	struct net_bridge_fdb_entry *f;
>> -	struct hlist_node *tmp;
>>  
>> -	spin_lock_bh(&br->hash_lock);
>> -	hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
>> -		if (!test_bit(BR_FDB_STATIC, &f->flags))
>> +	rcu_read_lock();
>> +	hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
>> +		if (!__fdb_flush_matches(br, f, desc))
>> +			continue;
>> +
>> +		spin_lock_bh(&br->hash_lock);
>> +		if (!hlist_unhashed(&f->fdb_node))
>>  			fdb_delete(br, f, true);
>> +		spin_unlock_bh(&br->hash_lock);
>>  	}
>> -	spin_unlock_bh(&br->hash_lock);
>> +	rcu_read_unlock();
>>  }
>>  
>>  /* Flush all entries referring to a specific port.
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index fe2211d4c0c7..6e6dce6880c9 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -1366,8 +1366,13 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
>>  		br_recalculate_fwd_mask(br);
>>  	}
>>  
>> -	if (data[IFLA_BR_FDB_FLUSH])
>> -		br_fdb_flush(br);
>> +	if (data[IFLA_BR_FDB_FLUSH]) {
>> +		struct net_bridge_fdb_flush_desc desc = {
>> +			.flags_mask = BR_FDB_STATIC
>> +		};
>> +
>> +		br_fdb_flush(br, &desc);
> 
> I wanted to ask why you are not doing the same for IFLA_BRPORT_FLUSH,
> but then I read the implementation of br_fdb_delete_by_port() and
> remembered the comment in the cover letter regarding fdb_delete vs
> fdb_delete_local. Probably best to note it in the commit message
> 

Yup, good point. Will add.

>> +	}
>>  
>>  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
>>  	if (data[IFLA_BR_MCAST_ROUTER]) {
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 6e62af2e07e9..e6930e9ee69d 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -274,6 +274,13 @@ struct net_bridge_fdb_entry {
>>  	struct rcu_head			rcu;
>>  };
>>  
>> +struct net_bridge_fdb_flush_desc {
>> +	unsigned long			flags;
>> +	unsigned long			flags_mask;
>> +	int				port_ifindex;
>> +	u16				vlan_id;
>> +};
>> +
>>  #define MDB_PG_FLAGS_PERMANENT	BIT(0)
>>  #define MDB_PG_FLAGS_OFFLOAD	BIT(1)
>>  #define MDB_PG_FLAGS_FAST_LEAVE	BIT(2)
>> @@ -759,7 +766,8 @@ int br_fdb_init(void);
>>  void br_fdb_fini(void);
>>  int br_fdb_hash_init(struct net_bridge *br);
>>  void br_fdb_hash_fini(struct net_bridge *br);
>> -void br_fdb_flush(struct net_bridge *br);
>> +void br_fdb_flush(struct net_bridge *br,
>> +		  const struct net_bridge_fdb_flush_desc *desc);
>>  void br_fdb_find_delete_local(struct net_bridge *br,
>>  			      const struct net_bridge_port *p,
>>  			      const unsigned char *addr, u16 vid);
>> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
>> index 3f7ca88c2aa3..612e367fff20 100644
>> --- a/net/bridge/br_sysfs_br.c
>> +++ b/net/bridge/br_sysfs_br.c
>> @@ -344,7 +344,11 @@ static DEVICE_ATTR_RW(group_addr);
>>  static int set_flush(struct net_bridge *br, unsigned long val,
>>  		     struct netlink_ext_ack *extack)
>>  {
>> -	br_fdb_flush(br);
>> +	struct net_bridge_fdb_flush_desc desc = {
>> +		.flags_mask = BR_FDB_STATIC
>> +	};
>> +
>> +	br_fdb_flush(br, &desc);
>>  	return 0;
>>  }
>>  
>> -- 
>> 2.35.1
>>

Re: [Bridge] [PATCH net-next 2/6] net: bridge: fdb: add support for fine-grained flushing

[Nikolay Aleksandrov]

On 11/04/2022 11:20, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:53PM +0300, Nikolay Aleksandrov wrote:
>> Add the ability to specify exactly which fdbs to be flushed. They are
>> described by a new structure - net_bridge_fdb_flush_desc. Currently it
>> can match on port/bridge ifindex, vlan id and fdb flags. It is used to
>> describe the existing dynamic fdb flush operation.
>>
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> ---
>>  net/bridge/br_fdb.c      | 36 +++++++++++++++++++++++++++++-------
>>  net/bridge/br_netlink.c  |  9 +++++++--
>>  net/bridge/br_private.h  | 10 +++++++++-
>>  net/bridge/br_sysfs_br.c |  6 +++++-
>>  4 files changed, 50 insertions(+), 11 deletions(-)
>>
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 6ccda68bd473..4b0bf88c4121 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -558,18 +558,40 @@ void br_fdb_cleanup(struct work_struct *work)
>>  	mod_delayed_work(system_long_wq, &br->gc_work, work_delay);
>>  }
>>  
>> -/* Completely flush all dynamic entries in forwarding database.*/
>> -void br_fdb_flush(struct net_bridge *br)
>> +static bool __fdb_flush_matches(const struct net_bridge *br,
>> +				const struct net_bridge_fdb_entry *f,
>> +				const struct net_bridge_fdb_flush_desc *desc)
>> +{
>> +	const struct net_bridge_port *dst = READ_ONCE(f->dst);
>> +	int port_ifidx, br_ifidx = br->dev->ifindex;
>> +
>> +	port_ifidx = dst ? dst->dev->ifindex : 0;
>> +
>> +	return (!desc->vlan_id || desc->vlan_id == f->key.vlan_id) &&
>> +	       (!desc->port_ifindex ||
>> +		(desc->port_ifindex == port_ifidx ||
>> +		 (!dst && desc->port_ifindex == br_ifidx))) &&
>> +	       (!desc->flags_mask ||
>> +		((f->flags & desc->flags_mask) == desc->flags));
> 
> I find this easier to read:
> 
> port_ifidx = dst ? dst->dev->ifindex : br_ifidx;
> 
> if (desc->vlan_id && desc->vlan_id != f->key.vlan_id)
> 	return false;
> if (desc->port_ifindex && desc->port_ifindex != port_ifidx)
> 	return false;
> if (desc->flags_mask && (f->flags & desc->flags_mask) != desc->flags)
> 	return false;
> 
> return true;
> 

Ack

>> +}
>> +
>> +/* Flush forwarding database entries matching the description */
>> +void br_fdb_flush(struct net_bridge *br,
>> +		  const struct net_bridge_fdb_flush_desc *desc)
>>  {
>>  	struct net_bridge_fdb_entry *f;
>> -	struct hlist_node *tmp;
>>  
>> -	spin_lock_bh(&br->hash_lock);
>> -	hlist_for_each_entry_safe(f, tmp, &br->fdb_list, fdb_node) {
>> -		if (!test_bit(BR_FDB_STATIC, &f->flags))
>> +	rcu_read_lock();
>> +	hlist_for_each_entry_rcu(f, &br->fdb_list, fdb_node) {
>> +		if (!__fdb_flush_matches(br, f, desc))
>> +			continue;
>> +
>> +		spin_lock_bh(&br->hash_lock);
>> +		if (!hlist_unhashed(&f->fdb_node))
>>  			fdb_delete(br, f, true);
>> +		spin_unlock_bh(&br->hash_lock);
>>  	}
>> -	spin_unlock_bh(&br->hash_lock);
>> +	rcu_read_unlock();
>>  }
>>  
>>  /* Flush all entries referring to a specific port.
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index fe2211d4c0c7..6e6dce6880c9 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -1366,8 +1366,13 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[],
>>  		br_recalculate_fwd_mask(br);
>>  	}
>>  
>> -	if (data[IFLA_BR_FDB_FLUSH])
>> -		br_fdb_flush(br);
>> +	if (data[IFLA_BR_FDB_FLUSH]) {
>> +		struct net_bridge_fdb_flush_desc desc = {
>> +			.flags_mask = BR_FDB_STATIC
>> +		};
>> +
>> +		br_fdb_flush(br, &desc);
> 
> I wanted to ask why you are not doing the same for IFLA_BRPORT_FLUSH,
> but then I read the implementation of br_fdb_delete_by_port() and
> remembered the comment in the cover letter regarding fdb_delete vs
> fdb_delete_local. Probably best to note it in the commit message
> 

Yup, good point. Will add.

>> +	}
>>  
>>  #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
>>  	if (data[IFLA_BR_MCAST_ROUTER]) {
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 6e62af2e07e9..e6930e9ee69d 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -274,6 +274,13 @@ struct net_bridge_fdb_entry {
>>  	struct rcu_head			rcu;
>>  };
>>  
>> +struct net_bridge_fdb_flush_desc {
>> +	unsigned long			flags;
>> +	unsigned long			flags_mask;
>> +	int				port_ifindex;
>> +	u16				vlan_id;
>> +};
>> +
>>  #define MDB_PG_FLAGS_PERMANENT	BIT(0)
>>  #define MDB_PG_FLAGS_OFFLOAD	BIT(1)
>>  #define MDB_PG_FLAGS_FAST_LEAVE	BIT(2)
>> @@ -759,7 +766,8 @@ int br_fdb_init(void);
>>  void br_fdb_fini(void);
>>  int br_fdb_hash_init(struct net_bridge *br);
>>  void br_fdb_hash_fini(struct net_bridge *br);
>> -void br_fdb_flush(struct net_bridge *br);
>> +void br_fdb_flush(struct net_bridge *br,
>> +		  const struct net_bridge_fdb_flush_desc *desc);
>>  void br_fdb_find_delete_local(struct net_bridge *br,
>>  			      const struct net_bridge_port *p,
>>  			      const unsigned char *addr, u16 vid);
>> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
>> index 3f7ca88c2aa3..612e367fff20 100644
>> --- a/net/bridge/br_sysfs_br.c
>> +++ b/net/bridge/br_sysfs_br.c
>> @@ -344,7 +344,11 @@ static DEVICE_ATTR_RW(group_addr);
>>  static int set_flush(struct net_bridge *br, unsigned long val,
>>  		     struct netlink_ext_ack *extack)
>>  {
>> -	br_fdb_flush(br);
>> +	struct net_bridge_fdb_flush_desc desc = {
>> +		.flags_mask = BR_FDB_STATIC
>> +	};
>> +
>> +	br_fdb_flush(br, &desc);
>>  	return 0;
>>  }
>>  
>> -- 
>> 2.35.1
>>

Re: [PATCH net-next 0/6] net: bridge: add flush filtering support

[Ido Schimmel]

On Mon, Apr 11, 2022 at 10:47:38AM +0300, Ido Schimmel wrote:
> 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

Thinking about it again, I guess this is more appropriate:

$ ip link set dev bridge type bridge fdb_flush port ens16 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
> > 

Re: [Bridge] [PATCH net-next 0/6] net: bridge: add flush filtering support

[Ido Schimmel]

On Mon, Apr 11, 2022 at 10:47:38AM +0300, Ido Schimmel wrote:
> 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

Thinking about it again, I guess this is more appropriate:

$ ip link set dev bridge type bridge fdb_flush port ens16 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
> > 

Re: [PATCH net-next 5/6] net: bridge: fdb: add support for flush filtering based on ifindex

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:56PM +0300, Nikolay Aleksandrov wrote:
> Add support for fdb flush filtering based on destination ifindex. The
> ifindex must either match a port's device ifindex or the bridge's.
> 
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  include/uapi/linux/if_bridge.h | 1 +
>  net/bridge/br_fdb.c            | 7 +++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 4638d7e39f2a..67ee12586844 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -819,6 +819,7 @@ enum {
>  	FDB_FLUSH_NDM_STATE_MASK,
>  	FDB_FLUSH_NDM_FLAGS,
>  	FDB_FLUSH_NDM_FLAGS_MASK,
> +	FDB_FLUSH_PORT_IFINDEX,
>  	__FDB_FLUSH_MAX
>  };
>  #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 340a2ace1d5e..53208adf7474 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -628,6 +628,7 @@ static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>  	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
>  	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
>  	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
> +	[FDB_FLUSH_PORT_IFINDEX]	= { .type = NLA_S32 },
>  };
>  
>  int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
> @@ -664,6 +665,12 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>  		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
>  		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
>  	}
> +	if (fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]) {
> +		int port_ifidx;
> +
> +		port_ifidx = nla_get_u32(fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]);
> +		desc.port_ifindex = port_ifidx;

Commit message says "ifindex must either match a port's device ifindex
or the bridge's", but there is no validation. I realize such an
operation won't flush anything, but it's cleaner to just reject it here.

> +	}
>  
>  	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
>  		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
> -- 
> 2.35.1
> 

Re: [Bridge] [PATCH net-next 5/6] net: bridge: fdb: add support for flush filtering based on ifindex

[Ido Schimmel]

On Sat, Apr 09, 2022 at 01:58:56PM +0300, Nikolay Aleksandrov wrote:
> Add support for fdb flush filtering based on destination ifindex. The
> ifindex must either match a port's device ifindex or the bridge's.
> 
> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
> ---
>  include/uapi/linux/if_bridge.h | 1 +
>  net/bridge/br_fdb.c            | 7 +++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index 4638d7e39f2a..67ee12586844 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -819,6 +819,7 @@ enum {
>  	FDB_FLUSH_NDM_STATE_MASK,
>  	FDB_FLUSH_NDM_FLAGS,
>  	FDB_FLUSH_NDM_FLAGS_MASK,
> +	FDB_FLUSH_PORT_IFINDEX,
>  	__FDB_FLUSH_MAX
>  };
>  #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 340a2ace1d5e..53208adf7474 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -628,6 +628,7 @@ static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>  	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
>  	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
>  	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
> +	[FDB_FLUSH_PORT_IFINDEX]	= { .type = NLA_S32 },
>  };
>  
>  int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
> @@ -664,6 +665,12 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>  		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
>  		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
>  	}
> +	if (fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]) {
> +		int port_ifidx;
> +
> +		port_ifidx = nla_get_u32(fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]);
> +		desc.port_ifindex = port_ifidx;

Commit message says "ifindex must either match a port's device ifindex
or the bridge's", but there is no validation. I realize such an
operation won't flush anything, but it's cleaner to just reject it here.

> +	}
>  
>  	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
>  		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
> -- 
> 2.35.1
> 

Re: [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Nikolay Aleksandrov]

On 11/04/2022 11:33, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:54PM +0300, Nikolay Aleksandrov wrote:
>> Add a new fdb flush call which parses the embedded attributes in
>> BRIDGE_FLUSH_FDB and fills in the fdb flush descriptor to delete only
>> matching entries. Currently it's a complete flush, support for more
>> fine-grained filtering will be added in the following patches.
>>
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> ---
>>  include/uapi/linux/if_bridge.h |  8 ++++++++
>>  net/bridge/br_fdb.c            | 24 ++++++++++++++++++++++++
>>  net/bridge/br_netlink.c        |  8 ++++++++
>>  net/bridge/br_private.h        |  2 ++
>>  4 files changed, 42 insertions(+)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index 221a4256808f..2f3799cf14b2 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -807,7 +807,15 @@ enum {
>>  /* embedded in IFLA_BRIDGE_FLUSH */
>>  enum {
>>  	BRIDGE_FLUSH_UNSPEC,
>> +	BRIDGE_FLUSH_FDB,
>>  	__BRIDGE_FLUSH_MAX
>>  };
>>  #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
>> +
>> +/* embedded in BRIDGE_FLUSH_FDB */
>> +enum {
>> +	FDB_FLUSH_UNSPEC,
>> +	__FDB_FLUSH_MAX
>> +};
>> +#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 4b0bf88c4121..62f694a739e1 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -594,6 +594,30 @@ void br_fdb_flush(struct net_bridge *br,
>>  	rcu_read_unlock();
>>  }
>>  
>> +static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>> +	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
>> +};
>> +
>> +int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>> +			struct netlink_ext_ack *extack)
>> +{
>> +	struct nlattr *fdb_flush_tb[FDB_FLUSH_MAX + 1];
>> +	struct net_bridge_fdb_flush_desc desc = {};
>> +	int err;
>> +
>> +	err = nla_parse_nested(fdb_flush_tb, FDB_FLUSH_MAX, fdb_flush_attr,
>> +			       br_fdb_flush_policy, extack);
>> +	if (err)
>> +		return err;
>> +
>> +	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
>> +		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
>> +
>> +	br_fdb_flush(br, &desc);
>> +
>> +	return 0;
>> +}
>> +
>>  /* Flush all entries referring to a specific port.
>>   * if do_all is set also flush static entries
>>   * if vid is set delete all entries that match the vlan_id
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 6e6dce6880c9..bd2c91e5723d 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -781,6 +781,7 @@ int br_process_vlan_info(struct net_bridge *br,
>>  
>>  static const struct nla_policy br_flush_policy[BRIDGE_FLUSH_MAX + 1] = {
>>  	[BRIDGE_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
>> +	[BRIDGE_FLUSH_FDB]	= { .type = NLA_NESTED },
> 
> In a previous submission [1] Jakub suggested using NLA_POLICY_NESTED()
> 
> [1] https://lore.kernel.org/netdev/20220224221447.6c7fa95d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/
> 

I didn't use it because I'll have to export the private object (fdb/mdb)
policy structs, it will also require some ifdefs due to conditional mdb.
They use nla_parse_nested internally, so they get validated properly.

>>  };
>>  
>>  static int br_flush(struct net_bridge *br, int cmd,
>> @@ -804,6 +805,13 @@ static int br_flush(struct net_bridge *br, int cmd,
>>  	if (err)
>>  		return err;
>>  
>> +	if (flush_tb[BRIDGE_FLUSH_FDB]) {
>> +		err = br_fdb_flush_nlattr(br, flush_tb[BRIDGE_FLUSH_FDB],
>> +					  extack);
>> +		if (err)
>> +			return err;
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index e6930e9ee69d..c7ea531d30ef 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -768,6 +768,8 @@ int br_fdb_hash_init(struct net_bridge *br);
>>  void br_fdb_hash_fini(struct net_bridge *br);
>>  void br_fdb_flush(struct net_bridge *br,
>>  		  const struct net_bridge_fdb_flush_desc *desc);
>> +int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>> +			struct netlink_ext_ack *extack);
>>  void br_fdb_find_delete_local(struct net_bridge *br,
>>  			      const struct net_bridge_port *p,
>>  			      const unsigned char *addr, u16 vid);
>> -- 
>> 2.35.1
>>

Re: [Bridge] [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Nikolay Aleksandrov]

On 11/04/2022 11:33, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:54PM +0300, Nikolay Aleksandrov wrote:
>> Add a new fdb flush call which parses the embedded attributes in
>> BRIDGE_FLUSH_FDB and fills in the fdb flush descriptor to delete only
>> matching entries. Currently it's a complete flush, support for more
>> fine-grained filtering will be added in the following patches.
>>
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> ---
>>  include/uapi/linux/if_bridge.h |  8 ++++++++
>>  net/bridge/br_fdb.c            | 24 ++++++++++++++++++++++++
>>  net/bridge/br_netlink.c        |  8 ++++++++
>>  net/bridge/br_private.h        |  2 ++
>>  4 files changed, 42 insertions(+)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index 221a4256808f..2f3799cf14b2 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -807,7 +807,15 @@ enum {
>>  /* embedded in IFLA_BRIDGE_FLUSH */
>>  enum {
>>  	BRIDGE_FLUSH_UNSPEC,
>> +	BRIDGE_FLUSH_FDB,
>>  	__BRIDGE_FLUSH_MAX
>>  };
>>  #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
>> +
>> +/* embedded in BRIDGE_FLUSH_FDB */
>> +enum {
>> +	FDB_FLUSH_UNSPEC,
>> +	__FDB_FLUSH_MAX
>> +};
>> +#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 4b0bf88c4121..62f694a739e1 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -594,6 +594,30 @@ void br_fdb_flush(struct net_bridge *br,
>>  	rcu_read_unlock();
>>  }
>>  
>> +static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>> +	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
>> +};
>> +
>> +int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>> +			struct netlink_ext_ack *extack)
>> +{
>> +	struct nlattr *fdb_flush_tb[FDB_FLUSH_MAX + 1];
>> +	struct net_bridge_fdb_flush_desc desc = {};
>> +	int err;
>> +
>> +	err = nla_parse_nested(fdb_flush_tb, FDB_FLUSH_MAX, fdb_flush_attr,
>> +			       br_fdb_flush_policy, extack);
>> +	if (err)
>> +		return err;
>> +
>> +	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
>> +		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
>> +
>> +	br_fdb_flush(br, &desc);
>> +
>> +	return 0;
>> +}
>> +
>>  /* Flush all entries referring to a specific port.
>>   * if do_all is set also flush static entries
>>   * if vid is set delete all entries that match the vlan_id
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 6e6dce6880c9..bd2c91e5723d 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -781,6 +781,7 @@ int br_process_vlan_info(struct net_bridge *br,
>>  
>>  static const struct nla_policy br_flush_policy[BRIDGE_FLUSH_MAX + 1] = {
>>  	[BRIDGE_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
>> +	[BRIDGE_FLUSH_FDB]	= { .type = NLA_NESTED },
> 
> In a previous submission [1] Jakub suggested using NLA_POLICY_NESTED()
> 
> [1] https://lore.kernel.org/netdev/20220224221447.6c7fa95d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/
> 

I didn't use it because I'll have to export the private object (fdb/mdb)
policy structs, it will also require some ifdefs due to conditional mdb.
They use nla_parse_nested internally, so they get validated properly.

>>  };
>>  
>>  static int br_flush(struct net_bridge *br, int cmd,
>> @@ -804,6 +805,13 @@ static int br_flush(struct net_bridge *br, int cmd,
>>  	if (err)
>>  		return err;
>>  
>> +	if (flush_tb[BRIDGE_FLUSH_FDB]) {
>> +		err = br_fdb_flush_nlattr(br, flush_tb[BRIDGE_FLUSH_FDB],
>> +					  extack);
>> +		if (err)
>> +			return err;
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index e6930e9ee69d..c7ea531d30ef 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -768,6 +768,8 @@ int br_fdb_hash_init(struct net_bridge *br);
>>  void br_fdb_hash_fini(struct net_bridge *br);
>>  void br_fdb_flush(struct net_bridge *br,
>>  		  const struct net_bridge_fdb_flush_desc *desc);
>> +int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>> +			struct netlink_ext_ack *extack);
>>  void br_fdb_find_delete_local(struct net_bridge *br,
>>  			      const struct net_bridge_port *p,
>>  			      const unsigned char *addr, u16 vid);
>> -- 
>> 2.35.1
>>

Re: [PATCH net-next 5/6] net: bridge: fdb: add support for flush filtering based on ifindex

[Nikolay Aleksandrov]

On 11/04/2022 11:57, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:56PM +0300, Nikolay Aleksandrov wrote:
>> Add support for fdb flush filtering based on destination ifindex. The
>> ifindex must either match a port's device ifindex or the bridge's.
>>
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> ---
>>  include/uapi/linux/if_bridge.h | 1 +
>>  net/bridge/br_fdb.c            | 7 +++++++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index 4638d7e39f2a..67ee12586844 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -819,6 +819,7 @@ enum {
>>  	FDB_FLUSH_NDM_STATE_MASK,
>>  	FDB_FLUSH_NDM_FLAGS,
>>  	FDB_FLUSH_NDM_FLAGS_MASK,
>> +	FDB_FLUSH_PORT_IFINDEX,
>>  	__FDB_FLUSH_MAX
>>  };
>>  #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 340a2ace1d5e..53208adf7474 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -628,6 +628,7 @@ static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>>  	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
>>  	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
>>  	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
>> +	[FDB_FLUSH_PORT_IFINDEX]	= { .type = NLA_S32 },
>>  };
>>  
>>  int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>> @@ -664,6 +665,12 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>>  		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
>>  		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
>>  	}
>> +	if (fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]) {
>> +		int port_ifidx;
>> +
>> +		port_ifidx = nla_get_u32(fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]);
>> +		desc.port_ifindex = port_ifidx;
> 
> Commit message says "ifindex must either match a port's device ifindex
> or the bridge's", but there is no validation. I realize such an
> operation won't flush anything, but it's cleaner to just reject it here.
> 

Sure, I can add a check for the device when specified.

>> +	}
>>  
>>  	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
>>  		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
>> -- 
>> 2.35.1
>>

Re: [Bridge] [PATCH net-next 5/6] net: bridge: fdb: add support for flush filtering based on ifindex

[Nikolay Aleksandrov]

On 11/04/2022 11:57, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:56PM +0300, Nikolay Aleksandrov wrote:
>> Add support for fdb flush filtering based on destination ifindex. The
>> ifindex must either match a port's device ifindex or the bridge's.
>>
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> ---
>>  include/uapi/linux/if_bridge.h | 1 +
>>  net/bridge/br_fdb.c            | 7 +++++++
>>  2 files changed, 8 insertions(+)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index 4638d7e39f2a..67ee12586844 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -819,6 +819,7 @@ enum {
>>  	FDB_FLUSH_NDM_STATE_MASK,
>>  	FDB_FLUSH_NDM_FLAGS,
>>  	FDB_FLUSH_NDM_FLAGS_MASK,
>> +	FDB_FLUSH_PORT_IFINDEX,
>>  	__FDB_FLUSH_MAX
>>  };
>>  #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 340a2ace1d5e..53208adf7474 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -628,6 +628,7 @@ static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>>  	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
>>  	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
>>  	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
>> +	[FDB_FLUSH_PORT_IFINDEX]	= { .type = NLA_S32 },
>>  };
>>  
>>  int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>> @@ -664,6 +665,12 @@ int br_fdb_flush_nlattr(struct net_bridge *br, struct nlattr *fdb_flush_attr,
>>  		ndm_flags_mask = nla_get_u16(fdb_flush_tb[FDB_FLUSH_NDM_FLAGS_MASK]);
>>  		desc.flags_mask |= __ndm_flags_to_fdb_flags(ndm_flags_mask);
>>  	}
>> +	if (fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]) {
>> +		int port_ifidx;
>> +
>> +		port_ifidx = nla_get_u32(fdb_flush_tb[FDB_FLUSH_PORT_IFINDEX]);
>> +		desc.port_ifindex = port_ifidx;
> 
> Commit message says "ifindex must either match a port's device ifindex
> or the bridge's", but there is no validation. I realize such an
> operation won't flush anything, but it's cleaner to just reject it here.
> 

Sure, I can add a check for the device when specified.

>> +	}
>>  
>>  	br_debug(br, "flushing port ifindex: %d vlan id: %u flags: 0x%lx flags mask: 0x%lx\n",
>>  		 desc.port_ifindex, desc.vlan_id, desc.flags, desc.flags_mask);
>> -- 
>> 2.35.1
>>

Re: [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Nikolay Aleksandrov]

On 11/04/2022 11:41, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:54PM +0300, Nikolay Aleksandrov wrote:
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index 221a4256808f..2f3799cf14b2 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -807,7 +807,15 @@ enum {
>>  /* embedded in IFLA_BRIDGE_FLUSH */
>>  enum {
>>  	BRIDGE_FLUSH_UNSPEC,
>> +	BRIDGE_FLUSH_FDB,
>>  	__BRIDGE_FLUSH_MAX
>>  };
>>  #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
>> +
>> +/* embedded in BRIDGE_FLUSH_FDB */
>> +enum {
>> +	FDB_FLUSH_UNSPEC,
> 
> BTW, is there a reason this is not called FLUSH_FDB_UNSPEC given it's
> embedded in BRIDGE_FLUSH_FDB, which is embedded in IFLA_BRIDGE_FLUSH ?
> 
> Regardless, in the cover letter you have '[ BRIDGE_FDB_FLUSH ]', which
> is actually BRIDGE_FLUSH_FDB. I only noticed it because the code didn't
> match what I had in my notebook, which I copied from the cover letter :)
> 

Oops, that's a mismatch between an older version of the set and this one. :)

>> +	__FDB_FLUSH_MAX
>> +};
>> +#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */

Re: [Bridge] [PATCH net-next 3/6] net: bridge: fdb: add new nl attribute-based flush call

[Nikolay Aleksandrov]

On 11/04/2022 11:41, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:54PM +0300, Nikolay Aleksandrov wrote:
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index 221a4256808f..2f3799cf14b2 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -807,7 +807,15 @@ enum {
>>  /* embedded in IFLA_BRIDGE_FLUSH */
>>  enum {
>>  	BRIDGE_FLUSH_UNSPEC,
>> +	BRIDGE_FLUSH_FDB,
>>  	__BRIDGE_FLUSH_MAX
>>  };
>>  #define BRIDGE_FLUSH_MAX (__BRIDGE_FLUSH_MAX - 1)
>> +
>> +/* embedded in BRIDGE_FLUSH_FDB */
>> +enum {
>> +	FDB_FLUSH_UNSPEC,
> 
> BTW, is there a reason this is not called FLUSH_FDB_UNSPEC given it's
> embedded in BRIDGE_FLUSH_FDB, which is embedded in IFLA_BRIDGE_FLUSH ?
> 
> Regardless, in the cover letter you have '[ BRIDGE_FDB_FLUSH ]', which
> is actually BRIDGE_FLUSH_FDB. I only noticed it because the code didn't
> match what I had in my notebook, which I copied from the cover letter :)
> 

Oops, that's a mismatch between an older version of the set and this one. :)

>> +	__FDB_FLUSH_MAX
>> +};
>> +#define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>>  #endif /* _UAPI_LINUX_IF_BRIDGE_H */

Re: [PATCH net-next 4/6] net: bridge: fdb: add support for flush filtering based on ndm flags and state

[Nikolay Aleksandrov]

On 11/04/2022 11:47, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:55PM +0300, Nikolay Aleksandrov wrote:
>> Add support for fdb flush filtering based on ndm flags and state. The
>> new attributes allow users to specify a mask and value which are mapped
>> to bridge-specific flags. NTF_USE is used to represent added_by_user
>> flag since it sets it on fdb add and we don't have a 1:1 mapping for it.
>>
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> ---
>>  include/uapi/linux/if_bridge.h |  4 +++
>>  net/bridge/br_fdb.c            | 55 ++++++++++++++++++++++++++++++++++
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index 2f3799cf14b2..4638d7e39f2a 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -815,6 +815,10 @@ enum {
>>  /* embedded in BRIDGE_FLUSH_FDB */
>>  enum {
>>  	FDB_FLUSH_UNSPEC,
>> +	FDB_FLUSH_NDM_STATE,
>> +	FDB_FLUSH_NDM_STATE_MASK,
>> +	FDB_FLUSH_NDM_FLAGS,
>> +	FDB_FLUSH_NDM_FLAGS_MASK,
>>  	__FDB_FLUSH_MAX
>>  };
>>  #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 62f694a739e1..340a2ace1d5e 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -594,8 +594,40 @@ void br_fdb_flush(struct net_bridge *br,
>>  	rcu_read_unlock();
>>  }
>>  
>> +static unsigned long __ndm_state_to_fdb_flags(u16 ndm_state)
>> +{
>> +	unsigned long flags = 0;
>> +
>> +	if (ndm_state & NUD_PERMANENT)
>> +		__set_bit(BR_FDB_LOCAL, &flags);
>> +	if (ndm_state & NUD_NOARP)
>> +		__set_bit(BR_FDB_STATIC, &flags);
>> +
>> +	return flags;
>> +}
>> +
>> +static unsigned long __ndm_flags_to_fdb_flags(u16 ndm_flags)
>> +{
>> +	unsigned long flags = 0;
>> +
>> +	if (ndm_flags & NTF_USE)
>> +		__set_bit(BR_FDB_ADDED_BY_USER, &flags);
>> +	if (ndm_flags & NTF_EXT_LEARNED)
>> +		__set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &flags);
>> +	if (ndm_flags & NTF_OFFLOADED)
>> +		__set_bit(BR_FDB_OFFLOADED, &flags);
>> +	if (ndm_flags & NTF_STICKY)
>> +		__set_bit(BR_FDB_STICKY, &flags);
>> +
>> +	return flags;
>> +}
>> +
>>  static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>>  	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
>> +	[FDB_FLUSH_NDM_STATE]	= { .type = NLA_U16 },
>> +	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
>> +	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
>> +	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
> 
> Might be better to use NLA_POLICY_MASK(NLA_U16, mask) and reject
> unsupported states / flags instead of just ignoring them?
> 

Yep, forgot about that one. Good point!

Re: [Bridge] [PATCH net-next 4/6] net: bridge: fdb: add support for flush filtering based on ndm flags and state

[Nikolay Aleksandrov]

On 11/04/2022 11:47, Ido Schimmel wrote:
> On Sat, Apr 09, 2022 at 01:58:55PM +0300, Nikolay Aleksandrov wrote:
>> Add support for fdb flush filtering based on ndm flags and state. The
>> new attributes allow users to specify a mask and value which are mapped
>> to bridge-specific flags. NTF_USE is used to represent added_by_user
>> flag since it sets it on fdb add and we don't have a 1:1 mapping for it.
>>
>> Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org>
>> ---
>>  include/uapi/linux/if_bridge.h |  4 +++
>>  net/bridge/br_fdb.c            | 55 ++++++++++++++++++++++++++++++++++
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index 2f3799cf14b2..4638d7e39f2a 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -815,6 +815,10 @@ enum {
>>  /* embedded in BRIDGE_FLUSH_FDB */
>>  enum {
>>  	FDB_FLUSH_UNSPEC,
>> +	FDB_FLUSH_NDM_STATE,
>> +	FDB_FLUSH_NDM_STATE_MASK,
>> +	FDB_FLUSH_NDM_FLAGS,
>> +	FDB_FLUSH_NDM_FLAGS_MASK,
>>  	__FDB_FLUSH_MAX
>>  };
>>  #define FDB_FLUSH_MAX (__FDB_FLUSH_MAX - 1)
>> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
>> index 62f694a739e1..340a2ace1d5e 100644
>> --- a/net/bridge/br_fdb.c
>> +++ b/net/bridge/br_fdb.c
>> @@ -594,8 +594,40 @@ void br_fdb_flush(struct net_bridge *br,
>>  	rcu_read_unlock();
>>  }
>>  
>> +static unsigned long __ndm_state_to_fdb_flags(u16 ndm_state)
>> +{
>> +	unsigned long flags = 0;
>> +
>> +	if (ndm_state & NUD_PERMANENT)
>> +		__set_bit(BR_FDB_LOCAL, &flags);
>> +	if (ndm_state & NUD_NOARP)
>> +		__set_bit(BR_FDB_STATIC, &flags);
>> +
>> +	return flags;
>> +}
>> +
>> +static unsigned long __ndm_flags_to_fdb_flags(u16 ndm_flags)
>> +{
>> +	unsigned long flags = 0;
>> +
>> +	if (ndm_flags & NTF_USE)
>> +		__set_bit(BR_FDB_ADDED_BY_USER, &flags);
>> +	if (ndm_flags & NTF_EXT_LEARNED)
>> +		__set_bit(BR_FDB_ADDED_BY_EXT_LEARN, &flags);
>> +	if (ndm_flags & NTF_OFFLOADED)
>> +		__set_bit(BR_FDB_OFFLOADED, &flags);
>> +	if (ndm_flags & NTF_STICKY)
>> +		__set_bit(BR_FDB_STICKY, &flags);
>> +
>> +	return flags;
>> +}
>> +
>>  static const struct nla_policy br_fdb_flush_policy[FDB_FLUSH_MAX + 1] = {
>>  	[FDB_FLUSH_UNSPEC]	= { .type = NLA_REJECT },
>> +	[FDB_FLUSH_NDM_STATE]	= { .type = NLA_U16 },
>> +	[FDB_FLUSH_NDM_FLAGS]	= { .type = NLA_U16 },
>> +	[FDB_FLUSH_NDM_STATE_MASK]	= { .type = NLA_U16 },
>> +	[FDB_FLUSH_NDM_FLAGS_MASK]	= { .type = NLA_U16 },
> 
> Might be better to use NLA_POLICY_MASK(NLA_U16, mask) and reject
> unsupported states / flags instead of just ignoring them?
> 

Yep, forgot about that one. Good point!