Change in osmo-mgw[master]: mgcp_vty: be more specific about E1 trunks

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/.

laforge gerrit-no-reply at lists.osmocom.org
Thu Jul 9 10:21:51 UTC 2020


laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/19103 )

Change subject: mgcp_vty: be more specific about E1 trunks
......................................................................


Patch Set 6: Code-Review-1

(2 comments)

https://gerrit.osmocom.org/c/osmo-mgw/+/19103/6/src/libosmo-mgcp/mgcp_vty.c 
File src/libosmo-mgcp/mgcp_vty.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/19103/6/src/libosmo-mgcp/mgcp_vty.c@858 
PS6, Line 858:       "trunk e1 <1-64>", "Configure Trunk\n" "E1 trunk\n" "Trunk Nr\n")
> do we need compatibility for old 'trunk <1-64>' command?
I don't think so, because the existing code in osmo-mgw for E1 trunks did not work anyway.  So there cannot be any people with [reasonable, working] config files that end up in trouble here.


https://gerrit.osmocom.org/c/osmo-mgw/+/19103/6/src/libosmo-mgcp/mgcp_vty.c@897 
PS6, Line 897: 		vty_out(vty, " trunk-e1 %d%s", trunk->trunk_nr, VTY_NEWLINE);
> above you wrote "trunk e1" so I doubt this can be parsed
I think we may also want some kind of dispatch here.  After all, we iterate over the global list of trunks (g_cfg->trunks) and not just the list of E1 trunks.

So either 

a )the list always only contains E1 trunks (which it doesn't, as the virtual trunk is part of the list).  If this were true, we could keep this patch as-is but rename the list to g_cfg->e1_trunks.

b) the list contains all kinds of trunks.  In this case,

* if we have a E1 trunk specific node, and a specific config_write_ for E1, we ust also make sure that we actually only dump trunks of E1 Type here, and skip all others

* alternatively, keep the node name and hence also the config_write function shared (non E1 specific), but simply have a switch statement inside this function dispatching different trunk types.

This current patch is, IMHO, creating a more confusing situation than we currently have.  We should be careful to avoid this.

I personally think that we should keep a common/shared trunk node and not rename the NODE define / name nor this function.  All that should be renamed is the 'trunk ...' VTY command to make it E1 specific, in order for us to be able to introduce compatible etensions later on.

But to be honest, we could also do that even with the existing 'trunk <1-64>', command:  It could later be extended to 'trunk <1-64 [(foo|bar|buz)]' with an implicit default to E1.  or we handle it like we handle bts models in OsmBSC: With an explicit 'type (foo|bar|baz)' command within the 'trunk' node.



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/19103
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I22c39ee9a36e4e737992c91677f3e315907a4c7e
Gerrit-Change-Number: 19103
Gerrit-PatchSet: 6
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Thu, 09 Jul 2020 10:21:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: neels <nhofmeyr at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200709/376ebf2b/attachment.htm>


More information about the gerrit-log mailing list