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

[Patrick McHardy <kaber@trash.net>]

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 inline size_t
> +ctnetlink_calculate_room_size(const struct nf_conn *ct, unsigned long events)
> +{
> +	size_t size = NLMSG_SPACE(sizeof(struct nfgenmsg));
> +
> +	size += calculate_tuple_room_size(ct) * 2 +  /* original and reply */
> +		nla_total_size(sizeof(u_int32_t)) +  /* status */
> +		nla_total_size(sizeof(u_int32_t));   /* id */
> +
> +#ifdef CONFIG_NF_CONNTRACK_MARK
> +	if (events & IPCT_MARK || ct->mark)
> +		size += nla_total_size(sizeof(u_int32_t));
> +#endif
> +
> +	if (events & IPCT_DESTROY) {
> +		const struct nf_conn_counter *acct;
> +
> +		acct = nf_conn_acct_find(ct);
> +		if (acct) {
> +			size += nla_total_size(0) * 2 +
> +				nla_total_size(sizeof(u_int64_t)) * 2 * 2;
> +		}
> +		return size;
> +	}
> +...

Holger's patch pre-calculates things where possible, which reduces
run-time overhead.

> I'm not sure about this. Instead, I noticed with oprofile that we spend
> more cycles to calculate the message size. I think it's more like a
> trade-off, reducing the message size to what we need reduces the chances
> to hit ENOBUFS.

"More cycles" is to be expected for calculation (which is not done
so far), but summed up, I always suspected a measurement error and
I'd be surprised if this is correct.

So we so far have your numbers, which showed a regression, and no
numbers from Holger, which is slightly better :)

OK seriously, we need *some* numbers showing an improvement since I
have basically zero base to decide between your patches, besides the
fact that its to be expected that Holger's will be slightly faster.