Re: [PATCH] Very first try: ipt_connrate patch.

[Patrick McHardy <kaber@trash.net>]

Nuutti Kotivuori wrote:
> I would be glad to get as many comments about it as early as
> possible, also on the usefulness of what I'm doing here.

> +void
> +ip_conntrack_rate_count(struct ip_conntrack_rate *ctr,
> +			unsigned int len)
> +{
> +	u_int32_t new_bytes;
> +	unsigned long now = jiffies;
> +
> +	new_bytes = (ctr->avgrate * IP_CONNTRACK_RATE_INTERVAL +
> +		     len * IP_CONNTRACK_RATE_SCALE);
> +	if(new_bytes > MAX_TOKENS_IN_WINDOW)
> +		new_bytes = MAX_TOKENS_IN_WINDOW;
> +	ctr->avgrate = new_bytes / (now - xchg(&ctr->prev, now) +
> +				    IP_CONNTRACK_RATE_INTERVAL);
> +}

You need locking here. Other CPUs can change ctr->avgrate between
reading and updating it. The xchg operation becomes unneccessary
with proper locking.

Regards,
Patrick