[ovs-dev] [RFC] Don't resize DPBUF_DPDK packets.

Tiago Lam tiago.lam at intel.com
Fri Jul 20 18:01:42 UTC 2018

This RFC is a proposal to avoid the manual assert in dp_packet_resize__() for
DPBUF_DPDK packets. It has been reported before, in [1], as something that can
lead to crashes.

Overall I can see two approaches to this problem:
1. Modify dp_packet's API entirely to deal with errors and report them to
   callers appropriately;
2. For DPDK packets only, check a priori if the prealloc_tailroom() and
   prealloc_headroom() operations are possible or not (the only callers of
   dp_packet_resize__(), which leads to the manual assert).

Approach 1. would lead to a fair amount of change without giving much in return
when using non-DPDK packets (as DPBUF_MALLOC). For example, any packet created
with dp_packet_new() / dp_packet_init() / dp_packet_use_stub(), or even cloned
(a DPBUF_DPDK is cloned into a DPBUF_MALLOC, for example), wouldn't have the
mentioned limitations.

Instead, this approach follows 2. and modifies dp-packet.c so that
dp_packet_put_uninit() and dp_packet_push_uninit() don't call neither
dp_packet_prealloc_tailroom() nor dp_packet_prealloc_headroom() for DPBUF_DPDK
packets. Instead, they check if there's enough space beforehand, and if not
return NULL, instead of the normal pointer to the data. Otherwise the
operations would continue as expected.

This means we would need to deal with the NULL case only where we may be
using DPBUF_DPDK packets (all the other cases remain unaffected as NULL
won't be possible there). The cases I've identified it happens is where
headers are being pushed (which seems to be what's causing the crashes
in [2] as well):
- In eth_push_vlan(), push_eth(), push_mpls() and push_nsh(), which get
  called from odp_execute_actions();
- In netdev_tnl_push_ip_header(), which gets called by the native
  tunnels implementations, such as netdev_gre_push_header() for GRE, that
  is ultimately called by push_tnl_action().

I haven't found a case where dp_packet_put_uninit() is actually used in
a DPBUF_DPDK packet. In all cases it seems to be a result of a packet
created locally by using dp_packet_new() or some variant.

The system-userspace-testsuite tests have been run successfully with theses
changes, but no other tests have been performed.

[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-May/346649.html

Tiago Lam (1):
  dp-packet: Don't resize DPBUF_DPDK packets.

 lib/dp-packet.c         | 126 +++++++++++++++++++++++++++++++++++++++++-------
 lib/netdev-native-tnl.c |  24 +++++++--
 lib/netdev-native-tnl.h |   6 +--
 lib/netdev-provider.h   |   2 +-
 lib/netdev.c            |  16 ++++--
 lib/odp-execute.c       |  49 +++++++++++++++----
 lib/packets.c           |  38 +++++++++++++--
 lib/packets.h           |   8 +--
 8 files changed, 224 insertions(+), 45 deletions(-)


More information about the dev mailing list