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

[Nikolay Aleksandrov <razor@blackwall.org>]

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