neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-upf/+/31484 )
Change subject: scale: tunmap: faster lookup of used nft chain_id ......................................................................
scale: tunmap: faster lookup of used nft chain_id
To ensure that an nft chain id for the tunmap nft ruleset is not used, we so far iterate *all* sessions of *all* peers for every new tunmap being set up, twice.
Instead, record all up_nft_tun in a hash table. This allows much faster lookup whether a given chain_id is already in use.
chain_id_test.c shows that the lookup works and functional behavior remains identical.
Scoping: an nft chain_id must be unique within the nft table. So no matter which local GTP IP address it belongs to, a chain_id must be unique within the entire osmo-upf process.
Related: OS#5900 Change-Id: I36a75ec4698cd83558185c1f202400eb53ae8ff6 --- M include/osmocom/upf/up_gtp_action.h M include/osmocom/upf/upf.h M include/osmocom/upf/upf_nft.h M src/osmo-upf/upf.c M src/osmo-upf/upf_nft.c 5 files changed, 65 insertions(+), 33 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-upf refs/changes/84/31484/1
diff --git a/include/osmocom/upf/up_gtp_action.h b/include/osmocom/upf/up_gtp_action.h index c47c35c..29d8f68 100644 --- a/include/osmocom/upf/up_gtp_action.h +++ b/include/osmocom/upf/up_gtp_action.h @@ -48,6 +48,9 @@ struct llist_head entry; struct up_session *session;
+ /* entry in hash table up_endpoint->gtp_actions_by_chain_id */ + struct hlist_node node_by_chain_id; + uint16_t pdr_access; uint16_t pdr_core;
diff --git a/include/osmocom/upf/upf.h b/include/osmocom/upf/upf.h index db73c1f..0d43785 100644 --- a/include/osmocom/upf/upf.h +++ b/include/osmocom/upf/upf.h @@ -28,6 +28,7 @@ #include <osmocom/core/socket.h> #include <osmocom/core/select.h> #include <osmocom/core/linuxlist.h> +#include <osmocom/core/hashtable.h>
struct osmo_tdef; struct ctrl_handle; @@ -112,7 +113,10 @@ char *table_name; int priority_pre; int priority_post; + uint32_t next_chain_id_state; + /* fast look up of used nft chain ids */ + DECLARE_HASHTABLE(nft_tun_by_chain_id, 6); } tunmap;
struct { @@ -141,4 +145,4 @@ void upf_gtp_devs_close();
uint32_t upf_next_local_teid(void); -uint32_t upf_next_chain_id(void); +uint32_t upf_next_chain_id(struct hlist_node *for_node); diff --git a/include/osmocom/upf/upf_nft.h b/include/osmocom/upf/upf_nft.h index be2ec0d..534107c 100644 --- a/include/osmocom/upf/upf_nft.h +++ b/include/osmocom/upf/upf_nft.h @@ -25,11 +25,17 @@
#include <stdint.h>
+#include <osmocom/core/hashtable.h> + #include <osmocom/upf/upf_tun.h>
struct upf_nft_tun { struct upf_tun tun; + + /* assigned id for nft ruleset, or 0 if none is assigned. */ uint32_t chain_id; + /* hashtable entry for g_upf->tunmap.nft_tun_by_chain_id */ + struct hlist_node node_by_chain_id; };
struct upf_tunmap { diff --git a/src/osmo-upf/upf.c b/src/osmo-upf/upf.c index 5dccce9..1e5e549 100644 --- a/src/osmo-upf/upf.c +++ b/src/osmo-upf/upf.c @@ -66,6 +66,8 @@ INIT_LLIST_HEAD(&g_upf->tunend.vty_cfg.devs); INIT_LLIST_HEAD(&g_upf->tunend.devs); INIT_LLIST_HEAD(&g_upf->netinst); + + hash_init(g_upf->tunmap.nft_tun_by_chain_id); }
int upf_pfcp_init(void) @@ -160,42 +162,26 @@ }
/* Return an unused chain_id, or 0 if none is found with sane effort. */ -uint32_t upf_next_chain_id(void) +uint32_t upf_next_chain_id(struct hlist_node *for_node) { uint32_t sanity;
/* Make sure the new chain_id is not used anywhere */ for (sanity = 2342; sanity; sanity--) { - struct up_peer *peer; uint32_t chain_id = upf_next_chain_id_inc(); bool taken = false;
- if (!g_upf->pfcp.ep) - return chain_id; - - llist_for_each_entry(peer, &g_upf->pfcp.ep->peers, entry) { - struct up_session *session; - int bkt; - hash_for_each(peer->sessions_by_up_seid, bkt, session, node_by_up_seid) { - struct up_gtp_action *a; - llist_for_each_entry(a, &session->active_gtp_actions, entry) { - if (a->kind != UP_GTP_U_TUNMAP) - continue; - if (a->tunmap.access.chain_id == chain_id - || a->tunmap.core.chain_id == chain_id) { - taken = true; - break; - } - } - if (taken) - break; - } - if (taken) - break; + struct upf_nft_tun *nft_tun; + hash_for_each_possible(g_upf->tunmap.nft_tun_by_chain_id, nft_tun, node_by_chain_id, chain_id) { + if (chain_id == nft_tun->chain_id) + taken = true; }
- if (!taken) + if (!taken) { + /* add entry to hash table for fast lookup of used chain_ids */ + hash_add(g_upf->tunmap.nft_tun_by_chain_id, for_node, chain_id); return chain_id; + } }
/* finding a chain_id became insane, return invalid = 0 */ diff --git a/src/osmo-upf/upf_nft.c b/src/osmo-upf/upf_nft.c index 4401f1e..7ff7421 100644 --- a/src/osmo-upf/upf_nft.c +++ b/src/osmo-upf/upf_nft.c @@ -357,25 +357,34 @@ return upf_nft_ruleset_tunmap_delete_c(ctx, &args); }
-static int upf_nft_tunmap_ensure_chain_id(struct upf_nft_tun *tun) +static int upf_nft_tunmap_chain_id_get(struct upf_nft_tun *tun) { - if (tun->chain_id) - return 0; - tun->chain_id = upf_next_chain_id(); + tun->chain_id = upf_next_chain_id(&tun->node_by_chain_id); if (!tun->chain_id) return -ENOSPC; return 0; }
+static void upf_nft_tunmap_chain_id_put(struct upf_nft_tun *tun) +{ + if (!tun->chain_id) + return; + hlist_del(&tun->node_by_chain_id); +} + int upf_nft_tunmap_create(struct upf_tunmap *tunmap) { - if (upf_nft_tunmap_ensure_chain_id(&tunmap->access) - || upf_nft_tunmap_ensure_chain_id(&tunmap->core)) + if (upf_nft_tunmap_chain_id_get(&tunmap->access) + || upf_nft_tunmap_chain_id_get(&tunmap->core)) return -ENOSPC; return upf_nft_run(upf_nft_tunmap_get_ruleset_str(OTC_SELECT, tunmap)); }
int upf_nft_tunmap_delete(struct upf_tunmap *tunmap) { - return upf_nft_run(upf_nft_tunmap_get_ruleset_del_str(OTC_SELECT, tunmap)); + if (upf_nft_run(upf_nft_tunmap_get_ruleset_del_str(OTC_SELECT, tunmap))) + return -EIO; + upf_nft_tunmap_chain_id_put(&tunmap->access); + upf_nft_tunmap_chain_id_put(&tunmap->core); + return 0; }