multiple neighbour cache tables for AF_INET

* multiple neighbour cache tables for AF_INET
@ 2005-01-28 13:00 Wilfried Weissmann
  2005-01-28 22:19 ` Herbert Xu
  0 siblings, 1 reply; 4+ messages in thread
From: Wilfried Weissmann @ 2005-01-28 13:00 UTC (permalink / raw)
  To: linux-kernel, linux-net, davem, 282492

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="us-ascii", Size: 1032 bytes --]

Hi,

The kernels 2.4.28+ and 2.6.9+ with IPv4 and ATM-CLIP enabled have bugs in
the neighbour cache code. neigh_delete() and neigh_add() only work properly
if one cache table per address family exist. After ATM-CLIP installed a
second cache table for AF_INET, neigh_delete() and neigh_add() only examine
the first table (the ATM-CLIP table if IPv4 and ATM-CLIP are compiled into
the kernel). neigh_dump_info() is also affected if the neigh_dump_table()
call fails.

This bug causes "ip neigh flush dev <some ethernet device>" to run into an
infinite loop when the arp cache is already populated (debian bug #282492).
"ip neigh delete ..." commands for non ATM-CLIP entries fail with an EINVAL.
This is because of the IPv4 neighbour table that is installed in arp.c is
never examined when sending delete commands.

I have attached a minimally tested patch for 2.4.28 that fixes this problem.

Greetings,
Wilfried

-- 
10 GB Mailbox, 100 FreeSMS http://www.gmx.net/de/go/topmail
+++ GMX - die erste Adresse für Mail, Message, More +++

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: neighbour.patch --]
[-- Type: text/x-diff; name="neighbour.patch", Size: 3427 bytes --]

--- linux-2.4.28/net/core/neighbour.c	Fri Jan 28 14:26:59 2005
+++ linux-2.4.28fix/net/core/neighbour.c	Fri Jan 28 13:22:06 2005
@@ -1351,7 +1351,6 @@ int neigh_delete(struct sk_buff *skb, st
 
 		if (tbl->family != ndm->ndm_family)
 			continue;
-		read_unlock(&neigh_tbl_lock);
 
 		err = -EINVAL;
 		if (nda[NDA_DST-1] == NULL ||
@@ -1360,18 +1359,28 @@ int neigh_delete(struct sk_buff *skb, st
 
 		if (ndm->ndm_flags&NTF_PROXY) {
 			err = pneigh_delete(tbl, RTA_DATA(nda[NDA_DST-1]), dev);
-			goto out;
+			if(err) {
+				continue;	/* maybe in another table */
+			}
+			else {
+				goto out;
+			}
 		}
 
-		if (dev == NULL)
-			return -EINVAL;
+		if (dev == NULL) {
+			goto out;
+		}
 
 		n = neigh_lookup(tbl, RTA_DATA(nda[NDA_DST-1]), dev);
 		if (n) {
 			err = neigh_update(n, NULL, NUD_FAILED, 1, 0);
 			neigh_release(n);
 		}
+		else {
+			continue;	/* maybe in another table */
+		}
 out:
+		read_unlock(&neigh_tbl_lock);
 		if (dev)
 			dev_put(dev);
 		return err;
@@ -1390,6 +1399,8 @@ int neigh_add(struct sk_buff *skb, struc
 	struct rtattr **nda = arg;
 	struct neigh_table *tbl;
 	struct net_device *dev = NULL;
+	int err = -EADDRNOTAVAIL;
+	int olderr;
 
 	if (ndm->ndm_ifindex) {
 		if ((dev = dev_get_by_index(ndm->ndm_ifindex)) == NULL)
@@ -1398,35 +1409,47 @@ int neigh_add(struct sk_buff *skb, struc
 
 	read_lock(&neigh_tbl_lock);
 	for (tbl=neigh_tables; tbl; tbl = tbl->next) {
-		int err = 0;
 		int override = 1;
 		struct neighbour *n;
 
 		if (tbl->family != ndm->ndm_family)
 			continue;
-		read_unlock(&neigh_tbl_lock);
 
-		err = -EINVAL;
 		if (nda[NDA_DST-1] == NULL ||
-		    nda[NDA_DST-1]->rta_len != RTA_LENGTH(tbl->key_len))
+		    nda[NDA_DST-1]->rta_len != RTA_LENGTH(tbl->key_len)) {
+			err = -EINVAL;
 			goto out;
+		}
+
 		if (ndm->ndm_flags&NTF_PROXY) {
 			err = -ENOBUFS;
-			if (pneigh_lookup(tbl, RTA_DATA(nda[NDA_DST-1]), dev, 1))
+			if (pneigh_lookup(tbl, RTA_DATA(nda[NDA_DST-1]), dev, 1)) {
 				err = 0;
+				goto out;
+			}
+			else {
+				continue;	/* maybe in another table */
+			}
+		}
+		if (dev == NULL) {
+			err = -EINVAL;
 			goto out;
 		}
-		if (dev == NULL)
-			return -EINVAL;
-		err = -EINVAL;
+
 		if (nda[NDA_LLADDR-1] != NULL &&
-		    nda[NDA_LLADDR-1]->rta_len != RTA_LENGTH(dev->addr_len))
+		    nda[NDA_LLADDR-1]->rta_len != RTA_LENGTH(dev->addr_len)) {
+			err = -EINVAL;
 			goto out;
+		}
+
+		olderr=err;
 		err = 0;
 		n = neigh_lookup(tbl, RTA_DATA(nda[NDA_DST-1]), dev);
 		if (n) {
-			if (nlh->nlmsg_flags&NLM_F_EXCL)
+			if (nlh->nlmsg_flags&NLM_F_EXCL) {
 				err = -EEXIST;
+				goto outneigh;
+			}
 			override = nlh->nlmsg_flags&NLM_F_REPLACE;
 		} else if (!(nlh->nlmsg_flags&NLM_F_CREATE))
 			err = -ENOENT;
@@ -1442,9 +1465,16 @@ int neigh_add(struct sk_buff *skb, struc
 					   ndm->ndm_state,
 					   override, 0);
 		}
+		else {
+			err=olderr;
+			continue;	/* maybe in another table */
+		}
+
+outneigh:
 		if (n)
 			neigh_release(n);
 out:
+		read_unlock(&neigh_tbl_lock);
 		if (dev)
 			dev_put(dev);
 		return err;
@@ -1453,7 +1483,7 @@ out:
 
 	if (dev)
 		dev_put(dev);
-	return -EADDRNOTAVAIL;
+	return err;
 }
 
 
@@ -1547,8 +1577,7 @@ int neigh_dump_info(struct sk_buff *skb,
 			continue;
 		if (t > s_t)
 			memset(&cb->args[1], 0, sizeof(cb->args)-sizeof(cb->args[0]));
-		if (neigh_dump_table(tbl, skb, cb) < 0) 
-			break;
+		neigh_dump_table(tbl, skb, cb);
 	}
 	read_unlock(&neigh_tbl_lock);
 

^ permalink raw reply	[flat|nested] 4+ messages in thread