Re: cleanup multicast rttimer queues

2022-04-28 Thread Claudio Jeker
On Thu, Apr 28, 2022 at 05:51:57PM +0200, Alexander Bluhm wrote:
> On Wed, Apr 27, 2022 at 12:10:59PM +0200, Claudio Jeker wrote:
> > There is no need to have a rttimer queue per rdomain. The rttimer itself
> > is rdomain aware and so this just make everything more complicated for no
> > gain.
> > 
> > This diff just drops back to a single queue and initializes the queues in
> > ip_init() and the IPv6 counterpart. I have no mrouter setup to test this
> > but it compiles and I see no reason why it would not work.
> 
> I have tested it with regress/sys/netinet/mcast and
> regress/sys/netinet6/mcast6 .  There is a minimal multicast router.
> 
> OK bluhm@ with two nits.
> 
> Could you put these declarations into header files?
> Should these globals have an ip_ prefix?
> 
> >  #ifdef MROUTING
> >  extern int ip_mrtproto;
> > +extern struct rttimer_queue *mrouterq;
> >  #endif
> 
> > +#ifdef MROUTING
> > +extern struct rttimer_queue *mrouter6q;
> > +#endif

If we put them in a header then I think naming them ip_mrouterq and
ip6_mrouterq (at least I think that is the proper naming for IPv6
structs). Also they should go into ip_mroute.h and ip6_mroute.h. 
I will do those changes before I commit.

-- 
:wq Claudio



Re: cleanup multicast rttimer queues

2022-04-28 Thread Alexander Bluhm
On Wed, Apr 27, 2022 at 12:10:59PM +0200, Claudio Jeker wrote:
> There is no need to have a rttimer queue per rdomain. The rttimer itself
> is rdomain aware and so this just make everything more complicated for no
> gain.
> 
> This diff just drops back to a single queue and initializes the queues in
> ip_init() and the IPv6 counterpart. I have no mrouter setup to test this
> but it compiles and I see no reason why it would not work.

I have tested it with regress/sys/netinet/mcast and
regress/sys/netinet6/mcast6 .  There is a minimal multicast router.

OK bluhm@ with two nits.

Could you put these declarations into header files?
Should these globals have an ip_ prefix?

>  #ifdef MROUTING
>  extern int ip_mrtproto;
> +extern struct rttimer_queue *mrouterq;
>  #endif

> +#ifdef MROUTING
> +extern struct rttimer_queue *mrouter6q;
> +#endif



cleanup multicast rttimer queues

2022-04-27 Thread Claudio Jeker
There is no need to have a rttimer queue per rdomain. The rttimer itself
is rdomain aware and so this just make everything more complicated for no
gain.

This diff just drops back to a single queue and initializes the queues in
ip_init() and the IPv6 counterpart. I have no mrouter setup to test this
but it compiles and I see no reason why it would not work.

-- 
:wq Claudio

Index: netinet/ip_input.c
===
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.367
diff -u -p -r1.367 ip_input.c
--- netinet/ip_input.c  20 Apr 2022 09:38:26 -  1.367
+++ netinet/ip_input.c  27 Apr 2022 09:43:24 -
@@ -108,6 +108,7 @@ int ip_frags = 0;
 
 #ifdef MROUTING
 extern int ip_mrtproto;
+extern struct rttimer_queue *mrouterq;
 #endif
 
 const struct sysctl_bounded_args ipctl_vars[] = {
@@ -223,6 +224,9 @@ ip_init(void)
arpinit();
 #ifdef IPSEC
ipsec_init();
+#endif
+#ifdef MROUTING
+   mrouterq = rt_timer_queue_create(MCAST_EXPIRE_FREQUENCY);
 #endif
 }
 
Index: netinet/ip_mroute.c
===
RCS file: /cvs/src/sys/netinet/ip_mroute.c,v
retrieving revision 1.131
diff -u -p -r1.131 ip_mroute.c
--- netinet/ip_mroute.c 15 Dec 2021 17:21:08 -  1.131
+++ netinet/ip_mroute.c 27 Apr 2022 09:43:36 -
@@ -96,7 +96,7 @@ int mcast_debug = 1;
  * except for netstat or debugging purposes.
  */
 struct socket  *ip_mrouter[RT_TABLEID_MAX + 1];
-struct rttimer_queue *mrouterq[RT_TABLEID_MAX + 1];
+struct rttimer_queue *mrouterq;
 uint64_tmrt_count[RT_TABLEID_MAX + 1];
 intip_mrtproto = IGMP_DVMRP;/* for netstat only */
 
@@ -515,12 +515,10 @@ ip_mrouter_init(struct socket *so, struc
if (*v != 1)
return (EINVAL);
 
-   if (ip_mrouter[rtableid] != NULL ||
-   mrouterq[rtableid] != NULL)
+   if (ip_mrouter[rtableid] != NULL)
return (EADDRINUSE);
 
ip_mrouter[rtableid] = so;
-   mrouterq[rtableid] = rt_timer_queue_create(MCAST_EXPIRE_FREQUENCY);
 
return (0);
 }
@@ -572,8 +570,6 @@ ip_mrouter_done(struct socket *so)
 
mrt_api_config = 0;
 
-   rt_timer_queue_destroy(mrouterq[rtableid]);
-   mrouterq[rtableid] = NULL;
ip_mrouter[rtableid] = NULL;
mrt_count[rtableid] = 0;
 
@@ -799,8 +795,7 @@ mfc_expire_route(struct rtentry *rt, str
/* Not expired, add it back to the queue. */
if (mfc->mfc_expire == 0) {
mfc->mfc_expire = 1;
-   rt_timer_add(rt, mfc_expire_route, mrouterq[rtableid],
-   rtableid);
+   rt_timer_add(rt, mfc_expire_route, mrouterq, rtableid);
return;
}
 
@@ -834,8 +829,7 @@ mfc_add_route(struct ifnet *ifp, struct 
 
rt->rt_llinfo = (caddr_t)mfc;
 
-   rt_timer_add(rt, mfc_expire_route, mrouterq[rtableid],
-   rtableid);
+   rt_timer_add(rt, mfc_expire_route, mrouterq, rtableid);
 
mfc->mfc_parent = mfccp->mfcc_parent;
mfc->mfc_pkt_cnt = 0;
Index: netinet6/ip6_input.c
===
RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.241
diff -u -p -r1.241 ip6_input.c
--- netinet6/ip6_input.c20 Apr 2022 09:38:26 -  1.241
+++ netinet6/ip6_input.c27 Apr 2022 09:47:07 -
@@ -130,6 +130,10 @@ static void ip6_send_dispatch(void *);
 static struct task ip6send_task =
TASK_INITIALIZER(ip6_send_dispatch, _mq);
 
+#ifdef MROUTING
+extern struct rttimer_queue *mrouter6q;
+#endif
+
 /*
  * IP6 initialization: fill in IP6 protocol switch table.
  * All protocols not implemented in kernel go to raw IP6 protocol handler.
@@ -158,6 +162,9 @@ ip6_init(void)
mq_init(_mq, 64, IPL_SOFTNET);
 
ip6counters = counters_alloc(ip6s_ncounters);
+#ifdef MROUTING
+   mrouter6q = rt_timer_queue_create(MCAST_EXPIRE_TIMEOUT);
+#endif
 }
 
 void
Index: netinet6/ip6_mroute.c
===
RCS file: /cvs/src/sys/netinet6/ip6_mroute.c,v
retrieving revision 1.127
diff -u -p -r1.127 ip6_mroute.c
--- netinet6/ip6_mroute.c   15 Dec 2021 17:21:08 -  1.127
+++ netinet6/ip6_mroute.c   27 Apr 2022 09:47:49 -
@@ -130,7 +130,7 @@ void phyint_send6(struct ifnet *, struct
  * except for netstat or debugging purposes.
  */
 struct socket  *ip6_mrouter[RT_TABLEID_MAX + 1];
-struct rttimer_queue *mrouter6q[RT_TABLEID_MAX + 1];
+struct rttimer_queue *mrouter6q;
 intip6_mrouter_ver = 0;
 intip6_mrtproto;/* for netstat only */
 struct mrt6statmrt6stat;
@@ -138,8 +138,6 @@ struct mrt6stat mrt6stat;
 #define NO_RTE_FOUND   0x1
 #define RTE_FOUND  0x2
 
-#defineMCAST_EXPIRE_TIMEOUT 30 /* seconds */
-
 /*
  * Macros to compute elapsed time efficiently
  * Borrowed from Van Jacobson's