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

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

[Wei Yongjun]

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 <yjwei@cn.fujitsu.com>
---
 net/ipv6/udp.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index e51da8c..1a63809 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -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 {
-- 
1.5.3.8

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

[David Miller]

From: Wei Yongjun <yjwei@cn.fujitsu.com>
Date: Sat, 01 Nov 2008 17:08:55 +0800

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

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

[Philip Craig]

David Miller wrote:
> From: Wei Yongjun <yjwei@cn.fujitsu.com>
> Date: Sat, 01 Nov 2008 17:08:55 +0800
> 
>> @@ -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
>  ...
>>  	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.

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

[Wei Yongjun]

Philip Craig wrote:
> David Miller wrote:
>   
>> From: Wei Yongjun <yjwei@cn.fujitsu.com>
>> Date: Sat, 01 Nov 2008 17:08:55 +0800
>>
>>     
>>> @@ -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
>>>       
>>  ...
>>     
>>>  	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.
>   

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.^_^

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

[Wei Yongjun]

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 <yjwei@cn.fujitsu.com>
---
 net/ipv6/udp.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index e51da8c..1a63809 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -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 {
-- 
1.5.3.8

Re: [Resent] [PATCH 1/2] udp: Fix the SNMP counter of UDP_MIB_INDATAGRAMS

[David Miller]

From: Wei Yongjun <yjwei@cn.fujitsu.com>
Date: Mon, 03 Nov 2008 10:11:01 +0800

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

Re: [Resent] [PATCH 1/2] udp: Fix the SNMP counter of UDP_MIB_INDATAGRAMS

[Pekka Savola]

On Sun, 2 Nov 2008, David Miller wrote:
> From: Wei Yongjun <yjwei@cn.fujitsu.com>
> Date: Mon, 03 Nov 2008 10:11:01 +0800
>
>> @@ -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 ?

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

Re: [Resent] [PATCH 1/2] udp: Fix the SNMP counter of UDP_MIB_INDATAGRAMS

[David Miller]

From: Pekka Savola <pekkas@netcore.fi>
Date: Mon, 3 Nov 2008 10:01:42 +0200 (EET)

> On Sun, 2 Nov 2008, David Miller wrote:
> > From: Wei Yongjun <yjwei@cn.fujitsu.com>
> > Date: Mon, 03 Nov 2008 10:11:01 +0800
> >
> >> @@ -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.

[Resend] [PATCH 2/2] udp: Fix the SNMP counter of UDP_MIB_INERRORS

[Wei Yongjun]

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 <yjwei@cn.fujitsu.com>
---
 net/ipv6/udp.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 1a63809..2f918a0 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -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)
-- 
1.5.3.8