osmo-mgw[master]: client: add an optional FSM interface

dexter gerrit-no-reply at lists.osmocom.org
Mon Feb 5 11:25:17 UTC 2018


Patch Set 10:

(8 comments)

> Waiting for feedback on dlcx in cleanup question, looks fine
 > otgherwise

Hmm. I have answered. I wonder why gerrit did not flush the draft messages... Hopefully it works this time.

https://gerrit.osmocom.org/#/c/5881/9/include/osmocom/mgcp_client/mgcp_client_fsm.h
File include/osmocom/mgcp_client/mgcp_client_fsm.h:

Line 11:  * 
> naming is not clear enough "conn_info" sounds like some generic information
"peer" sounds good to me.

The call-id and the endpoint in that struct are debatable. For the CRCX when I use the struct as input parameter I need to supply an endpoint-id and a call-id.

When I get the response. I might be interested in the endpoin-id the MGW actually supplied, so I need this parameter there too. I might not necessarily need the call-id since I already know it, but we can echo it here as service for the user so he does not need to memorize it. If the endpoint-id was already specific, than its the same for the endpoint-id

Trouble starts when it comes to an MDCX. Then I definitely do not need to supply a call-id or a connection-id. The FSM knows this by itself. So in this case its not entirely clear. The only way I see to resolve this is to have separate structs for each case, but this also would complicate the API then.


https://gerrit.osmocom.org/#/c/5881/9/include/osmocom/mgcp_client/mgcp_client_internal.h
File include/osmocom/mgcp_client/mgcp_client_internal.h:

Line 32: 					struct mgcp_client *mgcp,
> unrelated whitespace change?
Done


https://gerrit.osmocom.org/#/c/5881/9/src/libosmo-mgcp-client/mgcp_client_fsm.c
File src/libosmo-mgcp-client/mgcp_client_fsm.c:

Line 7:  * it under the terms of the GNU General Public License as published by
> no AGPL in those libraries.  the old libosmo-{legacy} mgcp was plain GPLv2-
Done


Line 112: static struct msgb *make_crcx_msg_bind_connect(struct mgcp_ctx *mgcp_ctx)
> static?
Done


Line 130: static struct msgb *make_mdcx_msg(struct mgcp_ctx *mgcp_ctx)
> static?
Done


Line 153: struct msgb *make_dlcx_msg(struct mgcp_ctx *mgcp_ctx)
> static?
Done


Line 443: static void fsm_cleanup_cb(struct osmo_fsm_inst *fi, enum osmo_fsm_term_cause cause)
> what about the cleanup issuing an unconditional DLCX to the MGW, just incas
It exits always through the cleanup_cb, I thought that this is what the cleanup is for? When we terminate regularly (OSMO_FSM_TERM_REGULAR) the cleanup is executed, right?

I was thinking about an unconditional DLCX in case of error situations, we could do this, but I think it does not make too much sense. Normally errors occur because the MGW times out. This usually means that the MGW is down. Sending a DLCX in those situation does not make too much sense. Thats why I skipped this.

If we want to implement the unconditional DLCX anyway, we could go by the cause code to distinguish clean exists from unclean ones.


Line 511: 	.name = "MGCP_CONN",
> I think it's not a client FSM, but a client connection (or mgcp connection 
I have renamed it now, but I think the other namings should stay as they are, there is already the "mgcp_client" and with "mgcp_client_fsm" I want to express the close relationship between the two.


-- 
To view, visit https://gerrit.osmocom.org/5881
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I887ce0c15a831dffeb6251a975337b83942af566
Gerrit-PatchSet: 10
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-HasComments: Yes


More information about the gerrit-log mailing list