Re: [dpdk-dev] [RFC v2] net: fix rte_vlan_insert with shared mbuf

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

On 4/4/19 7:54 PM, Stephen Hemminger wrote:
> On Sat, 30 Mar 2019 08:41:33 -0400
> Chas Williams <3chas3@gmail.com> wrote:
> 
>> Unfortunately, I think the complete fix is more complicated than this.
>> Drivers that use rte_vlan_insert don't anticipate that the mbuf might
>> change and that (hardware) transmit can fail.
>>
>> They make a copy of the mbuf pointer from the incoming transmit list
>> and don't update the original if rte_vlan_insert creates a new mbuf.
>> If transmit fails, the application needs to be the given the new mbuf
>> for re-transmission or freeing.
>>
>> On 3/28/19 4:53 PM, Stephen Hemminger wrote:
>>> If mbuf is shared then rte_vlan_insert() would clobber the original
>>> Ethernet header. The changed version handles this by getting
>>> an mbuf that will hold the new Ethernet and VLAN header followed
>>> by another mbuf (cloned) for the data.
>>>
>>> Fixes: c974021a5949 ("ether: add soft vlan encap/decap")
>>> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
> 
> The virtio driver is buggy, it saves original mbuf and doesn't handle
> rewrite. Dpaa2 is fine, should never happen since it checks refcnt etc.
> Netvsc PMD is using this on rx path and is safe.
> AF_packet PMD should work fine as well.

af_packet is broken as well. It needs to defer rte_vlan_insert until
after it gets the next incoming frame.  Otherwise the break can exit
the loop early.

> So virtio is the one that needs fixing

I agree with this.