neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-upf/+/30465 ) Change subject: clarify comments and naming around PDR+FAR classification ...................................................................... clarify comments and naming around PDR+FAR classification No functional change. Rename forw_to_core to access_to_core. Rename forw_from_core to core_to_access. Rename add_gtp_action_endecaps to add_gtp_action_tunend. Rename add_gtp_action_forw to add_gtp_action_tunmap. Add assertions to clearly indicate expected PDR and reverse PDR directions. Tweak various comments and log messages. Fix some comments that have Access / Core flipped. Change-Id: Ia199bb6944476eff6af89b5ab015a9a2f8ce330e --- M include/osmocom/upf/up_session.h M src/osmo-upf/up_session.c 2 files changed, 56 insertions(+), 50 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-upf refs/changes/65/30465/1 diff --git a/include/osmocom/upf/up_session.h b/include/osmocom/upf/up_session.h index 10cc9ff..c459cd7 100644 --- a/include/osmocom/upf/up_session.h +++ b/include/osmocom/upf/up_session.h @@ -94,8 +94,8 @@ bool rx_decaps; bool forw_encaps; - bool forw_to_core; - bool forw_from_core; + bool access_to_core; + bool core_to_access; struct pdr *reverse_pdr; bool active; diff --git a/src/osmo-upf/up_session.c b/src/osmo-upf/up_session.c index 915dd75..d2a405d 100644 --- a/src/osmo-upf/up_session.c +++ b/src/osmo-upf/up_session.c @@ -1071,8 +1071,8 @@ { pdr->rx_decaps = false; pdr->forw_encaps = false; - pdr->forw_to_core = false; - pdr->forw_from_core = false; + pdr->access_to_core = false; + pdr->core_to_access = false; if (!pdr->far) return; @@ -1084,10 +1084,10 @@ if (!action_is_forw(&pdr->far->desc.apply_action)) return; - pdr->forw_to_core = (pdr->desc.pdi.source_iface == OSMO_PFCP_SOURCE_IFACE_ACCESS - && pdr->far->desc.forw_params.destination_iface == OSMO_PFCP_DEST_IFACE_CORE); + pdr->access_to_core = (pdr->desc.pdi.source_iface == OSMO_PFCP_SOURCE_IFACE_ACCESS + && pdr->far->desc.forw_params.destination_iface == OSMO_PFCP_DEST_IFACE_CORE); - pdr->forw_from_core = (pdr->desc.pdi.source_iface == OSMO_PFCP_SOURCE_IFACE_CORE + pdr->core_to_access = (pdr->desc.pdi.source_iface == OSMO_PFCP_SOURCE_IFACE_CORE && pdr->far->desc.forw_params.destination_iface == OSMO_PFCP_DEST_IFACE_ACCESS); } @@ -1107,6 +1107,11 @@ pdr->reverse_pdr = NULL; } +/* Log that a PDR (and its reverse-PDR) is inactive. + * \param pdr The Access-to-Core PDR. + * \param desc Why it is inactive. + * \param pdr_to_str The PDR that desc describes, can be pdr or the reverse Core-to-Access PDR. + */ static void log_inactive_pdr_set(struct pdr *pdr, const char *desc, const struct pdr *pdr_to_str) { struct pdr *rpdr = pdr->reverse_pdr; @@ -1132,7 +1137,7 @@ * Its reverse-PDR's FAR must have an outer-header creation with a remote TEID. * \param pdr A rule detecting packets on Access, where pdr->reverse_pdr detects packets on Core. */ -static void add_gtp_action_endecaps(void *ctx, struct llist_head *dst, struct pdr *pdr) +static void add_gtp_action_tunend(void *ctx, struct llist_head *dst, struct pdr *pdr) { struct up_session *session = pdr->session; struct up_gtp_action *a; @@ -1143,18 +1148,20 @@ OSMO_ASSERT(pdr->far); OSMO_ASSERT(pdr->reverse_pdr); OSMO_ASSERT(pdr->reverse_pdr->far); - - /* To decaps, we need to have a local TEID assigned for which to receive GTP packets. */ - if (!pdr->local_f_teid || pdr->local_f_teid->choose_flag) { - log_inactive_pdr_set(pdr, "missing local TEID", pdr); - return; - } - - /* To encaps, we need to have a remote TEID assigned to send out in GTP packets, and we need to know where to - * send GTP to. */ rpdr = pdr->reverse_pdr; rfar = rpdr->far; rfar_forw = &rfar->desc.forw_params; + + OSMO_ASSERT(pdr->access_to_core); + OSMO_ASSERT(rpdr->core_to_access); + + /* To decaps incoming on Access, we need to have a local F-TEID assigned for which to receive GTP packets. */ + if (!pdr->local_f_teid || pdr->local_f_teid->choose_flag) { + log_inactive_pdr_set(pdr, "missing local F-TEID", pdr); + return; + } + + /* To encaps outgoing on Access, we need to have a remote F-TEID assigned to send out in GTP packets */ if (!rfar->desc.forw_params_present) { log_inactive_pdr_set(pdr, "missing FAR Forwarding Parameters", rpdr); return; @@ -1172,8 +1179,7 @@ return; } - /* To receive packets to be encapsulated, we need to know the assigned IP address for the UE, which receives the - * IP packets that should be placed into GTP. */ + /* To receive IP packets incoming on Core, we need to know the assigned IP address for the UE */ if (!rpdr->desc.pdi.ue_ip_address_present) { log_inactive_pdr_set(pdr, "missing UE IP Address in PDI", rpdr); return; @@ -1221,13 +1227,12 @@ llist_add_tail(&a->entry, dst); } -/* A GTP tunnel on Access side, TODO TODO TODO plain IP on Core side. - * The given PDR must have an outer-header-removal and a local F-TEID. - * Its reverse-PDR must have a UE address flagged as "Destination" IP addr. - * Its reverse-PDR's FAR must have an outer-header creation with a remote TEID. +/* A GTP tunnel on Access side, mapping to another GTP tunnel on Core side and vice versa. + * The PDR and its reverse PDR must both have an outer-header-removal and a local F-TEID. + * Both FARs must have an outer-header creation with a remote F-TEID. * \param pdr A rule detecting packets on Access, where pdr->reverse_pdr detects packets on Core. */ -static void add_gtp_action_forw(void *ctx, struct llist_head *dst, struct pdr *pdr) +static void add_gtp_action_tunmap(void *ctx, struct llist_head *dst, struct pdr *pdr) { struct up_session *session = pdr->session; struct up_gtp_action *a; @@ -1247,52 +1252,53 @@ rfar = rpdr->far; rfar_forw = &rfar->desc.forw_params; - /* To decaps, we need to have a local TEID assigned for which to receive GTP packets. */ - /* decaps from CORE */ + OSMO_ASSERT(pdr->access_to_core); + OSMO_ASSERT(rpdr->core_to_access); + + /* To decaps incoming on Access, we need to have a local F-TEID assigned for which to receive GTP packets. */ if (!pdr->local_f_teid || pdr->local_f_teid->choose_flag) { - log_inactive_pdr_set(pdr, "missing local TEID (CORE side)", pdr); + log_inactive_pdr_set(pdr, "missing local F-TEID (Access side)", pdr); return; } - /* decaps from ACCESS */ + /* To decaps incoming on Core, we need to have a local F-TEID assigned for which to receive GTP packets. */ if (!rpdr->local_f_teid || rpdr->local_f_teid->choose_flag) { - log_inactive_pdr_set(pdr, "missing local TEID (ACCESS side)", pdr); + log_inactive_pdr_set(pdr, "missing local F-TEID (Core side)", pdr); return; } - /* To encaps, we need to have a remote TEID assigned to send out in GTP packets, and we need to know where to - * send GTP to. */ - /* encaps towards ACCESS */ + /* To encaps outgoing on Core, we need to have a remote F-TEID assigned to send out in GTP packets */ if (!far->desc.forw_params_present) { - log_inactive_pdr_set(pdr, "missing FAR Forwarding Parameters", pdr); + log_inactive_pdr_set(pdr, "missing FAR Forwarding Parameters (Access side)", pdr); return; } if (!far_forw->outer_header_creation_present) { - log_inactive_pdr_set(pdr, "missing FAR Outer Header Creation", pdr); + log_inactive_pdr_set(pdr, "missing FAR Outer Header Creation (Access side)", pdr); return; } if (!far_forw->outer_header_creation.teid_present) { - log_inactive_pdr_set(pdr, "missing TEID in FAR Outer Header Creation", pdr); + log_inactive_pdr_set(pdr, "missing TEID in FAR Outer Header Creation (Access side)", pdr); return; } if (!far_forw->outer_header_creation.ip_addr.v4_present) { - log_inactive_pdr_set(pdr, "missing IPv4 in FAR Outer Header Creation", pdr); + log_inactive_pdr_set(pdr, "missing IPv4 in FAR Outer Header Creation (Access side)", pdr); return; } - /* encaps towards CORE */ + + /* To encaps outgoing on Access, we need to have a remote F-TEID assigned to send out in GTP packets */ if (!rfar->desc.forw_params_present) { - log_inactive_pdr_set(pdr, "missing FAR Forwarding Parameters", rpdr); + log_inactive_pdr_set(pdr, "missing FAR Forwarding Parameters (Access side)", rpdr); return; } if (!rfar_forw->outer_header_creation_present) { - log_inactive_pdr_set(pdr, "missing FAR Outer Header Creation", rpdr); + log_inactive_pdr_set(pdr, "missing FAR Outer Header Creation (Access side)", rpdr); return; } if (!rfar_forw->outer_header_creation.teid_present) { - log_inactive_pdr_set(pdr, "missing TEID in FAR Outer Header Creation", rpdr); + log_inactive_pdr_set(pdr, "missing TEID in FAR Outer Header Creation (Access side)", rpdr); return; } if (!rfar_forw->outer_header_creation.ip_addr.v4_present) { - log_inactive_pdr_set(pdr, "missing IPv4 in FAR Outer Header Creation", rpdr); + log_inactive_pdr_set(pdr, "missing IPv4 in FAR Outer Header Creation (Access side)", rpdr); return; } @@ -1356,13 +1362,13 @@ if (pdr->reverse_pdr) continue; - /* In this outer loop, only follow the forw_to_core directed PDRs, in the inner loop find the matching - * forw_from_core PDR. i.e. we are looking only at PDRs detecting packets on the Access side, pairing up + /* In this outer loop, only follow the access_to_core directed PDRs, in the inner loop find the matching + * core_to_access PDR. i.e. we are looking only at PDRs detecting packets on the Access side, pairing up * with "reverse PDRs" detecting packets on the Core side. */ - if (!pdr->forw_to_core) + if (!pdr->access_to_core) continue; - /* If a required TEID is not known, we cannot pair this PDR up */ + /* If a required local addr + TEID is not known, we cannot pair this PDR up */ if (pdr->rx_decaps && !pdr->local_f_teid) continue; @@ -1373,7 +1379,7 @@ continue; /* Looking for a PDR facing the other way */ - if (!other->forw_from_core) + if (!other->core_to_access) continue; /* GTP header-ness must match, in reverse. */ if (pdr->rx_decaps != other->forw_encaps @@ -1396,14 +1402,14 @@ continue; } - /* Iterate in direction to-Core, where pdr->reverse_pdr will be the from-Core counterpart. */ - if (!pdr->forw_to_core) + /* Iterate in direction Access-to-Core, where pdr->reverse_pdr will be the Core-to-Access counterpart. */ + if (!pdr->access_to_core) continue; if (pdr->rx_decaps && !pdr->forw_encaps) - add_gtp_action_endecaps(ctx, dst, pdr); + add_gtp_action_tunend(ctx, dst, pdr); else if (pdr->rx_decaps && pdr->forw_encaps) - add_gtp_action_forw(ctx, dst, pdr); + add_gtp_action_tunmap(ctx, dst, pdr); else { /* log the details of both PDRs in two separate log lines */ log_inactive_pdr_set(pdr, "not implemented", pdr); -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/30465 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Ia199bb6944476eff6af89b5ab015a9a2f8ce330e Gerrit-Change-Number: 30465 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange