On Mon, Sep 19, 2016 at 09:46:03AM +0100, Justin Cattle wrote:
> Hi Pavel,
> 
> 
> After running with this latest fixup commit for a week, I see mixed results.
> 
> With the first fix you created, all the processes remained using a very
> small amount of memory, consistently.  As per my previous email, around
> 80Mg.
> With the second fix, some of the bird processes are using up to about
> 600Mg, but some are still using more like the 80Mg from the first fix.
> And, there is a mixture in between those two extremes.
> 
> So my question is  - is this normal and expected now, of is there a
> potential issue with the second fix?

Hi

We found that there was one minor leak that was overlooked in the second fix.

You can try attached patch v3.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."
commit a290da25a16b7c79d4a7a87f522b4068bca04979
Author: Pavel Tvrdik <pawel.tvr...@gmail.com>
Date:   Tue Sep 6 17:08:45 2016 +0200

    rt-table: Fix kernel protocol export filter memory bug
    
    Kernel protocol calls rt_export_merged(), which used @rte_update_pool for
    temporary allocations, supposing it is called from other functions from
    rt-table.c that handles locking and flushing of the linpool. Therefore,
    linpool was not flushed properly and memory leaked.
    
    Add linpool argument to rt_export_merged() and use @krt_filter_lp when
    called from kernel protocol.
    
    Thanks to Justin Cattle and Alexander Frolkin for the bugreport.
    
    (Commit squashed and updated by Ondrej Zajicek)

diff --git a/nest/route.h b/nest/route.h
index bb4674b..2fcb189 100644
--- a/nest/route.h
+++ b/nest/route.h
@@ -276,7 +276,7 @@ void rte_update2(struct announce_hook *ah, net *net, rte *new, struct rte_src *s
 static inline void rte_update(struct proto *p, net *net, rte *new) { rte_update2(p->main_ahook, net, new, p->main_source); }
 void rte_discard(rtable *tab, rte *old);
 int rt_examine(rtable *t, ip_addr prefix, int pxlen, struct proto *p, struct filter *filter);
-rte *rt_export_merged(struct announce_hook *ah, net *net, rte **rt_free, struct ea_list **tmpa, int silent);
+rte *rt_export_merged(struct announce_hook *ah, net *net, rte **rt_free, struct ea_list **tmpa, linpool *pool, int silent);
 void rt_refresh_begin(rtable *t, struct announce_hook *ah);
 void rt_refresh_end(rtable *t, struct announce_hook *ah);
 void rte_dump(rte *);
diff --git a/nest/rt-table.c b/nest/rt-table.c
index f4df22a..d3aba08 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -245,7 +245,7 @@ rte_trace_out(uint flag, struct proto *p, rte *e, char *msg)
 }
 
 static rte *
-export_filter(struct announce_hook *ah, rte *rt0, rte **rt_free, ea_list **tmpa, int silent)
+export_filter_(struct announce_hook *ah, rte *rt0, rte **rt_free, ea_list **tmpa, linpool *pool, int silent)
 {
   struct proto *p = ah->proto;
   struct filter *filter = ah->out_filter;
@@ -260,9 +260,9 @@ export_filter(struct announce_hook *ah, rte *rt0, rte **rt_free, ea_list **tmpa,
   if (!tmpa)
     tmpa = &tmpb;
 
-  *tmpa = make_tmp_attrs(rt, rte_update_pool);
+  *tmpa = make_tmp_attrs(rt, pool);
 
-  v = p->import_control ? p->import_control(p, &rt, tmpa, rte_update_pool) : 0;
+  v = p->import_control ? p->import_control(p, &rt, tmpa, pool) : 0;
   if (v < 0)
     {
       if (silent)
@@ -281,7 +281,7 @@ export_filter(struct announce_hook *ah, rte *rt0, rte **rt_free, ea_list **tmpa,
     }
 
   v = filter && ((filter == FILTER_REJECT) ||
-		 (f_run(filter, &rt, tmpa, rte_update_pool, FF_FORCE_TMPATTR) > F_ACCEPT));
+		 (f_run(filter, &rt, tmpa, pool, FF_FORCE_TMPATTR) > F_ACCEPT));
   if (v)
     {
       if (silent)
@@ -304,6 +304,12 @@ export_filter(struct announce_hook *ah, rte *rt0, rte **rt_free, ea_list **tmpa,
   return NULL;
 }
 
+static inline rte *
+export_filter(struct announce_hook *ah, rte *rt0, rte **rt_free, ea_list **tmpa, int silent)
+{
+  return export_filter_(ah, rt0, rt_free, tmpa, rte_update_pool, silent);
+}
+
 static void
 do_rt_notify(struct announce_hook *ah, net *net, rte *new, rte *old, ea_list *tmpa, int refeed)
 {
@@ -586,7 +592,7 @@ rt_notify_accepted(struct announce_hook *ah, net *net, rte *new_changed, rte *ol
 
 
 static struct mpnh *
-mpnh_merge_rta(struct mpnh *nhs, rta *a, int max)
+mpnh_merge_rta(struct mpnh *nhs, rta *a, linpool *pool, int max)
 {
   struct mpnh nh = { .gw = a->gw, .iface = a->iface };
   struct mpnh *nh2 = (a->dest == RTD_MULTIPATH) ? a->nexthops : &nh;
@@ -594,7 +600,7 @@ mpnh_merge_rta(struct mpnh *nhs, rta *a, int max)
 }
 
 rte *
-rt_export_merged(struct announce_hook *ah, net *net, rte **rt_free, ea_list **tmpa, int silent)
+rt_export_merged(struct announce_hook *ah, net *net, rte **rt_free, ea_list **tmpa, linpool *pool, int silent)
 {
   // struct proto *p = ah->proto;
   struct mpnh *nhs = NULL;
@@ -606,7 +612,7 @@ rt_export_merged(struct announce_hook *ah, net *net, rte **rt_free, ea_list **tm
   if (!rte_is_valid(best0))
     return NULL;
 
-  best = export_filter(ah, best0, rt_free, tmpa, silent);
+  best = export_filter_(ah, best0, rt_free, tmpa, pool, silent);
 
   if (!best || !rte_is_reachable(best))
     return best;
@@ -616,13 +622,13 @@ rt_export_merged(struct announce_hook *ah, net *net, rte **rt_free, ea_list **tm
     if (!rte_mergable(best0, rt0))
       continue;
 
-    rt = export_filter(ah, rt0, &tmp, NULL, 1);
+    rt = export_filter_(ah, rt0, &tmp, NULL, pool, 1);
 
     if (!rt)
       continue;
 
     if (rte_is_reachable(rt))
-      nhs = mpnh_merge_rta(nhs, rt->attrs, ah->proto->merge_limit);
+      nhs = mpnh_merge_rta(nhs, rt->attrs, pool, ah->proto->merge_limit);
 
     if (tmp)
       rte_free(tmp);
@@ -630,11 +636,11 @@ rt_export_merged(struct announce_hook *ah, net *net, rte **rt_free, ea_list **tm
 
   if (nhs)
   {
-    nhs = mpnh_merge_rta(nhs, best->attrs, ah->proto->merge_limit);
+    nhs = mpnh_merge_rta(nhs, best->attrs, pool, ah->proto->merge_limit);
 
     if (nhs->next)
     {
-      best = rte_cow_rta(best, rte_update_pool);
+      best = rte_cow_rta(best, pool);
       best->attrs->dest = RTD_MULTIPATH;
       best->attrs->nexthops = nhs;
     }
@@ -685,7 +691,7 @@ rt_notify_merged(struct announce_hook *ah, net *net, rte *new_changed, rte *old_
 
   /* Prepare new merged route */
   if (new_best)
-    new_best = rt_export_merged(ah, net, &new_best_free, &tmpa, 0);
+    new_best = rt_export_merged(ah, net, &new_best_free, &tmpa, rte_update_pool, 0);
 
   /* Prepare old merged route (without proper merged next hops) */
   /* There are some issues with running filter on old route - see rt_notify_basic() */
@@ -2470,7 +2476,7 @@ rt_show_net(struct cli *c, net *n, struct rt_show_data *d)
       if ((d->export_mode == RSEM_EXPORT) && (d->export_protocol->accept_ra_types == RA_MERGED))
         {
 	  rte *rt_free;
-	  e = rt_export_merged(a, n, &rt_free, &tmpa, 1);
+	  e = rt_export_merged(a, n, &rt_free, &tmpa, rte_update_pool, 1);
 	  pass = 1;
 
 	  if (!e)
diff --git a/sysdep/unix/krt.c b/sysdep/unix/krt.c
index f5dee87..ef98cb3 100644
--- a/sysdep/unix/krt.c
+++ b/sysdep/unix/krt.c
@@ -613,7 +613,7 @@ krt_export_net(struct krt_proto *p, net *net, rte **rt_free, ea_list **tmpa)
   rte *rt;
 
   if (p->p.accept_ra_types == RA_MERGED)
-    return rt_export_merged(ah, net, rt_free, tmpa, 1);
+    return rt_export_merged(ah, net, rt_free, tmpa, krt_filter_lp, 1);
 
   rt = net->routes;
   *rt_free = NULL;

Attachment: signature.asc
Description: Digital signature

Reply via email to