Change in osmo-mgw[master]: refactor: use msgb to receive, pass and send RTP packets

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

pespin gerrit-no-reply at lists.osmocom.org
Fri Jun 19 10:59:42 UTC 2020


pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/18913 )

Change subject: refactor: use msgb to receive, pass and send RTP packets
......................................................................


Patch Set 1:

(5 comments)

https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/include/osmocom/mgcp/mgcp_endp.h 
File include/osmocom/mgcp/mgcp_endp.h:

https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/include/osmocom/mgcp/mgcp_endp.h@44 
PS1, Line 44: #define OSMO_RTP_MSG_CTX(MSGB) (*(struct osmo_rtp_msg_ctx**)&((MSGB)->dst))
Accordng to msgb.h:
"""
	/* Part of which TRX logical channel we were received / transmitted */
	/* FIXME: move them into the control buffer */
	union {
		void *dst; /*!< reference of origin/destination */
		struct gsm_bts_trx *trx;
	};
"""

So rather use ->cb for this, like we do in libosmo-netif's jibuf.c:
#define JIBUF_MSGB_CB(__msgb) ((struct osmo_jibuf_msgb_cb *)&((__msgb)->cb[0]))


https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_codec.c 
File src/libosmo-mgcp/mgcp_codec.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_codec.c@436 
PS1, Line 436: const struct mgcp_rtp_codec *mgcp_codec_pt_find_by_subtype_name(struct mgcp_conn_rtp *conn,
some documentation here would be welcome.


https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_network.c 
File src/libosmo-mgcp/mgcp_network.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_network.c@1302 
PS1, Line 1302: 	/* Since the msgb remains owned and freed by this function, the msg ctx data struct can just be on the stack and
It may be the case now and I'm ok with it, but in the long term I'd argue it makes more sense to transfer ownsership since the msgb is most likely gonna end up in someone else wqueue?


https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_network.c@1310 
PS1, Line 1310: 	LOG_CONN_RTP(conn_src, LOGL_DEBUG, "msg ctx: %d %p %s\n",
Do we really want to call osmo_hexdump for ALL rtp packets received? Even if it's not printed...


https://gerrit.osmocom.org/c/osmo-mgw/+/18913/1/src/libosmo-mgcp/mgcp_network.c@1330 
PS1, Line 1330: static int rx_rtp(struct msgb *msg)
Move this function on top of rtp_data_net and then you can drop the forward reference of rx_rtp at the top of the file.



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I3af40b63bc49f8636d4e7ea2f8f83bb67f6619ee
Gerrit-Change-Number: 18913
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Fri, 19 Jun 2020 10:59:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200619/af6902a2/attachment.htm>


More information about the gerrit-log mailing list