Hi Antonio,
thanks for the feedback
On 18/04/2016 09:56, "Fischetti, Antonio" <antonio.fische...@intel.com>
wrote:
>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)",
You're right there are several genuine errors reported by checkpatch.py.
I'll fix them.
>
>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 don't see this error. How did you get the patches? Did you use
patchwork?
In any case, as you point out it is a false positive.
I found another small checkpatch issue with this series and I've sent a
patch here:
http://openvswitch.org/pipermail/dev/2016-April/069795.html
>
>I'll go on and have a look into the code, I hope I provide some useful
>feedback.
>
>Thanks,
>Antonio
Thanks for the report,
Daniele
>
>> -----Original Message-----
>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di
>> Proietto
>> Sent: Saturday, April 16, 2016 1:03 AM
>> To: dev@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@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
