Project

General

Profile

Bug #2823

Use bsc_subscr_conn_fsm in BSC

Added by dexter 10 days ago. Updated 3 days ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
A interface (general)
Target version:
-
Start date:
01/08/2018
Due date:
% Done:

40%

Resolution:

Description

On laforge/fsm a draft FSM implementation can be found to make the handling of the subscriber connection safer and stateful.

History

#1 Updated by dexter 10 days ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 30

#2 Updated by dexter 10 days ago

I have read through the code of the FSM implementation. There is still a lot of stuff that needs to be completed, especially the handover stuff. I am not entirely sure if this FSM is breaking existing handover features. That would be good to know for sure first.

Also the there are a lot of signals sent from random code locations. Thats probably intentional, but maybe it would be better to call a function inside bsc_subscr_conn_fsm.c that then sends the signal like we did it with the MGCP FSMs. This would allow us to run some checks and so some assertions.

The FSM had a small bug that prevented it from working probably when making calls. I fixed that and it seems to run really well!

#3 Updated by dexter 8 days ago

I have tested the internal handover with the FSM. It works fine, so the the FSM did not break anything. Also the FSM now cleans up/frees correctly. The synchronization between the MGCP FSM is still problematic. We can not run them separately, they need to be synchronized because the port/ip information for the RTP streams becomes known at different times.

#4 Updated by dexter 6 days ago

While integrating the FSM I ran into some trouble, nothing that couldn't be fixed, but after discussion with Harald we decided to go a bit further and clean up the FSM that controls the MGCP side. The vision is to have a generalized FSM that can be used in osmo-bsc, osmo-msc and in any other project that somehow needs to interact with an MGW. The FSM works as a child FSM of some parent (in the bsc case this is the GSCON FSM).

For now the FSM has two states ST_READY and ST_WAIT. In ST_READY we wait for an action to do e.g. EV_MDCX. Then we call the mgcp_client, the mgcp_client calls the callback and the callback sends back an EV_MDCX_RESP. The code in ST_WAIT gets executed and a signal is sent back to the parant FSM. This works the dams for DLCX and CRCX. CRCX is an exception. When the function to execute the CRCX is calld, then the FSM is created and the EV_CRCX automatically issued. This is the only way to perform an CRCX, so once the FSM came up successfully we can be sure that the connection exists.

In case of error the FSM shall unlink from its parent, inform the parent via the term event and then safely destroy itsself. This allows us to tear down the SCCP connection quickly while the MGCP cleanups are still in progress. Its also much simpler for the parent FSM since the parent does not have to synchronize.

The API is just a set of three function, one for CRCX (sets up the FSM and does the actual CRCX), one for MDCX and one for DLCX (also takes care of cleaning up everything)

/*! allocate FSM, and create a new connection on the MGW.
 *  \param[in] mgcp MGCP client descriptor.
 *  \param[in] mgcpparent_fi Parent FSM instance.
 *  \param[in] parent_term_evt Event to be sent to parent when terminating.
 *  \param[in] parent_evt Event to be sent to parent when operation is done.
 *  \param[in] endpoint Optional endpoint identifier string.
 *  \param[in] addr Optional ip-address string.
 *  \param[in] port Optional ip-port number.
 *  \returns newly-allocated, initialized and registered FSM instance, NULL on error. */
struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm_inst *parent_fi, uint32_t parent_term_evt,
                       uint32_t parent_evt, char *endpoint, char *addr, uint16_t port)

/*! modify an existing connection on the MGW.
 *  \param[in] fi FSM instance.
 *  \param[in] parent_evt Event to be sent to parent when operation is done.
 *  \param[in] addr New ip-address string.
 *  \param[in] port New ip-port number.
 *  \returns 0 on success, -EINVAL on error. */
int mgcp_conn_modify(struct osmo_fsm_inst *fi, uint32_t parent_evt, char *addr, uint16_t port)

/*! delete existing connection on the MGW, destroy FSM afterwards.
 *  \param[in] fi FSM instance.
 *  \returns 0 on success, -EINVAL on error. */
int mgcp_conn_delete(struct osmo_fsm_inst *fi)

The signals that are sent back to the parent fsm will always contain a pointer to the private data of the FSM, from there the parent can get relevant information (ports, IP-Address, Endpoint identifier etc.)

I am directly implementing the FSM in osmo-mgw, so it won't be in osmo-bsc, osmo-bsc will just use it and we may also decide to use it in osmo-msc as well once we are confident that it works.

#5 Updated by laforge 5 days ago

On Fri, Jan 12, 2018 at 05:43:27PM +0000, dexter [REDMINE] wrote:

For now the FSM has two states ST_READY and ST_WAIT.

I would make sure to introduce more states. In the initial state, until the CRCX is acknowledged,
no MDCX is permitted. MDCX is only permitted after a successful CRCX response has been received,
so you need to differentiate thie state from the other?

The API is just a set of three function, one for CRCX (sets up the FSM and does the actual CRCX), one for MDCX and one for DLCX (also takes care of cleaning up everything)

struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm_inst *parent_fi, uint32_t parent_term_evt,
uint32_t parent_evt, char *endpoint, char *addr, uint16_t port)

I think the API should be more future-proof and flexible. Let's make sure that at leaset the addr/port
parameters are passed in via some struct pointer. This way we can extend it later on without breaking
the ABI.

The client will soon / should for example provide the SDP related bits such as which codec shall be used for that connection, ... - and once we start doing this, we don't want to break the API again and again.

It might make sense to have a struct for the local (mgw) side of the connection and another (or a copy of the saeme struct?) for the remote (bts/core network) side. If only the "local" is passed in while the "remote" struct is NULL, then the CRCX only does the "bind" operation. If both local+remote are specified, it performs bind+connect. This should map to the concepts of MGCP (local / remote connection options).

I am directly implementing the FSM in osmo-mgw, so it won't be in osmo-bsc, osmo-bsc will just use it and we may also decide to use it in osmo-msc as well once we are confident that it works.

great.

#6 Updated by dexter 3 days ago

  • % Done changed from 30 to 40

I would make sure to introduce more states...

I see, I have now moved more of the logic into separate states. In the end I think it also makes the code easier to understand. We have now:

    ST_CRCX,
    ST_CRCX_RESP,
    ST_READY,
    ST_MDCX_RESP,
    ST_DLCX_RESP,

ST_CRCX is entered immediately on startup. It sends the CRCX message to the MGW and enters ST_CRCX_RESP. When the response is received the FSM fires an EV_CRCX_RESP to tell the parent that the CRCX was successful. Then it waits in ST_READY for further operations. For the other operations, the mechanism is nearly the same. When e.g. an EV_MDCX is received, the a message is generated and sent to the MGW. We enter ST_MDCX_RESP and wait for the MGW response. When the response is received, we fire the matching event and return back to ST_READY.

Let's make sure that at leaset the addr/port

The parameters that relate on the connection are now encapsulated in struct mgcp_conn_info (see below). The same struct is used to return results with in the event pointer. This allows us to have the struct with the context information entirely private. I have removed it from the header file now, so that it is inaccessable from outside.

...If both local+remote are specified...

On the CRCX we may or may not pass in an struct mgcp_conn_info *conn_info. If it is not passed, then the we know that the user only wants to bind. If it is there it we know that the user wants to do bind+connect. The result is returned with the event pointer. (I do not really get what you mean. Why would we want to pass remote and local to the create function? To tell it a memory location where it can write to? I have reserved fixed memory in the contect struct now. I think this is more convenient, since we can return the pointer with the signal and the callee does not have to take care for the memory.)

/*! Connection information. This struct organizes the connection infromation
 *  one connection side (either remote or local). It is used to pass parameters
 *  (local) to the FSM and get responses (remote) from the FSM as pointer
 *  attached to the FSM event */
struct mgcp_conn_info {
    /*!< RTP connection IP-Address (string) */
    char addr[INET_ADDRSTRLEN];

    /*!< RTP connection IP-Port */
    uint16_t port;
};

/*! allocate FSM, and create a new connection on the MGW.
 *  \param[in] mgcp MGCP client descriptor.
 *  \param[in] mgcpparent_fi Parent FSM instance.
 *  \param[in] parent_term_evt Event to be sent to parent when terminating.
 *  \param[in] parent_evt Event to be sent to parent when operation is done.
 *  \param[in] endpoint Endpoint identifier string.
 *  \param[in] conn_info Optional connection information (ip, port...).
 *  \returns newly-allocated, initialized and registered FSM instance, NULL on error. */
struct osmo_fsm_inst *mgcp_conn_create(struct mgcp_client *mgcp, struct osmo_fsm_inst *parent_fi, uint32_t parent_term_evt,
                       uint32_t parent_evt, char *endpoint, struct mgcp_conn_info *conn_info)

/*! modify an existing connection on the MGW.
 *  \param[in] fi FSM instance.
 *  \param[in] parent_evt Event to be sent to parent when operation is done.
 *  \param[in] conn_info New connection information (ip, port...).
 *  \returns 0 on success, -EINVAL on error. */
int mgcp_conn_modify(struct osmo_fsm_inst *fi, uint32_t parent_evt, struct mgcp_conn_info *conn_info)

/*! delete existing connection on the MGW, destroy FSM afterwards.
 *  \param[in] fi FSM instance. */
void mgcp_conn_delete(struct osmo_fsm_inst *fi)

So far I have now the CRCX and the error handling (timeout, cleanup...) working. For debugging I created myself a small test FSM that acts as parent. There is also the question what should happen if someone tries to execute an operation while the FSM is budy. Under normal conditions this should never happen, since Child and parent are synchroinzed through events. But for DLCX we need something. I have solved this with a flag. If someone tries to trigger a DLCX while the FSM is busy, it just sets the flag and does nothing. When the FSM is done with the pending operation it checks the flag. If it finds the flag set. It continues with the DLCX operation.

Also available in: Atom PDF