[ovs-dev] [PATCH v4 ovn 5/8] northd: get rid of add_router_lb_flow

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Fri Jul 2 22:09:07 UTC 2021


> On 7/2/21 7:16 PM, Lorenzo Bianconi wrote:
> > Remove add_router_lb_flow routine and move leftover lb flow
> > installation code in build_lrouter_snat_flows_for_lb routine
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> > ---
> 
> [...]
> 
> > @@ -11828,6 +11795,7 @@ static void
> >  build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
> >                                  struct hmap *lflows,
> >                                  struct hmap *lbs,
> > +                                struct sset *nat_entries,
> >                                  struct ds *match, struct ds *actions)
> >  {
> >      if (!od->nbr) {
> > @@ -11855,8 +11823,6 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
> >          return;
> >      }
> >  
> > -    struct sset nat_entries = SSET_INITIALIZER(&nat_entries);
> > -
> >      bool dnat_force_snat_ip =
> >          !lport_addresses_is_empty(&od->dnat_force_snat_addrs);
> >      bool lb_force_snat_ip =
> > @@ -11883,7 +11849,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
> >  
> >          /* ARP resolve for NAT IPs. */
> >          if (od->l3dgw_port) {
> > -            if (!sset_contains(&nat_entries, nat->external_ip)) {
> > +            if (!sset_contains(nat_entries, nat->external_ip)) {
> >                  ds_clear(match);
> >                  ds_put_format(
> >                      match, "outport == %s && %s == %s",
> > @@ -11900,13 +11866,13 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
> >                                          100, ds_cstr(match),
> >                                          ds_cstr(actions),
> >                                          &nat->header_);
> > -                sset_add(&nat_entries, nat->external_ip);
> > +                sset_add(nat_entries, nat->external_ip);
> 
> I think this is still not correct, like in v3; we still add *all*
> nat->external_ip from *all* logical routers to a (almost) global sset.

Hi Dumitru,

I did not get why nat_entries is not a global set. It is defined at the
beginning of ovnnb_db_run() and it is initialized in build_lrouter_nat_defrag_and_lb()
looping over all possible datapath. Am I missing something?

Regards,
Lorenzo

> 
> I think it's best to add a field to "struct ovn_datapath" (after
> 'nat_entries'), e.g., "struct sset nat_external_ips".
> 
> We can populate that one in init_nat_entries() and clean it up in
> destroy_nat_entries().  These get called exactly once for each logical
> router in one iteration of the northd processing loop.
> 
> Thanks,
> Dumitru
> 


More information about the dev mailing list