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

Han Zhou zhouhan at gmail.com
Wed Sep 4 21:49:11 UTC 2019


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>


More information about the dev mailing list