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.c 5 Feb 2017 16:23:38 -0000 1.295
> +++ netinet/ip_input.c 8 Mar 2017 10:19:34 -0000
> @@ -34,6 +34,7 @@
>
> #include "pf.h"
> #include "carp.h"
> +#include "gif.h"
>
> #include <sys/param.h>
> #include <sys/systm.h>
> @@ -78,6 +79,10 @@
> #include <netinet/ip_carp.h>
> #endif
>
> +#if NGIF > 0
> +#include <netinet/ip_ipip.h>
> +#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 -0000 1.71
> +++ netinet/ip_ipip.c 8 Mar 2017 10:20:44 -0000
> @@ -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++;
> + ipipstat_inc(ipips_pdrops);
> m_freem(m);
> return IPPROTO_DONE;
> }
> @@ -291,7 +291,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 +302,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 +336,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 +375,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 +384,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 +441,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 +461,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 +480,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 +518,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 +533,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 +545,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 +554,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 +592,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 +614,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 7 Mar 2017 14:20:17 -0000
> @@ -73,9 +73,39 @@ 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);
> +}
> +
> 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
