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

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

On 4/16/19 12:28 PM, Bruce Richardson wrote:
> On Tue, Apr 16, 2019 at 04:51:26PM +0100, Ferruh Yigit wrote:
>> The vlan_insert() is buggy when it tires to handle the shared mbufs,
>> 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(-)
>>
> So what is the API to be used if one does want to insert a vlan tag into a
> shared mbuf?

It's unlikely you would ever want to do that.  Have one thread perform
some operation on the mbuf and other threads would expect this to have
happened? It seems counter to the way that packets might flow through an
application. Typically, you would insert the vlan and then share
the mbuf. Modifying a shared mbuf should make you ask, what are the
other copies expecting?

> Also, why is it such a problem to create new copies of data inside the
> driver if that is necessary? You create a copy and use that, freeing the
> original (i.e. in all likelyhood decrememting the ref-count since you no
> longer use it). You already have the pointer to the mbuf pool from the
> original buffer so you can get a copy from the same place. I'm curious to
> know why it would be impossible to do a functionally correct
> implementation?

It is not an issue to do this correctly. Hemminger did submit a patch
that appeared to do this correctly (I haven't tested it). As mentioned
earlier the tricky part is returning the buffer to the application. If
you create a copy and transmit fails, you need to free that buffer or
return it to the application for it to free. If you free the buffer when
making a buffer, you certainly can't return it to the application for
it to be freed a second time.

> /Bruce
>