Change in osmo-mgw[master]: mgcp_client: allow to reset endpoints on startup

neels gerrit-no-reply at
Sat Jul 24 23:06:41 UTC 2021

neels has posted comments on this change. ( )

Change subject: mgcp_client: allow to reset endpoints on startup

Patch Set 4:

File src/libosmo-mgcp-client/mgcp_client.c: 
PS4, Line 793: 		llist_add_tail(&actual_reset_ep->list, &mgcp->actual.reset_epnames);
seems to me that it is not necessary to copy this list.
IIUC the list is only used once at program startup, just after reading in the cfg.

Would it also work to just use conf->reset_epnames on connect?
that way, a client that re-connects without restarting the program
would also start using changes made to the list by telnet VTY immediately. 
PS4, Line 854: void _ignore_mgcp_response(struct mgcp_response *response, void *priv) { }
hm, interesting! I think we should change mgcp_client_tx() to still store an entry
via mgcp_client_pending_add() even if response_cb is NULL, see (x) below.

That way we can pass NULL to ignore the response and don't get an error log for it,
and don't need to define an empty function. 
PS4, Line 905: 		epname = _mgcp_client_name_append_domain(mgcp, reset_ep->name);
I think it is more elegant to define the complete endpoint name in the config,
and to not append a domain here.

For example, I'm the admin of an osmo-mgw, and the domain configured in osmo-mgw.cfg was "@mgw",
and say I'm now changing it to "@bsc".
The next time the client restarts, I still want to make sure the legacy domain name also gets reset.
So I might want to add both


If you hardcode here that a domain is still added internally, you are taking this
configuration freedom away from the user.

So I think the solution is to simply not append a domain name,
and document that the cfg must also contain the @domain part.

If you think it is important to be able to omit the domain name in the config,
then let's only add a domain name here when there is no '@' in the name yet?
i.e. only add a domain when it is missing, so that the user can decide? 
PS4, Line 1052: 	if (response_cb != NULL) {
(x) mentioned above, here remove the response_cb != NULL condition.
That's all that's needed, because we already avoid calling a NULL response_cb in mgcp_client_handle_response(). 
File src/libosmo-mgcp-client/mgcp_client_vty.c: 
PS4, Line 161: eindpoint

Let's write "Add" instead of "Set", so it's clear that there can be more than one.

Let's write "on connect" instead of "startup", because it's actually sent in mgcp_client_connect() 
PS4, Line 175: 	/* the domain name is not part of the actual endpoint name */
(but i think it should be) 
PS4, Line 202: startup
("on connect" as above)

Let's write "Remove an endpoint name from the reset-endpoint list" 
PS4, Line 210: 			reset_ep_del = reset_ep;
break; ?  or put the free() directly in the loop and return CMD_SUCCESS here?

To view, visit
To unsubscribe, or for help writing mail filters, visit

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I47e7ff858d5067b46d52329be5f362ff61c0dff8
Gerrit-Change-Number: 25008
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier at>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at>
Gerrit-Reviewer: pespin <pespin at>
Gerrit-CC: neels <nhofmeyr at>
Gerrit-Comment-Date: Sat, 24 Jul 2021 23:06:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <>

More information about the gerrit-log mailing list