Re: [PATCH net v2 2/2] 8021q: delete cleared egress QoS mappings

[Simon Horman <horms@kernel.org>]

On Mon, Apr 20, 2026 at 11:18:46AM +0800, Ren Wei wrote:
> From: Longxuan Yu <ylong030@ucr.edu>
> 
> vlan_dev_set_egress_priority() currently keeps cleared egress
> priority mappings in the hash as tombstones. Repeated set/clear cycles
> with distinct skb priorities therefore accumulate mapping nodes until
> device teardown and leak memory.
> 
> Delete mappings when vlan_prio is cleared instead of keeping tombstones.
> Now that the egress mapping lists are RCU protected, the node can be
> unlinked safely and freed after a grace period.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@kernel.org
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Co-developed-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Yuan Tan <yuantan098@gmail.com>
> Signed-off-by: Longxuan Yu <ylong030@ucr.edu>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
>  net/8021q/vlan_dev.c     | 20 ++++++++++++++------
>  net/8021q/vlan_netlink.c |  4 ----
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index a5340932b657..7aa3af8b10ea 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -172,26 +172,34 @@ int vlan_dev_set_egress_priority(const struct net_device *dev,
>  				 u32 skb_prio, u16 vlan_prio)
>  {
>  	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
> +	struct vlan_priority_tci_mapping __rcu **mpp;
>  	struct vlan_priority_tci_mapping *mp;
>  	struct vlan_priority_tci_mapping *np;
>  	u32 bucket = skb_prio & 0xF;
>  	u32 vlan_qos = (vlan_prio << VLAN_PRIO_SHIFT) & VLAN_PRIO_MASK;
>  
>  	/* See if a priority mapping exists.. */
> -	mp = rtnl_dereference(vlan->egress_priority_map[bucket]);
> +	mpp = &vlan->egress_priority_map[bucket];
> +	mp = rtnl_dereference(*mpp);
>  	while (mp) {
>  		if (mp->priority == skb_prio) {
> -			if (mp->vlan_qos && !vlan_qos)
> +			if (!vlan_qos) {
> +				rcu_assign_pointer(*mpp, rtnl_dereference(mp->next));
>  				vlan->nr_egress_mappings--;
> -			else if (!mp->vlan_qos && vlan_qos)
> -				vlan->nr_egress_mappings++;
> -			WRITE_ONCE(mp->vlan_qos, vlan_qos);
> +				kfree_rcu(mp, rcu);
> +			} else {
> +				WRITE_ONCE(mp->vlan_qos, vlan_qos);
> +			}
>  			return 0;
>  		}
> -		mp = rtnl_dereference(mp->next);
> +		mpp = &mp->next;
> +		mp = rtnl_dereference(*mpp);
>  	}

Hi Ren,

Thanks for splitting up the patchset, it is very helpful to me.

It seems to me that the mpp/mp construct used is a bit complex and
stems from the use of a hand-rolled list centred the next field of
struct vlan_priority_tci_mapping.

I wonder if things can be simplified by moving to use a standardised
list construct, such as an hlist. And the helpers available for using it.

>  
>  	/* Create a new mapping then. */
> +	if (!vlan_qos)
> +		return 0;
> +
>  	np = kmalloc_obj(struct vlan_priority_tci_mapping);
>  	if (!np)
>  		return -ENOBUFS;

...