[ovs-dev] [PATCH v3 ovn 4/4] controller: binding: add memory accounting for local_lports and related_lports

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Thu Oct 14 11:01:25 UTC 2021


> On 10/14/21 12:45 PM, Lorenzo Bianconi wrote:
> >> On 9/25/21 12:19 AM, Lorenzo Bianconi wrote:
> >>> Introduce memory accounting for:
> >>> - binding local_lports map
> >>> - binding related_lports map
> >>>
> >>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> >>> ---
> >>>  controller/binding.c        | 59 +++++++++++++++++++++++++++++++++++++
> >>>  controller/binding.h        |  5 ++++
> >>>  controller/ovn-controller.c |  5 ++--
> >>>  3 files changed, 67 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/controller/binding.c b/controller/binding.c
> >>> index 661b4bb24..ec1e8bec7 100644
> >>> --- a/controller/binding.c
> >>> +++ b/controller/binding.c
> >>> @@ -39,6 +39,31 @@
> >>>  
> >>>  VLOG_DEFINE_THIS_MODULE(binding);
> >>>  
> >>> +static uint64_t local_lports_usage;
> >>> +static uint64_t related_lports_usage;
> >>> +
> >>> +static void
> >>> +sset_mem_update(uint64_t *usage, struct sset *set,
> >>> +                const char *name, bool erase)
> >>> +{
> >>> +    struct sset_node *node = sset_find(set, name);
> >>> +
> >>> +    if (!node && !erase) { /* add new element */
> >>> +        *usage += (sizeof *node + strlen(name));
> >>> +    } else if (node && erase) { /* remove an element */
> >>> +        *usage -= (sizeof *node + strlen(name));
> >>> +    }
> >>> +}
> >>> +
> >>> +static void
> >>> +sset_mem_clear(struct sset *set, uint64_t *usage)
> >>> +{
> >>> +    const char *name;
> >>> +    SSET_FOR_EACH (name, set) {
> >>> +        sset_mem_update(usage, set, name, true);
> >>> +    }
> >>> +}
> >>> +
> >>
> >> I'm not too sure about this.  These functions are very generic.  I think
> >> this should be part of the sset code itself.  Maybe a sset should just
> >> automatically (or if configured) track all the memory it owns?  What do
> >> you think?
> > 
> > +Ilya
> > 
> > I guess it would be a nice to have. sset are widely used but I do not think
> > this patch will introduce a huge overhead.
> > 
> > Regarding the specific patch we can remove it from the series in order to
> > unlock it or we can have a local routines for the moment and then remove them
> > when we have sset support. What do you think?
> 
> I don't really expect the local_lports and related_lports ssets to grow
> too big so I think we can probably wait with this patch until we have a
> more generic solution, maybe directly in sset.

so we can just remove the patch from the series and repost it w/o 4/4

Regards,
Lorenzo

> 
> Regards,
> Dumitru
> 


More information about the dev mailing list