Change in osmo-mgw[master]: endp: add E1 endpoint interlocking

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/.

neels gerrit-no-reply at lists.osmocom.org
Mon Jun 29 15:54:06 UTC 2020


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

Change subject: endp: add E1 endpoint interlocking
......................................................................


Patch Set 5: Code-Review+1

(4 comments)

thanks

I'm still not convinced about mdcx:unavailable and dlcx:unavailable, as in inline comment. Otherwise letting my bikesheds rest.

https://gerrit.osmocom.org/c/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_endp.c 
File src/libosmo-mgcp/mgcp_endp.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_endp.c@468 
PS4, Line 468: }
> I thought about doing that while I was doing the implementation, but I prefer to have a single funct […]
I still disagree but leaving it up to you


https://gerrit.osmocom.org/c/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_endp.c@592 
PS4, Line 592: 		 * in a properly configured environment!) */
> If this happens its not a problem at the mgw side. […]
ok, so the comment means it is the peer program that is not properly configured?
The comment sounds like it would be osmo-mgw that is not properly configured, that's why I'm asking about this.
(Any program should always obviously guard against peers sending total nonsense...)

it's just a comment so just letting it be


https://gerrit.osmocom.org/c/osmo-mgw/+/18898/5/src/libosmo-mgcp/mgcp_endp.c 
File src/libosmo-mgcp/mgcp_endp.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/18898/5/src/libosmo-mgcp/mgcp_endp.c@600 
PS5, Line 600: bool mgcp_endp_avail(struct mgcp_endpoint *endp)
IMHO "available" is the wrong word, but leaving it up to you


https://gerrit.osmocom.org/c/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_ratectr.c 
File src/libosmo-mgcp/mgcp_ratectr.c:

https://gerrit.osmocom.org/c/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_ratectr.c@94 
PS4, Line 94: 	[MGCP_MDCX_FAIL_AVAIL] = { "mdcx:availability", "endpoint unavailable." },
> It does not matter if MDCX, CRCX etc would succeed or not, this is checked much later. […]
I was more thinking in this line: if we receive an MDCX for an endpoint for which we haven't previously seen a CRCX.
If there is an MDCX before a CRCX, that is a protocol error (like as if we sent "GARBAGE") and osmo-mgw doesn't even need to check availability.

When the protocol is followed correctly: if an E1 endpoint is blocked, then the first CRCX on it should hit crcx:unavailable above. After that CRCX was successful, there is no situation where an MDCX on the same would hit mdcx:unavailable.

So I doubt that we would ever see this happening.
Do you still think counting MDCX and DLCX on blocked endpoints makes sense?
I still have my doubts that the MDCX and DLCX counters are useful.



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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I18e90b10648a7e504371179ad144645fc82e1c27
Gerrit-Change-Number: 18898
Gerrit-PatchSet: 5
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Mon, 29 Jun 2020 15:54:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: dexter <pmaier at sysmocom.de>
Comment-In-Reply-To: neels <nhofmeyr at sysmocom.de>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200629/e6efa86f/attachment.htm>


More information about the gerrit-log mailing list