From 46b6947f1eb30bebf01d1a36f9b112b9b6b41c11 Mon Sep 17 00:00:00 2001 From: Heikki Hannikainen Date: Sun, 30 Oct 2022 19:21:27 +0200 Subject: [PATCH] accounting: Optimize CPU use of tx/rx accounting counters TX counters are updated much more often than RX counters. No need to bump RX counters for every outgoing packet, and no need to bump TX counters for every incoming packet. Split them out. --- src/accept.c | 2 +- src/incoming.c | 2 +- src/outgoing.c | 6 ++--- src/sctp.c | 2 +- src/worker.c | 70 ++++++++++++++++++++++++++++++++++++++------------ src/worker.h | 3 ++- 6 files changed, 62 insertions(+), 23 deletions(-) diff --git a/src/accept.c b/src/accept.c index 6519d30..73734d0 100644 --- a/src/accept.c +++ b/src/accept.c @@ -702,7 +702,7 @@ static void accept_process_udpsubmit(struct listen_t *l, char *buf, int len, cha udp_pseudoclient->portaccount = l->portaccount; e = pseudoclient_push_packet(udp_worker, udp_pseudoclient, username, packet, packet_len); - clientaccount_add(udp_pseudoclient, IPPROTO_UDP, len, 1, 0, 0, (e < 0) ? e : 0, 0); + clientaccount_add_rx(udp_pseudoclient, IPPROTO_UDP, len, 1, (e < 0) ? e : 0, 0); udp_pseudoclient->portaccount = NULL; if (e < 0) diff --git a/src/incoming.c b/src/incoming.c index 67f3f9b..a76d842 100644 --- a/src/incoming.c +++ b/src/incoming.c @@ -1189,7 +1189,7 @@ in_drop: /* Account the one incoming packet. * Incoming bytes were already accounted earlier. */ - clientaccount_add(c, l4proto, 0, 1, 0, 0, (e < 0) ? e : 0, 0); + clientaccount_add_rx(c, l4proto, 0, 1, (e < 0) ? e : 0, 0); return 0; } diff --git a/src/outgoing.c b/src/outgoing.c index f304c0e..94006d4 100644 --- a/src/outgoing.c +++ b/src/outgoing.c @@ -31,9 +31,9 @@ static inline void send_single(struct worker_t *self, struct client_t *c, char * * its TCP or SCTP or something. */ if (c->udp_port && c->udpclient) - clientaccount_add( c, IPPROTO_UDP, 0, 0, 0, 1, 0, 0); + clientaccount_add_tx( c, IPPROTO_UDP, 0, 1); else - clientaccount_add( c, c->ai_protocol, 0, 0, 0, 1, 0, 0); + clientaccount_add_tx( c, c->ai_protocol, 0, 1); c->write(self, c, data, len); } @@ -79,7 +79,7 @@ static void process_outgoing_single(struct worker_t *self, struct pbuf_t *pb) /* OPTIMIZE: we walk through all clients for each dupe - how to find it quickly? */ for (c = self->clients; (c); c = c->next) { if (c == origin) { - clientaccount_add(c, -1, 0, 0, 0, 0, 0, 1); + clientaccount_add_rx(c, -1, 0, 0, 0, 1); break; } } diff --git a/src/sctp.c b/src/sctp.c index 1822af2..4ba008b 100644 --- a/src/sctp.c +++ b/src/sctp.c @@ -286,7 +286,7 @@ int sctp_client_write(struct worker_t *self, struct client_t *c, char *p, int le c->obuf_writes++; if (client_buffer_outgoing_data(self, c, p, len) == -12) return -12; - clientaccount_add( c, IPPROTO_SCTP, 0, 0, len, 0, 0, 0); + clientaccount_add_tx(c, IPPROTO_SCTP, len, 0); if (c->obuf_writes > obuf_writes_threshold) { // Lots and lots of writes, switch to buffering... if (c->obuf_flushsize == 0) { diff --git a/src/worker.c b/src/worker.c index 7fc6fe7..4384777 100644 --- a/src/worker.c +++ b/src/worker.c @@ -660,7 +660,7 @@ char *hexsockaddr(const struct sockaddr *sa, const int addr_len) return hstrdup(eb); } -void clientaccount_add(struct client_t *c, int l4proto, int rxbytes, int rxpackets, int txbytes, int txpackets, int rxerr, int rxdupes) +void clientaccount_add_rx(struct client_t *c, int l4proto, int rxbytes, int rxpackets, int rxerr, int rxdupes) { struct portaccount_t *pa = NULL; int rxdrops = 0; @@ -674,9 +674,7 @@ void clientaccount_add(struct client_t *c, int l4proto, int rxbytes, int rxpacke /* worker local accounters do not need locks */ c->localaccount.rxbytes += rxbytes; - c->localaccount.txbytes += txbytes; c->localaccount.rxpackets += rxpackets; - c->localaccount.txpackets += txpackets; c->localaccount.rxdupes += rxdupes; if (rxdrops) { c->localaccount.rxdrops += 1; @@ -692,9 +690,7 @@ void clientaccount_add(struct client_t *c, int l4proto, int rxbytes, int rxpacke if (pa) { #ifdef HAVE_SYNC_FETCH_AND_ADD __sync_fetch_and_add(&pa->rxbytes, rxbytes); - __sync_fetch_and_add(&pa->txbytes, txbytes); __sync_fetch_and_add(&pa->rxpackets, rxpackets); - __sync_fetch_and_add(&pa->txpackets, txpackets); __sync_fetch_and_add(&pa->rxdupes, rxdupes); if (rxdrops) { __sync_fetch_and_add(&pa->rxdrops, 1); @@ -703,9 +699,7 @@ void clientaccount_add(struct client_t *c, int l4proto, int rxbytes, int rxpacke #else // FIXME: MUTEX !! -- this may or may not need locks.. pa->rxbytes += rxbytes; - pa->txbytes += txbytes; pa->rxpackets += rxpackets; - pa->txpackets += txpackets; pa->rxdupes += rxdupes; if (rxdrops) { pa->rxdrops += 1; @@ -729,9 +723,7 @@ void clientaccount_add(struct client_t *c, int l4proto, int rxbytes, int rxpacke #ifdef HAVE_SYNC_FETCH_AND_ADD __sync_fetch_and_add(&proto->rxbytes, rxbytes); - __sync_fetch_and_add(&proto->txbytes, txbytes); __sync_fetch_and_add(&proto->rxpackets, rxpackets); - __sync_fetch_and_add(&proto->txpackets, txpackets); if (rxdrops) { __sync_fetch_and_add(&proto->rxdrops, 1); __sync_fetch_and_add(&proto->rxerrs[rxerr], 1); @@ -739,9 +731,7 @@ void clientaccount_add(struct client_t *c, int l4proto, int rxbytes, int rxpacke #else // FIXME: MUTEX !! -- this may or may not need locks.. proto->rxbytes += rxbytes; - proto->txbytes += txbytes; proto->rxpackets += rxpackets; - proto->txpackets += txpackets; if (rxdrops) { proto->rxdrops += 1; proto->rxerrs[rxerr] += 1; @@ -749,6 +739,54 @@ void clientaccount_add(struct client_t *c, int l4proto, int rxbytes, int rxpacke #endif } +void clientaccount_add_tx(struct client_t *c, int l4proto, int txbytes, int txpackets) +{ + struct portaccount_t *pa = NULL; + + /* worker local accounters do not need locks */ + c->localaccount.txbytes += txbytes; + c->localaccount.txpackets += txpackets; + + if (l4proto == IPPROTO_UDP && c->udpclient && c->udpclient->portaccount) { + pa = c->udpclient->portaccount; + } else if (c->portaccount) { + pa = c->portaccount; + } + + if (pa) { +#ifdef HAVE_SYNC_FETCH_AND_ADD + __sync_fetch_and_add(&pa->txbytes, txbytes); + __sync_fetch_and_add(&pa->txpackets, txpackets); +#else + // FIXME: MUTEX !! -- this may or may not need locks.. + pa->txbytes += txbytes; + pa->txpackets += txpackets; +#endif + } + + struct portaccount_t *proto; + + if (l4proto == IPPROTO_TCP) + proto = &client_connects_tcp; + else if (l4proto == IPPROTO_UDP) + proto = &client_connects_udp; +#ifdef USE_SCTP + else if (l4proto == IPPROTO_SCTP) + proto = &client_connects_sctp; +#endif + else + return; + +#ifdef HAVE_SYNC_FETCH_AND_ADD + __sync_fetch_and_add(&proto->txbytes, txbytes); + __sync_fetch_and_add(&proto->txpackets, txpackets); +#else + // FIXME: MUTEX !! -- this may or may not need locks.. + proto->txbytes += txbytes; + proto->txpackets += txpackets; +#endif +} + static const char *protocol_str(struct client_t *c) { static const char unknown[] = "UNKNOWN-PROTOCOL"; @@ -929,7 +967,7 @@ int udp_client_write(struct worker_t *self, struct client_t *c, char *p, int len // hlog( LOG_DEBUG, "UDP from %d to client port %d, sendto rc=%d", c->udpclient->portnum, c->udp_port, i ); if (i > 0) - clientaccount_add( c, IPPROTO_UDP, 0, 0, i, 0, 0, 0); + clientaccount_add_tx( c, IPPROTO_UDP, i, 0); return i; } @@ -990,7 +1028,7 @@ static int ssl_client_write(struct worker_t *self, struct client_t *c, char *p, c->obuf_writes++; if (len > 0) - clientaccount_add( c, c->ai_protocol, 0, 0, len, 0, 0, 0); + clientaccount_add_tx( c, c->ai_protocol, len, 0); if (client_buffer_outgoing_data(self, c, p, len) == -12) return -12; @@ -1029,7 +1067,7 @@ static int tcp_client_write(struct worker_t *self, struct client_t *c, char *p, * will be incremented only when we actually transmit a packet * instead of a keepalive. */ - clientaccount_add( c, c->ai_protocol, 0, 0, len, 0, 0, 0); + clientaccount_add_tx( c, c->ai_protocol, len, 0); } if (client_buffer_outgoing_data(self, c, p, len) == -12) @@ -1195,7 +1233,7 @@ static int handle_corepeer_readable(struct worker_t *self, struct client_t *c) hlog(LOG_DEBUG, "worker thread passing UDP packet from %s to handler: %*s", addrs, r, c->ibuf); hfree(addrs); */ - clientaccount_add( rc, IPPROTO_UDP, r, 0, 0, 0, 0, 0); /* Account byte count. incoming_handler() will account packets. */ + clientaccount_add_rx(rc, IPPROTO_UDP, r, 0, 0, 0); /* Account byte count. incoming_handler() will account packets. */ rc->last_read = tick; /* Ignore CRs and LFs in UDP input packet - the current core peer system puts 1 APRS packet in each @@ -1256,7 +1294,7 @@ static int deframe_aprsis_input_lines(struct worker_t *self, struct client_t *c) int client_postread(struct worker_t *self, struct client_t *c, int r) { - clientaccount_add(c, c->ai_protocol, r, 0, 0, 0, 0, 0); /* Number of packets is now unknown, + clientaccount_add_rx(c, c->ai_protocol, r, 0, 0, 0); /* Number of packets is now unknown, byte count is collected. The incoming_handler() will account packets. */ diff --git a/src/worker.h b/src/worker.h index 2f2f3ed..2d4a2e4 100644 --- a/src/worker.h +++ b/src/worker.h @@ -523,7 +523,8 @@ extern void port_accounter_drop(struct portaccount_t *p); extern char *strsockaddr(const struct sockaddr *sa, const int addr_len); extern char *hexsockaddr(const struct sockaddr *sa, const int addr_len); -extern void clientaccount_add(struct client_t *c, int l4proto, int rxbytes, int rxpackets, int txbytes, int txpackets, int rxerr, int rxdupes); +extern void clientaccount_add_rx(struct client_t *c, int l4proto, int rxbytes, int rxpackets, int rxerr, int rxdupes); +extern void clientaccount_add_tx(struct client_t *c, int l4proto, int txbytes, int txpackets); extern void json_add_rxerrs(cJSON *root, const char *key, long long vals[]); extern int worker_client_list(cJSON *workers, cJSON *clients, cJSON *uplinks, cJSON *peers, cJSON *totals, cJSON *memory);