Skip to contentSkip to conversation

: Exclude "All deletion of equal addresses only differing by prefix length."

PATCH In review Thomas Graf 3 participants 5 messages Mar 31, 2005

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 : Exclude "All deletion of equal addresses only differing by prefix length."

The address deletion fix is no longer needed since a better fix has made it into iproute2. Better in terms of same hack but in userspace so we don't confuse other netlink users.

Please do a

bk pull bk://kernel.bkbits.net/tgraf/net-2.6-trunk

This will update the following files:

 include/linux/inetdevice.h |   20 --------------------
 include/linux/rtnetlink.h  |   13 -------------
 net/ipv4/devinet.c         |    6 ++----
 3 files changed, 2 insertions(+), 37 deletions(-)

through these ChangeSets:

<[email protected]> (05/03/31 1.2199)

   Cset exclude: [email protected]|ChangeSet|20050316221421|24742

# This is a BitKeeper generated diff -Nru style patch. #
# ChangeSet
# 2005/03/31 14:34:31+02:00 [email protected]
# Cset exclude: [email protected]|ChangeSet|20050316221421|24742
#

# net/ipv4/devinet.c
#   2005/03/31 14:34:23+02:00 [email protected] +0 -0
#   Exclude
# 
# include/linux/rtnetlink.h
#   2005/03/31 14:34:23+02:00 [email protected] +0 -0
#   Exclude
# 
# include/linux/inetdevice.h
#   2005/03/31 14:34:23+02:00 [email protected] +0 -0
#   Exclude
# 
diff -Nru a/include/linux/inetdevice.h b/include/linux/inetdevice.h
--- a/include/linux/inetdevice.h	2005-03-31 14:35:28 +02:00
+++ b/include/linux/inetdevice.h	2005-03-31 14:35:28 +02:00
@@ -7,7 +7,6 @@
 #include <linux/netdevice.h>
 #include <linux/rcupdate.h>
 #include <linux/timer.h>
-#include <linux/rtnetlink.h>

struct ipv4_devconf
{
@@ -130,25 +129,6 @@
if (mask & (mask+1))
return 1;
return 0;
-}
-
-static inline int inet_ifa_match_local_prefixlen(struct ifaddrmsg *ifm,
- struct in_ifaddr *ifa)
-{
- int real_prefixlen = IFA_REAL_DEL_PREFIX(ifm->ifa_prefixlen);
-
- /*
- * Since the prefix length hasn't been taken into account in
- * previous kernel versions, parts of the userspace rely on the fact
- * that the deletion of an address without specifying a prefix works.
- * We cannot break this and thus a prefix length of 32 still represents
- * a wildcard if no exact match is requested.
- */
- if (real_prefixlen != 32 || ifm->ifa_prefixlen & IFA_PREFIX_EXACT_DEL)
- if (real_prefixlen != ifa->ifa_prefixlen)
- return 0;
-
- return 1;
}

#define for_primary_ifa(in_dev) { struct in_ifaddr *ifa; \

include/linux/rtnetlink.h +0 -13
diff -Nru a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
@@ -398,19 +398,6 @@
 
 #define IFA_MAX (__IFA_MAX - 1)
 
-/*
- * Quirk for IPv4 address deletion to allow exact deletion of equal
- * addresses varying only in prefix length. A explicit exact comparison
- * of the prefix length will only be done if IFA_PREFIX_EXACT_DEL is
- * ORed to ifa_prefixlen.
- *
- * Note: This special treatment is only understood while deleting
- *       addresses and will lead to unexpected behaviour if used
- *       otherwise.
- */
-#define IFA_PREFIX_EXACT_DEL	0x40
-#define IFA_REAL_DEL_PREFIX(l)	((l) & 0x3f)
-
 /* ifa_flags */
 
 #define IFA_F_SECONDARY		0x01
net/ipv4/devinet.c +2 -4
diff -Nru a/net/ipv4/devinet.c b/net/ipv4/devinet.c
@@ -389,7 +389,6 @@
 	struct in_device *in_dev;
 	struct ifaddrmsg *ifm = NLMSG_DATA(nlh);
 	struct in_ifaddr *ifa, **ifap;
-	int real_prefixlen = IFA_REAL_DEL_PREFIX(ifm->ifa_prefixlen);
 
 	ASSERT_RTNL();
 
@@ -400,13 +399,12 @@
 	for (ifap = &in_dev->ifa_list; (ifa = *ifap) != NULL;
 	     ifap = &ifa->ifa_next) {
 		if ((rta[IFA_LOCAL - 1] &&
-		    (!inet_ifa_match_local_prefixlen(ifm, ifa) ||
 		     memcmp(RTA_DATA(rta[IFA_LOCAL - 1]),
-			    &ifa->ifa_local, 4))) ||
+			    &ifa->ifa_local, 4)) ||
 		    (rta[IFA_LABEL - 1] &&
 		     rtattr_strcmp(rta[IFA_LABEL - 1], ifa->ifa_label)) ||
 		    (rta[IFA_ADDRESS - 1] &&
-		     (real_prefixlen != ifa->ifa_prefixlen ||
+		     (ifm->ifa_prefixlen != ifa->ifa_prefixlen ||
 		      !inet_ifa_match(*(u32*)RTA_DATA(rta[IFA_ADDRESS - 1]),
 			      	      ifa))))
 			continue;
#1 lore raw
Quoted 3 lines · Thomas Graf

The address deletion fix is no longer needed since a better fix has made it into iproute2. Better in terms of same hack but in userspace so we don't confuse other netlink users.

So, does that mean that all the users are expected to always run the latest iproute2, so there's no need for backward-compat in the kernel?

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
#2 lore raw

* Pekka Savola <[email protected]> 2005-03-31 20:48

Quoted 7 lines · Thomas Graf
Quoted 3 lines

The address deletion fix is no longer needed since a better fix has made it into iproute2. Better in terms of same hack but in userspace so we don't confuse other netlink users.

So, does that mean that all the users are expected to always run the latest iproute2, so there's no need for backward-compat in the kernel?

The actual challenge in this rather trivial problem is that many scripts rely on this wrong behaviour by doing ip a a 1.1.1.1/24 dev lo; ip a d 1.1.1.1 dev lo; and nobody wants to break them. So even with the kernel fix we had, old iproute2 versions still did it wrong but it gave newer iproute2 versions a chance to work around the issue.

maintainer
#3 lore raw
In reply to Pekka Savola
Quoted 7 lines · On Thu, 31 Mar 2005 20:48:05 +0300 (EEST) Pekka Savola <[email protected]>…
Quoted 3 lines · Thomas Graf

The address deletion fix is no longer needed since a better fix has made it into iproute2. Better in terms of same hack but in userspace so we don't confuse other netlink users.

So, does that mean that all the users are expected to always run the latest iproute2, so there's no need for backward-compat in the kernel?

No, that's not what is happening here.

Thomas originally added a hack to the API in the kernel to handle this, but now he's backing that out because the problem can be solved purely in userspace.

This API change is only visible during 2.6.12 development and never appeared in a stable release, so we can pull it out now safely.

maintainer
#4 lore raw
In reply to Thomas Graf
Quoted 7 lines · On Thu, 31 Mar 2005 14:47:47 +0200 Thomas Graf <[email protected]> wrote:

The address deletion fix is no longer needed since a better fix has made it into iproute2. Better in terms of same hack but in userspace so we don't confuse other netlink users.

Please do a

bk pull bk://kernel.bkbits.net/tgraf/net-2.6-trunk

Done, thanks Thomas.