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

1 files changed, 45 insertions, 45 deletions

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);