Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/9402 )
Change subject: gb_proxy: Add ctrl interface and ns-state, gbproxy-state commands ...................................................................... Patch Set 3: Code-Review+1 (2 comments) https://gerrit.osmocom.org/#/c/9402/3/src/gprs/gb_proxy_ctrl.c File src/gprs/gb_proxy_ctrl.c: https://gerrit.osmocom.org/#/c/9402/3/src/gprs/gb_proxy_ctrl.c@47 PS3, Line 47: cmd->reply = talloc_asprintf_append(cmd->reply, This might be taking it a bit too far, but I think it makes sense to have the "convert a NSVC to a CTRL interface string representation" inside libosmogb. This way, we can ensure that the same syntax is used when we want to dump the NSVC state of the PCU or the SGSN in [other] patches. What do you think? https://gerrit.osmocom.org/#/c/9402/3/src/gprs/gb_proxy_ctrl.c@61 PS3, Line 61: CTRL_CMD_DEFINE_RO(ns_status, "ns-state"); as we're printing NSVC state, I think it might also make sense to call the command "nsvc-state". This way we make it clear it's about the NSVC and not the NSE, e.g. -- To view, visit https://gerrit.osmocom.org/9402 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82c74fd0bfcb9ba4ec3619d9fdaa0cae201b3177 Gerrit-Change-Number: 9402 Gerrit-PatchSet: 3 Gerrit-Owner: daniel <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <[email protected]> Gerrit-Comment-Date: Wed, 06 Jun 2018 19:30:44 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: Yes
