* [PATCH net] xen-netback: make sure skb linear area covers checksum field
@ 2013-12-11 16:37 Paul Durrant
2013-12-11 16:47 ` [Xen-devel] " Jan Beulich
2013-12-11 21:47 ` David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Paul Durrant @ 2013-12-11 16:37 UTC (permalink / raw)
To: xen-devel, netdev; +Cc: Paul Durrant, Wei Liu, Ian Campbell, David Vrabel
skb_partial_csum_set requires that the linear area of the skb covers the
checksum field. The checksum setup code in netback was only doing that
pullup in the case when the pseudo header checksum was being recalculated
though. This patch makes that pullup unconditional. (I pullup the whole
transport header just for simplicity; the requirement is only for the check
field but in the case of UDP this is the last field in the header and in the
case of TCP it's the last but one).
The lack of pullup manifested as failures running Microsoft HCK network
tests on a pair of Windows 8 VMs and it has been verified that this patch
fixes the problem.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
---
drivers/net/xen-netback/netback.c | 60 +++++++++++++++++--------------------
1 file changed, 28 insertions(+), 32 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index acf1392..56c3e5a 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1199,42 +1199,40 @@ static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
switch (ip_hdr(skb)->protocol) {
case IPPROTO_TCP:
+ err = maybe_pull_tail(skb,
+ off + sizeof(struct tcphdr),
+ MAX_IP_HDR_LEN);
+ if (err < 0)
+ goto out;
+
if (!skb_partial_csum_set(skb, off,
offsetof(struct tcphdr, check)))
goto out;
- if (recalculate_partial_csum) {
- err = maybe_pull_tail(skb,
- off + sizeof(struct tcphdr),
- MAX_IP_HDR_LEN);
- if (err < 0)
- goto out;
-
+ if (recalculate_partial_csum)
tcp_hdr(skb)->check =
~csum_tcpudp_magic(ip_hdr(skb)->saddr,
ip_hdr(skb)->daddr,
skb->len - off,
IPPROTO_TCP, 0);
- }
break;
case IPPROTO_UDP:
+ err = maybe_pull_tail(skb,
+ off + sizeof(struct udphdr),
+ MAX_IP_HDR_LEN);
+ if (err < 0)
+ goto out;
+
if (!skb_partial_csum_set(skb, off,
offsetof(struct udphdr, check)))
goto out;
- if (recalculate_partial_csum) {
- err = maybe_pull_tail(skb,
- off + sizeof(struct udphdr),
- MAX_IP_HDR_LEN);
- if (err < 0)
- goto out;
-
+ if (recalculate_partial_csum)
udp_hdr(skb)->check =
~csum_tcpudp_magic(ip_hdr(skb)->saddr,
ip_hdr(skb)->daddr,
skb->len - off,
IPPROTO_UDP, 0);
- }
break;
default:
goto out;
@@ -1342,42 +1340,40 @@ static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
switch (nexthdr) {
case IPPROTO_TCP:
+ err = maybe_pull_tail(skb,
+ off + sizeof(struct tcphdr),
+ MAX_IPV6_HDR_LEN);
+ if (err < 0)
+ goto out;
+
if (!skb_partial_csum_set(skb, off,
offsetof(struct tcphdr, check)))
goto out;
- if (recalculate_partial_csum) {
- err = maybe_pull_tail(skb,
- off + sizeof(struct tcphdr),
- MAX_IPV6_HDR_LEN);
- if (err < 0)
- goto out;
-
+ if (recalculate_partial_csum)
tcp_hdr(skb)->check =
~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
&ipv6_hdr(skb)->daddr,
skb->len - off,
IPPROTO_TCP, 0);
- }
break;
case IPPROTO_UDP:
+ err = maybe_pull_tail(skb,
+ off + sizeof(struct udphdr),
+ MAX_IPV6_HDR_LEN);
+ if (err < 0)
+ goto out;
+
if (!skb_partial_csum_set(skb, off,
offsetof(struct udphdr, check)))
goto out;
- if (recalculate_partial_csum) {
- err = maybe_pull_tail(skb,
- off + sizeof(struct udphdr),
- MAX_IPV6_HDR_LEN);
- if (err < 0)
- goto out;
-
+ if (recalculate_partial_csum)
udp_hdr(skb)->check =
~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
&ipv6_hdr(skb)->daddr,
skb->len - off,
IPPROTO_UDP, 0);
- }
break;
default:
goto out;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Xen-devel] [PATCH net] xen-netback: make sure skb linear area covers checksum field
2013-12-11 16:37 [PATCH net] xen-netback: make sure skb linear area covers checksum field Paul Durrant
@ 2013-12-11 16:47 ` Jan Beulich
2013-12-11 21:47 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2013-12-11 16:47 UTC (permalink / raw)
To: Paul Durrant; +Cc: David Vrabel, Ian Campbell, Wei Liu, xen-devel, netdev
>>> On 11.12.13 at 17:37, Paul Durrant <paul.durrant@citrix.com> wrote:
> skb_partial_csum_set requires that the linear area of the skb covers the
> checksum field. The checksum setup code in netback was only doing that
> pullup in the case when the pseudo header checksum was being recalculated
> though. This patch makes that pullup unconditional. (I pullup the whole
> transport header just for simplicity; the requirement is only for the check
> field but in the case of UDP this is the last field in the header and in the
> case of TCP it's the last but one).
>
> The lack of pullup manifested as failures running Microsoft HCK network
> tests on a pair of Windows 8 VMs and it has been verified that this patch
> fixes the problem.
>
Suggested-by: Jan Beulich <jbeulich@suse.com>
(or Reported-by, whatever you like better)
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> ---
> drivers/net/xen-netback/netback.c | 60 +++++++++++++++++--------------------
> 1 file changed, 28 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c
> b/drivers/net/xen-netback/netback.c
> index acf1392..56c3e5a 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1199,42 +1199,40 @@ static int checksum_setup_ip(struct xenvif *vif,
> struct sk_buff *skb,
>
> switch (ip_hdr(skb)->protocol) {
> case IPPROTO_TCP:
> + err = maybe_pull_tail(skb,
> + off + sizeof(struct tcphdr),
> + MAX_IP_HDR_LEN);
> + if (err < 0)
> + goto out;
> +
> if (!skb_partial_csum_set(skb, off,
> offsetof(struct tcphdr, check)))
> goto out;
>
> - if (recalculate_partial_csum) {
> - err = maybe_pull_tail(skb,
> - off + sizeof(struct tcphdr),
> - MAX_IP_HDR_LEN);
> - if (err < 0)
> - goto out;
> -
> + if (recalculate_partial_csum)
> tcp_hdr(skb)->check =
> ~csum_tcpudp_magic(ip_hdr(skb)->saddr,
> ip_hdr(skb)->daddr,
> skb->len - off,
> IPPROTO_TCP, 0);
> - }
> break;
> case IPPROTO_UDP:
> + err = maybe_pull_tail(skb,
> + off + sizeof(struct udphdr),
> + MAX_IP_HDR_LEN);
> + if (err < 0)
> + goto out;
> +
> if (!skb_partial_csum_set(skb, off,
> offsetof(struct udphdr, check)))
> goto out;
>
> - if (recalculate_partial_csum) {
> - err = maybe_pull_tail(skb,
> - off + sizeof(struct udphdr),
> - MAX_IP_HDR_LEN);
> - if (err < 0)
> - goto out;
> -
> + if (recalculate_partial_csum)
> udp_hdr(skb)->check =
> ~csum_tcpudp_magic(ip_hdr(skb)->saddr,
> ip_hdr(skb)->daddr,
> skb->len - off,
> IPPROTO_UDP, 0);
> - }
> break;
> default:
> goto out;
> @@ -1342,42 +1340,40 @@ static int checksum_setup_ipv6(struct xenvif *vif,
> struct sk_buff *skb,
>
> switch (nexthdr) {
> case IPPROTO_TCP:
> + err = maybe_pull_tail(skb,
> + off + sizeof(struct tcphdr),
> + MAX_IPV6_HDR_LEN);
> + if (err < 0)
> + goto out;
> +
> if (!skb_partial_csum_set(skb, off,
> offsetof(struct tcphdr, check)))
> goto out;
>
> - if (recalculate_partial_csum) {
> - err = maybe_pull_tail(skb,
> - off + sizeof(struct tcphdr),
> - MAX_IPV6_HDR_LEN);
> - if (err < 0)
> - goto out;
> -
> + if (recalculate_partial_csum)
> tcp_hdr(skb)->check =
> ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> &ipv6_hdr(skb)->daddr,
> skb->len - off,
> IPPROTO_TCP, 0);
> - }
> break;
> case IPPROTO_UDP:
> + err = maybe_pull_tail(skb,
> + off + sizeof(struct udphdr),
> + MAX_IPV6_HDR_LEN);
> + if (err < 0)
> + goto out;
> +
> if (!skb_partial_csum_set(skb, off,
> offsetof(struct udphdr, check)))
> goto out;
>
> - if (recalculate_partial_csum) {
> - err = maybe_pull_tail(skb,
> - off + sizeof(struct udphdr),
> - MAX_IPV6_HDR_LEN);
> - if (err < 0)
> - goto out;
> -
> + if (recalculate_partial_csum)
> udp_hdr(skb)->check =
> ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
> &ipv6_hdr(skb)->daddr,
> skb->len - off,
> IPPROTO_UDP, 0);
> - }
> break;
> default:
> goto out;
> --
> 1.7.10.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] xen-netback: make sure skb linear area covers checksum field
2013-12-11 16:37 [PATCH net] xen-netback: make sure skb linear area covers checksum field Paul Durrant
2013-12-11 16:47 ` [Xen-devel] " Jan Beulich
@ 2013-12-11 21:47 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2013-12-11 21:47 UTC (permalink / raw)
To: paul.durrant; +Cc: xen-devel, netdev, wei.liu2, ian.campbell, david.vrabel
From: Paul Durrant <paul.durrant@citrix.com>
Date: Wed, 11 Dec 2013 16:37:40 +0000
> skb_partial_csum_set requires that the linear area of the skb covers the
> checksum field. The checksum setup code in netback was only doing that
> pullup in the case when the pseudo header checksum was being recalculated
> though. This patch makes that pullup unconditional. (I pullup the whole
> transport header just for simplicity; the requirement is only for the check
> field but in the case of UDP this is the last field in the header and in the
> case of TCP it's the last but one).
>
> The lack of pullup manifested as failures running Microsoft HCK network
> tests on a pair of Windows 8 VMs and it has been verified that this patch
> fixes the problem.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Applied, thank you.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-12-11 21:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-11 16:37 [PATCH net] xen-netback: make sure skb linear area covers checksum field Paul Durrant
2013-12-11 16:47 ` [Xen-devel] " Jan Beulich
2013-12-11 21:47 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).