[S] Change in osmo-mgw[master]: IuUP: Allow Initialization with set rem IP address and unset rem port
pespin has submitted this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email )
Change subject: IuUP: Allow Initialization with set rem IP address and unset
rem port
..
IuUP: Allow Initialization with set rem IP address and unset rem port
Do not refuse IuUP Initialization messages coming in on an RTP port if
the remote port is not yet known.
If an IUFP conn is not yet configured (pre-Initialization), allow rx
from any address or port.
An osmo-mgw client (eg. osmo-hnbgw) may wish to initially set a remote
IP address as a hint during CRCX, hence the IP address may already be
set while the port may be unset.
Related: SYS#6657
Change-Id: Idd833997abce46886e9664505b2776fa5dadc8db
---
M src/libosmo-mgcp/mgcp_network.c
1 file changed, 21 insertions(+), 1 deletion(-)
Approvals:
osmith: Looks good to me, but someone else must approve
Jenkins Builder: Verified
neels: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index 994ce45..674c096 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -834,7 +834,8 @@
{
char ipbuf[INET6_ADDRSTRLEN];
- if (osmo_sockaddr_is_any(>end.addr) != 0) {
+ if (osmo_sockaddr_is_any(>end.addr) != 0 ||
+ osmo_sockaddr_port(>end.addr.u.sa) == 0) {
if (mgcp_conn_rtp_is_iuup(conn) && !conn->iuup.configured) {
/* Allow IuUP Initialization to get through even if we
don't have a remote address set yet.
* This is needed because hNodeB doesn't announce its
IuUP remote IP addr to the MGCP client
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db
Gerrit-Change-Number: 35176
Gerrit-PatchSet: 5
Gerrit-Owner: pespin
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels
Gerrit-Reviewer: osmith
Gerrit-Reviewer: pespin
Gerrit-MessageType: merged
[S] Change in osmo-mgw[master]: IuUP: Allow Initialization with set rem IP address and unset rem port
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email ) Change subject: IuUP: Allow Initialization with set rem IP address and unset rem port .. Patch Set 5: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db Gerrit-Change-Number: 35176 Gerrit-PatchSet: 5 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Reviewer: pespin Gerrit-Comment-Date: Wed, 06 Dec 2023 14:03:18 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: Allow Initialization with set rem IP address and unset rem port
Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email ) Change subject: IuUP: Allow Initialization with set rem IP address and unset rem port .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db Gerrit-Change-Number: 35176 Gerrit-PatchSet: 5 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Reviewer: osmith Gerrit-Attention: pespin Gerrit-Comment-Date: Wed, 06 Dec 2023 13:55:33 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: Allow Initialization with set rem IP address and unset rem port
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email ) Change subject: IuUP: Allow Initialization with set rem IP address and unset rem port .. Patch Set 5: Code-Review+1 (1 comment) Patchset: PS5: This is an invitation to others to add a +1, and not be scared off by the long discussions. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db Gerrit-Change-Number: 35176 Gerrit-PatchSet: 5 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Comment-Date: Wed, 06 Dec 2023 02:06:40 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: Allow Initialization with set rem IP address and unset rem port
Attention is currently required from: neels. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email ) Change subject: IuUP: Allow Initialization with set rem IP address and unset rem port .. Patch Set 4: (4 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/890ed34b_b58b214d PS4, Line 7: rem > (i'd rather write out "remote" and abbreviate "Init" and "addr") Done Patchset: PS4: > Ok, I see, and agree with this. […] I/we already invested time into separating them. No need to spend more time now joining them, let's leave it as is (I agree it would be arguable whether 1 or 2 is better). File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/fcdb22ec_e5acf916 PS4, Line 842: ASs > Ack Done https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/4141065f_d4203f9e PS4, Line 843: MDCX > Ack Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db Gerrit-Change-Number: 35176 Gerrit-PatchSet: 4 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Attention: neels Gerrit-Comment-Date: Tue, 05 Dec 2023 12:50:37 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: Allow Initialization with set rem IP address and unset rem port
Attention is currently required from: neels.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email )
Change subject: IuUP: Allow Initialization with set rem IP address and unset
rem port
..
Patch Set 4:
(4 comments)
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/ada45e97_26192709
PS4, Line 838: osmo_sockaddr_port(>end.addr.u.sa) == 0) {
> the most concise restriction would be: […]
We need to keep it this way I submitted, at least for address, since the
configured address at this point may be wrong (the hnb may actually be using
another IP address, what osmo-mgw was submitted was just a guess, and it cannot
be considered confirmed until IuUP Initialization happens).
So if IP address is set but port is not, we cannot validate IP address of
received packet against the configured one, because it is "a guess", we must
accept it anyway, at least to carry on with IuUP initialization.
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/01d4c9a3_be3f7c7e
PS4, Line 841: announce
> "not all hNodeB", but some are happily sending RAB Assignment success before
> IuUP Initialization. […]
that's why I didn't write "not all hNodeb", to leave it undetailed. I can add
"some", but since we didn't play with many others I preferred leaving it this
way. I think even in the diagrams in specs it shows up this way (IuUP
Initialization happening before RAB-Ass-Resp).
Do you know of any HNB not doing it this way?
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/d6994204_0c6123e7
PS4, Line 842: ASs
> (RAB Assignment Response at HNBGW)
Ack
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/238815b4_a883cf2a
PS4, Line 843: MDCX
> Writing "MDCX" is too specific: it is up to the client to do CRCX with SDP
> all-in-one or adjust with […]
Ack
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db
Gerrit-Change-Number: 35176
Gerrit-PatchSet: 4
Gerrit-Owner: pespin
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels
Gerrit-Attention: neels
Gerrit-Comment-Date: Tue, 05 Dec 2023 10:37:58 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels
Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: Allow Initialization with set rem IP address and unset rem port
Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email ) Change subject: IuUP: Allow Initialization with set rem IP address and unset rem port .. Patch Set 4: (3 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/4827b0ff_d321205f PS2, Line 7: mgcp_network: Allow rx of IuUP in loopback mode with set rem IP address and unset rem port > (you're getting into a habit of too long summary lines) Done Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/f1acf2a3_524febff PS4, Line 7: rem (i'd rather write out "remote" and abbreviate "Init" and "addr") File src/libosmo-mgcp/mgcp_network.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/22d2e586_68b0321b PS2, Line 866: HACK: for IuUP, we want to reply with an IuUP Initialization ACK upon the first RTP : * messag > Tests show these things: […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db Gerrit-Change-Number: 35176 Gerrit-PatchSet: 4 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Attention: pespin Gerrit-Comment-Date: Mon, 04 Dec 2023 23:47:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: Allow Initialization with set rem IP address and unset rem port
Attention is currently required from: pespin.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email )
Change subject: IuUP: Allow Initialization with set rem IP address and unset
rem port
..
Patch Set 4: Code-Review+1
(5 comments)
Patchset:
PS4:
Ok, I see, and agree with this.
Below some petty details below.
First petty detail: we could squash it into that other patch.
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/e0c4ce52_25437aa2
PS4, Line 838: osmo_sockaddr_port(>end.addr.u.sa) == 0) {
the most concise restriction would be:
- validate the IP address if present,
- and validate the port if present, independently.
I mean, in this patch we skip both checks if one of them is not set. Instead we
can skip only the port number check if port == 0, and skip only the address
check if address == ANY.
(That's mainly why it's so compelling to just skip sender validation for early
IuUP entirely; doing checks properly rejiggers this entire function.)
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/6ec9f8d2_73ec40b1
PS4, Line 841: announce
"not all hNodeB", but some are happily sending RAB Assignment success before
IuUP Initialization.
I find "MGCP client" confusing here, the hNodeB obviously does not talk MGCP
protocol.
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/a34c5956_389c6fc8
PS4, Line 842: ASs
(RAB Assignment Response at HNBGW)
https://gerrit.osmocom.org/c/osmo-mgw/+/35176/comment/f783b56a_3eb116ae
PS4, Line 843: MDCX
Writing "MDCX" is too specific: it is up to the client to do CRCX with SDP
all-in-one or adjust with MDCX later.
I'd rather write:
Hence the MGW may not yet know the remote IuUP address and port at the time of
receiving IuUP Initialization from the hNodeB.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db
Gerrit-Change-Number: 35176
Gerrit-PatchSet: 4
Gerrit-Owner: pespin
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels
Gerrit-Attention: pespin
Gerrit-Comment-Date: Mon, 04 Dec 2023 23:41:07 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
[S] Change in osmo-mgw[master]: IuUP: Allow Initialization with set rem IP address and unset rem port
Attention is currently required from: neels, pespin. Hello Jenkins Builder, neels, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Code-Review-1 by neels, Verified+1 by Jenkins Builder Change subject: IuUP: Allow Initialization with set rem IP address and unset rem port .. IuUP: Allow Initialization with set rem IP address and unset rem port Do not refuse IuUP Initialization messages coming in on an RTP port if the remote port is not yet known. If an IUFP conn is not yet configured (pre-Initialization), allow rx from any address or port. An osmo-mgw client (eg. osmo-hnbgw) may wish to initially set a remote IP address as a hint during CRCX, hence the IP address may already be set while the port may be unset. Related: SYS#6657 Change-Id: Idd833997abce46886e9664505b2776fa5dadc8db --- M src/libosmo-mgcp/mgcp_network.c 1 file changed, 21 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/76/35176/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35176?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: Idd833997abce46886e9664505b2776fa5dadc8db Gerrit-Change-Number: 35176 Gerrit-PatchSet: 4 Gerrit-Owner: pespin Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels Gerrit-Attention: neels Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
