[ovs-dev] [PATCH ovn] Handle GARP reply packets from provider networks only on gateway chassis

Numan Siddique nusiddiq at redhat.com
Thu Sep 5 11:00:21 UTC 2019


On Thu, Sep 5, 2019 at 3:19 AM Han Zhou <zhouhan at gmail.com> wrote:

>
>
> On Mon, Sep 2, 2019 at 9:32 AM <nusiddiq at redhat.com> wrote:
> >
> > From: Numan Siddique <nusiddiq at redhat.com>
> >
> > Suppose there is a provider network (with localnet port) and it is
> > connected to a logical router via a distributed gateway router port.
> > When an external switch sends GARP request packets, these packets
> > enter the br-int (via the patch port from the provider bridge) and
> > the action - put_arp is applied in the router pipeline only on the
> > gateway chassis where the distributed gateway router port is scheduled.
> > Other chassis even if they receive don't apply this action. This
> functionality
> > was added by the commit [1].
> >
> > But that is not the case with GARP reply packets sent by the external
> > switch. ovn-controllers running on all the chassis configured with
> > ovn-bridge-mappings receive these packets as packet-ins because
> > of put_arp action. This results in unnecessary processing of these
> > packets only to be ignored. pinctrl thread wakes up the main
> ovn-controller
> > thread wasting few CPU cycles. It is enough for the gateway chassis to
> > handle these packets where the distributed router gateway port is
> scheduled.
> >
> > This patch achieves the same - similar to GARP request packets. The below
> > logical flows are added
> >  - table=1 (lr_in_ip_input), priority=92,
> >    match=(inport == "lrp" &&
> >           !is_chassis_resident("cr-lrp") && eth.bcast &&
> >           arp.op == 2 && arp.spa == PROVIDER_NETWORK_CIDR),
> action=(drop;)
> >
> >  - table=1 (lr_in_ip_input), priority=92
> >    match=(inport == "lr0-public" && is_chassis_resident("cr-lr0-public")
> >           && eth.bcast && arp.op == 2 && arp.spa ==
> PROVIDER_NETWORK_CIDR),
> >    action=(put_arp(inport, arp.spa, arp.sha);)
> >
> > This patch doesn't affect the arp replies from the overlay networks in
> the
> > router pipeline. This only handles GARP replies from the provider
> networks.
> >
> > In the present master this is not much of any issue even if
> ovn-controller
> > wakes up, thanks to incremental processing engine. But in the older
> > versions - 2.11, 2.10 and 2.9, this results in complete flow calculations
> > resulting in much more CPU cyles. This patch will definitely help in
> saving
> > these CPU cyles if backported.
> >
> > [1] - 3dd9febe953f("ovn-northd: Support learning neighbor from ARP
> request.")
> >
> > CC: Han ZHou <hzhou8 at ebay.com>
> > Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
> > ---
> >  northd/ovn-northd.8.xml | 31 +++++++++++++++++++++++++++----
> >  northd/ovn-northd.c     | 35 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 62 insertions(+), 4 deletions(-)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index 442e899bc..a06e404dd 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -1442,10 +1442,33 @@ arp.sha = <var>external_mac</var>;
> >        </li>
> >
> >        <li>
> > -        ARP reply handling.  This flow uses ARP replies to populate the
> > -        logical router's ARP table.  A priority-90 flow with match
> <code>arp.op
> > -        == 2</code> has actions <code>put_arp(inport, arp.spa,
> > -        arp.sha);</code>.
> > +        <p>
> > +          ARP reply handling.  Following flows are added to handle ARP
> replies.
> > +        </p>
> > +
> > +        <p>
> > +          For each distributed gateway logical router port a
> priority-92 flow
> > +          with match <code>inport == <var>P</var> &amp;&amp;
> > +          is_chassis_resident(cr-<var>P</var>) &amp;&amp; eth.bcast
> &amp;&amp;
> > +          arp.op == 2 &amp;&amp; arp.spa == <var>I</var></code> with the
> > +          action <code>put_arp(inport, arp.spa, arp.sha);</code> so
> that the
> > +          resident gateway chassis can learn the GARP reply, where
> > +          <var>P</var> is the distributed gateway router port name,
> > +          <var>I</var> is the logical router port's network address.
> > +        </p>
> > +
> > +        <p>
> > +          For each distributed gateway logical router port a
> priority-92 flow
> > +          with match <code>inport == <var>P</var> &amp;&amp;
> > +          !is_chassis_resident(cr-<var>P</var>) &amp;&amp; eth.bcast
> &amp;&amp;
> > +          arp.op == 2 &amp;&amp; arp.spa == <var>I</var></code> with
> the action
> > +          <code>drop;</code> so that other chassis drop this packet.
> > +        </p>
> > +
> > +        <p>
> > +          A priority-90 flow with match <code>arp.op == 2</code> has
> actions
> > +          <code>put_arp(inport, arp.spa, arp.sha);</code>.
> > +        </p>
> >        </li>
> >
> >        <li>
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 9a2222282..a83b56362 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -6552,6 +6552,41 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
> >
> >          }
> >
> > +        /* Handle GARP reply packets received on a distributed router
> gateway
> > +         * port. GARP reply broadcast packets could be sent by external
> > +         * switches. We don't want them to be handled by all the
> > +         * ovn-controllers if they receive it. So add a priority-92
> flow to
> > +         * apply the put_arp action on a redirect chassis and drop it on
> > +         * other chassis.
> > +         * Note that we are already adding a priority-90 logical flow
> in the
> > +         * table S_ROUTER_IN_IP_INPUT to apply the put_arp action if
> > +         * arp.op == 2.
> > +         * */
> > +        if (op->od->l3dgw_port && op == op->od->l3dgw_port
> > +                && op->od->l3redirect_port) {
> > +            for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> > +                ds_clear(&match);
> > +                ds_put_format(&match,
> > +                              "inport == %s && is_chassis_resident(%s)
> && "
> > +                              "eth.bcast && arp.op == 2 && arp.spa ==
> %s/%u",
> > +                              op->json_key,
> op->od->l3redirect_port->json_key,
> > +                              op->lrp_networks.ipv4_addrs[i].network_s,
> > +                              op->lrp_networks.ipv4_addrs[i].plen);
> > +                ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 92,
> > +                              ds_cstr(&match),
> > +                              "put_arp(inport, arp.spa, arp.sha);");
> > +                ds_clear(&match);
> > +                ds_put_format(&match,
> > +                              "inport == %s && !is_chassis_resident(%s)
> && "
> > +                              "eth.bcast && arp.op == 2 && arp.spa ==
> %s/%u",
> > +                              op->json_key,
> op->od->l3redirect_port->json_key,
> > +                              op->lrp_networks.ipv4_addrs[i].network_s,
> > +                              op->lrp_networks.ipv4_addrs[i].plen);
> > +                ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_INPUT, 92,
> > +                              ds_cstr(&match), "drop;");
> > +            }
> > +        }
> > +
> >          /* A set to hold all load-balancer vips that need ARP
> responses. */
> >          struct sset all_ips = SSET_INITIALIZER(&all_ips);
> >          int addr_family;
> > --
> > 2.21.0
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> Thanks Numan.
>
> Acked-by: Han Zhou <hzhou8 at ebay.com>
>

Thanks for the review. I pushed this to master.

Numan


More information about the dev mailing list