osmo-bsc[master]: mgcp: cancel transactions on timeout

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Fri Dec 15 16:30:38 UTC 2017


Patch Set 4: Code-Review+2

(2 comments)

https://gerrit.osmocom.org/#/c/5157/1/include/osmocom/bsc/osmo_bsc_mgcp.h
File include/osmocom/bsc/osmo_bsc_mgcp.h:

Line 47: 	enum gsm48_chan_mode chan_mode;
> When I send out an MGCP message using mgcp_client_tx() I expect that the cl
The question asked comes down to: is the struct mgcp_ctx per call or per entire osmo-bsc instance. In other places we use the foo_ctx naming for global settings, so I too first thought this was one global struct for all, which of course it isn't: on current master, your comment already clarifies that this mgcp_ctx is the context for one subscriber conn, so even though the name might be tweaked to reflect that (struct conn_mgcp or something?), this implementation is perfectly fine for us, since you've confirmed that for each conn we go through the messages sequentially (send the next request only after having received a previous response). 

Overall it is of course possible and necessary and supported that the MGCP requests across different conns and endpoints get handled in any random order. The only limitation is that here we can store only a single pending transaction to be able to cancel it later if required.


https://gerrit.osmocom.org/#/c/5157/1/src/osmo-bsc/osmo_bsc_mgcp.c
File src/osmo-bsc/osmo_bsc_mgcp.c:

Line 825: 		mgcp_client_cancel(mgcp, mgcp_ctx->mgw_pending_trans);
> It should not because we do this only when an MGW transaction timed out, so
I'm also thinking, since this is an issue for all MGCP clients, it would make sense to incorporate an explicit endpoint context with a list of pending transactions in the API itself, which would alleviate all of these questions in a clear and universal way... obviously not for this patch.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40794dff7d10e2b6a96863a2da7e9fbd5662a1bf
Gerrit-PatchSet: 4
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list