All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] net: Introduce skb_checksum_start_offset()
  2010-12-15  1:24 [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2] Michał Mirosław
  2010-12-15  1:24 ` [PATCH 2/3] net: Use skb_checksum_start_offset() Michał Mirosław
  2010-12-15  1:24 ` [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start Michał Mirosław
@ 2010-12-15  1:24 ` Michał Mirosław
  2010-12-15  1:29 ` [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2] David Miller
  2010-12-16 22:43 ` David Miller
  4 siblings, 0 replies; 10+ messages in thread
From: Michał Mirosław @ 2010-12-15  1:24 UTC (permalink / raw)
  To: netdev

Introduce skb_checksum_start_offset() to replace repetitive calculation.

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 include/linux/skbuff.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 19f37a6..0491da5 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1355,6 +1355,11 @@ static inline void skb_set_mac_header(struct sk_buff *skb, const int offset)
 }
 #endif /* NET_SKBUFF_DATA_USES_OFFSET */
 
+static inline int skb_checksum_start_offset(const struct sk_buff *skb)
+{
+	return skb->csum_start - skb_headroom(skb);
+}
+
 static inline int skb_transport_offset(const struct sk_buff *skb)
 {
 	return skb_transport_header(skb) - skb->data;
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2]
@ 2010-12-15  1:24 Michał Mirosław
  2010-12-15  1:24 ` [PATCH 2/3] net: Use skb_checksum_start_offset() Michał Mirosław
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Michał Mirosław @ 2010-12-15  1:24 UTC (permalink / raw)
  To: netdev

This is a split version of a patch fixing NETIF_F_HW_CSUM implementations
in network drivers. For easier review, this is split into three patches:

 1. introduce skb_checksum_start_offset() helper
 2. convert correct implementations to use the helper from #1
 3. fix bad uses of skb_transport_offset() using helper from #1

Patches are against net-next, commit 0dbaee3b37e118a96bb7b8eb0d9bbaeeb46264be

	net: Abstract default ADVMSS behind an accessor.

Best Regards,
Michał Mirosław

---

Michał Mirosław (3):
  net: Introduce skb_checksum_start_offset()
  net: Use skb_checksum_start_offset()
  net: Fix drivers advertising HW_CSUM feature to use csum_start

 drivers/net/atl1c/atl1c_main.c    |    2 +-
 drivers/net/atl1e/atl1e_main.c    |    2 +-
 drivers/net/atlx/atl1.c           |    2 +-
 drivers/net/cassini.c             |    2 +-
 drivers/net/e1000/e1000_main.c    |    2 +-
 drivers/net/e1000e/netdev.c       |    2 +-
 drivers/net/enic/enic_main.c      |    2 +-
 drivers/net/ixgb/ixgb_main.c      |    2 +-
 drivers/net/ll_temac_main.c       |    2 +-
 drivers/net/macvtap.c             |    3 +--
 drivers/net/myri10ge/myri10ge.c   |    2 +-
 drivers/net/niu.c                 |    2 +-
 drivers/net/skge.c                |    2 +-
 drivers/net/sungem.c              |    2 +-
 drivers/net/sunhme.c              |    2 +-
 drivers/net/tun.c                 |    2 +-
 drivers/net/usb/smsc95xx.c        |    7 +++----
 drivers/net/virtio_net.c          |    2 +-
 drivers/net/vmxnet3/vmxnet3_drv.c |    4 ++--
 include/linux/skbuff.h            |    5 +++++
 net/core/dev.c                    |    6 +++---
 net/core/skbuff.c                 |    2 +-
 net/ipv4/udp.c                    |    2 +-
 net/packet/af_packet.c            |    3 +--
 24 files changed, 33 insertions(+), 31 deletions(-)

-- 
1.7.2.3


^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start
  2010-12-15  1:24 [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2] Michał Mirosław
  2010-12-15  1:24 ` [PATCH 2/3] net: Use skb_checksum_start_offset() Michał Mirosław
@ 2010-12-15  1:24 ` Michał Mirosław
  2010-12-15  3:07   ` Stephen Hemminger
  2010-12-15  1:24 ` [PATCH 1/3] net: Introduce skb_checksum_start_offset() Michał Mirosław
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Michał Mirosław @ 2010-12-15  1:24 UTC (permalink / raw)
  To: netdev
  Cc: Roopa Prabhu, Eric Dumazet, Jiri Pirko, e1000-devel,
	Shreyas Bhatewara, Jie Yang, Vasanthy Kolluri, VMware, Inc.,
	Brice Goglin, Andrew Gallatin, Joe Perches, David Wang,
	Stephen Hemminger, David S. Miller, H Hartley Sweeten

Some drivers are using skb_transport_offset(skb) instead of skb->csum_start
for NETIF_F_HW_CSUM offload.  This does not matter now, but if someone
implements checksumming of encapsulated packets then this will break silently.

TSO output paths are left as they are, since they are for IP+TCP only
(might be worth converting though).

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 drivers/net/atl1c/atl1c_main.c    |    2 +-
 drivers/net/atl1e/atl1e_main.c    |    2 +-
 drivers/net/cassini.c             |    2 +-
 drivers/net/e1000/e1000_main.c    |    2 +-
 drivers/net/e1000e/netdev.c       |    2 +-
 drivers/net/enic/enic_main.c      |    2 +-
 drivers/net/ixgb/ixgb_main.c      |    2 +-
 drivers/net/ll_temac_main.c       |    2 +-
 drivers/net/myri10ge/myri10ge.c   |    2 +-
 drivers/net/niu.c                 |    2 +-
 drivers/net/skge.c                |    2 +-
 drivers/net/sungem.c              |    2 +-
 drivers/net/sunhme.c              |    2 +-
 drivers/net/vmxnet3/vmxnet3_drv.c |    4 ++--
 14 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/atl1c/atl1c_main.c b/drivers/net/atl1c/atl1c_main.c
index 09b099b..e48ea95 100644
--- a/drivers/net/atl1c/atl1c_main.c
+++ b/drivers/net/atl1c/atl1c_main.c
@@ -2078,7 +2078,7 @@ static int atl1c_tso_csum(struct atl1c_adapter *adapter,
 check_sum:
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
 		u8 css, cso;
-		cso = skb_transport_offset(skb);
+		cso = skb_checksum_start_offset(skb);
 
 		if (unlikely(cso & 0x1)) {
 			if (netif_msg_tx_err(adapter))
diff --git a/drivers/net/atl1e/atl1e_main.c b/drivers/net/atl1e/atl1e_main.c
index ef6349b..e28f8ba 100644
--- a/drivers/net/atl1e/atl1e_main.c
+++ b/drivers/net/atl1e/atl1e_main.c
@@ -1649,7 +1649,7 @@ check_sum:
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
 		u8 css, cso;
 
-		cso = skb_transport_offset(skb);
+		cso = skb_checksum_start_offset(skb);
 		if (unlikely(cso & 0x1)) {
 			netdev_err(adapter->netdev,
 				   "payload offset should not ant event number\n");
diff --git a/drivers/net/cassini.c b/drivers/net/cassini.c
index a8a32bc..73502fe 100644
--- a/drivers/net/cassini.c
+++ b/drivers/net/cassini.c
@@ -2788,7 +2788,7 @@ static inline int cas_xmit_tx_ringN(struct cas *cp, int ring,
 
 	ctrl = 0;
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		const u64 csum_start_off = skb_transport_offset(skb);
+		const u64 csum_start_off = skb_checksum_start_offset(skb);
 		const u64 csum_stuff_off = csum_start_off + skb->csum_offset;
 
 		ctrl =  TX_DESC_CSUM_EN |
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index 491bf2a..340e12d 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -2726,7 +2726,7 @@ static bool e1000_tx_csum(struct e1000_adapter *adapter,
 		break;
 	}
 
-	css = skb_transport_offset(skb);
+	css = skb_checksum_start_offset(skb);
 
 	i = tx_ring->next_to_use;
 	buffer_info = &tx_ring->buffer_info[i];
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 02d093d..a45dafd 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4473,7 +4473,7 @@ static bool e1000_tx_csum(struct e1000_adapter *adapter, struct sk_buff *skb)
 		break;
 	}
 
-	css = skb_transport_offset(skb);
+	css = skb_checksum_start_offset(skb);
 
 	i = tx_ring->next_to_use;
 	buffer_info = &tx_ring->buffer_info[i];
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 77d9138..9710047 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -702,7 +702,7 @@ static inline void enic_queue_wq_skb_csum_l4(struct enic *enic,
 {
 	unsigned int head_len = skb_headlen(skb);
 	unsigned int len_left = skb->len - head_len;
-	unsigned int hdr_len = skb_transport_offset(skb);
+	unsigned int hdr_len = skb_checksum_start_offset(skb);
 	unsigned int csum_offset = hdr_len + skb->csum_offset;
 	int eop = (len_left == 0);
 
diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
index b021798..5639ccc 100644
--- a/drivers/net/ixgb/ixgb_main.c
+++ b/drivers/net/ixgb/ixgb_main.c
@@ -1262,7 +1262,7 @@ ixgb_tx_csum(struct ixgb_adapter *adapter, struct sk_buff *skb)
 
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
 		struct ixgb_buffer *buffer_info;
-		css = skb_transport_offset(skb);
+		css = skb_checksum_start_offset(skb);
 		cso = css + skb->csum_offset;
 
 		i = adapter->tx_ring.next_to_use;
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index 9f8e702..e2c2a72 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -692,7 +692,7 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 
 	cur_p->app0 = 0;
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		unsigned int csum_start_off = skb_transport_offset(skb);
+		unsigned int csum_start_off = skb_checksum_start_offset(skb);
 		unsigned int csum_index_off = csum_start_off + skb->csum_offset;
 
 		cur_p->app0 |= 1; /* TX Checksum Enabled */
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 1ce0207..a37fcf1 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -2736,7 +2736,7 @@ again:
 	odd_flag = 0;
 	flags = (MXGEFW_FLAGS_NO_TSO | MXGEFW_FLAGS_FIRST);
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
-		cksum_offset = skb_transport_offset(skb);
+		cksum_offset = skb_checksum_start_offset(skb);
 		pseudo_hdr_offset = cksum_offset + skb->csum_offset;
 		/* If the headers are excessively large, then we must
 		 * fall back to a software checksum */
diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index f64c424..2541321 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -6589,7 +6589,7 @@ static u64 niu_compute_tx_flags(struct sk_buff *skb, struct ethhdr *ehdr,
 			     (ip_proto == IPPROTO_UDP ?
 			      TXHDR_CSUM_UDP : TXHDR_CSUM_SCTP));
 
-		start = skb_transport_offset(skb) -
+		start = skb_checksum_start_offset(skb) -
 			(pad_bytes + sizeof(struct tx_pkt_hdr));
 		stuff = start + skb->csum_offset;
 
diff --git a/drivers/net/skge.c b/drivers/net/skge.c
index 8c1404b..50815fb 100644
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -2764,7 +2764,7 @@ static netdev_tx_t skge_xmit_frame(struct sk_buff *skb,
 	td->dma_hi = map >> 32;
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		const int offset = skb_transport_offset(skb);
+		const int offset = skb_checksum_start_offset(skb);
 
 		/* This seems backwards, but it is what the sk98lin
 		 * does.  Looks like hardware is wrong?
diff --git a/drivers/net/sungem.c b/drivers/net/sungem.c
index 9e992ca..1c5408f 100644
--- a/drivers/net/sungem.c
+++ b/drivers/net/sungem.c
@@ -1004,7 +1004,7 @@ static netdev_tx_t gem_start_xmit(struct sk_buff *skb,
 
 	ctrl = 0;
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		const u64 csum_start_off = skb_transport_offset(skb);
+		const u64 csum_start_off = skb_checksum_start_offset(skb);
 		const u64 csum_stuff_off = csum_start_off + skb->csum_offset;
 
 		ctrl = (TXDCTRL_CENAB |
diff --git a/drivers/net/sunhme.c b/drivers/net/sunhme.c
index 5e28c41..55bbb9c 100644
--- a/drivers/net/sunhme.c
+++ b/drivers/net/sunhme.c
@@ -2266,7 +2266,7 @@ static netdev_tx_t happy_meal_start_xmit(struct sk_buff *skb,
 
 	tx_flags = TXFLAG_OWN;
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
-		const u32 csum_start_off = skb_transport_offset(skb);
+		const u32 csum_start_off = skb_checksum_start_offset(skb);
 		const u32 csum_stuff_off = csum_start_off + skb->csum_offset;
 
 		tx_flags = (TXFLAG_OWN | TXFLAG_CSENABLE |
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 0169be7..d87a230 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -798,7 +798,7 @@ vmxnet3_parse_and_copy_hdr(struct sk_buff *skb, struct vmxnet3_tx_queue *tq,
 {
 	struct Vmxnet3_TxDataDesc *tdd;
 
-	if (ctx->mss) {
+	if (ctx->mss) {	/* TSO */
 		ctx->eth_ip_hdr_size = skb_transport_offset(skb);
 		ctx->l4_hdr_size = ((struct tcphdr *)
 				   skb_transport_header(skb))->doff * 4;
@@ -807,7 +807,7 @@ vmxnet3_parse_and_copy_hdr(struct sk_buff *skb, struct vmxnet3_tx_queue *tq,
 		unsigned int pull_size;
 
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
-			ctx->eth_ip_hdr_size = skb_transport_offset(skb);
+			ctx->eth_ip_hdr_size = skb_checksum_start_offset(skb);
 
 			if (ctx->ipv4) {
 				struct iphdr *iph = (struct iphdr *)
-- 
1.7.2.3


------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/3] net: Use skb_checksum_start_offset()
  2010-12-15  1:24 [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2] Michał Mirosław
@ 2010-12-15  1:24 ` Michał Mirosław
  2010-12-15  1:24 ` [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start Michał Mirosław
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Michał Mirosław @ 2010-12-15  1:24 UTC (permalink / raw)
  To: netdev
  Cc: Jay Cliburn, Chris Snook, Jie Yang, Steve Glendinning,
	Greg Kroah-Hartman, Patrick McHardy, David S. Miller,
	Alexey Kuznetsov, James Morris

Replace skb->csum_start - skb_headroom(skb) with skb_checksum_start_offset().

Note for usb/smsc95xx: skb->data - skb->head == skb_headroom(skb).

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

---
 drivers/net/atlx/atl1.c    |    2 +-
 drivers/net/macvtap.c      |    3 +--
 drivers/net/tun.c          |    2 +-
 drivers/net/usb/smsc95xx.c |    7 +++----
 drivers/net/virtio_net.c   |    2 +-
 net/core/dev.c             |    6 +++---
 net/core/skbuff.c          |    2 +-
 net/ipv4/udp.c             |    2 +-
 net/packet/af_packet.c     |    3 +--
 9 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/drivers/net/atlx/atl1.c b/drivers/net/atlx/atl1.c
index 5336310..def8df8 100644
--- a/drivers/net/atlx/atl1.c
+++ b/drivers/net/atlx/atl1.c
@@ -2174,7 +2174,7 @@ static int atl1_tx_csum(struct atl1_adapter *adapter, struct sk_buff *skb,
 	u8 css, cso;
 
 	if (likely(skb->ip_summed == CHECKSUM_PARTIAL)) {
-		css = (u8) (skb->csum_start - skb_headroom(skb));
+		css = skb_checksum_start_offset(skb);
 		cso = css + (u8) skb->csum_offset;
 		if (unlikely(css & 0x1)) {
 			/* L1 hardware requires an even number here */
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 4256727..21845af 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -504,8 +504,7 @@ static int macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
 		vnet_hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-		vnet_hdr->csum_start = skb->csum_start -
-					skb_headroom(skb);
+		vnet_hdr->csum_start = skb_checksum_start_offset(skb);
 		vnet_hdr->csum_offset = skb->csum_offset;
 	} /* else everything is zero */
 
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 55f3a3e..7599c45 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -757,7 +757,7 @@ static __inline__ ssize_t tun_put_user(struct tun_struct *tun,
 
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
 			gso.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-			gso.csum_start = skb->csum_start - skb_headroom(skb);
+			gso.csum_start = skb_checksum_start_offset(skb);
 			gso.csum_offset = skb->csum_offset;
 		} /* else everything is zero */
 
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 65cb1ab..bc86f4b 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1163,9 +1163,8 @@ static int smsc95xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 static u32 smsc95xx_calc_csum_preamble(struct sk_buff *skb)
 {
-	int len = skb->data - skb->head;
-	u16 high_16 = (u16)(skb->csum_offset + skb->csum_start - len);
-	u16 low_16 = (u16)(skb->csum_start - len);
+	u16 low_16 = (u16)skb_checksum_start_offset(skb);
+	u16 high_16 = low_16 + skb->csum_offset;
 	return (high_16 << 16) | low_16;
 }
 
@@ -1193,7 +1192,7 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
 		if (skb->len <= 45) {
 			/* workaround - hardware tx checksum does not work
 			 * properly with extremely small packets */
-			long csstart = skb->csum_start - skb_headroom(skb);
+			long csstart = skb_checksum_start_offset(skb);
 			__wsum calc = csum_partial(skb->data + csstart,
 				skb->len - csstart, 0);
 			*((__sum16 *)(skb->data + csstart
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b6d4028..90a23e4 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -519,7 +519,7 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL) {
 		hdr->hdr.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-		hdr->hdr.csum_start = skb->csum_start - skb_headroom(skb);
+		hdr->hdr.csum_start = skb_checksum_start_offset(skb);
 		hdr->hdr.csum_offset = skb->csum_offset;
 	} else {
 		hdr->hdr.flags = 0;
diff --git a/net/core/dev.c b/net/core/dev.c
index d28b3a0..f937726 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1761,7 +1761,7 @@ int skb_checksum_help(struct sk_buff *skb)
 		goto out_set_summed;
 	}
 
-	offset = skb->csum_start - skb_headroom(skb);
+	offset = skb_checksum_start_offset(skb);
 	BUG_ON(offset >= skb_headlen(skb));
 	csum = skb_checksum(skb, offset, skb->len - offset, 0);
 
@@ -2058,8 +2058,8 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			 * checksumming here.
 			 */
 			if (skb->ip_summed == CHECKSUM_PARTIAL) {
-				skb_set_transport_header(skb, skb->csum_start -
-					      skb_headroom(skb));
+				skb_set_transport_header(skb,
+					skb_checksum_start_offset(skb));
 				if (!dev_can_checksum(dev, skb) &&
 				     skb_checksum_help(skb))
 					goto out_kfree_skb;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8814a9a..19d6c21 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1824,7 +1824,7 @@ void skb_copy_and_csum_dev(const struct sk_buff *skb, u8 *to)
 	long csstart;
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL)
-		csstart = skb->csum_start - skb_headroom(skb);
+		csstart = skb_checksum_start_offset(skb);
 	else
 		csstart = skb_headlen(skb);
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index b37181d..1198adf 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2226,7 +2226,7 @@ struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb, int features)
 	/* Do software UFO. Complete and fill in the UDP checksum as HW cannot
 	 * do checksum of UDP packets sent as multiple IP fragments.
 	 */
-	offset = skb->csum_start - skb_headroom(skb);
+	offset = skb_checksum_start_offset(skb);
 	csum = skb_checksum(skb, offset, skb->len - offset, 0);
 	offset += skb->csum_offset;
 	*(__sum16 *)(skb->data + offset) = csum_fold(csum);
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index e79efaf..91cb1d7 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1650,8 +1650,7 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
 
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
 			vnet_hdr.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
-			vnet_hdr.csum_start = skb->csum_start -
-							skb_headroom(skb);
+			vnet_hdr.csum_start = skb_checksum_start_offset(skb);
 			vnet_hdr.csum_offset = skb->csum_offset;
 		} /* else everything is zero */
 
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2]
  2010-12-15  1:24 [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2] Michał Mirosław
                   ` (2 preceding siblings ...)
  2010-12-15  1:24 ` [PATCH 1/3] net: Introduce skb_checksum_start_offset() Michał Mirosław
@ 2010-12-15  1:29 ` David Miller
  2010-12-16 22:43 ` David Miller
  4 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-12-15  1:29 UTC (permalink / raw)
  To: mirq-linux; +Cc: netdev

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Wed, 15 Dec 2010 02:24:08 +0100 (CET)

> This is a split version of a patch fixing NETIF_F_HW_CSUM implementations
> in network drivers. For easier review, this is split into three patches:
> 
>  1. introduce skb_checksum_start_offset() helper
>  2. convert correct implementations to use the helper from #1
>  3. fix bad uses of skb_transport_offset() using helper from #1

I don't have any idea why you'd CC: as the networking maintainer only
on some of these patches.

That doesn't make any sense at all.

Either CC: me on all of them, or none.  Don't partially CC: as that
makes it painful for me to manage my inbox.

Thank you.


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start
  2010-12-15  1:24 ` [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start Michał Mirosław
@ 2010-12-15  3:07   ` Stephen Hemminger
  2010-12-15 18:16     ` Michał Mirosław
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2010-12-15  3:07 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Roopa Prabhu, Dumazet, Jiri Pirko, e1000-devel, netdev, Vasanthy,
	Jie Yang, Eric, Kolluri, VMware, Inc.,
	Brice Goglin, H, Andrew Gallatin, Joe Perches, David Wang,
	Shreyas Bhatewara, David S. Miller, Sweeten

On Wed, 15 Dec 2010 02:24:08 +0100 (CET)
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> Some drivers are using skb_transport_offset(skb) instead of skb->csum_start
> for NETIF_F_HW_CSUM offload.  This does not matter now, but if someone
> implements checksumming of encapsulated packets then this will break silently.
> 
> TSO output paths are left as they are, since they are for IP+TCP only
> (might be worth converting though).
> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>

Which if any of these drivers did you test on real hardware?


-- 

------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start
  2010-12-15  3:07   ` Stephen Hemminger
@ 2010-12-15 18:16     ` Michał Mirosław
  2010-12-15 18:25       ` Stephen Hemminger
  0 siblings, 1 reply; 10+ messages in thread
From: Michał Mirosław @ 2010-12-15 18:16 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Roopa Prabhu, Eric Dumazet, Jiri Pirko, e1000-devel, netdev,
	Shreyas Bhatewara, Jie Yang, Vasanthy Kolluri, VMware, Inc.,
	Brice Goglin, Andrew Gallatin, Joe Perches, David Wang,
	David S. Miller, H Hartley Sweeten

On Tue, Dec 14, 2010 at 07:07:27PM -0800, Stephen Hemminger wrote:
> On Wed, 15 Dec 2010 02:24:08 +0100 (CET)
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > Some drivers are using skb_transport_offset(skb) instead of skb->csum_start
> > for NETIF_F_HW_CSUM offload.  This does not matter now, but if someone
> > implements checksumming of encapsulated packets then this will break silently.
> > 
> > TSO output paths are left as they are, since they are for IP+TCP only
> > (might be worth converting though).
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Which if any of these drivers did you test on real hardware?

I'm running it on skge, with additional patch that changes it to use
NETIF_F_HW_CSUM instead of NETIF_F_IP_CSUM. BTW, why it is not advertising
full HW checksum in vanilla kernel? Are there any known hardware bugs in there?

Best Regards,
Michał Mirosław

---
lspci:

02:05.0 Ethernet controller: 3Com Corporation 3c940 10/100/1000Base-T [Marvell] (rev 12)
        Subsystem: ASUSTeK Computer Inc. A7V600/P4P800/K8V motherboard
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV+ VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
        Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 64 (5750ns min, 7750ns max), Cache Line Size: 16 bytes
        Interrupt: pin A routed to IRQ 22
        Region 0: Memory at feafc000 (32-bit, non-prefetchable) [size=16K]
        Region 1: I/O ports at d800 [size=256]
        Capabilities: <access denied>
        Kernel driver in use: skge
        Kernel modules: skge

ethtool -k:

Offload parameters for eth3:
rx-checksumming: on
tx-checksumming: on
scatter-gather: on
tcp segmentation offload: off
udp fragmentation offload: off
generic segmentation offload: on
large receive offload: off


------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start
  2010-12-15 18:16     ` Michał Mirosław
@ 2010-12-15 18:25       ` Stephen Hemminger
  2010-12-15 18:58         ` Michał Mirosław
  0 siblings, 1 reply; 10+ messages in thread
From: Stephen Hemminger @ 2010-12-15 18:25 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Roopa Prabhu, Dumazet, Jiri Pirko, e1000-devel, netdev, Vasanthy,
	Jie Yang, Eric, Kolluri, VMware, Inc.,
	Brice Goglin, H, Andrew Gallatin, Joe Perches, David Wang,
	Shreyas Bhatewara, David S. Miller, Sweeten

On Wed, 15 Dec 2010 19:16:55 +0100
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> On Tue, Dec 14, 2010 at 07:07:27PM -0800, Stephen Hemminger wrote:
> > On Wed, 15 Dec 2010 02:24:08 +0100 (CET)
> > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > Some drivers are using skb_transport_offset(skb) instead of skb->csum_start
> > > for NETIF_F_HW_CSUM offload.  This does not matter now, but if someone
> > > implements checksumming of encapsulated packets then this will break silently.
> > > 
> > > TSO output paths are left as they are, since they are for IP+TCP only
> > > (might be worth converting though).
> > > 
> > > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> > Which if any of these drivers did you test on real hardware?
> 
> I'm running it on skge, with additional patch that changes it to use
> NETIF_F_HW_CSUM instead of NETIF_F_IP_CSUM. BTW, why it is not advertising
> full HW checksum in vanilla kernel? Are there any known hardware bugs in there?
> 
> Best Regards,
> Michał Mirosław

The driver was written from vendor sk98lin driver and that driver
only did IP checksum, therefore I did not trust the hardware.
The chipset is old, and not used in new designs, no documentation
is up to date (and I didn't have any).

Please don't enable non-IPv4 checksum offload on this hardware, there
is too big a risk of it not working on all hardware.

-- 

------------------------------------------------------------------------------
Lotusphere 2011
Register now for Lotusphere 2011 and learn how
to connect the dots, take your collaborative environment
to the next level, and enter the era of Social Business.
http://p.sf.net/sfu/lotusphere-d2d
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start
  2010-12-15 18:25       ` Stephen Hemminger
@ 2010-12-15 18:58         ` Michał Mirosław
  0 siblings, 0 replies; 10+ messages in thread
From: Michał Mirosław @ 2010-12-15 18:58 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: netdev, e1000-devel, Vasanthy Kolluri, Roopa Prabhu, David Wang,
	Andrew Gallatin, Brice Goglin, Shreyas Bhatewara, VMware, Inc.,
	David S. Miller, Eric Dumazet, Jiri Pirko, H Hartley Sweeten,
	Jie Yang, Joe Perches

On Wed, Dec 15, 2010 at 10:25:36AM -0800, Stephen Hemminger wrote:
> On Wed, 15 Dec 2010 19:16:55 +0100
> Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > On Tue, Dec 14, 2010 at 07:07:27PM -0800, Stephen Hemminger wrote:
> > > On Wed, 15 Dec 2010 02:24:08 +0100 (CET)
> > > Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:
> > > > Some drivers are using skb_transport_offset(skb) instead of skb->csum_start
> > > > for NETIF_F_HW_CSUM offload.  This does not matter now, but if someone
> > > > implements checksumming of encapsulated packets then this will break silently.
[...]
> > > Which if any of these drivers did you test on real hardware?
> > I'm running it on skge, with additional patch that changes it to use
> > NETIF_F_HW_CSUM instead of NETIF_F_IP_CSUM. BTW, why it is not advertising
> > full HW checksum in vanilla kernel? Are there any known hardware bugs in there?
> The driver was written from vendor sk98lin driver and that driver
> only did IP checksum, therefore I did not trust the hardware.
> The chipset is old, and not used in new designs, no documentation
> is up to date (and I didn't have any).
> 
> Please don't enable non-IPv4 checksum offload on this hardware, there
> is too big a risk of it not working on all hardware.

Sure, this can stay in my private patchlist. I can also add a module option
(disabled by default) for the brave ones if you'd accept that.

Best Regards,
Michał Mirosław

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2]
  2010-12-15  1:24 [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2] Michał Mirosław
                   ` (3 preceding siblings ...)
  2010-12-15  1:29 ` [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2] David Miller
@ 2010-12-16 22:43 ` David Miller
  4 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-12-16 22:43 UTC (permalink / raw)
  To: mirq-linux; +Cc: netdev

From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Date: Wed, 15 Dec 2010 02:24:08 +0100 (CET)

> This is a split version of a patch fixing NETIF_F_HW_CSUM implementations
> in network drivers. For easier review, this is split into three patches:
> 
>  1. introduce skb_checksum_start_offset() helper
>  2. convert correct implementations to use the helper from #1
>  3. fix bad uses of skb_transport_offset() using helper from #1

Looks good, all applied, thanks!

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-12-16 22:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-15  1:24 [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2] Michał Mirosław
2010-12-15  1:24 ` [PATCH 2/3] net: Use skb_checksum_start_offset() Michał Mirosław
2010-12-15  1:24 ` [PATCH 3/3] net: Fix drivers advertising HW_CSUM feature to use csum_start Michał Mirosław
2010-12-15  3:07   ` Stephen Hemminger
2010-12-15 18:16     ` Michał Mirosław
2010-12-15 18:25       ` Stephen Hemminger
2010-12-15 18:58         ` Michał Mirosław
2010-12-15  1:24 ` [PATCH 1/3] net: Introduce skb_checksum_start_offset() Michał Mirosław
2010-12-15  1:29 ` [PATCH 0/3] Fix NETIF_F_HW_CSUM implementations [v2] David Miller
2010-12-16 22:43 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.