On 5/26/23 10:21, Lorenzo Bianconi wrote: > In the current codebase for distributed gw router port use-case, > it is not possible to add a load balancer that redirects the traffic > to a back-end if it is used as internal IP of a FIP NAT rule since > the reply traffic is never centralized. Fix the issue centralizing the > traffic if it is the reply packet for the load balancer. > > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2023609 > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > Changes since v3: > - add ovn unit-test and get rid of system-ovn unit-test. > - minor changes in ovn-northd unit-test. > Changes since v2: > - rebase on top of ovn main branch > - fix spelling > Changes since v1: > - add new system-test and unit-test > --- > northd/northd.c | 15 +++++ > northd/ovn-northd.8.xml | 16 +++++ > tests/ovn-northd.at | 50 +++++++++++++++ > tests/ovn.at | 137 ++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 218 insertions(+) > > diff --git a/northd/northd.c b/northd/northd.c > index a6eca916b..6317c36fd 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -10740,6 +10740,8 @@ build_distr_lrouter_nat_flows_for_lb(struct > lrouter_nat_lb_flows_ctx *ctx, > struct ovn_datapath *od) > { > struct ovn_port *dgp = od->l3dgw_ports[0]; > + struct ds gw_redir_match = DS_EMPTY_INITIALIZER; > + struct ds gw_redir_action = DS_EMPTY_INITIALIZER; > > const char *undnat_action; > > @@ -10784,6 +10786,17 @@ build_distr_lrouter_nat_flows_for_lb(struct > lrouter_nat_lb_flows_ctx *ctx, > return; > } > > + /* We need to centralize the LB traffic to properly perform > + * the undnat stage. > + */ > + ds_put_format(&gw_redir_match, "%s) && outport == %s", > + ds_cstr(ctx->undnat_match), dgp->json_key); > + ds_put_format(&gw_redir_action, "outport = %s; next;", > + dgp->cr_port->json_key);
Hi, Lorenzo. Not a full review, but this part of a code is potentially on a hot path in northd processing, so it's better to limit the amount of memory allocations. It should be easy to use ctx->undnat_match instead of gw_redir_match and truncate it back after the lflow creation. And we could store gw_redir_action in the ctx structure and only ds_clear() it before re-using. This should help with avoiding memory allocations per datapath. Best regards, Ilya Maximets. > + ovn_lflow_add_with_hint(ctx->lflows, od, S_ROUTER_IN_GW_REDIRECT, > + 200, ds_cstr(&gw_redir_match), > + ds_cstr(&gw_redir_action), > &ctx->lb->nlb->header_); > + > ds_put_format(ctx->undnat_match, ") && (inport == %s || outport == %s)" > " && is_chassis_resident(%s)", dgp->json_key, > dgp->json_key, > dgp->cr_port->json_key); > @@ -10791,6 +10804,8 @@ build_distr_lrouter_nat_flows_for_lb(struct > lrouter_nat_lb_flows_ctx *ctx, > ds_cstr(ctx->undnat_match), undnat_action, > &ctx->lb->nlb->header_); > ds_truncate(ctx->undnat_match, undnat_match_len); > + ds_destroy(&gw_redir_match); > + ds_destroy(&gw_redir_action); > } _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
