From 765a97e681bab60c26e1ad6e520a791c8733a82c Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Wed, 30 Aug 2023 11:06:11 -0600 Subject: [PATCH 1/9] Reduce severity of joold's desync message Weird. According to the output, Jool upgraded the error to kernel WARN() because joold employed log_err() during a soft irq, which is supposed to break log_err()'s contract. But the relevant code is a userspace request handler. I'm going to have to research this more. Also, the error message was too long, and log_err() was truncating it to 256 characters. log_warn_once() fixes that for now. Also raise Jool's version, so suffieldacademy can more easily tell what they're running. --- src/common/xlat.h | 2 +- src/mod/common/joold.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/xlat.h b/src/common/xlat.h index 8c3c6b10..a41f9938 100644 --- a/src/common/xlat.h +++ b/src/common/xlat.h @@ -10,7 +10,7 @@ #define JOOL_VERSION_MAJOR 4 #define JOOL_VERSION_MINOR 1 #define JOOL_VERSION_REV 10 -#define JOOL_VERSION_DEV 0 +#define JOOL_VERSION_DEV 1 /** See http://stackoverflow.com/questions/195975 */ #define STR_VALUE(arg) #arg diff --git a/src/mod/common/joold.c b/src/mod/common/joold.c index 0df30490..ba391916 100644 --- a/src/mod/common/joold.c +++ b/src/mod/common/joold.c @@ -353,7 +353,7 @@ static enum session_fate collision_cb(struct session_entry *old, void *arg) return FATE_TIMER_SLOW; } - log_err("We're out of sync: Incoming session entry " SEPP + log_warn_once("We're out of sync: Incoming session entry " SEPP " collides with DB entry " SEPP ".", SEPA(new), SEPA(old)); params->success = false; From 3d5c94da45ed37c7c8598ba7826491242c44bdff Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Wed, 30 Aug 2023 17:15:33 -0600 Subject: [PATCH 2/9] Add misc stat counters for joold Might help monitor joold, as well as debug #410. Print them with jool stat display --all | grep JSTAT_JOOLD --- src/common/stats.h | 16 +++++++++++++++ src/common/xlat.h | 2 +- src/mod/common/joold.c | 45 ++++++++++++++++++++++++++++++++++-------- src/usr/nl/stats.c | 17 ++++++++++++++++ 4 files changed, 71 insertions(+), 9 deletions(-) diff --git a/src/common/stats.h b/src/common/stats.h index ed05d389..b93df96b 100644 --- a/src/common/stats.h +++ b/src/common/stats.h @@ -93,6 +93,22 @@ enum jool_stat_id { JSTAT_ICMPEXT_BIG, + JSTAT_JOOLD_EMPTY, + JSTAT_JOOLD_TIMEOUT, + JSTAT_JOOLD_MISSING_ACK, + JSTAT_JOOLD_AD_ONGOING, + JSTAT_JOOLD_FLUSH_ASAP, + JSTAT_JOOLD_PKT_FULL, + JSTAT_JOOLD_QUEUING, + + JSTAT_JOOLD_SSS_QUEUED, + JSTAT_JOOLD_SSS_SENT, + JSTAT_JOOLD_SSS_RCVD, + JSTAT_JOOLD_PKT_SENT, + JSTAT_JOOLD_PKT_RCVD, + JSTAT_JOOLD_ADS, + JSTAT_JOOLD_ACKS, + /* These 3 need to be last, and in this order. */ JSTAT_UNKNOWN, /* "WTF was that" errors only. */ JSTAT_PADDING, diff --git a/src/common/xlat.h b/src/common/xlat.h index a41f9938..6a63b5e2 100644 --- a/src/common/xlat.h +++ b/src/common/xlat.h @@ -10,7 +10,7 @@ #define JOOL_VERSION_MAJOR 4 #define JOOL_VERSION_MINOR 1 #define JOOL_VERSION_REV 10 -#define JOOL_VERSION_DEV 1 +#define JOOL_VERSION_DEV 2 /** See http://stackoverflow.com/questions/195975 */ #define STR_VALUE(arg) #arg diff --git a/src/mod/common/joold.c b/src/mod/common/joold.c index ba391916..31c80446 100644 --- a/src/mod/common/joold.c +++ b/src/mod/common/joold.c @@ -119,23 +119,39 @@ static bool should_send(struct xlator *jool) queue = jool->nat64.joold; - if (queue->deferred.count == 0) + if (queue->deferred.count == 0) { + jstat_inc(jool->stats, JSTAT_JOOLD_EMPTY); return false; + } deadline = msecs_to_jiffies(GLOBALS(jool).flush_deadline); - if (time_before(queue->last_flush_time + deadline, jiffies)) + if (time_before(queue->last_flush_time + deadline, jiffies)) { + jstat_inc(jool->stats, JSTAT_JOOLD_TIMEOUT); return true; + } - if (!(queue->flags & JQF_ACK_RECEIVED)) + if (!(queue->flags & JQF_ACK_RECEIVED)) { + jstat_inc(jool->stats, JSTAT_JOOLD_MISSING_ACK); return false; + } - if (queue->flags & JQF_AD_ONGOING) + if (queue->flags & JQF_AD_ONGOING) { + jstat_inc(jool->stats, JSTAT_JOOLD_AD_ONGOING); + return true; + } + + if (GLOBALS(jool).flush_asap) { + jstat_inc(jool->stats, JSTAT_JOOLD_FLUSH_ASAP); return true; + } - if (GLOBALS(jool).flush_asap) + if (queue->deferred.count >= GLOBALS(jool).max_sessions_per_pkt) { + jstat_inc(jool->stats, JSTAT_JOOLD_PKT_FULL); return true; + } - return queue->deferred.count >= GLOBALS(jool).max_sessions_per_pkt; + jstat_inc(jool->stats, JSTAT_JOOLD_QUEUING); + return false; } static bool too_many_sessions(struct xlator *jool) @@ -209,7 +225,7 @@ static void send_to_userspace(struct xlator *jool, struct list_head *sessions) struct joolnlhdr *jhdr; struct nlattr *root; struct deferred_session *session; - unsigned int count; + int count; int error; if (list_empty(sessions)) @@ -244,6 +260,9 @@ static void send_to_userspace(struct xlator *jool, struct list_head *sessions) count++; } + jstat_add(jool->stats, JSTAT_JOOLD_SSS_SENT, count); + jstat_inc(jool->stats, JSTAT_JOOLD_PKT_SENT); + nla_nest_end(skb, root); genlmsg_end(skb, jhdr); sendpkt_multicast(jool, skb); @@ -333,6 +352,7 @@ void joold_add(struct xlator *jool, struct session_entry *_session) spin_unlock_bh(&queue->lock); send_to_userspace(jool, &prepared); + jstat_inc(jool->stats, JSTAT_JOOLD_SSS_QUEUED); } struct add_params { @@ -409,14 +429,21 @@ int joold_sync(struct xlator *jool, struct nlattr *root) { struct nlattr *attr; int rem; + int rcvd; bool success; if (joold_disabled(jool)) return -EINVAL; success = true; - nla_for_each_nested(attr, root, rem) + rcvd = 0; + nla_for_each_nested(attr, root, rem) { success &= add_new_session(jool, attr); + rcvd++; + } + + jstat_add(jool->stats, JSTAT_JOOLD_SSS_RCVD, rcvd); + jstat_inc(jool->stats, JSTAT_JOOLD_PKT_RCVD); __log_debug(jool, "Done."); return success ? 0 : -EINVAL; @@ -470,6 +497,7 @@ int joold_advertise(struct xlator *jool) spin_unlock_bh(&queue->lock); send_to_userspace(jool, &prepared); + jstat_inc(jool->stats, JSTAT_JOOLD_ADS); return 0; } @@ -490,6 +518,7 @@ void joold_ack(struct xlator *jool) spin_unlock_bh(&queue->lock); send_to_userspace(jool, &prepared); + jstat_inc(jool->stats, JSTAT_JOOLD_ACKS); } /** diff --git a/src/usr/nl/stats.c b/src/usr/nl/stats.c index 0b42bc6c..3b0c1218 100644 --- a/src/usr/nl/stats.c +++ b/src/usr/nl/stats.c @@ -89,6 +89,23 @@ static struct joolnl_stat_metadata const jstat_metadatas[] = { DEFINE_STAT(JSTAT_ICMP4ERR_SUCCESS, "ICMPv4 errors (created by Jool, not translated) sent successfully."), DEFINE_STAT(JSTAT_ICMP4ERR_FAILURE, "ICMPv4 errors (created by Jool, not translated) that could not be sent."), DEFINE_STAT(JSTAT_ICMPEXT_BIG, "Illegal ICMP header length. (Exceeds available payload in packet.)"), + + DEFINE_STAT(JSTAT_JOOLD_EMPTY, "Joold packet not sent; no sessions queued."), + DEFINE_STAT(JSTAT_JOOLD_TIMEOUT, "Joold packet sent; ss-flush-deadline reached."), + DEFINE_STAT(JSTAT_JOOLD_MISSING_ACK, "Joold packet not sent; still waiting for ACK."), + DEFINE_STAT(JSTAT_JOOLD_AD_ONGOING, "Joold packet sent; advertise still ongoing."), + DEFINE_STAT(JSTAT_JOOLD_FLUSH_ASAP, "Joold packet sent; ss-flush-asap enabled."), + DEFINE_STAT(JSTAT_JOOLD_PKT_FULL, "Joold packet sent; session packet full."), + DEFINE_STAT(JSTAT_JOOLD_QUEUING, "Joold packet not sent; packet still has room for more sessions."), + + DEFINE_STAT(JSTAT_JOOLD_SSS_QUEUED, "Joold: Total sessions queued."), + DEFINE_STAT(JSTAT_JOOLD_SSS_SENT, "Joold: Total sessions successfully sent."), + DEFINE_STAT(JSTAT_JOOLD_SSS_RCVD, "Joold: Total sessions successfully received."), + DEFINE_STAT(JSTAT_JOOLD_PKT_SENT, "Joold: Total session packets successfully sent."), + DEFINE_STAT(JSTAT_JOOLD_PKT_RCVD, "Joold: Total session packets successfully received."), + DEFINE_STAT(JSTAT_JOOLD_ADS, "Joold: Total advertises queued."), + DEFINE_STAT(JSTAT_JOOLD_ACKS, "Joold: Total ACKs received from userspace."), + DEFINE_STAT(JSTAT_UNKNOWN, TC "Programming error found. The module recovered, but the packet was dropped."), DEFINE_STAT(JSTAT_PADDING, "Dummy; ignore this one."), }; From ced7e7af87263ea77eb5c0b3c46dd9bd72822d61 Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Sat, 9 Sep 2023 13:26:46 -0600 Subject: [PATCH 3/9] joold: Validate iname when receiving kernel packet Prevents Jool instances in the same namespace from receiving each other's sessions. --- src/common/xlat.h | 2 +- src/usr/joold/modsocket.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/common/xlat.h b/src/common/xlat.h index 6a63b5e2..2c454abc 100644 --- a/src/common/xlat.h +++ b/src/common/xlat.h @@ -10,7 +10,7 @@ #define JOOL_VERSION_MAJOR 4 #define JOOL_VERSION_MINOR 1 #define JOOL_VERSION_REV 10 -#define JOOL_VERSION_DEV 2 +#define JOOL_VERSION_DEV 3 /** See http://stackoverflow.com/questions/195975 */ #define STR_VALUE(arg) #arg diff --git a/src/usr/joold/modsocket.c b/src/usr/joold/modsocket.c index 10bf08ce..f3ffbbbe 100644 --- a/src/usr/joold/modsocket.c +++ b/src/usr/joold/modsocket.c @@ -61,6 +61,8 @@ static int updated_entries_cb(struct nl_msg *msg, void *arg) pr_result(&result); goto fail; } + if (strcasecmp(jhdr->iname, iname) != 0) + return 0; /* Packet is not intended for us. */ if (jhdr->flags & JOOLNLHDR_FLAGS_ERROR) { result = joolnl_msg2result(msg); pr_result(&result); From e5b8595e1af05ae1abe9b0b878540bed0e1a2832 Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Sat, 16 Sep 2023 15:05:52 -0600 Subject: [PATCH 4/9] Fix joold memory leak Hopefully ends #410. --- src/common/xlat.h | 2 +- src/mod/common/joold.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/xlat.h b/src/common/xlat.h index 2c454abc..f99dd71e 100644 --- a/src/common/xlat.h +++ b/src/common/xlat.h @@ -10,7 +10,7 @@ #define JOOL_VERSION_MAJOR 4 #define JOOL_VERSION_MINOR 1 #define JOOL_VERSION_REV 10 -#define JOOL_VERSION_DEV 3 +#define JOOL_VERSION_DEV 4 /** See http://stackoverflow.com/questions/195975 */ #define STR_VALUE(arg) #arg diff --git a/src/mod/common/joold.c b/src/mod/common/joold.c index 31c80446..f5835a0c 100644 --- a/src/mod/common/joold.c +++ b/src/mod/common/joold.c @@ -182,6 +182,7 @@ static void send_to_userspace_prepare(struct xlator *jool, if (session) { if (too_many_sessions(jool)) { log_warn_once("joold: Too many sessions deferred! I need to drop some; sorry."); + FREE_DEFERRED(session); } else { list_add_tail(&session->lh, &queue->deferred.list); queue->deferred.count++; From b9a2fc3372df010a6767d0f5df2044c114d9f43e Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Sat, 16 Sep 2023 15:24:24 -0600 Subject: [PATCH 5/9] Fix userspace jool's default instance name Was crashing because the default instance name was `NULL`, not "default". --- src/usr/argp/wargp/instance.c | 2 +- src/usr/joold/modsocket.c | 16 ++++++---------- src/usr/nl/core.c | 2 +- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/usr/argp/wargp/instance.c b/src/usr/argp/wargp/instance.c index 944a3b13..cef83a0a 100644 --- a/src/usr/argp/wargp/instance.c +++ b/src/usr/argp/wargp/instance.c @@ -51,7 +51,7 @@ static int parse_iname(void *void_field, int key, char *str) struct wargp_type wt_iname = { .argument = "", .parse = parse_iname, - .candidates = "default", + .candidates = INAME_DEFAULT, }; struct display_args { diff --git a/src/usr/joold/modsocket.c b/src/usr/joold/modsocket.c index f3ffbbbe..809636d8 100644 --- a/src/usr/joold/modsocket.c +++ b/src/usr/joold/modsocket.c @@ -98,8 +98,8 @@ static int read_json(int argc, char **argv) struct jool_result result; if (argc < 3) { - iname = NULL; - return 0; + iname = strdup(INAME_DEFAULT); + return (iname != NULL) ? 0 : -ENOMEM; } syslog(LOG_INFO, "Opening file %s...", argv[2]); @@ -119,14 +119,10 @@ static int read_json(int argc, char **argv) free(file); child = cJSON_GetObjectItem(json, "instance"); - if (child) { - iname = strdup(child->valuestring); - if (!iname) { - cJSON_Delete(json); - return -ENOMEM; - } - } else { - iname = NULL; + iname = strdup(child ? child->valuestring : INAME_DEFAULT); + if (!iname) { + cJSON_Delete(json); + return -ENOMEM; } cJSON_Delete(json); diff --git a/src/usr/nl/core.c b/src/usr/nl/core.c index 92606991..8ce11ebc 100644 --- a/src/usr/nl/core.c +++ b/src/usr/nl/core.c @@ -61,7 +61,7 @@ struct jool_result joolnl_alloc_msg(struct joolnl_socket *socket, hdr->version = htonl(xlat_version()); hdr->xt = socket->xt; hdr->flags = flags; - strcpy(hdr->iname, iname ? iname : "default"); + strcpy(hdr->iname, iname ? iname : INAME_DEFAULT); *out = msg; return result_success(); From 1ad5737bd8c927e8c4c5841a833022a32f98c755 Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Sat, 16 Sep 2023 15:56:30 -0600 Subject: [PATCH 6/9] Add new stat counter: JSTAT_JOOLD_SSS_ENOSPC Because it rate-limits itself, the "too many sessions deferred" warning isn't a useful indicator of how many sessions have been dropped. Hence, stat counter. --- docs/en/usr-flags-global.md | 7 +++++++ src/common/stats.h | 1 + src/mod/common/joold.c | 1 + src/usr/nl/stats.c | 1 + 4 files changed, 10 insertions(+) diff --git a/docs/en/usr-flags-global.md b/docs/en/usr-flags-global.md index 180f10cb..8fa8c3ba 100644 --- a/docs/en/usr-flags-global.md +++ b/docs/en/usr-flags-global.md @@ -673,6 +673,13 @@ Watch out for this message in the kernel logs: joold: Too many sessions deferred! I need to drop some; sorry. +If you want to find out how many sessions have been lost, query the `JSTAT_JOOLD_SSS_ENOSPC` [stat](https://nicmx.github.io/Jool/en/usr-flags-stats.html): + +``` +$ jool stats display --all | grep JSTAT_JOOLD_SSS_ENOSPC +JSTAT_JOOLD_SSS_ENOSPC: 0 +``` + ### `ss-max-payload` - Type: Integer diff --git a/src/common/stats.h b/src/common/stats.h index b93df96b..eb8d498d 100644 --- a/src/common/stats.h +++ b/src/common/stats.h @@ -104,6 +104,7 @@ enum jool_stat_id { JSTAT_JOOLD_SSS_QUEUED, JSTAT_JOOLD_SSS_SENT, JSTAT_JOOLD_SSS_RCVD, + JSTAT_JOOLD_SSS_ENOSPC, JSTAT_JOOLD_PKT_SENT, JSTAT_JOOLD_PKT_RCVD, JSTAT_JOOLD_ADS, diff --git a/src/mod/common/joold.c b/src/mod/common/joold.c index f5835a0c..273a1cff 100644 --- a/src/mod/common/joold.c +++ b/src/mod/common/joold.c @@ -182,6 +182,7 @@ static void send_to_userspace_prepare(struct xlator *jool, if (session) { if (too_many_sessions(jool)) { log_warn_once("joold: Too many sessions deferred! I need to drop some; sorry."); + jstat_inc(jool->stats, JSTAT_JOOLD_SSS_ENOSPC); FREE_DEFERRED(session); } else { list_add_tail(&session->lh, &queue->deferred.list); diff --git a/src/usr/nl/stats.c b/src/usr/nl/stats.c index 3b0c1218..a2226a3a 100644 --- a/src/usr/nl/stats.c +++ b/src/usr/nl/stats.c @@ -101,6 +101,7 @@ static struct joolnl_stat_metadata const jstat_metadatas[] = { DEFINE_STAT(JSTAT_JOOLD_SSS_QUEUED, "Joold: Total sessions queued."), DEFINE_STAT(JSTAT_JOOLD_SSS_SENT, "Joold: Total sessions successfully sent."), DEFINE_STAT(JSTAT_JOOLD_SSS_RCVD, "Joold: Total sessions successfully received."), + DEFINE_STAT(JSTAT_JOOLD_SSS_ENOSPC, "Joold: Total sessions dropped because the queue was full."), DEFINE_STAT(JSTAT_JOOLD_PKT_SENT, "Joold: Total session packets successfully sent."), DEFINE_STAT(JSTAT_JOOLD_PKT_RCVD, "Joold: Total session packets successfully received."), DEFINE_STAT(JSTAT_JOOLD_ADS, "Joold: Total advertises queued."), From 1b98e0cd07c6ed01fbe1876c640fc42d587cc49e Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Sat, 16 Sep 2023 18:55:13 -0600 Subject: [PATCH 7/9] Compress serialized sessions Issue #410 is really highlighting the importance of session buffering, and the old serialization algorithm involves too many subheaders and padding, making it difficult to fit a decent number of sessions in a single packet. Therefore, serialize more conservatively at byte level. I could still chop off 4 more bytes by stripping the attribute header, but the Netlink API makes it awkward. --- docs/en/usr-flags-global.md | 6 +- src/common/config.h | 1 + src/mod/common/nl/attribute.c | 141 ++++++++++++++++++++-------------- test/unit/Makefile | 1 + test/unit/joold/joold_test.c | 10 +++ 5 files changed, 97 insertions(+), 62 deletions(-) diff --git a/docs/en/usr-flags-global.md b/docs/en/usr-flags-global.md index 8fa8c3ba..f1ea59d2 100644 --- a/docs/en/usr-flags-global.md +++ b/docs/en/usr-flags-global.md @@ -692,7 +692,7 @@ Deprecated; does nothing as of Jool 4.1.11. ### `ss-max-sessions-per-packet` - Type: Integer -- Default: 10 +- Default: 27 - Modes: Stateful NAT64 only - Source: [Issue 113]({{ site.repository-url }}/issues/113), [issue 410]({{ site.repository-url }}/issues/410) @@ -721,13 +721,13 @@ $ make $ sudo make test | head ... Jool: Netlink attribute header size: 4 -Jool: Serialized session size: 140 +Jool: Serialized session size: 52 ... ``` So the default value came out of ``` -floor((1500 - max(20, 40) - 8 - 4) / 140) +floor((1500 - max(20, 40) - 8 - 4) / 52) ``` diff --git a/src/common/config.h b/src/common/config.h index 4799c087..67121af8 100644 --- a/src/common/config.h +++ b/src/common/config.h @@ -189,6 +189,7 @@ enum joolnl_attr_bib { extern struct nla_policy joolnl_bib_entry_policy[JNLAB_COUNT]; +/* TODO (fine) Most of these fields are obsolete; rm them in a minor release. */ enum joolnl_attr_session { JNLASE_SRC6 = 1, JNLASE_DST6, diff --git a/src/mod/common/nl/attribute.c b/src/mod/common/nl/attribute.c index 0b1a96d9..643c7d92 100644 --- a/src/mod/common/nl/attribute.c +++ b/src/mod/common/nl/attribute.c @@ -4,6 +4,9 @@ #include "common/constants.h" #include "mod/common/log.h" +#define SERIALIZED_SESSION_SIZE (2 * sizeof(struct in6_addr) \ + + sizeof(struct in_addr) + sizeof(__be32) + 4 * sizeof(__be16)) + static int validate_null(struct nlattr *attr, char const *name) { if (!attr) { @@ -388,10 +391,17 @@ static int get_timeout(struct bib_config *config, struct session_entry *entry) return 0; } +#define READ_RAW(serialized, field) \ + memcpy(&field, serialized, sizeof(field)); \ + serialized += sizeof(field); + int jnla_get_session(struct nlattr *attr, char const *name, struct bib_config *config, struct session_entry *entry) { - struct nlattr *attrs[JNLASE_COUNT]; + __u8 *serialized; + __be32 tmp32; + __be16 tmp16; + __u16 __tmp16; unsigned long expiration; int error; @@ -399,53 +409,42 @@ int jnla_get_session(struct nlattr *attr, char const *name, if (error) return error; - error = jnla_parse_nested(attrs, JNLASE_MAX, attr, - joolnl_session_entry_policy, name); - if (error) - return error; + if (attr->nla_len < SERIALIZED_SESSION_SIZE) { + log_err("Invalid request: Session size (%u) < %zu", + attr->nla_len, SERIALIZED_SESSION_SIZE); + return -EINVAL; + } memset(entry, 0, sizeof(*entry)); + serialized = nla_data(attr); - if (attrs[JNLASE_SRC6]) { - error = jnla_get_taddr6(attrs[JNLASE_SRC6], - "IPv6 source address", &entry->src6); - if (error) - return error; - } - if (attrs[JNLASE_DST6]) { - error = jnla_get_taddr6(attrs[JNLASE_DST6], - "IPv6 destination address", &entry->dst6); - if (error) - return error; - } - if (attrs[JNLASE_SRC4]) { - error = jnla_get_taddr4(attrs[JNLASE_SRC4], - "IPv4 source address", &entry->src4); - if (error) - return error; - } - if (attrs[JNLASE_DST4]) { - error = jnla_get_taddr4(attrs[JNLASE_DST4], - "IPv4 destination address", &entry->dst4); - if (error) - return error; - } + READ_RAW(serialized, entry->src6.l3); + READ_RAW(serialized, entry->dst6.l3); + READ_RAW(serialized, entry->src4.l3); + READ_RAW(serialized, tmp32); + + READ_RAW(serialized, tmp16); + entry->src6.l4 = ntohs(tmp16); + READ_RAW(serialized, tmp16); + entry->dst6.l4 = ntohs(tmp16); + READ_RAW(serialized, tmp16); + entry->src4.l4 = ntohs(tmp16); + READ_RAW(serialized, tmp16); + __tmp16 = ntohs(tmp16); - if (attrs[JNLASE_PROTO]) - entry->proto = nla_get_u8(attrs[JNLASE_PROTO]); - if (attrs[JNLASE_STATE]) - entry->state = nla_get_u8(attrs[JNLASE_STATE]); - if (attrs[JNLASE_TIMER]) - entry->timer_type = nla_get_u8(attrs[JNLASE_TIMER]); + entry->dst4.l3.s_addr = entry->dst6.l3.s6_addr32[3]; + entry->dst4.l4 = entry->dst6.l4; + + entry->proto = (__tmp16 >> 5) & 3; + entry->state = (__tmp16 >> 2) & 7; + entry->timer_type = __tmp16 & 3; error = get_timeout(config, entry); if (error) return error; - if (attrs[JNLASE_EXPIRATION]) { - expiration = msecs_to_jiffies(nla_get_u32(attrs[JNLASE_EXPIRATION])); - entry->update_time = jiffies + expiration - entry->timeout; - } + expiration = msecs_to_jiffies(ntohl(tmp32)); + entry->update_time = jiffies + expiration - entry->timeout; entry->has_stored = false; return 0; @@ -717,16 +716,35 @@ int jnla_put_bib(struct sk_buff *skb, int attrtype, struct bib_entry const *bib) return 0; } +#define ADD_RAW(buffer, offset, content) \ + memcpy(buffer + offset, &content, sizeof(content)); \ + offset += sizeof(content) + int jnla_put_session(struct sk_buff *skb, int attrtype, struct session_entry const *entry) { - struct nlattr *root; + __u8 buffer[SERIALIZED_SESSION_SIZE]; + size_t offset; unsigned long dying_time; - int error; + __be32 tmp32; + __be16 tmp16; - root = nla_nest_start(skb, attrtype); - if (!root) - return -EMSGSIZE; + /* + * The session object is huge, and joold wants to fit as many sessions + * as possible in one single packet. + * Therefore, instead of adding each field as a Netlink attribute, + * we'll do some low level byte hacking. + */ + + offset = 0; + + /* 128 bit fields */ + ADD_RAW(buffer, offset, entry->src6.l3); + ADD_RAW(buffer, offset, entry->dst6.l3); + + /* 32 bit fields */ + ADD_RAW(buffer, offset, entry->src4.l3); + /* Skip dst4; it can be inferred from dst6. */ dying_time = entry->update_time + entry->timeout; dying_time = (dying_time > jiffies) @@ -735,21 +753,26 @@ int jnla_put_session(struct sk_buff *skb, int attrtype, if (dying_time > MAX_U32) dying_time = MAX_U32; - error = jnla_put_taddr6(skb, JNLASE_SRC6, &entry->src6) - || jnla_put_taddr6(skb, JNLASE_DST6, &entry->dst6) - || jnla_put_taddr4(skb, JNLASE_SRC4, &entry->src4) - || jnla_put_taddr4(skb, JNLASE_DST4, &entry->dst4) - || nla_put_u8(skb, JNLASE_PROTO, entry->proto) - || nla_put_u8(skb, JNLASE_STATE, entry->state) - || nla_put_u8(skb, JNLASE_TIMER, entry->timer_type) - || nla_put_u32(skb, JNLASE_EXPIRATION, dying_time); - if (error) { - nla_nest_cancel(skb, root); - return error; - } - - nla_nest_end(skb, root); - return 0; + tmp32 = htonl(dying_time); + ADD_RAW(buffer, offset, tmp32); + + /* 16 bit fields */ + tmp16 = htons(entry->src6.l4); + ADD_RAW(buffer, offset, tmp16); + tmp16 = htons(entry->dst6.l4); + ADD_RAW(buffer, offset, tmp16); + tmp16 = htons(entry->src4.l4); + ADD_RAW(buffer, offset, tmp16); + + /* Well, this fits in a byte, but use 2 to avoid slop */ + tmp16 = htons( + (entry->proto << 5) /* 2 bits */ + | (entry->state << 2) /* 3 bits */ + | entry->timer_type /* 2 bits */ + ); + ADD_RAW(buffer, offset, tmp16); + + return nla_put(skb, attrtype, sizeof(buffer), buffer); } int jnla_put_plateaus(struct sk_buff *skb, int attrtype, diff --git a/test/unit/Makefile b/test/unit/Makefile index 212cf529..e31f38ce 100644 --- a/test/unit/Makefile +++ b/test/unit/Makefile @@ -16,6 +16,7 @@ PROJECTS += sessiontable PROJECTS += pool4db PROJECTS += bibdb PROJECTS += sessiondb +PROJECTS += joold # Layer 4 tests (utils that depend on the dbs) #PROJECTS += joolns diff --git a/test/unit/joold/joold_test.c b/test/unit/joold/joold_test.c index 0f53a5ff..15620859 100644 --- a/test/unit/joold/joold_test.c +++ b/test/unit/joold/joold_test.c @@ -62,6 +62,16 @@ int bib_add_session(struct xlator *jool, return -EINVAL; } +void jstat_inc(struct jool_stats *stats, enum jool_stat_id stat) +{ + /* Empty */ +} + +void jstat_add(struct jool_stats *stats, enum jool_stat_id stat, int addend) +{ + /* Empty */ +} + /********************** Init **********************/ static void init_session(unsigned int index, struct session_entry *result) From fcc5ccc4be2fbca697b2a4a2e447dc9206b83f44 Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Mon, 26 Feb 2024 17:34:27 -0600 Subject: [PATCH 8/9] Add stats on userspace joold Will serve stats through an UDP socket. Start joold with a third argument representing the port number: $ joold netsocket.json modsocket.json 45678 Then query using a simple UDP request: $ echo "" | nc -u 127.0.0.1 45678 KERNEL_SENT_PKTS,4 KERNEL_SENT_BYTES,208 NET_RCVD_PKTS,0 NET_RCVD_BYTES,0 NET_SENT_PKTS,4 NET_SENT_BYTES,208 - KERNEL_SENT_PKTS: Packets sent to the kernel module. (Should match the local instance's JSTAT_JOOLD_PKT_RCVD.) - KERNEL_SENT_BYTES: Session bytes sent to the kernel module. (Should match the local instance's JSTAT_JOOLD_SSS_RCVD multiplied by the session size.) - NET_RCVD_PKTS: Packets received from the network. (Should match the remote instance's JSTAT_JOOLD_PKT_SENT.) - NET_RCVD_BYTES: Session bytes received from the network. (Should match the remote instance's JSTAT_JOOLD_SSS_SENT multiplied by the session size.) - NET_SENT_PKTS: Packets sent to the network. (Should match the remote joold's NET_RCVD_PKTS.) - NET_SENT_BYTES: Session bytes sent to the network. (Should match the remote joold's NET_RCVD_BYTES.) Will not start the server if the port is absent. --- src/usr/joold/Makefile.am | 3 +- src/usr/joold/joold.c | 4 ++ src/usr/joold/modsocket.c | 7 ++ src/usr/joold/netsocket.c | 14 +++- src/usr/joold/statsocket.c | 136 +++++++++++++++++++++++++++++++++++++ src/usr/joold/statsocket.h | 6 ++ 6 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 src/usr/joold/statsocket.c create mode 100644 src/usr/joold/statsocket.h diff --git a/src/usr/joold/Makefile.am b/src/usr/joold/Makefile.am index 2be1dcdb..7979e51c 100644 --- a/src/usr/joold/Makefile.am +++ b/src/usr/joold/Makefile.am @@ -6,7 +6,8 @@ joold_SOURCES = \ joold.c \ log.c log.h \ modsocket.c modsocket.h \ - netsocket.c netsocket.h + netsocket.c netsocket.h \ + statsocket.c statsocket.h joold_CFLAGS = ${WARNINGCFLAGS} joold_CFLAGS += -I${top_srcdir}/src diff --git a/src/usr/joold/joold.c b/src/usr/joold/joold.c index 26053e39..f9438f08 100644 --- a/src/usr/joold/joold.c +++ b/src/usr/joold/joold.c @@ -7,6 +7,7 @@ #include "common/xlat.h" #include "usr/joold/modsocket.h" #include "usr/joold/netsocket.h" +#include "usr/joold/statsocket.h" static void cancel_thread(pthread_t thread) { @@ -44,6 +45,9 @@ int main(int argc, char **argv) netsocket_teardown(); goto end; } + error = statsocket_start(argc, argv); + if (error) + goto clean; error = pthread_create(&mod2net_thread, NULL, modsocket_listen, NULL); if (error) { diff --git a/src/usr/joold/modsocket.c b/src/usr/joold/modsocket.c index 809636d8..adb30e54 100644 --- a/src/usr/joold/modsocket.c +++ b/src/usr/joold/modsocket.c @@ -1,6 +1,7 @@ #include "modsocket.h" #include +#include #include #include #include @@ -15,6 +16,9 @@ static struct joolnl_socket jsocket; static char *iname; +atomic_int modsocket_pkts_sent; +atomic_int modsocket_bytes_sent; + /* Called by the net socket whenever joold receives data from the network. */ void modsocket_send(void *request, size_t request_len) { @@ -82,6 +86,9 @@ static int updated_entries_cb(struct nl_msg *msg, void *arg) */ netsocket_send(nla_data(root), nla_len(root)); do_ack(); + + modsocket_pkts_sent++; + modsocket_bytes_sent += nla_len(root); return 0; einval: diff --git a/src/usr/joold/netsocket.c b/src/usr/joold/netsocket.c index 935add88..5691bea8 100644 --- a/src/usr/joold/netsocket.c +++ b/src/usr/joold/netsocket.c @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -48,6 +49,11 @@ static struct addrinfo *addr_candidates; /** Candidate from @addr_candidates that we managed to bind the socket with. */ static struct addrinfo *bound_address; +atomic_int netsocket_pkts_rcvd; +atomic_int netsocket_bytes_rcvd; +atomic_int netsocket_pkts_sent; +atomic_int netsocket_bytes_sent; + static struct in_addr *get_addr4(struct addrinfo *addr) { return &((struct sockaddr_in *)addr->ai_addr)->sin_addr; @@ -482,6 +488,9 @@ void *netsocket_listen(void *arg) continue; } + netsocket_pkts_rcvd++; + netsocket_bytes_rcvd += bytes; + syslog(LOG_DEBUG, "Received %d bytes from the network.", bytes); modsocket_send(buffer, bytes); } while (true); @@ -499,6 +508,9 @@ void netsocket_send(void *buffer, size_t size) bound_address->ai_addrlen); if (bytes < 0) pr_perror("Could not send a packet to the network", errno); - else + else { syslog(LOG_DEBUG, "Sent %d bytes to the network.\n", bytes); + netsocket_pkts_sent++; + netsocket_bytes_sent += bytes; + } } diff --git a/src/usr/joold/statsocket.c b/src/usr/joold/statsocket.c new file mode 100644 index 00000000..55bd4ac4 --- /dev/null +++ b/src/usr/joold/statsocket.c @@ -0,0 +1,136 @@ +#include "usr/joold/statsocket.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "log.h" + +static int create_socket(char const *port, int *fd) +{ + int sk; + struct addrinfo hints = { 0 }; + struct addrinfo *ais, *ai; + int err; + + syslog(LOG_INFO, "Setting up statsocket (port %s)...", port); + + hints.ai_family = AF_UNSPEC; + hints.ai_socktype = SOCK_DGRAM; + hints.ai_flags |= AI_PASSIVE; + + err = getaddrinfo(NULL, port, &hints, &ais); + if (err) { + syslog(LOG_ERR, "getaddrinfo() failed: %s", gai_strerror(err)); + return err; + } + + for (ai = ais; ai; ai = ai->ai_next) { + syslog(LOG_INFO, "Trying an address candidate..."); + + sk = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol); + if (sk < 0) { + pr_perror("socket() failed", errno); + continue; + } + + if (bind(sk, ai->ai_addr, ai->ai_addrlen)) { + pr_perror("bind() failed", errno); + close(sk); + continue; + } + + syslog(LOG_INFO, "Statsocket created successfully."); + freeaddrinfo(ais); + *fd = sk; + return 0; + } + + syslog(LOG_ERR, "None of the candidates yielded a valid statsocket."); + freeaddrinfo(ais); + return 1; +} + +extern atomic_int modsocket_pkts_sent; +extern atomic_int modsocket_bytes_sent; +extern atomic_int netsocket_pkts_rcvd; +extern atomic_int netsocket_bytes_rcvd; +extern atomic_int netsocket_pkts_sent; +extern atomic_int netsocket_bytes_sent; + +#define BUFFER_SIZE 1024 + +void *serve_stats(void *arg) +{ + int sk; + struct sockaddr_storage peer_addr; + socklen_t peer_addr_len; + int nread, nstr, nwritten; + char buffer[BUFFER_SIZE]; + + sk = *((int *)arg); + free(arg); + + while (true) { + peer_addr_len = sizeof(peer_addr); + nread = recvfrom(sk, buffer, BUFFER_SIZE, 0, + (struct sockaddr *) &peer_addr, + &peer_addr_len); + if (nread == -1) + continue; /* Ignore failed request */ + + nstr = snprintf(buffer, BUFFER_SIZE, + "KERNEL_SENT_PKTS,%d\nKERNEL_SENT_BYTES,%d\n" + "NET_RCVD_PKTS,%d\nNET_RCVD_BYTES,%d\n" + "NET_SENT_PKTS,%d\nNET_SENT_BYTES,%d\n", + modsocket_pkts_sent, modsocket_bytes_sent, + netsocket_pkts_rcvd, netsocket_bytes_rcvd, + netsocket_pkts_sent, netsocket_bytes_sent); + if (nstr >= BUFFER_SIZE) + snprintf(buffer, BUFFER_SIZE, "Bug!"); + + nwritten = sendto(sk, buffer, nstr, 0, + (struct sockaddr *) &peer_addr, + peer_addr_len); + if (nwritten != nstr) + syslog(LOG_ERR, "statsocket error: %s\n", strerror(errno)); + } +} + +int statsocket_start(int argc, char **argv) +{ + int sk, *sk2; + pthread_t thread; + int error; + + if (argc < 3) { + syslog(LOG_INFO, "statsocket port unavailable; skipping statsocket."); + return 0; + } + + error = create_socket(argv[2], &sk); + if (error) + return error; + + sk2 = malloc(sizeof(int)); + if (!sk2) + return -ENOMEM; + *sk2 = sk; + + error = pthread_create(&thread, NULL, serve_stats, sk2); + if (error) { + free(sk2); + close(sk); + } + + return error; +} diff --git a/src/usr/joold/statsocket.h b/src/usr/joold/statsocket.h new file mode 100644 index 00000000..c9fafeb9 --- /dev/null +++ b/src/usr/joold/statsocket.h @@ -0,0 +1,6 @@ +#ifndef SRC_USR_JOOLD_STATSOCKET_H_ +#define SRC_USR_JOOLD_STATSOCKET_H_ + +int statsocket_start(int argc, char **argv); + +#endif /* SRC_USR_JOOLD_STATSOCKET_H_ */ From b1e502102965fbd84653b68400efde8b28de7077 Mon Sep 17 00:00:00 2001 From: Alberto Leiva Popper Date: Tue, 2 Apr 2024 13:01:14 -0600 Subject: [PATCH 9/9] Fix argcount validation --- src/usr/joold/statsocket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/usr/joold/statsocket.c b/src/usr/joold/statsocket.c index 55bd4ac4..234ca199 100644 --- a/src/usr/joold/statsocket.c +++ b/src/usr/joold/statsocket.c @@ -112,7 +112,7 @@ int statsocket_start(int argc, char **argv) pthread_t thread; int error; - if (argc < 3) { + if (argc < 4) { syslog(LOG_INFO, "statsocket port unavailable; skipping statsocket."); return 0; }