Patchwork skb: add a comment to skb_csum_unnecessary to avoid miuse

login
register
mail settings
Submitter Koki Sanagi
Date Jan. 23, 2013, 12:30 a.m.
Message ID <50FF2F18.1050408@jp.fujitsu.com>
Download mbox | patch
Permalink /patch/214699/
State Rejected
Delegated to: David Miller
Headers show

Comments

Koki Sanagi - Jan. 23, 2013, 12:30 a.m.
Due to its name and appearance, someone thinks this only checks if ip_summed is
CHECKSUM_UNNECESARRY.  But actually, this returns true even if ip_summed is
CHECKSUM_PARTIAL.  To avoid misuse, this patch a comment which specifies that
CHECKSUM_PARTIAL is OK.

Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
---
 include/linux/skbuff.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 23, 2013, 1:53 a.m.
From: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
Date: Wed, 23 Jan 2013 09:30:16 +0900

> Due to its name and appearance, someone thinks this only checks if ip_summed is
> CHECKSUM_UNNECESARRY.  But actually, this returns true even if ip_summed is
> CHECKSUM_PARTIAL.  To avoid misuse, this patch a comment which specifies that
> CHECKSUM_PARTIAL is OK.
> 
> Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>

I'm not applying this, sorry.  It's a one line function and it's not
so non-obvious that it deserves an 8 line comment.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Koki Sanagi - Jan. 23, 2013, 2:32 a.m.
> From: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
> Date: Wed, 23 Jan 2013 09:30:16 +0900
> 
> > Due to its name and appearance, someone thinks this only checks if
> > ip_summed is CHECKSUM_UNNECESARRY.  But actually, this returns true
> > even if ip_summed is CHECKSUM_PARTIAL.  To avoid misuse, this patch a
> > comment which specifies that CHECKSUM_PARTIAL is OK.
> >
> > Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
> 
> I'm not applying this, sorry.  It's a one line function and it's not so
> non-obvious that it deserves an 8 line comment.

OK.  I just felt weird that CHECKSUM_* is not bit flag but this function handles
it as if it was bit flag.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 23, 2013, 4:13 a.m.
From: "Sanagi, Koki" <sanagi.koki@jp.fujitsu.com>
Date: Wed, 23 Jan 2013 02:32:52 +0000

>> From: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
>> Date: Wed, 23 Jan 2013 09:30:16 +0900
>> 
>> > Due to its name and appearance, someone thinks this only checks if
>> > ip_summed is CHECKSUM_UNNECESARRY.  But actually, this returns true
>> > even if ip_summed is CHECKSUM_PARTIAL.  To avoid misuse, this patch a
>> > comment which specifies that CHECKSUM_PARTIAL is OK.
>> >
>> > Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com>
>> 
>> I'm not applying this, sorry.  It's a one line function and it's not so
>> non-obvious that it deserves an 8 line comment.
> 
> OK.  I just felt weird that CHECKSUM_* is not bit flag but this function handles
> it as if it was bit flag.

The function name says what it does, it determines whether a checksum
is necessary or not.  How that is implemented is another issue.
--
To unsubscribe from this list: send the line "unsubscribe netdev" 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/include/linux/skbuff.h b/include/linux/skbuff.h
index 8b2256e..bc41f64 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2522,6 +2522,16 @@  void skb_complete_wifi_ack(struct sk_buff *skb, bool acked);
 extern __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len);
 extern __sum16 __skb_checksum_complete(struct sk_buff *skb);
 
+/**
+ * skb_csum_unnecessary - check if the checksum needs to be verified
+ * @skb: skb to check
+ *
+ * check if the checksum of this skb needs to be verified.  This function is
+ * effective only against skbs on inbound path.
+ *
+ * NB: This returns true if ip_summed is CHECKSUM_UNNECESSARY or
+ *     CHECKSUM_PARTIAL.
+ **/
 static inline int skb_csum_unnecessary(const struct sk_buff *skb)
 {
 	return skb->ip_summed & CHECKSUM_UNNECESSARY;