July 2022 - gerrit-log - lists.osmocom.org

Change in osmo-msc[master]: sdp_msg: s/sdp_audio_codec_/sdp_audio_codecs_

by neels

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28784 ) Change subject: sdp_msg: s/sdp_audio_codec_/sdp_audio_codecs_ ...................................................................... sdp_msg: s/sdp_audio_codec_/sdp_audio_codecs_ Some functions act on a struct sdp_audio_codecs but begin with the name sdp_audio_codec (singular). That's confusing. Related: SYS#5066 Change-Id: Id87eb350c1f17f8dbf776909824bfa06634c1d04 --- M include/osmocom/msc/sdp_msg.h M src/libmsc/sdp_msg.c M tests/sdp_msg/sdp_msg_test.c 3 files changed, 27 insertions(+), 27 deletions(-) Approvals: Jenkins Builder: Verified dexter: Looks good to me, but someone else must approve neels: Looks good to me, approved diff --git a/include/osmocom/msc/sdp_msg.h b/include/osmocom/msc/sdp_msg.h index 537713c..9073e91 100644 --- a/include/osmocom/msc/sdp_msg.h +++ b/include/osmocom/msc/sdp_msg.h @@ -41,15 +41,15 @@ int sdp_audio_codecs_cmp(const struct sdp_audio_codecs *a, const struct sdp_audio_codecs *b, bool cmp_fmtp, bool cmp_payload_type); -struct sdp_audio_codec *sdp_audio_codec_add(struct sdp_audio_codecs *ac, unsigned int payload_type, - const char *subtype_name, unsigned int rate, const char *fmtp); -struct sdp_audio_codec *sdp_audio_codec_add_copy(struct sdp_audio_codecs *ac, - const struct sdp_audio_codec *codec); -int sdp_audio_codec_remove(struct sdp_audio_codecs *ac, const struct sdp_audio_codec *codec); -struct sdp_audio_codec *sdp_audio_codec_by_payload_type(struct sdp_audio_codecs *ac, - unsigned int payload_type, bool create); -struct sdp_audio_codec *sdp_audio_codec_by_descr(struct sdp_audio_codecs *ac, - const struct sdp_audio_codec *codec); +struct sdp_audio_codec *sdp_audio_codecs_add(struct sdp_audio_codecs *ac, unsigned int payload_type, + const char *subtype_name, unsigned int rate, const char *fmtp); +struct sdp_audio_codec *sdp_audio_codecs_add_copy(struct sdp_audio_codecs *ac, + const struct sdp_audio_codec *codec); +int sdp_audio_codecs_remove(struct sdp_audio_codecs *ac, const struct sdp_audio_codec *codec); +struct sdp_audio_codec *sdp_audio_codecs_by_payload_type(struct sdp_audio_codecs *ac, + unsigned int payload_type, bool create); +struct sdp_audio_codec *sdp_audio_codecs_by_descr(struct sdp_audio_codecs *ac, + const struct sdp_audio_codec *codec); void sdp_audio_codecs_intersection(struct sdp_audio_codecs *ac_dest, const struct sdp_audio_codecs *ac_other, bool translate_payload_type_numbers); diff --git a/src/libmsc/sdp_msg.c b/src/libmsc/sdp_msg.c index a2a2d3d..eda6ac9 100644 --- a/src/libmsc/sdp_msg.c +++ b/src/libmsc/sdp_msg.c @@ -116,13 +116,13 @@ * The payload_type must exist in sdp_msg_payload_type_names. * Return the audio codec created or already existing for this payload type number. */ -struct sdp_audio_codec *sdp_audio_codec_add(struct sdp_audio_codecs *ac, unsigned int payload_type, - const char *subtype_name, unsigned int rate, const char *fmtp) +struct sdp_audio_codec *sdp_audio_codecs_add(struct sdp_audio_codecs *ac, unsigned int payload_type, + const char *subtype_name, unsigned int rate, const char *fmtp) { struct sdp_audio_codec *codec; /* Does an entry already exist? */ - codec = sdp_audio_codec_by_payload_type(ac, payload_type, false); + codec = sdp_audio_codecs_by_payload_type(ac, payload_type, false); if (codec) { /* Already exists, sanity check */ if (!codec->subtype_name[0]) @@ -141,7 +141,7 @@ } /* None exists, create codec entry for this payload type number */ - codec = sdp_audio_codec_by_payload_type(ac, payload_type, true); + codec = sdp_audio_codecs_by_payload_type(ac, payload_type, true); /* NULL means unable to add an entry */ if (!codec) return NULL; @@ -152,14 +152,14 @@ return codec; } -struct sdp_audio_codec *sdp_audio_codec_add_copy(struct sdp_audio_codecs *ac, const struct sdp_audio_codec *codec) +struct sdp_audio_codec *sdp_audio_codecs_add_copy(struct sdp_audio_codecs *ac, const struct sdp_audio_codec *codec) { - return sdp_audio_codec_add(ac, codec->payload_type, codec->subtype_name, codec->rate, - codec->fmtp[0] ? codec->fmtp : NULL); + return sdp_audio_codecs_add(ac, codec->payload_type, codec->subtype_name, codec->rate, + codec->fmtp[0] ? codec->fmtp : NULL); } -struct sdp_audio_codec *sdp_audio_codec_by_payload_type(struct sdp_audio_codecs *ac, unsigned int payload_type, - bool create) +struct sdp_audio_codec *sdp_audio_codecs_by_payload_type(struct sdp_audio_codecs *ac, unsigned int payload_type, + bool create) { struct sdp_audio_codec *codec; foreach_sdp_audio_codec(codec, ac) { @@ -185,7 +185,7 @@ /* Return a given sdp_msg's codec entry that matches the subtype_name and rate of the given codec, or NULL if no * match is found. Comparison is made by sdp_audio_codec_cmp(cmp_payload_type=false). */ -struct sdp_audio_codec *sdp_audio_codec_by_descr(struct sdp_audio_codecs *ac, const struct sdp_audio_codec *codec) +struct sdp_audio_codec *sdp_audio_codecs_by_descr(struct sdp_audio_codecs *ac, const struct sdp_audio_codec *codec) { struct sdp_audio_codec *i; foreach_sdp_audio_codec(i, ac) { @@ -196,9 +196,9 @@ } /* Remove the codec entry pointed at by 'codec'. 'codec' must point at an entry of 'sdp' (to use an external codec - * instance, use sdp_audio_codec_by_descr()). + * instance, use sdp_audio_codecs_by_descr()). * Return 0 on success, -ENOENT if codec does not point at the sdp->codec array. */ -int sdp_audio_codec_remove(struct sdp_audio_codecs *ac, const struct sdp_audio_codec *codec) +int sdp_audio_codecs_remove(struct sdp_audio_codecs *ac, const struct sdp_audio_codec *codec) { struct sdp_audio_codec *i; if ((codec < ac->codec) @@ -290,7 +290,7 @@ if (!audio_name || audio_name >= sdp_msg_line_end(src)) return -EINVAL; - codec = sdp_audio_codec_by_payload_type(&sdp->audio_codecs, payload_type, true); + codec = sdp_audio_codecs_by_payload_type(&sdp->audio_codecs, payload_type, true); if (!codec) return -ENOSPC; @@ -314,7 +314,7 @@ if (fmtp_str >= line_end) return -EINVAL; - codec = sdp_audio_codec_by_payload_type(&sdp->audio_codecs, payload_type, true); + codec = sdp_audio_codecs_by_payload_type(&sdp->audio_codecs, payload_type, true); if (!codec) return -ENOSPC; @@ -400,7 +400,7 @@ if (sscanf(payload_type_str, "%u", &payload_type) < 1) return -EINVAL; - codec = sdp_audio_codec_by_payload_type(&sdp->audio_codecs, payload_type, true); + codec = sdp_audio_codecs_by_payload_type(&sdp->audio_codecs, payload_type, true); if (!codec) return -ENOSPC; @@ -519,10 +519,10 @@ struct sdp_audio_codec *other; OSMO_ASSERT(i < ARRAY_SIZE(ac_dest->codec)); - other = sdp_audio_codec_by_descr((struct sdp_audio_codecs*)ac_other, codec); + other = sdp_audio_codecs_by_descr((struct sdp_audio_codecs *)ac_other, codec); if (!other) { - OSMO_ASSERT(sdp_audio_codec_remove(ac_dest, codec) == 0); + OSMO_ASSERT(sdp_audio_codecs_remove(ac_dest, codec) == 0); i--; continue; } diff --git a/tests/sdp_msg/sdp_msg_test.c b/tests/sdp_msg/sdp_msg_test.c index e891953..ee48e80 100644 --- a/tests/sdp_msg/sdp_msg_test.c +++ b/tests/sdp_msg/sdp_msg_test.c @@ -531,7 +531,7 @@ break; } printf("SDP: %s\n", sdp_audio_codecs_to_str(&sdp.audio_codecs)); - codec = sdp_audio_codec_by_payload_type(&sdp.audio_codecs, t->select_payload_type, false); + codec = sdp_audio_codecs_by_payload_type(&sdp.audio_codecs, t->select_payload_type, false); OSMO_ASSERT(codec); printf("Select: %s\n", sdp_audio_codec_to_str(codec)); -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28784 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Id87eb350c1f17f8dbf776909824bfa06634c1d04 Gerrit-Change-Number: 28784 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 10 months

1
0
0 0

Change in osmo-msc[master]: sdp_msg: add sdp_audio_codecs_cmp(), add compare flags

by neels

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28783 ) Change subject: sdp_msg: add sdp_audio_codecs_cmp(), add compare flags ...................................................................... sdp_msg: add sdp_audio_codecs_cmp(), add compare flags A problem with SDP fmtp handling is visible in this patch: when cmp_fmtp is true, we compare fmtp strings 1:1, which is not how things should be done. The intention is to fix fmtp handling in a later patch. At least there now is a flag to bypass fmtp comparison altogether. Related: SYS#5066 Change-Id: I18d33e189674229501afec950aa1c732386455a2 --- M include/osmocom/msc/sdp_msg.h M src/libmsc/sdp_msg.c 2 files changed, 73 insertions(+), 17 deletions(-) Approvals: Jenkins Builder: Verified dexter: Looks good to me, but someone else must approve neels: Looks good to me, approved diff --git a/include/osmocom/msc/sdp_msg.h b/include/osmocom/msc/sdp_msg.h index 44a8a30..537713c 100644 --- a/include/osmocom/msc/sdp_msg.h +++ b/include/osmocom/msc/sdp_msg.h @@ -36,7 +36,10 @@ const char *sdp_msg_line_end(const char *src); -int sdp_audio_codec_cmp(const struct sdp_audio_codec *a, const struct sdp_audio_codec *b); +int sdp_audio_codec_cmp(const struct sdp_audio_codec *a, const struct sdp_audio_codec *b, + bool cmp_fmtp, bool cmp_payload_type); +int sdp_audio_codecs_cmp(const struct sdp_audio_codecs *a, const struct sdp_audio_codecs *b, + bool cmp_fmtp, bool cmp_payload_type); struct sdp_audio_codec *sdp_audio_codec_add(struct sdp_audio_codecs *ac, unsigned int payload_type, const char *subtype_name, unsigned int rate, const char *fmtp); diff --git a/src/libmsc/sdp_msg.c b/src/libmsc/sdp_msg.c index 08a2186..a2a2d3d 100644 --- a/src/libmsc/sdp_msg.c +++ b/src/libmsc/sdp_msg.c @@ -31,30 +31,83 @@ #include <osmocom/msc/sdp_msg.h> /* Compare name, rate and fmtp, returning typical cmp result: 0 on match, and -1 / 1 on mismatch. - * Do *not* compare the payload_type number. + * If cmp_fmtp is false, do *not* compare the fmtp string; if true, compare fmtp 1:1 as strings. + * If cmp_payload_type is false, do *not* compare the payload_type number. * The fmtp is only string-compared -- e.g. if AMR parameters appear in a different order, it amounts to a mismatch even * though all parameters are the same. */ -int sdp_audio_codec_cmp(const struct sdp_audio_codec *a, const struct sdp_audio_codec *b) +int sdp_audio_codec_cmp(const struct sdp_audio_codec *a, const struct sdp_audio_codec *b, + bool cmp_fmtp, bool cmp_payload_type) { - int rc; + int cmp; if (a == b) return 0; if (!a) return -1; if (!b) return 1; - rc = strncmp(a->subtype_name, b->subtype_name, sizeof(a->subtype_name)); - if (rc) - return rc; + cmp = strncmp(a->subtype_name, b->subtype_name, sizeof(a->subtype_name)); + if (cmp) + return cmp; + cmp = OSMO_CMP(a->rate, b->rate); + if (cmp) + return cmp; + if (cmp_fmtp) { + cmp = strncmp(a->fmtp, b->fmtp, sizeof(a->fmtp)); + if (cmp) + return cmp; + } + if (cmp_payload_type) { + cmp = OSMO_CMP(a->payload_type, b->payload_type); + if (cmp) + return cmp; + } + return 0; +} - if (a->rate < b->rate) +/* Compare two lists of audio codecs, returning typical cmp result: 0 on match, and -1 / 1 on mismatch. + * The ordering in the two lists may differ, except that the first codec in 'a' must also be the first codec in 'b'. + * This is because the first codec typically expresses the preferred codec to use. + * If cmp_fmtp is false, do *not* compare the fmtp strings; if true, compare fmtp 1:1 as strings. + * If cmp_payload_type is false, do *not* compare the payload_type numbers. + * The fmtp is only string-compared -- e.g. if AMR parameters appear in a different order, it amounts to a mismatch even + * though all parameters are the same. */ +int sdp_audio_codecs_cmp(const struct sdp_audio_codecs *a, const struct sdp_audio_codecs *b, + bool cmp_fmtp, bool cmp_payload_type) +{ + const struct sdp_audio_codec *codec_a; + const struct sdp_audio_codec *codec_b; + int cmp; + if (a == b) + return 0; + if (!a) return -1; - if (a->rate > b->rate) + if (!b) return 1; - rc = strncmp(a->fmtp, b->fmtp, sizeof(a->fmtp)); - if (rc) - return rc; + cmp = OSMO_CMP(a->count, b->count); + if (cmp) + return cmp; + + if (!a->count) + return 0; + + /* The first codec is the "chosen" codec and should match. The others may appear in different order. */ + cmp = sdp_audio_codec_cmp(&a->codec[0], &b->codec[0], cmp_fmtp, cmp_payload_type); + if (cmp) + return cmp; + + /* See if each codec in a is also present in b */ + foreach_sdp_audio_codec(codec_a, a) { + bool match_found = false; + foreach_sdp_audio_codec(codec_b, b) { + if (!sdp_audio_codec_cmp(codec_a, codec_b, cmp_fmtp, cmp_payload_type)) { + match_found = true; + break; + } + } + if (!match_found) + return -1; + } return 0; } @@ -130,13 +183,13 @@ return codec; } -/* Return a given sdp_msg's codec entry that matches the subtype_name, rate and fmtp of the given codec, or NULL if no - * match is found. Comparison is made by sdp_audio_codec_cmp(). */ +/* Return a given sdp_msg's codec entry that matches the subtype_name and rate of the given codec, or NULL if no + * match is found. Comparison is made by sdp_audio_codec_cmp(cmp_payload_type=false). */ struct sdp_audio_codec *sdp_audio_codec_by_descr(struct sdp_audio_codecs *ac, const struct sdp_audio_codec *codec) { struct sdp_audio_codec *i; foreach_sdp_audio_codec(i, ac) { - if (!sdp_audio_codec_cmp(i, codec)) + if (!sdp_audio_codec_cmp(i, codec, false, false)) return i; } return NULL; @@ -452,8 +505,8 @@ } /* Leave only those codecs in 'ac_dest' that are also present in 'ac_other'. - * The matching is made by sdp_audio_codec_cmp(), i.e. payload_type numbers are not compared and fmtp parameters are - * compared 1:1 as plain strings. + * The matching is made by sdp_audio_codec_cmp(cmp_payload_type=false), i.e. payload_type numbers are not compared and + * fmtp parameters are compared 1:1 as plain strings. * If translate_payload_type_numbers has an effect if ac_dest and ac_other have mismatching payload_type numbers for the * same SDP codec descriptions. If translate_payload_type_numbers is true, take the payload_type numbers from ac_other. * If false, keep payload_type numbers in ac_dest unchanged. */ -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28783 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I18d33e189674229501afec950aa1c732386455a2 Gerrit-Change-Number: 28783 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 10 months

1
0
0 0

Change in osmo-msc[master]: sdp_msg: s/sdp_audio_codec_/sdp_audio_codecs_

by neels

Attention is currently required from: fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28784 ) Change subject: sdp_msg: s/sdp_audio_codec_/sdp_audio_codecs_ ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: this is a pretty trivial thing, i'm going to pass this for the benefit of the rest of the codecs patches -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28784 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Id87eb350c1f17f8dbf776909824bfa06634c1d04 Gerrit-Change-Number: 28784 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Sat, 30 Jul 2022 19:45:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 10 months

1
0
0 0

Change in osmo-msc[master]: sdp_msg: add sdp_audio_codecs_cmp(), add compare flags

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28783 ) Change subject: sdp_msg: add sdp_audio_codecs_cmp(), add compare flags ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28783 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I18d33e189674229501afec950aa1c732386455a2 Gerrit-Change-Number: 28783 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sat, 30 Jul 2022 19:43:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 10 months

1
0
0 0

Change in osmo-cbc[master]: sbcap: Log info about messages received and trasmitted

by neels

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-cbc/+/28819 ) Change subject: sbcap: Log info about messages received and trasmitted ...................................................................... Patch Set 3: (2 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-cbc/+/28819/comment/12db3bfe_4af0829b PS2, Line 8: > I see no need for writing an issue if it's a generic patch simply refactoring/improving some code. […] it's about a hint to me where i can account the review work to File src/sbcap/sbcap_common.c: https://gerrit.osmocom.org/c/osmo-cbc/+/28819/comment/f96f757d_038eb039 PS2, Line 162: static char pdu_name[256] = "<unknown>"; > I rather have a static buf in the app than using OTC_SELECT. […] hmm. a while back my impression was that we consciously decided to avoid static buffers, because over the years they build up, amounting to a lot of memory being hogged, completely unused for the vast majority of runtime. Ever since that decision i've been going out of my way to avoid static buffers There are functional benefits to talloc strings; in addition to the ones i posted before, a talloc string will never cut short a string because the static buffer is too small; i.e. we don't need to pick a size that the largest possible string might ever need. Since my attention was brought to this aspect, my opinion is that static buffers should be avoided. they are bad in various ways. this patch is already merged but i consider this still an open issue you brushed over -- To view, visit https://gerrit.osmocom.org/c/osmo-cbc/+/28819 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-cbc Gerrit-Branch: master Gerrit-Change-Id: Ifd1eca79bf4fac63de8066cd6a18004138d51d04 Gerrit-Change-Number: 28819 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sat, 30 Jul 2022 19:41:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 10 months

1
0
0 0

Change in osmo-ttcn3-hacks[master]: BTS_Tests: send SACCH blocks in TC_rsl_ms_pwr_dyn_{up,ass_updown}()

by fixeria

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/28869 to look at the new patch set (#2). Change subject: BTS_Tests: send SACCH blocks in TC_rsl_ms_pwr_dyn_{up,ass_updown}() ...................................................................... BTS_Tests: send SACCH blocks in TC_rsl_ms_pwr_dyn_{up,ass_updown}() We used to rely on trxcon sendig dummy RR Measurement Reports and patching the L1 SACCH Header with the configured MS Power Level and Timing Advance values. Recently TC_rsl_ms_pwr_dyn_ass_updown started to fail because trxcon does not patch dummy MRs anymore. Do not rely on dummy MRs, send SACCH blocks with the expected MS Power Level and Timing Advance values from the testsuites. Change-Id: I31dd6b9026d04403092256176f67785a0a6486ad Related: OS#5635 --- M bts/BTS_Tests.ttcn 1 file changed, 16 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/69/28869/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/28869 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I31dd6b9026d04403092256176f67785a0a6486ad Gerrit-Change-Number: 28869 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

2 years, 10 months

1
0
0 0

Change in osmo-ttcn3-hacks[master]: library/GSM_RR_Types: make ts_SacchL1Header easier to use

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/28875 ) Change subject: library/GSM_RR_Types: make ts_SacchL1Header easier to use ...................................................................... library/GSM_RR_Types: make ts_SacchL1Header easier to use Move parameter 'fpc' at the end and assign false by default, so that there is not need to pass false. We never set it to true anyway. Change-Id: I8a0ef562c2426a637fbb9fe3d50711ee7738d04f --- M bts/BTS_Tests.ttcn M library/GSM_RR_Types.ttcn M library/LAPDm_RAW_PT.ttcn 3 files changed, 8 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/75/28875/1 diff --git a/bts/BTS_Tests.ttcn b/bts/BTS_Tests.ttcn index 47d7ce1..adb0c2f 100644 --- a/bts/BTS_Tests.ttcn +++ b/bts/BTS_Tests.ttcn @@ -795,8 +795,7 @@ /* TITAN has weird (and often unusable) padding model, so we pad here manaully */ l3 := f_pad_oct(enc_GsmRrL3Message(meas_rep), 18, '00'O); l2 := f_pad_oct(enc_LapdmFrameAB(valueof(ts_LAPDm_AB(0, meas_rep))), 21, '00'O); - l1h := ts_SacchL1Header(g_pars.l1_pars.ms_power_level, false, - g_pars.l1_pars.ms_actual_ta); + l1h := ts_SacchL1Header(g_pars.l1_pars.ms_power_level, g_pars.l1_pars.ms_actual_ta); /* Send RR Measurement Report over the Um interface */ L1CTL.send(ts_L1CTL_DATA_REQ_SACCH(g_chan_nr, ts_RslLinkID_SACCH(0), l1h, l2)); @@ -2121,8 +2120,7 @@ log("LAPDm: ", lb); var template (value) SacchL1Header l1h := ts_SacchL1Header( - g_pars.l1_pars.ms_power_level, false, - g_pars.l1_pars.ms_actual_ta); + g_pars.l1_pars.ms_power_level, g_pars.l1_pars.ms_actual_ta); /* According to 3GPP TS 44.018, section 10.5.2.20, we should pad with zeroes */ var octetstring l2 := f_pad_oct(enc_LapdmFrameAB(valueof(lb)), 21, '00'O); @@ -6897,7 +6895,7 @@ /* TODO: we can use an extension of TTCN-3 for padding, i.e. PADDING('2B'O) */ if (valueof(link_id.c) == SACCH) { /* Compose dummy L1 header */ - l1h := ts_SacchL1Header(g_pars.l1_pars.ms_power_level, false, g_pars.l1_pars.ms_actual_ta); + l1h := ts_SacchL1Header(g_pars.l1_pars.ms_power_level, g_pars.l1_pars.ms_actual_ta); L1CTL.send(ts_L1CTL_DATA_REQ_SACCH(g_chan_nr, link_id, l1h, f_pad_oct(l2, 21, '2B'O))); } else { /* If required, pad L2 frame with constant 0x2b filling */ diff --git a/library/GSM_RR_Types.ttcn b/library/GSM_RR_Types.ttcn index 980020e..335e290 100644 --- a/library/GSM_RR_Types.ttcn +++ b/library/GSM_RR_Types.ttcn @@ -198,7 +198,10 @@ external function dec_SacchL1Header(in octetstring stream) return SacchL1Header with { extension "prototype(convert) decode(RAW)" }; - template (value) SacchL1Header ts_SacchL1Header(uint5_t ms_power_lvl, boolean fpc, uint8_t actual_ta) := { + template (value) SacchL1Header + ts_SacchL1Header(uint5_t ms_power_lvl, + uint8_t actual_ta, + boolean fpc := false) := { reserved := 0, fpc := fpc, ms_power_lvl := ms_power_lvl, diff --git a/library/LAPDm_RAW_PT.ttcn b/library/LAPDm_RAW_PT.ttcn index 3e2fba2..946da67 100644 --- a/library/LAPDm_RAW_PT.ttcn +++ b/library/LAPDm_RAW_PT.ttcn @@ -494,7 +494,7 @@ link_id := valueof(ts_RslLinkID_SACCH(lpd.sapi)); buf := f_pad_oct(enc_LapdmFrame(lpd.lapdm), 21, '2B'O); var SacchL1Header l1h := valueof(ts_SacchL1Header(ms_power_lvl, - false, timing_adv)); + timing_adv)); L1CTL.send(ts_L1CTL_DATA_REQ_SACCH(chan_desc.chan_nr, link_id, l1h, buf)); } else { -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/28875 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I8a0ef562c2426a637fbb9fe3d50711ee7738d04f Gerrit-Change-Number: 28875 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 10 months

1
0
0 0

Change in osmo-ttcn3-hacks[master]: BTS_Tests: populate SACCH cache in f_TC_ms_pwr_ctrl_{constant,pf_ewma}

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/28876 ) Change subject: BTS_Tests: populate SACCH cache in f_TC_ms_pwr_ctrl_{constant,pf_ewma} ...................................................................... BTS_Tests: populate SACCH cache in f_TC_ms_pwr_ctrl_{constant,pf_ewma} Change-Id: I89eb90815e86db466ea626f4c25f2634c1d942d5 Depends: osmocom-bb.git I0f467fc07cf844cc73465f235b36ba7d00788c9f Related: OS#5635 --- M bts/BTS_Tests.ttcn 1 file changed, 12 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/76/28876/1 diff --git a/bts/BTS_Tests.ttcn b/bts/BTS_Tests.ttcn index adb0c2f..270c0fb 100644 --- a/bts/BTS_Tests.ttcn +++ b/bts/BTS_Tests.ttcn @@ -7860,6 +7860,12 @@ /* Ensure that 'rx-current' equals 'rx-target' */ f_trxc_fake_rssi(mp_uplink_power_target); + /* Populate SACCH cache with a Measurement Report */ + L1CTL.send(ts_L1CTL_DATA_REQ_SACCH(g_chan_nr, ts_RslLinkID_SACCH(0), + ts_SacchL1Header(g_pars.l1_pars.ms_power_level, + g_pars.l1_pars.ms_actual_ta), + f_pad_oct('010349'O & '0615004001C0'O, 21, '00'O))); + /* Establish a dedicated channel */ f_est_dchan(more_ies := valueof(ies)); @@ -7921,6 +7927,12 @@ /* Ensure that 'rx-current' equals 'rx-target' */ f_trxc_fake_rssi(mp_uplink_power_target); + /* Populate SACCH cache with a Measurement Report */ + L1CTL.send(ts_L1CTL_DATA_REQ_SACCH(g_chan_nr, ts_RslLinkID_SACCH(0), + ts_SacchL1Header(g_pars.l1_pars.ms_power_level, + g_pars.l1_pars.ms_actual_ta), + f_pad_oct('010349'O & '0615004001C0'O, 21, '00'O))); + /* Establish a dedicated channel */ f_est_dchan(more_ies := valueof(ies)); -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/28876 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I89eb90815e86db466ea626f4c25f2634c1d942d5 Gerrit-Change-Number: 28876 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 10 months

1
0
0 0

Change in osmocom-bb[master]: trxcon: allow populating global SACCH cache via L1CTL

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmocom-bb/+/28874 ) Change subject: trxcon: allow populating global SACCH cache via L1CTL ...................................................................... trxcon: allow populating global SACCH cache via L1CTL Change-Id: I0f467fc07cf844cc73465f235b36ba7d00788c9f Related: OS#5635, OS#5599 --- M src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h M src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h M src/host/trxcon/src/l1ctl.c M src/host/trxcon/src/sched_prim.c M src/host/trxcon/src/sched_trx.c M src/host/trxcon/src/trxcon_fsm.c 6 files changed, 75 insertions(+), 63 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/74/28874/1 diff --git a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h index 3f7dae0..04303f0 100644 --- a/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h +++ b/src/host/trxcon/include/osmocom/bb/l1sched/l1sched.h @@ -374,6 +374,8 @@ void (*clock_cb)(struct l1sched_state *sched); /*! List of timeslots maintained by this scheduler */ struct l1sched_ts *ts[TRX_TS_COUNT]; + /*! SACCH cache (common for all lchans) */ + uint8_t sacch_cache[GSM_MACBLOCK_LEN]; /*! BSIC value learned from SCH bursts */ uint8_t bsic; /*! Logging context (used as prefix for messages) */ diff --git a/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h b/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h index 6a43bcb..430b26e 100644 --- a/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h +++ b/src/host/trxcon/include/osmocom/bb/trxcon/trxcon.h @@ -25,6 +25,7 @@ TRXCON_EV_SET_TCH_MODE_REQ, TRXCON_EV_PHY_CONFIG_REQ, TRXCON_EV_TX_ACCESS_BURST_REQ, + TRXCON_EV_UPDATE_SACCH_CACHE_REQ, TRXCON_EV_DEDICATED_ESTABLISH_REQ, TRXCON_EV_DEDICATED_RELEASE_REQ, TRXCON_EV_TX_TRAFFIC_REQ, diff --git a/src/host/trxcon/src/l1ctl.c b/src/host/trxcon/src/l1ctl.c index b907917..c6939f5 100644 --- a/src/host/trxcon/src/l1ctl.c +++ b/src/host/trxcon/src/l1ctl.c @@ -644,10 +644,18 @@ "Recv %s Req (chan_nr=0x%02x, link_id=0x%02x, len=%zu)\n", traffic ? "TRAFFIC" : "DATA", req.chan_nr, req.link_id, req.data_len); - if (traffic) - osmo_fsm_inst_dispatch(trxcon->fi, TRXCON_EV_TX_TRAFFIC_REQ, &req); - else - osmo_fsm_inst_dispatch(trxcon->fi, TRXCON_EV_TX_DATA_REQ, &req); + switch (trxcon->fi->state) { + case TRXCON_ST_DEDICATED: + if (traffic) + osmo_fsm_inst_dispatch(trxcon->fi, TRXCON_EV_TX_TRAFFIC_REQ, &req); + else + osmo_fsm_inst_dispatch(trxcon->fi, TRXCON_EV_TX_DATA_REQ, &req); + break; + default: + if (!traffic && req.link_id == 0x40) + osmo_fsm_inst_dispatch(trxcon->fi, TRXCON_EV_UPDATE_SACCH_CACHE_REQ, &req); + /* TODO: log an error about uhnandled DATA.req / TRAFFIC.req */ + } msgb_free(msg); return 0; diff --git a/src/host/trxcon/src/sched_prim.c b/src/host/trxcon/src/sched_prim.c index 9b6d28d..f97e505 100644 --- a/src/host/trxcon/src/sched_prim.c +++ b/src/host/trxcon/src/sched_prim.c @@ -116,8 +116,7 @@ } /** - * Composes a new primitive using either cached (if populated), - * or "dummy" Measurement Report message. + * Composes a new primitive from cached RR Measurement Report. * * @param lchan lchan to assign a primitive * @return SACCH primitive to be transmitted @@ -125,38 +124,8 @@ static struct l1sched_ts_prim *prim_compose_mr(struct l1sched_lchan_state *lchan) { struct l1sched_ts_prim *prim; - uint8_t *mr_src_ptr; bool cached; - /* "Dummy" Measurement Report */ - static const uint8_t meas_rep_dummy[] = { - /* L1 SACCH pseudo-header */ - 0x0f, 0x00, - - /* LAPDm header */ - 0x01, 0x03, 0x49, - - /* RR Management messages, Measurement Report */ - 0x06, 0x15, - - /* Measurement results (see 3GPP TS 44.018, section 10.5.2.20): - * 0... .... = BA-USED: 0 - * .0.. .... = DTX-USED: DTX was not used - * ..11 0110 = RXLEV-FULL-SERVING-CELL: -57 <= x < -56 dBm (54) - * 0... .... = 3G-BA-USED: 0 - * .1.. .... = MEAS-VALID: The measurement results are not valid - * ..11 0110 = RXLEV-SUB-SERVING-CELL: -57 <= x < -56 dBm (54) - * 0... .... = SI23_BA_USED: 0 - * .000 .... = RXQUAL-FULL-SERVING-CELL: BER < 0.2%, Mean value 0.14% (0) - * .... 000. = RXQUAL-SUB-SERVING-CELL: BER < 0.2%, Mean value 0.14% (0) - * .... ...1 11.. .... = NO-NCELL-M: Neighbour cell information not available */ - 0x36, 0x76, 0x01, 0xc0, - - /* 0** -- Padding with zeroes */ - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - }; - /* Allocate a new primitive */ prim = prim_alloc(lchan, GSM_MACBLOCK_LEN, L1SCHED_PRIM_DATA, l1sched_lchan_desc[lchan->type].chan_nr, @@ -167,43 +136,24 @@ cached = (lchan->sacch.mr_cache[2] != 0x00 && lchan->sacch.mr_cache[3] != 0x00 && lchan->sacch.mr_cache[4] != 0x00); - if (cached) { /* Use the cached one */ - mr_src_ptr = lchan->sacch.mr_cache; - lchan->sacch.mr_cache_usage++; - } else { /* Use "dummy" one */ - mr_src_ptr = (uint8_t *) meas_rep_dummy; + if (!cached) { + memcpy(&lchan->sacch.mr_cache[0], + &lchan->ts->sched->sacch_cache[0], + sizeof(lchan->sacch.mr_cache)); } /* Compose a new Measurement Report primitive */ - memcpy(prim->payload, mr_src_ptr, GSM_MACBLOCK_LEN); - - /** - * Update the L1 SACCH pseudo-header (only for cached MRs) - * - * TODO: filling of the actual values into cached Measurement - * Reports would break the distance spoofing feature. If it - * were known whether the spoofing is enabled or not, we could - * decide whether to update the cached L1 SACCH header here. - */ - if (!cached) { -#warning "FIXME: no direct access to trxcon->l1p.{tx_power,ta}" -#if 0 - prim->payload[0] = lchan->ts->sched->trx->tx_power; - prim->payload[1] = lchan->ts->sched->trx->ta; -#endif - } + memcpy(&prim->payload[0], &lchan->sacch.mr_cache[0], GSM_MACBLOCK_LEN); /* Inform about the cache usage count */ - if (cached && lchan->sacch.mr_cache_usage > 5) { + if (++lchan->sacch.mr_cache_usage > 5) { LOGP_LCHAND(lchan, LOGL_NOTICE, "SACCH MR cache usage count=%u > 5 " "=> ancient measurements, please fix!\n", lchan->sacch.mr_cache_usage); } - LOGP_LCHAND(lchan, LOGL_NOTICE, - "Using a %s Measurement Report\n", - cached ? "cached" : "dummy"); + LOGP_LCHAND(lchan, LOGL_NOTICE, "Using cached Measurement Report\n"); return prim; } diff --git a/src/host/trxcon/src/sched_trx.c b/src/host/trxcon/src/sched_trx.c index 10a4c5f..00adcd6 100644 --- a/src/host/trxcon/src/sched_trx.c +++ b/src/host/trxcon/src/sched_trx.c @@ -39,6 +39,35 @@ int l1sched_log_cat_common = DLGLOBAL; int l1sched_log_cat_data = DLGLOBAL; +/* "Dummy" Measurement Report */ +static const uint8_t meas_rep_dummy[] = { + /* L1 SACCH pseudo-header */ + 0x0f, 0x00, + + /* LAPDm header */ + 0x01, 0x03, 0x49, + + /* RR Management messages, Measurement Report */ + 0x06, 0x15, + + /* Measurement results (see 3GPP TS 44.018, section 10.5.2.20): + * 0... .... = BA-USED: 0 + * .0.. .... = DTX-USED: DTX was not used + * ..11 0110 = RXLEV-FULL-SERVING-CELL: -57 <= x < -56 dBm (54) + * 0... .... = 3G-BA-USED: 0 + * .1.. .... = MEAS-VALID: The measurement results are not valid + * ..11 0110 = RXLEV-SUB-SERVING-CELL: -57 <= x < -56 dBm (54) + * 0... .... = SI23_BA_USED: 0 + * .000 .... = RXQUAL-FULL-SERVING-CELL: BER < 0.2%, Mean value 0.14% (0) + * .... 000. = RXQUAL-SUB-SERVING-CELL: BER < 0.2%, Mean value 0.14% (0) + * .... ...1 11.. .... = NO-NCELL-M: Neighbour cell information not available */ + 0x36, 0x76, 0x01, 0xc0, + + /* 0** -- Padding with zeroes */ + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, +}; + static int l1sched_cfg_pchan_comb_req(struct l1sched_state *sched, uint8_t tn, enum gsm_phys_chan_config pchan) { @@ -175,6 +204,8 @@ .priv = priv, }; + memcpy(&sched->sacch_cache[0], &meas_rep_dummy[0], sizeof(meas_rep_dummy)); + if (cfg->log_prefix == NULL) sched->log_prefix = talloc_asprintf(sched, "l1sched[0x%p]: ", sched); else @@ -217,6 +248,8 @@ /* Stop and reset clock counter if required */ if (reset_clock) l1sched_clck_reset(sched); + + memcpy(&sched->sacch_cache[0], &meas_rep_dummy[0], sizeof(meas_rep_dummy)); } struct l1sched_ts *l1sched_add_ts(struct l1sched_state *sched, int tn) diff --git a/src/host/trxcon/src/trxcon_fsm.c b/src/host/trxcon/src/trxcon_fsm.c index fb9bd8f..b5a153a 100644 --- a/src/host/trxcon/src/trxcon_fsm.c +++ b/src/host/trxcon/src/trxcon_fsm.c @@ -82,6 +82,22 @@ } break; } + case TRXCON_EV_UPDATE_SACCH_CACHE_REQ: + { + const struct trxcon_param_tx_traffic_data_req *req = data; + + if (req->link_id != L1SCHED_CH_LID_SACCH) { + LOGPFSML(fi, LOGL_ERROR, "Unexpected link_id=%u\n", req->link_id); + break; + } + if (req->data_len != GSM_MACBLOCK_LEN) { + LOGPFSML(fi, LOGL_ERROR, "Unexpected data length=%u\n", req->data_len); + break; + } + + memcpy(&trxcon->sched->sacch_cache[0], req->data, req->data_len); + break; + } default: OSMO_ASSERT(0); } @@ -483,6 +499,7 @@ OSMO_VALUE_STRING(TRXCON_EV_SET_TCH_MODE_REQ), OSMO_VALUE_STRING(TRXCON_EV_PHY_CONFIG_REQ), OSMO_VALUE_STRING(TRXCON_EV_TX_ACCESS_BURST_REQ), + OSMO_VALUE_STRING(TRXCON_EV_UPDATE_SACCH_CACHE_REQ), OSMO_VALUE_STRING(TRXCON_EV_DEDICATED_ESTABLISH_REQ), OSMO_VALUE_STRING(TRXCON_EV_DEDICATED_RELEASE_REQ), OSMO_VALUE_STRING(TRXCON_EV_TX_TRAFFIC_REQ), @@ -503,7 +520,8 @@ | S(TRXCON_EV_L2IF_FAILURE) | S(TRXCON_EV_RESET_FULL_REQ) | S(TRXCON_EV_RESET_SCHED_REQ) - | S(TRXCON_EV_PHY_CONFIG_REQ), + | S(TRXCON_EV_PHY_CONFIG_REQ) + | S(TRXCON_EV_UPDATE_SACCH_CACHE_REQ), .allstate_action = &trxcon_allstate_action, .timer_cb = &trxcon_timer_cb, .pre_term = &trxcon_fsm_pre_term_cb, -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/28874 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I0f467fc07cf844cc73465f235b36ba7d00788c9f Gerrit-Change-Number: 28874 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 10 months

1
0
0 0

Change in docker-playground[master]: OsmoMSC: add image to test gerrit patchset

by msuraev

msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/28873 ) Change subject: OsmoMSC: add image to test gerrit patchset ...................................................................... Patch Set 1: This change is ready for review. -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/28873 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: I9cc4c804825e653c2da285396a395e4d986fc157 Gerrit-Change-Number: 28873 Gerrit-PatchSet: 1 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Sat, 30 Jul 2022 17:00:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 10 months

1
0
0 0

2025

2024

2023

2022

gerrit-log July 2022