October 2023 - gerrit-log - lists.osmocom.org

[S] Change in libosmo-netif[master]: stream (test): Fix Coverity CID 323456
by pespin 05 Oct '23

05 Oct '23

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/34671?usp=email ) Change subject: stream (test): Fix Coverity CID 323456 ...................................................................... Patch Set 1: (1 comment) File tests/stream/stream_test.c: https://gerrit.osmocom.org/c/libosmo-netif/+/34671/comment/dca0e02b_b5fdde32 PS1, Line 681: if (5 < ipa_msg_type) { > I thought about it, but then I also thought somebody else might complain that leaving it the way it […] feel free to change it if you want, it shouldn't have been in the other way before anyway imho. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/34671?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2efb28feae4d4fa7516702f01026af09aa3777ac Gerrit-Change-Number: 34671 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Oct 2023 17:09:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: Make RLC timing data configurable
by pespin 05 Oct '23

05 Oct '23

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email ) Change subject: Make RLC timing data configurable ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc/net_init.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/5a1a2b04_521c0d30 PS2, Line 51: { .T = 3142, .default_val = 20, > I don't have a GSM background, let alone ever configured/tuned any GSM network settings. […] I am (and others) available for questions if you are unsure where to put stuff :) If you are uncertain, then better raising the topic before writing tons of lines next time may be good. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675 Gerrit-Change-Number: 31878 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Oct 2023 17:09:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream (test): Fix Coverity CID 323456
by arehbein 05 Oct '23

05 Oct '23

Attention is currently required from: pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/34671?usp=email ) Change subject: stream (test): Fix Coverity CID 323456 ...................................................................... Patch Set 1: (1 comment) File tests/stream/stream_test.c: https://gerrit.osmocom.org/c/libosmo-netif/+/34671/comment/67f9a54b_46126c34 PS1, Line 681: if (5 < ipa_msg_type) { > this syntax for comparision is a bit weird imho. I'd say we usually have the vars on the left side. […] I thought about it, but then I also thought somebody else might complain that leaving it the way it is would make reading the diff more easy, so I left it the way it was. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/34671?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2efb28feae4d4fa7516702f01026af09aa3777ac Gerrit-Change-Number: 34671 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Oct 2023 17:04:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: Make RLC timing data configurable
by arehbein 05 Oct '23

05 Oct '23

Attention is currently required from: pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email ) Change subject: Make RLC timing data configurable ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc/net_init.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/aa4fc799_e07c215a PS2, Line 51: { .T = 3142, .default_val = 20, > > So, how do I know which timers are BTS-specific or depend on BTS support? […] I don't have a GSM background, let alone ever configured/tuned any GSM network settings. These timers touch different kinds of procedures described across different specs, too, I would guess. So while it may or may not be easy to do so once I've acquired the right knowledge to make these decisions, it will probably be quite time consuming for me to read up on all of that. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675 Gerrit-Change-Number: 31878 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Oct 2023 17:02:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream (test): Fix Coverity CID 323456
by pespin 05 Oct '23

05 Oct '23

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/34671?usp=email ) Change subject: stream (test): Fix Coverity CID 323456 ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File tests/stream/stream_test.c: https://gerrit.osmocom.org/c/libosmo-netif/+/34671/comment/a4d7729e_1e75ea35 PS1, Line 681: if (5 < ipa_msg_type) { this syntax for comparision is a bit weird imho. I'd say we usually have the vars on the left side. That's basically: "if (ipa_msg_type > 5)" right? not criticial though. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/34671?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2efb28feae4d4fa7516702f01026af09aa3777ac Gerrit-Change-Number: 34671 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Oct 2023 16:54:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: Make RLC timing data configurable
by pespin 05 Oct '23

05 Oct '23

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email ) Change subject: Make RLC timing data configurable ...................................................................... Patch Set 2: (2 comments) File src/osmo-bsc/net_init.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/0b3ff55e_64a5f16d PS2, Line 51: { .T = 3142, .default_val = 20, > So, how do I know which timers are BTS-specific or depend on BTS support? Easy procedure: Would it make sense for a user/operator to set different values per BTS? Answer: yes! => per bts. Wuthout looking in detail, some of thse values clearly could be tunned different on different BTS, with different operation constrains, different volume of users, different load, etc. I think to have one group named "bts" should be enough, even if some bts don't support setting some of the params. File tests/nanobts_omlattr/nanobts_omlattr_test.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/4931c325_0debbf22 PS2, Line 228: 0x02, 0x00, 0x02, 0xa3, 0x00, 0x09, > No default values have been changed. […] ah ok. Can you at least take the chance to align them to the same column? -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675 Gerrit-Change-Number: 31878 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Oct 2023 16:51:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream (test): Fix Coverity CID 323456
by arehbein 05 Oct '23

05 Oct '23

arehbein has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/34671?usp=email ) Change subject: stream (test): Fix Coverity CID 323456 ...................................................................... stream (test): Fix Coverity CID 323456 Fix Coverity CID 323456 ("Control flow issues (NO_EFFECT)") by removing 'greater than 0'-check on unsigned integer-type variable. Change-Id: I2efb28feae4d4fa7516702f01026af09aa3777ac --- M tests/stream/stream_test.c 1 file changed, 13 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/71/34671/1 diff --git a/tests/stream/stream_test.c b/tests/stream/stream_test.c index e945a1e..960d8fa 100644 --- a/tests/stream/stream_test.c +++ b/tests/stream/stream_test.c @@ -678,7 +678,7 @@ } LOGSRV(conn, "Received message from stream (total len including stripped headers = %lu)\n", osmo_ntohs(h->len) + sizeof(*h)); - if (ipa_msg_type < 0 || 5 < ipa_msg_type) { + if (5 < ipa_msg_type) { fprintf(stderr, "Received unexpected IPAC message type %"PRIu8"\n", ipa_msg_type); return -ENOMSG; } -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/34671?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2efb28feae4d4fa7516702f01026af09aa3777ac Gerrit-Change-Number: 34671 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[M] Change in osmo-bsc[master]: Make RLC timing data configurable
by arehbein 05 Oct '23

05 Oct '23

Attention is currently required from: pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email ) Change subject: Make RLC timing data configurable ...................................................................... Patch Set 2: (4 comments) File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/2a852ad1_4b2065fe PS2, Line 50: #define GSM_N3101_STRICT_LOWER_BOUND 8UL > STRICT_LOWER_BOUND means simply "MIN" ? :D Well, no, https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/80fbf532_679c8459 PS2, Line 50: #define GSM_N3101_STRICT_LOWER_BOUND 8UL > STRICT_LOWER_BOUND means simply "MIN" ? :D The spec states that 'N3101 > 8', so it isn't a minimum (it's not an allowed value). I put the strict lower bound and not the minimum as a define, because that's how it's stated in the spec. File src/osmo-bsc/net_init.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/d1a74a6b_efb0cb0c PS2, Line 51: { .T = 3142, .default_val = 20, > I know tons of parameters are already added here, but we should not continue adding BTS-specific par […] So, how do I know which timers are BTS-specific or depend on BTS support? I first thought the groupings suggested on https://www.rfwireless-world.com/Terminology/GSM-timers.html (GSM Timers, GSM Timers Network Side(BTS), ...) might make sense, but T3142 isn't listed there as a BTS timer, it's listed as a timer for the mobile side (MSC I guess). I was thinking about adding a group named "bts". Or am I supposed to add several groups for different kinds of BTSs/ File tests/nanobts_omlattr/nanobts_omlattr_test.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31878/comment/623669e9_742473c6 PS2, Line 228: 0x02, 0x00, 0x02, 0xa3, 0x00, 0x09, > why is this changing? are you changing the default values? No default values have been changed. I just added changed the paragraph formatting a bit and added a comment line so that if anyone else reads the code in the future, it's possible to read which octet contains which information without having to go through the entire code. It shouldn't be necessary for anybody to lose time over looking up this information again, at least not for that line. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675 Gerrit-Change-Number: 31878 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Oct 2023 16:35:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in ...osmo-epdg[master]: gitreview: new file
by osmith 05 Oct '23

05 Oct '23

osmith has submitted this change. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/34668?usp=email ) Change subject: gitreview: new file ...................................................................... gitreview: new file Change-Id: I3e00e433f57df47e0d873d8405fae55c6656781f --- A .gitreview 1 file changed, 12 insertions(+), 0 deletions(-) Approvals: osmith: Verified pespin: Looks good to me, approved diff --git a/.gitreview b/.gitreview new file mode 100644 index 0000000..f73c47a --- /dev/null +++ b/.gitreview @@ -0,0 +1,3 @@ +[gerrit] +host=gerrit.osmocom.org +project=erlang/osmo-epdg -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/34668?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: erlang/osmo-epdg Gerrit-Branch: master Gerrit-Change-Id: I3e00e433f57df47e0d873d8405fae55c6656781f Gerrit-Change-Number: 34668 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in ...osmo-epdg[master]: gitreview: new file
by osmith 05 Oct '23

05 Oct '23

osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/34668?usp=email ) Change subject: gitreview: new file ...................................................................... Patch Set 1: Verified+1 -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/34668?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: erlang/osmo-epdg Gerrit-Branch: master Gerrit-Change-Id: I3e00e433f57df47e0d873d8405fae55c6656781f Gerrit-Change-Number: 34668 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 05 Oct 2023 16:20:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0