xen-netfront: reset skb transport header before checksum
Subsystems (3)
PATCH 1/1 xen-netfront: reset skb transport header before checksum
In ed1f50c3a ("net: add skb_checksum_setup") some checksum functions were introduced in core. Subsequent change b5cf66cd1 ("xen-netfront:
use new skb_checksum_setup function") made use of those functions to replace its own implementation. With that change ip_hdr() and tcp_hdr() were not pointing at the right place. d554f73df ("xen-netfront: reset skb network header before checksum") fixed the ip_hdr().
This patch fixes tcp_hdr().
We saw this problem when LRO was enabled on the host and it results in netfront setting skb->ip_summed to CHECKSUM_UNNECESSARY.
--- drivers/net/xen-netfront.c | 1 + 1 file changed, 1 insertion(+)
drivers/net/xen-netfront.c +1 -0
@@ -889,6 +889,7 @@ static int handle_incoming_queue(struct netfront_queue *queue,/* Ethernet work: Delayed to here as it peeks the header. */ skb->protocol =eth_type_trans (skb, queue->info->netdev);skb_reset_network_header (skb); +skb_set_transport_header (skb,ip_hdrlen (skb));if (checksum_setup (queue->info->netdev, skb)) {kfree_skb (skb);
Signature · 1 line
1.8.2.1
Quoted 22 lines · Venkat Venkatsubra
In
ed1f50c3a("net: addskb_checksum_setup") some checksum functions were introduced in core. Subsequent changeb5cf66cd1("xen-netfront:
use newskb_checksum_setupfunction") made use of those functions to replace its own implementation. With that changeip_hdr()andtcp_hdr()were not pointing at the right place.d554f73df("xen-netfront: reset skb network header before checksum") fixed theip_hdr().This patch fixes
tcp_hdr().We saw this problem when LRO was enabled on the host and it results in netfront setting
skb->ip_summedtoCHECKSUM_UNNECESSARY.Signed-off-by Venkat Venkatsubra <[email protected]>--- drivers/net/xen-netfront.c | 1 + 1 file changed, 1 insertion(+)drivers/net/xen-netfront.c +0
-0
drivers/net/xen-netfront.c static int handle_incoming_queue(struct netfront_queue *queue, · line 895
Quoted 7 lines
drivers/net/xen-netfront.c +1
-0@@ -889,6 +889,7 @@ static int handle_incoming_queue(struct netfront_queue *queue,/* Ethernet work: Delayed to here as it peeks the header. */ skb->protocol =eth_type_trans (skb, queue->info->netdev);skb_reset_network_header (skb); +skb_set_transport_header (skb,ip_hdrlen (skb));if (checksum_setup (queue->info->netdev, skb)) {kfree_skb (skb);
This looks bogus for IPv6.
I would rather fix skb_checksum_setup_ip() and skb_checksum_setup_ipv6(), since you already use correctly skb_maybe_pull_tail() there.
In general, setting network and transport header at this point seems extra work.
Quoted 22 lines · Venkat Venkatsubra
In
ed1f50c3a("net: addskb_checksum_setup") some checksum functions were introduced in core. Subsequent changeb5cf66cd1("xen-netfront:
use newskb_checksum_setupfunction") made use of those functions to replace its own implementation. With that changeip_hdr()andtcp_hdr()were not pointing at the right place.d554f73df("xen-netfront: reset skb network header before checksum") fixed theip_hdr().This patch fixes
tcp_hdr().We saw this problem when LRO was enabled on the host and it results in netfront setting
skb->ip_summedtoCHECKSUM_UNNECESSARY.Signed-off-by Venkat Venkatsubra <[email protected]>--- drivers/net/xen-netfront.c | 1 + 1 file changed, 1 insertion(+)drivers/net/xen-netfront.c +0
-0
drivers/net/xen-netfront.c static int handle_incoming_queue(struct netfront_queue *queue, · line 895
Quoted 7 lines
drivers/net/xen-netfront.c +1
-0@@ -889,6 +889,7 @@ static int handle_incoming_queue(struct netfront_queue *queue,/* Ethernet work: Delayed to here as it peeks the header. */ skb->protocol =eth_type_trans (skb, queue->info->netdev);skb_reset_network_header (skb); +skb_set_transport_header (skb,ip_hdrlen (skb));if (checksum_setup (queue->info->netdev, skb)) {kfree_skb (skb);
This looks bogus for IPv6.
I would rather fix skb_checksum_setup_ip() and skb_checksum_setup_ipv6(), since you already use correctly skb_maybe_pull_tail() there.
In general, setting network and transport header at this point seems extra work.
Venkat Venkatsubra
Acked-by Eric DumazetQuoted 10 lines
This looks bogus for IPv6.
I would rather fix
skb_checksum_setup_ip()andskb_checksum_setup_ipv6(), since you already use correctlyskb_maybe_pull_tail()there.In general, setting network and transport header at this point seems extra work.
Yes, I missed the IPv6 case. ;-) Thanks Eric for pointing that out. Looking further the below commit might have already resolved the issue.
commit e5d5decaedf41f5b03e6819789cc3ba8744d9f54
Author: Jason Wang <[email protected]> Date: Tue Mar 26 23:11:20 2013 +0000
net: core: let skb_partial_csum_set() set transport header
For untrusted packets with partial checksum, we need to set the transport he for precise packet length estimation. We can just let skb_pratial_csum_set() do this to avoid extra call to skb_flow_dissect() and simplify the caller.
The guest we were running was older to this and didn't have the above commit.
Thanks.
Venkat
Venkat Venkatsubra
Acked-by Eric DumazetQuoted 10 lines
This looks bogus for IPv6.
I would rather fix
skb_checksum_setup_ip()andskb_checksum_setup_ipv6(), since you already use correctlyskb_maybe_pull_tail()there.In general, setting network and transport header at this point seems extra work.
Yes, I missed the IPv6 case. ;-) Thanks Eric for pointing that out. Looking further the below commit might have already resolved the issue.
commit e5d5decaedf41f5b03e6819789cc3ba8744d9f54
Author: Jason Wang <[email protected]> Date: Tue Mar 26 23:11:20 2013 +0000
net: core: let skb_partial_csum_set() set transport header
For untrusted packets with partial checksum, we need to set the transport he for precise packet length estimation. We can just let skb_pratial_csum_set() do this to avoid extra call to skb_flow_dissect() and simplify the caller.
The guest we were running was older to this and didn't have the above commit.
Thanks.
Venkat