[ovs-dev,v2] OVN: run local logical flows first in S_ROUTER_OUT_SNAT table

Run local logical flows first if the gw router port is scheduled
on the local chassis in order to properly manage snat traffic

Tested-by: Eran Kuris <ekuris@redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
Changes since v1:
- add priority change in ovn-northd.8.xml
---
 ovn/northd/ovn-northd.8.xml | 3 ++-
 ovn/northd/ovn-northd.c     | 6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

On Wed, Jun 19, 2019 at 6:15 PM Lorenzo Bianconi <
lorenzo.bianconi@redhat.com> wrote:

> Run local logical flows first if the gw router port is scheduled
> on the local chassis in order to properly manage snat traffic
>
> Tested-by: Eran Kuris <ekuris@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>

Acked-by: Numan Siddique <nusiddiq@redhat.com>

Thanks
Numan


> ---
> Changes since v1:
> - add priority change in ovn-northd.8.xml
> ---
>  ovn/northd/ovn-northd.8.xml | 3 ++-
>  ovn/northd/ovn-northd.c     | 6 ++++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index e6417220f..a51f4a162 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -2421,7 +2421,8 @@ nd_ns {
>          <p>
>            If the NAT rule cannot be handled in a distributed manner, then
>            the flow above is only programmed on the
> -          <code>redirect-chassis</code>.
> +          <code>redirect-chassis</code> increasing flow priority by 128 in
> +          order to be run first
>          </p>
>
>          <p>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 0b0a96a3a..fba5e6d44 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -6566,6 +6566,7 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>                                    count_1bits(ntohl(mask)) + 1,
>                                    ds_cstr(&match), ds_cstr(&actions));
>                  } else {
> +                    uint16_t priority = count_1bits(ntohl(mask)) + 1;
>                      /* Distributed router. */
>                      ds_clear(&match);
>                      ds_put_format(&match, "ip && ip4.src == %s"
> @@ -6575,6 +6576,7 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>                      if (!distributed && od->l3redirect_port) {
>                          /* Flows for NAT rules that are centralized are
> only
>                           * programmed on the "redirect-chassis". */
> +                        priority += 128;
>                          ds_put_format(&match, " &&
> is_chassis_resident(%s)",
>                                        od->l3redirect_port->json_key);
>                      }
> @@ -6589,8 +6591,8 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
>                       * nat->logical_ip with the longest mask gets a higher
>                       * priority. */
>                      ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT,
> -                                  count_1bits(ntohl(mask)) + 1,
> -                                  ds_cstr(&match), ds_cstr(&actions));
> +                                  priority, ds_cstr(&match),
> +                                  ds_cstr(&actions));
>                  }
>              }
>
> --
> 2.21.0
>
>

On Wed, Jun 19, 2019 at 02:45:34PM +0200, Lorenzo Bianconi wrote:
> Run local logical flows first if the gw router port is scheduled
> on the local chassis in order to properly manage snat traffic
> 
> Tested-by: Eran Kuris <ekuris@redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
> Changes since v1:
> - add priority change in ovn-northd.8.xml

Did something important change since this patch was posted?  It doesn't
build:

  CC       ovn/northd/ovn-northd.o
../ovn/northd/ovn-northd.c:6552:25: error: use of undeclared identifier 'priority'

>
> On Wed, Jun 19, 2019 at 02:45:34PM +0200, Lorenzo Bianconi wrote:
> > Run local logical flows first if the gw router port is scheduled
> > on the local chassis in order to properly manage snat traffic
> >
> > Tested-by: Eran Kuris <ekuris@redhat.com>
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> > ---
> > Changes since v1:
> > - add priority change in ovn-northd.8.xml
>
> Did something important change since this patch was posted?  It doesn't
> build:

Hi Ben,

I guess the right context has been moved down and patch finds another
context similar to the original one closer to the original line, so it
applies the chunk there :)
I will post a v3 fixing the compilation issue

Regards,
Lorenzo

>
>   CC       ovn/northd/ovn-northd.o
> ../ovn/northd/ovn-northd.c:6552:25: error: use of undeclared identifier 'priority'