[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> &&
> > + is_chassis_resident(cr-<var>P</var>) && eth.bcast
> &&
> > + arp.op == 2 && 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> &&
> > + !is_chassis_resident(cr-<var>P</var>) && eth.bcast
> &&
> > + arp.op == 2 && 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