[ovs-dev,ovn] Restrict ARP replies for LB VIP only on chassis redirect port
diff mbox series

Message ID 20200109103206.887751-1-numans@ovn.org
State Accepted
Headers show
Series
  • [ovs-dev,ovn] Restrict ARP replies for LB VIP only on chassis redirect port
Related show

Commit Message

Numan Siddique Jan. 9, 2020, 10:32 a.m. UTC
From: Numan Siddique <numans@ovn.org>

Presently when ARP request for the load balance VIP is received
from the provider network, all the ovn-controllers' reply to the ARP
request which have ovn-bridge-mappings configured.

This patch restricts these ARP replies only on the chassis where the
chassis redirect port of the distributed router port is resident.

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/ovn-northd.8.xml | 14 ++++++++++++++
 northd/ovn-northd.c     |  4 ++++
 2 files changed, 18 insertions(+)

Comments

Dumitru Ceara Jan. 9, 2020, 10:40 a.m. UTC | #1
On Thu, Jan 9, 2020 at 11:32 AM <numans@ovn.org> wrote:
>
> From: Numan Siddique <numans@ovn.org>
>
> Presently when ARP request for the load balance VIP is received
> from the provider network, all the ovn-controllers' reply to the ARP
> request which have ovn-bridge-mappings configured.
>
> This patch restricts these ARP replies only on the chassis where the
> chassis redirect port of the distributed router port is resident.
>
> Signed-off-by: Numan Siddique <numans@ovn.org>

Hi Numan,

The patch looks good to me but you forgot to add the
"is_chassis_resident" check for IPv6 LB. I see you did add the section
for v6 to the manpage :)

Regards,
Dumitru

> ---
>  northd/ovn-northd.8.xml | 14 ++++++++++++++
>  northd/ovn-northd.c     |  4 ++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index c6d5d96b9..4b227ca71 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1680,6 +1680,13 @@ flags.loopback = 1;
>  output;
>          </pre>
>
> +        <p>
> +          If the router port <var>P</var> is a distributed gateway router
> +          port, then the <code>is_chassis_resident(<var>P</var>)</code> is
> +          also added in the match condition for the load balancer IPv4
> +          VIP <var>A</var>.
> +        </p>
> +
>          <p>
>            IPv6: For a configured DNAT IP address or a load balancer
>            IPv6 VIP <var>A</var>, solicited node address <var>S</var>,
> @@ -1704,6 +1711,13 @@ nd_na {
>  }
>          </pre>
>
> +        <p>
> +          If the router port <var>P</var> is a distributed gateway router
> +          port, then the <code>is_chassis_resident(<var>P</var>)</code>
> +          is also added in the match condition for the load balancer IPv6
> +          VIP <var>A</var>.
> +        </p>
> +
>          <p>
>            For the gateway port on a distributed logical router with NAT
>            (where one of the logical router ports specifies a
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index d91a008b7..925f25079 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -7511,6 +7511,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>                            "inport == %s && arp.tpa == %s && arp.op == 1",
>                            op->json_key, ip_address);
>
> +            if (op == op->od->l3dgw_port) {
> +                ds_put_format(&match, " && is_chassis_resident(%s)",
> +                              op->od->l3redirect_port->json_key);
> +            }
>              ds_clear(&actions);
>              ds_put_format(&actions,
>                            "eth.dst = eth.src; "
> --
> 2.24.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Numan Siddique Jan. 9, 2020, 12:19 p.m. UTC | #2
On Thu, Jan 9, 2020 at 4:11 PM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On Thu, Jan 9, 2020 at 11:32 AM <numans@ovn.org> wrote:
> >
> > From: Numan Siddique <numans@ovn.org>
> >
> > Presently when ARP request for the load balance VIP is received
> > from the provider network, all the ovn-controllers' reply to the ARP
> > request which have ovn-bridge-mappings configured.
> >
> > This patch restricts these ARP replies only on the chassis where the
> > chassis redirect port of the distributed router port is resident.
> >
> > Signed-off-by: Numan Siddique <numans@ovn.org>
>
> Hi Numan,
>
> The patch looks good to me but you forgot to add the
> "is_chassis_resident" check for IPv6 LB. I see you did add the section
> for v6 to the manpage :)

Oops. Thanks for pointing it out. Submitted v2 -
https://patchwork.ozlabs.org/patch/1220339/

Thanks
Numan

>
> Regards,
> Dumitru
>
> > ---
> >  northd/ovn-northd.8.xml | 14 ++++++++++++++
> >  northd/ovn-northd.c     |  4 ++++
> >  2 files changed, 18 insertions(+)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index c6d5d96b9..4b227ca71 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -1680,6 +1680,13 @@ flags.loopback = 1;
> >  output;
> >          </pre>
> >
> > +        <p>
> > +          If the router port <var>P</var> is a distributed gateway router
> > +          port, then the <code>is_chassis_resident(<var>P</var>)</code> is
> > +          also added in the match condition for the load balancer IPv4
> > +          VIP <var>A</var>.
> > +        </p>
> > +
> >          <p>
> >            IPv6: For a configured DNAT IP address or a load balancer
> >            IPv6 VIP <var>A</var>, solicited node address <var>S</var>,
> > @@ -1704,6 +1711,13 @@ nd_na {
> >  }
> >          </pre>
> >
> > +        <p>
> > +          If the router port <var>P</var> is a distributed gateway router
> > +          port, then the <code>is_chassis_resident(<var>P</var>)</code>
> > +          is also added in the match condition for the load balancer IPv6
> > +          VIP <var>A</var>.
> > +        </p>
> > +
> >          <p>
> >            For the gateway port on a distributed logical router with NAT
> >            (where one of the logical router ports specifies a
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index d91a008b7..925f25079 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -7511,6 +7511,10 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> >                            "inport == %s && arp.tpa == %s && arp.op == 1",
> >                            op->json_key, ip_address);
> >
> > +            if (op == op->od->l3dgw_port) {
> > +                ds_put_format(&match, " && is_chassis_resident(%s)",
> > +                              op->od->l3redirect_port->json_key);
> > +            }
> >              ds_clear(&actions);
> >              ds_put_format(&actions,
> >                            "eth.dst = eth.src; "
> > --
> > 2.24.1
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Patch
diff mbox series

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index c6d5d96b9..4b227ca71 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1680,6 +1680,13 @@  flags.loopback = 1;
 output;
         </pre>
 
+        <p>
+          If the router port <var>P</var> is a distributed gateway router
+          port, then the <code>is_chassis_resident(<var>P</var>)</code> is
+          also added in the match condition for the load balancer IPv4
+          VIP <var>A</var>.
+        </p>
+
         <p>
           IPv6: For a configured DNAT IP address or a load balancer
           IPv6 VIP <var>A</var>, solicited node address <var>S</var>,
@@ -1704,6 +1711,13 @@  nd_na {
 }
         </pre>
 
+        <p>
+          If the router port <var>P</var> is a distributed gateway router
+          port, then the <code>is_chassis_resident(<var>P</var>)</code>
+          is also added in the match condition for the load balancer IPv6
+          VIP <var>A</var>.
+        </p>
+
         <p>
           For the gateway port on a distributed logical router with NAT
           (where one of the logical router ports specifies a
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index d91a008b7..925f25079 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -7511,6 +7511,10 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                           "inport == %s && arp.tpa == %s && arp.op == 1",
                           op->json_key, ip_address);
 
+            if (op == op->od->l3dgw_port) {
+                ds_put_format(&match, " && is_chassis_resident(%s)",
+                              op->od->l3redirect_port->json_key);
+            }
             ds_clear(&actions);
             ds_put_format(&actions,
                           "eth.dst = eth.src; "