> On 4/28/23 15:57, 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 backed if it is even the 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]>
> > ---
>
> Hi Lorenzo,
Hi Dumitru,
thx for the review.
>
> > northd/northd.c | 15 +++++++++++++++
> > northd/ovn-northd.8.xml | 14 ++++++++++++++
>
> Would it be possible to add a unit test (ovn.at/ovn-northd.at) that
> depicts the logical flows that need to be installed and a system test
> (system-ovn.at) for this change?
ack, will do
>
> It would really help understand what's changing here.
>
> > 2 files changed, 29 insertions(+)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index d59a54b32..e33289b18 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -10680,6 +10680,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;
> >
> > @@ -10724,6 +10726,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);
> > + 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);
> > @@ -10731,6 +10744,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);
> > }
> >
> > static void
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index 70153dc9e..46afaeb82 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -4440,6 +4440,20 @@ icmp6 {
> > </p>
> >
> > <ul>
> > + <li>
> > + For all the configured load balancing rules that includes an IPv4
> > + address <var>VIP</var>, for every backend IPv4 address <var>B</var>
> > + defined for the <var>VIP</var> a priority-200 flow that matches
>
> I really thought you meant a flow for each backend but I think it's
> actually a single lflow for all backends, right?
yep, correct.
Regards,
Lorenzo
>
> > + <code>ip && ip4.src == <var>B</var></code> with an action
> > + <code>outport = <var>CR</var>; next;</code> where <var>CR</var> is
> > the
> > + <code>chassisredirect</code> port representing the instance of the
> > + logical router distributed gateway port on the gateway chassis.
> > + If the backend IPv4 address <var>B</var> is also configured with
> > + L4 port <var>PORT</var> of protocol <var>P</var>, then the match
> > + also includes <code>P.src</code> == <var>PORT</var>.
> > + Similar flows are addeded for IPv6 counterpart.
> > + </li>
> > +
> > <li>
> > For each NAT rule in the OVN Northbound database that can
> > be handled in a distributed manner, a priority-100 logical
>
> Thanks,
> Dumitru
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev