Re: [ovs-dev] [patch_v7 0/9] Userspace Datapath: Introduce NAT support.

2017-04-29 Thread Daniele Di Proietto 
Hi Darrell,

I took another look at the series and provided a few comments inline.

Other than those the patches look good to me, but I haven't looked at
every possible corner case :-)

Thanks,

Daniele

2017-03-24 2:15 GMT-07:00 Darrell Ball :
> This patch series introduces NAT support for the userspace datapath.
>
> The per packet scope of lookups for NAT and un_NAT is at
> the bucket level rather than global. One hash table is
> introduced to support create/delete handling. The create/delete
> events may be further optimized, if the need becomes clear.
>
> The existing NAT tests are enabled for the dpdk datapath,
> with some added enhancements.
>
> Some NAT options with limited utility (persistent, random) are
> not supported yet, but will be supported in a later patch.
>
> alg and fragmentation support are not included here but are
> being worked on.
>
> I realize patch 3 is big. It may be clearer and easier to keep
> as a single patch, so I have done that after some discussion.
>
> v6->v7: A couple patches were committed.
>
> Three news patches were added, including new
> orig tuple context recovery.
>
> Cleanup residual batch sorting deprecated array.
>
> Fix ICMP potential issue.
>
> Add more complete ICMP related handling, originally
> scoped for another patch series.
>
> Splice out a few functions for easier maintenance.
> There will be slight performance hit due to less
> narrow context.
>
> Use structure assignment vs memcpy in a few places.
> Remove unneeded memset.
> Rate limit a vlog.
> Remove a couple hand-rolled netlink parsings.
>
> v5->v6: Add releases file NAT alert, as pointed out by Flavio.
> Add some missing details in commit message in a couple
> patches as mentioned by Flavio.
> Flushed the bug queue - found a couple bugs in testing
> over the last week.
> a) nat_range_hash was missing the intended conn entry
>address and port fields :-); I guess missed since the
>corresponding nat info address and port fields were
>there.
> b) The netlink parsing math was off for min/max address
>in NAT range.
>
> v4->v5: Remove packet sorting in userspace datapath conntrack.
> Simplify conntrack state code.
> Fix sparse error.
> Address code review comments from Daniele.
>
> v3->v4: Fix rev_key vs key for nat_conn_keys access in a couple
> places; this would have affected cleanup; at same time
> rename some variables and change nat_conn_keys APIs to
> use conn key, rather than conn.
>
> Fix conntrack_flush() CT_CONN_TYPE_DEFAULT flag placement;
> the intention was that it be the same as in sweep_bucket().
>
> Fix nat_ipv6_addrs_delta() max boundary checking logic. I
> also enhanced the conntrack - IPv6 HTTP with NAT test to
> give it more coverage as partial penance.
>
> Rebase
>
> v2->v3: Fix a theoretical resend for closed connection restart.
> Parse out a function to help and also limit
> conn_state_update() to one.
>
> I decided to cap V6 address range delta at 4 billion using
> internal adjustment (user visibility not required).
>
> Some cleanup of deprecated code path.
>
> Parse out some more changes as separate patches.
>
> v1->v2: Updates/fixes that were missed in v1 patches.
>
> Darrell Ball (9):
>   dpdk: Parse NAT netlink for userspace datapath.
>   dpdk: Remove batch sorting in userspace conntrack.
>   dpdk: Userspace Datapath: Introduce NAT Support.
>   dpdk: Add more ICMP Related NAT support.
>   dpdk: Add orig tuple context recovery.
>   System Tests: Enhance NAT tests.
>   Add some system test fixes
>   dpdk: Enable NAT tests for userspace datapath.
>   dpdk: Update feature alert documentation.
>
>  Documentation/faq/releases.rst   |2 +-
>  NEWS |2 +
>  lib/conntrack-private.h  |   25 +-
>  lib/conntrack.c  | 1033 
> +-
>  lib/conntrack.h  |   76 ++-
>  lib/dpif-netdev.c|   85 +++-
>  lib/packets.h|7 +
>  tests/atlocal.in |3 +
>  tests/system-traffic.at  |  113 -
>  tests/system-userspace-macros.at |7 +-
>  tests/test-conntrack.c   |9 +-
>  11 files changed, 1198 insertions(+), 164 deletions(-)
>
> --
> 1.9.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [patch_v7 0/9] Userspace Datapath: Introduce NAT support.

2017-03-24 Thread Darrell Ball 
This patch series introduces NAT support for the userspace datapath.

The per packet scope of lookups for NAT and un_NAT is at
the bucket level rather than global. One hash table is
introduced to support create/delete handling. The create/delete
events may be further optimized, if the need becomes clear.

The existing NAT tests are enabled for the dpdk datapath,
with some added enhancements.

Some NAT options with limited utility (persistent, random) are
not supported yet, but will be supported in a later patch.

alg and fragmentation support are not included here but are
being worked on.

I realize patch 3 is big. It may be clearer and easier to keep
as a single patch, so I have done that after some discussion.

v6->v7: A couple patches were committed.

Three news patches were added, including new
orig tuple context recovery.

Cleanup residual batch sorting deprecated array.

Fix ICMP potential issue.

Add more complete ICMP related handling, originally
scoped for another patch series.

Splice out a few functions for easier maintenance.
There will be slight performance hit due to less
narrow context.

Use structure assignment vs memcpy in a few places.
Remove unneeded memset.
Rate limit a vlog.
Remove a couple hand-rolled netlink parsings.
  
v5->v6: Add releases file NAT alert, as pointed out by Flavio.
Add some missing details in commit message in a couple
patches as mentioned by Flavio.
Flushed the bug queue - found a couple bugs in testing
over the last week.
a) nat_range_hash was missing the intended conn entry
   address and port fields :-); I guess missed since the 
   corresponding nat info address and port fields were
   there.
b) The netlink parsing math was off for min/max address
   in NAT range.

v4->v5: Remove packet sorting in userspace datapath conntrack.
Simplify conntrack state code.
Fix sparse error.
Address code review comments from Daniele.

v3->v4: Fix rev_key vs key for nat_conn_keys access in a couple
places; this would have affected cleanup; at same time
rename some variables and change nat_conn_keys APIs to
use conn key, rather than conn.

Fix conntrack_flush() CT_CONN_TYPE_DEFAULT flag placement;
the intention was that it be the same as in sweep_bucket().

Fix nat_ipv6_addrs_delta() max boundary checking logic. I
also enhanced the conntrack - IPv6 HTTP with NAT test to
give it more coverage as partial penance.

Rebase

v2->v3: Fix a theoretical resend for closed connection restart.
Parse out a function to help and also limit
conn_state_update() to one.

I decided to cap V6 address range delta at 4 billion using
internal adjustment (user visibility not required).

Some cleanup of deprecated code path.

Parse out some more changes as separate patches.

v1->v2: Updates/fixes that were missed in v1 patches.

Darrell Ball (9):
  dpdk: Parse NAT netlink for userspace datapath.
  dpdk: Remove batch sorting in userspace conntrack.
  dpdk: Userspace Datapath: Introduce NAT Support.
  dpdk: Add more ICMP Related NAT support.
  dpdk: Add orig tuple context recovery.
  System Tests: Enhance NAT tests.
  Add some system test fixes
  dpdk: Enable NAT tests for userspace datapath.
  dpdk: Update feature alert documentation.

 Documentation/faq/releases.rst   |2 +-
 NEWS |2 +
 lib/conntrack-private.h  |   25 +-
 lib/conntrack.c  | 1033 +-
 lib/conntrack.h  |   76 ++-
 lib/dpif-netdev.c|   85 +++-
 lib/packets.h|7 +
 tests/atlocal.in |3 +
 tests/system-traffic.at  |  113 -
 tests/system-userspace-macros.at |7 +-
 tests/test-conntrack.c   |9 +-
 11 files changed, 1198 insertions(+), 164 deletions(-)

-- 
1.9.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev