Re: [PATCH 2/3][CTNETLINK] Atomically set/unset status bits

[Patrick McHardy <kaber@trash.net>]

Pablo Neira Ayuso wrote:
> [CTNETLINK] Atomically set/unset status bits
> 
> This patch guarantees that status bits are atomically set/unset. A minor
> cleanup to save one extra useless line in the code is introduced.
> 
> -	ct->status |= status & ~(IPS_NAT_DONE_MASK | IPS_NAT_MASK);
> +	d &= ~(IPS_NAT_DONE_MASK | IPS_NAT_MASK);
> +
> +	for (i = 0; i < sizeof(ct->status); i++)
> +		if (d & (1 << i)) {
> +			if (status & (1 << i))
> +				set_bit(i, &ct->status);
> +			else
> +				clear_bit(i, &ct->status);
> +		}
> +
>  	return 0;

We already hold the lock, what is the purpose of this change?

It also changes the API, so far bits can only be set. I wonder
where this comes from, I'm pretty sure my original code allowed
to unset specific bits and the checks at the top of that function
indicate that unsetting is possible as well.