Re: per cpu counters for icmp
Jonathan Matthew writes: > This is much like the other per cpu counter conversions, except the counter > enum has gaps in it to match the arrays in struct icmpstat. I missed this mail and implemented basically the same diff. :-/ So your diff looks fine to me, the difference that I could spot are: - icmp_sysctl_icmpstat before icmp_sysctl to remove the need for a decl - I removed the 'if (newp != NULL)' in the ICMPCTL_STATS case, sysctl_rdstruct shoudl take care of that - declare icmpcounters before the definition of icmpstat_inc in icmp_var.h, that removes the need for the extern decl within said function - s/KASSERT/CTASSERT/ in icmp_sysctl_icmpstat I'd say that CTASSERT is a real improvement, the rest is cosmetic. -- jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: per cpu counters for icmp
> On 18 Nov 2016, at 20:11, Jonathan Matthew wrote:
>
> This is much like the other per cpu counter conversions, except the counter
> enum has gaps in it to match the arrays in struct icmpstat.
ok by me
>
> Index: icmp_var.h
> ===
> RCS file: /cvs/src/sys/netinet/icmp_var.h,v
> retrieving revision 1.14
> diff -u -p -u -p -r1.14 icmp_var.h
> --- icmp_var.h19 Jan 2014 05:01:50 - 1.14
> +++ icmp_var.h18 Nov 2016 09:02:48 -
> @@ -91,6 +91,32 @@ struct icmpstat {
> }
>
> #ifdef _KERNEL
> -extern structicmpstat icmpstat;
> +
> +#include
> +
> +enum icmpstat_counters {
> + icps_error,
> + icps_toofreq,
> + icps_oldshort,
> + icps_oldicmp,
> + icps_outhist,
> + icps_badcode = icps_outhist + ICMP_MAXTYPE + 1,
> + icps_tooshort,
> + icps_checksum,
> + icps_badlen,
> + icps_reflect,
> + icps_bmcastecho,
> + icps_inhist,
> + icps_ncounters = icps_inhist + ICMP_MAXTYPE + 1
> +};
> +
> +static inline void
> +icmpstat_inc(enum icmpstat_counters c)
> +{
> + extern struct cpumem *icmpcounters;
> + counters_inc(icmpcounters, c);
> +}
> +
> +extern struct cpumem *icmpcounters;
> #endif /* _KERNEL */
> #endif /* _NETINET_ICMP_VAR_H_ */
> Index: ip_icmp.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
> retrieving revision 1.156
> diff -u -p -u -p -r1.156 ip_icmp.c
> --- ip_icmp.c 16 Nov 2016 12:48:19 - 1.156
> +++ ip_icmp.c 18 Nov 2016 09:02:48 -
> @@ -121,7 +121,7 @@ static int icmperrpps_count = 0;
> static struct timeval icmperrppslim_last;
>
> static struct rttimer_queue *icmp_redirect_timeout_q = NULL;
> -struct icmpstat icmpstat;
> +struct cpumem *icmpcounters;
>
> int *icmpctl_vars[ICMPCTL_MAXID] = ICMPCTL_VARS;
>
> @@ -129,10 +129,12 @@ void icmp_mtudisc_timeout(struct rtentry
> int icmp_ratelimit(const struct in_addr *, const int, const int);
> void icmp_redirect_timeout(struct rtentry *, struct rttimer *);
> void icmp_input_if(struct ifnet *, struct mbuf *, int);
> +int icmp_sysctl_icmpstat(void *, size_t *, void *);
>
> void
> icmp_init(void)
> {
> + icmpcounters = counters_alloc(icps_ncounters, M_COUNTERS);
> /*
>* This is only useful if the user initializes redirtimeout to
>* something other than zero.
> @@ -157,7 +159,7 @@ icmp_do_error(struct mbuf *n, int type,
> printf("icmp_error(%x, %d, %d)\n", oip, type, code);
> #endif
> if (type != ICMP_REDIRECT)
> - icmpstat.icps_error++;
> + icmpstat_inc(icps_error);
> /*
>* Don't send error if not the first fragment of message.
>* Don't error if the old packet protocol was ICMP
> @@ -169,7 +171,7 @@ icmp_do_error(struct mbuf *n, int type,
> n->m_len >= oiplen + ICMP_MINLEN &&
> !ICMP_INFOTYPE(((struct icmp *)
> ((caddr_t)oip + oiplen))->icmp_type)) {
> - icmpstat.icps_oldicmp++;
> + icmpstat_inc(icps_oldicmp);
> goto freeit;
> }
> /* Don't send error in response to a multicast or broadcast packet */
> @@ -180,7 +182,7 @@ icmp_do_error(struct mbuf *n, int type,
>* First, do a rate limitation check.
>*/
> if (icmp_ratelimit(&oip->ip_src, type, code)) {
> - icmpstat.icps_toofreq++;
> + icmpstat_inc(icps_toofreq);
> goto freeit;
> }
>
> @@ -227,7 +229,7 @@ icmp_do_error(struct mbuf *n, int type,
> icp = mtod(m, struct icmp *);
> if ((u_int)type > ICMP_MAXTYPE)
> panic("icmp_error");
> - icmpstat.icps_outhist[type]++;
> + icmpstat_inc(icps_outhist + type);
> icp->icmp_type = type;
> if (type == ICMP_REDIRECT)
> icp->icmp_gwaddr.s_addr = dest;
> @@ -351,17 +353,17 @@ icmp_input_if(struct ifnet *ifp, struct
> }
> #endif
> if (icmplen < ICMP_MINLEN) {
> - icmpstat.icps_tooshort++;
> + icmpstat_inc(icps_tooshort);
> goto freeit;
> }
> i = hlen + min(icmplen, ICMP_ADVLENMIN);
> if (m->m_len < i && (m = m_pullup(m, i)) == NULL) {
> - icmpstat.icps_tooshort++;
> + icmpstat_inc(icps_tooshort);
> return;
> }
> ip = mtod(m, struct ip *);
> if (in4_cksum(m, 0, hlen, icmplen)) {
> - icmpstat.icps_checksum++;
> + icmpstat_inc(icps_checksum);
> goto freeit;
> }
>
> @@ -399,7 +401,7 @@ icmp_input_if(struct ifnet *ifp, struct
> }
> }
> #endif /* NPF */
> - icmpstat.icps_inhist[icp->icmp_type]++;
> + icmpstat_inc(icps_inhist + icp->icmp_type);
> code = icp->icmp_code;
> switch (icp->icmp_type) {
>
> @@ -464,7 +466,7 @@ icmp_input_if(struct ifnet *ifp, struct
>*/
> if (icmplen < ICMP_ADVLENMIN ||
per cpu counters for icmp
This is much like the other per cpu counter conversions, except the counter
enum has gaps in it to match the arrays in struct icmpstat.
Index: icmp_var.h
===
RCS file: /cvs/src/sys/netinet/icmp_var.h,v
retrieving revision 1.14
diff -u -p -u -p -r1.14 icmp_var.h
--- icmp_var.h 19 Jan 2014 05:01:50 - 1.14
+++ icmp_var.h 18 Nov 2016 09:02:48 -
@@ -91,6 +91,32 @@ struct icmpstat {
}
#ifdef _KERNEL
-extern struct icmpstat icmpstat;
+
+#include
+
+enum icmpstat_counters {
+ icps_error,
+ icps_toofreq,
+ icps_oldshort,
+ icps_oldicmp,
+ icps_outhist,
+ icps_badcode = icps_outhist + ICMP_MAXTYPE + 1,
+ icps_tooshort,
+ icps_checksum,
+ icps_badlen,
+ icps_reflect,
+ icps_bmcastecho,
+ icps_inhist,
+ icps_ncounters = icps_inhist + ICMP_MAXTYPE + 1
+};
+
+static inline void
+icmpstat_inc(enum icmpstat_counters c)
+{
+ extern struct cpumem *icmpcounters;
+ counters_inc(icmpcounters, c);
+}
+
+extern struct cpumem *icmpcounters;
#endif /* _KERNEL */
#endif /* _NETINET_ICMP_VAR_H_ */
Index: ip_icmp.c
===
RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.156
diff -u -p -u -p -r1.156 ip_icmp.c
--- ip_icmp.c 16 Nov 2016 12:48:19 - 1.156
+++ ip_icmp.c 18 Nov 2016 09:02:48 -
@@ -121,7 +121,7 @@ static int icmperrpps_count = 0;
static struct timeval icmperrppslim_last;
static struct rttimer_queue *icmp_redirect_timeout_q = NULL;
-struct icmpstat icmpstat;
+struct cpumem *icmpcounters;
int *icmpctl_vars[ICMPCTL_MAXID] = ICMPCTL_VARS;
@@ -129,10 +129,12 @@ void icmp_mtudisc_timeout(struct rtentry
int icmp_ratelimit(const struct in_addr *, const int, const int);
void icmp_redirect_timeout(struct rtentry *, struct rttimer *);
void icmp_input_if(struct ifnet *, struct mbuf *, int);
+int icmp_sysctl_icmpstat(void *, size_t *, void *);
void
icmp_init(void)
{
+ icmpcounters = counters_alloc(icps_ncounters, M_COUNTERS);
/*
* This is only useful if the user initializes redirtimeout to
* something other than zero.
@@ -157,7 +159,7 @@ icmp_do_error(struct mbuf *n, int type,
printf("icmp_error(%x, %d, %d)\n", oip, type, code);
#endif
if (type != ICMP_REDIRECT)
- icmpstat.icps_error++;
+ icmpstat_inc(icps_error);
/*
* Don't send error if not the first fragment of message.
* Don't error if the old packet protocol was ICMP
@@ -169,7 +171,7 @@ icmp_do_error(struct mbuf *n, int type,
n->m_len >= oiplen + ICMP_MINLEN &&
!ICMP_INFOTYPE(((struct icmp *)
((caddr_t)oip + oiplen))->icmp_type)) {
- icmpstat.icps_oldicmp++;
+ icmpstat_inc(icps_oldicmp);
goto freeit;
}
/* Don't send error in response to a multicast or broadcast packet */
@@ -180,7 +182,7 @@ icmp_do_error(struct mbuf *n, int type,
* First, do a rate limitation check.
*/
if (icmp_ratelimit(&oip->ip_src, type, code)) {
- icmpstat.icps_toofreq++;
+ icmpstat_inc(icps_toofreq);
goto freeit;
}
@@ -227,7 +229,7 @@ icmp_do_error(struct mbuf *n, int type,
icp = mtod(m, struct icmp *);
if ((u_int)type > ICMP_MAXTYPE)
panic("icmp_error");
- icmpstat.icps_outhist[type]++;
+ icmpstat_inc(icps_outhist + type);
icp->icmp_type = type;
if (type == ICMP_REDIRECT)
icp->icmp_gwaddr.s_addr = dest;
@@ -351,17 +353,17 @@ icmp_input_if(struct ifnet *ifp, struct
}
#endif
if (icmplen < ICMP_MINLEN) {
- icmpstat.icps_tooshort++;
+ icmpstat_inc(icps_tooshort);
goto freeit;
}
i = hlen + min(icmplen, ICMP_ADVLENMIN);
if (m->m_len < i && (m = m_pullup(m, i)) == NULL) {
- icmpstat.icps_tooshort++;
+ icmpstat_inc(icps_tooshort);
return;
}
ip = mtod(m, struct ip *);
if (in4_cksum(m, 0, hlen, icmplen)) {
- icmpstat.icps_checksum++;
+ icmpstat_inc(icps_checksum);
goto freeit;
}
@@ -399,7 +401,7 @@ icmp_input_if(struct ifnet *ifp, struct
}
}
#endif /* NPF */
- icmpstat.icps_inhist[icp->icmp_type]++;
+ icmpstat_inc(icps_inhist + icp->icmp_type);
code = icp->icmp_code;
switch (icp->icmp_type) {
@@ -464,7 +466,7 @@ icmp_input_if(struct ifnet *ifp, struct
*/
if (icmplen < ICMP_ADVLENMIN || icmplen < ICMP_ADVLEN(icp) ||
icp->icmp_ip.ip_hl < (sizeof(struct ip) >> 2)) {
- icmpstat.icps_badlen++;
+ icmpstat_inc(icps_badlen)
