Re: [iproute2] iproute2: Fix 'addr flush secondary' logic.

[Ben Greear <greearb@candelatech.com>]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

On 08/11/2010 01:41 PM, Brian Haley wrote:
> On 08/11/2010 01:19 PM, Ben Greear wrote:
>> @@ -648,7 +648,7 @@ int print_addrinfo_secondary(const struct sockaddr_nl *who, struct nlmsghdr *n,
>>   {
>>   	struct ifaddrmsg *ifa = NLMSG_DATA(n);
>>
>> -	if (ifa->ifa_flags&  IFA_F_SECONDARY)
>> +	if (!ifa->ifa_flags&  IFA_F_SECONDARY)
>>   		return 0;
>
> Shouldn't this be:
>
> 	if (!(ifa->ifa_flags&  IFA_F_SECONDARY))
>
> -Brian

Looks like the code was broken in several different places.

* It ran only a single filter if there were multiple.
* Don't want to flush in a loop if you are doing primary
   because otherwise promoted seconaries will get deleted
   for each additional loop (10 in upstream code).
* No idea what a while (0); statement at the end of a for
   loop does, but I don't think it needed to be there!

The attached patch makes it work for me, supporting
flushing primary or secondary addresses.

It could certainly use some review, however.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


[-- Attachment #2: iproute_flush.patch --]
[-- Type: text/plain, Size: 2813 bytes --]

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 5f0789c..803df17 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -637,7 +637,7 @@ int print_addrinfo_primary(const struct sockaddr_nl *who, struct nlmsghdr *n,
 {
 	struct ifaddrmsg *ifa = NLMSG_DATA(n);
 
-	if (!ifa->ifa_flags & IFA_F_SECONDARY)
+	if (ifa->ifa_flags & IFA_F_SECONDARY)
 		return 0;
 
 	return print_addrinfo(who, n, arg);
@@ -648,7 +648,7 @@ int print_addrinfo_secondary(const struct sockaddr_nl *who, struct nlmsghdr *n,
 {
 	struct ifaddrmsg *ifa = NLMSG_DATA(n);
 
-	if (ifa->ifa_flags & IFA_F_SECONDARY)
+	if (!(ifa->ifa_flags & IFA_F_SECONDARY))
 		return 0;
 
 	return print_addrinfo(who, n, arg);
@@ -865,6 +865,13 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 				printf("\n*** Round %d, deleting %d addresses ***\n", round, filter.flushed);
 				fflush(stdout);
 			}
+
+			/* If we are flushing, and specifying primary, then we want to flush only a single round.
+			 * Otherwise, we'll start flushing secondaries that were promoted to primaries
+			 */
+			if (!(filter.flags & IFA_F_SECONDARY) && (filter.flagmask & IFA_F_SECONDARY)) {
+				return 0;
+			}
 		}
 		fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", max_flush_loops);
 		fflush(stderr);
diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index cfeb894..d18e8a0 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -189,6 +189,8 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
 	while (1) {
 		int status;
 		const struct rtnl_dump_filter_arg *a;
+		int found_done = 0;
+		int msglen = 0;
 
 		iov.iov_len = sizeof(buf);
 		status = recvmsg(rth->fd, &msg, 0);
@@ -208,8 +210,9 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
 
 		for (a = arg; a->filter; a++) {
 			struct nlmsghdr *h = (struct nlmsghdr*)buf;
+			msglen = status;
 
-			while (NLMSG_OK(h, status)) {
+			while (NLMSG_OK(h, msglen)) {
 				int err;
 
 				if (nladdr.nl_pid != 0 ||
@@ -224,8 +227,10 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
 					goto skip_it;
 				}
 
-				if (h->nlmsg_type == NLMSG_DONE)
-					return 0;
+				if (h->nlmsg_type == NLMSG_DONE) {
+					found_done = 1;
+					break; /* process next filter */
+				}
 				if (h->nlmsg_type == NLMSG_ERROR) {
 					struct nlmsgerr *err = (struct nlmsgerr*)NLMSG_DATA(h);
 					if (h->nlmsg_len < NLMSG_LENGTH(sizeof(struct nlmsgerr))) {
@@ -242,14 +247,18 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
 					return err;
 
 skip_it:
-				h = NLMSG_NEXT(h, status);
+				h = NLMSG_NEXT(h, msglen);
 			}
-		} while (0);
+		}
+
+		if (found_done)
+			return 0;
+
 		if (msg.msg_flags & MSG_TRUNC) {
 			fprintf(stderr, "Message truncated\n");
 			continue;
 		}
-		if (status) {
+		if (msglen) {
 			fprintf(stderr, "!!!Remnant of size %d\n", status);
 			exit(1);
 		}