[ovs-dev] [PATCH ovn 2/2] northd: Fix the missing force_snat_for_lb flows when router_ip is configured.

Ben Pfaff blp at ovn.org
Thu Mar 4 20:59:54 UTC 2021


On Wed, Mar 03, 2021 at 11:32:39PM +0530, numans at ovn.org wrote:
> From: Numan Siddique <numans at ovn.org>
> 
> The commit c6e21a23bd8 which supported the option 'lb_force_snat_ip=router_ip'
> on a gateway router, missed out on
>   - updating the flows in 'lr_in_dnat' to set 'flags.force_snat_for_lb = 1'.
>   - removing the flow to drop if ip.dst == router port IP in 'lr_in_ip_input'
>     stage.
> 
> This patch fixes these issue and adds a system test to cover the
> hairpin load balancer traffic for the gateway routers.
> 
> Fixes: c6e21a23bd8("northd: Provide the Gateway router option 'lb_force_snat_ip' to take router port ips.")
> 
> CC: Ben Pfaff <blp at ovn.org>
> Signed-off-by: Numan Siddique <numans at ovn.org>

Thanks!  I took a look at the differences between the C and the DDlog
versions.  I have a few comments.

The C version adds a new flow to the output.  The DDlog version doesn't
appear to, yet I do see a similar flow in the DDlog code.  I am not sure
how that happened.

Looking at the following:
>      &RouterPort(.lrp = nb::Logical_Router_Port{._uuid = lrp_uuid},
>                  .router = &Router{.snat_ips = snat_ips,
> +                                  .force_lb_snat = force_lb_snat,
>                                    .lr = nb::Logical_Router{._uuid = lr_uuid}},
>                  .networks = networks),
>      var addr = FlatMap(networks.ipv4_addrs),
>      not snat_ips.contains_key(IPv4{addr.addr}),
> +    not force_lb_snat,
>      var match_ips = "${addr.addr}".group_by((lr_uuid, lrp_uuid)).to_vec().

You can optimize it by writing
> +                                  .force_lb_snat = false,
and then omitting the later clause.  With the way DDlog currently
works, this isn't just cosmetic, it will actually yield faster
code because DDlog will discard nonmatching records earlier.  (If
that's an actual performance problem it will come out later in
profiling, I'm just giving a tip here that I only recently learned
myself.)

Thanks,

Ben.


More information about the dev mailing list