| Message ID | 660a9954df90398d93b6f754faac449f193acc9c.1664377841.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | [ovs-dev,v2] northd: do not centralize FIP traffic if redirect-type is set to fixed | expand |
Thanks Lorenzo, Acked-by: Mark Michelson <mmichels@redhat.com> On 9/28/22 11:13, Lorenzo Bianconi wrote: > Keep FIP traffic distributed and do not centralize it even if the > CMS sets redirect-type option to bridged for distributed gateway port. > > Tested-by: Jianlin Shi <jishi@redhat.com> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2007120 > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > Changes since v1: > - add unit-test > - fix typos > --- > northd/northd.c | 29 +++++++++++++++++++++++++++++ > northd/northd.h | 2 ++ > northd/ovn-northd.8.xml | 17 +++++++++++++++++ > tests/ovn-northd.at | 37 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 85 insertions(+) > > diff --git a/northd/northd.c b/northd/northd.c > index 84440a47f..a60467963 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -2616,6 +2616,13 @@ join_logical_ports(struct northd_input *input_data, > op->od = od; > ovs_list_push_back(&od->port_list, &op->dp_node); > > + if (!od->redirect_bridged) { > + const char *redirect_type = > + smap_get(&nbrp->options, "redirect-type"); > + od->redirect_bridged = > + redirect_type && !strcasecmp(redirect_type, "bridged"); > + } > + > if (op->nbrp->ha_chassis_group || > op->nbrp->n_gateway_chassis) { > /* Additional "derived" ovn_port crp represents the > @@ -13731,6 +13738,28 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, > 100, ds_cstr(match), > ds_cstr(actions), > &nat->header_); > + if (od->redirect_bridged && distributed) { > + ds_clear(match); > + ds_put_format( > + match, > + "outport == %s && ip%s.src == %s " > + "&& is_chassis_resident(\"%s\")", > + od->l3dgw_ports[0]->json_key, > + is_v6 ? "6" : "4", nat->logical_ip, > + nat->logical_port); > + ds_clear(actions); > + if (is_v6) { > + ds_put_cstr(actions, > + "get_nd(outport, " REG_NEXT_HOP_IPV6 "); next;"); > + } else { > + ds_put_cstr(actions, > + "get_arp(outport, " REG_NEXT_HOP_IPV4 "); next;"); > + } > + ovn_lflow_add_with_hint(lflows, od, > + S_ROUTER_IN_ARP_RESOLVE, 90, > + ds_cstr(match), ds_cstr(actions), > + &nat->header_); > + } > sset_add(&nat_entries, nat->external_ip); > } > } > diff --git a/northd/northd.h b/northd/northd.h > index aa9a3ae6e..60601803f 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -229,6 +229,8 @@ struct ovn_datapath { > size_t n_nat_entries; > > bool has_distributed_nat; > + /* router datapath has a logical port with redirect-type set to bridged. */ > + bool redirect_bridged; > > /* Set of nat external ips on the router. */ > struct sset external_ips; > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index d9e9a7345..009a380bd 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -4053,6 +4053,23 @@ outport = <var>P</var> > </p> > </li> > > + <li> > + <p> > + If the router datapath runs a port with <code>redirect-type</code> > + set to <code>bridged</code>, for each distributed NAT rule with IP > + <var>A</var> in the > + <ref column="logical_ip" table="NAT" db="OVN_Northbound"/> column > + and logical port <var>P</var> in the > + <ref column="logical_port" table="NAT" db="OVN_Northbound"/> column > + of <ref table="NAT" db="OVN_Northbound"/> table, a priority-90 flow > + with the match <code>outport == <var>Q</var> && ip.src === > + <var>A</var> && is_chassis_resident(<var>P</var>)</code>, > + where <code>Q</code> is the distributed logical router port and > + action <code>get_arp(outport, reg0); next;</code> for IPv4 and > + <code>get_nd(outport, xxreg0); next;</code> for IPv6. > + </p> > + </li> > + > <li> > <p> > Traffic with IP destination an address owned by the router should be > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 093e01c6d..a210fc575 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -7861,3 +7861,40 @@ check_column "" sb:load_balancer datapaths name=lb0 > > AT_CLEANUP > ]) > + > +AT_SETUP([check fip flows with redirect-type bridged]) > +AT_KEYWORDS([fip-redirect-type-bridged]) > +ovn_start > + > +ovn-nbctl lr-add R1 > +ovn-nbctl lrp-add R1 R1-S0 02:ac:10:01:00:01 10.0.0.1/24 1000::a/64 > +ovn-nbctl lrp-add R1 R1-PUB 02:ac:20:01:01:01 172.16.0.1/24 3000::a/64 > +ovn-nbctl lrp-set-gateway-chassis R1-PUB hv1 20 > + > +ovn-nbctl ls-add S0 > +ovn-nbctl lsp-add S0 S0-R1 > +ovn-nbctl lsp-set-type S0-R1 router > +ovn-nbctl lsp-set-addresses S0-R1 02:ac:10:01:00:01 > +ovn-nbctl lsp-set-options S0-R1 router-port=R1-S0 > +ovn-nbctl lsp-add S0 S0-P0 > +ovn-nbctl lsp-set-addresses S0-P0 "50:54:00:00:00:03 10.0.0.3 1000::3" > + > +ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.0.110 10.0.0.3 S0-P0 30:54:00:00:00:03 > +ovn-nbctl lr-nat-add R1 dnat_and_snat 3000::c 1000::3 S0-P0 40:54:00:00:00:03 > + > +ovn-sbctl dump-flows R1 > R1flows > +AT_CAPTURE_FILE([R1flows]) > +AT_CHECK([grep "lr_in_arp_resolve" R1flows | grep priority=90 | sort], [0], [dnl > +]) > + > +ovn-nbctl --wait=sb set logical_router_port R1-PUB options:redirect-type=bridged > +ovn-sbctl dump-flows R1 > R1flows > +AT_CAPTURE_FILE([R1flows]) > + > +AT_CHECK([grep "lr_in_arp_resolve" R1flows | grep priority=90 | sort], [0], [dnl > + table=15(lr_in_arp_resolve ), priority=90 , match=(outport == "R1-PUB" && ip4.src == 10.0.0.3 && is_chassis_resident("S0-P0")), action=(get_arp(outport, reg0); next;) > + table=15(lr_in_arp_resolve ), priority=90 , match=(outport == "R1-PUB" && ip6.src == 1000::3 && is_chassis_resident("S0-P0")), action=(get_nd(outport, xxreg0); next;) > +]) > + > +AT_CLEANUP > +])
On 9/29/22 20:23, Mark Michelson wrote: > Thanks Lorenzo, > > Acked-by: Mark Michelson <mmichels@redhat.com> Thanks, Lorenzo and Mark! I applied this to the main branch.
On 9/30/22 12:43, Dumitru Ceara wrote: > On 9/29/22 20:23, Mark Michelson wrote: >> Thanks Lorenzo, >> >> Acked-by: Mark Michelson <mmichels@redhat.com> > > Thanks, Lorenzo and Mark! > > I applied this to the main branch. We received a request to backport this bug fix all the way down to branch-21.12 (from Luis in CC). I'm planning to do that but I'll wait a day or two to give people some time to raise any points they may have. Regards, Dumitru
On 3/22/23 10:13, Dumitru Ceara wrote: > On 9/30/22 12:43, Dumitru Ceara wrote: >> On 9/29/22 20:23, Mark Michelson wrote: >>> Thanks Lorenzo, >>> >>> Acked-by: Mark Michelson <mmichels@redhat.com> >> >> Thanks, Lorenzo and Mark! >> >> I applied this to the main branch. > > We received a request to backport this bug fix all the way down to > branch-21.12 (from Luis in CC). I'm planning to do that but I'll wait a > day or two to give people some time to raise any points they may have. > I backported this all the way down to branch-21.12. Regards, Dumitru
diff --git a/northd/northd.c b/northd/northd.c index 84440a47f..a60467963 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -2616,6 +2616,13 @@ join_logical_ports(struct northd_input *input_data, op->od = od; ovs_list_push_back(&od->port_list, &op->dp_node); + if (!od->redirect_bridged) { + const char *redirect_type = + smap_get(&nbrp->options, "redirect-type"); + od->redirect_bridged = + redirect_type && !strcasecmp(redirect_type, "bridged"); + } + if (op->nbrp->ha_chassis_group || op->nbrp->n_gateway_chassis) { /* Additional "derived" ovn_port crp represents the @@ -13731,6 +13738,28 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od, struct hmap *lflows, 100, ds_cstr(match), ds_cstr(actions), &nat->header_); + if (od->redirect_bridged && distributed) { + ds_clear(match); + ds_put_format( + match, + "outport == %s && ip%s.src == %s " + "&& is_chassis_resident(\"%s\")", + od->l3dgw_ports[0]->json_key, + is_v6 ? "6" : "4", nat->logical_ip, + nat->logical_port); + ds_clear(actions); + if (is_v6) { + ds_put_cstr(actions, + "get_nd(outport, " REG_NEXT_HOP_IPV6 "); next;"); + } else { + ds_put_cstr(actions, + "get_arp(outport, " REG_NEXT_HOP_IPV4 "); next;"); + } + ovn_lflow_add_with_hint(lflows, od, + S_ROUTER_IN_ARP_RESOLVE, 90, + ds_cstr(match), ds_cstr(actions), + &nat->header_); + } sset_add(&nat_entries, nat->external_ip); } } diff --git a/northd/northd.h b/northd/northd.h index aa9a3ae6e..60601803f 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -229,6 +229,8 @@ struct ovn_datapath { size_t n_nat_entries; bool has_distributed_nat; + /* router datapath has a logical port with redirect-type set to bridged. */ + bool redirect_bridged; /* Set of nat external ips on the router. */ struct sset external_ips; diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index d9e9a7345..009a380bd 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -4053,6 +4053,23 @@ outport = <var>P</var> </p> </li> + <li> + <p> + If the router datapath runs a port with <code>redirect-type</code> + set to <code>bridged</code>, for each distributed NAT rule with IP + <var>A</var> in the + <ref column="logical_ip" table="NAT" db="OVN_Northbound"/> column + and logical port <var>P</var> in the + <ref column="logical_port" table="NAT" db="OVN_Northbound"/> column + of <ref table="NAT" db="OVN_Northbound"/> table, a priority-90 flow + with the match <code>outport == <var>Q</var> && ip.src === + <var>A</var> && is_chassis_resident(<var>P</var>)</code>, + where <code>Q</code> is the distributed logical router port and + action <code>get_arp(outport, reg0); next;</code> for IPv4 and + <code>get_nd(outport, xxreg0); next;</code> for IPv6. + </p> + </li> + <li> <p> Traffic with IP destination an address owned by the router should be diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 093e01c6d..a210fc575 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -7861,3 +7861,40 @@ check_column "" sb:load_balancer datapaths name=lb0 AT_CLEANUP ]) + +AT_SETUP([check fip flows with redirect-type bridged]) +AT_KEYWORDS([fip-redirect-type-bridged]) +ovn_start + +ovn-nbctl lr-add R1 +ovn-nbctl lrp-add R1 R1-S0 02:ac:10:01:00:01 10.0.0.1/24 1000::a/64 +ovn-nbctl lrp-add R1 R1-PUB 02:ac:20:01:01:01 172.16.0.1/24 3000::a/64 +ovn-nbctl lrp-set-gateway-chassis R1-PUB hv1 20 + +ovn-nbctl ls-add S0 +ovn-nbctl lsp-add S0 S0-R1 +ovn-nbctl lsp-set-type S0-R1 router +ovn-nbctl lsp-set-addresses S0-R1 02:ac:10:01:00:01 +ovn-nbctl lsp-set-options S0-R1 router-port=R1-S0 +ovn-nbctl lsp-add S0 S0-P0 +ovn-nbctl lsp-set-addresses S0-P0 "50:54:00:00:00:03 10.0.0.3 1000::3" + +ovn-nbctl lr-nat-add R1 dnat_and_snat 172.16.0.110 10.0.0.3 S0-P0 30:54:00:00:00:03 +ovn-nbctl lr-nat-add R1 dnat_and_snat 3000::c 1000::3 S0-P0 40:54:00:00:00:03 + +ovn-sbctl dump-flows R1 > R1flows +AT_CAPTURE_FILE([R1flows]) +AT_CHECK([grep "lr_in_arp_resolve" R1flows | grep priority=90 | sort], [0], [dnl +]) + +ovn-nbctl --wait=sb set logical_router_port R1-PUB options:redirect-type=bridged +ovn-sbctl dump-flows R1 > R1flows +AT_CAPTURE_FILE([R1flows]) + +AT_CHECK([grep "lr_in_arp_resolve" R1flows | grep priority=90 | sort], [0], [dnl + table=15(lr_in_arp_resolve ), priority=90 , match=(outport == "R1-PUB" && ip4.src == 10.0.0.3 && is_chassis_resident("S0-P0")), action=(get_arp(outport, reg0); next;) + table=15(lr_in_arp_resolve ), priority=90 , match=(outport == "R1-PUB" && ip6.src == 1000::3 && is_chassis_resident("S0-P0")), action=(get_nd(outport, xxreg0); next;) +]) + +AT_CLEANUP +])