Regards, Vladislav Odintsov
> On 19 Aug 2022, at 04:05, Han Zhou <[email protected]> wrote: > > > > On Thu, Aug 18, 2022 at 12:50 PM Vladislav Odintsov <[email protected] > <mailto:[email protected]>> wrote: > > > Regards, > Vladislav Odintsov > >> On 18 Aug 2022, at 20:07, Han Zhou <[email protected] <mailto:[email protected]>> >> wrote: >> >> >> >> On Thu, Aug 18, 2022 at 8:54 AM Vladislav Odintsov <[email protected] >> <mailto:[email protected]>> wrote: >> Hi Han, >> >> As I could understand, admission control flows were introduced for >> "gateway/outside" LSs, with GW LRPs. >> The scenario, which was covered with this change shouldn’t overlap with a >> "gateway/outside" LS scenario: >> User creates an LS, adds normal VIF ports (VMs) to LS and additionally he >> may want to attach vtep lport to this LS to extend L2 domain outside of the >> OVN with VTEP. I can’t imaging user wants to have a L3 GW LRP here - this >> could be the only one problem place. >> >> Thanks for explaining, but I am still confused. Here is your original >> description of the problem from the earlier thread: >> --- >> The initial problem is that user may want to create simple topology: >> >> <vm1> - <ls1 192.168.0.0/24 <http://192.168.0.0/24>> - <LR> - <ls2 >> 192.168.1.0/24 <http://192.168.1.0/24>> - <vm2> >> >> And add VTEP LSP to ls2 (setting GW chassis to LRP in ls2). >> If user wants to add GW services to this setup, VMs from ls2 will not be >> able to access >> external network due to lr_in_admission rules. >> --- >> So, the LRP to LS2 (Let's call it LRP-LS2) is the one with GW chassis set, >> which means it is L3 GW LRP (DGP), right? LS2 has a DGP, a VTEP, and a VIF >> (VM2), and you want to allow traffic to LRP-LS2 not only on the GW chassis, >> but also on any HVs, right? Maybe you are saying, you don't connect a >> localnet port to the LS2, and LS2 is an overlay network, instead of underlay? > > Yes this is exactly what I mean. > >> But if that's the case why would you set the gateway-chassis to it at all? > > This is the only way how to enable routing with HW VTEP gateway (which is a > L2 service). > You can check this out [1] to see more details about GW chassis for the VTEP > L3 scenario. > > I see. Sorry that I didn't know the usage of gateway chassis in this way, but > for the patch [1], I wonder if the VTEP chassis (where the > ovn-controller-vtep is running) should be responsible for replying ARP > requests for the LRP IP from the LS's ARP responder pipeline, instead of > requiring a DGP to perform this? I didn't expect DGP to be required for VTEP > to work. ovn-controller-vtep is only responsible for converting ovn-sb entities to hardware_vtep schema for futher processing by thirdparies like cumulus vtep, vtep emulator by ovs, arista vtep and so on. It doesn’t directly participate in datapath processing. > > But anyway, this answers my question to the current patch. Thanks! > Glad to help you! > Han > > 1: > https://github.com/ovn-org/ovn/commit/4e90bcf55c2ef1ab9940836455e4bfe36e57f307 > > <https://github.com/ovn-org/ovn/commit/4e90bcf55c2ef1ab9940836455e4bfe36e57f307> >> >> Thanks, >> han >> >> And this is_chassis_redirect() check is not added to flow only if VTEP lport >> is in associated LS. All other cases left untouched. >> >> Let me know if you have any questions. >> >> Regards, >> Vladislav Odintsov >> >>> On 18 Aug 2022, at 18:45, Han Zhou <[email protected] <mailto:[email protected]>> >>> wrote: >>> >>> >>> On Mon, Aug 15, 2022 at 10:01 PM Odintsov Vladislav <[email protected] >>> <mailto:[email protected]>> wrote: >>> > >>> > Thanks Numan. >>> > >>> > regards, >>> > Vladislav Odintsov >>> > >>> > > On 16 Aug 2022, at 04:56, Numan Siddique <[email protected] >>> > > <mailto:[email protected]>> wrote: >>> > > >>> > > On Tue, Aug 16, 2022 at 12:19 AM Vladislav Odintsov <[email protected] >>> > > <mailto:[email protected]>> wrote: >>> > >> >>> > >> If LRP's logical switch has attached LSP of vtep type, the >>> > >> is_chassis_resident() part is not added to lflow to allow traffic >>> > >> originated from logical switch to reach LR services (LBs, NAT). >>> > >> >>> >>> Sorry that I just noticed this and have a question here. I think we had >>> is_chassis_resident() for the admission control flows for a reason. I don't >>> remember all details, but probably one of the reasons is to prevent >>> underlay BUM packets to be handled by multiple chassises? >>> If we disable that check, would it create problems? Is VTEP attached LS >>> immune to those problems? >>> >>> Thanks, >>> Han >>> >>> > >>> > >> Signed-off-by: Vladislav Odintsov <[email protected] >>> > >> <mailto:[email protected]>> >>> > > >>> > > Thanks for v2 and for the test case. >>> > > Applied the patch to main. >>> > > >>> > > Numan >>> > > >>> > > >>> > >> --- >>> > >> This is a continuation from [1] as a v2 edition after Numan's review. >>> > >> - reworked to use od->has_vtep_lports and removed lrp's confusing >>> > >> option >>> > >> 'is_distributed' >>> > >> - added related tests >>> > >> - updated ovn-northd flows docs >>> > >> >>> > >> 1: >>> > >> https://mail.openvswitch.org/pipermail/ovs-dev/2022-August/396796.html >>> > >> <https://mail.openvswitch.org/pipermail/ovs-dev/2022-August/396796.html> >>> > >> --- >>> > >> northd/northd.c | 33 +++++++++++++++++--- >>> > >> northd/ovn-northd.8.xml | 4 +++ >>> > >> tests/ovn-northd.at <http://ovn-northd.at/> | 69 >>> > >> +++++++++++++++++++++++++++++++++++++++++ >>> > >> 3 files changed, 102 insertions(+), 4 deletions(-) >>> > >> >>> > >> diff --git a/northd/northd.c b/northd/northd.c >>> > >> index facd41a59..b1e9ffc87 100644 >>> > >> --- a/northd/northd.c >>> > >> +++ b/northd/northd.c >>> > >> @@ -637,6 +637,7 @@ struct ovn_datapath { >>> > >> bool has_lb_vip; >>> > >> bool has_unknown; >>> > >> bool has_acls; >>> > >> + bool has_vtep_lports; >>> > >> >>> > >> /* IPAM data. */ >>> > >> struct ipam_info ipam_info; >>> > >> @@ -1847,6 +1848,12 @@ lsp_is_localnet(const struct >>> > >> nbrec_logical_switch_port *nbsp) >>> > >> return !strcmp(nbsp->type, "localnet"); >>> > >> } >>> > >> >>> > >> +static bool >>> > >> +lsp_is_vtep(const struct nbrec_logical_switch_port *nbsp) >>> > >> +{ >>> > >> + return !strcmp(nbsp->type, "vtep"); >>> > >> +} >>> > >> + >>> > >> static bool >>> > >> localnet_can_learn_mac(const struct nbrec_logical_switch_port *nbsp) >>> > >> { >>> > >> @@ -2655,6 +2662,10 @@ join_logical_ports(struct northd_input >>> > >> *input_data, >>> > >> od->localnet_ports[od->n_localnet_ports++] = op; >>> > >> } >>> > >> >>> > >> + if (lsp_is_vtep(nbsp)) { >>> > >> + od->has_vtep_lports = true; >>> > >> + } >>> > >> + >>> > >> op->lsp_addrs >>> > >> = xmalloc(sizeof *op->lsp_addrs * >>> > >> nbsp->n_addresses); >>> > >> for (size_t j = 0; j < nbsp->n_addresses; j++) { >>> > >> @@ -5518,7 +5529,7 @@ build_lswitch_port_sec_op(struct ovn_port *op, >>> > >> struct hmap *lflows, >>> > >> ds_put_format(actions, "set_queue(%s); ", queue_id); >>> > >> } >>> > >> >>> > >> - if (!strcmp(op->nbsp->type, "vtep")) { >>> > >> + if (lsp_is_vtep(op->nbsp)) { >>> > >> ds_put_format(actions, REGBIT_FROM_RAMP" = 1; "); >>> > >> ds_put_format(actions, "next(pipeline=ingress, table=%d);", >>> > >> ovn_stage_get_table(S_SWITCH_IN_HAIRPIN)); >>> > >> @@ -10894,6 +10905,22 @@ build_gateway_mtu_flow(struct hmap *lflows, >>> > >> struct ovn_port *op, >>> > >> va_end(extra_actions_args); >>> > >> } >>> > >> >>> > >> +static bool >>> > >> +consider_l3dwg_port_is_centralized(struct ovn_port *op) >>> > >> +{ >>> > >> + if (op->peer && op->peer->od->has_vtep_lports) { >>> > >> + return false; >>> > >> + } >>> > >> + >>> > >> + if (is_l3dgw_port(op)) { >>> > >> + /* Traffic with eth.dst = l3dgw_port->lrp_networks.ea_s >>> > >> + * should only be received on the gateway chassis. */ >>> > >> + return true; >>> > >> + } >>> > >> + >>> > >> + return false; >>> > >> +} >>> > >> + >>> > >> /* Logical router ingress Table 0: L2 Admission Control >>> > >> * This table drops packets that the router shouldn’t see at all based >>> > >> * on their Ethernet headers. >>> > >> @@ -10930,9 +10957,7 @@ build_adm_ctrl_flows_for_lrouter_port( >>> > >> ds_clear(match); >>> > >> ds_put_format(match, "eth.dst == %s && inport == %s", >>> > >> op->lrp_networks.ea_s, op->json_key); >>> > >> - if (is_l3dgw_port(op)) { >>> > >> - /* Traffic with eth.dst = l3dgw_port->lrp_networks.ea_s >>> > >> - * should only be received on the gateway chassis. */ >>> > >> + if (consider_l3dwg_port_is_centralized(op)) { >>> > >> ds_put_format(match, " && is_chassis_resident(%s)", >>> > >> op->cr_port->json_key); >>> > >> } >>> > >> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml >>> > >> index ff21c0737..9b6459d9e 100644 >>> > >> --- a/northd/ovn-northd.8.xml >>> > >> +++ b/northd/ovn-northd.8.xml >>> > >> @@ -2114,6 +2114,10 @@ output; >>> > >> gateway chassis), the above flow matching >>> > >> <code>eth.dst == <var>E</var></code> is only programmed on >>> > >> the gateway port instance on the gateway chassis. >>> > >> + If LRP's logical switch has attached LSP of >>> > >> <code>vtep</code> type, >>> > >> + the <code>is_chassis_resident()</code> part is not added to >>> > >> lflow to >>> > >> + allow traffic originated from logical switch to reach LR >>> > >> services >>> > >> + (LBs, NAT). >>> > >> </p> >>> > >> >>> > >> <p> >>> > >> diff --git a/tests/ovn-northd.at <http://ovn-northd.at/> >>> > >> b/tests/ovn-northd.at <http://ovn-northd.at/> >>> > >> index 5b5eeb0ee..3ffa39419 100644 >>> > >> --- a/tests/ovn-northd.at <http://ovn-northd.at/> >>> > >> +++ b/tests/ovn-northd.at <http://ovn-northd.at/> >>> > >> @@ -5747,6 +5747,75 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep >>> > >> cr-DR | sed 's/table=../table=?? >>> > >> AT_CLEANUP >>> > >> ]) >>> > >> >>> > >> +OVN_FOR_EACH_NORTHD([ >>> > >> +AT_SETUP([ovn-northd -- lr admission with vtep lports]) >>> > >> +AT_KEYWORDS([multiple-l3dgw-ports]) >>> > >> +ovn_start NORTHD_TYPE >>> > >> +check ovn-sbctl chassis-add ch1 geneve 127.0.0.2 >>> > >> + >>> > >> +check ovn-nbctl lr-add lr1 >>> > >> +check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24 >>> > >> <http://10.0.0.1/24> >>> > >> +check ovn-nbctl ls-add ls1 >>> > >> +check ovn-nbctl lsp-add ls1 lsp1 -- \ >>> > >> + lsp-set-addresses lsp1 router -- \ >>> > >> + lsp-set-type lsp1 router -- \ >>> > >> + lsp-set-options lsp1 router-port=lrp1 >>> > >> + >>> > >> +# ensure initial flows are installed without is_chassis_resident >>> > >> match part >>> > >> +ovn-nbctl --wait=sb sync >>> > >> +ovn-sbctl dump-flows lr1 > lrflows >>> > >> +AT_CAPTURE_FILE([lrflows]) >>> > >> + >>> > >> +# Check the flows in lr_in_admission stage >>> > >> +AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed >>> > >> 's/table=../table=??/' | sort], [0], [dnl >>> > >> + table=??(lr_in_admission ), priority=50 , match=(eth.dst == >>> > >> 00:00:00:00:00:01 && inport == "lrp1"), action=(xreg0[[0..47]] = >>> > >> 00:00:00:00:00:01; next;) >>> > >> + table=??(lr_in_admission ), priority=50 , match=(eth.mcast && >>> > >> inport == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) >>> > >> +]) >>> > >> + >>> > >> +# make lrp a cr-port and check its flows >>> > >> +check ovn-nbctl lrp-set-gateway-chassis lrp1 ch1 >>> > >> + >>> > >> +ovn-nbctl --wait=sb sync >>> > >> +ovn-sbctl dump-flows lr1 > lrflows >>> > >> +AT_CAPTURE_FILE([lrflows]) >>> > >> + >>> > >> +# Check the flows in lr_in_admission stage >>> > >> +AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed >>> > >> 's/table=../table=??/' | sort], [0], [dnl >>> > >> + table=??(lr_in_admission ), priority=50 , match=(eth.dst == >>> > >> 00:00:00:00:00:01 && inport == "lrp1" && >>> > >> is_chassis_resident("cr-lrp1")), action=(xreg0[[0..47]] = >>> > >> 00:00:00:00:00:01; next;) >>> > >> + table=??(lr_in_admission ), priority=50 , match=(eth.mcast && >>> > >> inport == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) >>> > >> +]) >>> > >> + >>> > >> +# attach vtep logical port to logical switch and check flows. >>> > >> +# there should not be is_chassis_resident part. >>> > >> +check ovn-nbctl lsp-add ls1 lsp-vtep -- lsp-set-type lsp-vtep vtep >>> > >> + >>> > >> +ovn-nbctl --wait=sb sync >>> > >> +ovn-sbctl dump-flows lr1 > lrflows >>> > >> +AT_CAPTURE_FILE([lrflows]) >>> > >> + >>> > >> +# Check the flows in lr_in_admission stage >>> > >> +AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed >>> > >> 's/table=../table=??/' | sort], [0], [dnl >>> > >> + table=??(lr_in_admission ), priority=50 , match=(eth.dst == >>> > >> 00:00:00:00:00:01 && inport == "lrp1"), action=(xreg0[[0..47]] = >>> > >> 00:00:00:00:00:01; next;) >>> > >> + table=??(lr_in_admission ), priority=50 , match=(eth.mcast && >>> > >> inport == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) >>> > >> +]) >>> > >> + >>> > >> +# delete vtep lport and check lrp has is_chassis_resident match part >>> > >> again. >>> > >> +check ovn-nbctl lsp-del lsp-vtep >>> > >> + >>> > >> +ovn-nbctl --wait=sb sync >>> > >> +ovn-sbctl dump-flows lr1 > lrflows >>> > >> +AT_CAPTURE_FILE([lrflows]) >>> > >> + >>> > >> +# Check the flows in lr_in_admission stage >>> > >> +AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed >>> > >> 's/table=../table=??/' | sort], [0], [dnl >>> > >> + table=??(lr_in_admission ), priority=50 , match=(eth.dst == >>> > >> 00:00:00:00:00:01 && inport == "lrp1" && >>> > >> is_chassis_resident("cr-lrp1")), action=(xreg0[[0..47]] = >>> > >> 00:00:00:00:00:01; next;) >>> > >> + table=??(lr_in_admission ), priority=50 , match=(eth.mcast && >>> > >> inport == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;) >>> > >> +]) >>> > >> + >>> > >> +AT_CLEANUP >>> > >> +]) >>> > >> + >>> > >> + >>> > >> OVN_FOR_EACH_NORTHD([ >>> > >> AT_SETUP([check options:requested-chassis fills requested_chassis col]) >>> > >> ovn_start NORTHD_TYPE >>> > >> -- >>> > >> 2.36.1 >>> > >> >>> > >> _______________________________________________ >>> > >> dev mailing list >>> > >> [email protected] <mailto:[email protected]> >>> > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> > >> <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> >>> > > _______________________________________________ >>> > > dev mailing list >>> > > [email protected] <mailto:[email protected]> >>> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> > > <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> >>> > _______________________________________________ >>> > dev mailing list >>> > [email protected] <mailto:[email protected]> >>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> > <https://mail.openvswitch.org/mailman/listinfo/ovs-dev> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
