[ovs-dev] Resize of DPDK packets (was Re: [PATCH v6 00/13] Support multi-segment mbufs)

Lam, Tiago tiago.lam at intel.com
Wed Jul 25 16:48:18 UTC 2018


On 24/07/2018 17:55, Ilya Maximets wrote:
> Hi.
> Just wanted to add some comments for the use-cases and testing methodology.
> See inline.
>> And I'm actually not sure if there any profit from this patch set?
> It looks like an internal mbuf handling rework that only degrades
> the performance and complicates the code.
> 
> Please, don't consider me as merge blocker. I just want to understand
> why you think we need this 1200 LOCs?
> 
> ---
> About 'resize()' related discussion:
> Maybe it's worth to allow dp_packet APIs to return different dp_packet.
> In this case we'll be able to just clone the packet to malloced memory
> and resize in cases of not enough headroom available.
> Like:
> 	packet = eth_push_vlan(packet, vlan->vlan_tpid, vlan->vlan_tci);
> or
> 	eth_push_vlan(&packet, vlan->vlan_tpid, vlan->vlan_tci);
> 
> This will have a little performance penalty in compare with data shifting
> inside the mbuf, but will be much more elegant, and will allow to eliminate
> all the OVS_NOT_REACHED cases.
> 

Thanks for picking up the discussion.

To me, it comes down to, is 128B not enough space for headers? Given the
VXLAN example, it would be enough to push two headers. Is this too much
of a constraint? What would be a good constraint?

If we plan to support as many headers as the system's memory allows then
we won't have much choice, and this option of malloc'ing from the system
and copying to it might be the only one. My concerns would be on the
performance penalty (as you mention), but also on users seeing weird
performance benefits when using 2 VXLAN headers vs using 3 (for
example). It would be yet another question to ask when debugging a
system (how many headers are you pushing?)

I see benefits on having a hard limit as well, and not allowing anything
else to be pushed above those 128B, but since the 128B are set on DPDK
at compile time, it seems this is calling for trouble in the future as
if that changes between versions, we might start supporting less
headers. In this case both this approach or the mbuf shifting would
provide more reliability against future changes in DPDK's
RTE_PKTMBUF_HEADROOM.

Also, this is a bit aside, but ideally I would like to keep this
discussion out of this patch series as I think this is creating
confusion - this is an issue aside of the multi-segment work, it exists
already for single mbufs DPDK packets. There's an RFC, "Don't resize
DPBUF_DPDK packets.", which should be the ideal place.


More information about the dev mailing list