Skip to contentSkip to conversation

Fix the "InAddrErrors" increasing problem

PATCH In review Gui Jianfeng 3 participants 6 messages Oct 22, 2007

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Gui Jianfeng

author
#0 lore raw

PATCH 1/1 Fix the "InAddrErrors" increasing problem

Hi,
When kernel receives a package with a wrong destination ipv4 address, it can't increase "InAddrErrors" number correctly. InAddrErrors is located in /proc/net/snmp.

This is a patch for fixing this problem.

Signed-off-by Gui Jianfeng <[email protected]>

---

net/ipv4/ip_input.c +6 -2
diff -Narup linux-2.4.35.3/net/ipv4/ip_input.c linux-2.4.35.3-prep/net/ipv4/ip_input.c
@@ -310,8 +310,12 @@ static inline int ip_rcv_finish(struct s
 	 *	how the packet travels inside Linux networking.
 	 */ 
 	if (skb->dst == NULL) {
-		if (ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev))
-			goto drop; 
+		int err = ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev);
+		if (unlikely(err)) {
+			if (err == -EHOSTUNREACH)
+				IP_INC_STATS_BH(IpInAddrErrors);
+		}
+		goto drop; 
 	}
 
 #ifdef CONFIG_NET_CLS_ROUTE
net/ipv4/route.c +7 -2
diff -Narup linux-2.4.35.3/net/ipv4/route.c linux-2.4.35.3-prep/net/ipv4/route.c
@@ -1450,7 +1450,7 @@ int ip_route_input_slow(struct sk_buff *
 	 */
 	if ((err = fib_lookup(&key, &res)) != 0) {
 		if (!IN_DEV_FORWARD(in_dev))
-			goto e_inval;
+			goto e_hostunreach;
 		goto no_route;
 	}
 	free_res = 1;

@@ -1499,7 +1499,7 @@ int ip_route_input_slow(struct sk_buff *
 	}
 
 	if (!IN_DEV_FORWARD(in_dev))
-		goto e_inval;
+		goto e_hostunreach;
 	if (res.type != RTN_UNICAST)
 		goto martian_destination;
 

@@ -1668,6 +1668,11 @@ martian_destination:
 			"%u.%u.%u.%u, dev %s\n",
 			NIPQUAD(daddr), NIPQUAD(saddr), dev->name);
 #endif
+
+e_hostunreach:
+	err = -EHOSTUNREACH;
+	goto done;
+
 e_inval:
 	err = -EINVAL;
 	goto done;
maintainer
#1 lore raw

From: Gui Jianfeng <[email protected]>
Date: Mon, 22 Oct 2007 14:07:07 +0800

Quoted 6 lines

When kernel receives a package with a wrong destination ipv4 address, it can't increase "InAddrErrors" number correctly. InAddrErrors is located in /proc/net/snmp.

This is a patch for fixing this problem.

Signed-off-by Gui Jianfeng <[email protected]>

This patch looks fine, but I really don't handle 2.4.x kernel patches for networking as 2.6.x takes enough of my time and effort.

#2 lore raw
In reply to Gui Jianfeng

Gui Jianfeng wrote on 10/22/2007 11:37:07 AM:

net/ipv4/ip_input.c static inline int ip_rcv_finish(struct s · line 313

Quoted 7 lines
net/ipv4/ip_input.c +1 -2
@@ -310,8 +310,12 @@ static inline int ip_rcv_finish(struct s
     *   how the packet travels inside Linux networking.
     */
    if (skb->dst == NULL) {
-      if (ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev))
-         goto drop;
+      int err = ip_route_input(skb, iph->daddr, iph->saddr, iph->tos,

dev);

net/ipv4/ip_input.c static inline int ip_rcv_finish(struct s · line 319

Quoted 6 lines
net/ipv4/ip_input.c +5 -0
+      if (unlikely(err)) {
+         if (err == -EHOSTUNREACH)
+            IP_INC_STATS_BH(IpInAddrErrors);
+      }
+      goto drop;
    }

Shouldn't the "goto drop" be inside the "if (unlikely(err)) {" case? And normally it is nice to have a blank line after variable declaration.

- KK

author
#3 lore raw
Quoted 10 lines · Krishna Kumar2 写道:

Gui Jianfeng wrote on 10/22/2007 11:37:07 AM:

net/ipv4/ip_input.c static inline int ip_rcv_finish(struct s · line 313

Quoted 7 lines
net/ipv4/ip_input.c +1 -2
@@ -310,8 +310,12 @@ static inline int ip_rcv_finish(struct s
     *   how the packet travels inside Linux networking.
     */
    if (skb->dst == NULL) {
-      if (ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev))
-         goto drop;
+      int err = ip_route_input(skb, iph->daddr, iph->saddr, iph->tos,

dev);

Quoted 8 lines

net/ipv4/ip_input.c static inline int ip_rcv_finish(struct s · line 319

Quoted 6 lines
net/ipv4/ip_input.c +5 -0
+      if (unlikely(err)) {
+         if (err == -EHOSTUNREACH)
+            IP_INC_STATS_BH(IpInAddrErrors);
+      }
+      goto drop;
    }

Shouldn't the "goto drop" be inside the "if (unlikely(err)) {" case?

yes, you are right :-)

Quoted 6 lines

And normally it is nice to have a blank line after variable declaration.

- KK

Signature · 13 lines
Regards
Gui Jianfeng
--------------------------------------------------
Gui Jianfeng
Development Dept.I
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
8/F., Civil Defense Building, No.189 Guangzhou Road,
Nanjing, 210029, China
TEL: +86+25-86630566-851
COINS: 79955-851
FAX: +86+25-83317685
MAIL:[email protected]
--------------------------------------------------
author
#4 lore raw
In reply to Krishna Kumar2
Quoted 17 lines · Krishna Kumar2 写道:

Gui Jianfeng wrote on 10/22/2007 11:37:07 AM:

Review static inline int ip_rcv_finish(struct s · line 313

Quoted 7 lines
(quoted diff) +1 -2
@@ -310,8 +310,12 @@ static inline int ip_rcv_finish(struct s
     *   how the packet travels inside Linux networking.
     */
    if (skb->dst == NULL) {
-      if (ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev))
-         goto drop;
+      int err = ip_route_input(skb, iph->daddr, iph->saddr, iph->tos,

dev);

Review static inline int ip_rcv_finish(struct s · line 319

Quoted 6 lines
(quoted diff) +5 -0
+      if (unlikely(err)) {
+         if (err == -EHOSTUNREACH)
+            IP_INC_STATS_BH(IpInAddrErrors);
+      }
+      goto drop;
    }
Quoted 2 lines

Shouldn't the "goto drop" be inside the "if (unlikely(err)) {" case? And normally it is nice to have a blank line after variable declaration.

sorry for my careless, here is the correct one

Signed-off-by Gui Jianfeng <[email protected]>

-------

net/ipv4/ip_input.c +6 -1
diff -Narup linux-2.4.35.3/net/ipv4/ip_input.c linux-2.4.35.3-prep/net/ipv4/ip_input.c
@@ -310,8 +310,13 @@ static inline int ip_rcv_finish(struct s
 	 *	how the packet travels inside Linux networking.
 	 */ 
 	if (skb->dst == NULL) {
-		if (ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev))
+		int err = ip_route_input(skb, iph->daddr, iph->saddr, iph->tos, dev);
+
+		if (unlikely(err)) {
+			if (err == -EHOSTUNREACH)
+				IP_INC_STATS_BH(IpInAddrErrors);
 			goto drop; 
+		}
 	}
 
 #ifdef CONFIG_NET_CLS_ROUTE
net/ipv4/route.c +7 -2
diff -Narup linux-2.4.35.3/net/ipv4/route.c linux-2.4.35.3-prep/net/ipv4/route.c
@@ -1450,7 +1450,7 @@ int ip_route_input_slow(struct sk_buff *
 	 */
 	if ((err = fib_lookup(&key, &res)) != 0) {
 		if (!IN_DEV_FORWARD(in_dev))
-			goto e_inval;
+			goto e_hostunreach;
 		goto no_route;
 	}
 	free_res = 1;

@@ -1499,7 +1499,7 @@ int ip_route_input_slow(struct sk_buff *
 	}
 
 	if (!IN_DEV_FORWARD(in_dev))
-		goto e_inval;
+		goto e_hostunreach;
 	if (res.type != RTN_UNICAST)
 		goto martian_destination;
 

@@ -1668,6 +1668,11 @@ martian_destination:
 			"%u.%u.%u.%u, dev %s\n",
 			NIPQUAD(daddr), NIPQUAD(saddr), dev->name);
 #endif
+
+e_hostunreach:
+	err = -EHOSTUNREACH;
+	goto done;
+
 e_inval:
 	err = -EINVAL;
 	goto done;
Quoted 5 lines

- KK

maintainer
#5 lore raw

From: Gui Jianfeng <[email protected]>
Date: Mon, 22 Oct 2007 15:10:46 +0800

Quoted 3 lines

sorry for my careless, here is the correct one

Signed-off-by Gui Jianfeng <[email protected]>

BTW, I would also like to say that I think a statistics correction is totally inappropriate for the 2.4.x kernel series which is in super maintainence mode.

Only crash fixes and fixes for bugs that severely harm functionality should go into that tree.

These obscure statistic counter cases absolutely do not qualify.