Re: [PATCH 7/8] [PATCH] dynamic calculation of event message size for ctnetlink

[Patrick McHardy <kaber@trash.net>]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
>> Pablo Neira Ayuso wrote:
>>> This patch adds dynamic message size calculation for ctnetlink. This
>>> reduces CPU consumption since the overhead in the message trimming
>>> is removed.
>>>
>>>  static int ctnetlink_conntrack_event(struct notifier_block *this,
>>>                       unsigned long events, void *ptr)
>>>  {
>>> @@ -437,7 +538,7 @@ static int ctnetlink_conntrack_event(struct
>>> notifier_block *this,
>>>      if (!nfnetlink_has_listeners(group))
>>>          return NOTIFY_DONE;
>>>  
>>> -    skb = alloc_skb(NLMSG_GOODSIZE, GFP_ATOMIC);
>>> +    skb = alloc_skb(ctnetlink_calculate_room_size(ct, events),
>>> GFP_ATOMIC);
>>>      if (!skb)
>>>          return NOTIFY_DONE;
>> These calculations look somewhat expensive to perform for every message.
>> Do you have any numbers for this new patch that shows the difference
>> in CPU usage compared to the resizing done by af_netlink.c?
> 
> Fabian Hugelshofer reported some reduction (~5%) on an embedded
> environment but he was using top to measure the difference. I'll collect
> some more trustable data and get back to you.

Thanks.

>> Since many of these are static in size an alternative would be to
>> update those sizes during protocol/helper/whatever (un)registration.
>> But if this patch already improves things, we can put it in and work
>> on perfecting it later :)
> 
> We can same save some branches doing so, but still ctnetlink has a lot
> of attributes that are dependent of the message type.

Indeed. But f.i. all the ->size(void) functions could obviously be
constants.

> What if we reduce the size of the skb allocated for netlink to more
> reasonable size instead of the accurate calculation? I'll check this
> possibility.

I don't think we need to be 100% accurate, just find a good middle
ground between grossly overallocating and the resulting unnecessary
copy, potentially expensive size calculations and socket memory waste.

> BTW, probably you already know, but in case that you try to use oprofile
> with your current tree, since it is based on 2.6.28-rc2, oprofile was
> broken here, I had to pull the fixes from somewhere else.

I'll probably merge what I have to Dave soon so I can resync with
his tree.