[ovs-dev] [PATCH v3 ovn 4/4] controller: binding: add memory accounting for local_lports and related_lports
Ilya Maximets
i.maximets at ovn.org
Thu Oct 14 10:55:35 UTC 2021
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 <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.
I'm not sure about this. Calling strlen for every add/remove
sounds costly for a generic sset.
>
> Regards,
> Dumitru
>
More information about the dev
mailing list