Re: [ovs-dev] [RFC v6 00/11] Support multi-segment mbufs
Hi Ilya, Thanks for having look, much appreciated! On 17/05/2018 14:26, Ilya Maximets wrote: > Hello. > Thanks for working on this. But it seems that this patch > completely breaks build without dpdk. There are a lot of places > where dpdk functions in use regardless of define DPDK_NETDEV. Thanks for pointing it out. I've mistakenly left the "--with-dpdk" flag in and thought I was testing with and without DPDK. I've fixed it locally now, as it was just missing a couple of #ifdef's. I'll include it in the next iteration. > Beside that I see that you're allocating new mbufs inside generic > dp_packet_* functions. But, in context of non-PMD threads this > must be under 'non_pmd_mutex'. Otherwise, mempool could be corrupted. This makes sense, it is a consequence of handling resizing and not calling OVS_NOT_REACHED() anymore. I'll work on adding it for the next iteration. Just to note, though, that new mbufs are only being allocated in dp_packet_resize__() - but I see your point, as this function gets called by others, like dp_packet_prealloc_tailroom() and dp_packet_put_uninit(). > Best regards, Ilya Maximets. Thanks, Ilya. I don't think the fix above alone warrants sending a new version, thus I'll leave it here for a few more days for further feedback, which will be much appreciated. Also, I'm writing tests for some of the functionalities introduced. I'm planning to send that in the next iteration as well. Tiago. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC v6 00/11] Support multi-segment mbufs
Hello.
Thanks for working on this. But it seems that this patch
completely breaks build without dpdk. There are a lot of places
where dpdk functions in use regardless of define DPDK_NETDEV.
Beside that I see that you're allocating new mbufs inside generic
dp_packet_* functions. But, in context of non-PMD threads this
must be under 'non_pmd_mutex'. Otherwise, mempool could be corrupted.
Best regards, Ilya Maximets.
On 16.05.2018 20:32, Tiago Lam wrote:
> Overview
>
> This patchset introduces support for multi-segment mbufs to OvS-DPDK.
> Multi-segment mbufs are typically used when the size of an mbuf is
> insufficient to contain the entirety of a packet's data. Instead, the
> data is split across numerous mbufs, each carrying a portion, or
> 'segment', of the packet data. mbufs are chained via their 'next'
> attribute (an mbuf pointer).
>
> Use Cases
> =
> i. Handling oversized (guest-originated) frames, which are marked
> for hardware accelration/offload (TSO, for example).
>
> Packets which originate from a non-DPDK source may be marked for
> offload; as such, they may be larger than the permitted ingress
> interface's MTU, and may be stored in an oversized dp-packet. In
> order to transmit such packets over a DPDK port, their contents
> must be copied to a DPDK mbuf (via dpdk_do_tx_copy). However, in
> its current implementation, that function only copies data into
> a single mbuf; if the space available in the mbuf is exhausted,
> but not all packet data has been copied, then it is lost.
> Similarly, when cloning a DPDK mbuf, it must be considered
> whether that mbuf contains multiple segments. Both issues are
> resolved within this patchset.
>
> ii. Handling jumbo frames.
>
> While OvS already supports jumbo frames, it does so by increasing
> mbuf size, such that the entirety of a jumbo frame may be handled
> in a single mbuf. This is certainly the preferred, and most
> performant approach (and remains the default). However, it places
> high demands on system memory; multi-segment mbufs may be
> prefereable for systems which are memory-constrained.
>
> Enabling multi-segment mbufs
>
> Multi-segment and single-segment mbufs are mutually exclusive, and the
> user must decide on which approach to adopt on init. The introduction
> of a new OVSDB field, 'dpdk-multi-seg-mbufs', facilitates this.
>
> This is a global boolean value, which determines how jumbo frames are
> represented across all DPDK ports. In the absence of a user-supplied
> value, 'dpdk-multi-seg-mbufs' defaults to false, i.e. multi-segment
> mbufs must be explicitly enabled / single-segment mbufs remain the
> default.
>
> Setting the field is identical to setting existing DPDK-specific OVSDB
> fields:
>
> ovs-vsctl set Open_vSwitch . other_config:dpdk-init=true
> ovs-vsctl set Open_vSwitch . other_config:dpdk-lcore-mask=0x10
> ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-mem=4096,0
> ==> ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true
>
> ---
>
> v6: - Rebase on master 7c0cb29 ("conntrack-tcp: Handle tcp session
>reuse.");
> - Further improve dp_packet_put_uninit() and dp_packet_shift() to
>support multi-seg mbufs;
> - Add support for multi-seg mbufs in dp_packet_l4_size() and
>improve other helper funcs, such as dp_packet_tail() and dp_
>packet_tailroom().
> - Add support for multi-seg mbufs in dp_packet_put(), dp_packet_
>put_zeros(), as well as dp_packet_resize__() - allocating new
>mbufs and linking them together;
> Restructured patchset:
> - Squash patch 5 into patch 6, since they were both related to
>copying data while handling multi-seg mbufs;
> - Split patch 4 into two separate patches - one that introduces the
>changes in helper functions to deal with multi-seg mbufs and
>two others for the shift() and put_uninit() functionality;
> - Move patch 4 to before patch 3, so that ihelper functions come
>before functionality improvement that rely on those helpers.
>
> v5: - Rebased on master e5e22dc ("datapath-windows: Prevent ct-counters
> from getting redundantly incremented");
> - Sugesh's comments have been addressed:
> - Changed dp_packet_set_data() and dp_packet_set_size() logic to
> make them independent of each other;
> - Dropped patch 3 now that dp_packet_set_data() and dp_packet_set_
> size() are independent;
> - dp_packet_clone_with_headroom() now has split functions for
> handling DPDK sourced packets and non-DPDK packets;
> - Modified various functions in dp-packet.h to account for multi-seg
> mbufs - dp_packet_put_uninit(), dp_packet_tail(), dp_packet_tail()
> and dp_packet_at();
> - Added support for shifting packet data in multi-seg mbufs, using
>
[ovs-dev] [RFC v6 00/11] Support multi-segment mbufs
Overview
This patchset introduces support for multi-segment mbufs to OvS-DPDK.
Multi-segment mbufs are typically used when the size of an mbuf is
insufficient to contain the entirety of a packet's data. Instead, the
data is split across numerous mbufs, each carrying a portion, or
'segment', of the packet data. mbufs are chained via their 'next'
attribute (an mbuf pointer).
Use Cases
=
i. Handling oversized (guest-originated) frames, which are marked
for hardware accelration/offload (TSO, for example).
Packets which originate from a non-DPDK source may be marked for
offload; as such, they may be larger than the permitted ingress
interface's MTU, and may be stored in an oversized dp-packet. In
order to transmit such packets over a DPDK port, their contents
must be copied to a DPDK mbuf (via dpdk_do_tx_copy). However, in
its current implementation, that function only copies data into
a single mbuf; if the space available in the mbuf is exhausted,
but not all packet data has been copied, then it is lost.
Similarly, when cloning a DPDK mbuf, it must be considered
whether that mbuf contains multiple segments. Both issues are
resolved within this patchset.
ii. Handling jumbo frames.
While OvS already supports jumbo frames, it does so by increasing
mbuf size, such that the entirety of a jumbo frame may be handled
in a single mbuf. This is certainly the preferred, and most
performant approach (and remains the default). However, it places
high demands on system memory; multi-segment mbufs may be
prefereable for systems which are memory-constrained.
Enabling multi-segment mbufs
Multi-segment and single-segment mbufs are mutually exclusive, and the
user must decide on which approach to adopt on init. The introduction
of a new OVSDB field, 'dpdk-multi-seg-mbufs', facilitates this.
This is a global boolean value, which determines how jumbo frames are
represented across all DPDK ports. In the absence of a user-supplied
value, 'dpdk-multi-seg-mbufs' defaults to false, i.e. multi-segment
mbufs must be explicitly enabled / single-segment mbufs remain the
default.
Setting the field is identical to setting existing DPDK-specific OVSDB
fields:
ovs-vsctl set Open_vSwitch . other_config:dpdk-init=true
ovs-vsctl set Open_vSwitch . other_config:dpdk-lcore-mask=0x10
ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-mem=4096,0
==> ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true
---
v6: - Rebase on master 7c0cb29 ("conntrack-tcp: Handle tcp session
reuse.");
- Further improve dp_packet_put_uninit() and dp_packet_shift() to
support multi-seg mbufs;
- Add support for multi-seg mbufs in dp_packet_l4_size() and
improve other helper funcs, such as dp_packet_tail() and dp_
packet_tailroom().
- Add support for multi-seg mbufs in dp_packet_put(), dp_packet_
put_zeros(), as well as dp_packet_resize__() - allocating new
mbufs and linking them together;
Restructured patchset:
- Squash patch 5 into patch 6, since they were both related to
copying data while handling multi-seg mbufs;
- Split patch 4 into two separate patches - one that introduces the
changes in helper functions to deal with multi-seg mbufs and
two others for the shift() and put_uninit() functionality;
- Move patch 4 to before patch 3, so that ihelper functions come
before functionality improvement that rely on those helpers.
v5: - Rebased on master e5e22dc ("datapath-windows: Prevent ct-counters
from getting redundantly incremented");
- Sugesh's comments have been addressed:
- Changed dp_packet_set_data() and dp_packet_set_size() logic to
make them independent of each other;
- Dropped patch 3 now that dp_packet_set_data() and dp_packet_set_
size() are independent;
- dp_packet_clone_with_headroom() now has split functions for
handling DPDK sourced packets and non-DPDK packets;
- Modified various functions in dp-packet.h to account for multi-seg
mbufs - dp_packet_put_uninit(), dp_packet_tail(), dp_packet_tail()
and dp_packet_at();
- Added support for shifting packet data in multi-seg mbufs, using
dp_packet_shift();
- Fixed some minor inconsistencies.
Note that some of the changes in v5 have been contributed by Mark
Kavanagh as well.
v4: - restructure patchset
- account for 128B ARM cacheline when sizing mbufs
Mark Kavanagh (4):
netdev-dpdk: fix mbuf sizing
dp-packet: Init specific mbuf fields.
netdev-dpdk: copy large packet to multi-seg. mbufs
netdev-dpdk: support multi-segment jumbo frames
Michael Qiu (1):
dp-packet: copy data from multi-seg. DPDK mbuf
Tiago Lam (6):
dp-packet: Fix allocated size on DPDK init.
dp-packet: Fix data_len handling multi-seg mbufs.
dp-packet: Handle multi-seg mbufs in
