Attention is currently required from: laforge, pespin.

neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-mgw/+/35205?usp=email )

Change subject: IuUP: allow Initialization from any address if not yet set
......................................................................


Patch Set 3: Code-Review+1

(3 comments)

Commit Message:

https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/1192328b_6d1965a0
PS1, Line 28: Decided for now that it's not worth the extra effort to make this 
more
            : restrictive
> "we do allow any source address to send MGCP to the MGW and actually". […]
ack

(So for early IuUP Initialization ACK, we use the sender's address somehow, for 
ip probing?)

This modified patch and the following ones do make the code more restrictive as 
requested, so resolving this


File src/libosmo-mgcp/mgcp_network.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/d3d00eb9_a8b469c1
PS1, Line 838:  if (mgcp_conn_rtp_is_iuup(conn) && !conn->iuup.configured) {
> I updated https://gerrit.osmocom. […]
Re the linked patch, I see only address and port related code in that patch, 
nothing about codecs nor IUFP? wrong link / am i not seeing it?

i'm a bit confused with these aspects:

1) whether the conn is indicated to be IUFP from the first CRCX
2) whether SDP is sent in the first CRCX
3) whether the remote RTP IP address is known from the first CRCX

For 1), I'm pretty sure that we are always telling the MGW about IUFP codec 
right from the first CRCX. In the MGCP header's "L:" line if no SDP is present, 
and in the SDP if SDP is present. And not only since yesterday.
(May need to qualify this statement for {msc,hnbgw}x{nightly,latest})
So if you add SDP even if port=0, it does not add the IUFP information;
IUFP codec was already indicated before, only now it is in the SDP instead of 
MGCP head.

For 2), we can send IUFP codec even when there is no SDP.
SDP is needed only for address and port.

For 3), I'm pretty sure we DO NOT send the remote RTP IP address right from the 
start, nor can we always do that, AFAICT?
>From your patch I gather that it is sometimes possible to know the remote 
>address right in the first CRCX, and would like to understand it...
hnbgw knows msc's address right from the start; but cannot know RNC's address?
osmo-msc MO cannot know MT's address right from the start.
AFAICT there will always be cases where we cannot include a remote IP address 
in the first CRCX, because we don't know the remote address yet, right? I am 
asking because, if we can teach all clients to always include a remote address, 
then this patch is not needed (besides maybe for backwards compat).
We do still need/want to have that check_rtp() skipping for IuUP Init, right?


File src/libosmo-mgcp/mgcp_network.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/35205/comment/60546d0c_82fe0ab5
PS2, Line 837: != 0)
> It's not a bool, it's a tristate 1, 0, -1. It's a bool + error. […]
(...which is exactly the same as handling the return val as bool)



--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35205?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: I6c365559a7bd197349f0ea99f7a13b56a4bb580b
Gerrit-Change-Number: 35205
Gerrit-PatchSet: 3
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
Gerrit-CC: laforge <lafo...@osmocom.org>
Gerrit-CC: pespin <pes...@sysmocom.de>
Gerrit-Attention: laforge <lafo...@osmocom.org>
Gerrit-Attention: pespin <pes...@sysmocom.de>
Gerrit-Comment-Date: Wed, 06 Dec 2023 02:05:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: neels <nhofm...@sysmocom.de>
Comment-In-Reply-To: laforge <lafo...@osmocom.org>
Comment-In-Reply-To: pespin <pes...@sysmocom.de>
Gerrit-MessageType: comment

Reply via email to