[RFC v3 iproute2-next 0/2] Add mdb offload failure notification

[RFC v3 iproute2-next 0/2] Add mdb offload failure notification

[Joseph Huang]

Add support to handle mdb offload failure notifications.

The link to kernel changes:
https://lore.kernel.org/netdev/20250404212940.1837879-1-Joseph.Huang@garmin.com/

Joseph Huang (2):
  bridge: mdb: Support offload failed flag
  iplink_bridge: Add mdb_offload_fail_notification

 bridge/mdb.c          |  2 ++
 ip/iplink_bridge.c    | 19 +++++++++++++++++++
 man/man8/ip-link.8.in |  7 +++++++
 3 files changed, 28 insertions(+)

---
v1: https://lore.kernel.org/netdev/20250318225026.145501-1-Joseph.Huang@garmin.com/
v2: https://lore.kernel.org/netdev/20250403235452.1534269-1-Joseph.Huang@garmin.com/
    Change multi-valued option mdb_notify_on_flag_change to bool option
    mdb_offload_fail_notification
v3: Patch 2/2 Use strcmp instead of matches

-- 
2.49.0

[RFC v3 iproute2-next 1/2] bridge: mdb: Support offload failed flag

[Joseph Huang]

Add support for the MDB_FLAGS_OFFLOAD_FAILED flag to indicate that
an attempt to offload an mdb entry to switchdev has failed.

Signed-off-by: Joseph Huang <Joseph.Huang@garmin.com>
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>
---
 bridge/mdb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bridge/mdb.c b/bridge/mdb.c
index 196363a5..72490971 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -256,6 +256,8 @@ static void print_mdb_entry(FILE *f, int ifindex, const struct br_mdb_entry *e,
 		print_string(PRINT_ANY, NULL, " %s", "added_by_star_ex");
 	if (e->flags & MDB_FLAGS_BLOCKED)
 		print_string(PRINT_ANY, NULL, " %s", "blocked");
+	if (e->flags & MDB_FLAGS_OFFLOAD_FAILED)
+		print_string(PRINT_ANY, NULL, " %s", "offload_failed");
 	close_json_array(PRINT_JSON, NULL);
 
 	if (e->vid)
-- 
2.49.0

[RFC v3 iproute2-next 2/2] iplink_bridge: Add mdb_offload_fail_notification

[Joseph Huang]

Add mdb_offload_fail_notification option support.

Signed-off-by: Joseph Huang <Joseph.Huang@garmin.com>
---
 ip/iplink_bridge.c    | 19 +++++++++++++++++++
 man/man8/ip-link.8.in |  7 +++++++
 2 files changed, 26 insertions(+)

diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
index 1fe89551..c730aa68 100644
--- a/ip/iplink_bridge.c
+++ b/ip/iplink_bridge.c
@@ -62,6 +62,7 @@ static void print_explain(FILE *f)
 		"		  [ nf_call_iptables NF_CALL_IPTABLES ]\n"
 		"		  [ nf_call_ip6tables NF_CALL_IP6TABLES ]\n"
 		"		  [ nf_call_arptables NF_CALL_ARPTABLES ]\n"
+		"		  [ mdb_offload_fail_notification MDB_OFFLOAD_FAIL_NOTIFICATION ]\n"
 		"\n"
 		"Where: VLAN_PROTOCOL := { 802.1Q | 802.1ad }\n"
 	);
@@ -413,6 +414,18 @@ static int bridge_parse_opt(struct link_util *lu, int argc, char **argv,
 
 			addattr8(n, 1024, IFLA_BR_NF_CALL_ARPTABLES,
 				 nf_call_arpt);
+		} else if (strcmp(*argv, "mdb_offload_fail_notification") == 0) {
+			__u32 mofn_bit = 1 << BR_BOOLOPT_MDB_OFFLOAD_FAIL_NOTIFICATION;
+			__u8 mofn;
+
+			NEXT_ARG();
+			if (get_u8(&mofn, *argv, 0))
+				invarg("invalid mdb_offload_fail_notification", *argv);
+			bm.optmask |= 1 << BR_BOOLOPT_MDB_OFFLOAD_FAIL_NOTIFICATION;
+			if (mofn)
+				bm.optval |= mofn_bit;
+			else
+				bm.optval &= ~mofn_bit;
 		} else if (matches(*argv, "help") == 0) {
 			explain();
 			return -1;
@@ -623,6 +636,7 @@ static void bridge_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		__u32 mcvl_bit = 1 << BR_BOOLOPT_MCAST_VLAN_SNOOPING;
 		__u32 no_ll_learn_bit = 1 << BR_BOOLOPT_NO_LL_LEARN;
 		__u32 mst_bit = 1 << BR_BOOLOPT_MST_ENABLE;
+		__u32 mofn_bit = 1 << BR_BOOLOPT_MDB_OFFLOAD_FAIL_NOTIFICATION;
 		struct br_boolopt_multi *bm;
 
 		bm = RTA_DATA(tb[IFLA_BR_MULTI_BOOLOPT]);
@@ -641,6 +655,11 @@ static void bridge_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 				   "mst_enabled",
 				   "mst_enabled %u ",
 				   !!(bm->optval & mst_bit));
+		if (bm->optmask & mofn_bit)
+			print_uint(PRINT_ANY,
+				   "mdb_offload_fail_notification",
+				   "mdb_offload_fail_notification %u ",
+				   !!(bm->optval & mofn_bit));
 	}
 
 	if (tb[IFLA_BR_MCAST_ROUTER])
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index efb62481..3a7d1045 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -1753,6 +1753,8 @@ the following additional arguments are supported:
 .BI nf_call_ip6tables " NF_CALL_IP6TABLES "
 ] [
 .BI nf_call_arptables " NF_CALL_ARPTABLES "
+] [
+.BI mdb_offload_fail_notification " MDB_OFFLOAD_FAIL_NOTIFICATION "
 ]
 
 .in +8
@@ -1977,6 +1979,11 @@ or disable
 .RI ( NF_CALL_ARPTABLES " == 0) "
 arptables hooks on the bridge.
 
+.BI mdb_offload_fail_notification " MDB_OFFLOAD_FAIL_NOTIFICATION "
+- turn mdb offload fail notification on
+.RI ( MDB_OFFLOAD_FAIL_NOTIFICATION " > 0) "
+or off
+.RI ( MDB_OFFLOAD_FAIL_NOTIFICATION " == 0). "
 
 .in -8
 
-- 
2.49.0

Re: [RFC v3 iproute2-next 0/2] Add mdb offload failure notification

[Nikolay Aleksandrov]

On 4/5/25 00:53, Joseph Huang wrote:
> Add support to handle mdb offload failure notifications.
> 
> The link to kernel changes:
> https://lore.kernel.org/netdev/20250404212940.1837879-1-Joseph.Huang@garmin.com/
> 
> Joseph Huang (2):
>   bridge: mdb: Support offload failed flag
>   iplink_bridge: Add mdb_offload_fail_notification
> 
>  bridge/mdb.c          |  2 ++
>  ip/iplink_bridge.c    | 19 +++++++++++++++++++
>  man/man8/ip-link.8.in |  7 +++++++
>  3 files changed, 28 insertions(+)
> 
> ---
> v1: https://lore.kernel.org/netdev/20250318225026.145501-1-Joseph.Huang@garmin.com/
> v2: https://lore.kernel.org/netdev/20250403235452.1534269-1-Joseph.Huang@garmin.com/
>     Change multi-valued option mdb_notify_on_flag_change to bool option
>     mdb_offload_fail_notification
> v3: Patch 2/2 Use strcmp instead of matches
> 

Somehow you've CCed me only on patch 1. :)

Re: [RFC v3 iproute2-next 2/2] iplink_bridge: Add mdb_offload_fail_notification

[Nikolay Aleksandrov]

On 4/5/25 00:53, Joseph Huang wrote:
> Add mdb_offload_fail_notification option support.
> 
> Signed-off-by: Joseph Huang <Joseph.Huang@garmin.com>
> ---
>  ip/iplink_bridge.c    | 19 +++++++++++++++++++
>  man/man8/ip-link.8.in |  7 +++++++
>  2 files changed, 26 insertions(+)
> 

Sorry, but a few more things I just noticed below,

> diff --git a/ip/iplink_bridge.c b/ip/iplink_bridge.c
> index 1fe89551..c730aa68 100644
> --- a/ip/iplink_bridge.c
> +++ b/ip/iplink_bridge.c
> @@ -62,6 +62,7 @@ static void print_explain(FILE *f)
>  		"		  [ nf_call_iptables NF_CALL_IPTABLES ]\n"
>  		"		  [ nf_call_ip6tables NF_CALL_IP6TABLES ]\n"
>  		"		  [ nf_call_arptables NF_CALL_ARPTABLES ]\n"
> +		"		  [ mdb_offload_fail_notification MDB_OFFLOAD_FAIL_NOTIFICATION ]\n"
>  		"\n"
>  		"Where: VLAN_PROTOCOL := { 802.1Q | 802.1ad }\n"
>  	);
> @@ -413,6 +414,18 @@ static int bridge_parse_opt(struct link_util *lu, int argc, char **argv,
>  
>  			addattr8(n, 1024, IFLA_BR_NF_CALL_ARPTABLES,
>  				 nf_call_arpt);
> +		} else if (strcmp(*argv, "mdb_offload_fail_notification") == 0) {
> +			__u32 mofn_bit = 1 << BR_BOOLOPT_MDB_OFFLOAD_FAIL_NOTIFICATION;
> +			__u8 mofn;
> +
> +			NEXT_ARG();
> +			if (get_u8(&mofn, *argv, 0))
> +				invarg("invalid mdb_offload_fail_notification", *argv);
> +			bm.optmask |= 1 << BR_BOOLOPT_MDB_OFFLOAD_FAIL_NOTIFICATION;
> +			if (mofn)
> +				bm.optval |= mofn_bit;
> +			else
> +				bm.optval &= ~mofn_bit;
>  		} else if (matches(*argv, "help") == 0) {
>  			explain();
>  			return -1;
> @@ -623,6 +636,7 @@ static void bridge_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
>  		__u32 mcvl_bit = 1 << BR_BOOLOPT_MCAST_VLAN_SNOOPING;
>  		__u32 no_ll_learn_bit = 1 << BR_BOOLOPT_NO_LL_LEARN;
>  		__u32 mst_bit = 1 << BR_BOOLOPT_MST_ENABLE;
> +		__u32 mofn_bit = 1 << BR_BOOLOPT_MDB_OFFLOAD_FAIL_NOTIFICATION;

Please keep this arranged in reverse xmas tree, i.e. longest to shortest line.

>  		struct br_boolopt_multi *bm;
>  
>  		bm = RTA_DATA(tb[IFLA_BR_MULTI_BOOLOPT]);
> @@ -641,6 +655,11 @@ static void bridge_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
>  				   "mst_enabled",
>  				   "mst_enabled %u ",
>  				   !!(bm->optval & mst_bit));
> +		if (bm->optmask & mofn_bit)
> +			print_uint(PRINT_ANY,
> +				   "mdb_offload_fail_notification",
> +				   "mdb_offload_fail_notification %u ",
> +				   !!(bm->optval & mofn_bit));
>  	}
>  
>  	if (tb[IFLA_BR_MCAST_ROUTER])
> diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
> index efb62481..3a7d1045 100644
> --- a/man/man8/ip-link.8.in
> +++ b/man/man8/ip-link.8.in
> @@ -1753,6 +1753,8 @@ the following additional arguments are supported:
>  .BI nf_call_ip6tables " NF_CALL_IP6TABLES "
>  ] [
>  .BI nf_call_arptables " NF_CALL_ARPTABLES "
> +] [
> +.BI mdb_offload_fail_notification " MDB_OFFLOAD_FAIL_NOTIFICATION "
>  ]
>  
>  .in +8
> @@ -1977,6 +1979,11 @@ or disable
>  .RI ( NF_CALL_ARPTABLES " == 0) "
>  arptables hooks on the bridge.
>  
> +.BI mdb_offload_fail_notification " MDB_OFFLOAD_FAIL_NOTIFICATION "
> +- turn mdb offload fail notification on
> +.RI ( MDB_OFFLOAD_FAIL_NOTIFICATION " > 0) "
> +or off
> +.RI ( MDB_OFFLOAD_FAIL_NOTIFICATION " == 0). "


Please add also what is the default value.
Other than that the patches look good and you can drop the RFC.

Thanks,
 Nik