netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] SNMPv2 udpInDatagrams counter error
@ 2006-07-31 11:21 Wei Yongjun
  0 siblings, 0 replies; 6+ messages in thread
From: Wei Yongjun @ 2006-07-31 11:21 UTC (permalink / raw)
  To: Gerrit Renker; +Cc: netdev

This change does not effect to tcpdump, only let UDP filter can not 
received UDP datagrams with checksum error. It is not a good idea, but
I think is the best way to resolve this problem. If you want to capture
error UDP packet, you can used tcpdump.

On Monday 31 July 2006 04:57, Gerrit Renker wrote:
> Hi,
>
> |  if (!sk->sk_filter && skb->ip_summed != CHECKSUM_UNNECESSARY) {
> |
> |  IPv6 doesn't do this, so I think delete condition 'sk->sk_filter'
is
> | better. Do you think so?
>
> I think the sk->sk_filter is there for a good reason. If you delete
it,
> that routine is forced to always compute UDP checksums, even if the
only
> receiving application is a tcpdump process. I may be wrong here, but I
> think that deleting the sk_filter statement is not at a good idea.
>
> The other alternatives discussed (afaik) so far were:
>
> 1) Move the increment of UDP_MIB_INDATAGRAMS from udp_queue_rcv_skb()
to
> udp_recvmsg() (first patch uploaded to 
> http://bugzilla.kernel.org/show_bug.cgi?id=6660). This was discussed:
not a
> good idea, since in-kernel applications may use the data_ready handler
> rather than udp_recvmsg().
>
> 2) Decrement UDP_MIB_INDATAGRAMS in udp_recvmsg() when the checksum
turns
> out to be wrong (second patch uploaded to above address). This would
be a
> fix to the problem you are stating, it also solves the problem of
missing
> out the data_ready handlers in (1), and was suggested earlier on this
> mailing list.


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] SNMPv2 udpInDatagrams counter error
@ 2006-07-31 10:03 Wei Yongjun
  2006-07-31 10:42 ` Herbert Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Yongjun @ 2006-07-31 10:03 UTC (permalink / raw)
  To: Gerrit Renker, netdev; +Cc: David Miller

Yes, you are right. 
I also send the same mail several ago, and get no response. You
patch is fine. But I think following code has no effect:

if (sk->sk_filter && skb->ip_summed != CHECKSUM_UNNECESSARY) {

It just let UDP datagrams with checksum error be added into  UDP receive
queue, and then discard it. I think this can be used to capture a UDP
datagrams use a filter. But I if use a filter to capture a UDP
datagrams,  the code should like that:

if (!sk->sk_filter && skb->ip_summed != CHECKSUM_UNNECESSARY) {

IPv6 doesn't do this, so I think delete condition 'sk->sk_filter' is
better. Do you think so?

Specially, do you try to send UDP datagrams with checksum error to echo-
udp(port 7), may be your patch will let neither udpInDatagrams nor
udpInErrors be increased. Because in my test, that datagrams can be send
to echo-udp and get a echo reply.

----- Original Message ----- 
From: "Gerrit Renker" <gerrit@erg.abdn.ac.uk>
To: <netdev@vger.kernel.org>
Sent: Monday, July 31, 2006 4:19 PM
Subject: Re: [PATCH] SNMPv2 udpInDatagrams counter error


> This has been raised earlier, cf.
http://bugzilla.kernel.org/show_bug.cgi?id=6660
> 
> Wei Yongjun wrote:
> |  When I send a UDP datagrams with checksum error to target, I found
that:
> |  Under IPv6, counter udpInErrors increased, but under IPv4 counter
> |  udpInDatagrams increased. I lookup into the source code, and found
that,
> |  under IPv4 UDP datagrams with checksum error will be delivered to
UDP
> |  receive queue, but IPv6 does not. IPv4 delivered into UDP receive
queue,
> |  increased udpInDatagrams, then discard it before delivered to UDP
user.
> |  RFC said udpInDatagrams is the total number of UDP datagrams
delivered
> |  to UDP users, so udpInDatagrams should not be increased while UDP
> |  datagrams with checksum error received.
> |  
> |  Refer to RFC2013:
> |    udpInDatagrams OBJECT-TYPE
> |        SYNTAX      Counter32
> |        MAX-ACCESS  read-only
> |        STATUS      current
> |        DESCRIPTION
> |                "The total number of UDP datagrams delivered to UDP
> |  users."
> |        ::= { udp 1 }
> |  
> |  Following is my patch:
> |  
> |  --- a/net/ipv4/udp.c 2006-07-31 09:33:45.392479344 -0400
> |  +++ b/net/ipv4/udp.c 2006-07-31 09:34:26.430240656 -0400
> |  @@ -1018,7 +1018,7 @@ static int udp_queue_rcv_skb(struct sock
> |   /* FALLTHROUGH -- it's a UDP Packet */
> |   }
> |   
> |  - if (sk->sk_filter && skb->ip_summed != CHECKSUM_UNNECESSARY) {
> |  + if (skb->ip_summed != CHECKSUM_UNNECESSARY) {
> |   if (__udp_checksum_complete(skb)) {
> |   UDP_INC_STATS_BH(UDP_MIB_INERRORS);
> |   kfree_skb(skb);
> |  
> |  Signed-off-by: Wei Yongjun <yjwei@nanjing-fnst.com>
> |  
> |  
> |  -
> |  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
> |  
> |  
> -
> 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
* [PATCH] SNMPv2 udpInDatagrams counter error
@ 2006-07-31  8:51 Wei Yongjun
  2006-07-31  8:19 ` Gerrit Renker
  0 siblings, 1 reply; 6+ messages in thread
From: Wei Yongjun @ 2006-07-31  8:51 UTC (permalink / raw)
  To: netdev

When I send a UDP datagrams with checksum error to target, I found that:
Under IPv6, counter udpInErrors increased, but under IPv4 counter
udpInDatagrams increased. I lookup into the source code, and found that,
under IPv4 UDP datagrams with checksum error will be delivered to UDP
receive queue, but IPv6 does not. IPv4 delivered into UDP receive queue,
increased udpInDatagrams, then discard it before delivered to UDP user.
RFC said udpInDatagrams is the total number of UDP datagrams delivered
to UDP users, so udpInDatagrams should not be increased while UDP
datagrams with checksum error received.

Refer to RFC2013:
  udpInDatagrams OBJECT-TYPE
      SYNTAX      Counter32
      MAX-ACCESS  read-only
      STATUS      current
      DESCRIPTION
              "The total number of UDP datagrams delivered to UDP
users."
      ::= { udp 1 }

Following is my patch:

--- a/net/ipv4/udp.c	2006-07-31 09:33:45.392479344 -0400
+++ b/net/ipv4/udp.c	2006-07-31 09:34:26.430240656 -0400
@@ -1018,7 +1018,7 @@ static int udp_queue_rcv_skb(struct sock
 		/* FALLTHROUGH -- it's a UDP Packet */
 	}
 
-	if (sk->sk_filter && skb->ip_summed != CHECKSUM_UNNECESSARY) {
+	if (skb->ip_summed != CHECKSUM_UNNECESSARY) {
 		if (__udp_checksum_complete(skb)) {
 			UDP_INC_STATS_BH(UDP_MIB_INERRORS);
 			kfree_skb(skb);

Signed-off-by: Wei Yongjun <yjwei@nanjing-fnst.com>



^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-07-31 10:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-07-31 11:21 [PATCH] SNMPv2 udpInDatagrams counter error Wei Yongjun
  -- strict thread matches above, loose matches on Subject: below --
2006-07-31 10:03 Wei Yongjun
2006-07-31 10:42 ` Herbert Xu
2006-07-31  8:51 Wei Yongjun
2006-07-31  8:19 ` Gerrit Renker
     [not found]   ` <075801c6b47d$08c49350$6004a8c0@WeiYongjun>
2006-07-31  8:57     ` Gerrit Renker

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