* [PATCH] iproute2: Fix memory hog of ip batched command.
@ 2012-07-13 1:21 Pravin B Shelar
2012-07-13 16:10 ` Stephen Hemminger
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Pravin B Shelar @ 2012-07-13 1:21 UTC (permalink / raw)
To: shemminger, netdev; +Cc: jpettit, jesse, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iproute2: Fix memory hog of ip batched command.
2012-07-13 1:21 [PATCH] iproute2: Fix memory hog of ip batched command Pravin B Shelar
@ 2012-07-13 16:10 ` Stephen Hemminger
2012-07-13 16:26 ` Stephen Hemminger
2012-07-13 16:53 ` Stephen Hemminger
2 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2012-07-13 16:10 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: netdev, jpettit, jesse
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;
}
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iproute2: Fix memory hog of ip batched command.
2012-07-13 1:21 [PATCH] iproute2: Fix memory hog of ip batched command Pravin B Shelar
2012-07-13 16:10 ` Stephen Hemminger
@ 2012-07-13 16:26 ` Stephen Hemminger
2012-07-13 16:53 ` Stephen Hemminger
2 siblings, 0 replies; 5+ messages in thread
From: Stephen Hemminger @ 2012-07-13 16:26 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: netdev, jpettit, jesse
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iproute2: Fix memory hog of ip batched command.
2012-07-13 1:21 [PATCH] iproute2: Fix memory hog of ip batched command Pravin B Shelar
2012-07-13 16:10 ` Stephen Hemminger
2012-07-13 16:26 ` Stephen Hemminger
@ 2012-07-13 16:53 ` Stephen Hemminger
2012-07-13 19:42 ` Pravin Shelar
2 siblings, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2012-07-13 16:53 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: netdev, jpettit, jesse
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;
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iproute2: Fix memory hog of ip batched command.
2012-07-13 16:53 ` Stephen Hemminger
@ 2012-07-13 19:42 ` Pravin Shelar
0 siblings, 0 replies; 5+ messages in thread
From: Pravin Shelar @ 2012-07-13 19:42 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, jpettit, jesse
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;
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-07-13 19:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-13 1:21 [PATCH] iproute2: Fix memory hog of ip batched command Pravin B Shelar
2012-07-13 16:10 ` Stephen Hemminger
2012-07-13 16:26 ` Stephen Hemminger
2012-07-13 16:53 ` Stephen Hemminger
2012-07-13 19:42 ` Pravin Shelar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.