[ovs-dev] [patch v5 00/11] Userspace datapath: Add fragmentation support.
Aaron Conole
aconole at redhat.com
Thu Feb 22 18:24:00 UTC 2018
Darrell Ball <dlu998 at gmail.com> writes:
> On Fri, Feb 9, 2018 at 1:35 PM, Aaron Conole <aconole at redhat.com> wrote:
>
> Hi Darrell,
>
> Darrell Ball <dlu998 at gmail.com> writes:
>
> > Fragmentation support for userspace datapath conntrack is added; both
> > v4 and v6 are supported. See the patches for additional details.
>
> Very pumped about this!
>
> I went to start reviewing this, but found out that 04/ didn't apply
> correctly. No problem, I'll proceed - just figured I'd let you know
> that:
>
> The merge conflict in NEWS was due to 2 subsequent applies on Feb 6, while
> the series was posted on Feb 4.
Yep. I've worked around it.
> 1. I'm looking at it :)
>
> Thanks Aaron
I have a question on patches 5-7:
Do you think it's worthwhile having these as configuration options in
the database?
I can see where it could be considered that they aren't appropriate
(after all, the linux fragmentation support is enabled/disabled not via
the netlink interface, but by the system administrator outside of the
ovs configuration). On the other hand, from a user perspective it's
probably more friendly to have that configuration knob be persistent.
I think there is value in having the configuration for ipfrag be stored
in a database so that each 'restart' doesn't require an extra set of
commands be run. I envision a situation where users have their system
configured, restart, and get confused because they have to manually
re-enable the configuration. There are a few ways this could be done
(for instance, a post-start script that runs after ovs-ctl), but I think
the database might be a useful way of accomplishing that goal - it
exists and we use it for dpdk configurations, ex.
Something to think on.
I'm planning on running performance tests this coming week, and will
give my patch 3/11 comments then.
Thanks for working on this, Darrell!
> Darrell
>
>
> 2. it'll need at least one more spin (but no need to rush that since
> I'll move myself past the error)
>
> Thanks,
> -Aaron
>
> > v4->v5: Added a sub-feature to optionally dump fragmentation lists.
> > This is useful for DOS forensics and debugging.
> >
> > The testing coverage was also extended including checking
> > more counters and frag list occupancies.
> >
> > Fixed a few bugs:
> > 1/ Handle dpdk mempool source restrictions for a batch of
> > packets from multiple sources; this also brings in a purge
> > frag list function to handle pathological cases of stuck frags.
> > 2/ ipf_destroy was missing packet frees for frag lists.
> > 3/ A setting of CS_INVALID was missing for expired packets -
> > I mentioned this earlier for version 4.
> >
> > Some enhancements and coding standards changes for Patch 3.
> >
> > v3->v4: Add V6 support to the patches.
> > Fix possible race cleanup bug when the user disables
> > fragmentation and there are list occupancies, not cleaned up
> > yet.
> > Add missed orig tuple fields for copy from reassembled packet
> > to fragments.
> > Fix an fragment list increment check - shoiuld have been "> 0"
> > rather then "!= 0".
> > Fix max frags calculation in case of theoretical corner case.
> > Add proper lock annotations.
> > Made some other improvements while adding V6 support.
> >
> > v2->v3: Patch 2 was updated:
> > Remove "XXX" todo items by implementing the ones needed,
> > including realloc frag_list contexts to save memory.
> > Fix related bug with max_frag_list_size when min_frag_size is
> > reconfigured.
> >
> > Tighten ip_tot_len sanity check for reassembled packets which
> > was more loose than intended.
> >
> > Add another sanity check for fragment ip_tot_len; even though
> > it be redundant, add for completeness.
> >
> > v1->v2: Few fixes, improvements and cleanups.
> >
> > Darrell Ball (11):
> > dp-packet: Add const qualifiers for checksum apis.
> > flow: Enhance parse_ipv6_ext_hdrs.
> > Userspace datapath: Add fragmentation handling.
> > conntrack: Support fragmentation.
> > ipf: Add command to enable fragmentation handling.
> > ipf: Add set minimum fragment size command.
> > ipf: Add set maximum fragments supported command.
> > ipf: Add command to get fragmentation handling status.
> > ipf: Enhance ipf_get_status.
> > tests: Add missed local stack checks.
> > tests: Enable fragmentation for userspace datapath.
> >
> > NEWS | 10 +
> > include/sparse/netinet/ip6.h | 1 +
> > lib/automake.mk | 2 +
> > lib/conntrack.c | 10 +-
> > lib/ct-dpif.c | 69 ++
> > lib/ct-dpif.h | 13 +
> > lib/dp-packet.h | 4 +-
> > lib/dpctl.c | 216 ++++++
> > lib/dpctl.man | 32 +
> > lib/dpif-netdev.c | 83 +++
> > lib/dpif-netlink.c | 7 +
> > lib/dpif-provider.h | 18 +
> > lib/flow.c | 23 +-
> > lib/flow.h | 3 +-
> > lib/ipf.c | 1390 ++++++++++++++++++++++++++++++++++++++
> > lib/ipf.h | 86 +++
> > tests/system-kmod-macros.at | 30 +-
> > tests/system-traffic.at | 45 +-
> > tests/system-userspace-macros.at | 125 +++-
> > 19 files changed, 2129 insertions(+), 38 deletions(-)
> > create mode 100644 lib/ipf.c
> > create mode 100644 lib/ipf.h
More information about the dev
mailing list