Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/12357 )
Change subject: mgcp_client: make domain part of endpoint configurable ...................................................................... Patch Set 4: (3 comments) https://gerrit.osmocom.org/#/c/12357/3//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/12357/3//COMMIT_MSG@20 PS3, Line 20: Rationale: reading pcaps becomes so much easier when each of osmo-bsc and : osmo-msc address their MGW with differing domain names. Otherwise, both will : have a '0@mgw' endpoint and it gets really confusing. > How cold both use the same endpoint? The endpoint names (local part in front > of @) is allocated by […] In a scenario where both osmo-msc and osmo-bsc use the same osmo-mgw, indeed each new endpoint will get a new number. But when you have one osmo-mgw each, then both osmo-mgw instances independently create endpoints, so for the first call occuring, both actually create a '0@mgw' and wireshark shows identically named endpoints on separate osmo-mgw instances. Then there is the need to also look at the source/dest address and/or port numbers to figure out what is going on. https://gerrit.osmocom.org/#/c/12357/3/include/osmocom/mgcp_client/mgcp_client.h File include/osmocom/mgcp_client/mgcp_client.h: https://gerrit.osmocom.org/#/c/12357/3/include/osmocom/mgcp_client/mgcp_client.h@28 PS3, Line 28: MG > why 64? Is there some kind of maximum DNS domain name length we should use? I'm not sure at all. In fact I was just picking something at random and not bothering with details, just to quickly use this at 35c3. Same with other parts of this patch. I'm glad code review worked nicely here :) Found MGCP_ENDPOINT_MAXLEN, though that would include the "123@" part, it is a good rough choice I guess. https://gerrit.osmocom.org/#/c/12357/3/src/libosmo-mgcp-client/mgcp_client.c File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/#/c/12357/3/src/libosmo-mgcp-client/mgcp_client.c@830 PS3, Line 830: > Oh, wow, this breaks Gerrit's syntax highlighting :D yeah, I'm also getting C comment warnings when trying to write 'rtpbridge/*@foo' in a comment :P -- To view, visit https://gerrit.osmocom.org/12357 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: Ia662016f29dd8727d9c4626d726729641e21e1f8 Gerrit-Change-Number: 12357 Gerrit-PatchSet: 4 Gerrit-Owner: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Max <[email protected]> Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-CC: Stefan Sperling <[email protected]> Gerrit-CC: Vadim Yanitskiy <[email protected]> Gerrit-Comment-Date: Thu, 03 Jan 2019 01:20:31 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
