> 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 &amp;&amp; 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

Reply via email to