Change in osmo-sgsn[master]: Port gbproxy to NS2

[daniel]

daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/21029 )

Change subject: Port gbproxy to NS2
......................................................................


Patch Set 1:

(11 comments)

https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/include/osmocom/sgsn/gb_proxy.h 
File include/osmocom/sgsn/gb_proxy.h:

https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/include/osmocom/sgsn/gb_proxy.h@249 
PS1, Line 249: //int gbprox_reset_persistent_nsvcs(struct gprs_ns2_inst *nsi);
> what about this?
Removed it, should be configurable in config with:
encapsulation udp use-reset-block-unblock enabled


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy.c 
File src/gbproxy/gb_proxy.c:

https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy.c@80 
PS1, Line 80: 			     uint16_t ns_bvci);
> unrelated whitespace fix, can be done separately.
Ack


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy.c@807 
PS1, Line 807: 			     uint16_t ns_bvci)
> unrelated ws fix
Done


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy.c@1221 
PS1, Line 1221: 		/* simple case: BVCI IE is mandatory */
> This block looks like a separate patch.
Ack


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy.c@1379 
PS1, Line 1379: //int gbprox_reset_persistent_nsvcs(struct gprs_ns_inst *nsi)
> what about this
Yeah, handled by ns2 now. Removed it


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy.c@1445 
PS1, Line 1445: void gprs_ns_prim_status_cb(struct gbproxy_config *cfg, struct osmo_gprs_ns2_prim *nsp)
> Move this above gprs_ns_prim_cb then you can drop forward declaration.
Done


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy_ctrl.c 
File src/gbproxy/gb_proxy_ctrl.c:

https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy_ctrl.c@37 
PS1, Line 37: //static int get_nsvc_state(struct ctrl_cmd *cmd, void *data)
> So what do we do here? we wait until it is supported in ns2?
Yeah, I'm working on adding support back in. I implemented the parts that are already there.


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy_main.c 
File src/gbproxy/gb_proxy_main.c:

https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy_main.c@84 
PS1, Line 84: //static int proxy_ns_cb(enum gprs_ns_evt event, struct gprs_nsvc *nsvc,
> what about this
Thanks, not removed it as it relates to the old ns code


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy_main.c@372 
PS1, Line 372: //	rc = gprs_ns_nsip_listen(bssgp_nsi);
> what about his
Removed, thanks. Handled by the bind config


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy_vty.c 
File src/gbproxy/gb_proxy_vty.c:

https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/src/gbproxy/gb_proxy_vty.c@683 
PS1, Line 683: //		llist_for_each_entry_safe(nsvc, nsvc2, &nsi->gprs_nsvcs, list) {
> what about this
I kept the todo about not deleting persistent NSVC for the future and also fixed the dry_run logic but otherwise removed the loop


https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/tests/gbproxy/gbproxy_test.c 
File tests/gbproxy/gbproxy_test.c:

https://gerrit.osmocom.org/c/osmo-sgsn/+/21029/1/tests/gbproxy/gbproxy_test.c@36 
PS1, Line 36: 
> Drop ws
Ack



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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: Iecfd0408a35a11638d254c1db3c1d477b1a11524
Gerrit-Change-Number: 21029
Gerrit-PatchSet: 1
Gerrit-Owner: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Tue, 03 Nov 2020 20:35:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201103/0b1e8fe8/attachment.htm>