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 gerrit-no-reply at lists.osmocom.orgneels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/25008 )
Change subject: mgcp_client: allow to reset endpoints on startup
......................................................................
Patch Set 4:
(8 comments)
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client.c
File src/libosmo-mgcp-client/mgcp_client.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client.c@793
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.
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client.c@854
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.
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client.c@905
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
rtpbridge/*@mgw
rtpbridge/*@bsc
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?
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client.c@1052
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().
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client_vty.c
File src/libosmo-mgcp-client/mgcp_client_vty.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client_vty.c@161
PS4, Line 161: eindpoint
"endpoint"
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()
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client_vty.c@175
PS4, Line 175: /* the domain name is not part of the actual endpoint name */
(but i think it should be)
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client_vty.c@202
PS4, Line 202: startup
("on connect" as above)
Let's write "Remove an endpoint name from the reset-endpoint list"
https://gerrit.osmocom.org/c/osmo-mgw/+/25008/4/src/libosmo-mgcp-client/mgcp_client_vty.c@210
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 https://gerrit.osmocom.org/c/osmo-mgw/+/25008
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I47e7ff858d5067b46d52329be5f362ff61c0dff8
Gerrit-Change-Number: 25008
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-CC: neels <nhofmeyr at sysmocom.de>
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: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210724/ca8b39c9/attachment.htm>