On Wed, Dec 15, 2021 at 07:21:46PM +0300, Vitaliy Makkoveev wrote: > The previous version of this diff exposed UAF issue we had after > tdb_delete(). bluhm@ fixed this and proposes to put per-cpu counters > diff again to tree. This is the updated diff to be against the resent > sources.
OK bluhm@ after tdb walker sleep has been fixed > Index: sys/net/pfkeyv2_convert.c > =================================================================== > RCS file: /cvs/src/sys/net/pfkeyv2_convert.c,v > retrieving revision 1.77 > diff -u -p -r1.77 pfkeyv2_convert.c > --- sys/net/pfkeyv2_convert.c 11 Dec 2021 16:33:46 -0000 1.77 > +++ sys/net/pfkeyv2_convert.c 15 Dec 2021 15:45:26 -0000 > @@ -963,18 +963,21 @@ export_satype(void **p, struct tdb *tdb) > void > export_counter(void **p, struct tdb *tdb) > { > + uint64_t counters[tdb_ncounters]; > struct sadb_x_counter *scnt = (struct sadb_x_counter *)*p; > > + counters_read(tdb->tdb_counters, counters, tdb_ncounters); > + > scnt->sadb_x_counter_len = sizeof(struct sadb_x_counter) / > sizeof(uint64_t); > scnt->sadb_x_counter_pad = 0; > - scnt->sadb_x_counter_ipackets = tdb->tdb_ipackets; > - scnt->sadb_x_counter_opackets = tdb->tdb_opackets; > - scnt->sadb_x_counter_ibytes = tdb->tdb_ibytes; > - scnt->sadb_x_counter_obytes = tdb->tdb_obytes; > - scnt->sadb_x_counter_idrops = tdb->tdb_idrops; > - scnt->sadb_x_counter_odrops = tdb->tdb_odrops; > - scnt->sadb_x_counter_idecompbytes = tdb->tdb_idecompbytes; > - scnt->sadb_x_counter_ouncompbytes = tdb->tdb_ouncompbytes; > + scnt->sadb_x_counter_ipackets = counters[tdb_ipackets]; > + scnt->sadb_x_counter_opackets = counters[tdb_opackets]; > + scnt->sadb_x_counter_ibytes = counters[tdb_ibytes]; > + scnt->sadb_x_counter_obytes = counters[tdb_obytes]; > + scnt->sadb_x_counter_idrops = counters[tdb_idrops]; > + scnt->sadb_x_counter_odrops = counters[tdb_odrops]; > + scnt->sadb_x_counter_idecompbytes = counters[tdb_idecompbytes]; > + scnt->sadb_x_counter_ouncompbytes = counters[tdb_ouncompbytes]; > *p += sizeof(struct sadb_x_counter); > } > Index: sys/netinet/ip_ah.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_ah.c,v > retrieving revision 1.169 > diff -u -p -r1.169 ip_ah.c > --- sys/netinet/ip_ah.c 11 Dec 2021 16:33:46 -0000 1.169 > +++ sys/netinet/ip_ah.c 15 Dec 2021 15:45:26 -0000 > @@ -608,7 +608,7 @@ ah_input(struct mbuf **mp, struct tdb *t > /* Update the counters. */ > ibytes = (m->m_pkthdr.len - skip - hl * sizeof(u_int32_t)); > tdb->tdb_cur_bytes += ibytes; > - tdb->tdb_ibytes += ibytes; > + tdbstat_add(tdb, tdb_ibytes, ibytes); > ahstat_add(ahs_ibytes, ibytes); > > /* Hard expiration. */ > Index: sys/netinet/ip_esp.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_esp.c,v > retrieving revision 1.189 > diff -u -p -r1.189 ip_esp.c > --- sys/netinet/ip_esp.c 11 Dec 2021 16:33:47 -0000 1.189 > +++ sys/netinet/ip_esp.c 15 Dec 2021 15:45:26 -0000 > @@ -420,7 +420,7 @@ esp_input(struct mbuf **mp, struct tdb * > > /* Update the counters */ > tdb->tdb_cur_bytes += plen; > - tdb->tdb_ibytes += plen; > + tdbstat_add(tdb, tdb_ibytes, plen); > espstat_add(esps_ibytes, plen); > > /* Hard expiration */ > Index: sys/netinet/ip_ipcomp.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_ipcomp.c,v > retrieving revision 1.89 > diff -u -p -r1.89 ip_ipcomp.c > --- sys/netinet/ip_ipcomp.c 11 Dec 2021 16:33:47 -0000 1.89 > +++ sys/netinet/ip_ipcomp.c 15 Dec 2021 15:45:26 -0000 > @@ -193,7 +193,7 @@ ipcomp_input(struct mbuf **mp, struct td > /* update the counters */ > ibytes = m->m_pkthdr.len - (skip + hlen); > tdb->tdb_cur_bytes += ibytes; > - tdb->tdb_ibytes += ibytes; > + tdbstat_add(tdb, tdb_ibytes, ibytes); > ipcompstat_add(ipcomps_ibytes, ibytes); > > /* Hard expiration */ > Index: sys/netinet/ip_ipsp.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_ipsp.c,v > retrieving revision 1.265 > diff -u -p -r1.265 ip_ipsp.c > --- sys/netinet/ip_ipsp.c 14 Dec 2021 17:50:37 -0000 1.265 > +++ sys/netinet/ip_ipsp.c 15 Dec 2021 15:45:26 -0000 > @@ -1062,6 +1062,9 @@ tdb_alloc(u_int rdomain) > tdbp->tdb_rdomain = rdomain; > tdbp->tdb_rdomain_post = rdomain; > > + /* Initialize counters. */ > + tdbp->tdb_counters = counters_alloc(tdb_ncounters); > + > /* Initialize timeouts. */ > timeout_set_proc(&tdbp->tdb_timer_tmo, tdb_timeout, tdbp); > timeout_set_proc(&tdbp->tdb_first_tmo, tdb_firstuse, tdbp); > @@ -1099,6 +1102,8 @@ tdb_free(struct tdb *tdbp) > tdbp->tdb_tag = 0; > } > #endif > + > + counters_free(tdbp->tdb_counters, tdb_ncounters); > > KASSERT(tdbp->tdb_onext == NULL); > KASSERT(tdbp->tdb_inext == NULL); > Index: sys/netinet/ip_ipsp.h > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_ipsp.h,v > retrieving revision 1.231 > diff -u -p -r1.231 ip_ipsp.h > --- sys/netinet/ip_ipsp.h 14 Dec 2021 17:50:37 -0000 1.231 > +++ sys/netinet/ip_ipsp.h 15 Dec 2021 15:45:26 -0000 > @@ -136,17 +136,6 @@ struct ipsecstat { > uint64_t ipsec_exctdb; /* TDBs with hardlimit excess */ > }; > > -struct tdb_data { > - uint64_t tdd_ipackets; /* Input IPsec packets */ > - uint64_t tdd_opackets; /* Output IPsec packets */ > - uint64_t tdd_ibytes; /* Input bytes */ > - uint64_t tdd_obytes; /* Output bytes */ > - uint64_t tdd_idrops; /* Dropped on input */ > - uint64_t tdd_odrops; /* Dropped on output */ > - uint64_t tdd_idecompbytes; /* Input bytes, decompressed */ > - uint64_t tdd_ouncompbytes; /* Output bytes, uncompressed */ > -}; > - > #ifdef _KERNEL > > #include <sys/timeout.h> > @@ -400,7 +389,8 @@ struct tdb { /* tunnel > descriptor blo > u_int64_t tdb_last_used; /* When was this SA last used */ > u_int64_t tdb_last_marked;/* Last SKIPCRYPTO status change */ > > - struct tdb_data tdb_data; /* stats about this TDB */ > + struct cpumem *tdb_counters; /* stats about this TDB */ > + > u_int64_t tdb_cryptoid; /* Crypto session ID */ > > u_int32_t tdb_spi; /* [I] SPI */ > @@ -446,15 +436,37 @@ struct tdb { /* tunnel > descriptor blo > TAILQ_HEAD(tdb_policy_head, ipsec_policy) tdb_policy_head; /* [p] */ > TAILQ_ENTRY(tdb) tdb_sync_entry; > }; > -#define tdb_ipackets tdb_data.tdd_ipackets > -#define tdb_opackets tdb_data.tdd_opackets > -#define tdb_ibytes tdb_data.tdd_ibytes > -#define tdb_obytes tdb_data.tdd_obytes > -#define tdb_idrops tdb_data.tdd_idrops > -#define tdb_odrops tdb_data.tdd_odrops > -#define tdb_idecompbytes tdb_data.tdd_idecompbytes > -#define tdb_ouncompbytes tdb_data.tdd_ouncompbytes > > +enum tdb_counters { > + tdb_ipackets, /* Input IPsec packets */ > + tdb_opackets, /* Output IPsec packets */ > + tdb_ibytes, /* Input bytes */ > + tdb_obytes, /* Output bytes */ > + tdb_idrops, /* Dropped on input */ > + tdb_odrops, /* Dropped on output */ > + tdb_idecompbytes, /* Input bytes, decompressed */ > + tdb_ouncompbytes, /* Output bytes, uncompressed */ > + tdb_ncounters > +}; > + > +static inline void > +tdbstat_inc(struct tdb *tdb, enum tdb_counters c) > +{ > + counters_inc(tdb->tdb_counters, c); > +} > + > +static inline void > +tdbstat_add(struct tdb *tdb, enum tdb_counters c, uint64_t v) > +{ > + counters_add(tdb->tdb_counters, c, v); > +} > + > +static inline void > +tdbstat_pkt(struct tdb *tdb, enum tdb_counters pc, enum tdb_counters bc, > + uint64_t bytes) > +{ > + counters_pkt(tdb->tdb_counters, pc, bc, bytes); > +} > > struct tdb_ident { > u_int32_t spi; > Index: sys/netinet/ip_output.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_output.c,v > retrieving revision 1.377 > diff -u -p -r1.377 ip_output.c > --- sys/netinet/ip_output.c 3 Dec 2021 17:18:34 -0000 1.377 > +++ sys/netinet/ip_output.c 15 Dec 2021 15:45:26 -0000 > @@ -662,7 +662,7 @@ ip_output_ipsec_send(struct tdb *tdb, st > error = ipsp_process_packet(m, tdb, AF_INET, 0); > if (error) { > ipsecstat_inc(ipsec_odrops); > - tdb->tdb_odrops++; > + tdbstat_inc(tdb, tdb_odrops); > } > if (ip_mtudisc && error == EMSGSIZE) > ip_output_ipsec_pmtu_update(tdb, ro, dst, rtableid, 0); > Index: sys/netinet/ipsec_input.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ipsec_input.c,v > retrieving revision 1.197 > diff -u -p -r1.197 ipsec_input.c > --- sys/netinet/ipsec_input.c 8 Dec 2021 14:24:18 -0000 1.197 > +++ sys/netinet/ipsec_input.c 15 Dec 2021 15:45:26 -0000 > @@ -340,8 +340,7 @@ ipsec_common_input(struct mbuf **mp, int > } > } > > - tdbp->tdb_ipackets++; > - tdbp->tdb_ibytes += m->m_pkthdr.len; > + tdbstat_pkt(tdbp, tdb_ipackets, tdb_ibytes, m->m_pkthdr.len); > > /* > * Call appropriate transform and return -- callback takes care of > @@ -350,7 +349,7 @@ ipsec_common_input(struct mbuf **mp, int > prot = (*(tdbp->tdb_xform->xf_input))(mp, tdbp, skip, protoff); > if (prot == IPPROTO_DONE) { > ipsecstat_inc(ipsec_idrops); > - tdbp->tdb_idrops++; > + tdbstat_inc(tdbp, tdb_idrops); > } > tdb_unref(tdbp); > return prot; > @@ -359,7 +358,7 @@ ipsec_common_input(struct mbuf **mp, int > m_freemp(mp); > ipsecstat_inc(ipsec_idrops); > if (tdbp != NULL) > - tdbp->tdb_idrops++; > + tdbstat_inc(tdbp, tdb_idrops); > tdb_unref(tdbp); > return IPPROTO_DONE; > } > @@ -537,7 +536,7 @@ ipsec_common_input_cb(struct mbuf **mp, > m->m_flags |= M_TUNNEL; > > ipsecstat_add(ipsec_idecompbytes, m->m_pkthdr.len); > - tdbp->tdb_idecompbytes += m->m_pkthdr.len; > + tdbstat_add(tdbp, tdb_idecompbytes, m->m_pkthdr.len); > > #if NBPFILTER > 0 > encif = enc_getif(tdbp->tdb_rdomain_post, tdbp->tdb_tap); > Index: sys/netinet/ipsec_output.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ipsec_output.c,v > retrieving revision 1.94 > diff -u -p -r1.94 ipsec_output.c > --- sys/netinet/ipsec_output.c 11 Dec 2021 16:33:47 -0000 1.94 > +++ sys/netinet/ipsec_output.c 15 Dec 2021 15:45:26 -0000 > @@ -367,7 +367,7 @@ ipsp_process_packet(struct mbuf *m, stru > } > > ipsecstat_add(ipsec_ouncompbytes, m->m_pkthdr.len); > - tdb->tdb_ouncompbytes += m->m_pkthdr.len; > + tdbstat_add(tdb, tdb_ouncompbytes, m->m_pkthdr.len); > > /* Non expansion policy for IPCOMP */ > if (tdb->tdb_sproto == IPPROTO_IPCOMP) { > @@ -507,8 +507,7 @@ ipsp_process_done(struct mbuf *m, struct > m_tag_prepend(m, mtag); > > ipsecstat_pkt(ipsec_opackets, ipsec_obytes, m->m_pkthdr.len); > - tdb->tdb_opackets++; > - tdb->tdb_obytes += m->m_pkthdr.len; > + tdbstat_pkt(tdb, tdb_opackets, tdb_obytes, m->m_pkthdr.len); > > /* If there's another (bundled) TDB to apply, do so. */ > tdbo = tdb_ref(tdb->tdb_onext); > Index: sys/netinet6/ip6_output.c > =================================================================== > RCS file: /cvs/src/sys/netinet6/ip6_output.c,v > retrieving revision 1.263 > diff -u -p -r1.263 ip6_output.c > --- sys/netinet6/ip6_output.c 3 Dec 2021 17:18:34 -0000 1.263 > +++ sys/netinet6/ip6_output.c 15 Dec 2021 15:45:26 -0000 > @@ -2875,7 +2875,7 @@ ip6_output_ipsec_send(struct tdb *tdb, s > rtableid, transportmode); > if (error) { > ipsecstat_inc(ipsec_odrops); > - tdb->tdb_odrops++; > + tdbstat_inc(tdb, tdb_odrops); > m_freem(m); > return error; > } > @@ -2897,7 +2897,7 @@ ip6_output_ipsec_send(struct tdb *tdb, s > error = ipsp_process_packet(m, tdb, AF_INET6, tunalready); > if (error) { > ipsecstat_inc(ipsec_odrops); > - tdb->tdb_odrops++; > + tdbstat_inc(tdb, tdb_odrops); > } > if (ip_mtudisc && error == EMSGSIZE) > ip6_output_ipsec_pmtu_update(tdb, ro, &dst, ifidx, rtableid, 0);