Re: route timer pool
On Tue, Apr 19, 2022 at 06:53:28PM +0200, Alexander Bluhm wrote:
> On Tue, Apr 19, 2022 at 08:59:25AM +0200, Claudio Jeker wrote:
> > On Tue, Apr 19, 2022 at 01:44:40AM +0200, Alexander Bluhm wrote:
> > > Hi,
> > >
> > > Can we use a pool for rttimer_queue_pool?
> >
> > Another option would be to use static rttimer_queues instead of allocating
> > them. Not that many timers are used.
>
> Multicast allocates queues on demand per routing table. So static
> allocation is not an option.
Ah that code is wrong anyway. The way rtable/rdomain support was
shoehorned into multicast was wrong. It is on my cleanup list.
I do not understand why multicast needs per routing table queues while
IP and IPv6 do not. But as I said that code needs some work.
> sys_setsockopt -> sosetopt -> rip_ctloutput -> ip_mrouter_set ->
> ip_mrouter_init -> rt_timer_queue_create
>
> Diff merged to current.
>
> ok?
>
> > Requires additional changes in the
> > sysctl handlers (but that code is strange anyway).
>
> I am trying to clean up that mess.
>
> bluhm
>
> Index: net/route.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
> retrieving revision 1.403
> diff -u -p -r1.403 route.c
> --- net/route.c 19 Apr 2022 15:44:56 - 1.403
> +++ net/route.c 19 Apr 2022 15:53:59 -
> @@ -148,8 +148,9 @@ struct cpumem * rtcounters;
> int rttrash;/* routes not in table but not freed */
> int ifatrash; /* ifas not in ifp list but not free */
>
> -struct pool rtentry_pool; /* pool for rtentry structures */
> -struct pool rttimer_pool; /* pool for rttimer structures */
> +struct pool rtentry_pool; /* pool for rtentry structures */
> +struct pool rttimer_pool; /* pool for rttimer structures */
> +struct pool rttimer_queue_pool; /* pool for rttimer_queue structures */
>
> int rt_setgwroute(struct rtentry *, u_int);
> void rt_putgwroute(struct rtentry *);
> @@ -183,7 +184,7 @@ route_init(void)
> {
> rtcounters = counters_alloc(rts_ncounters);
>
> - pool_init(&rtentry_pool, sizeof(struct rtentry), 0, IPL_SOFTNET, 0,
> + pool_init(&rtentry_pool, sizeof(struct rtentry), 0, IPL_MPFLOOR, 0,
> "rtentry", NULL);
>
> while (rt_hashjitter == 0)
> @@ -1387,8 +1388,10 @@ rt_timer_init(void)
> {
> static struct timeout rt_timer_timeout;
>
> - pool_init(&rttimer_pool, sizeof(struct rttimer), 0, IPL_SOFTNET, 0,
> - "rttmr", NULL);
> + pool_init(&rttimer_pool, sizeof(struct rttimer), 0,
> + IPL_MPFLOOR, 0, "rttmr", NULL);
> + pool_init(&rttimer_queue_pool, sizeof(struct rttimer_queue), 0,
> + IPL_MPFLOOR, 0, "rttmrq", NULL);
>
> LIST_INIT(&rttimer_queue_head);
> timeout_set_proc(&rt_timer_timeout, rt_timer_timer, &rt_timer_timeout);
> @@ -1400,7 +1403,8 @@ rt_timer_queue_create(u_int timeout)
> {
> struct rttimer_queue*rtq;
>
> - if ((rtq = malloc(sizeof(*rtq), M_RTABLE, M_NOWAIT|M_ZERO)) == NULL)
> + rtq = pool_get(&rttimer_queue_pool, PR_NOWAIT | PR_ZERO);
> + if (rtq == NULL)
> return (NULL);
>
> rtq->rtq_timeout = timeout;
> @@ -1436,7 +1440,7 @@ rt_timer_queue_destroy(struct rttimer_qu
> }
>
> LIST_REMOVE(rtq, rtq_link);
> - free(rtq, M_RTABLE, sizeof(*rtq));
> + pool_put(&rttimer_queue_pool, rtq);
> }
>
> unsigned long
>
--
:wq Claudio
Re: route timer pool
On Tue, Apr 19, 2022 at 08:59:25AM +0200, Claudio Jeker wrote:
> On Tue, Apr 19, 2022 at 01:44:40AM +0200, Alexander Bluhm wrote:
> > Hi,
> >
> > Can we use a pool for rttimer_queue_pool?
>
> Another option would be to use static rttimer_queues instead of allocating
> them. Not that many timers are used.
Multicast allocates queues on demand per routing table. So static
allocation is not an option.
sys_setsockopt -> sosetopt -> rip_ctloutput -> ip_mrouter_set ->
ip_mrouter_init -> rt_timer_queue_create
Diff merged to current.
ok?
> Requires additional changes in the
> sysctl handlers (but that code is strange anyway).
I am trying to clean up that mess.
bluhm
Index: net/route.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
retrieving revision 1.403
diff -u -p -r1.403 route.c
--- net/route.c 19 Apr 2022 15:44:56 - 1.403
+++ net/route.c 19 Apr 2022 15:53:59 -
@@ -148,8 +148,9 @@ struct cpumem * rtcounters;
intrttrash;/* routes not in table but not freed */
intifatrash; /* ifas not in ifp list but not free */
-struct poolrtentry_pool; /* pool for rtentry structures */
-struct poolrttimer_pool; /* pool for rttimer structures */
+struct poolrtentry_pool; /* pool for rtentry structures */
+struct poolrttimer_pool; /* pool for rttimer structures */
+struct poolrttimer_queue_pool; /* pool for rttimer_queue structures */
intrt_setgwroute(struct rtentry *, u_int);
void rt_putgwroute(struct rtentry *);
@@ -183,7 +184,7 @@ route_init(void)
{
rtcounters = counters_alloc(rts_ncounters);
- pool_init(&rtentry_pool, sizeof(struct rtentry), 0, IPL_SOFTNET, 0,
+ pool_init(&rtentry_pool, sizeof(struct rtentry), 0, IPL_MPFLOOR, 0,
"rtentry", NULL);
while (rt_hashjitter == 0)
@@ -1387,8 +1388,10 @@ rt_timer_init(void)
{
static struct timeout rt_timer_timeout;
- pool_init(&rttimer_pool, sizeof(struct rttimer), 0, IPL_SOFTNET, 0,
- "rttmr", NULL);
+ pool_init(&rttimer_pool, sizeof(struct rttimer), 0,
+ IPL_MPFLOOR, 0, "rttmr", NULL);
+ pool_init(&rttimer_queue_pool, sizeof(struct rttimer_queue), 0,
+ IPL_MPFLOOR, 0, "rttmrq", NULL);
LIST_INIT(&rttimer_queue_head);
timeout_set_proc(&rt_timer_timeout, rt_timer_timer, &rt_timer_timeout);
@@ -1400,7 +1403,8 @@ rt_timer_queue_create(u_int timeout)
{
struct rttimer_queue*rtq;
- if ((rtq = malloc(sizeof(*rtq), M_RTABLE, M_NOWAIT|M_ZERO)) == NULL)
+ rtq = pool_get(&rttimer_queue_pool, PR_NOWAIT | PR_ZERO);
+ if (rtq == NULL)
return (NULL);
rtq->rtq_timeout = timeout;
@@ -1436,7 +1440,7 @@ rt_timer_queue_destroy(struct rttimer_qu
}
LIST_REMOVE(rtq, rtq_link);
- free(rtq, M_RTABLE, sizeof(*rtq));
+ pool_put(&rttimer_queue_pool, rtq);
}
unsigned long
Re: route timer pool
On Tue, Apr 19, 2022 at 01:44:40AM +0200, Alexander Bluhm wrote:
> Hi,
>
> Can we use a pool for rttimer_queue_pool?
Another option would be to use static rttimer_queues instead of allocating
them. Not that many timers are used. Requires additional changes in the
sysctl handlers (but that code is strange anyway).
> As we run without kernel lock, these pools should have IPL_MPFLOOR
> protection.
That makes sense to me. OK claudio@
> ok?
>
> bluhm
>
> Index: net/route.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
> retrieving revision 1.402
> diff -u -p -r1.402 route.c
> --- net/route.c 22 Feb 2022 01:15:02 - 1.402
> +++ net/route.c 18 Apr 2022 23:42:09 -
> @@ -148,8 +148,9 @@ struct cpumem * rtcounters;
> int rttrash;/* routes not in table but not freed */
> int ifatrash; /* ifas not in ifp list but not free */
>
> -struct pool rtentry_pool; /* pool for rtentry structures */
> -struct pool rttimer_pool; /* pool for rttimer structures */
> +struct pool rtentry_pool; /* pool for rtentry structures */
> +struct pool rttimer_pool; /* pool for rttimer structures */
> +struct pool rttimer_queue_pool; /* pool for rttimer_queue structures */
>
> void rt_timer_init(void);
> int rt_setgwroute(struct rtentry *, u_int);
> @@ -184,7 +185,7 @@ route_init(void)
> {
> rtcounters = counters_alloc(rts_ncounters);
>
> - pool_init(&rtentry_pool, sizeof(struct rtentry), 0, IPL_SOFTNET, 0,
> + pool_init(&rtentry_pool, sizeof(struct rtentry), 0, IPL_MPFLOOR, 0,
> "rtentry", NULL);
>
> while (rt_hashjitter == 0)
> @@ -1392,8 +1393,10 @@ rt_timer_init(void)
> if (rt_init_done)
> panic("rt_timer_init: already initialized");
>
> - pool_init(&rttimer_pool, sizeof(struct rttimer), 0, IPL_SOFTNET, 0,
> - "rttmr", NULL);
> + pool_init(&rttimer_pool, sizeof(struct rttimer), 0,
> + IPL_MPFLOOR, 0, "rttmr", NULL);
> + pool_init(&rttimer_queue_pool, sizeof(struct rttimer_queue), 0,
> + IPL_MPFLOOR, 0, "rttmrq", NULL);
>
> LIST_INIT(&rttimer_queue_head);
> timeout_set_proc(&rt_timer_timeout, rt_timer_timer, &rt_timer_timeout);
> @@ -1409,7 +1412,8 @@ rt_timer_queue_create(u_int timeout)
> if (rt_init_done == 0)
> rt_timer_init();
>
> - if ((rtq = malloc(sizeof(*rtq), M_RTABLE, M_NOWAIT|M_ZERO)) == NULL)
> + rtq = pool_get(&rttimer_queue_pool, PR_NOWAIT | PR_ZERO);
> + if (rtq == NULL)
> return (NULL);
>
> rtq->rtq_timeout = timeout;
> @@ -1445,7 +1449,7 @@ rt_timer_queue_destroy(struct rttimer_qu
> }
>
> LIST_REMOVE(rtq, rtq_link);
> - free(rtq, M_RTABLE, sizeof(*rtq));
> + pool_put(&rttimer_queue_pool, rtq);
> }
>
> unsigned long
>
--
:wq Claudio
route timer pool
Hi,
Can we use a pool for rttimer_queue_pool?
As we run without kernel lock, these pools should have IPL_MPFLOOR
protection.
ok?
bluhm
Index: net/route.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/net/route.c,v
retrieving revision 1.402
diff -u -p -r1.402 route.c
--- net/route.c 22 Feb 2022 01:15:02 - 1.402
+++ net/route.c 18 Apr 2022 23:42:09 -
@@ -148,8 +148,9 @@ struct cpumem * rtcounters;
intrttrash;/* routes not in table but not freed */
intifatrash; /* ifas not in ifp list but not free */
-struct poolrtentry_pool; /* pool for rtentry structures */
-struct poolrttimer_pool; /* pool for rttimer structures */
+struct poolrtentry_pool; /* pool for rtentry structures */
+struct poolrttimer_pool; /* pool for rttimer structures */
+struct poolrttimer_queue_pool; /* pool for rttimer_queue structures */
void rt_timer_init(void);
intrt_setgwroute(struct rtentry *, u_int);
@@ -184,7 +185,7 @@ route_init(void)
{
rtcounters = counters_alloc(rts_ncounters);
- pool_init(&rtentry_pool, sizeof(struct rtentry), 0, IPL_SOFTNET, 0,
+ pool_init(&rtentry_pool, sizeof(struct rtentry), 0, IPL_MPFLOOR, 0,
"rtentry", NULL);
while (rt_hashjitter == 0)
@@ -1392,8 +1393,10 @@ rt_timer_init(void)
if (rt_init_done)
panic("rt_timer_init: already initialized");
- pool_init(&rttimer_pool, sizeof(struct rttimer), 0, IPL_SOFTNET, 0,
- "rttmr", NULL);
+ pool_init(&rttimer_pool, sizeof(struct rttimer), 0,
+ IPL_MPFLOOR, 0, "rttmr", NULL);
+ pool_init(&rttimer_queue_pool, sizeof(struct rttimer_queue), 0,
+ IPL_MPFLOOR, 0, "rttmrq", NULL);
LIST_INIT(&rttimer_queue_head);
timeout_set_proc(&rt_timer_timeout, rt_timer_timer, &rt_timer_timeout);
@@ -1409,7 +1412,8 @@ rt_timer_queue_create(u_int timeout)
if (rt_init_done == 0)
rt_timer_init();
- if ((rtq = malloc(sizeof(*rtq), M_RTABLE, M_NOWAIT|M_ZERO)) == NULL)
+ rtq = pool_get(&rttimer_queue_pool, PR_NOWAIT | PR_ZERO);
+ if (rtq == NULL)
return (NULL);
rtq->rtq_timeout = timeout;
@@ -1445,7 +1449,7 @@ rt_timer_queue_destroy(struct rttimer_qu
}
LIST_REMOVE(rtq, rtq_link);
- free(rtq, M_RTABLE, sizeof(*rtq));
+ pool_put(&rttimer_queue_pool, rtq);
}
unsigned long
