Attention is currently required from: neels. laforge has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/osmo-upf/+/37830?usp=email )
Change subject: add VTY 'gtp-echo' command ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File src/osmo-upf/upf_gtpu_echo.c: https://gerrit.osmocom.org/c/osmo-upf/+/37830/comment/bc291ff2_34eb2e44?usp=email : PS1, Line 151: rc = sendto(dev->gtpv1.ofd.fd, msgbuf, GTP_ECHO_REQ_SIZE, 0, &remote->u.sa, sizeof(*remote)); this is introducing even more direct socket interaction without going through osmo_io. One can of course do that, but all of this code IMHO is lacking the use of abstractions. Why don't we have separate functions to encode/decode GTP-U packets, including the IEs? Why are we having low-level code to puzzle together a message header bit by bit and calling sendto in the same function? We're not using msgb, not osmo_io, and neither some kind of gtp-u custom abstraction. Yes, one can do all that, and it will work. but for osmo-*, it's unusual, IMHO. We usually build infrastructure/abstraction and then use that. -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/37830?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: I970dccd7a27b098eea9e660822e24e2c4b059fc6 Gerrit-Change-Number: 37830 Gerrit-PatchSet: 1 Gerrit-Owner: neels <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Attention: neels <[email protected]> Gerrit-Comment-Date: Sat, 17 Aug 2024 05:16:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes
