pespin has submitted this change. (
https://gerrit.osmocom.org/c/libosmo-pfcp/+/41876?usp=email )
Change subject: Avoid marking rx PFCP Assoc Setup Req as duplicate
......................................................................
Avoid marking rx PFCP Assoc Setup Req as duplicate
Newer versions of PFCP spec state that "A PFCP
function shall ignore the Recovery Timestamp
received in the PFCP Association Setup Request message."
Hence, there's no real way to make sure an incoming PFCP ASSOC
SETUP REQ is a duplicate or is simply a new message from a new instance
of the peer node which "decided" to use the same SeqNr as the previous
one. In that case, it's better to be on the safe side and process it to
tear down state rather than ignoring it and keeping old state. If it
turns out to be a duplicate (rare scenario), we'd maybe tear down some
stuff which would have been set up a few seconds ago.
Change-Id: Ia461550e6791aaf00d18e0310bb4f17fdd2a3f65
---
M src/libosmo-pfcp/pfcp_endpoint.c
1 file changed, 24 insertions(+), 14 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, approved
osmith: Looks good to me, but someone else must approve
laforge: Looks good to me, but someone else must approve
diff --git a/src/libosmo-pfcp/pfcp_endpoint.c b/src/libosmo-pfcp/pfcp_endpoint.c
index 7c1058f..64ea347 100644
--- a/src/libosmo-pfcp/pfcp_endpoint.c
+++ b/src/libosmo-pfcp/pfcp_endpoint.c
@@ -161,8 +161,13 @@
return osmo_tdef_get(ep->cfg.tdefs, OSMO_PFCP_TIMER_T1, OSMO_TDEF_MS,
-1);
}
-static unsigned int ep_keep_resp(struct osmo_pfcp_endpoint *ep)
+static unsigned int ep_keep_resp(const struct osmo_pfcp_endpoint *ep, const
struct osmo_pfcp_msg *m)
{
+ /* Don't check for PFCP Assoc Setup Req duplicates: There's no way to
+ * differentiate a duplicate from a new instance of a CP peer which
chooses
+ * (willingly or randomly) after restart the same Sequence Number as in
previous run. */
+ if (m->h.message_type == OSMO_PFCP_MSGT_ASSOC_SETUP_REQ)
+ return 0;
return osmo_tdef_get(ep->cfg.tdefs, OSMO_PFCP_TIMER_KEEP_RESP,
OSMO_TDEF_MS, -1);
}
@@ -271,18 +276,23 @@
static int osmo_pfcp_endpoint_retrans_queue_add(struct osmo_pfcp_endpoint
*endpoint, struct osmo_pfcp_msg *m)
{
struct osmo_pfcp_queue_entry *qe;
- unsigned int n1 = ep_n1(endpoint);
- unsigned int t1_ms = ep_t1(endpoint);
- unsigned int keep_resp_ms = ep_keep_resp(endpoint);
- unsigned int timeout = m->is_response ? keep_resp_ms : t1_ms;
+ unsigned int timeout_ms;
+ unsigned int n1 = 0;
- LOGP(DLPFCP, LOGL_DEBUG, "retransmit unanswered Requests %u x %ums;
keep sent Responses for %ums\n",
- n1, t1_ms, keep_resp_ms);
- /* If there are no retransmissions or no timeout, it makes no sense to
add to the queue. */
- if (!n1 || !t1_ms) {
- if (!m->is_response && m->ctx.resp_cb)
- m->ctx.resp_cb(m, NULL, "PFCP timeout is zero, cannot
wait for a response");
- return 0;
+ if (m->is_response) {
+ timeout_ms = ep_keep_resp(endpoint, m);
+ OSMO_LOG_PFCP_MSG(m, LOGL_DEBUG, "keep sent Responses for
%ums\n", timeout_ms);
+ } else {
+ timeout_ms = ep_t1(endpoint);
+ n1 = ep_n1(endpoint);
+
+ OSMO_LOG_PFCP_MSG(m, LOGL_DEBUG, "retransmit unanswered
Requests %u x %ums\n", n1, timeout_ms);
+ /* If there are no retransmissions or no timeout, it makes no
sense to add to the queue. */
+ if (!n1 || !timeout_ms) {
+ if (!m->is_response && m->ctx.resp_cb)
+ m->ctx.resp_cb(m, NULL, "PFCP timeout is zero,
cannot wait for a response");
+ return 0;
+ }
}
qe = talloc(endpoint, struct osmo_pfcp_queue_entry);
@@ -290,7 +300,7 @@
*qe = (struct osmo_pfcp_queue_entry){
.ep = endpoint,
.m = m,
- .n1_remaining = m->is_response ? 0 : n1,
+ .n1_remaining = n1,
};
talloc_steal(qe, m);
@@ -308,7 +318,7 @@
}
talloc_set_destructor(qe, osmo_pfcp_queue_destructor);
- osmo_timer_schedule(&qe->t1, timeout/1000, (timeout % 1000) * 1000);
+ osmo_timer_schedule(&qe->t1, timeout_ms/1000, (timeout_ms % 1000) *
1000);
return 0;
}
--
To view, visit https://gerrit.osmocom.org/c/libosmo-pfcp/+/41876?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: libosmo-pfcp
Gerrit-Branch: master
Gerrit-Change-Id: Ia461550e6791aaf00d18e0310bb4f17fdd2a3f65
Gerrit-Change-Number: 41876
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <[email protected]>
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
