[S] Change in libosmo-pfcp[master]: Avoid marking rx PFCP Assoc Setup Req as duplicate

[pespin]

pespin has uploaded this change for review. ( 
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(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmo-pfcp refs/changes/76/41876/1

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: newchange
Gerrit-Project: libosmo-pfcp
Gerrit-Branch: master
Gerrit-Change-Id: Ia461550e6791aaf00d18e0310bb4f17fdd2a3f65
Gerrit-Change-Number: 41876
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pes...@sysmocom.de>