> > Hi Lorenzo, > > I only had one minor comment down below. It's one of those things that > likely gets optimized by the compiler anyway, so I'm not too concerned > about it.
Hi Mark, thx for review, I will fix it in v2. Regards, Lorenzo > > Acked-by: Mark Michelson <[email protected]> > > On 4/4/19 1:07 PM, Lorenzo Bianconi wrote: > > When DVR is enabled FIP traffic need to be forwarded directly using > > external connection to the underlay network and not be distributed > > through geneve tunnels. > > Fix this adding new logical flows to take care of distributed DNAT/SNAT > > > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > --- > > ovn/northd/ovn-northd.8.xml | 51 ++++++++++++++++++ > > ovn/northd/ovn-northd.c | 105 ++++++++++++++++++++++++++++++++++++ > > 2 files changed, 156 insertions(+) > > > > diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml > > index 392a5efc9..1cec0a69d 100644 > > --- a/ovn/northd/ovn-northd.8.xml > > +++ b/ovn/northd/ovn-northd.8.xml > > @@ -1828,6 +1828,37 @@ output; > > </p> > > > > <ul> > > + <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 ip source/destination couple that matches > > the > > + <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 two following NAT rules have been configured: > > + </p> > > + > > + <pre> > > +external_ip{0,1} = <var>EIP{0,1}</var>; > > +external_mac{0,1} = <var>MAC{0,1}</var>; > > +logical_ip{0,1} = <var>LIP{0,1}</var>; > > + </pre> > > + > > + <p> > > + the following action will be applied: > > + </p> > > + > > + <pre> > > +eth.dst = <var>MAC0</var>; > > +eth.src = <var>MAC1</var>; > > +reg0 = ip4.dst; > > +reg1 = <var>EIP1</var>; > > +outport = <code>redirect-chassis-port</code>; > > +<code>REGBIT_DISTRIBUTED_NAT = 1; next;</code>. > > + </pre> > > + </li> > > + > > <li> > > <p> > > For distributed logical routers where one of the logical router > > @@ -1924,6 +1955,12 @@ next; > > > > <ul> > > <li> > > + <p> > > + For distributed logical routers where one of the logical router > > + ports specifies a <code>redirect-chassis</code>, a priority-400 > > + logical flow with match <code>REGBIT_DISTRIBUTED_NAT == 1</code> > > + has action <code>next;</code> > > + </p> > > <p> > > For distributed logical routers where one of the logical router > > ports specifies a <code>redirect-chassis</code>, a priority-200 > > @@ -2012,6 +2049,11 @@ next; > > </p> > > > > <ul> > > + <li> > > + A priority-300 logical flow with match > > + <code>REGBIT_DISTRIBUTED_NAT == 1</code> has action > > + <code>next;</code> > > + </li> > > <li> > > A priority-200 logical flow with match > > <code>REGBIT_NAT_REDIRECT == 1</code> has actions > > @@ -2321,6 +2363,15 @@ nd_ns { > > > > <ul> > > <li> > > + <p> > > + For each <code>dnat_and_snat</code> NAT rule couple in the > > + OVN Northbound database on a distributed router, > > + a priority-200 logical with match > > + <code>ip4.dst == <var>external_ip0</var> && > > + ip4.src == <var>external_ip1</var></code>, has action > > + <code>next;</code> > > + </p> > > + > > <p> > > For each NAT rule in the OVN Northbound database on a > > distributed router, a priority-100 logical flow with match > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > > index 05b8aad4f..0f42b2ba5 100644 > > --- a/ovn/northd/ovn-northd.c > > +++ b/ovn/northd/ovn-northd.c > > @@ -179,6 +179,7 @@ enum ovn_stage { > > * logical router dropping packets with source IP address equals > > * one of the logical router's own IP addresses. */ > > #define REGBIT_EGRESS_LOOPBACK "reg9[1]" > > +#define REGBIT_DISTRIBUTED_NAT "reg9[2]" > > > > /* Returns an "enum ovn_stage" built from the arguments. */ > > static enum ovn_stage > > @@ -4678,6 +4679,66 @@ find_lrp_member_ip(const struct ovn_port *op, const > > char *ip_s) > > return NULL; > > } > > > > +static void > > +add_distributed_nat_routes(struct hmap *lflows, const struct ovn_port *op) > > +{ > > + struct ds actions = DS_EMPTY_INITIALIZER; > > + struct ds match = DS_EMPTY_INITIALIZER; > > + > > + if (!op->od->l3dgw_port) { > > + return; > > + } > > + > > + for (size_t i = 0; i < op->od->nbr->n_nat; i++) { > > + const struct nbrec_nat *nat = op->od->nbr->nat[i]; > > + bool found = false; > > + > > + if (strcmp(nat->type, "dnat_and_snat") || > > + !nat->external_mac || !nat->external_ip) { > > + continue; > > + } > > + > > + if (!op->peer || !op->peer->od->nbs) { > > + continue; > > + } > > I think this if statement could be moved outside the for loop. > > > + > > + const struct ovn_datapath *peer_dp = op->peer->od; > > + for (size_t j = 0; j < peer_dp->nbs->n_ports; j++) { > > + if (!strcmp(peer_dp->nbs->ports[j]->name, nat->logical_port)) { > > + found = true; > > + break; > > + } > > + } > > + if (!found) { > > + continue; > > + } > > + > > + for (size_t j = 0; j < op->od->nbr->n_nat; j++) { > > + const struct nbrec_nat *nat2 = op->od->nbr->nat[j]; > > + > > + if (nat == nat2 || strcmp(nat2->type, "dnat_and_snat") || > > + !nat2->external_mac || !nat2->external_ip) > > + continue; > > + > > + ds_put_format(&match, "inport == %s && " > > + "ip4.src == %s && ip4.dst == %s", > > + op->json_key, nat->logical_ip, > > nat2->external_ip); > > + ds_put_format(&actions, "outport = %s; " > > + "eth.src = %s; eth.dst = %s; " > > + "reg0 = ip4.dst; reg1 = %s; " > > + REGBIT_DISTRIBUTED_NAT" = 1; " > > + REGBIT_NAT_REDIRECT" = 0; next;", > > + op->od->l3dgw_port->json_key, > > + op->od->l3dgw_port->lrp_networks.ea_s, > > + nat2->external_mac, nat->external_ip); > > + ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_ROUTING, 400, > > + 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, > > @@ -6061,6 +6122,41 @@ build_lrouter_flows(struct hmap *datapaths, struct > > hmap *ports, > > * ingress pipeline with inport = outport. */ > > if (od->l3dgw_port) { > > /* Distributed router. */ > > + if (!strcmp(nat->type, "dnat_and_snat") && > > + nat->external_mac && nat->external_ip) { > > + for (int j = 0; j < od->nbr->n_nat; j++) { > > + const struct nbrec_nat *nat2 = od->nbr->nat[j]; > > + > > + if (nat2 == nat || > > + strcmp(nat2->type, "dnat_and_snat") || > > + !nat2->external_mac || !nat2->external_ip) { > > + continue; > > + } > > + > > + ds_clear(&match); > > + ds_put_format(&match, "is_chassis_resident(\"%s\") > > && " > > + "ip4.src == %s && ip4.dst == %s", > > + nat->logical_port, nat2->external_ip, > > + nat->external_ip); > > + ds_clear(&actions); > > + ds_put_format(&actions, > > + "inport = outport; outport = \"\"; " > > + "flags = 0; flags.loopback = 1; " > > + REGBIT_EGRESS_LOOPBACK" = 1; " > > + "next(pipeline=ingress, table=0); "); > > + ovn_lflow_add(lflows, od, S_ROUTER_OUT_EGR_LOOP, > > 300, > > + ds_cstr(&match), ds_cstr(&actions)); > > + > > + ds_clear(&match); > > + ds_put_format(&match, > > + "ip4.src == %s && ip4.dst == %s", > > + nat2->external_ip, nat->external_ip); > > + ovn_lflow_add(lflows, od, S_ROUTER_OUT_EGR_LOOP, > > 200, > > + ds_cstr(&match), "next;"); > > + ds_clear(&match); > > + } > > + } > > + > > ds_clear(&match); > > ds_put_format(&match, "ip4.dst == %s && outport == %s", > > nat->external_ip, > > @@ -6132,6 +6228,9 @@ build_lrouter_flows(struct hmap *datapaths, struct > > hmap *ports, > > ovn_lflow_add(lflows, od, S_ROUTER_IN_DNAT, 50, > > "ip", "flags.loopback = 1; ct_dnat;"); > > } else { > > + ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 400, > > + REGBIT_DISTRIBUTED_NAT" == 1", "next;"); > > + > > /* For NAT on a distributed router, add flows to Ingress > > * IP Routing table, Ingress ARP Resolution table, and > > * Ingress Gateway Redirect Table that are not specific to a > > @@ -6367,6 +6466,9 @@ build_lrouter_flows(struct hmap *datapaths, struct > > hmap *ports, > > continue; > > } > > > > + /* create logical flows for DVR floating IPs */ > > + add_distributed_nat_routes(lflows, op); > > + > > for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { > > add_route(lflows, op, op->lrp_networks.ipv4_addrs[i].addr_s, > > op->lrp_networks.ipv4_addrs[i].network_s, > > @@ -6607,6 +6709,9 @@ build_lrouter_flows(struct hmap *datapaths, struct > > hmap *ports, > > continue; > > } > > if (od->l3dgw_port && od->l3redirect_port) { > > + ovn_lflow_add(lflows, od, S_ROUTER_IN_GW_REDIRECT, 300, > > + REGBIT_DISTRIBUTED_NAT" == 1", "next;"); > > + > > /* For traffic with outport == l3dgw_port, if the > > * packet did not match any higher priority redirect > > * rule, then the traffic is redirected to the central > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
