Martin Pieuchot <m...@openbsd.org> 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 -0000       1.74
+++ netinet/in_proto.c  8 Mar 2017 11:56:45 -0000
@@ -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 -0000      1.71
+++ netinet/ip_ipip.c   8 Mar 2017 11:59:04 -0000
@@ -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(ECN_ALLOWED, &otos, &itos)) {
                        DPRINTF(("ipip_input(): ip_ecn_egress() failed"));
-                       ipipstat.ipips_pdrops++;
+                       ipipstat_inc(ipips_pdrops);
                        m_freem(m);
                        return IPPROTO_DONE;
                }
@@ -291,7 +297,7 @@ ipip_input(struct mbuf **mp, int *offp, 
                rt = rtalloc((struct sockaddr *)&ss, 0,
                    m->m_pkthdr.ph_rtableid);
                if ((rt != NULL) && (rt->rt_flags & RTF_LOCAL)) {
-                       ipipstat.ipips_spoof++;
+                       ipipstat_inc(ipips_spoof);
                        m_freem(m);
                        rtfree(rt);
                        return IPPROTO_DONE;
@@ -302,7 +308,7 @@ ipip_input(struct mbuf **mp, int *offp, 
        }
 
        /* Statistics */
-       ipipstat.ipips_ibytes += m->m_pkthdr.len - iphlen;
+       ipipstat_add(ipips_ibytes, m->m_pkthdr.len - iphlen);
 
        /*
         * Interface pointer stays the same; if no IPsec processing has
@@ -336,7 +342,7 @@ ipip_input(struct mbuf **mp, int *offp, 
 #endif
 
        if (niq_enqueue(ifq, m) != 0) {
-               ipipstat.ipips_qfull++;
+               ipipstat_inc(ipips_qfull);
                DPRINTF(("ipip_input(): packet dropped because of full "
                    "queue\n"));
        }
@@ -375,7 +381,7 @@ ipip_output(struct mbuf *m, struct tdb *
                            ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
                            ntohl(tdb->tdb_spi)));
 
-                       ipipstat.ipips_unspec++;
+                       ipipstat_inc(ipips_unspec);
                        m_freem(m);
                        *mp = NULL;
                        return EINVAL;
@@ -384,7 +390,7 @@ ipip_output(struct mbuf *m, struct tdb *
                M_PREPEND(m, sizeof(struct ip), M_DONTWAIT);
                if (m == NULL) {
                        DPRINTF(("ipip_output(): M_PREPEND failed\n"));
-                       ipipstat.ipips_hdrops++;
+                       ipipstat_inc(ipips_hdrops);
                        *mp = NULL;
                        return ENOBUFS;
                }
@@ -441,7 +447,7 @@ ipip_output(struct mbuf *m, struct tdb *
                else {
                        m_freem(m);
                        *mp = NULL;
-                       ipipstat.ipips_family++;
+                       ipipstat_inc(ipips_family);
                        return EAFNOSUPPORT;
                }
 
@@ -461,7 +467,7 @@ ipip_output(struct mbuf *m, struct tdb *
                            ipsp_address(&tdb->tdb_dst, buf, sizeof(buf)),
                            ntohl(tdb->tdb_spi)));
 
-                       ipipstat.ipips_unspec++;
+                       ipipstat_inc(ipips_unspec);
                        m_freem(m);
                        *mp = NULL;
                        return ENOBUFS;
@@ -480,7 +486,7 @@ ipip_output(struct mbuf *m, struct tdb *
                M_PREPEND(m, sizeof(struct ip6_hdr), M_DONTWAIT);
                if (m == NULL) {
                        DPRINTF(("ipip_output(): M_PREPEND failed\n"));
-                       ipipstat.ipips_hdrops++;
+                       ipipstat_inc(ipips_hdrops);
                        *mp = NULL;
                        return ENOBUFS;
                }
@@ -518,7 +524,7 @@ ipip_output(struct mbuf *m, struct tdb *
                        } else {
                                m_freem(m);
                                *mp = NULL;
-                               ipipstat.ipips_family++;
+                               ipipstat_inc(ipips_family);
                                return EAFNOSUPPORT;
                        }
 
@@ -533,11 +539,11 @@ ipip_output(struct mbuf *m, struct tdb *
                    tdb->tdb_dst.sa.sa_family));
                m_freem(m);
                *mp = NULL;
-               ipipstat.ipips_family++;
+               ipipstat_inc(ipips_family);
                return EAFNOSUPPORT;
        }
 
-       ipipstat.ipips_opackets++;
+       ipipstat_inc(ipips_opackets);
        *mp = m;
 
        if (tdb->tdb_dst.sa.sa_family == AF_INET) {
@@ -545,7 +551,7 @@ ipip_output(struct mbuf *m, struct tdb *
                        tdb->tdb_cur_bytes +=
                            m->m_pkthdr.len - sizeof(struct ip);
 
-               ipipstat.ipips_obytes += m->m_pkthdr.len - sizeof(struct ip);
+               ipipstat_add(ipips_obytes, m->m_pkthdr.len - sizeof(struct ip));
        }
 
 #ifdef INET6
@@ -554,8 +560,8 @@ ipip_output(struct mbuf *m, struct tdb *
                        tdb->tdb_cur_bytes +=
                            m->m_pkthdr.len - sizeof(struct ip6_hdr);
 
-               ipipstat.ipips_obytes +=
-                   m->m_pkthdr.len - sizeof(struct ip6_hdr);
+               ipipstat_add(ipips_obytes,
+                   m->m_pkthdr.len - sizeof(struct ip6_hdr));
        }
 #endif /* INET6 */
 
@@ -592,6 +598,17 @@ ipe4_input(struct mbuf *m, int hlen, int
 #endif /* IPSEC */
 
 int
+ipip_sysctl_ipipstat(void *oldp, size_t *oldlenp, void *newp)
+{
+       struct ipipstat ipipstat;
+
+       CTASSERT(sizeof(ipipstat) == (ipips_ncounters * sizeof(uint64_t)));
+       counters_read(ipipcounters, (uint64_t *)&ipipstat, ipips_ncounters);
+       return (sysctl_rdstruct(oldp, oldlenp, newp,
+           &ipipstat, sizeof(ipipstat)));
+}
+
+int
 ipip_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
     size_t newlen)
 {
@@ -603,10 +620,7 @@ ipip_sysctl(int *name, u_int namelen, vo
        case IPIPCTL_ALLOW:
                return (sysctl_int(oldp, oldlenp, newp, newlen, &ipip_allow));
        case IPIPCTL_STATS:
-               if (newp != NULL)
-                       return (EPERM);
-               return (sysctl_struct(oldp, oldlenp, newp, newlen,
-                   &ipipstat, sizeof(ipipstat)));
+               return (ipip_sysctl_ipipstat(oldp, oldlenp, newp));
        default:
                return (ENOPROTOOPT);
        }
Index: netinet/ip_ipip.h
===================================================================
RCS file: /d/cvs/src/sys/netinet/ip_ipip.h,v
retrieving revision 1.7
diff -u -p -r1.7 ip_ipip.h
--- netinet/ip_ipip.h   20 Feb 2017 17:04:25 -0000      1.7
+++ netinet/ip_ipip.h   8 Mar 2017 11:57:46 -0000
@@ -73,9 +73,40 @@ struct ipipstat {
 }
 
 #ifdef _KERNEL
+
+#include <sys/percpu.h>
+
+enum ipipstat_counters {
+       ipips_ipackets,
+       ipips_opackets,
+       ipips_hdrops,
+       ipips_qfull,
+       ipips_ibytes,
+       ipips_obytes,
+       ipips_pdrops,
+       ipips_spoof,
+       ipips_family,
+       ipips_unspec,
+       ipips_ncounters
+};
+
+extern struct cpumem *ipipcounters;
+
+static inline void
+ipipstat_inc(enum ipipstat_counters c)
+{
+       counters_inc(ipipcounters, c);
+}
+
+static inline void
+ipipstat_add(enum ipipstat_counters c, uint64_t v)
+{
+       counters_add(ipipcounters, c, v);
+}
+
+void   ipip_init(void);
 int    ipip_sysctl(int *, u_int, void *, size_t *, void *, size_t);
 
 extern int ipip_allow;
-extern struct ipipstat ipipstat;
 #endif /* _KERNEL */
 #endif /* _NETINET_IPIP_H_ */


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to