Attention is currently required from: laforge, pespin.

fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmo-sccp/+/35796?usp=email )

Change subject: Implement M3UA-over-TCP (in addition to SCTP)
......................................................................


Patch Set 4:

(11 comments)

This change is ready for review.

Patchset:

PS1:
> IMHO we shouldn't do those more neutral, but rather keep them specific. […]
I hope later patchset revisions are now closer to what we concluded here, so 
marking as resolved. The question whether to rename `sctp-role (client|server)` 
is now being discussed in a separate thread.


File include/osmocom/sigtran/osmo_ss7.h:

https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/35c6700d_e8d1d87b
PS3, Line 293: bool osmo_ss7_asp_proto_check_trans_proto(enum 
osmo_ss7_asp_protocol proto, int trans_proto);
> Ack, I will drop the `osmo_` prefix and move to `ss7_internal.h`. […]
Done


https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/28e3372c_1bf846c0
PS3, Line 349: struct osmo_ss7_as *osmo_ss7_as_find_by_proto2(struct 
osmo_ss7_instance *inst,
> I added this because internally this function looks for a matching ASP: […]
Done


https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/e287e85b_ea919b35
PS3, Line 616: osmo_sccp_simple_client2(void *ctx, const char *name, uint32_t 
default_pc,
> Ok, I will revert my changes to the `simple_{server,client}` API and let it 
> use the default transpor […]
Done


File src/osmo_ss7_asp.c:

https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/4bb8cd94_f2964a84
PS3, Line 137:  case PROTO_MUX(OSMO_SS7_ASP_PROT_IPA, IPPROTO_TCP):
> I can do `ipproto << 16`, which would allow for values up to 65535. […]
Actually, we don't need to change anything here because by doing:

```((trans_proto << 8) | (asp_proto << 0))```

we leave one byte and thus 256 unique values for the `asp_proto` 
(`OSMO_SS7_ASP_PROT_*`), and 3 or 7 bytes for the `trans_proto` (depending on 
the architecture). Does that make sense? Or am I missing something?


https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/d532c02d_16cc31b3
PS3, Line 532:               "ASP protocol '%s' is not compatible with 
transport protocol %d\n",
> Ack, I'll then reword as follows: […]
Done


File src/osmo_ss7_vty.c:

https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/7a3189a3_dbacd8ec
PS3, Line 65: #define IPPROTO_VAR_STR "([sctp]|[tcp])"
> I will see what I can do here, but I am really not getting what's so special 
> about using this syntax […]
I implemented the optional transport protocol parameter using aliases.
Marking as resolved.


https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/37e9bf09_3058db9f
PS3, Line 995: DEFUN_ATTR(sctp_role, asp_sctp_role_cmd,
> > Why don't have "transport-protocol (sctp|tcp)" command around here? […]
Done in a separate change:

https://gerrit.osmocom.org/c/libosmo-sccp/+/35978 VTY: rename 'sctp-role' to 
'transport-role', add an alias


https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/ea0563bc_28cbc19a
PS3, Line 1641:                 get_value_string(ipproto_vals, 
asp->cfg.trans_proto),
> Ack
Done


File src/xua_rkm.c:

https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/b2fa22d9_fac97148
PS1, Line 240: M3UA-over-SCTP or M3UA-over-TCP?  Can we use asp->cfg.proto 
maybe?
> The XUA vs IPA has significant differences in the layer on top of TCP/SCTP 
> which back the fact of ha […]
This is no longer a relevant question, marking thread as resolved.


File tests/vty/ss7_asp_test.vty:

https://gerrit.osmocom.org/c/libosmo-sccp/+/35796/comment/49f8d013_3099aaca
PS3, Line 408: sctp
> well, the fact that it shows up in a VTY transcript test means (to my 
> understanding) that loading an […]
Done



--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/35796?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: I8c76d271472befacbeb998a93bbdc9e8660d9b5d
Gerrit-Change-Number: 35796
Gerrit-PatchSet: 4
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-CC: jolly <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Comment-Date: Wed, 14 Feb 2024 22:41:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <[email protected]>
Comment-In-Reply-To: pespin <[email protected]>
Comment-In-Reply-To: fixeria <[email protected]>
Gerrit-MessageType: comment

Reply via email to