> On 10/14/21 12:49, Dumitru Ceara wrote: > > 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 <[email protected]> > >>>> --- > >>>> 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. > > I'm not sure about this. Calling strlen for every add/remove > sounds costly for a generic sset.
ok, so do you prefer to have some just local or add a switch to enable/disable it? > > > > > Regards, > > Dumitru > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
