From 1a9a3ad342bcd558c8e06f238f91644116d45028 Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Sat, 23 Jul 2022 13:44:33 +0200 Subject: apply code review: refactor pfcp_endpoint API Code review requested that the API should use functions instead of direct access to a struct. I have moved all user provided config to a separate struct osmo_pfcp_endpoint_cfg, to be passed to osmo_pfcp_endpoint_create(). Halfway through those changes, I am not so certain whether that is what reviewers had in mind. It makes sense from the point of view to keep nr of arguments passed to osmo_pfcp_endpoint_create() small, and to allow changing the user provided config without requiring a new osmo_pfcp_endpoint_create2() API function. Though that again has ABI compat problems, and makes no sense from the point of view that all access should be done via API functions. Personally I don't really agree with this change, which is probably the reason why this patch ended up this way. Related: SYS#5599 Change-Id: If80c35c6a942bf9593781b5a6bc28ba37323ce5e --- include/osmocom/pfcp/pfcp_endpoint.h | 90 ++++++++++++++++++------------------ 1 file changed, 45 insertions(+), 45 deletions(-) (limited to 'include') diff --git a/include/osmocom/pfcp/pfcp_endpoint.h b/include/osmocom/pfcp/pfcp_endpoint.h index acc878e..d0ed113 100644 --- a/include/osmocom/pfcp/pfcp_endpoint.h +++ b/include/osmocom/pfcp/pfcp_endpoint.h @@ -32,13 +32,15 @@ struct osmo_pfcp_endpoint; struct osmo_fsm_inst; -#define OSMO_PFCP_TIMER_HEARTBEAT_REQ -19 -#define OSMO_PFCP_TIMER_HEARTBEAT_RESP -20 -#define OSMO_PFCP_TIMER_GRACEFUL_REL -21 -#define OSMO_PFCP_TIMER_T1 -22 -#define OSMO_PFCP_TIMER_N1 -23 -#define OSMO_PFCP_TIMER_KEEP_RESP -24 -#define OSMO_PFCP_TIMER_ASSOC_RETRY -26 +enum osmo_pfcp_timers { + OSMO_PFCP_TIMER_HEARTBEAT_REQ = -19, + OSMO_PFCP_TIMER_HEARTBEAT_RESP = -20, + OSMO_PFCP_TIMER_GRACEFUL_REL = -21, + OSMO_PFCP_TIMER_T1 = -22, + OSMO_PFCP_TIMER_N1 = -23, + OSMO_PFCP_TIMER_KEEP_RESP = -24, + OSMO_PFCP_TIMER_ASSOC_RETRY = -26, +}; extern struct osmo_tdef osmo_pfcp_tdefs[]; @@ -51,51 +53,41 @@ typedef void (*osmo_pfcp_endpoint_cb)(struct osmo_pfcp_endpoint *ep, struct osmo struct osmo_pfcp_msg *req); /* Send/receive PFCP messages to/from remote PFCP endpoints. */ -struct osmo_pfcp_endpoint { - struct { - /* Local address */ - struct osmo_sockaddr local_addr; - /* Local PFCP Node ID, as sent in outgoing messages' Node ID IE */ - struct osmo_pfcp_ie_node_id local_node_id; - - /* Timer definitions to use, if any. See t1_ms, keep_resp_ms. Use osmo_pfcp_tdefs by default. It is - * convenient to add osmo_pfcp_tdefs as one of your program's osmo_tdef_group entries and call - * osmo_tdef_vty_init() to expose PFCP timers on the VTY. */ - const struct osmo_tdef *tdefs; - } cfg; - - /* PFCP socket */ - struct osmo_fd pfcp_fd; - - /* The time at which this endpoint last restarted, as seconds since unix epoch. */ - uint32_t recovery_time_stamp; - - /* State for determining the next sequence number for transmitting a request message */ - uint32_t seq_nr_state; - - /* This function is called just after decoding and before handling the message. - * This function may set ctx.peer_fi and ctx.session_fi, used for logging context during message decoding. - * The caller may also use these fi pointers to reduce lookup iterations in rx_msg(). - */ - osmo_pfcp_endpoint_cb set_msg_ctx; +struct osmo_pfcp_endpoint; - /* Callback to receive single incoming PFCP messages from a remote peer, already decoded. */ - osmo_pfcp_endpoint_cb rx_msg; +struct osmo_pfcp_endpoint_cfg { + /* Local address */ + struct osmo_sockaddr local_addr; + /* Local PFCP Node ID, as sent in outgoing messages' Node ID IE */ + struct osmo_pfcp_ie_node_id local_node_id; + + /* If non-NULL, this function is called just after decoding and before handling the osmo_pfcp_msg passed as + * argument m. + * The caller (you) usually implements this to set m->ctx.peer_fi and m->ctx.session_fi as appropriate, + * so that these are used for logging context during message handling. The caller may also use m->ctx.peer_fi + * and m->ctx.session_fi pointers to reduce lookup iterations in e.g. rx_msg(). */ + osmo_pfcp_endpoint_cb set_msg_ctx_cb; + + /* Callback to receive a single incoming PFCP message from a remote peer, already decoded. See also the doc for + * osmo_pfcp_endpoint_cb. */ + osmo_pfcp_endpoint_cb rx_msg_cb; + + /* Custom timer definitions to use, if any. Relevant timers are: OSMO_PFCP_TIMER_N1, OSMO_PFCP_TIMER_T1, + * OSMO_PFCP_TIMER_KEEP_RESP. These are used for the PFCP message retransmission queue. + * If passed NULL, use the timer definitions from the global osmo_pfcp_tdefs. + * To expose retransmission timers on the VTY configuration, it is convenient to add osmo_pfcp_tdefs as one of + * your program's osmo_tdef_group entries and call osmo_tdef_vty_init(). */ + const struct osmo_tdef *tdefs; /* application-private data */ void *priv; - /* All transmitted PFCP Request messages, list of osmo_pfcp_queue_entry. - * 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; - /* 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; + /* Always false in this API version. When adding new members to this struct in the future, they shall be added + * after this 'more_items' flag, and such members shall be accessed only when more_items == true. */ + bool more_items; }; -struct osmo_pfcp_endpoint *osmo_pfcp_endpoint_create(void *ctx, void *priv); +struct osmo_pfcp_endpoint *osmo_pfcp_endpoint_create(void *ctx, const struct osmo_pfcp_endpoint_cfg *cfg); int osmo_pfcp_endpoint_bind(struct osmo_pfcp_endpoint *ep); void osmo_pfcp_endpoint_close(struct osmo_pfcp_endpoint *ep); void osmo_pfcp_endpoint_free(struct osmo_pfcp_endpoint **ep); @@ -105,3 +97,11 @@ int osmo_pfcp_endpoint_tx_data(struct osmo_pfcp_endpoint *ep, struct osmo_pfcp_m int osmo_pfcp_endpoint_tx_heartbeat_req(struct osmo_pfcp_endpoint *ep, const struct osmo_sockaddr *remote_addr); void osmo_pfcp_endpoint_invalidate_ctx(struct osmo_pfcp_endpoint *ep, struct osmo_fsm_inst *deleted_fi); + +const struct osmo_pfcp_endpoint_cfg *osmo_pfcp_endpoint_get_cfg(const struct osmo_pfcp_endpoint *ep); +void *osmo_pfcp_endpoint_get_priv(const struct osmo_pfcp_endpoint *ep); +uint32_t osmo_pfcp_endpoint_get_recovery_timestamp(const struct osmo_pfcp_endpoint *ep); +const struct osmo_sockaddr *osmo_pfcp_endpoint_get_local_addr(const struct osmo_pfcp_endpoint *ep); +void osmo_pfcp_endpoint_set_seq_nr_state(struct osmo_pfcp_endpoint *ep, uint32_t seq_nr_state); + +bool osmo_pfcp_endpoint_retrans_queue_is_busy(const struct osmo_pfcp_endpoint *ep); -- cgit v1.2.3