Re: [PATCH 0/10] updates for ctnetlink and conntrack core

[Patrick McHardy <kaber@trash.net>]

Pablo Neira Ayuso wrote:
> Patrick McHardy wrote:
> 
>> Patrick McHardy wrote:
>>
>>> [...]
> 
> 4% is not too much but it is something, anyway it is true that my
> argument of bandwidth consumption is weak.
> 
> But I still like patch 8, if the event cache works fine, I can't see why
> we should force the dumping of all fields. BTW, patch 7 is a new feature
> that I think that we need.

Patch 7 is something we definitely want. For patch 8 we need to come
to a conclusion on how ctnetlink should work first, I prefer to keep
it similar to other network netlink protocols and have it send full
update notifications, containing the entire current state. I also
fail to see the difference it makes, of the four things that are
dumped conditionally (status, refresh, protoinfo, helpinfo) the
first three are always set for new conntracks. The last one only
if we have a helper, but if we don't nothing is dumped anyway.

>> Some more unscientific tests: saving the netlink_trim call by using
>> an allocation size of 200 reduces overhead for the entire notification
>> function (including broadcasting) by about 13% (and also reduces
>> the overruns experienced by conntrack -E for ~390000 packets with
>> 65536 new connections from 17 to 7, which I don't really understand
>> since it shouldn't save any netlink bandwidth and this is a SMP system).
> 
> 
> Who ever said that benchmarking can be scientific? :) I usually have to
> redo my tests several times. These results are interesting but, in
> previous discussions, I thought that we could not change current
> allocation size because of memory fragmentation issues?

I _decreased_ the size, the messages sent are about 150-200 bytes, yet
we're allocating an entire page, which makes netlink clone the skb
and shrink it to its actual size. We should just to it right ourselves.