Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: mp-safe multicast counter
To:
Jan Klemkow <jan@openbsd.org>
Cc:
tech@openbsd.org
Date:
Fri, 9 May 2025 08:44:33 +0300

Download raw body.

Thread
On Fri, May 09, 2025 at 11:55:30AM +0200, Jan Klemkow wrote:
> Hi,
> 
> This diff uses the per CPU counters infrastructure for mrtstats.  While
> here, I also moved the ip_mrouterq() call in a new mrt_init() function.
> 
> ok?
> 

ok mvs

> bye,
> Jan
> 
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> diff -u -p -r1.405 ip_input.c
> --- netinet/ip_input.c	12 Mar 2025 23:27:17 -0000	1.405
> +++ netinet/ip_input.c	8 May 2025 20:53:57 -0000
> @@ -235,8 +235,7 @@ ip_init(void)
>  	ipsec_init();
>  #endif
>  #ifdef MROUTING
> -	rt_timer_queue_init(&ip_mrouterq, MCAST_EXPIRE_FREQUENCY,
> -	    &mfc_expire_route);
> +	mrt_init();
>  #endif
>  }
>  
> @@ -1790,8 +1789,7 @@ ip_sysctl(int *name, u_int namelen, void
>  		return (ip_sysctl_ipstat(oldp, oldlenp, newp));
>  #ifdef MROUTING
>  	case IPCTL_MRTSTATS:
> -		return (sysctl_rdstruct(oldp, oldlenp, newp,
> -		    &mrtstat, sizeof(mrtstat)));
> +		return (mrt_sysctl_mrtstat(oldp, oldlenp, newp));
>  	case IPCTL_MRTMFC:
>  		if (newp)
>  			return (EPERM);
> Index: netinet/ip_mroute.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_mroute.c,v
> diff -u -p -r1.144 ip_mroute.c
> --- netinet/ip_mroute.c	11 Mar 2025 15:31:03 -0000	1.144
> +++ netinet/ip_mroute.c	8 May 2025 21:04:05 -0000
> @@ -64,6 +64,7 @@
>  #include <sys/protosw.h>
>  #include <sys/ioctl.h>
>  #include <sys/syslog.h>
> +#include <sys/sysctl.h>
>  
>  #include <net/if.h>
>  #include <net/if_var.h>
> @@ -100,7 +101,7 @@ struct rttimer_queue ip_mrouterq;
>  uint64_t	 mrt_count[RT_TABLEID_MAX + 1];
>  int		ip_mrtproto = IGMP_DVMRP;    /* for netstat only */
>  
> -struct mrtstat	mrtstat;
> +struct cpumem *mrtcounters;
>  
>  struct rtentry	*mfc_find(struct ifnet *, struct in_addr *,
>      struct in_addr *, unsigned int);
> @@ -113,6 +114,7 @@ int get_version(struct mbuf *);
>  int add_vif(struct socket *, struct mbuf *);
>  int del_vif(struct socket *, struct mbuf *);
>  void update_mfc_params(struct mfcctl2 *, int, unsigned int);
> +void mfc_expire_route(struct rtentry *, u_int);
>  int mfc_add(struct mfcctl2 *, struct in_addr *, struct in_addr *,
>      int, unsigned int, int);
>  int add_mfc(struct socket *, struct mbuf *);
> @@ -140,8 +142,8 @@ static u_int32_t mrt_api_config = 0;
>  /*
>   * Find a route for a given origin IP address and Multicast group address
>   * Type of service parameter to be added in the future!!!
> - * Statistics are updated by the caller if needed
> - * (mrtstat.mrts_mfc_lookups and mrtstat.mrts_mfc_misses)
> + * Statistics are updated by the caller if needed (mrts_mfc_lookups and
> + * mrts_mfc_misses)
>   */
>  struct rtentry *
>  mfc_find(struct ifnet *ifp, struct in_addr *origin, struct in_addr *group,
> @@ -249,6 +251,15 @@ ip_mrouter_get(struct socket *so, int op
>  	return (error);
>  }
>  
> +void
> +mrt_init(void)
> +{
> +	mrtcounters = counters_alloc(mrts_ncounters);
> +
> +	rt_timer_queue_init(&ip_mrouterq, MCAST_EXPIRE_FREQUENCY,
> +	    &mfc_expire_route);
> +}
> +
>  /*
>   * Handle ioctl commands to obtain information from the cache
>   */
> @@ -463,6 +474,38 @@ mrt_rtwalk_mfcsysctl(struct rtentry *rt,
>  }
>  
>  int
> +mrt_sysctl_mrtstat(void *oldp, size_t *oldlenp, void *newp)
> +{
> +	uint64_t counters[mrts_ncounters];
> +	struct mrtstat mrtstat;
> +	int i = 0;
> +
> +#define ASSIGN(field)	do { mrtstat.field = counters[i++]; } while (0)
> +
> +	memset(&mrtstat, 0, sizeof mrtstat);
> +	counters_read(mrtcounters, counters, nitems(counters), NULL);
> +
> +	ASSIGN(mrts_mfc_lookups);
> +	ASSIGN(mrts_mfc_misses);
> +	ASSIGN(mrts_upcalls);
> +	ASSIGN(mrts_no_route);
> +	ASSIGN(mrts_bad_tunnel);
> +	ASSIGN(mrts_cant_tunnel);
> +	ASSIGN(mrts_wrong_if);
> +	ASSIGN(mrts_upq_ovflw);
> +	ASSIGN(mrts_cache_cleanups);
> +	ASSIGN(mrts_drop_sel);
> +	ASSIGN(mrts_q_overflow);
> +	ASSIGN(mrts_pkt2large);
> +	ASSIGN(mrts_upq_sockfull);
> +
> +#undef ASSIGN
> +
> +	return (sysctl_rdstruct(oldp, oldlenp, newp,
> +	    &mrtstat, sizeof(mrtstat)));
> +}
> +
> +int
>  mrt_sysctl_mfc(void *oldp, size_t *oldlenp)
>  {
>  	unsigned int		 rtableid;
> @@ -1116,7 +1159,7 @@ ip_mforward(struct mbuf *m, struct ifnet
>  	/*
>  	 * Determine forwarding vifs from the forwarding cache table
>  	 */
> -	++mrtstat.mrts_mfc_lookups;
> +	mrtstat_inc(mrts_mfc_lookups);
>  	rt = mfc_find(NULL, &ip->ip_src, &ip->ip_dst, rtableid);
>  
>  	/* Entry exists, so forward if necessary */
> @@ -1129,8 +1172,8 @@ ip_mforward(struct mbuf *m, struct ifnet
>  		 */
>  		int hlen = ip->ip_hl << 2;
>  
> -		++mrtstat.mrts_mfc_misses;
> -		mrtstat.mrts_no_route++;
> +		mrtstat_inc(mrts_mfc_misses);
> +		mrtstat_inc(mrts_no_route);
>  
>  		{
>  			struct igmpmsg *im;
> @@ -1161,13 +1204,13 @@ ip_mforward(struct mbuf *m, struct ifnet
>  			im->im_mbz = 0;
>  			im->im_vif = v->v_id;
>  
> -			mrtstat.mrts_upcalls++;
> +			mrtstat_inc(mrts_upcalls);
>  
>  			sin.sin_addr = ip->ip_src;
>  			if (socket_send(ip_mrouter[rtableid], mm, &sin) < 0) {
>  				log(LOG_WARNING, "ip_mforward: ip_mrouter "
>  				    "socket queue full\n");
> -				++mrtstat.mrts_upq_sockfull;
> +				mrtstat_inc(mrts_upq_sockfull);
>  				return (ENOBUFS);
>  			}
>  
> @@ -1203,7 +1246,7 @@ ip_mdq(struct mbuf *m, struct ifnet *ifp
>  	 */
>  	if (mfc->mfc_parent != v->v_id) {
>  		/* came in the wrong interface */
> -		++mrtstat.mrts_wrong_if;
> +		mrtstat_inc(mrts_wrong_if);
>  		mfc->mfc_wrong_if++;
>  		rtfree(rt);
>  		return (0);
> Index: netinet/ip_mroute.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_mroute.h,v
> diff -u -p -r1.33 ip_mroute.h
> --- netinet/ip_mroute.h	1 Jan 2025 13:44:22 -0000	1.33
> +++ netinet/ip_mroute.h	9 May 2025 07:34:39 -0000
> @@ -167,15 +167,36 @@ struct mrtstat {
>  	u_long	mrts_upq_sockfull;	/* upcalls dropped - socket full */
>  };
>  
> -
>  #ifdef _KERNEL
>  
> +enum mrtstat_counters {
> +	mrts_mfc_lookups,
> +	mrts_mfc_misses,
> +	mrts_upcalls,
> +	mrts_no_route,
> +	mrts_bad_tunnel,
> +	mrts_cant_tunnel,
> +	mrts_wrong_if,
> +	mrts_upq_ovflw,
> +	mrts_cache_cleanups,
> +	mrts_drop_sel,
> +	mrts_q_overflow,
> +	mrts_pkt2large,
> +	mrts_upq_sockfull,
> +	mrts_ncounters
> +};
> +
> +extern struct cpumem *mrtcounters;
> +
> +static inline void
> +mrtstat_inc(enum mrtstat_counters c)
> +{
> +	counters_inc(mrtcounters, c);
> +}
> +
>  /* How frequent should we look for expired entries (in seconds). */
>  #define MCAST_EXPIRE_FREQUENCY		30
>  
> -extern struct rttimer_queue ip_mrouterq;
> -void mfc_expire_route(struct rtentry *, u_int);
> -
>  extern int ip_mrtproto;
>  
>  /*
> @@ -228,8 +249,10 @@ struct igmpmsg {
>  
>  int	ip_mrouter_set(struct socket *, int, struct mbuf *);
>  int	ip_mrouter_get(struct socket *, int, struct mbuf *);
> +void	mrt_init(void);
>  int	mrt_ioctl(struct socket *, u_long, caddr_t);
>  int	mrt_sysctl_vif(void *, size_t *);
> +int	mrt_sysctl_mrtstat(void *, size_t *, void *);
>  int	mrt_sysctl_mfc(void *, size_t *);
>  int	ip_mrouter_done(struct socket *);
>  void	vif_delete(struct ifnet *);
>