Change in osmo-mgw[master]: add a VTY command which shows a specific mgcp endpoint

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
Thu Jul 19 00:02:19 UTC 2018


Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/10022 )

Change subject: add a VTY command which shows a specific mgcp endpoint
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

tldr: yes, but let's discuss UI vs implementation quirks with pmaier first.

https://gerrit.osmocom.org/#/c/10022/1/src/libosmo-mgcp/mgcp_vty.c
File src/libosmo-mgcp/mgcp_vty.c:

https://gerrit.osmocom.org/#/c/10022/1/src/libosmo-mgcp/mgcp_vty.c@193
PS1, Line 193: 	vty_out(vty, "Endpoint %s0x%.2x:%s",
not sure if we should include a 0x here (I remember there has been confusion around decimal vs hex on that topic before, and I used to argue for a 0x in log output; which doesn't really make sense to me anymore). Rather stay with the exact syntax sent via MGCP?

Interesting, what does the '.2' do for %x?

edit 1: Ah I see now you're just moving previous code. These remain interesting questions but not related to this patch then.

edit 2: wait no, you are actually changing the format to prepend "rtpbridge/". So again yes, with this change let's copy the code from actual MGCP message composition? (it's sent as character string)


https://gerrit.osmocom.org/#/c/10022/1/src/libosmo-mgcp/mgcp_vty.c@201
PS1, Line 201: 			/* FIXME: Also add verbosity for other
use the line width instead of breaking lines?

edit: also just moved code


https://gerrit.osmocom.org/#/c/10022/1/src/libosmo-mgcp/mgcp_vty.c@256
PS1, Line 256:       "Display information about an MGCP Media Gateway endpoint\n"
Common prefix with other commands, here 'show mgcp', should share similar doc strings. This means that here it should also say "..about the MGCP Media Gateway" like above, and in fact it's a candidate for a common #define like the SHOW_STR.


What is the trunk number for the user? is it what follows "rtpbridge/" in the endpoint name we got from the MGW? No, that's part of the endpoint NAME, is it?? Looks to me like also allowing to omit 'endpoint NAME' would be good, to show one entire trunk? and/or to allow passing an endpoint name without having to pass a trunk number? (I think I'm confused on what a trunk is)

e.g. when reading osmo-bsc logs, AFAICT an endpoint name looks like "rtpbridge/2 at mgw", I think we should use similar syntax here to cut out conversions that the user needs to understand to use this command. The command should be so that we can seamlessly add e1 endpoints in the future, hiding weird implementation details (like a hex number) we might have at the moment. Would be nice if pmaier could say something here. The idea: just printing out a listing is less critical than exposing VTY commands; we should try to not require to incompatibly change the vty command UI in the future. So we may prefer to not glorify the weird hex number as proper VTY UI, when the MGCP protocol only ever has character strings. Am I making sense?

(also thinking, connections on each endpoint have long random generated ids, CI, extremely likely to be unique across all the MGW. Could be nice to be able to query by CI without having to also pass the endpoint name? oh well, maybe not that useful, querying by endpoint name should suffice. What does pmaier think?)


https://gerrit.osmocom.org/#/c/10022/1/src/libosmo-mgcp/mgcp_vty.c@280
PS1, Line 280: 		vty_out(vty, "endpoint name '%s' is not a hex number%s", argv[1], VTY_NEWLINE);
(re "rtpbridge/2 at mgw" above)



-- 
To view, visit https://gerrit.osmocom.org/10022
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5330e697ec34bf215de91d44209048a8dc226d51
Gerrit-Change-Number: 10022
Gerrit-PatchSet: 1
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Comment-Date: Thu, 19 Jul 2018 00:02:19 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180719/95621ced/attachment.htm>


More information about the gerrit-log mailing list