pespin has submitted this change. (
https://gerrit.osmocom.org/c/osmo-upf/+/39452?usp=email )
Change subject: Introduce hashtable to lookup chain_id
......................................................................
Introduce hashtable to lookup chain_id
This lookup was taking ages specially when UPF already managed thousands
of sessions.
Related: SYS#6398
Change-Id: I7df8fd945eedbda98bd08e9fb2f382e0f55c2983
---
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/up_gtp_action.c
M src/osmo-upf/up_session.c
M src/osmo-upf/upf.c
M src/osmo-upf/upf_nft.c
7 files changed, 74 insertions(+), 88 deletions(-)
Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
laforge: Looks good to me, but someone else must approve
diff --git a/include/osmocom/upf/up_gtp_action.h
b/include/osmocom/upf/up_gtp_action.h
index c47c35c..9bcd839 100644
--- a/include/osmocom/upf/up_gtp_action.h
+++ b/include/osmocom/upf/up_gtp_action.h
@@ -64,6 +64,9 @@
void *handle;
};
+struct up_gtp_action *up_gtp_action_alloc(void *ctx, struct up_session
*session, enum up_gtp_action_kind kind, struct llist_head *dst);
+void up_gtp_action_free(struct up_gtp_action *a);
+
int up_gtp_action_cmp(const struct up_gtp_action *a, const struct
up_gtp_action *b);
int up_gtp_action_enable(struct up_gtp_action *a);
diff --git a/include/osmocom/upf/upf.h b/include/osmocom/upf/upf.h
index 0e5406f..1e5d702 100644
--- a/include/osmocom/upf/upf.h
+++ b/include/osmocom/upf/upf.h
@@ -115,6 +115,8 @@
int priority_pre;
int priority_post;
uint32_t next_chain_id_state;
+ /* hashtable of (struct upf_nft_tun)->node_by_chain_id: */
+ DECLARE_HASHTABLE(nft_tun_by_chain_id, 10);
} tunmap;
struct {
diff --git a/include/osmocom/upf/upf_nft.h b/include/osmocom/upf/upf_nft.h
index be2ec0d..a194faf 100644
--- a/include/osmocom/upf/upf_nft.h
+++ b/include/osmocom/upf/upf_nft.h
@@ -24,10 +24,12 @@
#pragma once
#include <stdint.h>
+#include <osmocom/core/hashtable.h>
#include <osmocom/upf/upf_tun.h>
struct upf_nft_tun {
+ struct hlist_node node_by_chain_id; /* item in
g_upf->tunmap.nft_tun_by_chain_id */
struct upf_tun tun;
uint32_t chain_id;
};
diff --git a/src/osmo-upf/up_gtp_action.c b/src/osmo-upf/up_gtp_action.c
index 28dd078..a3c6b68 100644
--- a/src/osmo-upf/up_gtp_action.c
+++ b/src/osmo-upf/up_gtp_action.c
@@ -198,3 +198,33 @@
{
OSMO_NAME_C_IMPL(ctx, 128, "ERROR", up_gtp_action_to_str_buf, a)
}
+
+struct up_gtp_action *up_gtp_action_alloc(void *ctx, struct up_session
*session, enum up_gtp_action_kind kind, struct llist_head *dst)
+{
+ struct up_gtp_action *a = talloc_zero(ctx, struct up_gtp_action);
+ OSMO_ASSERT(a);
+ a->session = session;
+ a->kind = kind;
+
+ if (kind == UP_GTP_U_TUNMAP) {
+ INIT_HLIST_NODE(&a->tunmap.access.node_by_chain_id);
+ INIT_HLIST_NODE(&a->tunmap.core.node_by_chain_id);
+ }
+ llist_add_tail(&a->entry, dst);
+ return a;
+}
+
+void up_gtp_action_free(struct up_gtp_action *a)
+{
+ if (!a)
+ return;
+ up_gtp_action_disable(a);
+ llist_del(&a->entry);
+ if (a->kind == UP_GTP_U_TUNMAP) {
+ if (!hlist_unhashed(&a->tunmap.access.node_by_chain_id))
+ hash_del(&a->tunmap.access.node_by_chain_id);
+ if (!hlist_unhashed(&a->tunmap.core.node_by_chain_id))
+ hash_del(&a->tunmap.core.node_by_chain_id);
+ }
+ talloc_free(a);
+}
diff --git a/src/osmo-upf/up_session.c b/src/osmo-upf/up_session.c
index 359d116..97c9367 100644
--- a/src/osmo-upf/up_session.c
+++ b/src/osmo-upf/up_session.c
@@ -824,9 +824,7 @@
/* Shut down all active GTP rules */
while ((a = llist_first_entry_or_null(&session->active_gtp_actions,
struct up_gtp_action, entry))) {
- up_gtp_action_disable(a);
- llist_del(&a->entry);
- talloc_free(a);
+ up_gtp_action_free(a);
}
}
@@ -1202,31 +1200,14 @@
talloc_free(rpdr->inactive_reason);
rpdr->inactive_reason = NULL;
- a = talloc(ctx, struct up_gtp_action);
- OSMO_ASSERT(a);
- *a = (struct up_gtp_action){
- .session = session,
- .pdr_access = pdr->desc.pdr_id,
- .pdr_core = rpdr->desc.pdr_id,
- .kind = UP_GTP_U_TUNEND,
- .tunend = {
- .access = {
- .local = {
- .addr =
pdr->local_f_teid->fixed.ip_addr.v4,
- .teid = pdr->local_f_teid->fixed.teid,
- },
- .remote = {
- .addr =
rfar_forw->outer_header_creation.ip_addr.v4,
- .teid =
rfar_forw->outer_header_creation.teid,
- },
- },
- .core = {
- .ue_local_addr =
rpdr->desc.pdi.ue_ip_address.ip_addr.v4,
- },
- },
- };
-
- llist_add_tail(&a->entry, dst);
+ a = up_gtp_action_alloc(ctx, session, UP_GTP_U_TUNEND, dst);
+ a->pdr_access = pdr->desc.pdr_id;
+ a->pdr_core = rpdr->desc.pdr_id;
+ a->tunend.access.local.addr = pdr->local_f_teid->fixed.ip_addr.v4;
+ a->tunend.access.local.teid = pdr->local_f_teid->fixed.teid;
+ a->tunend.access.remote.addr =
rfar_forw->outer_header_creation.ip_addr.v4;
+ a->tunend.access.remote.teid = rfar_forw->outer_header_creation.teid;
+ a->tunend.core.ue_local_addr = rpdr->desc.pdi.ue_ip_address.ip_addr.v4;
}
/* A GTP tunnel on Access side, mapping to another GTP tunnel on Core side and
vice versa.
@@ -1316,38 +1297,17 @@
talloc_free(rpdr->inactive_reason);
rpdr->inactive_reason = NULL;
- a = talloc(ctx, struct up_gtp_action);
- OSMO_ASSERT(a);
- *a = (struct up_gtp_action){
- .session = session,
- .pdr_access = pdr->desc.pdr_id,
- .pdr_core = rpdr->desc.pdr_id,
- .kind = UP_GTP_U_TUNMAP,
- .tunmap = {
- .access.tun = {
- .local = {
- .addr =
pdr->local_f_teid->fixed.ip_addr.v4,
- .teid = pdr->local_f_teid->fixed.teid,
- },
- .remote = {
- .addr =
rfar_forw->outer_header_creation.ip_addr.v4,
- .teid =
rfar_forw->outer_header_creation.teid,
- },
- },
- .core.tun = {
- .local = {
- .addr =
rpdr->local_f_teid->fixed.ip_addr.v4,
- .teid = rpdr->local_f_teid->fixed.teid,
- },
- .remote = {
- .addr =
far_forw->outer_header_creation.ip_addr.v4,
- .teid =
far_forw->outer_header_creation.teid,
- },
- },
- },
- };
-
- llist_add_tail(&a->entry, dst);
+ a = up_gtp_action_alloc(ctx, session, UP_GTP_U_TUNMAP, dst);
+ a->pdr_access = pdr->desc.pdr_id;
+ a->pdr_core = rpdr->desc.pdr_id;
+ a->tunmap.access.tun.local.addr = pdr->local_f_teid->fixed.ip_addr.v4;
+ a->tunmap.access.tun.local.teid = pdr->local_f_teid->fixed.teid;
+ a->tunmap.access.tun.remote.addr =
rfar_forw->outer_header_creation.ip_addr.v4;
+ a->tunmap.access.tun.remote.teid =
rfar_forw->outer_header_creation.teid;
+ a->tunmap.core.tun.local.addr = rpdr->local_f_teid->fixed.ip_addr.v4;
+ a->tunmap.core.tun.local.teid = rpdr->local_f_teid->fixed.teid;
+ a->tunmap.core.tun.remote.addr =
far_forw->outer_header_creation.ip_addr.v4;
+ a->tunmap.core.tun.remote.teid = far_forw->outer_header_creation.teid;
}
/* Analyse all PDRs and FARs and find configurations that match either a GTP
encaps/decaps or a GTP forward rule. Add to
@@ -1472,9 +1432,7 @@
continue;
LOGPFSML(session->fi, LOGL_DEBUG, "disabling: %s\n",
up_gtp_action_to_str_c(OTC_SELECT, a));
- up_gtp_action_disable(a);
- llist_del(&a->entry);
- talloc_free(a);
+ up_gtp_action_free(a);
}
/* Set up all GTP tunnels requested in the session setup, but not
active yet */
diff --git a/src/osmo-upf/upf.c b/src/osmo-upf/upf.c
index c82227d..e442d1c 100644
--- a/src/osmo-upf/upf.c
+++ b/src/osmo-upf/upf.c
@@ -78,6 +78,7 @@
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);
hash_init(g_upf->gtp.pdrs_by_local_f_teid);
}
@@ -175,6 +176,17 @@
return g_upf->tunmap.next_chain_id_state;
}
+static bool upf_is_chain_id_in_use(uint32_t chain_id)
+{
+ 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 (nft_tun->chain_id != chain_id)
+ continue;
+ return true;
+ }
+ return false;
+}
+
/* Return an unused chain_id, or 0 if none is found with sane effort. */
uint32_t upf_next_chain_id(void)
{
@@ -182,36 +194,14 @@
/* 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;
- }
-
- if (!taken)
- return chain_id;
+ if (upf_is_chain_id_in_use(chain_id))
+ continue;
+ 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 bc0ee36..7e5b80f 100644
--- a/src/osmo-upf/upf_nft.c
+++ b/src/osmo-upf/upf_nft.c
@@ -489,6 +489,7 @@
tun->chain_id = upf_next_chain_id();
if (!tun->chain_id)
return -ENOSPC;
+ hash_add(g_upf->tunmap.nft_tun_by_chain_id, &tun->node_by_chain_id,
tun->chain_id);
return 0;
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/39452?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-upf
Gerrit-Branch: master
Gerrit-Change-Id: I7df8fd945eedbda98bd08e9fb2f382e0f55c2983
Gerrit-Change-Number: 39452
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
