| Message ID | eb8d52a3d7ed0f49cb3d3efc30aece63a42e3d34.1582825683.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v2,ovn] manage ARP process locally in a DVR scenario | expand |
On Thu, Feb 27, 2020 at 11:21 PM Lorenzo Bianconi <lorenzo.bianconi@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@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, > +} > + > 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); } 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@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
> On Thu, Feb 27, 2020 at 11:21 PM Lorenzo Bianconi > <lorenzo.bianconi@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@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@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > >
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); + } +} + 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 + * 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); + } + /* 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
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@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(-)