mbox series

[ovs-dev,ovn,0/6] Add IPv6 NAT support

Message ID 20191030013138.9390-1-russell@ovn.org
Headers show
Series Add IPv6 NAT support | expand

Message

Russell Bryant Oct. 30, 2019, 1:31 a.m. UTC 
This came up with reviewing the usage of ovn-kubernetes with IPv6.
It's more straight forward to start with using IPv6 NAT, matching the
IPv4 network topology with Kubernetes.  Eventually, we'd want to allow
routable IPv6 addresses everywhere, but this gives us the option of
using NAT where it's helpful.

[PATCH 1/6] northd: Fix table ID for IPv6 router ingress.
[PATCH 2/6] actions: Add IPv6 support to lflow NAT actions
[PATCH 3/6] ovn-nbctl: Allow IPv6 NAT rules to be added
[PATCH 4/6] northd: Add lflows for IPv6 NAT.
[PATCH 5/6] system-ovn: Add IPv6 NAT test cases
[PATCH 6/6] NEWS: Add IPv6 NAT support

 NEWS                  |    6 
 include/ovn/actions.h |    6 
 lib/actions.c         |   35 +-
 northd/ovn-northd.c   |  378 ++++++++++++++++-----
 tests/ovn-nbctl.at    |   41 +-
 tests/ovn.at          |   18 -
 tests/system-ovn.at   |  862 +++++++++++++++++++++++++++++++++++++++++++++++++-
 utilities/ovn-nbctl.c |   48 ++
 utilities/ovn-trace.c |   14 
 9 files changed, 1266 insertions(+), 142 deletions(-)

--
Russell Bryant

Comments

Numan Siddique Oct. 30, 2019, 12:36 p.m. UTC | #1 
On Wed, Oct 30, 2019 at 7:02 AM Russell Bryant <russell@ovn.org> wrote:
>
> This came up with reviewing the usage of ovn-kubernetes with IPv6.
> It's more straight forward to start with using IPv6 NAT, matching the
> IPv4 network topology with Kubernetes.  Eventually, we'd want to allow
> routable IPv6 addresses everywhere, but this gives us the option of
> using NAT where it's helpful.
>
> [PATCH 1/6] northd: Fix table ID for IPv6 router ingress.
> [PATCH 2/6] actions: Add IPv6 support to lflow NAT actions
> [PATCH 3/6] ovn-nbctl: Allow IPv6 NAT rules to be added
> [PATCH 4/6] northd: Add lflows for IPv6 NAT.
> [PATCH 5/6] system-ovn: Add IPv6 NAT test cases
> [PATCH 6/6] NEWS: Add IPv6 NAT support
>

Hi Russell,

I looked into this series. I have few comments, otherwise looks good to me.

1. There are few checkpatch warnings. I think mostly related to line
length > 80.
     Can you please address them.

2. In patch 3, I am getting below compilation warning when configured
with "--enable-Werror --enable-sparse"
mv -f $depbase.Tpo $depbase.Po
../utilities/ovn-nbctl.c:3870:29: error: missing braces around initializer
../utilities/ovn-nbctl.c:3870:29: error: missing braces around initializer
make[1]: *** [Makefile:2226: utilities/ovn-nbctl.o] Error 1

3. I think you need update ovn-northd.8.xml in patch 4, since it adds
and deletes few logical flows.

Thanks
Numan


>  NEWS                  |    6
>  include/ovn/actions.h |    6
>  lib/actions.c         |   35 +-
>  northd/ovn-northd.c   |  378 ++++++++++++++++-----
>  tests/ovn-nbctl.at    |   41 +-
>  tests/ovn.at          |   18 -
>  tests/system-ovn.at   |  862 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  utilities/ovn-nbctl.c |   48 ++
>  utilities/ovn-trace.c |   14
>  9 files changed, 1266 insertions(+), 142 deletions(-)
>
> --
> Russell Bryant
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev