Skip to contentSkip to conversation

virtio_net: Fix skb->csum_start computation (2.6.24, 2.6.25)

PATCH In review Rusty Russell 2 participants 3 messages Jun 16, 2008 – Jun 17, 2008

Subsystems (3)
Network drivers virtio core virtio net driver
lore
Share & tools
lore

Rusty Russell

author
#0 lore raw

PATCH 1/1 virtio_net: Fix skb->csum_start computation (2.6.24, 2.6.25)

From: Mark McLoughlin <[email protected]>

hdr->csum_start is the offset from the start of the ethernet header to the transport layer checksum field. skb->csum_start is the offset from skb->head.

skb_partial_csum_set() assumes that skb->data points to the ethernet header - i.e. it computes skb->csum_start by adding the headroom to hdr->csum_start.

Since eth_type_trans() skb_pull()s the ethernet header, skb_partial_csum_set() should be called before eth_type_trans().

(Without this patch, GSO packets from a guest to the world outside the host are corrupted).

Signed-off-by Mark McLoughlin <[email protected]>
Acked-by Herbert Xu <[email protected]>
Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/virtio_net.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/net/virtio_net.c +5 -3
index 50e6e59..503486f 100644
@@ -86,9 +86,7 @@ static void receive_skb(struct net_device *dev, struct sk_buff *skb,
 	BUG_ON(len > MAX_PACKET_LEN);
 
 	skb_trim(skb, len);
-	skb->protocol = eth_type_trans(skb, dev);
-	pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
-		 ntohs(skb->protocol), skb->len, skb->pkt_type);
+
 	dev->stats.rx_bytes += skb->len;
 	dev->stats.rx_packets++;
 

@@ -98,6 +96,10 @@ static void receive_skb(struct net_device *dev, struct sk_buff *skb,
 			goto frame_err;
 	}
 
+	skb->protocol = eth_type_trans(skb, dev);
+	pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
+		 ntohs(skb->protocol), skb->len, skb->pkt_type);
+
 	if (hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
 		pr_debug("GSO!\n");
 		switch (hdr->gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
#1 lore raw
Quoted 10 lines · Rusty Russell

From: Mark McLoughlin <[email protected]>

hdr->csum_start is the offset from the start of the ethernet header to the transport layer checksum field. skb->csum_start is the offset from skb->head.

skb_partial_csum_set() assumes that skb->data points to the ethernet header - i.e. it computes skb->csum_start by adding the headroom to hdr->csum_start.

Quoted 6 lines

Since eth_type_trans() skb_pull()s the ethernet header, skb_partial_csum_set() should be called before eth_type_trans().

(Without this patch, GSO packets from a guest to the world outside the host are corrupted).

Is this in Linus's tree already? If so, what is the commit id.

If not, we need a good reason to be able to take it into the -stable tree (note that 2.6.24 isn't being maintained anymore...)

thanks,

greg k-h

author
#2 lore raw
Quoted 11 lines · On Tuesday 17 June 2008 05:38:45 Greg KH wrote:
Quoted 10 lines · Rusty Russell

From: Mark McLoughlin <[email protected]>

hdr->csum_start is the offset from the start of the ethernet header to the transport layer checksum field. skb->csum_start is the offset from skb->head.

skb_partial_csum_set() assumes that skb->data points to the ethernet header - i.e. it computes skb->csum_start by adding the headroom to hdr->csum_start.

Quoted 8 lines
Quoted 6 lines

Since eth_type_trans() skb_pull()s the ethernet header, skb_partial_csum_set() should be called before eth_type_trans().

(Without this patch, GSO packets from a guest to the world outside the host are corrupted).

Is this in Linus's tree already? If so, what is the commit id.

Sure: 23cde76d801246a702e7a84c3fe3d655b35c89a1 "virtio_net: Fix skb->csum_start computation"

Quoted 1 line

(note that 2.6.24 isn't being maintained anymore...)

OK, wasn't sure.

Thanks,
Rusty.