Darrell Ball <dlu...@gmail.com> writes: > On Fri, Feb 9, 2018 at 1:35 PM, Aaron Conole <acon...@redhat.com> wrote: > > Hi Darrell, > > Darrell Ball <dlu...@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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev