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?usp... : 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?usp... : 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?usp... : 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?usp... : 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?usp... : 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?usp... : PS11, Line 1458: "SGSN Context Request expected only on GTPCv1: %u\n", version); same here