Patchwork [v3,kernel,1/29] bugfix: pkts/bytes need to be specified simultaneously

login
register
mail settings
Submitter Michael Zintakis
Date July 10, 2013, 6:24 p.m.
Message ID <1373480727-11254-2-git-send-email-michael.zintakis@googlemail.com>
Download mbox | patch
Permalink /patch/258189/
State Not Applicable
Headers show

Comments

Michael Zintakis - July 10, 2013, 6:24 p.m.
* in nfnetlink_acct.c::nfnl_acct_new enforce a check ensuring that
packet and byte values are specified simultaneously - return -EINVAL if
that is not the case.

Signed-off-by: Michael Zintakis <michael.zintakis@googlemail.com>
---
 net/netfilter/nfnetlink_acct.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Florian Westphal - July 10, 2013, 8:04 p.m.
Michael Zintakis <michael.zintakis@googlemail.com> wrote:
> * in nfnetlink_acct.c::nfnl_acct_new enforce a check ensuring that
> packet and byte values are specified simultaneously - return -EINVAL if
> that is not the case.

Ok, I understand why it appears to be un-intuitive to have
an accounting object that shows '0 bytes in 100000 packets'.

But still, whats the point?

I could also set '1 byte in 100000 packets', no?

[ so my point is, why bother with checking this? ]
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Zintakis - July 11, 2013, 6:56 p.m.
Florian Westphal wrote:
> Michael Zintakis <michael.zintakis@googlemail.com> wrote:
>> * in nfnetlink_acct.c::nfnl_acct_new enforce a check ensuring that
>> packet and byte values are specified simultaneously - return -EINVAL if
>> that is not the case.
> 
> Ok, I understand why it appears to be un-intuitive to have
> an accounting object that shows '0 bytes in 100000 packets'.
> 
> But still, whats the point?
The fact that it isn't un-intuitive is not the reason for creating that patch.

> I could also set '1 byte in 100000 packets', no?
> 
> [ so my point is, why bother with checking this? ]
Indeed you can, but that would be your own (conscious) decision and you will be responsible for it (which is what counts here).

When either packet or byte counters aren't specified that isn't necessarily the case (you may forgot to set them up for example).

Let me illustrate my point with a little real-life example: suppose you have an alarm clock which lags behind quite often and you need to set it up to wake you in the morning for an important meeting.

Now, if you deliberately choose to put the clock backwards (the equivalent of specifying "0 bytes in 100000 packets") you will be late for that meeting and, as you did not set your clock properly, you are responsible for the flak you are going to get when you are late, while if you leave the clock to drift and be late, you may have an excuse (provided anyone believes that excuse, but that is not the point I am trying to make).

So, nothing is stopping you from specifying "0 bytes in 100000 packets" if you want to, but that would ultimately be your own conscious decision (and responsibility!), as oppose to "forgetting" to specify either bytes or packets, which may or may not be entirely your fault.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index c7b6d46..526abd7 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -45,7 +45,9 @@  nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
 	struct nf_acct *nfacct, *matching = NULL;
 	char *acct_name;
 
-	if (!tb[NFACCT_NAME])
+	if (!tb[NFACCT_NAME] ||
+	    (tb[NFACCT_BYTES] && !tb[NFACCT_PKTS]) ||
+	    (!tb[NFACCT_BYTES] && tb[NFACCT_PKTS]))
 		return -EINVAL;
 
 	acct_name = nla_data(tb[NFACCT_NAME]);