[M] Change in osmo-mgw[master]: vty and log: also show local port for RTP conns
neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email ) Change subject: vty and log: also show local port for RTP conns .. vty and log: also show local port for RTP conns Before: CONN: (1226/rtp, id:0xD94316AD, ip:127.0.0.2, rtp:2344 rtcp:2345) After: CONN: (1226/rtp C:D94316AD r=127.0.0.2:2344<->l=127.0.0.1:4002) While changing that string, also include these changes for consistency and readability: - use the same r:...<->l:... format as osmo_sock_get_name(). - Instead of 'id:0x' use the actual MGCP format 'C: 9B686BE3'. - drop the commas - drop RTCP port: it is always RTP+1 and always an odd number. Rationale: The CONN pairs associated with each endpoint show remote RTP ports. When osmo-mgw is being used by both BSC and MSC, one side of the pair is showing the internal loop connection inside osmo-mgw, while my intuition suggested this connection pair is showing me the RTP port tuple of a single RTP stream. Adding the local port to the display makes it more clear, IMHO. Seeing the local port can also help to correlate the MGW vty dump with a capture of RTP. Implementation: I first tried directly using osmo_sock_get_name_buf() on conn->u.rtp.end.rtp.fd, but that might hide already known information when the fd is not actively used yet (before SDP): the local address and port would then be shown from the fd, not from conn->u.rtp.end.local_addr/_port == hidden before the fd is set up. Patch-By: whytek, nhofmeyr Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 --- M src/libosmo-mgcp/mgcp_conn.c 1 file changed, 59 insertions(+), 32 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved keith: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/libosmo-mgcp/mgcp_conn.c b/src/libosmo-mgcp/mgcp_conn.c index afc9e4e..283a213 100644 --- a/src/libosmo-mgcp/mgcp_conn.c +++ b/src/libosmo-mgcp/mgcp_conn.c @@ -358,53 +358,37 @@ { static char str[sizeof(conn->name)+sizeof(conn->id)+256]; char ipbuf[INET6_ADDRSTRLEN]; + struct osmo_strbuf sb = { .buf = str, .len = sizeof(str) }; - if (!conn) { - snprintf(str, sizeof(str), "(null connection)"); - return str; - } + if (!conn) + return "NULL"; switch (conn->type) { case MGCP_CONN_TYPE_RTP: + OSMO_STRBUF_PRINTF(sb, "(%s/%s C:%s r=%s:%u<->l=%s:%u", + conn->name, + mgcp_conn_rtp_type_name(conn->type), + conn->id, + osmo_sockaddr_ntop(&conn->u.rtp.end.addr.u.sa, ipbuf) ? : "NULL", + osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa), + conn->u.rtp.end.local_addr ? : "NULL", + conn->u.rtp.end.local_port); + switch (conn->u.rtp.type) { - case MGCP_RTP_DEFAULT: - /* Dump RTP connection */ - snprintf(str, sizeof(str), "(%s/rtp, id:0x%s, ip:%s, " - "rtp:%u rtcp:%u)", - conn->name, conn->id, - osmo_sockaddr_ntop(&conn->u.rtp.end.addr.u.sa, ipbuf), - osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa), - ntohs(conn->u.rtp.end.rtcp_port)); - break; case MGCP_RTP_OSMUX: - snprintf(str, sizeof(str), "(%s/osmux, id:0x%s, ip:%s, " - "port:%u CID:%u)", - conn->name, conn->id, - osmo_sockaddr_ntop(&conn->u.rtp.end.addr.u.sa, ipbuf), - osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa), - conn->u.rtp.osmux.local_cid); - break; - case MGCP_RTP_IUUP: - snprintf(str, sizeof(str), "(%s/iuup, id:0x%s, ip:%s, " - "port:%u)", - conn->name, conn->id, - osmo_sockaddr_ntop(&conn->u.rtp.end.addr.u.sa, ipbuf), - osmo_sockaddr_port(&conn->u.rtp.end.addr.u.sa)); + OSMO_STRBUF_PRINTF(sb, " CID=%u", conn->u.rtp.osmux.local_cid); break; default: - /* Should not happen, we should be able to dump -* every possible connection type. */ - snprintf(str, sizeof(str), "(unknown conn_rtp connection type %u)", -conn->u.rtp.type); break; } + + OSMO_STRBUF_PRI
[M] Change in osmo-mgw[master]: vty and log: also show local port for RTP conns
Attention is currently required from: dexter, keith, neels, osmith. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email ) Change subject: vty and log: also show local port for RTP conns .. Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 Gerrit-Change-Number: 34150 Gerrit-PatchSet: 4 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: keith Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: neels Gerrit-Attention: keith Gerrit-Attention: dexter Gerrit-Comment-Date: Thu, 02 Nov 2023 21:41:32 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: vty and log: also show local port for RTP conns
Attention is currently required from: dexter, keith, neels, osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email ) Change subject: vty and log: also show local port for RTP conns .. Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 Gerrit-Change-Number: 34150 Gerrit-PatchSet: 4 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: keith Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: neels Gerrit-Attention: keith Gerrit-Attention: dexter Gerrit-Comment-Date: Thu, 02 Nov 2023 19:25:39 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: vty and log: also show local port for RTP conns
Attention is currently required from: dexter, neels, osmith, pespin. keith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email ) Change subject: vty and log: also show local port for RTP conns .. Patch Set 4: Code-Review+1 (1 comment) File src/libosmo-mgcp/mgcp_conn.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34150/comment/3e112894_625f9a83 PS3, Line 363: if (!conn) { > unneeded {} Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 Gerrit-Change-Number: 34150 Gerrit-PatchSet: 4 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: keith Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Thu, 02 Nov 2023 18:47:02 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: vty and log: also show local port for RTP conns
Attention is currently required from: dexter, neels, osmith. keith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email ) Change subject: vty and log: also show local port for RTP conns .. Patch Set 3: Code-Review+1 (1 comment) Patchset: PS3: Thanks for dealing with this! Dropping the RTCP port is good, as is the C vs 0x part. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 Gerrit-Change-Number: 34150 Gerrit-PatchSet: 3 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: keith Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: neels Gerrit-Attention: dexter Gerrit-Comment-Date: Thu, 02 Nov 2023 18:42:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: vty and log: also show local port for RTP conns
Attention is currently required from: dexter, keith, neels, osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email ) Change subject: vty and log: also show local port for RTP conns .. Patch Set 3: Code-Review+1 (1 comment) File src/libosmo-mgcp/mgcp_conn.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34150/comment/184051ed_61bd0d75 PS3, Line 363: if (!conn) { unneeded {} -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 Gerrit-Change-Number: 34150 Gerrit-PatchSet: 3 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: neels Gerrit-Attention: keith Gerrit-Attention: dexter Gerrit-Comment-Date: Thu, 02 Nov 2023 09:42:45 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: vty and log: also show local port for RTP conns
Attention is currently required from: dexter, keith, osmith, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email ) Change subject: vty and log: also show local port for RTP conns .. Patch Set 3: Code-Review+1 (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/34150/comment/2ba4b0fa_c39d8df3 PS3, Line 15: CONN: (1226/rtp C:D94316AD r=127.0.0.2:2344<->l=127.0.0.1:4002) i'm not so sure about the id:0x -> C: change. We print 0x for MGCP conn ids everywhere else, right? I like the C: better because it is shorter and matches the MGCP protocol. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 Gerrit-Change-Number: 34150 Gerrit-PatchSet: 3 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: pespin Gerrit-Attention: keith Gerrit-Attention: dexter Gerrit-Comment-Date: Wed, 01 Nov 2023 20:51:55 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: vty and log: also show local port for RTP conns
Attention is currently required from: dexter, keith, osmith, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email ) Change subject: vty and log: also show local port for RTP conns .. Patch Set 3: (2 comments) Patchset: PS3: This is a good patch of yours. I can imagine the annoyance of not seeing the useful information! I fancied trying around a bit and ended up with this ready new patch set as a suggestion, feel free to discard / push over it if it's not to your liking. File src/libosmo-mgcp/mgcp_conn.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34150/comment/671355d6_f4628bf2 PS1, Line 373: "rtp:%u<->%u rtcp:%u)", > I'm totally fine with dropping the rtcp part. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 Gerrit-Change-Number: 34150 Gerrit-PatchSet: 3 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-CC: neels Gerrit-Attention: osmith Gerrit-Attention: pespin Gerrit-Attention: keith Gerrit-Attention: dexter Gerrit-Comment-Date: Wed, 01 Nov 2023 20:49:30 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: keith Comment-In-Reply-To: dexter Gerrit-MessageType: comment
[M] Change in osmo-mgw[master]: vty and log: also show local port for RTP conns
Attention is currently required from: dexter, keith, osmith. neels has uploaded a new patch set (#3) to the change originally created by keith. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email ) The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: vty and log: also show local port for RTP conns .. vty and log: also show local port for RTP conns Before: CONN: (1226/rtp, id:0xD94316AD, ip:127.0.0.2, rtp:2344 rtcp:2345) After: CONN: (1226/rtp C:D94316AD r=127.0.0.2:2344<->l=127.0.0.1:4002) While changing that string, also include these changes for consistency and readability: - use the same r:...<->l:... format as osmo_sock_get_name(). - Instead of 'id:0x' use the actual MGCP format 'C: 9B686BE3'. - drop the commas - drop RTCP port: it is always RTP+1 and always an odd number. Rationale: The CONN pairs associated with each endpoint show remote RTP ports. When osmo-mgw is being used by both BSC and MSC, one side of the pair is showing the internal loop connection inside osmo-mgw, while my intuition suggested this connection pair is showing me the RTP port tuple of a single RTP stream. Adding the local port to the display makes it more clear, IMHO. Seeing the local port can also help to correlate the MGW vty dump with a capture of RTP. Implementation: I first tried directly using osmo_sock_get_name_buf() on conn->u.rtp.end.rtp.fd, but that might hide already known information when the fd is not actively used yet (before SDP): the local address and port would then be shown from the fd, not from conn->u.rtp.end.local_addr/_port == hidden before the fd is set up. Patch-By: whytek, nhofmeyr Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 --- M src/libosmo-mgcp/mgcp_conn.c 1 file changed, 58 insertions(+), 30 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/50/34150/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34150?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib89a6779e1d68c6600f00699d4303f6c0ee07132 Gerrit-Change-Number: 34150 Gerrit-PatchSet: 3 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Attention: osmith Gerrit-Attention: keith Gerrit-Attention: dexter Gerrit-MessageType: newpatchset