| Message ID | 366a394750805779e4bb95c4fa8f5e86b4767090.1583141641.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3,ovn] manage ARP process locally in a DVR scenario | expand |
On Mon, Mar 2, 2020 at 3:08 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> Thanks Lorenzo. I applied this patch to master and branch-20.03 (as it's a pretty serious bug and we need the fix) I applied by adding the below test case and some modifications to ovn-northd.8.xml. The patch modified the priority of some of the existing logical flows. I updated the documentation accordingly. ************* diff --git a/tests/ovn.at b/tests/ovn.at index 3f0fb9c18..a04f22c4c 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -9503,6 +9503,20 @@ AT_CHECK([as hv3 ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=p OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-vsctl show | \ grep "Port patch-br-int-to-ln_port" | wc -l`]) +AT_CHECK([test 1 = `ovn-sbctl dump-flows lr0 | grep lr_in_ip_routing | \ +grep "ip4.src == 10.0.0.3 && is_chassis_resident(\"foo1\")" -c`]) +AT_CHECK([test 1 = `ovn-sbctl dump-flows lr0 | grep lr_in_ip_routing | \ +grep "ip4.src == 10.0.0.4 && is_chassis_resident(\"foo2\")" -c`]) + +key=`ovn-sbctl --bare --columns tunnel_key list datapath_Binding lr0` +# Check that the OVS flows appear for the dnat_and_snat entries in +# lr_in_ip_routing table. +OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-ofctl dump-flows br-int table=17 | \ +grep "priority=400,ip,metadata=0x$key,nw_src=10.0.0.3" -c`]) + +OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-ofctl dump-flows br-int table=17 | \ +grep "priority=400,ip,metadata=0x$key,nw_src=10.0.0.4" -c`]) + # Re-add nat-addresses option ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router" ************** Thanks Numan > --- > Changes since v2: > - fix memory leak in add_distributed_routes > - move check on logical router port before running add_distributed_routes > > Changes since v1: > - add comments > - cosmetics > --- > northd/ovn-northd.8.xml | 31 ++++++++++++++++++++++++ > northd/ovn-northd.c | 53 ++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 83 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 3a24d3c4f..0d43322cf 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -6972,6 +6972,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; > @@ -7359,6 +7361,40 @@ 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; > + > + 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); > + } > + > + ds_destroy(&actions); > + ds_destroy(&match); > +} > + > static void > add_route(struct hmap *lflows, const struct ovn_port *op, > const char *lrp_addr_s, const char *network_s, int plen, > @@ -7380,6 +7416,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 be 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 = ", > @@ -8948,7 +8990,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_); > } > > @@ -9229,6 +9271,15 @@ 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) { > + if (od->nbr && od->l3dgw_port) { > + 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 > -- > 2.24.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
> On Mon, Mar 2, 2020 at 3:08 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> > > Thanks Lorenzo. I applied this patch to master and branch-20.03 (as > it's a pretty serious bug > and we need the fix) > > I applied by adding the below test case and some modifications to > ovn-northd.8.xml. The patch modified > the priority of some of the existing logical flows. I updated the > documentation accordingly. ack, thx a lot Numan. Regards, Lorenzo > > ************* > diff --git a/tests/ovn.at b/tests/ovn.at > index 3f0fb9c18..a04f22c4c 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -9503,6 +9503,20 @@ AT_CHECK([as hv3 ovs-vsctl set Open_vSwitch . > external-ids:ovn-bridge-mappings=p > OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-vsctl show | \ > grep "Port patch-br-int-to-ln_port" | wc -l`]) > > +AT_CHECK([test 1 = `ovn-sbctl dump-flows lr0 | grep lr_in_ip_routing | \ > +grep "ip4.src == 10.0.0.3 && is_chassis_resident(\"foo1\")" -c`]) > +AT_CHECK([test 1 = `ovn-sbctl dump-flows lr0 | grep lr_in_ip_routing | \ > +grep "ip4.src == 10.0.0.4 && is_chassis_resident(\"foo2\")" -c`]) > + > +key=`ovn-sbctl --bare --columns tunnel_key list datapath_Binding lr0` > +# Check that the OVS flows appear for the dnat_and_snat entries in > +# lr_in_ip_routing table. > +OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-ofctl dump-flows br-int table=17 | \ > +grep "priority=400,ip,metadata=0x$key,nw_src=10.0.0.3" -c`]) > + > +OVS_WAIT_UNTIL([test 1 = `as hv3 ovs-ofctl dump-flows br-int table=17 | \ > +grep "priority=400,ip,metadata=0x$key,nw_src=10.0.0.4" -c`]) > + > # Re-add nat-addresses option > ovn-nbctl lsp-set-options lrp0-rp router-port=lrp0 nat-addresses="router" > > ************** > > Thanks > Numan > > > --- > > Changes since v2: > > - fix memory leak in add_distributed_routes > > - move check on logical router port before running add_distributed_routes > > > > Changes since v1: > > - add comments > > - cosmetics > > --- > > northd/ovn-northd.8.xml | 31 ++++++++++++++++++++++++ > > northd/ovn-northd.c | 53 ++++++++++++++++++++++++++++++++++++++++- > > 2 files changed, 83 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 3a24d3c4f..0d43322cf 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -6972,6 +6972,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; > > @@ -7359,6 +7361,40 @@ 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; > > + > > + 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); > > + } > > + > > + ds_destroy(&actions); > > + ds_destroy(&match); > > +} > > + > > static void > > add_route(struct hmap *lflows, const struct ovn_port *op, > > const char *lrp_addr_s, const char *network_s, int plen, > > @@ -7380,6 +7416,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 be 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 = ", > > @@ -8948,7 +8990,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_); > > } > > > > @@ -9229,6 +9271,15 @@ 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) { > > + if (od->nbr && od->l3dgw_port) { > > + 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 > > -- > > 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 3a24d3c4f..0d43322cf 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -6972,6 +6972,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; @@ -7359,6 +7361,40 @@ 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; + + 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); + } + + ds_destroy(&actions); + ds_destroy(&match); +} + static void add_route(struct hmap *lflows, const struct ovn_port *op, const char *lrp_addr_s, const char *network_s, int plen, @@ -7380,6 +7416,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 be 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 = ", @@ -8948,7 +8990,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_); } @@ -9229,6 +9271,15 @@ 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) { + if (od->nbr && od->l3dgw_port) { + 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 v2: - fix memory leak in add_distributed_routes - move check on logical router port before running add_distributed_routes Changes since v1: - add comments - cosmetics --- northd/ovn-northd.8.xml | 31 ++++++++++++++++++++++++ northd/ovn-northd.c | 53 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-)