pespin has submitted this change. ( 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(-) Approvals: laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified 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 -- To view, visit https://gerrit.osmocom.org/c/libosmo-pfcp/+/39456?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: I1b7138206ed035bed3d266f713ce1dd62cbe7286 Gerrit-Change-Number: 39456 Gerrit-PatchSet: 1 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt; Gerrit-Reviewer: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt;