udp: Fix the SNMP counter of UDP_MIB_INDATAGRAMS

PATCH In review Wei Yongjun 4 participants 9 messages Nov 1, 2008 – Nov 3, 2008

Subsystems (3)

Networking Network drivers UDP

lore

Share & tools

lore

Wei Yongjun

author

2008-11-01 09:13 UTC #0 lore raw

PATCH 1/2 udp: Fix the SNMP counter of UDP_MIB_INDATAGRAMS

If UDP echo is sent to xinetd/echo-dgram, the UDP reply will be received at the sender. But the SNMP counter of UDP_MIB_INDATAGRAMS will be not increased, UDP6_MIB_INDATAGRAMS will be increased instead.

Endpoint A Endpoint B
UDP Echo request ----------->
(IPv4, Dst port=7)

                  <----------    UDP Echo Reply
                                 (IPv4, Src port=7)

This bug is come from this patch cb75994ec311b2cd50e5205efdcc0696abd6675d.

It do counter UDP[6]_MIB_INDATAGRAMS until udp[v6]_recvmsg. Because xinetd used IPv6 socket to receive UDP messages, thus, when received UDP packet, the UDP6_MIB_INDATAGRAMS will be increased in function udpv6_recvmsg() even if the packet is a IPv4 UDP packet.

This patch fixed the problem.

Signed-off-by Wei Yongjun <[email protected]>

---
net/ipv6/udp.c | 18 +++++++++++++-----

1 files changed, 13 insertions(+), 5 deletions(-)

net/ipv6/udp.c +0 -0

index e51da8c..1a63809 100644
@@ -138,6 +138,7 @@ int udpv6_recvmsg(struct kiocb *iocb, struct sock *sk,

	int peeked;
	int err;
	int is_udplite = IS_UDPLITE(sk);

+ int is_udp4;

	if (addr_len)
		*addr_len=sizeof(struct sockaddr_in6);
@@ -158,6 +159,8 @@ try_again:
	else if (copied < ulen)
		msg->msg_flags |= MSG_TRUNC;

+ is_udp4 = (skb->protocol == htons(ETH_P_IP));
+
/*
* If checksum is needed at all, try to do it while copying the
* data. If the data is truncated, or if we only want a partial
@@ -180,9 +183,14 @@ try_again:

	if (err)
		goto out_free;

- if (!peeked)
- UDP6_INC_STATS_USER(sock_net(sk),
- UDP_MIB_INDATAGRAMS, is_udplite);
+ if (!peeked) {
+ if (is_udp4)
+ UDP_INC_STATS_USER(sock_net(sk),
+ UDP_MIB_INDATAGRAMS, is_udplite);
+ else
+ UDP6_INC_STATS_USER(sock_net(sk),
+ UDP_MIB_INDATAGRAMS, is_udplite);
+ }

	sock_recv_timestamp(msg, sk, skb);

@@ -196,7 +204,7 @@ try_again:

		sin6->sin6_flowinfo = 0;
		sin6->sin6_scope_id = 0;

- if (skb->protocol == htons(ETH_P_IP))
+ if (is_udp4)

			ipv6_addr_set(&sin6->sin6_addr, 0, 0,
				      htonl(0xffff), ip_hdr(skb)->saddr);
		else {
@@ -207,7 +215,7 @@ try_again:
		}

	}
-	if (skb->protocol == htons(ETH_P_IP)) {
+	if (is_udp4) {
		if (inet->cmsg_flags)
			ip_cmsg_recv(msg, skb);
	} else {

Signature · 1 line

1.5.3.8

David Miller

maintainer

2008-11-02 04:35 UTC #1 lore raw

From: Wei Yongjun <[email protected]>
Date: Sat, 01 Nov 2008 17:08:55 +0800

Quoted 8 lines

(quoted diff) +1 -0

@@ -158,6 +159,8 @@ try_again:
 	else if (copied < ulen)
 		msg->msg_flags |= MSG_TRUNC;
 +	is_udp4 = (skb->protocol == htons(ETH_P_IP));
+
 	/*
 	 * If checksum is needed at all, try to do it while copying the
 	 * data.  If the data is truncated, or if we only want a partial

...

Review

Quoted 7 lines

(quoted diff) +1 -0

 	sock_recv_timestamp(msg, sk, skb);
 @@ -196,7 +204,7 @@ try_again:
 		sin6->sin6_flowinfo = 0;
 		sin6->sin6_scope_id = 0;
 -		if (skb->protocol == htons(ETH_P_IP))
+		if (is_udp4)
 			ipv6_addr_set(&sin6->sin6_addr, 0, 0,

This patch is corrupted in ways I thought I would never see in my entire life. Congratulations!

Please clean this up and resubmit, along with your second patch of this set.

Thanks.

Philip Craig

2008-11-03 01:46 UTC #2 lore raw

Quoted 12 lines · David Miller

From: Wei Yongjun <[email protected]>
Date: Sat, 01 Nov 2008 17:08:55 +0800

Quoted 8 lines

(quoted diff) +1 -0

@@ -158,6 +159,8 @@ try_again:
 	else if (copied < ulen)
 		msg->msg_flags |= MSG_TRUNC;
 +	is_udp4 = (skb->protocol == htons(ETH_P_IP));
+
 	/*
 	 * If checksum is needed at all, try to do it while copying the
 	 * data.  If the data is truncated, or if we only want a partial

...

Quoted 10 lines

Review
Quoted 7 lines
(quoted diff) +1 -0
 	sock_recv_timestamp(msg, sk, skb);
 @@ -196,7 +204,7 @@ try_again:
 		sin6->sin6_flowinfo = 0;
 		sin6->sin6_scope_id = 0;
 -		if (skb->protocol == htons(ETH_P_IP))
+		if (is_udp4)
 			ipv6_addr_set(&sin6->sin6_addr, 0, 0,
This patch is corrupted in ways I thought I would never see in my entire life. Congratulations!

The patch looked okay to me, but judging by the bits you quoted, the format=flowed is causing the problem.

Wei Yongjun

author

2008-11-03 02:42 UTC #3 lore raw

Quoted 29 lines · Philip Craig

Quoted 15 lines · David Miller
From: Wei Yongjun <[email protected]>
Date: Sat, 01 Nov 2008 17:08:55 +0800
Quoted 9 lines
(quoted diff) +1 -0
@@ -158,6 +159,8 @@ try_again:
 	else if (copied < ulen)
 		msg->msg_flags |= MSG_TRUNC;
 +	is_udp4 = (skb->protocol == htons(ETH_P_IP));
+
 	/*
 	 * If checksum is needed at all, try to do it while copying the
 	 * data.  If the data is truncated, or if we only want a partial
...
Quoted 11 lines
Review
Quoted 8 lines
(quoted diff) +1 -0
 	sock_recv_timestamp(msg, sk, skb);
 @@ -196,7 +204,7 @@ try_again:
 		sin6->sin6_flowinfo = 0;
 		sin6->sin6_scope_id = 0;
 -		if (skb->protocol == htons(ETH_P_IP))
+		if (is_udp4)
 			ipv6_addr_set(&sin6->sin6_addr, 0, 0,
This patch is corrupted in ways I thought I would never see in my entire life. Congratulations!

Quoted 3 lines

The patch looked okay to me, but judging by the bits you quoted, the format=flowed is causing the problem.

It is also ok at http://patchwork.ozlabs.org/patch/6776/ and http://marc.info/?l=linux-netdev&m=122553080530450&w=2, maybe the mail client's action is strange. Anyway I have resent them, maybe this time it is not corrupted.^_^

Wei Yongjun

author

2008-11-03 02:15 UTC #4 lore raw

PATCH 1/2 udp: Fix the SNMP counter of UDP_MIB_INDATAGRAMS

In reply to David Miller

Endpoint A Endpoint B
UDP Echo request ----------->
(IPv4, Dst port=7)

                  <----------    UDP Echo Reply
                                 (IPv4, Src port=7)

This bug is come from this patch cb75994ec311b2cd50e5205efdcc0696abd6675d.

This patch fixed the problem.

Signed-off-by Wei Yongjun <[email protected]>

---
net/ipv6/udp.c | 18 +++++++++++++-----

1 files changed, 13 insertions(+), 5 deletions(-)

net/ipv6/udp.c +0 -0

index e51da8c..1a63809 100644
@@ -138,6 +138,7 @@ int udpv6_recvmsg(struct kiocb *iocb, struct sock *sk,

	int peeked;
	int err;
	int is_udplite = IS_UDPLITE(sk);

+ int is_udp4;

	if (addr_len)
		*addr_len=sizeof(struct sockaddr_in6);
@@ -158,6 +159,8 @@ try_again:
	else if (copied < ulen)
		msg->msg_flags |= MSG_TRUNC;

	if (err)
		goto out_free;

	sock_recv_timestamp(msg, sk, skb);

@@ -196,7 +204,7 @@ try_again:

		sin6->sin6_flowinfo = 0;
		sin6->sin6_scope_id = 0;

- if (skb->protocol == htons(ETH_P_IP))
+ if (is_udp4)

			ipv6_addr_set(&sin6->sin6_addr, 0, 0,
				      htonl(0xffff), ip_hdr(skb)->saddr);
		else {
@@ -207,7 +215,7 @@ try_again:
		}

	}
-	if (skb->protocol == htons(ETH_P_IP)) {
+	if (is_udp4) {
		if (inet->cmsg_flags)
			ip_cmsg_recv(msg, skb);
	} else {

Signature · 1 line

1.5.3.8

David Miller

maintainer

2008-11-03 07:46 UTC #5 lore raw

From: Wei Yongjun <[email protected]>
Date: Mon, 03 Nov 2008 10:11:01 +0800

Review try_again: · line 162

Quoted 5 lines

(quoted diff) +1 -0

@@ -158,6 +159,8 @@ try_again:
 	else if (copied < ulen)
 		msg->msg_flags |= MSG_TRUNC;
 +	is_udp4 = (skb->protocol == htons(ETH_P_IP));
+

I'm still getting these corruptions.

I will investigate the problem and submit a bug report for Mew if necessary.

I'll pull the patches out of patchwork since they look OK there.

However, this is not optimal for me until that Mew issue gets fixed, so for future submissions could you not use format=flowed? Or at least put the patch part in an attachment.

Thank you.

Pekka Savola

2008-11-03 08:02 UTC #6 lore raw

Quoted 8 lines · David Miller

From: Wei Yongjun <[email protected]>
Date: Mon, 03 Nov 2008 10:11:01 +0800
Review try_again: · line 162
Quoted 5 lines
(quoted diff) +1 -0
@@ -158,6 +159,8 @@ try_again:
 	else if (copied < ulen)
 		msg->msg_flags |= MSG_TRUNC;
 +	is_udp4 = (skb->protocol == htons(ETH_P_IP));
+

Does this work with non-ethernet link-layers ?

Signature · 3 lines

Pekka Savola                 "You each name yourselves king, yet the
Netcore Oy                    kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings

David Miller

maintainer

2008-11-03 08:08 UTC #7 lore raw

From: Pekka Savola <[email protected]>
Date: Mon, 3 Nov 2008 10:01:42 +0200 (EET)

Quoted 11 lines · David Miller

Quoted 8 lines
From: Wei Yongjun <[email protected]>
Date: Mon, 03 Nov 2008 10:11:01 +0800
Review try_again: · line 162
Quoted 5 lines
(quoted diff) +1 -0
@@ -158,6 +159,8 @@ try_again:
 	else if (copied < ulen)
 		msg->msg_flags |= MSG_TRUNC;
 +	is_udp4 = (skb->protocol == htons(ETH_P_IP));
+
Does this work with non-ethernet link-layers ?

Regardless of link layer, the skb gets marked with the ethernet protocol codes.

Wei Yongjun

author

2008-11-03 02:18 UTC #8 lore raw

PATCH RESEND 2/2 udp: Fix the SNMP counter of UDP_MIB_INERRORS

In reply to David Miller

UDP packets received in udpv6_recvmsg() are not only IPv6 UDP packets, but also have IPv4 UDP packets, so when do the counter of UDP_MIB_INERRORS in udpv6_recvmsg(), we should check whether the packet is a IPv6 UDP packet or a IPv4 UDP packet.

Signed-off-by Wei Yongjun <[email protected]>

---
net/ipv6/udp.c | 10 ++++++++--

1 files changed, 8 insertions(+), 2 deletions(-)

net/ipv6/udp.c +0 -0

index 1a63809..2f918a0 100644
@@ -236,8 +236,14 @@ out:

csum_copy_err:

	lock_sock(sk);

- if (!skb_kill_datagram(sk, skb, flags))
- UDP6_INC_STATS_USER(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
+ if (!skb_kill_datagram(sk, skb, flags)) {
+ if (is_udp4)
+ UDP_INC_STATS_USER(sock_net(sk),
+ UDP_MIB_INERRORS, is_udplite);
+ else
+ UDP6_INC_STATS_USER(sock_net(sk),
+ UDP_MIB_INERRORS, is_udplite);
+ }

	release_sock(sk);

	if (flags & MSG_DONTWAIT)

Signature · 1 line

1.5.3.8