[PATCH] iproute2: Fix memory hog of ip batched command.

[PATCH] iproute2: Fix memory hog of ip batched command.

[Pravin B Shelar]

ipaddr_list_or_flush() builds list of all device at start of
every flush or list operation, but does not free memory at end.
This can hog lot of memory for large batched command.
Following patch fixes it.

Reported-by: Justin Pettit <jpettit@nicira.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
 ip/ipaddress.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 1db7fd0..3ce3706 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -775,6 +775,8 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 	struct nlmsg_list *l, *n;
 	char *filter_dev = NULL;
 	int no_link = 0;
+	int rc = 0;
+	int print_info = 0;
 
 	ipaddr_reset_filter(oneline);
 	filter.showqueue = 1;
@@ -877,7 +879,8 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 		filter.ifindex = ll_name_to_index(filter_dev);
 		if (filter.ifindex <= 0) {
 			fprintf(stderr, "Device \"%s\" does not exist.\n", filter_dev);
-			return -1;
+			rc = -1;
+			goto out;
 		}
 	}
 
@@ -922,11 +925,14 @@ flush_done:
 						printf("*** Flush is complete after %d round%s ***\n", round, round>1?"s":"");
 				}
 				fflush(stdout);
-				return 0;
+				rc = 0;
+				goto out;
 			}
 			round++;
-			if (flush_update() < 0)
-				return 1;
+			if (flush_update() < 0) {
+				rc = 1;
+				goto out;
+			}
 
 			if (show_stats) {
 				printf("\n*** Round %d, deleting %d addresses ***\n", round, filter.flushed);
@@ -943,7 +949,8 @@ flush_done:
 		}
 		fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", max_flush_loops);
 		fflush(stderr);
-		return 1;
+		rc = 1;
+		goto out;
 	}
 
 	if (filter.family != AF_PACKET) {
@@ -1018,18 +1025,21 @@ flush_done:
 		}
 	}
 
+	print_info = 1;
+out:
 	for (l = linfo.head; l; l = n) {
 		n = l->next;
-		if (no_link || print_linkinfo(NULL, &l->h, stdout) == 0) {
+		if (print_info &&
+		    (no_link || print_linkinfo(NULL, &l->h, stdout) == 0)) {
 			struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
 			if (filter.family != AF_PACKET)
 				print_selected_addrinfo(ifi->ifi_index, ainfo.head, stdout);
+			fflush(stdout);
 		}
-		fflush(stdout);
 		free(l);
 	}
 
-	return 0;
+	return rc;
 }
 
 int ipaddr_list_link(int argc, char **argv)
-- 
1.7.10

Re: [PATCH] iproute2: Fix memory hog of ip batched command.

[Stephen Hemminger]

On Thu, 12 Jul 2012 18:21:06 -0700
Pravin B Shelar <pshelar@nicira.com> wrote:

> ipaddr_list_or_flush() builds list of all device at start of
> every flush or list operation, but does not free memory at end.
> This can hog lot of memory for large batched command.
> Following patch fixes it.
> 
> Reported-by: Justin Pettit <jpettit@nicira.com>
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>


Given all the conditional's and goto's this introduces, I prefer to
refactor the code to split up the large function. 


There is an additional leak here where element is pruned from list and not freed.



diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 1db7fd0..5e03d1e 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1011,9 +1011,10 @@ flush_done:
                                ok = 1;
                                break;
                        }
-                       if (!ok)
+                       if (!ok) {
                                *lp = l->next;
-                       else
+                               free(l);
+                       } else
                                lp = &l->next;
                }
        }

Re: [PATCH] iproute2: Fix memory hog of ip batched command.

[Stephen Hemminger]

On Thu, 12 Jul 2012 18:21:06 -0700
Pravin B Shelar <pshelar@nicira.com> wrote:

> ipaddr_list_or_flush() builds list of all device at start of
> every flush or list operation, but does not free memory at end.
> This can hog lot of memory for large batched command.
> Following patch fixes it.
> 
> Reported-by: Justin Pettit <jpettit@nicira.com>
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>

The list of addresses (ainfo) also leaks here.

Re: [PATCH] iproute2: Fix memory hog of ip batched command.

[Stephen Hemminger]

On Thu, 12 Jul 2012 18:21:06 -0700
Pravin B Shelar <pshelar@nicira.com> wrote:

> ipaddr_list_or_flush() builds list of all device at start of
> every flush or list operation, but does not free memory at end.
> This can hog lot of memory for large batched command.
> Following patch fixes it.
> 
> Reported-by: Justin Pettit <jpettit@nicira.com>
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>

What about this instead? It also has a couple of other changes:
  1. stdout isn't flushed on each print only at end
  2. address list does not need to be fetched when doing flush

It comes up clean under valgrind.

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 5e03d1e..8870ae8 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -768,11 +768,145 @@ static int store_nlmsg(const struct sockaddr_nl *who, struct nlmsghdr *n,
 	return 0;
 }
 
+static void free_nlmsg_chain(struct nlmsg_chain *info)
+{
+	struct nlmsg_list *l, *n;
+
+	for (l = info->head; l; l = n) {
+		n = l->next;
+		free(l);
+	}
+}
+
+static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
+{
+	struct nlmsg_list *l, **lp;
+
+	lp = &linfo->head;
+	while ( (l = *lp) != NULL) {
+		int ok = 0;
+		struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
+		struct nlmsg_list *a;
+
+		for (a = ainfo->head; a; a = a->next) {
+			struct nlmsghdr *n = &a->h;
+			struct ifaddrmsg *ifa = NLMSG_DATA(n);
+
+			if (ifa->ifa_index != ifi->ifi_index ||
+			    (filter.family && filter.family != ifa->ifa_family))
+				continue;
+			if ((filter.scope^ifa->ifa_scope)&filter.scopemask)
+				continue;
+			if ((filter.flags^ifa->ifa_flags)&filter.flagmask)
+				continue;
+			if (filter.pfx.family || filter.label) {
+				struct rtattr *tb[IFA_MAX+1];
+				parse_rtattr(tb, IFA_MAX, IFA_RTA(ifa), IFA_PAYLOAD(n));
+				if (!tb[IFA_LOCAL])
+					tb[IFA_LOCAL] = tb[IFA_ADDRESS];
+
+				if (filter.pfx.family && tb[IFA_LOCAL]) {
+					inet_prefix dst;
+					memset(&dst, 0, sizeof(dst));
+					dst.family = ifa->ifa_family;
+					memcpy(&dst.data, RTA_DATA(tb[IFA_LOCAL]), RTA_PAYLOAD(tb[IFA_LOCAL]));
+					if (inet_addr_match(&dst, &filter.pfx, filter.pfx.bitlen))
+						continue;
+				}
+				if (filter.label) {
+					SPRINT_BUF(b1);
+					const char *label;
+					if (tb[IFA_LABEL])
+						label = RTA_DATA(tb[IFA_LABEL]);
+					else
+						label = ll_idx_n2a(ifa->ifa_index, b1);
+					if (fnmatch(filter.label, label, 0) != 0)
+						continue;
+				}
+			}
+
+			ok = 1;
+			break;
+		}
+		if (!ok) {
+			*lp = l->next;
+			free(l);
+		} else
+			lp = &l->next;
+	}
+}
+
+static int ipaddr_flush(void)
+{
+	int round = 0;
+	char flushb[4096-512];
+
+	filter.flushb = flushb;
+	filter.flushp = 0;
+	filter.flushe = sizeof(flushb);
+
+	while ((max_flush_loops == 0) || (round < max_flush_loops)) {
+		const struct rtnl_dump_filter_arg a[3] = {
+			{
+				.filter = print_addrinfo_secondary,
+				.arg1 = stdout,
+			},
+			{
+				.filter = print_addrinfo_primary,
+				.arg1 = stdout,
+			},
+			{
+				.filter = NULL,
+				.arg1 = NULL,
+			},
+		};
+		if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
+			perror("Cannot send dump request");
+			exit(1);
+		}
+		filter.flushed = 0;
+		if (rtnl_dump_filter_l(&rth, a) < 0) {
+			fprintf(stderr, "Flush terminated\n");
+			exit(1);
+		}
+		if (filter.flushed == 0) {
+ flush_done:
+			if (show_stats) {
+				if (round == 0)
+					printf("Nothing to flush.\n");
+				else
+					printf("*** Flush is complete after %d round%s ***\n", round, round>1?"s":"");
+			}
+			fflush(stdout);
+			return 0;
+		}
+		round++;
+		if (flush_update() < 0)
+			return 1;
+
+		if (show_stats) {
+			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))
+			goto flush_done;
+	}
+	fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", max_flush_loops);
+	fflush(stderr);
+	return 1;
+}
+
 static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 {
 	struct nlmsg_chain linfo = { NULL, NULL};
 	struct nlmsg_chain ainfo = { NULL, NULL};
-	struct nlmsg_list *l, *n;
+	struct nlmsg_list *l;
 	char *filter_dev = NULL;
 	int no_link = 0;
 
@@ -863,16 +997,6 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 		argv++; argc--;
 	}
 
-	if (rtnl_wilddump_request(&rth, preferred_family, RTM_GETLINK) < 0) {
-		perror("Cannot send dump request");
-		exit(1);
-	}
-
-	if (rtnl_dump_filter(&rth, store_nlmsg, &linfo) < 0) {
-		fprintf(stderr, "Dump terminated\n");
-		exit(1);
-	}
-
 	if (filter_dev) {
 		filter.ifindex = ll_name_to_index(filter_dev);
 		if (filter.ifindex <= 0) {
@@ -881,72 +1005,23 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 		}
 	}
 
-	if (flush) {
-		int round = 0;
-		char flushb[4096-512];
-
-		filter.flushb = flushb;
-		filter.flushp = 0;
-		filter.flushe = sizeof(flushb);
-
-		while ((max_flush_loops == 0) || (round < max_flush_loops)) {
-			const struct rtnl_dump_filter_arg a[3] = {
-				{
-					.filter = print_addrinfo_secondary,
-					.arg1 = stdout,
-				},
-				{
-					.filter = print_addrinfo_primary,
-					.arg1 = stdout,
-				},
-				{
-					.filter = NULL,
-					.arg1 = NULL,
-				},
-			};
-			if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
-				perror("Cannot send dump request");
-				exit(1);
-			}
-			filter.flushed = 0;
-			if (rtnl_dump_filter_l(&rth, a) < 0) {
-				fprintf(stderr, "Flush terminated\n");
-				exit(1);
-			}
-			if (filter.flushed == 0) {
-flush_done:
-				if (show_stats) {
-					if (round == 0)
-						printf("Nothing to flush.\n");
-					else 
-						printf("*** Flush is complete after %d round%s ***\n", round, round>1?"s":"");
-				}
-				fflush(stdout);
-				return 0;
-			}
-			round++;
-			if (flush_update() < 0)
-				return 1;
+	if (flush)
+		return ipaddr_flush();
 
-			if (show_stats) {
-				printf("\n*** Round %d, deleting %d addresses ***\n", round, filter.flushed);
-				fflush(stdout);
-			}
+	if (rtnl_wilddump_request(&rth, preferred_family, RTM_GETLINK) < 0) {
+		perror("Cannot send dump request");
+		exit(1);
+	}
 
-			/* 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))
-				goto flush_done;
-		}
-		fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", max_flush_loops);
-		fflush(stderr);
-		return 1;
+	if (rtnl_dump_filter(&rth, store_nlmsg, &linfo) < 0) {
+		fprintf(stderr, "Dump terminated\n");
+		exit(1);
 	}
 
-	if (filter.family != AF_PACKET) {
+	if (filter.family && filter.family != AF_PACKET) {
+		if (filter.oneline)
+			no_link = 1;
+
 		if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
 			perror("Cannot send dump request");
 			exit(1);
@@ -956,80 +1031,24 @@ flush_done:
 			fprintf(stderr, "Dump terminated\n");
 			exit(1);
 		}
-	}
-
-
-	if (filter.family && filter.family != AF_PACKET) {
-		struct nlmsg_list **lp;
-		lp = &linfo.head;
-
-		if (filter.oneline)
-			no_link = 1;
 
-		while ((l=*lp)!=NULL) {
-			int ok = 0;
-			struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
-			struct nlmsg_list *a;
-
-			for (a = ainfo.head; a; a = a->next) {
-				struct nlmsghdr *n = &a->h;
-				struct ifaddrmsg *ifa = NLMSG_DATA(n);
-
-				if (ifa->ifa_index != ifi->ifi_index ||
-				    (filter.family && filter.family != ifa->ifa_family))
-					continue;
-				if ((filter.scope^ifa->ifa_scope)&filter.scopemask)
-					continue;
-				if ((filter.flags^ifa->ifa_flags)&filter.flagmask)
-					continue;
-				if (filter.pfx.family || filter.label) {
-					struct rtattr *tb[IFA_MAX+1];
-					parse_rtattr(tb, IFA_MAX, IFA_RTA(ifa), IFA_PAYLOAD(n));
-					if (!tb[IFA_LOCAL])
-						tb[IFA_LOCAL] = tb[IFA_ADDRESS];
-
-					if (filter.pfx.family && tb[IFA_LOCAL]) {
-						inet_prefix dst;
-						memset(&dst, 0, sizeof(dst));
-						dst.family = ifa->ifa_family;
-						memcpy(&dst.data, RTA_DATA(tb[IFA_LOCAL]), RTA_PAYLOAD(tb[IFA_LOCAL]));
-						if (inet_addr_match(&dst, &filter.pfx, filter.pfx.bitlen))
-							continue;
-					}
-					if (filter.label) {
-						SPRINT_BUF(b1);
-						const char *label;
-						if (tb[IFA_LABEL])
-							label = RTA_DATA(tb[IFA_LABEL]);
-						else
-							label = ll_idx_n2a(ifa->ifa_index, b1);
-						if (fnmatch(filter.label, label, 0) != 0)
-							continue;
-					}
-				}
-
-				ok = 1;
-				break;
-			}
-			if (!ok) {
-				*lp = l->next;
-				free(l);
-			} else
-				lp = &l->next;
-		}
+		ipaddr_filter(&linfo, &ainfo);
 	}
 
-	for (l = linfo.head; l; l = n) {
-		n = l->next;
-		if (no_link || print_linkinfo(NULL, &l->h, stdout) == 0) {
-			struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
-			if (filter.family != AF_PACKET)
-				print_selected_addrinfo(ifi->ifi_index, ainfo.head, stdout);
+	if (!no_link) {
+		for (l = linfo.head; l; l = l->next) {
+			if (print_linkinfo(NULL, &l->h, stdout) == 0) {
+				struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
+				if (filter.family != AF_PACKET)
+					print_selected_addrinfo(ifi->ifi_index, ainfo.head, stdout);
+			}
 		}
 		fflush(stdout);
-		free(l);
 	}
 
+	free_nlmsg_chain(&ainfo);
+	free_nlmsg_chain(&linfo);
+
 	return 0;
 }
 

Re: [PATCH] iproute2: Fix memory hog of ip batched command.

[Pravin Shelar]

On Fri, Jul 13, 2012 at 9:53 AM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Thu, 12 Jul 2012 18:21:06 -0700
> Pravin B Shelar <pshelar@nicira.com> wrote:
>
>> ipaddr_list_or_flush() builds list of all device at start of
>> every flush or list operation, but does not free memory at end.
>> This can hog lot of memory for large batched command.
>> Following patch fixes it.
>>
>> Reported-by: Justin Pettit <jpettit@nicira.com>
>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
>
> What about this instead? It also has a couple of other changes:
>   1. stdout isn't flushed on each print only at end
>   2. address list does not need to be fetched when doing flush
>
sounds good.

> It comes up clean under valgrind.
>
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 5e03d1e..8870ae8 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -768,11 +768,145 @@ static int store_nlmsg(const struct sockaddr_nl *who, struct nlmsghdr *n,
>         return 0;
>  }
>
> +static void free_nlmsg_chain(struct nlmsg_chain *info)
> +{
> +       struct nlmsg_list *l, *n;
> +
> +       for (l = info->head; l; l = n) {
> +               n = l->next;
> +               free(l);
> +       }
> +}
> +
> +static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
> +{
> +       struct nlmsg_list *l, **lp;
> +
> +       lp = &linfo->head;
> +       while ( (l = *lp) != NULL) {
> +               int ok = 0;
> +               struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
> +               struct nlmsg_list *a;
> +
> +               for (a = ainfo->head; a; a = a->next) {
> +                       struct nlmsghdr *n = &a->h;
> +                       struct ifaddrmsg *ifa = NLMSG_DATA(n);
> +
> +                       if (ifa->ifa_index != ifi->ifi_index ||
> +                           (filter.family && filter.family != ifa->ifa_family))
> +                               continue;
> +                       if ((filter.scope^ifa->ifa_scope)&filter.scopemask)
> +                               continue;
> +                       if ((filter.flags^ifa->ifa_flags)&filter.flagmask)
> +                               continue;
> +                       if (filter.pfx.family || filter.label) {
> +                               struct rtattr *tb[IFA_MAX+1];
> +                               parse_rtattr(tb, IFA_MAX, IFA_RTA(ifa), IFA_PAYLOAD(n));
> +                               if (!tb[IFA_LOCAL])
> +                                       tb[IFA_LOCAL] = tb[IFA_ADDRESS];
> +
> +                               if (filter.pfx.family && tb[IFA_LOCAL]) {
> +                                       inet_prefix dst;
> +                                       memset(&dst, 0, sizeof(dst));
> +                                       dst.family = ifa->ifa_family;
> +                                       memcpy(&dst.data, RTA_DATA(tb[IFA_LOCAL]), RTA_PAYLOAD(tb[IFA_LOCAL]));
> +                                       if (inet_addr_match(&dst, &filter.pfx, filter.pfx.bitlen))
> +                                               continue;
> +                               }
> +                               if (filter.label) {
> +                                       SPRINT_BUF(b1);
> +                                       const char *label;
> +                                       if (tb[IFA_LABEL])
> +                                               label = RTA_DATA(tb[IFA_LABEL]);
> +                                       else
> +                                               label = ll_idx_n2a(ifa->ifa_index, b1);
> +                                       if (fnmatch(filter.label, label, 0) != 0)
> +                                               continue;
> +                               }
> +                       }
> +
> +                       ok = 1;
> +                       break;
> +               }
> +               if (!ok) {
> +                       *lp = l->next;
> +                       free(l);
> +               } else
> +                       lp = &l->next;
> +       }
> +}
> +
> +static int ipaddr_flush(void)
> +{
> +       int round = 0;
> +       char flushb[4096-512];
> +
> +       filter.flushb = flushb;
> +       filter.flushp = 0;
> +       filter.flushe = sizeof(flushb);
> +
> +       while ((max_flush_loops == 0) || (round < max_flush_loops)) {
> +               const struct rtnl_dump_filter_arg a[3] = {
> +                       {
> +                               .filter = print_addrinfo_secondary,
> +                               .arg1 = stdout,
> +                       },
> +                       {
> +                               .filter = print_addrinfo_primary,
> +                               .arg1 = stdout,
> +                       },
> +                       {
> +                               .filter = NULL,
> +                               .arg1 = NULL,
> +                       },
> +               };
> +               if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
> +                       perror("Cannot send dump request");
> +                       exit(1);
> +               }
> +               filter.flushed = 0;
> +               if (rtnl_dump_filter_l(&rth, a) < 0) {
> +                       fprintf(stderr, "Flush terminated\n");
> +                       exit(1);
> +               }
> +               if (filter.flushed == 0) {
> + flush_done:
> +                       if (show_stats) {
> +                               if (round == 0)
> +                                       printf("Nothing to flush.\n");
> +                               else
> +                                       printf("*** Flush is complete after %d round%s ***\n", round, round>1?"s":"");
> +                       }
> +                       fflush(stdout);
> +                       return 0;
> +               }
> +               round++;
> +               if (flush_update() < 0)
> +                       return 1;
> +
> +               if (show_stats) {
> +                       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))
> +                       goto flush_done;
> +       }
> +       fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", max_flush_loops);
> +       fflush(stderr);
> +       return 1;
> +}
> +
>  static int ipaddr_list_or_flush(int argc, char **argv, int flush)
>  {
>         struct nlmsg_chain linfo = { NULL, NULL};
>         struct nlmsg_chain ainfo = { NULL, NULL};
> -       struct nlmsg_list *l, *n;
> +       struct nlmsg_list *l;
>         char *filter_dev = NULL;
>         int no_link = 0;
>
> @@ -863,16 +997,6 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
>                 argv++; argc--;
>         }
>
> -       if (rtnl_wilddump_request(&rth, preferred_family, RTM_GETLINK) < 0) {
> -               perror("Cannot send dump request");
> -               exit(1);
> -       }
> -
> -       if (rtnl_dump_filter(&rth, store_nlmsg, &linfo) < 0) {
> -               fprintf(stderr, "Dump terminated\n");
> -               exit(1);
> -       }
> -
>         if (filter_dev) {
>                 filter.ifindex = ll_name_to_index(filter_dev);
>                 if (filter.ifindex <= 0) {
> @@ -881,72 +1005,23 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
>                 }
>         }
>
> -       if (flush) {
> -               int round = 0;
> -               char flushb[4096-512];
> -
> -               filter.flushb = flushb;
> -               filter.flushp = 0;
> -               filter.flushe = sizeof(flushb);
> -
> -               while ((max_flush_loops == 0) || (round < max_flush_loops)) {
> -                       const struct rtnl_dump_filter_arg a[3] = {
> -                               {
> -                                       .filter = print_addrinfo_secondary,
> -                                       .arg1 = stdout,
> -                               },
> -                               {
> -                                       .filter = print_addrinfo_primary,
> -                                       .arg1 = stdout,
> -                               },
> -                               {
> -                                       .filter = NULL,
> -                                       .arg1 = NULL,
> -                               },
> -                       };
> -                       if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
> -                               perror("Cannot send dump request");
> -                               exit(1);
> -                       }
> -                       filter.flushed = 0;
> -                       if (rtnl_dump_filter_l(&rth, a) < 0) {
> -                               fprintf(stderr, "Flush terminated\n");
> -                               exit(1);
> -                       }
> -                       if (filter.flushed == 0) {
> -flush_done:
> -                               if (show_stats) {
> -                                       if (round == 0)
> -                                               printf("Nothing to flush.\n");
> -                                       else
> -                                               printf("*** Flush is complete after %d round%s ***\n", round, round>1?"s":"");
> -                               }
> -                               fflush(stdout);
> -                               return 0;
> -                       }
> -                       round++;
> -                       if (flush_update() < 0)
> -                               return 1;
> +       if (flush)
> +               return ipaddr_flush();
>
> -                       if (show_stats) {
> -                               printf("\n*** Round %d, deleting %d addresses ***\n", round, filter.flushed);
> -                               fflush(stdout);
> -                       }
> +       if (rtnl_wilddump_request(&rth, preferred_family, RTM_GETLINK) < 0) {
> +               perror("Cannot send dump request");
> +               exit(1);
> +       }
>
> -                       /* 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))
> -                               goto flush_done;
> -               }
> -               fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", max_flush_loops);
> -               fflush(stderr);
> -               return 1;
> +       if (rtnl_dump_filter(&rth, store_nlmsg, &linfo) < 0) {
> +               fprintf(stderr, "Dump terminated\n");
> +               exit(1);
>         }
>
> -       if (filter.family != AF_PACKET) {
> +       if (filter.family && filter.family != AF_PACKET) {
> +               if (filter.oneline)
> +                       no_link = 1;
> +
>                 if (rtnl_wilddump_request(&rth, filter.family, RTM_GETADDR) < 0) {
>                         perror("Cannot send dump request");
>                         exit(1);
> @@ -956,80 +1031,24 @@ flush_done:
>                         fprintf(stderr, "Dump terminated\n");
>                         exit(1);
>                 }
> -       }
> -
> -
> -       if (filter.family && filter.family != AF_PACKET) {
> -               struct nlmsg_list **lp;
> -               lp = &linfo.head;
> -
> -               if (filter.oneline)
> -                       no_link = 1;
>
> -               while ((l=*lp)!=NULL) {
> -                       int ok = 0;
> -                       struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
> -                       struct nlmsg_list *a;
> -
> -                       for (a = ainfo.head; a; a = a->next) {
> -                               struct nlmsghdr *n = &a->h;
> -                               struct ifaddrmsg *ifa = NLMSG_DATA(n);
> -
> -                               if (ifa->ifa_index != ifi->ifi_index ||
> -                                   (filter.family && filter.family != ifa->ifa_family))
> -                                       continue;
> -                               if ((filter.scope^ifa->ifa_scope)&filter.scopemask)
> -                                       continue;
> -                               if ((filter.flags^ifa->ifa_flags)&filter.flagmask)
> -                                       continue;
> -                               if (filter.pfx.family || filter.label) {
> -                                       struct rtattr *tb[IFA_MAX+1];
> -                                       parse_rtattr(tb, IFA_MAX, IFA_RTA(ifa), IFA_PAYLOAD(n));
> -                                       if (!tb[IFA_LOCAL])
> -                                               tb[IFA_LOCAL] = tb[IFA_ADDRESS];
> -
> -                                       if (filter.pfx.family && tb[IFA_LOCAL]) {
> -                                               inet_prefix dst;
> -                                               memset(&dst, 0, sizeof(dst));
> -                                               dst.family = ifa->ifa_family;
> -                                               memcpy(&dst.data, RTA_DATA(tb[IFA_LOCAL]), RTA_PAYLOAD(tb[IFA_LOCAL]));
> -                                               if (inet_addr_match(&dst, &filter.pfx, filter.pfx.bitlen))
> -                                                       continue;
> -                                       }
> -                                       if (filter.label) {
> -                                               SPRINT_BUF(b1);
> -                                               const char *label;
> -                                               if (tb[IFA_LABEL])
> -                                                       label = RTA_DATA(tb[IFA_LABEL]);
> -                                               else
> -                                                       label = ll_idx_n2a(ifa->ifa_index, b1);
> -                                               if (fnmatch(filter.label, label, 0) != 0)
> -                                                       continue;
> -                                       }
> -                               }
> -
> -                               ok = 1;
> -                               break;
> -                       }
> -                       if (!ok) {
> -                               *lp = l->next;
> -                               free(l);
> -                       } else
> -                               lp = &l->next;
> -               }
> +               ipaddr_filter(&linfo, &ainfo);
>         }
>
> -       for (l = linfo.head; l; l = n) {
> -               n = l->next;
> -               if (no_link || print_linkinfo(NULL, &l->h, stdout) == 0) {
> -                       struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
> -                       if (filter.family != AF_PACKET)
> -                               print_selected_addrinfo(ifi->ifi_index, ainfo.head, stdout);
> +       if (!no_link) {
> +               for (l = linfo.head; l; l = l->next) {
> +                       if (print_linkinfo(NULL, &l->h, stdout) == 0) {
> +                               struct ifinfomsg *ifi = NLMSG_DATA(&l->h);
> +                               if (filter.family != AF_PACKET)
> +                                       print_selected_addrinfo(ifi->ifi_index, ainfo.head, stdout);
> +                       }
>                 }
I am not sure why you have changed no_link check for printing address.

otherwise looks good.

>                 fflush(stdout);
> -               free(l);
>         }
>
> +       free_nlmsg_chain(&ainfo);
> +       free_nlmsg_chain(&linfo);
> +
>         return 0;
>  }
>
>