Re: [ovs-dev] [PATCH v5 00/16] Userspace (DPDK) connection tracker

[Daniele Di Proietto]

I pushed the intended version except that I forgot to squash that commit with
the previous one.  Sorry about this


On 28/07/2016 00:40, "Ilya Maximets"  wrote:

>Sorry.
>TO: Daniele Di Proietto 
>
>On 28.07.2016 09:27, Ilya Maximets wrote:
>> I guess, you pushed some development version of this patch set.
>> 
>> There is strange commit there:
>> 
>> commit 6c54734ed27bc22975d7035a6bd5f32a412335a0
>> Author: Daniele Di Proietto 
>> Date:   Wed Jul 27 18:32:15 2016 -0700
>> 
>> XXX Improve comment.
>> 
>> 
>> Best regards, Ilya Maximets.
>> 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 00/16] Userspace (DPDK) connection tracker

[Joe Stringer]

On 27 July 2016 at 23:27, Ilya Maximets  wrote:
> I guess, you pushed some development version of this patch set.
>
> There is strange commit there:
>
> commit 6c54734ed27bc22975d7035a6bd5f32a412335a0
> Author: Daniele Di Proietto 
> Date:   Wed Jul 27 18:32:15 2016 -0700
>
> XXX Improve comment.
>
>
> Best regards, Ilya Maximets.

As Ben mentioned in the other thread, it looks like this was intended
to be squashed into the commit next to it. This was the only piece of
feedback that changed something in the series, so most likely Daniele
just forgot to actually squash it before committing.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 00/16] Userspace (DPDK) connection tracker

[Joe Stringer]

Congrats! This has been a while coming:)

On 27 July 2016 at 23:17, Daniele Di Proietto  wrote:
> Thanks for the reviews, I pushed this to master except for the system tests 
> part.
>
>
>
> On 26/07/2016 17:58, "Daniele Di Proietto"  wrote:
>
>>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.
>>
>>This can also be downloaded at:
>>
>>https://github.com/ddiproietto/ovs/tree/userconntrack_20160726
>>
>>Any feedback is appreciated, thanks.
>>
>>v4 -> v5:
>>* Rebase: hmap.h is moved, include ct_* field in some unit tests,
>>  skip and adapt to the new ct dump format the OVN tests.
>>* Style and typo fixes.
>>* Add coverage counter to detect long cleanup.
>>* Use ovs_barrier instead of pthread_barrier in test (fix compilation
>>  on OS X).
>>* Fix dumping tcp state in the reply direction.
>>* Squash together flow_compose improvements (checksum and udp_len).
>>
>>v3 -> v4:
>>* Rebase: use struct dp_packet_batch, add extra ct_ fields in some
>>  new tests, use struct hmap_pos, skip some new system NAT tests.
>>* Style and typo fixes.
>>* Add OVS_NOT_REACHED() in switch in process_one().
>>* New commit: use dl_type from flow or matching megaflow.
>>
>>v2 -> v3:
>>* Rebased.
>>* Squashed commits for flushing (in dpif-netdev and conntrack).
>>* Squashed commits for dumping (in dpif-netdev and conntrack).
>>* Use adaptive mutex instead of spinlock: this prevents livelock
>>  if the cleanup thread is executed on the same CPU as a forwarding
>>  thread.  Performance impact in minimal.
>>* Validate L3 and L4 checksum.
>>* Use proper L3 and L4 checksum in hardcoded packets in system and unit
>>  tests.
>>* Consider ICMPv6 as well as ICMP in l4_protos and conn_key_to_tuple.
>>* Mention conntrack in NEWS and FAQ.md.
>>* Use uint16_t for ct_state.
>>* Fix possible NULL dereference for conn in process_one().
>>* Add OVS_U128_MIN, OVS_U128_ZERO.
>>* Use HMAP_FOR_EACH_POP.
>>* Check that UDP length is valid.
>>* Style fix: prefer 'sizeof *object' instead of 'sizeof type'
>>* Don't accept packets from/to UDP/TCP port 0.
>>* Use defines for timeouts.
>>* Check expiration inside lookup loop in conn_key_lookup().
>>* Limit the number of connections.
>>* Simplify case if tcp_get_wscale().
>>* Introduce general INT_MOD_* macros for comparisons in modular arithmetic.
>>* Improve comments.
>>* New cleanup mechanism: we keep connections in an ordered list and we have
>>  a separate thread to performs the cleanup.  This doesn't block the main
>>  thread for long intervals anymore.
>>* Correctly fill UDP length and UDP/TCP/ICMP checksums in flow_compose():
>>  it's useful to write testcases for the connection tracker.
>>* Added system test with ICMP traffic through the connection tracker.
>>* Track ICMP type and code.
>>
>>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 (16):
>>  packets: Define ICMP types.
>>  flow: Export parse_ipv6_ext_hdrs().
>>  flow: Introduce parse_dl_type().
>>  conntrack: New userspace connection tracker.
>>  conntrack: Periodically delete expired connections.
>>  tests: Add very simple conntrack benchmark.
>>  tests: Add test-conntrack pcap test.
>>  dpif-netdev: Execute conntrack action.
>>  dpif-netdev: Implement conntrack dump functions.
>>  dpif-netdev: Implement conntrack flush interface.
>>  flow: Generate checksum and udp_len in flow_compose().
>>  tests: Add conntrack ofproto-dpif tests.
>>  system-tests: Run conntrack tests with userspace.
>>  system-tests: 

Re: [ovs-dev] [PATCH v5 00/16] Userspace (DPDK) connection tracker

[Ilya Maximets]

Sorry.
TO: Daniele Di Proietto 

On 28.07.2016 09:27, Ilya Maximets wrote:
> I guess, you pushed some development version of this patch set.
> 
> There is strange commit there:
> 
> commit 6c54734ed27bc22975d7035a6bd5f32a412335a0
> Author: Daniele Di Proietto 
> Date:   Wed Jul 27 18:32:15 2016 -0700
> 
> XXX Improve comment.
> 
> 
> Best regards, Ilya Maximets.
> 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 00/16] Userspace (DPDK) connection tracker

[Ilya Maximets]

I guess, you pushed some development version of this patch set.

There is strange commit there:

commit 6c54734ed27bc22975d7035a6bd5f32a412335a0
Author: Daniele Di Proietto 
Date:   Wed Jul 27 18:32:15 2016 -0700

XXX Improve comment.


Best regards, Ilya Maximets.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 00/16] Userspace (DPDK) connection tracker

[Daniele Di Proietto]

Thanks for the reviews, I pushed this to master except for the system tests 
part.



On 26/07/2016 17:58, "Daniele Di Proietto"  wrote:

>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.
>
>This can also be downloaded at:
>
>https://github.com/ddiproietto/ovs/tree/userconntrack_20160726
>
>Any feedback is appreciated, thanks.
>
>v4 -> v5:
>* Rebase: hmap.h is moved, include ct_* field in some unit tests,
>  skip and adapt to the new ct dump format the OVN tests.
>* Style and typo fixes.
>* Add coverage counter to detect long cleanup.
>* Use ovs_barrier instead of pthread_barrier in test (fix compilation
>  on OS X).
>* Fix dumping tcp state in the reply direction.
>* Squash together flow_compose improvements (checksum and udp_len).
>
>v3 -> v4:
>* Rebase: use struct dp_packet_batch, add extra ct_ fields in some
>  new tests, use struct hmap_pos, skip some new system NAT tests.
>* Style and typo fixes.
>* Add OVS_NOT_REACHED() in switch in process_one().
>* New commit: use dl_type from flow or matching megaflow.
>
>v2 -> v3:
>* Rebased.
>* Squashed commits for flushing (in dpif-netdev and conntrack).
>* Squashed commits for dumping (in dpif-netdev and conntrack).
>* Use adaptive mutex instead of spinlock: this prevents livelock
>  if the cleanup thread is executed on the same CPU as a forwarding
>  thread.  Performance impact in minimal.
>* Validate L3 and L4 checksum.
>* Use proper L3 and L4 checksum in hardcoded packets in system and unit
>  tests.
>* Consider ICMPv6 as well as ICMP in l4_protos and conn_key_to_tuple.
>* Mention conntrack in NEWS and FAQ.md.
>* Use uint16_t for ct_state.
>* Fix possible NULL dereference for conn in process_one().
>* Add OVS_U128_MIN, OVS_U128_ZERO.
>* Use HMAP_FOR_EACH_POP.
>* Check that UDP length is valid.
>* Style fix: prefer 'sizeof *object' instead of 'sizeof type'
>* Don't accept packets from/to UDP/TCP port 0.
>* Use defines for timeouts.
>* Check expiration inside lookup loop in conn_key_lookup().
>* Limit the number of connections.
>* Simplify case if tcp_get_wscale().
>* Introduce general INT_MOD_* macros for comparisons in modular arithmetic.
>* Improve comments.
>* New cleanup mechanism: we keep connections in an ordered list and we have
>  a separate thread to performs the cleanup.  This doesn't block the main
>  thread for long intervals anymore.
>* Correctly fill UDP length and UDP/TCP/ICMP checksums in flow_compose():
>  it's useful to write testcases for the connection tracker.
>* Added system test with ICMP traffic through the connection tracker.
>* Track ICMP type and code.
>
>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 (16):
>  packets: Define ICMP types.
>  flow: Export parse_ipv6_ext_hdrs().
>  flow: Introduce parse_dl_type().
>  conntrack: New userspace connection tracker.
>  conntrack: Periodically delete expired connections.
>  tests: Add very simple conntrack benchmark.
>  tests: Add test-conntrack pcap test.
>  dpif-netdev: Execute conntrack action.
>  dpif-netdev: Implement conntrack dump functions.
>  dpif-netdev: Implement conntrack flush interface.
>  flow: Generate checksum and udp_len in flow_compose().
>  tests: Add conntrack ofproto-dpif tests.
>  system-tests: Run conntrack tests with userspace.
>  system-tests: Add ping through conntrack test.
>  conntrack: Track ICMP type and code.
>  conntrack: Add 'dl_type' parameter to conntrack_execute().
>
> COPYING  |1 +
> FAQ.md   |2 +-
> NEWS