Re: [dpdk-dev] [PATCH] net: do not insert VLAN tag to shared mbufs

[Chas Williams <3chas3@gmail.com>]

On 4/16/19 11:51 AM, Ferruh Yigit wrote:
> The vlan_insert() is buggy when it tires to handle the shared mbufs,

s/tries/tries/

> instead don't support inserting VLAN tag into shared mbufs and return
> an error for that case.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Cc: Chas Williams <chas3@att.com>
> 
> This is another approach to RFC to fix the vlan_insert:
> https://patches.dpdk.org/patch/51870/
> 
> vlan_insert() mostly used by drivers to insert VLAN tag into packet
> data in Tx path, drivers creating new copies of mbufs in Tx path may
> result unexpected behavior, like not freed or double freed mbufs.
> ---
>   lib/librte_net/rte_ether.h | 11 ++---------
>   1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> index 3a87ff184..a1df911b6 100644
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -388,15 +388,8 @@ static inline int rte_vlan_insert(struct rte_mbuf **m)
>   	struct vlan_hdr *vh;
>   
>   	/* Can't insert header if mbuf is shared */
> -	if (rte_mbuf_refcnt_read(*m) > 1) {
> -		struct rte_mbuf *copy;
> -
> -		copy = rte_pktmbuf_clone(*m, (*m)->pool);
> -		if (unlikely(copy == NULL))
> -			return -ENOMEM;
> -		rte_pktmbuf_free(*m);
> -		*m = copy;
> -	}
> +	if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
> +		return -EINVAL;
>   
>   	oh = rte_pktmbuf_mtod(*m, struct ether_hdr *);
>   	nh = (struct ether_hdr *)
>