Attention is currently required from: daniel, lynxis lazus.
laforge has posted comments on this change by lynxis lazus. (
https://gerrit.osmocom.org/c/osmo-ggsn/+/38938?usp=email )
Change subject: gtp: add support for SGSN Context Req/Resp/Ack
......................................................................
Patch Set 11:
(6 comments)
File gtp/gtp.c:
https://gerrit.osmocom.org/c/osmo-ggsn/+/38938/comment/e2e69e10_106b6071?us… :
PS11, Line 899: struct
Im surprised that the destination peer is not 'const'. Likeweise I'd normally
expect any othe rinput data like local_ref or ie to be 'const' as the called
function shouldn't modify any of these. Probably you're doing it this way as a
lot / all of the existing code does it this way?
https://gerrit.osmocom.org/c/osmo-ggsn/+/38938/comment/9712d78b_434ec111?us… :
PS11, Line 1134: int gtp_decode_pdp_ctx(const uint8_t *buf, unsigned int size, struct
pdp_t *pdp, uint16_t *sapi)
I think the function could deserve some documentation as a comment [doxygen style or not]
https://gerrit.osmocom.org/c/osmo-ggsn/+/38938/comment/b510fb7e_0eb96e0f?us… :
PS11, Line 1273: int gtp_encode_pdp_ctx(uint8_t *buf, unsigned int size, const struct
pdp_t *pdp, uint16_t sapi)
I think the function could deserve some documentation as a comment [doxygen style or not]
https://gerrit.osmocom.org/c/osmo-ggsn/+/38938/comment/3a753755_77d7eb15?us… :
PS11, Line 1403: LOGP(DLGTP, LOGL_NOTICE,
shouldn't we print some kind of context? Like which GSN peer (address) has sent us the
message with the wrong version?
https://gerrit.osmocom.org/c/osmo-ggsn/+/38938/comment/2acd816c_11141355?us… :
PS11, Line 1410: "SGSN Context Response expected Sequence flag set. Flags
%d\n", packet->flags);
shouldn't we print some kind of context? Like which GSN peer (address) has sent us the
message with the wrong flags?
https://gerrit.osmocom.org/c/osmo-ggsn/+/38938/comment/4777d45d_0b8ecfc0?us… :
PS11, Line 1458: "SGSN Context Request expected only on GTPCv1: %u\n",
version);
same here
--
To view, visit
https://gerrit.osmocom.org/c/osmo-ggsn/+/38938?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-ggsn
Gerrit-Branch: master
Gerrit-Change-Id: Idb8ed0bb87200a68bb8caedd734fc070df9179c0
Gerrit-Change-Number: 38938
Gerrit-PatchSet: 11
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: daniel <dwillmann(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Tue, 25 Feb 2025 12:11:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No