pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-pfcp/+/39456?usp=email )
Change subject: Use hashtable to look up req/resp by seq_nr ......................................................................
Use hashtable to look up req/resp by seq_nr
The list of requests/responses may grow quite a lot on busy instances.
Related: SYS#6398 Change-Id: I1b7138206ed035bed3d266f713ce1dd62cbe7286 --- M src/libosmo-pfcp/pfcp_endpoint.c 1 file changed, 39 insertions(+), 17 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-pfcp refs/changes/56/39456/1
diff --git a/src/libosmo-pfcp/pfcp_endpoint.c b/src/libosmo-pfcp/pfcp_endpoint.c index 7e08d8e..e2b25d4 100644 --- a/src/libosmo-pfcp/pfcp_endpoint.c +++ b/src/libosmo-pfcp/pfcp_endpoint.c @@ -28,6 +28,8 @@ #include <osmocom/core/talloc.h> #include <osmocom/core/timer.h> #include <osmocom/core/tdef.h> +#include <osmocom/core/linuxlist.h> +#include <osmocom/core/hashtable.h>
#include <osmocom/pfcp/pfcp_endpoint.h> #include <osmocom/pfcp/pfcp_msg.h> @@ -49,16 +51,20 @@ * For a transmitted Request message, wait for a matching Response from a remote peer; if none arrives, * retransmit (see n1 and t1_ms). */ struct llist_head sent_requests; + DECLARE_HASHTABLE(sent_requests_by_seq_nr, 10); /* All transmitted PFCP Response messages, list of osmo_pfcp_queue_entry. * For a transmitted Response message, keep it in the queue for a fixed amount of time. If the peer retransmits * the original Request, do not dispatch the Request, but respond with the queued message directly. */ struct llist_head sent_responses; + DECLARE_HASHTABLE(sent_responses_by_seq_nr, 10); };
/*! Entry of pfcp_endpoint message queue of PFCP messages, for re-transsions. */ struct osmo_pfcp_queue_entry { /* entry in osmo_pfcp_endpoint.sent_requests or .sent_responses */ struct llist_head entry; + /* item in osmo_pfcp_endpoint's sent_responses_by_seq_nr or sent_responses_by_seq_nr */ + struct hlist_node node_by_seq_nr; /* back-pointer */ struct osmo_pfcp_endpoint *ep; /* message we have transmitted */ @@ -70,22 +76,6 @@ unsigned int n1_remaining; };
-/* Find a matching osmo_pfcp_queue_entry for given rx. - * A returned osmo_pfcp_queue_entry is guaranteed to be a Response if rx is a Request, and vice versa. */ -static struct osmo_pfcp_queue_entry *osmo_pfcp_queue_find(struct llist_head *queue, const struct osmo_pfcp_msg *rx) -{ - struct osmo_pfcp_queue_entry *qe; - /* It's important to match only a Request to a Response and vice versa, because the remote peer makes its own - * sequence_nr. There could be a collision of sequence_nr. But as long as all Requests look for a Response and - * vice versa, the sequence_nr scopes don't overlap. */ - llist_for_each_entry(qe, queue, entry) { - if (qe->m->is_response != rx->is_response - && qe->m->h.sequence_nr == rx->h.sequence_nr) - return qe; - } - return NULL; -} - /* clean up and deallocate the given osmo_pfcp_queue_entry */ static void osmo_pfcp_queue_del(struct osmo_pfcp_queue_entry *qe) { @@ -96,6 +86,7 @@ static int osmo_pfcp_queue_destructor(struct osmo_pfcp_queue_entry *qe) { osmo_timer_del(&qe->t1); + hash_del(&qe->node_by_seq_nr); llist_del(&qe->entry); return 0; } @@ -144,6 +135,8 @@
INIT_LLIST_HEAD(&ep->sent_requests); INIT_LLIST_HEAD(&ep->sent_responses); + hash_init(ep->sent_requests_by_seq_nr); + hash_init(ep->sent_responses_by_seq_nr);
ep->pfcp_fd.fd = -1;
@@ -311,9 +304,11 @@ * Add sent responses to the end of the list: they will rarely be retransmitted at all. */ if (m->is_response) { llist_add_tail(&qe->entry, &endpoint->sent_responses); + hash_add(endpoint->sent_responses_by_seq_nr, &qe->node_by_seq_nr, m->h.sequence_nr); osmo_timer_setup(&qe->t1, pfcp_queue_sent_resp_timer_cb, qe); } else { llist_add(&qe->entry, &endpoint->sent_requests); + hash_add(endpoint->sent_requests_by_seq_nr, &qe->node_by_seq_nr, m->h.sequence_nr); osmo_timer_setup(&qe->t1, pfcp_queue_sent_req_timer_cb, qe); } talloc_set_destructor(qe, osmo_pfcp_queue_destructor); @@ -353,6 +348,30 @@ return 0; }
+static struct osmo_pfcp_queue_entry *osmo_pfcp_enfpoint_find_sent_request(const struct osmo_pfcp_endpoint *ep, uint32_t seq_nr) +{ + struct osmo_pfcp_queue_entry *qe; + hash_for_each_possible(ep->sent_requests_by_seq_nr, qe, node_by_seq_nr, seq_nr) { + OSMO_ASSERT(qe->m); + if (qe->m->h.sequence_nr != seq_nr) + continue; + return qe; + } + return NULL; +} + +static struct osmo_pfcp_queue_entry *osmo_pfcp_enfpoint_find_sent_response(const struct osmo_pfcp_endpoint *ep, uint32_t seq_nr) +{ + struct osmo_pfcp_queue_entry *qe; + hash_for_each_possible(ep->sent_responses_by_seq_nr, qe, node_by_seq_nr, seq_nr) { + OSMO_ASSERT(qe->m); + if (qe->m->h.sequence_nr != seq_nr) + continue; + return qe; + } + return NULL; +} + static void osmo_pfcp_endpoint_handle_rx(struct osmo_pfcp_endpoint *ep, struct osmo_pfcp_msg *m) { bool dispatch_rx = true; @@ -370,7 +389,10 @@ /* If this is receiving a response, search for matching sent request that is now completed. * If this is receiving a request, search for a matching sent response that can be retransmitted. * A match is found by sequence_nr. */ - prev_msg = osmo_pfcp_queue_find(m->is_response ? &ep->sent_requests : &ep->sent_responses, m); + if (m->is_response) + prev_msg = osmo_pfcp_enfpoint_find_sent_request(ep, m->h.sequence_nr); + else + prev_msg = osmo_pfcp_enfpoint_find_sent_response(ep, m->h.sequence_nr);
if (prev_msg && !m->is_response) { /* m is a request, and we have already sent a response to this same request earlier. Retransmit the same