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 <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: dexter <[email protected]> Gerrit-HasComments: Yes
