[PATCH net] pktgen: fix UDP checksum computation

Subsystems: networking [general], the rest

STALE4277d

6 messages, 5 authors, 2015-02-06 · open the first message on its own page

[PATCH net] pktgen: fix UDP checksum computation

From: Sabrina Dubroca <sd@queasysnail.net>
Date: 2015-02-04 22:10:44

This patch fixes two issues in UDP checksum computation in pktgen.

First, the pseudo-header uses the source and destination IP
addresses. Currently, the ports are used for IPv4.

Second, the UDP checksum covers both header and data.  So we need to
generate the data earlier (move pktgen_finalize_skb up), and compute
the checksum for UDP header + data.

Fixes: c26bf4a51308c ("pktgen: Add UDPCSUM flag to support UDP checksums")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/core/pktgen.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index da934fc3faa8..9fa25b0ea145 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2842,25 +2842,25 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
 	skb->dev = odev;
 	skb->pkt_type = PACKET_HOST;
 
+	pktgen_finalize_skb(pkt_dev, skb, datalen);
+
 	if (!(pkt_dev->flags & F_UDPCSUM)) {
 		skb->ip_summed = CHECKSUM_NONE;
 	} else if (odev->features & NETIF_F_V4_CSUM) {
 		skb->ip_summed = CHECKSUM_PARTIAL;
 		skb->csum = 0;
-		udp4_hwcsum(skb, udph->source, udph->dest);
+		udp4_hwcsum(skb, iph->saddr, iph->daddr);
 	} else {
-		__wsum csum = udp_csum(skb);
+		__wsum csum = skb_checksum(skb, skb_transport_offset(skb), datalen + 8, 0);
 
 		/* add protocol-dependent pseudo-header */
-		udph->check = csum_tcpudp_magic(udph->source, udph->dest,
+		udph->check = csum_tcpudp_magic(iph->saddr, iph->daddr,
 						datalen + 8, IPPROTO_UDP, csum);
 
 		if (udph->check == 0)
 			udph->check = CSUM_MANGLED_0;
 	}
 
-	pktgen_finalize_skb(pkt_dev, skb, datalen);
-
 #ifdef CONFIG_XFRM
 	if (!process_ipsec(pkt_dev, skb, protocol))
 		return NULL;
@@ -2976,6 +2976,8 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
 	skb->dev = odev;
 	skb->pkt_type = PACKET_HOST;
 
+	pktgen_finalize_skb(pkt_dev, skb, datalen);
+
 	if (!(pkt_dev->flags & F_UDPCSUM)) {
 		skb->ip_summed = CHECKSUM_NONE;
 	} else if (odev->features & NETIF_F_V6_CSUM) {
@@ -2984,7 +2986,7 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
 		skb->csum_offset = offsetof(struct udphdr, check);
 		udph->check = ~csum_ipv6_magic(&iph->saddr, &iph->daddr, udplen, IPPROTO_UDP, 0);
 	} else {
-		__wsum csum = udp_csum(skb);
+		__wsum csum = skb_checksum(skb, skb_transport_offset(skb), udplen, 0);
 
 		/* add protocol-dependent pseudo-header */
 		udph->check = csum_ipv6_magic(&iph->saddr, &iph->daddr, udplen, IPPROTO_UDP, csum);
@@ -2993,8 +2995,6 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
 			udph->check = CSUM_MANGLED_0;
 	}
 
-	pktgen_finalize_skb(pkt_dev, skb, datalen);
-
 	return skb;
 }
 
-- 
2.2.2

Re: [PATCH net] pktgen: fix UDP checksum computation

From: Thomas Graf <tgraf@suug.ch>
Date: 2015-02-05 10:52:19

On 02/04/15 at 11:08pm, Sabrina Dubroca wrote:
This patch fixes two issues in UDP checksum computation in pktgen.

First, the pseudo-header uses the source and destination IP
addresses. Currently, the ports are used for IPv4.

Second, the UDP checksum covers both header and data.  So we need to
generate the data earlier (move pktgen_finalize_skb up), and compute
the checksum for UDP header + data.

Fixes: c26bf4a51308c ("pktgen: Add UDPCSUM flag to support UDP checksums")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Acked-by: Thomas Graf <tgraf@suug.ch>

Thanks for fixing this.

Re: [PATCH net] pktgen: fix UDP checksum computation

From: David Miller <davem@davemloft.net>
Date: 2015-02-05 23:42:11

From: Sabrina Dubroca <sd@queasysnail.net>
Date: Wed,  4 Feb 2015 23:08:50 +0100
This patch fixes two issues in UDP checksum computation in pktgen.

First, the pseudo-header uses the source and destination IP
addresses. Currently, the ports are used for IPv4.

Second, the UDP checksum covers both header and data.  So we need to
generate the data earlier (move pktgen_finalize_skb up), and compute
the checksum for UDP header + data.

Fixes: c26bf4a51308c ("pktgen: Add UDPCSUM flag to support UDP checksums")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Applied and queued up for -stable, thanks.

Re: [PATCH net] pktgen: fix UDP checksum computation

From: Tom Herbert <hidden>
Date: 2015-02-05 23:57:48

On Wed, Feb 4, 2015 at 2:08 PM, Sabrina Dubroca [off-list ref] wrote:
quoted hunk
This patch fixes two issues in UDP checksum computation in pktgen.

First, the pseudo-header uses the source and destination IP
addresses. Currently, the ports are used for IPv4.

Second, the UDP checksum covers both header and data.  So we need to
generate the data earlier (move pktgen_finalize_skb up), and compute
the checksum for UDP header + data.

Fixes: c26bf4a51308c ("pktgen: Add UDPCSUM flag to support UDP checksums")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
 net/core/pktgen.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index da934fc3faa8..9fa25b0ea145 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2842,25 +2842,25 @@ static struct sk_buff *fill_packet_ipv4(struct net_device *odev,
        skb->dev = odev;
        skb->pkt_type = PACKET_HOST;

+       pktgen_finalize_skb(pkt_dev, skb, datalen);
+
        if (!(pkt_dev->flags & F_UDPCSUM)) {
                skb->ip_summed = CHECKSUM_NONE;
        } else if (odev->features & NETIF_F_V4_CSUM) {
                skb->ip_summed = CHECKSUM_PARTIAL;
                skb->csum = 0;
-               udp4_hwcsum(skb, udph->source, udph->dest);
+               udp4_hwcsum(skb, iph->saddr, iph->daddr);
        } else {
-               __wsum csum = udp_csum(skb);
+               __wsum csum = skb_checksum(skb, skb_transport_offset(skb), datalen + 8, 0);

                /* add protocol-dependent pseudo-header */
-               udph->check = csum_tcpudp_magic(udph->source, udph->dest,
+               udph->check = csum_tcpudp_magic(iph->saddr, iph->daddr,
                                                datalen + 8, IPPROTO_UDP, csum);

                if (udph->check == 0)
                        udph->check = CSUM_MANGLED_0;
        }
Would it be possible to use udp_set_csum to handle all of this?

Thanks,
Tom

quoted hunk
-       pktgen_finalize_skb(pkt_dev, skb, datalen);
-
 #ifdef CONFIG_XFRM
        if (!process_ipsec(pkt_dev, skb, protocol))
                return NULL;
@@ -2976,6 +2976,8 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
        skb->dev = odev;
        skb->pkt_type = PACKET_HOST;

+       pktgen_finalize_skb(pkt_dev, skb, datalen);
+
        if (!(pkt_dev->flags & F_UDPCSUM)) {
                skb->ip_summed = CHECKSUM_NONE;
        } else if (odev->features & NETIF_F_V6_CSUM) {
@@ -2984,7 +2986,7 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
                skb->csum_offset = offsetof(struct udphdr, check);
                udph->check = ~csum_ipv6_magic(&iph->saddr, &iph->daddr, udplen, IPPROTO_UDP, 0);
        } else {
-               __wsum csum = udp_csum(skb);
+               __wsum csum = skb_checksum(skb, skb_transport_offset(skb), udplen, 0);

                /* add protocol-dependent pseudo-header */
                udph->check = csum_ipv6_magic(&iph->saddr, &iph->daddr, udplen, IPPROTO_UDP, csum);
@@ -2993,8 +2995,6 @@ static struct sk_buff *fill_packet_ipv6(struct net_device *odev,
                        udph->check = CSUM_MANGLED_0;
        }

-       pktgen_finalize_skb(pkt_dev, skb, datalen);
-
        return skb;
 }

--
2.2.2

--
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

Re: [PATCH net] pktgen: fix UDP checksum computation

From: Eric Dumazet <hidden>
Date: 2015-02-06 00:28:04

On Thu, 2015-02-05 at 15:57 -0800, Tom Herbert wrote:
Would it be possible to use udp_set_csum to handle all of this?
This reminds me we have following sparse errors with checksum code,
could you take a look Tom ?

make C=2 CF=-D__CHECK_ENDIAN__  ...

include/linux/skbuff.h:3022:24: warning: incorrect type in return expression (different base types)
include/linux/skbuff.h:3022:24:    expected restricted __sum16
include/linux/skbuff.h:3022:24:    got int

net/ipv6/ip6_checksum.c:83:16: warning: incorrect type in return expression (different base types)
net/ipv6/ip6_checksum.c:83:16:    expected int
net/ipv6/ip6_checksum.c:83:16:    got restricted __sum16 [assigned] [usertype] __ret

  CHECK   net/ipv4/tcp_offload.c
net/ipv4/tcp_offload.c:139:60: warning: incorrect type in argument 2 (different base types)
net/ipv4/tcp_offload.c:139:60:    expected restricted __wsum [usertype] res
net/ipv4/tcp_offload.c:139:60:    got fouled restricted __sum16
include/linux/skbuff.h:3317:14: warning: incorrect type in assignment (different base types)
include/linux/skbuff.h:3317:14:    expected unsigned short [unsigned] [usertype] csum
include/linux/skbuff.h:3317:14:    got restricted __sum16
include/linux/skbuff.h:3322:16: warning: incorrect type in return expression (different base types)
include/linux/skbuff.h:3322:16:    expected restricted __sum16
include/linux/skbuff.h:3322:16:    got unsigned short [unsigned] [usertype] csum
net/ipv4/tcp_offload.c:173:52: warning: incorrect type in argument 2 (different base types)
net/ipv4/tcp_offload.c:173:52:    expected restricted __wsum [usertype] res
net/ipv4/tcp_offload.c:173:52:    got fouled restricted __sum16
include/linux/skbuff.h:3317:14: warning: incorrect type in assignment (different base types)
include/linux/skbuff.h:3317:14:    expected unsigned short [unsigned] [usertype] csum
include/linux/skbuff.h:3317:14:    got restricted __sum16
include/linux/skbuff.h:3322:16: warning: incorrect type in return expression (different base types)
include/linux/skbuff.h:3322:16:    expected restricted __sum16
include/linux/skbuff.h:3322:16:    got unsigned short [unsigned] [usertype] csum

Re: [PATCH net] pktgen: fix UDP checksum computation

From: Sabrina Dubroca <sd@queasysnail.net>
Date: 2015-02-06 13:46:50

2015-02-05, 16:28:02 -0800, Eric Dumazet wrote:
On Thu, 2015-02-05 at 15:57 -0800, Tom Herbert wrote:
quoted
Would it be possible to use udp_set_csum to handle all of this?
I will look.

This reminds me we have following sparse errors with checksum code,
could you take a look Tom ?

make C=2 CF=-D__CHECK_ENDIAN__  ...

include/linux/skbuff.h:3022:24: warning: incorrect type in return expression (different base types)
include/linux/skbuff.h:3022:24:    expected restricted __sum16
include/linux/skbuff.h:3022:24:    got int

net/ipv6/ip6_checksum.c:83:16: warning: incorrect type in return expression (different base types)
net/ipv6/ip6_checksum.c:83:16:    expected int
net/ipv6/ip6_checksum.c:83:16:    got restricted __sum16 [assigned] [usertype] __ret

  CHECK   net/ipv4/tcp_offload.c
net/ipv4/tcp_offload.c:139:60: warning: incorrect type in argument 2 (different base types)
net/ipv4/tcp_offload.c:139:60:    expected restricted __wsum [usertype] res
net/ipv4/tcp_offload.c:139:60:    got fouled restricted __sum16
include/linux/skbuff.h:3317:14: warning: incorrect type in assignment (different base types)
include/linux/skbuff.h:3317:14:    expected unsigned short [unsigned] [usertype] csum
include/linux/skbuff.h:3317:14:    got restricted __sum16
include/linux/skbuff.h:3322:16: warning: incorrect type in return expression (different base types)
include/linux/skbuff.h:3322:16:    expected restricted __sum16
include/linux/skbuff.h:3322:16:    got unsigned short [unsigned] [usertype] csum
net/ipv4/tcp_offload.c:173:52: warning: incorrect type in argument 2 (different base types)
net/ipv4/tcp_offload.c:173:52:    expected restricted __wsum [usertype] res
net/ipv4/tcp_offload.c:173:52:    got fouled restricted __sum16
include/linux/skbuff.h:3317:14: warning: incorrect type in assignment (different base types)
include/linux/skbuff.h:3317:14:    expected unsigned short [unsigned] [usertype] csum
include/linux/skbuff.h:3317:14:    got restricted __sum16
include/linux/skbuff.h:3322:16: warning: incorrect type in return expression (different base types)
include/linux/skbuff.h:3322:16:    expected restricted __sum16
include/linux/skbuff.h:3322:16:    got unsigned short [unsigned] [usertype] csum
I have patches ready for that and other sparse warnings, I just need
to check and send them.


Thanks

-- 
Sabrina
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help