neels has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/libosmo-pfcp/+/28750 )


Change subject: clarify osmo_pfcp_msg alloc API
......................................................................

clarify osmo_pfcp_msg alloc API

Looking at the osmo_pfcp_msg_alloc API with a bit of distance now, I
found that:

- it is confusing to have a single function for req and resp. A resp
  may pass remote_addr as NULL, and a req may pass in_reply_to as NULL.
  Make this much more obvious with separate req/resp functions.

- the osmo_pfcp_endpoint_tx() implicitly puts the local Node ID into
  sent PFCP messages, so the local_node_id arg for msg alloc is
  redundant. Drop that.

Refactor without backwards compat, because we have not yet officially
released this API. This requires a fixup patch to osmo-upf.git (and
affects unmerged patches to osmo-hnbgw.git).

Related: SYS#5599
Related: I73e6da3b80f05e9408c81f41ac05d6578b8e31cf (osmo-upf)
Change-Id: I0d71134e42932cc72992eba73a15e82bc7cd11bd
---
M include/osmocom/pfcp/pfcp_cp_peer.h
M include/osmocom/pfcp/pfcp_msg.h
M src/libosmo-pfcp/pfcp_cp_peer.c
M src/libosmo-pfcp/pfcp_endpoint.c
M src/libosmo-pfcp/pfcp_msg.c
5 files changed, 48 insertions(+), 27 deletions(-)



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

diff --git a/include/osmocom/pfcp/pfcp_cp_peer.h 
b/include/osmocom/pfcp/pfcp_cp_peer.h
index 8c6e448..e555a5f 100644
--- a/include/osmocom/pfcp/pfcp_cp_peer.h
+++ b/include/osmocom/pfcp/pfcp_cp_peer.h
@@ -58,6 +58,9 @@
                                                  const struct osmo_sockaddr 
*remote_addr);
 int osmo_pfcp_cp_peer_associate(struct osmo_pfcp_cp_peer *cp_peer);
 bool osmo_pfcp_cp_peer_is_associated(const struct osmo_pfcp_cp_peer *cp_peer);
-struct osmo_pfcp_msg *osmo_pfcp_cp_peer_new_msg_tx(struct osmo_pfcp_cp_peer 
*cp_peer,
-                                                  enum osmo_pfcp_message_type 
msg_type);
+struct osmo_pfcp_msg *osmo_pfcp_cp_peer_new_req(struct osmo_pfcp_cp_peer 
*cp_peer,
+                                               enum osmo_pfcp_message_type 
msg_type);
+struct osmo_pfcp_msg *osmo_pfcp_cp_peer_new_resp(struct osmo_pfcp_cp_peer 
*cp_peer,
+                                                const struct osmo_pfcp_msg 
*in_reply_to,
+                                                enum osmo_pfcp_message_type 
msg_type);
 void osmo_pfcp_cp_peer_set_msg_ctx(struct osmo_pfcp_cp_peer *cp_peer, struct 
osmo_pfcp_msg *m);
diff --git a/include/osmocom/pfcp/pfcp_msg.h b/include/osmocom/pfcp/pfcp_msg.h
index 2833408..4affa54 100644
--- a/include/osmocom/pfcp/pfcp_msg.h
+++ b/include/osmocom/pfcp/pfcp_msg.h
@@ -169,10 +169,10 @@
 int osmo_pfcp_msg_decode_tlv(struct osmo_pfcp_msg *m, struct osmo_gtlv_load 
*tlv);

 struct osmo_pfcp_msg *osmo_pfcp_msg_alloc_rx(void *ctx, const struct 
osmo_sockaddr *remote_addr);
-struct osmo_pfcp_msg *osmo_pfcp_msg_alloc_tx(void *ctx, const struct 
osmo_sockaddr *remote_addr,
-                                            const struct osmo_pfcp_ie_node_id 
*local_node_id,
-                                            const struct osmo_pfcp_msg 
*in_reply_to,
-                                            enum osmo_pfcp_message_type 
msg_type);
+struct osmo_pfcp_msg *osmo_pfcp_msg_alloc_tx_req(void *ctx, const struct 
osmo_sockaddr *remote_addr,
+                                                enum osmo_pfcp_message_type 
msg_type);
+struct osmo_pfcp_msg *osmo_pfcp_msg_alloc_tx_resp(void *ctx, const struct 
osmo_pfcp_msg *in_reply_to,
+                                                 enum osmo_pfcp_message_type 
msg_type);

 void osmo_pfcp_msg_invalidate_ctx(struct osmo_pfcp_msg *m, struct 
osmo_fsm_inst *deleted_fi);

diff --git a/src/libosmo-pfcp/pfcp_cp_peer.c b/src/libosmo-pfcp/pfcp_cp_peer.c
index c47b440..e1dd9ac 100644
--- a/src/libosmo-pfcp/pfcp_cp_peer.c
+++ b/src/libosmo-pfcp/pfcp_cp_peer.c
@@ -160,7 +160,7 @@
        struct osmo_pfcp_cp_peer *cp_peer = fi->priv;
        struct osmo_pfcp_msg *m;

-       m = osmo_pfcp_cp_peer_new_msg_tx(cp_peer, 
OSMO_PFCP_MSGT_ASSOC_SETUP_REQ);
+       m = osmo_pfcp_cp_peer_new_req(cp_peer, OSMO_PFCP_MSGT_ASSOC_SETUP_REQ);
        m->ies.assoc_setup_req.recovery_time_stamp = 
cp_peer->ep->recovery_time_stamp;

        m->ies.assoc_setup_req.cp_function_features_present = true;
@@ -391,14 +391,25 @@
        osmo_use_count_get_put(m->ctx.peer_use_count, m->ctx.peer_use_token, 1);
 }

-struct osmo_pfcp_msg *osmo_pfcp_cp_peer_new_msg_tx(struct osmo_pfcp_cp_peer 
*cp_peer,
-                                                  enum osmo_pfcp_message_type 
msg_type)
+/* Allocate a new PFCP request message to be sent to cp_peer->remote_addr. */
+struct osmo_pfcp_msg *osmo_pfcp_cp_peer_new_req(struct osmo_pfcp_cp_peer 
*cp_peer,
+                                               enum osmo_pfcp_message_type 
msg_type)
 {
        struct osmo_pfcp_msg *m;
-       m = osmo_pfcp_msg_alloc_tx(cp_peer->ep, &cp_peer->remote_addr, 
&cp_peer->ep->cfg.local_node_id, NULL,
-                                  msg_type);
-       if (!m)
-               return m;
+       m = osmo_pfcp_msg_alloc_tx_req(cp_peer->ep, &cp_peer->remote_addr, 
msg_type);
+       OSMO_ASSERT(m);
+       osmo_pfcp_cp_peer_set_msg_ctx(cp_peer, m);
+       return m;
+}
+
+/* Allocate a new PFCP response message to be sent to cp_peer->remote_addr. */
+struct osmo_pfcp_msg *osmo_pfcp_cp_peer_new_resp(struct osmo_pfcp_cp_peer 
*cp_peer,
+                                                const struct osmo_pfcp_msg 
*in_reply_to,
+                                                enum osmo_pfcp_message_type 
msg_type)
+{
+       struct osmo_pfcp_msg *m;
+       m = osmo_pfcp_msg_alloc_tx_resp(cp_peer->ep, in_reply_to, msg_type);
+       OSMO_ASSERT(m);
        osmo_pfcp_cp_peer_set_msg_ctx(cp_peer, m);
        return m;
 }
diff --git a/src/libosmo-pfcp/pfcp_endpoint.c b/src/libosmo-pfcp/pfcp_endpoint.c
index f446858..6162086 100644
--- a/src/libosmo-pfcp/pfcp_endpoint.c
+++ b/src/libosmo-pfcp/pfcp_endpoint.c
@@ -242,8 +242,7 @@

 int osmo_pfcp_endpoint_tx_heartbeat_req(struct osmo_pfcp_endpoint *ep, const 
struct osmo_sockaddr *remote_addr)
 {
-       struct osmo_pfcp_msg *tx = osmo_pfcp_msg_alloc_tx(OTC_SELECT, 
remote_addr, NULL, NULL,
-                                                         
OSMO_PFCP_MSGT_HEARTBEAT_REQ);
+       struct osmo_pfcp_msg *tx = osmo_pfcp_msg_alloc_tx_req(OTC_SELECT, 
remote_addr, OSMO_PFCP_MSGT_HEARTBEAT_REQ);
        tx->ies.heartbeat_req.recovery_time_stamp = ep->recovery_time_stamp;
        tx->h.sequence_nr = osmo_pfcp_next_seq_nr(&ep->seq_nr_state);
        return osmo_pfcp_endpoint_tx_data(ep, tx);
@@ -326,7 +325,7 @@

        if (m->h.message_type == OSMO_PFCP_MSGT_HEARTBEAT_REQ) {
                /* Directly answer with a Heartbeat Response. */
-               struct osmo_pfcp_msg *resp = osmo_pfcp_msg_alloc_tx(OTC_SELECT, 
NULL, NULL, m, OSMO_PFCP_MSGT_HEARTBEAT_RESP);
+               struct osmo_pfcp_msg *resp = 
osmo_pfcp_msg_alloc_tx_resp(OTC_SELECT, m, OSMO_PFCP_MSGT_HEARTBEAT_RESP);
                resp->ies.heartbeat_resp.recovery_time_stamp = 
ep->recovery_time_stamp;
                osmo_pfcp_endpoint_tx_data(ep, resp);
                /* Still also dispatch the Rx event to the peer. */
diff --git a/src/libosmo-pfcp/pfcp_msg.c b/src/libosmo-pfcp/pfcp_msg.c
index 9d65efc..5f55575 100644
--- a/src/libosmo-pfcp/pfcp_msg.c
+++ b/src/libosmo-pfcp/pfcp_msg.c
@@ -418,31 +418,39 @@
        return rx;
 }

-struct osmo_pfcp_msg *osmo_pfcp_msg_alloc_tx(void *ctx, const struct 
osmo_sockaddr *remote_addr,
-                                            const struct osmo_pfcp_ie_node_id 
*node_id,
-                                            const struct osmo_pfcp_msg 
*in_reply_to,
-                                            enum osmo_pfcp_message_type 
msg_type)
+static struct osmo_pfcp_msg *osmo_pfcp_msg_alloc_tx(void *ctx, const struct 
osmo_sockaddr *remote_addr,
+                                                   const struct osmo_pfcp_msg 
*in_reply_to,
+                                                   enum osmo_pfcp_message_type 
msg_type)
 {
        struct osmo_pfcp_msg *tx;
        if (!remote_addr && in_reply_to)
                remote_addr = &in_reply_to->remote_addr;
        OSMO_ASSERT(remote_addr);
        tx = _osmo_pfcp_msg_alloc(ctx, remote_addr);
+       OSMO_ASSERT(tx);
        tx->is_response = osmo_pfcp_msgtype_is_response(msg_type);
        tx->h.message_type = msg_type;
        if (in_reply_to)
                tx->h.sequence_nr = in_reply_to->h.sequence_nr;
        osmo_pfcp_msg_set_memb_ofs(tx);
-
-       /* Write the local node id data to the correct tx->ies.* member. */
-       if (node_id) {
-               struct osmo_pfcp_ie_node_id *tx_node_id = 
osmo_pfcp_msg_node_id(tx);
-               if (tx_node_id)
-                       *tx_node_id = *node_id;
-       }
        return tx;
 }

+/* Allocate a new PFCP Request message to be transmitted to a peer. */
+struct osmo_pfcp_msg *osmo_pfcp_msg_alloc_tx_req(void *ctx, const struct 
osmo_sockaddr *remote_addr,
+                                                enum osmo_pfcp_message_type 
msg_type)
+{
+       return osmo_pfcp_msg_alloc_tx(ctx, remote_addr, NULL, msg_type);
+}
+
+/* Allocate a new PFCP Response message to be transmitted to a peer, as a 
response to a received PFCP message.
+ * Pass the received PFCP Request in in_reply_to; take the remote address and 
sequence nr from in_reply_to. */
+struct osmo_pfcp_msg *osmo_pfcp_msg_alloc_tx_resp(void *ctx, const struct 
osmo_pfcp_msg *in_reply_to,
+                                                 enum osmo_pfcp_message_type 
msg_type)
+{
+       return osmo_pfcp_msg_alloc_tx(ctx, NULL, in_reply_to, msg_type);
+}
+
 static int osmo_pfcp_msg_destructor(struct osmo_pfcp_msg *m)
 {
        OSMO_LOG_PFCP_MSG(m, LOGL_DEBUG, "discarding\n");

--
To view, visit https://gerrit.osmocom.org/c/libosmo-pfcp/+/28750
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-pfcp
Gerrit-Branch: master
Gerrit-Change-Id: I0d71134e42932cc72992eba73a15e82bc7cd11bd
Gerrit-Change-Number: 28750
Gerrit-PatchSet: 1
Gerrit-Owner: neels <[email protected]>
Gerrit-MessageType: newchange

Reply via email to