Please check out the v2: https://patchwork.ozlabs.org/project/ovn/patch/20230121164609.3625347-1-odiv...@gmail.com/
Regards, Vladislav Odintsov > On 21 Jan 2023, at 15:47, Vladislav Odintsov <odiv...@gmail.com> wrote: > > Hi Simon, > > thanks for the review. > I agree with all your comments and I’ll address them in v2. > > Regards, > Vladislav Odintsov > >> On 18 Jan 2023, at 16:08, Simon Horman <simon.hor...@corigine.com> wrote: >> >> On Thu, Dec 22, 2022 at 08:43:08PM +0300, Vladislav Odintsov wrote: >>> There were two issues prior to this patch: >>> 1. It was unable to have connectivity to networks over a router in >>> physical network connected through VTEP (ramp) gateway. >>> Consider next topology: >>> >>> ovn-nbctl lr-add lr1 >>> ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24 >>> ovn-nbctl ls-add ls1 >>> ovn-nbctl lrp-add ls1 lsp1 -- \ >>> lsp-set-addresses lsp1 router -- \ >>> lsp-set-type lsp1 router -- \ >>> lsp-set-options lsp1 router-port=lrp1 >>> ovn-nbctl lsp-add ls1 lsp-vtep -- \ >>> lsp-set-type lsp-vtep vtep -- \ >>> lsp-set-addresses lsp-vtep unknown -- \ >>> lsp-set-options lsp-vtep vtep-physical-switch=<..> >>> vtep-logical-switch=<..> >>> ovn-nbctl lr-route-add lr1 192.168.0.0/24 10.0.0.100 >>> >>> If one issues ping from lsp1 to some address from 192.168.0.0/24 (via >>> vtep lsp), to enable routing support with vtep it is required to set >>> redirect chassis or ha chassis group on lrp1. This topology didn't >>> provide connectivity. Now such traffic flow will work properly. >>> >>> 2. Traffic from lport in one subnet to vtep lport in another subnet of >>> same LR previously traversed via l3gw chassis, now in 'to vtep lport' >>> direction goes directly from hypervisor handling lport to VTEP (RAMP) >>> switch. In the opposite direction traffic still goes from VTEP (RAMP) >>> switch through l3gw chassis and then to hypervisor. >> >> I think it would be good to explain how the logic changes >> addresses these problems. And breifly describe which tests have been added. >> >>> >>> Signed-off-by: Vladislav Odintsov <odiv...@gmail.com> >>> --- >>> northd/northd.c | 16 +++++++++++++++- >>> tests/ovn-northd.at | 26 +++++++++++++++++++++++++- >>> 2 files changed, 40 insertions(+), 2 deletions(-) >>> >>> diff --git a/northd/northd.c b/northd/northd.c >>> index 4751feab4..07fb0ab9a 100644 >>> --- a/northd/northd.c >>> +++ b/northd/northd.c >>> @@ -3362,7 +3362,12 @@ ovn_port_update_sbrec(struct northd_input >>> *input_data, >>> } >>> smap_add(&new, "distributed-port", op->nbrp->name); >>> >>> - bool always_redirect = !op->od->has_distributed_nat; >>> + bool always_redirect = !( >>> + op->od->has_distributed_nat || >>> + (op->l3dgw_port->peer && >>> + op->l3dgw_port->peer->od->has_vtep_lports) >>> + ); >> >> This seems slightly easier on my brain: >> >> bool always_redirect = >> !op->od->has_distributed_nat && >> !(op->l3dgw_port->peer && >> op->l3dgw_port->peer->od->has_vtep_lports) >> >>> + >>> if (redirect_type) { >>> smap_add(&new, "redirect-type", redirect_type); >>> /* XXX Why can't we enable always-redirect when >>> redirect-type >>> @@ -12815,6 +12820,15 @@ build_gateway_redirect_flows_for_lrouter( >>> return; >>> } >>> for (size_t i = 0; i < od->n_l3dgw_ports; i++) { >>> + if (od->l3dgw_ports[i]->peer && >>> + od->l3dgw_ports[i]->peer->od->has_vtep_lports) { >> >> This condition appears twice in this patch. >> Perhaps a helper would make its meaning more obvious. >> >>> + /* Skip adding redirect rule for vtep-enabled l3dgw ports. >>> + Traffic from hypervisor to VTEP (ramp) switch should go in >>> + distributed manner. Only returning routed traffic must go >>> + through centralized gateway (or ha-chassis-group). */ >> >> nit: >> /* Multi-line comments >> * look like this. */ >> >>> + continue; >>> + } >>> + >>> const struct ovsdb_idl_row *stage_hint = NULL; >>> bool add_def_flow = true; >>> >> >> ... > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev