[ovs-dev] [PATCH v2 ovn] manage ARP process locally in a DVR scenario

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Mon Mar 2 09:04:59 UTC 2020


> On Thu, Feb 27, 2020 at 11:21 PM Lorenzo Bianconi
> <lorenzo.bianconi at redhat.com> wrote:
> >
> > OVN currently performs L2 address resolution and IP buffering on the
> > gw node. If the system relies on FIPs, OVN will re-inject the buffered
> > IP packets on the gw node, while following packets will go though
> > the localnet port on the compute node resulting in a ToR switch
> > misconfiguration. This patch addresses the issue managing ARP
> > and IP buffering locally if FIPs are configured on the node
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> > ---
> > Changes since v1:
> > - add comments
> > - cosmetics
> > ---
> >  northd/ovn-northd.8.xml | 31 ++++++++++++++++++++++++
> >  northd/ovn-northd.c     | 52 ++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 82 insertions(+), 1 deletion(-)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index a27dfa951..23b385377 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -2405,6 +2405,37 @@ output;
> >          </p>
> >        </li>
> >
> > +      <li>
> > +        <p>
> > +          For distributed logical routers where one of the logical router ports
> > +          specifies a <code>redirect-chassis</code>, a priority-400 logical
> > +          flow for each <code>dnat_and_snat</code> NAT rules configured.
> > +          These flows will allow to properly forward traffic to the external
> > +          connections if available and avoid sending it through the tunnel.
> > +          Assuming the following NAT rule has been configured:
> > +        </p>
> > +
> > +        <pre>
> > +external_ip = <var>A</var>;
> > +external_mac = <var>B</var>;
> > +logical_ip = <var>C</var>;
> > +        </pre>
> > +
> > +        <p>
> > +          the following action will be applied:
> > +        </p>
> > +
> > +        <pre>
> > +ip.ttl--;
> > +reg0 = <var>ip.dst</var>;
> > +reg1 = <var>A</var>;
> > +eth.src = <var>B</var>;
> > +outport = <var>router-port</var>;
> > +next;
> > +        </pre>
> > +
> > +      </li>
> > +
> >        <li>
> >          <p>
> >            IPv4 routing table.  For each route to IPv4 network <var>N</var> with
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index d007ba610..53958b26b 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -6967,6 +6967,8 @@ build_routing_policy_flow(struct hmap *lflows, struct ovn_datapath *od,
> >      ds_destroy(&actions);
> >  }
> >
> > +/* default logical flow prioriry for distributed routes */
> > +#define DROUTE_PRIO 400
> >  struct parsed_route {
> >      struct ovs_list list_node;
> >      struct v46_ip prefix;
> > @@ -7354,6 +7356,41 @@ build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od,
> >      ds_destroy(&actions);
> >  }
> >
> > +static void
> > +add_distributed_routes(struct hmap *lflows, struct ovn_datapath *od)
> > +{
> > +    struct ds actions = DS_EMPTY_INITIALIZER;
> > +    struct ds match = DS_EMPTY_INITIALIZER;
> > +
> > +    if (!od->l3dgw_port || !od->nbr) {
> > +        return;
> > +    }
> > +
> > +    for (size_t i = 0; i < od->nbr->n_nat; i++) {
> > +        const struct nbrec_nat *nat = od->nbr->nat[i];
> > +
> > +        if (strcmp(nat->type, "dnat_and_snat") ||
> > +            !nat->external_mac) {
> > +            continue;
> > +        }
> > +
> > +        bool is_ipv4 = strchr(nat->logical_ip, '.') ? true : false;
> > +        ds_put_format(&match, "ip%s.src == %s && is_chassis_resident(\"%s\")",
> > +                      is_ipv4 ? "4" : "6", nat->logical_ip,
> > +                      nat->logical_port);
> > +        char *prefix = is_ipv4 ? "" : "xx";
> > +        ds_put_format(&actions, "outport = %s; eth.src = %s; "
> > +                      "%sreg0 = ip%s.dst; %sreg1 = %s; next;",
> > +                      od->l3dgw_port->json_key, nat->external_mac,
> > +                      prefix, is_ipv4 ? "4" : "6",
> > +                      prefix, nat->external_ip);
> > +        ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, DROUTE_PRIO,
> > +                      ds_cstr(&match), ds_cstr(&actions));
> > +        ds_clear(&match);
> > +        ds_clear(&actions);
> > +    }
> 
> You need to ds_destroy match and actions, otherwise there'll be a memory leak,

Hi Numan,

Thx for the review.

ack, I will fix it in v3

> 
> 
> > +}
> > +
> >  static void
> >  add_route(struct hmap *lflows, const struct ovn_port *op,
> >            const char *lrp_addr_s, const char *network_s, int plen,
> > @@ -7375,6 +7412,12 @@ add_route(struct hmap *lflows, const struct ovn_port *op,
> >      }
> >      build_route_match(op_inport, network_s, plen, is_src_route, is_ipv4,
> >                        &match, &priority);
> > +    /* traffic for internal IPs of logical switch ports must me sent to
> s/me/be
> 
> > +     * the gw controller through the overlay tunnels
> > +     */
> > +    if (op->nbrp && !op->nbrp->n_gateway_chassis) {
> > +        priority += DROUTE_PRIO;
> > +    }
> >
> >      struct ds actions = DS_EMPTY_INITIALIZER;
> >      ds_put_format(&actions, "ip.ttl--; "REG_ECMP_GROUP_ID" = 0; %sreg0 = ",
> > @@ -8943,7 +8986,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> >                                nat->logical_ip,
> >                                od->l3dgw_port->json_key);
> >                  ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_GW_REDIRECT,
> > -                                        100, ds_cstr(&match), "next;",
> > +                                        200, ds_cstr(&match), "next;",
> >                                          &nat->header_);
> >              }
> >
> > @@ -9224,6 +9267,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> >          ovn_lflow_add(lflows, od, S_ROUTER_IN_ND_RA_RESPONSE, 0, "1", "next;");
> >      }
> >
> > +    /* Logical router ingress table IP_ROUTING - IP routing for distributed
> > +     * logical router
> > +     */
> > +    HMAP_FOR_EACH (od, key_node, datapaths) {
> > +        add_distributed_routes(lflows, od);
> 
> This function gets called even for logical switches. It's better to call it as
> 
>  if (od->nbr && od->l3dgw_port) {
>      add_distributed_routes(lflows, od);
>  }

ack, I will fix it in v3

Regards,
Lorenzo

> 
> Thanks
> Numan
> 
> > +    }
> > +
> >      /* Logical router ingress table IP_ROUTING & IP_ROUTING_ECMP: IP Routing.
> >       *
> >       * A packet that arrives at this table is an IP packet that should be
> > --
> > 2.24.1
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> 


More information about the dev mailing list