[ovs-dev] [PATCH v2 00/15] Userspace (DPDK) connection tracker

[Fischetti, Antonio]

Hi Daniele,
I just started to have a look to your new v2 patch set.
A minor comment - I know this is a bit of nit-picking - but I ran
utilities/checkpatch.py and I got some output like

patch 4/15
W(1692): Line has trailing whitespace
+static inline void ct_lock_lock(struct ct_lock *lock)

patch 5/15
W(166): Line is greater than 79-characters long
+        threads[i].thread = ovs_thread_create("ct_thread", ct_thread_main, &threads[i]);

W(191): Line is greater than 79-characters long
+    {"benchmark", "n_threads n_pkts batch_size [change_connection]", 3, 4, test_benchmark},

patch 6/15
W(52): Line is greater than 79-characters long
+            ovs_fatal(0, "batch_size must be between 1 and NETDEV_MAX_BURST(%u)",

patches 7/15 and 8/15, 10/15, 11/15 - as there's no comment into the patch, maybe 
they just need an empty line before 
“Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>”?
The output is
E: No signatures found.
E: Too many signoffs; are you missing Co-authored-by lines?

I'll go on and have a look into the code, I hope I provide some useful feedback.

Thanks,
Antonio

> -----Original Message-----
> From: dev [mailto:dev-bounces at openvswitch.org] On Behalf Of Daniele Di
> Proietto
> Sent: Saturday, April 16, 2016 1:03 AM
> To: dev at openvswitch.org
> Subject: [ovs-dev] [PATCH v2 00/15] Userspace (DPDK) connection tracker
> 
> This series aims to implement the ct() action for the dpif-netdev datapath.
> The bulk of the code is in the new conntrack module: it contains some packet
> parsing code, some lookup tables and the logic to implements all the ct bits.
> 
> The conntrack module is helped by conntrack-tcp, for TCP window and flags
> tracking: the bulk of the code of this submodule is from the FreeBSD's pf
> subsystem, therefore is BSD licensed.
> 
> The rest of the series integrates the connection tracker with the rest of
> OVS: the ct() action is implemented in dpif-netdev, and the debugging
> interfaces required by dpctl/{dump,flush}-conntrack are implemented.
> 
> Besides adding some unit tests, this series ports the existing conntrack
> system test to the userspace datapath.  Some small modifications are
> required to pass the testsuite, and some tests still have to be skipped.
> 
> On newer kernels the userspace testsuite has some problems with offloads,
> so a workaround is included.
> 
> This can also be downloaded at:
> 
> https://github.com/ddiproietto/ovs/tree/userconntrack_20160415
> 
> Any feedback is appreciated, thanks.
> 
> v1 -> v2:
> * Fixed bug in tcp_get_wscale(), related to TCP options parsing.
> * Changed names of ICMP constants: now they're different from Linux and
>   FreeBSD.
> * Fixed bug in parse_ipv6_ext_hdrs().
> * Used ALWAYS_INLINE in parse_vlan and parse_ethertype, to avoid a
>   performance regression in miniflow_extract().
> * Updated copyright info in COPYING and debian/copyright.in.
> * Rebased.
> * Changed batching strategy in conntrack_execute() to allow a newly
>   created connection to be picked up by packets in the same batch.
> * Added an ovs-test module to throw pcap files at the connection tracker.
> * Added a workaround for the userspace testsuite on new kernels and a tcp
>   non-conntrack test.
> 
> Daniele Di Proietto (15):
>   packets: Define ICMP types.
>   flow: Export parse_ipv6_ext_hdrs().
>   flow: Introduce parse_dl_type().
>   conntrack: New userspace connection tracker.
>   tests: Add very simple conntrack benchmark.
>   tests: Add test-conntrack pcap test.
>   conntrack: Implement flush function.
>   conntrack: Implement dumping to ct_entry.
>   dpif-netdev: Execute conntrack action.
>   dpif-netdev: Implement conntrack dump functions.
>   dpif-netdev: Implement conntrack flush interface.
>   tests: Add conntrack ofproto-dpif tests.
>   system-tests: Disable offloads in userspace tests.
>   system-tests: Add tcp simple test.
>   system-tests: Run conntrack tests with userspace
> 
>  COPYING                          |   1 +
>  debian/copyright.in              |   4 +
>  lib/automake.mk                  |   5 +
>  lib/conntrack-other.c            |  91 ++++
>  lib/conntrack-private.h          |  80 ++++
>  lib/conntrack-tcp.c              | 510 ++++++++++++++++++++
>  lib/conntrack.c                  | 997
> +++++++++++++++++++++++++++++++++++++++
>  lib/conntrack.h                  | 162 +++++++
>  lib/dpif-netdev.c                | 138 +++++-
>  lib/flow.c                       | 154 +++---
>  lib/flow.h                       |   4 +
>  lib/packets.h                    |  14 +-
>  tests/automake.mk                |   1 +
>  tests/dpif-netdev.at             |  14 +-
>  tests/ofproto-dpif.at            | 698 ++++++++++++++++++++++++++-
>  tests/system-common-macros.at    |   1 +
>  tests/system-kmod-macros.at      |  35 ++
>  tests/system-traffic.at          |  69 ++-
>  tests/system-userspace-macros.at |  63 ++-
>  tests/test-conntrack.c           | 232 +++++++++
>  20 files changed, 3164 insertions(+), 109 deletions(-)
>  create mode 100644 lib/conntrack-other.c
>  create mode 100644 lib/conntrack-private.h
>  create mode 100644 lib/conntrack-tcp.c
>  create mode 100644 lib/conntrack.c
>  create mode 100644 lib/conntrack.h
>  create mode 100644 tests/test-conntrack.c
> 
> --
> 2.1.4
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev