> A few questions, see below.
>
> First one is, don't we need a new match revision for this new option?
We were very careful to do this in a way that doesn't need a new revision.
That's what basically dictates most of the design.
If we bump the revision then you don't get fixed semantics unless
you update both kernel and userspace iptables versions... and additionally
we basically end up with two copies of xt_quota in the kernel source since
there's pretty much nothing that can be shared between the two.
> So 1 means, don't keep updating, quota is depleted?
The counter is always 1 higher then the remaining quota.
So 1 means 0.
And 0 means uninitialized and is only present on input from userspace.
> This current_count = 1 would be exposed to userspace too, right?
>
> Hm, this semantics are going to be a bit awkwards to users I think, I
> would prefer to expose this in a different way.
New userspace iptables hides this from users by adding/decrementing by
one as needed on ingress/egress from kernel.
Old iptables never looks at this field.
>> + if (current_count <= skb->len) {
>> + atomic64_set(&q->counter, 1);
>> + return ret;
>> + }
>> + old_count = current_count;
>> + new_count = current_count - skb->len;
>> + current_count = atomic64_cmpxchg(&q->counter, old_count,
>> + new_count);
>> + } while (current_count != old_count);
>
> Probably we simplify this via atomic64_add_return()?
Unfortunately that doesn't work because it's racy if current value is
2 and two (or three) threads both add -1 you end up at zero (or even
+lots).
> I guess problem is userspace may get a current counter that is larger
> than the quota, but we could handle this from userspace iptables to
> print a value that equals the quota, ie. from userspace, before
> printing:
I'm not sure what you mean.
>
> if (consumed > quota)
> printf("--consumed %PRIu64 ", quota);
> else
> printf("--consumed %PRIu64 ", consumed);
>
>> + return !ret;
>> }
>
> Thanks !
Maciej Żenczykowski, Kernel Networking Developer @ Google
