On Mon, Dec 09, 2019 at 03:16:20PM +1100, Jason Tubnor wrote:
> On Mon, 9 Dec 2019 at 10:44, Remi Locherer <remi.loche...@relo.ch> wrote:
> 
> >
> > >
> > > I can reproduce this issue. But only when I combine the use of
> > > "interface XY { passive }" and "redistribute connected". When I remove
> > > the passive interfaces from ripd.conf memory consumption is stable.
> > >
> > > Why do you combine these configs?
> >
> > Below diff fixes the memory leak for me.
> >
> > In addition to clear r_list I also moved the check for passive interface
> > a bit up so that the function can return as soon as possible.
> >
> > OK?
> >
> > Remi
> >
> >
> >
> >
> This patch applied cleanly and has fixed the memory leak issue when
> interfaces are configured 'passive'.  Tested with 1 active and 6 passive
> interfaces on one host and with a little memory consumption on startup
> [expected], it settled back down to 984KB of consumed RAM for 391 subnets
> and didn't move over 1.5 hours.
> 
> As /cvs/src/usr.sbin/ripd/message.c hasn't been touched since 2014, this
> patch would apply cleanly to 6.5 and 6.6 if an errata notice needed to be
> created (I can test on these if validation is required - will just take a
> little time to spin them up).
> 
> Thanks for your help Remi.
> 
> Cheers,
> 
> Jason.

Any OKs for this?

Below again the patch.

Remi




Index: message.c
===================================================================
RCS file: /cvs/src/usr.sbin/ripd/message.c,v
retrieving revision 1.12
diff -u -p -r1.12 message.c
--- message.c   25 Oct 2014 03:23:49 -0000      1.12
+++ message.c   8 Dec 2019 23:35:42 -0000
@@ -105,15 +105,15 @@ send_triggered_update(struct iface *ifac
        u_int16_t                afi, route_tag;
        u_int32_t                address, netmask, nexthop, metric;
 
+       if (iface->passive)
+               return (0);
+
        inet_aton(ALL_RIP_ROUTERS, &dst.sin_addr);
 
        dst.sin_port = htons(RIP_PORT);
        dst.sin_family = AF_INET;
        dst.sin_len = sizeof(struct sockaddr_in);
 
-       if (iface->passive)
-               return (0);
-
        if ((buf = ibuf_open(iface->mtu - sizeof(struct ip) -
            sizeof(struct udphdr))) == NULL)
                fatal("send_triggered_update");
@@ -166,13 +166,15 @@ send_request(struct packet_head *r_list,
                port = htons(RIP_PORT);
        }
 
+       if (iface->passive) {
+               clear_list(r_list);
+               return (0);
+       }
+
        dst.sin_port = port;
        dst.sin_family = AF_INET;
        dst.sin_len = sizeof(struct sockaddr_in);
 
-       if (iface->passive)
-               return (0);
-
        while (!TAILQ_EMPTY(r_list)) {
                if ((buf = ibuf_open(iface->mtu - sizeof(struct ip) -
                    sizeof(struct udphdr))) == NULL)
@@ -240,13 +242,15 @@ send_response(struct packet_head *r_list
                port = htons(RIP_PORT);
        }
 
+       if (iface->passive) {
+               clear_list(r_list);
+               return (0);
+       }
+
        dst.sin_port = port;
        dst.sin_family = AF_INET;
        dst.sin_len = sizeof(struct sockaddr_in);
 
-       if (iface->passive)
-               return (0);
-
        while (!TAILQ_EMPTY(r_list)) {
                if ((buf = ibuf_open(iface->mtu - sizeof(struct ip) -
                    sizeof(struct udphdr))) == NULL)

Reply via email to