* [PATCH net] pktgen: fix UDP checksum computation
@ 2015-02-04 22:08 Sabrina Dubroca
2015-02-05 10:52 ` Thomas Graf
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sabrina Dubroca @ 2015-02-04 22:08 UTC (permalink / raw)
To: davem; +Cc: netdev, tgraf, Sabrina Dubroca
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net] pktgen: fix UDP checksum computation
2015-02-04 22:08 [PATCH net] pktgen: fix UDP checksum computation Sabrina Dubroca
@ 2015-02-05 10:52 ` Thomas Graf
2015-02-05 23:42 ` David Miller
2015-02-05 23:57 ` Tom Herbert
2 siblings, 0 replies; 6+ messages in thread
From: Thomas Graf @ 2015-02-05 10:52 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: davem, netdev
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] pktgen: fix UDP checksum computation
2015-02-04 22:08 [PATCH net] pktgen: fix UDP checksum computation Sabrina Dubroca
2015-02-05 10:52 ` Thomas Graf
@ 2015-02-05 23:42 ` David Miller
2015-02-05 23:57 ` Tom Herbert
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2015-02-05 23:42 UTC (permalink / raw)
To: sd; +Cc: netdev, tgraf
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] pktgen: fix UDP checksum computation
2015-02-04 22:08 [PATCH net] pktgen: fix UDP checksum computation Sabrina Dubroca
2015-02-05 10:52 ` Thomas Graf
2015-02-05 23:42 ` David Miller
@ 2015-02-05 23:57 ` Tom Herbert
2015-02-06 0:28 ` Eric Dumazet
2 siblings, 1 reply; 6+ messages in thread
From: Tom Herbert @ 2015-02-05 23:57 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: David Miller, Linux Netdev List, Thomas Graf
On Wed, Feb 4, 2015 at 2:08 PM, Sabrina Dubroca <sd@queasysnail.net> 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>
> ---
> 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
> - 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] pktgen: fix UDP checksum computation
2015-02-05 23:57 ` Tom Herbert
@ 2015-02-06 0:28 ` Eric Dumazet
2015-02-06 13:46 ` Sabrina Dubroca
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2015-02-06 0:28 UTC (permalink / raw)
To: Tom Herbert; +Cc: Sabrina Dubroca, David Miller, Linux Netdev List, Thomas Graf
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] pktgen: fix UDP checksum computation
2015-02-06 0:28 ` Eric Dumazet
@ 2015-02-06 13:46 ` Sabrina Dubroca
0 siblings, 0 replies; 6+ messages in thread
From: Sabrina Dubroca @ 2015-02-06 13:46 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Tom Herbert, David Miller, Linux Netdev List, Thomas Graf
2015-02-05, 16:28:02 -0800, Eric Dumazet wrote:
> 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?
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-02-06 13:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-04 22:08 [PATCH net] pktgen: fix UDP checksum computation Sabrina Dubroca
2015-02-05 10:52 ` Thomas Graf
2015-02-05 23:42 ` David Miller
2015-02-05 23:57 ` Tom Herbert
2015-02-06 0:28 ` Eric Dumazet
2015-02-06 13:46 ` Sabrina Dubroca
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).