Re: ip_ipip.c / gif(4) percpu counters

2017-03-09 Thread David Hill
On Thu, Mar 09, 2017 at 03:49:21PM +0100, Jeremie Courreges-Anglas wrote:
> Martin Pieuchot  writes:
> 
> > On 08/03/17(Wed) 12:03, Jeremie Courreges-Anglas wrote:
> >> [...] 
> >> So here's a refreshed diff that initializes the counters directly from
> >> ip_init().  I remove the ipip_init() wrapper to make it clear that
> >> ip_init() is responsible for the job.
> >> 
> >> (Still) ok?
> >
> > I'm against adding more "#if PEUDOIF" in the stack everywhere and it
> > makes no sense to move more globals outside of ip_ipip.c.
> 
> The previous diff was just plain wrong.
> 
> > If you really don't want to use .pr_init, then call ipip_input() from
> > ip_input().
> 
> Actually I'm fine with .pr_init.  There are other protosw entries that
> reach the counters but one shouldn't remove entries carelessly anyway.
> 
> Is this better?
> 
Definitely. OK dhill@
> 
> Index: netinet/in_proto.c
> ===
> RCS file: /d/cvs/src/sys/netinet/in_proto.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 in_proto.c
> --- netinet/in_proto.c2 Mar 2017 08:58:24 -   1.74
> +++ netinet/in_proto.c8 Mar 2017 11:56:45 -
> @@ -239,7 +239,8 @@ struct protosw inetsw[] = {
>.pr_output = rip_output,
>.pr_ctloutput  = rip_ctloutput,
>.pr_usrreq = rip_usrreq,
> -  .pr_sysctl = ipip_sysctl
> +  .pr_sysctl = ipip_sysctl,
> +  .pr_init   = ipip_init
>  },
>  {
>.pr_type   = SOCK_RAW,
> @@ -284,7 +285,8 @@ struct protosw inetsw[] = {
>.pr_output = rip_output,
>.pr_ctloutput  = rip_ctloutput,
>.pr_usrreq = rip_usrreq,
> -  .pr_sysctl = ipip_sysctl
> +  .pr_sysctl = ipip_sysctl,
> +  .pr_init   = ipip_init
>  },
>  #ifdef INET6
>  {
> Index: netinet/ip_ipip.c
> ===
> RCS file: /d/cvs/src/sys/netinet/ip_ipip.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 ip_ipip.c
> --- netinet/ip_ipip.c 29 Jan 2017 19:58:47 -  1.71
> +++ netinet/ip_ipip.c 8 Mar 2017 11:59:04 -
> @@ -84,7 +84,13 @@
>   */
>  int ipip_allow = 0;
>  
> -struct ipipstat ipipstat;
> +struct cpumem *ipipcounters;
> +
> +void
> +ipip_init(void)
> +{
> + ipipcounters = counters_alloc(ipips_ncounters);
> +}
>  
>  /*
>   * Really only a wrapper for ipip_input(), for use with pr_input.
> @@ -95,7 +101,7 @@ ip4_input(struct mbuf **mp, int *offp, i
>   /* If we do not accept IP-in-IP explicitly, drop.  */
>   if (!ipip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
>   DPRINTF(("ip4_input(): dropped due to policy\n"));
> - ipipstat.ipips_pdrops++;
> + ipipstat_inc(ipips_pdrops);
>   m_freem(*mp);
>   return IPPROTO_DONE;
>   }
> @@ -129,7 +135,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   u_int8_t v;
>   sa_family_t af;
>  
> - ipipstat.ipips_ipackets++;
> + ipipstat_inc(ipips_ipackets);
>  
>   m_copydata(m, 0, 1, &v);
>  
> @@ -143,7 +149,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   break;
>  #endif
>   default:
> - ipipstat.ipips_family++;
> + ipipstat_inc(ipips_family);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -152,7 +158,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   if (m->m_len < hlen) {
>   if ((m = m_pullup(m, hlen)) == NULL) {
>   DPRINTF(("ipip_input(): m_pullup() failed\n"));
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   return IPPROTO_DONE;
>   }
>   }
> @@ -179,7 +185,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>  
>   /* Sanity check */
>   if (m->m_pkthdr.len < sizeof(struct ip)) {
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -195,7 +201,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   break;
>  #endif
>   default:
> - ipipstat.ipips_family++;
> + ipipstat_inc(ipips_family);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -206,7 +212,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   if (m->m_len < hlen) {
>   if ((m = m_pullup(m, hlen)) == NULL) {
>   DPRINTF(("ipip_input(): m_pullup() failed\n"));
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   return IPPROTO_DONE;
>   }
>   }
> @@ -229,7 +235,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   ECN_ALLOWED_IPSEC : ECN_ALLOWED;
>   if (!ip_ecn_egress(mode, &otos, &ipo->ip_tos)) {
>   DPRINTF(("ipip_input(): ip_ecn_egress() failed"));
> - ipipstat.ipips_pdrops++;
> + ipipstat_inc(ipips_pdrops);
>  

Re: ip_ipip.c / gif(4) percpu counters

2017-03-09 Thread Jeremie Courreges-Anglas
Martin Pieuchot  writes:

> On 08/03/17(Wed) 12:03, Jeremie Courreges-Anglas wrote:
>> [...] 
>> So here's a refreshed diff that initializes the counters directly from
>> ip_init().  I remove the ipip_init() wrapper to make it clear that
>> ip_init() is responsible for the job.
>> 
>> (Still) ok?
>
> I'm against adding more "#if PEUDOIF" in the stack everywhere and it
> makes no sense to move more globals outside of ip_ipip.c.

The previous diff was just plain wrong.

> If you really don't want to use .pr_init, then call ipip_input() from
> ip_input().

Actually I'm fine with .pr_init.  There are other protosw entries that
reach the counters but one shouldn't remove entries carelessly anyway.

Is this better?


Index: netinet/in_proto.c
===
RCS file: /d/cvs/src/sys/netinet/in_proto.c,v
retrieving revision 1.74
diff -u -p -r1.74 in_proto.c
--- netinet/in_proto.c  2 Mar 2017 08:58:24 -   1.74
+++ netinet/in_proto.c  8 Mar 2017 11:56:45 -
@@ -239,7 +239,8 @@ struct protosw inetsw[] = {
   .pr_output   = rip_output,
   .pr_ctloutput= rip_ctloutput,
   .pr_usrreq   = rip_usrreq,
-  .pr_sysctl   = ipip_sysctl
+  .pr_sysctl   = ipip_sysctl,
+  .pr_init = ipip_init
 },
 {
   .pr_type = SOCK_RAW,
@@ -284,7 +285,8 @@ struct protosw inetsw[] = {
   .pr_output   = rip_output,
   .pr_ctloutput= rip_ctloutput,
   .pr_usrreq   = rip_usrreq,
-  .pr_sysctl   = ipip_sysctl
+  .pr_sysctl   = ipip_sysctl,
+  .pr_init = ipip_init
 },
 #ifdef INET6
 {
Index: netinet/ip_ipip.c
===
RCS file: /d/cvs/src/sys/netinet/ip_ipip.c,v
retrieving revision 1.71
diff -u -p -r1.71 ip_ipip.c
--- netinet/ip_ipip.c   29 Jan 2017 19:58:47 -  1.71
+++ netinet/ip_ipip.c   8 Mar 2017 11:59:04 -
@@ -84,7 +84,13 @@
  */
 int ipip_allow = 0;
 
-struct ipipstat ipipstat;
+struct cpumem *ipipcounters;
+
+void
+ipip_init(void)
+{
+   ipipcounters = counters_alloc(ipips_ncounters);
+}
 
 /*
  * Really only a wrapper for ipip_input(), for use with pr_input.
@@ -95,7 +101,7 @@ ip4_input(struct mbuf **mp, int *offp, i
/* If we do not accept IP-in-IP explicitly, drop.  */
if (!ipip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
DPRINTF(("ip4_input(): dropped due to policy\n"));
-   ipipstat.ipips_pdrops++;
+   ipipstat_inc(ipips_pdrops);
m_freem(*mp);
return IPPROTO_DONE;
}
@@ -129,7 +135,7 @@ ipip_input(struct mbuf **mp, int *offp, 
u_int8_t v;
sa_family_t af;
 
-   ipipstat.ipips_ipackets++;
+   ipipstat_inc(ipips_ipackets);
 
m_copydata(m, 0, 1, &v);
 
@@ -143,7 +149,7 @@ ipip_input(struct mbuf **mp, int *offp, 
break;
 #endif
default:
-   ipipstat.ipips_family++;
+   ipipstat_inc(ipips_family);
m_freem(m);
return IPPROTO_DONE;
}
@@ -152,7 +158,7 @@ ipip_input(struct mbuf **mp, int *offp, 
if (m->m_len < hlen) {
if ((m = m_pullup(m, hlen)) == NULL) {
DPRINTF(("ipip_input(): m_pullup() failed\n"));
-   ipipstat.ipips_hdrops++;
+   ipipstat_inc(ipips_hdrops);
return IPPROTO_DONE;
}
}
@@ -179,7 +185,7 @@ ipip_input(struct mbuf **mp, int *offp, 
 
/* Sanity check */
if (m->m_pkthdr.len < sizeof(struct ip)) {
-   ipipstat.ipips_hdrops++;
+   ipipstat_inc(ipips_hdrops);
m_freem(m);
return IPPROTO_DONE;
}
@@ -195,7 +201,7 @@ ipip_input(struct mbuf **mp, int *offp, 
break;
 #endif
default:
-   ipipstat.ipips_family++;
+   ipipstat_inc(ipips_family);
m_freem(m);
return IPPROTO_DONE;
}
@@ -206,7 +212,7 @@ ipip_input(struct mbuf **mp, int *offp, 
if (m->m_len < hlen) {
if ((m = m_pullup(m, hlen)) == NULL) {
DPRINTF(("ipip_input(): m_pullup() failed\n"));
-   ipipstat.ipips_hdrops++;
+   ipipstat_inc(ipips_hdrops);
return IPPROTO_DONE;
}
}
@@ -229,7 +235,7 @@ ipip_input(struct mbuf **mp, int *offp, 
ECN_ALLOWED_IPSEC : ECN_ALLOWED;
if (!ip_ecn_egress(mode, &otos, &ipo->ip_tos)) {
DPRINTF(("ipip_input(): ip_ecn_egress() failed"));
-   ipipstat.ipips_pdrops++;
+   ipipstat_inc(ipips_pdrops);
m_freem(m);
return IPPROTO_DONE;
}
@@ -249,7 +255,7 @@ ipip_input(struct mbuf **mp, int *offp, 
itos = (ntohl(ip6->ip6_flow) >> 20) & 0xff;
if (!ip_ecn_egress(EC

Re: ip_ipip.c / gif(4) percpu counters

2017-03-08 Thread Alexander Bluhm
On Wed, Mar 08, 2017 at 12:03:12PM +0100, Jeremie Courreges-Anglas wrote:
> So here's a refreshed diff that initializes the counters directly from
> ip_init().  I remove the ipip_init() wrapper to make it clear that
> ip_init() is responsible for the job.
> 
> (Still) ok?

File netinet/ip_ipip.c is always linked into the kernel and the
ipipcounters are defined there.  So a kernel without gif would run
with uninitialized counters.

I think the #if NGIF is wrong.

> 
> 
> Index: netinet/ip_input.c
> ===
> RCS file: /d/cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.295
> diff -u -p -r1.295 ip_input.c
> --- netinet/ip_input.c5 Feb 2017 16:23:38 -   1.295
> +++ netinet/ip_input.c8 Mar 2017 10:19:34 -
> @@ -34,6 +34,7 @@
>  
>  #include "pf.h"
>  #include "carp.h"
> +#include "gif.h"
>  
>  #include 
>  #include 
> @@ -78,6 +79,10 @@
>  #include 
>  #endif
>  
> +#if NGIF > 0
> +#include 
> +#endif
> +
>  struct ipqhead ipq;
>  
>  int encdebug = 0;
> @@ -168,6 +173,9 @@ ip_init(void)
>   const u_int16_t defrootonlyports_udp[] = DEFROOTONLYPORTS_UDP;
>  
>   ipcounters = counters_alloc(ips_ncounters);
> +#if NGIF > 0
> + ipipcounters = counters_alloc(ipips_ncounters);
> +#endif
>  
>   pool_init(&ipqent_pool, sizeof(struct ipqent), 0,
>   IPL_SOFTNET, 0, "ipqe",  NULL);
> Index: netinet/ip_ipip.c
> ===
> RCS file: /d/cvs/src/sys/netinet/ip_ipip.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 ip_ipip.c
> --- netinet/ip_ipip.c 29 Jan 2017 19:58:47 -  1.71
> +++ netinet/ip_ipip.c 8 Mar 2017 10:20:44 -
> @@ -84,7 +84,7 @@
>   */
>  int ipip_allow = 0;
>  
> -struct ipipstat ipipstat;
> +struct cpumem *ipipcounters;
>  
>  /*
>   * Really only a wrapper for ipip_input(), for use with pr_input.
> @@ -95,7 +95,7 @@ ip4_input(struct mbuf **mp, int *offp, i
>   /* If we do not accept IP-in-IP explicitly, drop.  */
>   if (!ipip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
>   DPRINTF(("ip4_input(): dropped due to policy\n"));
> - ipipstat.ipips_pdrops++;
> + ipipstat_inc(ipips_pdrops);
>   m_freem(*mp);
>   return IPPROTO_DONE;
>   }
> @@ -129,7 +129,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   u_int8_t v;
>   sa_family_t af;
>  
> - ipipstat.ipips_ipackets++;
> + ipipstat_inc(ipips_ipackets);
>  
>   m_copydata(m, 0, 1, &v);
>  
> @@ -143,7 +143,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   break;
>  #endif
>   default:
> - ipipstat.ipips_family++;
> + ipipstat_inc(ipips_family);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -152,7 +152,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   if (m->m_len < hlen) {
>   if ((m = m_pullup(m, hlen)) == NULL) {
>   DPRINTF(("ipip_input(): m_pullup() failed\n"));
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   return IPPROTO_DONE;
>   }
>   }
> @@ -179,7 +179,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>  
>   /* Sanity check */
>   if (m->m_pkthdr.len < sizeof(struct ip)) {
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -195,7 +195,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   break;
>  #endif
>   default:
> - ipipstat.ipips_family++;
> + ipipstat_inc(ipips_family);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -206,7 +206,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   if (m->m_len < hlen) {
>   if ((m = m_pullup(m, hlen)) == NULL) {
>   DPRINTF(("ipip_input(): m_pullup() failed\n"));
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   return IPPROTO_DONE;
>   }
>   }
> @@ -229,7 +229,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   ECN_ALLOWED_IPSEC : ECN_ALLOWED;
>   if (!ip_ecn_egress(mode, &otos, &ipo->ip_tos)) {
>   DPRINTF(("ipip_input(): ip_ecn_egress() failed"));
> - ipipstat.ipips_pdrops++;
> + ipipstat_inc(ipips_pdrops);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -249,7 +249,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   itos = (ntohl(ip6->ip6_flow) >> 20) & 0xff;
>   if (!ip_ecn_egress(ECN_ALLOWED, &otos, &itos)) {
>   DPRINTF(("ipip_input(): ip_ecn_egress() failed"));
> - ipipstat.ipips_pdrops++;
> + ip

Re: ip_ipip.c / gif(4) percpu counters

2017-03-08 Thread Martin Pieuchot
On 08/03/17(Wed) 12:03, Jeremie Courreges-Anglas wrote:
> [...] 
> So here's a refreshed diff that initializes the counters directly from
> ip_init().  I remove the ipip_init() wrapper to make it clear that
> ip_init() is responsible for the job.
> 
> (Still) ok?

I'm against adding more "#if PEUDOIF" in the stack everywhere and it
makes no sense to move more globals outside of ip_ipip.c.

If you really don't want to use .pr_init, then call ipip_input() from
ip_input().



Re: ip_ipip.c / gif(4) percpu counters

2017-03-08 Thread Jeremie Courreges-Anglas
Alexander Bluhm  writes:

> On Tue, Mar 07, 2017 at 06:16:30PM +0100, Jeremie Courreges-Anglas wrote:
>> 
>> I failed to find a nice place where to initialize the counters.  The
>> code that uses counters is reachable even if gif(4) isn't compiled in.
>> 
>> I can think of 3 obvious ways to call the init function.
>> 
>> 1. call ipip_init() through .pr_init.  The idea would be to call
>>ipip_init() once per protosw entry that needs it, so the function
>>should return early if it was already run.
>> 
>> 2. call ipip_init() from ip_init(), after all ip_init() is always
>>compiled in.
>> 
>> 3. call ipip_init() from init_main.c.
>> 
>> The diff implements option 3, but what do you folks prefer?
>> Thoughts / ok?
>
> I would use option 2 as ip_ipip.c is always comiled in.  Then you
> don't need the "if (ipipcounters == NULL)" check.
>
> OK bluhm@

So here's a refreshed diff that initializes the counters directly from
ip_init().  I remove the ipip_init() wrapper to make it clear that
ip_init() is responsible for the job.

(Still) ok?


Index: netinet/ip_input.c
===
RCS file: /d/cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.295
diff -u -p -r1.295 ip_input.c
--- netinet/ip_input.c  5 Feb 2017 16:23:38 -   1.295
+++ netinet/ip_input.c  8 Mar 2017 10:19:34 -
@@ -34,6 +34,7 @@
 
 #include "pf.h"
 #include "carp.h"
+#include "gif.h"
 
 #include 
 #include 
@@ -78,6 +79,10 @@
 #include 
 #endif
 
+#if NGIF > 0
+#include 
+#endif
+
 struct ipqhead ipq;
 
 int encdebug = 0;
@@ -168,6 +173,9 @@ ip_init(void)
const u_int16_t defrootonlyports_udp[] = DEFROOTONLYPORTS_UDP;
 
ipcounters = counters_alloc(ips_ncounters);
+#if NGIF > 0
+   ipipcounters = counters_alloc(ipips_ncounters);
+#endif
 
pool_init(&ipqent_pool, sizeof(struct ipqent), 0,
IPL_SOFTNET, 0, "ipqe",  NULL);
Index: netinet/ip_ipip.c
===
RCS file: /d/cvs/src/sys/netinet/ip_ipip.c,v
retrieving revision 1.71
diff -u -p -r1.71 ip_ipip.c
--- netinet/ip_ipip.c   29 Jan 2017 19:58:47 -  1.71
+++ netinet/ip_ipip.c   8 Mar 2017 10:20:44 -
@@ -84,7 +84,7 @@
  */
 int ipip_allow = 0;
 
-struct ipipstat ipipstat;
+struct cpumem *ipipcounters;
 
 /*
  * Really only a wrapper for ipip_input(), for use with pr_input.
@@ -95,7 +95,7 @@ ip4_input(struct mbuf **mp, int *offp, i
/* If we do not accept IP-in-IP explicitly, drop.  */
if (!ipip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
DPRINTF(("ip4_input(): dropped due to policy\n"));
-   ipipstat.ipips_pdrops++;
+   ipipstat_inc(ipips_pdrops);
m_freem(*mp);
return IPPROTO_DONE;
}
@@ -129,7 +129,7 @@ ipip_input(struct mbuf **mp, int *offp, 
u_int8_t v;
sa_family_t af;
 
-   ipipstat.ipips_ipackets++;
+   ipipstat_inc(ipips_ipackets);
 
m_copydata(m, 0, 1, &v);
 
@@ -143,7 +143,7 @@ ipip_input(struct mbuf **mp, int *offp, 
break;
 #endif
default:
-   ipipstat.ipips_family++;
+   ipipstat_inc(ipips_family);
m_freem(m);
return IPPROTO_DONE;
}
@@ -152,7 +152,7 @@ ipip_input(struct mbuf **mp, int *offp, 
if (m->m_len < hlen) {
if ((m = m_pullup(m, hlen)) == NULL) {
DPRINTF(("ipip_input(): m_pullup() failed\n"));
-   ipipstat.ipips_hdrops++;
+   ipipstat_inc(ipips_hdrops);
return IPPROTO_DONE;
}
}
@@ -179,7 +179,7 @@ ipip_input(struct mbuf **mp, int *offp, 
 
/* Sanity check */
if (m->m_pkthdr.len < sizeof(struct ip)) {
-   ipipstat.ipips_hdrops++;
+   ipipstat_inc(ipips_hdrops);
m_freem(m);
return IPPROTO_DONE;
}
@@ -195,7 +195,7 @@ ipip_input(struct mbuf **mp, int *offp, 
break;
 #endif
default:
-   ipipstat.ipips_family++;
+   ipipstat_inc(ipips_family);
m_freem(m);
return IPPROTO_DONE;
}
@@ -206,7 +206,7 @@ ipip_input(struct mbuf **mp, int *offp, 
if (m->m_len < hlen) {
if ((m = m_pullup(m, hlen)) == NULL) {
DPRINTF(("ipip_input(): m_pullup() failed\n"));
-   ipipstat.ipips_hdrops++;
+   ipipstat_inc(ipips_hdrops);
return IPPROTO_DONE;
}
}
@@ -229,7 +229,7 @@ ipip_input(struct mbuf **mp, int *offp, 
ECN_ALLOWED_IPSEC : ECN_ALLOWED;
if (!ip_ecn_egress(mode, &otos, &ipo->ip_tos)) {
DPRINTF(("ipip_input(): ip_ecn_egress() failed"));
-   ipipstat.ipips_pdrops++;
+   ipipstat_

Re: ip_ipip.c / gif(4) percpu counters

2017-03-08 Thread Martin Pieuchot
On 07/03/17(Tue) 18:16, Jeremie Courreges-Anglas wrote:
> [...] 
> 1. call ipip_init() through .pr_init.  The idea would be to call
>ipip_init() once per protosw entry that needs it, so the function
>should return early if it was already run.

Only one protosw entry uses ipip_sysctl, I'd put it there as well.



Re: ip_ipip.c / gif(4) percpu counters

2017-03-07 Thread Alexander Bluhm
On Tue, Mar 07, 2017 at 06:16:30PM +0100, Jeremie Courreges-Anglas wrote:
> 
> I failed to find a nice place where to initialize the counters.  The
> code that uses counters is reachable even if gif(4) isn't compiled in.
> 
> I can think of 3 obvious ways to call the init function.
> 
> 1. call ipip_init() through .pr_init.  The idea would be to call
>ipip_init() once per protosw entry that needs it, so the function
>should return early if it was already run.
> 
> 2. call ipip_init() from ip_init(), after all ip_init() is always
>compiled in.
> 
> 3. call ipip_init() from init_main.c.
> 
> The diff implements option 3, but what do you folks prefer?
> Thoughts / ok?

I would use option 2 as ip_ipip.c is always comiled in.  Then you
don't need the "if (ipipcounters == NULL)" check.

OK bluhm@

> 
> 
> Index: kern/init_main.c
> ===
> RCS file: /d/cvs/src/sys/kern/init_main.c,v
> retrieving revision 1.267
> diff -u -p -r1.267 init_main.c
> --- kern/init_main.c  6 Mar 2017 10:48:16 -   1.267
> +++ kern/init_main.c  7 Mar 2017 14:20:17 -
> @@ -147,6 +147,7 @@ void  taskq_init(void);
>  void timeout_proc_init(void);
>  void pool_gc_pages(void *);
>  void percpu_init(void);
> +void ipip_init(void);
>  
>  extern char sigcode[], esigcode[], sigcoderet[];
>  #ifdef SYSCALL_DEBUG
> @@ -364,6 +365,9 @@ main(void *framep)
>  
>   /* Per CPU memory allocation */
>   percpu_init();
> +
> + /* IP-in-IP memory allocation */
> + ipip_init();
>  
>   /* Initialize the file systems. */
>  #if defined(NFSSERVER) || defined(NFSCLIENT)
> Index: netinet/ip_ipip.c
> ===
> RCS file: /d/cvs/src/sys/netinet/ip_ipip.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 ip_ipip.c
> --- netinet/ip_ipip.c 29 Jan 2017 19:58:47 -  1.71
> +++ netinet/ip_ipip.c 7 Mar 2017 14:20:17 -
> @@ -84,7 +84,14 @@
>   */
>  int ipip_allow = 0;
>  
> -struct ipipstat ipipstat;
> +struct cpumem *ipipcounters;
> +
> +void
> +ipip_init(void)
> +{
> + if (ipipcounters == NULL)
> + ipipcounters = counters_alloc(ipips_ncounters);
> +}
>  
>  /*
>   * Really only a wrapper for ipip_input(), for use with pr_input.
> @@ -95,7 +102,7 @@ ip4_input(struct mbuf **mp, int *offp, i
>   /* If we do not accept IP-in-IP explicitly, drop.  */
>   if (!ipip_allow && ((*mp)->m_flags & (M_AUTH|M_CONF)) == 0) {
>   DPRINTF(("ip4_input(): dropped due to policy\n"));
> - ipipstat.ipips_pdrops++;
> + ipipstat_inc(ipips_pdrops);
>   m_freem(*mp);
>   return IPPROTO_DONE;
>   }
> @@ -129,7 +136,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   u_int8_t v;
>   sa_family_t af;
>  
> - ipipstat.ipips_ipackets++;
> + ipipstat_inc(ipips_ipackets);
>  
>   m_copydata(m, 0, 1, &v);
>  
> @@ -143,7 +150,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   break;
>  #endif
>   default:
> - ipipstat.ipips_family++;
> + ipipstat_inc(ipips_family);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -152,7 +159,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   if (m->m_len < hlen) {
>   if ((m = m_pullup(m, hlen)) == NULL) {
>   DPRINTF(("ipip_input(): m_pullup() failed\n"));
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   return IPPROTO_DONE;
>   }
>   }
> @@ -179,7 +186,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>  
>   /* Sanity check */
>   if (m->m_pkthdr.len < sizeof(struct ip)) {
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -195,7 +202,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   break;
>  #endif
>   default:
> - ipipstat.ipips_family++;
> + ipipstat_inc(ipips_family);
>   m_freem(m);
>   return IPPROTO_DONE;
>   }
> @@ -206,7 +213,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   if (m->m_len < hlen) {
>   if ((m = m_pullup(m, hlen)) == NULL) {
>   DPRINTF(("ipip_input(): m_pullup() failed\n"));
> - ipipstat.ipips_hdrops++;
> + ipipstat_inc(ipips_hdrops);
>   return IPPROTO_DONE;
>   }
>   }
> @@ -229,7 +236,7 @@ ipip_input(struct mbuf **mp, int *offp, 
>   ECN_ALLOWED_IPSEC : ECN_ALLOWED;
>   if (!ip_ecn_egress(mode, &otos, &ipo->ip_tos)) {
>   DPRINTF(("ipip_input(): ip_ecn_egress() failed"));
> - ipipstat.ipips_pdrops++;
> + ipipstat_inc(ipips_pdrops);
>   m_freem(m);
>