Change in ...osmo-sip-connector[master]: Add further logging.
keith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 ) Change subject: Add further logging. .. Patch Set 7: > (1 comment) yep.. that's what it looks like. I was doing the patch while you wrote this. -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b Gerrit-Change-Number: 14997 Gerrit-PatchSet: 7 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: keith Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Tue, 06 Aug 2019 12:08:42 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 )
Change subject: Add further logging.
..
Patch Set 7:
(1 comment)
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c
File src/mncc.c:
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c@191
PS1, Line 191: struct in_addr net = { .s_addr = other->ip };
> just making notes... […]
So the result is probably fine because htonl() and ntohl() are implemented the
same way afaik, but from what you say, it should be ntohl() then since
apparently other->ip is network byte order, and what we send over the unix MNCC
socket appears to be host byte order.
You can make sure also by checking implementation of the peer on the other side
of the unix mncc socket.
--
To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b
Gerrit-Change-Number: 14997
Gerrit-PatchSet: 7
Gerrit-Owner: keith
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-Comment-Date: Tue, 06 Aug 2019 12:00:14 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin
Comment-In-Reply-To: keith
Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
keith has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 )
Change subject: Add further logging.
..
Patch Set 1:
(1 comment)
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c
File src/mncc.c:
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c@191
PS1, Line 191: struct in_addr net = { .s_addr = other->ip };
> Well, according to existing code [mncc. […]
just making notes...
in sdp_extract_sdp, the code is:
struct in_addr addr;
inet_aton(conn->c_address, );
leg->base.ip = addr.s_addr;
so the values stored in (struct call)->ip are always network byte order. but
then, the original code was:
mncc.ip = htonl(other->ip);
so this is wrong, no?
--
To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b
Gerrit-Change-Number: 14997
Gerrit-PatchSet: 1
Gerrit-Owner: keith
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-Comment-Date: Tue, 06 Aug 2019 11:55:04 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin
Comment-In-Reply-To: keith
Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
keith has abandoned this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 ) Change subject: Add further logging. .. Abandoned These changes were erroneously included in https://gerrit.osmocom.org/#/c/osmo-sip-connector/+/14996/ -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b Gerrit-Change-Number: 14997 Gerrit-PatchSet: 7 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: keith Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-MessageType: abandon
Change in ...osmo-sip-connector[master]: Add further logging.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 )
Change subject: Add further logging.
..
Patch Set 7:
(2 comments)
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c
File src/mncc.c:
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c@191
PS1, Line 191: struct in_addr net = { .s_addr = other->ip };
> Yes, mncc.ip is in host byte order, if that's the correct terminology.
> certainly, using mncc. […]
Well, according to existing code [mncc.ip = htonl(other->ip)], other->ip is
host byte order, and mncc.ip is network byte order. And afar the struct in_addr
net expects to have content in net byte order.
So if I'm not wrong, either use "struct in_addr net = { .s_addr = mncc.ip };"
or function above should be ntohl() instead of htonl().
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c@192
PS1, Line 192: LOGP(DMNCC, LOGL_DEBUG, "SEND rtp_connect: IP=(%s)
PORT=(%u)\n", inet_ntoa(net), mncc.port);
> There are other places in the code that use inet_ntoa() […]
Having inet_ntoa already in some places in code doesn't mean we should keep
adding references to it. Using inet_ntop() doesn't imply a big change, it's
mostly declaring a local buffer in the stack + using the new function, so let's
use proper functions before merging since it's not a lot of extra hassle.
man inet_ntop: "inet_ntop() extends the inet_ntoa(3) function to support
multiple address families, inet_ntoa(3) is now considered to be deprecated
in favor of inet_ntop()."
--
To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b
Gerrit-Change-Number: 14997
Gerrit-PatchSet: 7
Gerrit-Owner: keith
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-Comment-Date: Tue, 06 Aug 2019 08:10:18 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin
Comment-In-Reply-To: keith
Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
keith has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 )
Change subject: Add further logging.
..
Patch Set 1:
(2 comments)
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c
File src/mncc.c:
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c@191
PS1, Line 191: struct in_addr net = { .s_addr = other->ip };
> Are you sure this is correct? or you need to use mncc. […]
Yes, mncc.ip is in host byte order, if that's the correct terminology.
certainly, using mncc.ip here result in the IP in the log message being printed
"backwards"
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c@192
PS1, Line 192: LOGP(DMNCC, LOGL_DEBUG, "SEND rtp_connect: IP=(%s)
PORT=(%u)\n", inet_ntoa(net), mncc.port);
> Better use inet_ntop().
There are other places in the code that use inet_ntoa()
Would it be better to accept this patch and then submit another changing them
all to inet_ntop() ?
--
To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b
Gerrit-Change-Number: 14997
Gerrit-PatchSet: 1
Gerrit-Owner: keith
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith
Gerrit-Reviewer: laforge
Gerrit-Reviewer: neels
Gerrit-Reviewer: pespin
Gerrit-Comment-Date: Mon, 05 Aug 2019 19:13:09 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin
Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 ) Change subject: Add further logging. .. Patch Set 7: > Patch Set 6: Code-Review-1 > > MY previous comments on this patches were ignored, still not applied. can't see previous comments? -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b Gerrit-Change-Number: 14997 Gerrit-PatchSet: 7 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Mon, 05 Aug 2019 17:05:31 + Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 ) Change subject: Add further logging. .. Patch Set 6: Code-Review-1 MY previous comments on this patches were ignored, still not applied. -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b Gerrit-Change-Number: 14997 Gerrit-PatchSet: 6 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-Reviewer: pespin Gerrit-Comment-Date: Mon, 05 Aug 2019 15:21:30 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 ) Change subject: Add further logging. .. Patch Set 6: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b Gerrit-Change-Number: 14997 Gerrit-PatchSet: 6 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Thu, 01 Aug 2019 15:16:32 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
Hello neels, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 to look at the new patch set (#6). Change subject: Add further logging. .. Add further logging. Log the IP port, payload type for calls. Also log the SIP Call-ID, this help for correlation with SIP traces. Use nua_event_name() rather than the number to log the nua event. Log a DEBUG level message for each nua event that is not handled by the nua_callback() Log MEDIA changes in re-INVITES. Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b --- M src/mncc.c M src/sip.c 2 files changed, 23 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-sip-connector refs/changes/97/14997/6 -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b Gerrit-Change-Number: 14997 Gerrit-PatchSet: 6 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in ...osmo-sip-connector[master]: Add further logging.
Hello neels, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 to look at the new patch set (#5). Change subject: Add further logging. .. Add further logging. Log the IP port, payload type for calls. Also log the SIP Call-ID, this help for correlation with SIP traces. Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b --- M src/mncc.c M src/sip.c 2 files changed, 10 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-sip-connector refs/changes/97/14997/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b Gerrit-Change-Number: 14997 Gerrit-PatchSet: 5 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-MessageType: newpatchset
Change in ...osmo-sip-connector[master]: Add further logging.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 ) Change subject: Add further logging. .. Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-sip-connector Gerrit-Branch: master Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b Gerrit-Change-Number: 14997 Gerrit-PatchSet: 3 Gerrit-Owner: keith Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-CC: pespin Gerrit-Comment-Date: Tue, 30 Jul 2019 16:37:16 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997 )
Change subject: Add further logging.
..
Patch Set 2:
(3 comments)
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c
File src/mncc.c:
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c@191
PS1, Line 191: struct in_addr net = { .s_addr = other->ip };
Are you sure this is correct? or you need to use mncc.ip?
https://gerrit.osmocom.org/#/c/14997/1/src/mncc.c@192
PS1, Line 192: LOGP(DMNCC, LOGL_DEBUG, "SEND rtp_connect: IP=(%s)
PORT=(%u)\n", inet_ntoa(net), mncc.port);
Better use inet_ntop().
https://gerrit.osmocom.org/#/c/14997/1/src/sip.c
File src/sip.c:
https://gerrit.osmocom.org/#/c/14997/1/src/sip.c@163
PS1, Line 163: inet_ntoa(net),
Better use inet_ntop().
--
To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b
Gerrit-Change-Number: 14997
Gerrit-PatchSet: 2
Gerrit-Owner: keith
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin
Gerrit-Comment-Date: Tue, 30 Jul 2019 15:14:29 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Change in ...osmo-sip-connector[master]: Add further logging.
keith has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997
Change subject: Add further logging.
..
Add further logging.
Log the IP port, payload type for calls.
Also log the SIP Call-ID, this help for correlation with SIP traces.
Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b
---
M src/mncc.c
M src/sip.c
2 files changed, 10 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-sip-connector
refs/changes/97/14997/1
diff --git a/src/mncc.c b/src/mncc.c
index 3f93126..5bd8af4 100644
--- a/src/mncc.c
+++ b/src/mncc.c
@@ -188,6 +188,8 @@
* FIXME: mncc.payload_msg_type should already be compatible.. but
* payload_type should be different..
*/
+ struct in_addr net = { .s_addr = other->ip };
+ LOGP(DMNCC, LOGL_DEBUG, "SEND rtp_connect: IP=(%s) PORT=(%u)\n",
inet_ntoa(net), mncc.port);
rc = write(leg->conn->fd.fd, , sizeof(mncc));
if (rc != sizeof(mncc)) {
LOGP(DMNCC, LOGL_ERROR, "Failed to send message leg(%u)\n",
diff --git a/src/sip.c b/src/sip.c
index 236c332..8f81a3f 100644
--- a/src/sip.c
+++ b/src/sip.c
@@ -98,7 +98,7 @@
return;
}
- LOGP(DSIP, LOGL_NOTICE, "leg(%p) is now connected.\n", leg);
+ LOGP(DSIP, LOGL_NOTICE, "leg(%p) is now connected(%s).\n", leg,
sip->sip_call_id->i_id);
leg->state = SIP_CC_CONNECTED;
other->connect_call(other);
nua_ack(leg->nua_handle, TAG_END());
@@ -111,7 +111,7 @@
struct sip_call_leg *leg;
const char *from = NULL, *to = NULL;
- LOGP(DSIP, LOGL_DEBUG, "Incoming call handle(%p)\n", nh);
+ LOGP(DSIP, LOGL_DEBUG, "Incoming call(%s) handle(%p)\n",
sip->sip_call_id->i_id, nh);
if (!sdp_screen_sdp(sip)) {
LOGP(DSIP, LOGL_ERROR, "No supported codec.\n");
@@ -158,6 +158,11 @@
call_leg_release(>base);
return;
}
+ struct in_addr net = { .s_addr = leg->base.ip };
+ LOGP(DSIP, LOGL_DEBUG, "SDP Extracted: IP=(%s) PORT=(%u)
PAYLOAD=(%u).\n",
+ inet_ntoa(net),
+ leg->base.port,
+ leg->base.payload_type);
leg->base.release_call = sip_release_call;
leg->base.ring_call = sip_ring_call;
@@ -328,6 +333,7 @@
other->release_call(other);
} else if (event == nua_i_invite) {
/* new incoming leg or re-INVITE */
+ LOGP(DSIP, LOGL_NOTICE, "Processing INVITE Call-ID: %s\n",
sip->sip_call_id->i_id);
if (status == 100) {
struct sip_call_leg *leg = sip_find_leg(nh);
--
To view, visit https://gerrit.osmocom.org/c/osmo-sip-connector/+/14997
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sip-connector
Gerrit-Branch: master
Gerrit-Change-Id: I2620cce245be199d849d8fad3fc998c96c243f6b
Gerrit-Change-Number: 14997
Gerrit-PatchSet: 1
Gerrit-Owner: keith
Gerrit-MessageType: newchange
