linux-kernel - Re: [Patch net-next 1/3] net: bridge: mcast: Add offload failed mdb flag

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c90151bc-a529-4f4e-a0b9-5831a6b803f7@blackwall.org>
Date: Fri, 21 Mar 2025 10:19:42 +0200
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Joseph Huang <Joseph.Huang@...min.com>, netdev@...r.kernel.org
Cc: Joseph Huang <joseph.huang.2024@...il.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Roopa Prabhu <roopa@...dia.com>, Simon Horman <horms@...nel.org>,
 linux-kernel@...r.kernel.org, bridge@...ts.linux.dev
Subject: Re: [Patch net-next 1/3] net: bridge: mcast: Add offload failed mdb
 flag

On 3/19/25 00:42, Joseph Huang wrote:
> Add MDB_FLAGS_OFFLOAD_FAILED and MDB_PG_FLAGS_OFFLOAD_FAILED to indicate
> that an attempt to offload the MDB entry to switchdev has failed.
> 
> Signed-off-by: Joseph Huang <Joseph.Huang@...min.com>
> ---
>  include/uapi/linux/if_bridge.h |  9 +++++----
>  net/bridge/br_mdb.c            |  2 ++
>  net/bridge/br_private.h        | 11 ++++++-----
>  net/bridge/br_switchdev.c      | 10 +++++-----
>  4 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index a5b743a2f775..f2a6de424f3f 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -699,10 +699,11 @@ struct br_mdb_entry {
>  #define MDB_TEMPORARY 0
>  #define MDB_PERMANENT 1
>  	__u8 state;
> -#define MDB_FLAGS_OFFLOAD	(1 << 0)
> -#define MDB_FLAGS_FAST_LEAVE	(1 << 1)
> -#define MDB_FLAGS_STAR_EXCL	(1 << 2)
> -#define MDB_FLAGS_BLOCKED	(1 << 3)
> +#define MDB_FLAGS_OFFLOAD		(1 << 0)
> +#define MDB_FLAGS_FAST_LEAVE		(1 << 1)
> +#define MDB_FLAGS_STAR_EXCL		(1 << 2)
> +#define MDB_FLAGS_BLOCKED		(1 << 3)
> +#define MDB_FLAGS_OFFLOAD_FAILED	(1 << 4)
>  	__u8 flags;
>  	__u16 vid;
>  	struct {
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index 1a52a0bca086..0639691cd19b 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -144,6 +144,8 @@ static void __mdb_entry_fill_flags(struct br_mdb_entry *e, unsigned char flags)
>  		e->flags |= MDB_FLAGS_STAR_EXCL;
>  	if (flags & MDB_PG_FLAGS_BLOCKED)
>  		e->flags |= MDB_FLAGS_BLOCKED;
> +	if (flags & MDB_PG_FLAGS_OFFLOAD_FAILED)
> +		e->flags |= MDB_FLAGS_OFFLOAD_FAILED;
>  }
>  
>  static void __mdb_entry_to_br_ip(struct br_mdb_entry *entry, struct br_ip *ip,
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 1054b8a88edc..cd6b4e91e7d6 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -306,11 +306,12 @@ struct net_bridge_fdb_flush_desc {
>  	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)
> -#define MDB_PG_FLAGS_STAR_EXCL	BIT(3)
> -#define MDB_PG_FLAGS_BLOCKED	BIT(4)
> +#define MDB_PG_FLAGS_PERMANENT		BIT(0)
> +#define MDB_PG_FLAGS_OFFLOAD		BIT(1)
> +#define MDB_PG_FLAGS_FAST_LEAVE		BIT(2)
> +#define MDB_PG_FLAGS_STAR_EXCL		BIT(3)
> +#define MDB_PG_FLAGS_BLOCKED		BIT(4)
> +#define MDB_PG_FLAGS_OFFLOAD_FAILED	BIT(5)
>  
>  #define PG_SRC_ENT_LIMIT	32
>  
> diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
> index 7b41ee8740cb..68dccc2ff7b1 100644
> --- a/net/bridge/br_switchdev.c
> +++ b/net/bridge/br_switchdev.c
> @@ -505,9 +505,6 @@ static void br_switchdev_mdb_complete(struct net_device *dev, int err, void *pri
>  	struct net_bridge_port *port = data->port;
>  	struct net_bridge *br = port->br;
>  
> -	if (err)
> -		goto err;
> -
>  	spin_lock_bh(&br->multicast_lock);
>  	mp = br_mdb_ip_get(br, &data->ip);
>  	if (!mp)
> @@ -516,11 +513,14 @@ static void br_switchdev_mdb_complete(struct net_device *dev, int err, void *pri
>  	     pp = &p->next) {
>  		if (p->key.port != port)
>  			continue;
> -		p->flags |= MDB_PG_FLAGS_OFFLOAD;
> +
> +		if (err)
> +			p->flags |= MDB_PG_FLAGS_OFFLOAD_FAILED;
> +		else
> +			p->flags |= MDB_PG_FLAGS_OFFLOAD;

These two should be mutually exclusive, either it's offloaded or it failed an offload,
shouldn't be possible to have both set. I'd recommend adding some helper that takes
care of that.

>  	}
>  out:
>  	spin_unlock_bh(&br->multicast_lock);
> -err:
>  	kfree(priv);
>  }
>