[ovs-dev] [PATCH ovn] northd: do not insert remote FIP entries in S_ROUTER_IN_ARP_RESOLVE stage

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Tue Mar 17 17:10:59 UTC 2020


> On Thu, Mar 12, 2020 at 7:43 PM Lorenzo Bianconi
> <lorenzo.bianconi at redhat.com> wrote:
> >
> > Do not insert FIP entries in S_ROUTER_IN_ARP_RESOLVE stage if the
> > destination is not resident on the local chassis since the the L2
> > address will be resolved by the arp action
> 
> Hi Lorenzo,
> 

Hi Numan,

thx for the review.

> If eth.dst for the FIP can be resolved using the arp action, then is
> there a need to add these
> flows at all ? If so, we can probably just remove these lflows from
> the S_ROUTER_IN_ARP_RESOLVE
> stage right ?

there is just one case where we need these flows, when both FIPs are on the
same hv (e.g ovn -- DNAT and SNAT on distributed router - E/W in
system-ovn.at). In this case the arp action can't resolve the L2 address.
So IIUC we can't remove them. Do you agree?

Regards,
Lorenzo

> 
> Right now for N dnat_and_snat entries defined for a logical router
> we are adding N logical flows in S_ROUTER_IN_ARP_RESOLVE stage. Even
> after this patch we would
> still do that. Just that these lflows will not be installed on other
> chassis. I don't see much benefit in this patch.
> 
> I think it'll add benefit if we remove these lflows altogether. what
> do you think?
> 
> Thanks
> Numan
> 
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> > ---
> >  northd/ovn-northd.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 921fe1865..16710c6cd 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -8847,6 +8847,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> >                          &match, "outport == %s && %s == %s",
> >                          od->l3dgw_port->json_key,
> >                          is_v6 ? "xxreg0" : "reg0", nat->external_ip);
> > +                    if (distributed) {
> > +                        ds_put_format(&match,
> > +                                      " && is_chassis_resident(\"%s\")",
> > +                                      nat->logical_port);
> > +                    }
> >                      ds_clear(&actions);
> >                      ds_put_format(
> >                          &actions, "eth.dst = %s; next;",
> > --
> > 2.24.1
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> 


More information about the dev mailing list