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

[S] Change in osmo-pcu[master]: tbf_dl_fsm: Ignore DL_ACKNACK_MISS events in WAIT_{RELEASE,REUSE_TFI}...
by pespin 29 Jun '23

29 Jun '23

Attention is currently required from: pespin. Hello osmith, Jenkins Builder, fixeria, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcu/+/33455 to look at the new patch set (#4). Change subject: tbf_dl_fsm: Ignore DL_ACKNACK_MISS events in WAIT_{RELEASE,REUSE_TFI} states ...................................................................... tbf_dl_fsm: Ignore DL_ACKNACK_MISS events in WAIT_{RELEASE,REUSE_TFI} states If in those states, we already left the FINISHED step which means we already received a FinalACk previously, hence it means we are missing requested retransmissions of the last DL ACK/NACK due to fn-advance (several DL blocks in transit before receiving UL response). Change-Id: Ib0f23a9cc3c614fe428b682e01502930cd2e478f --- M src/tbf_dl_fsm.c 1 file changed, 26 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/55/33455/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33455 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ib0f23a9cc3c614fe428b682e01502930cd2e478f Gerrit-Change-Number: 33455 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-pcu[master]: Reestore last LLC frames never completely acked when freeing DL TBF
by pespin 29 Jun '23

29 Jun '23

Attention is currently required from: pespin. Hello osmith, Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-pcu/+/33525 to look at the new patch set (#3). Change subject: Reestore last LLC frames never completely acked when freeing DL TBF ...................................................................... Reestore last LLC frames never completely acked when freeing DL TBF Scenario: A DL TBF is assigned over PCH (CCCH) and we start transmitting DL data blocks blindly after X2002, but at the same time the MS start packet-access-procedure to request an UL TBF. Right now osmo-pcu correctly detects the MS is available in PDCH and re-assigns a DL TBF using PACCH, but the LLC frames it transmitted in the old PCH-assigned DL TBF get lost when that older TBF is freed (because the DL blocks were removed from the GprsMs llc_queue). This issue is now more frequent since X2002 timer was added recently to delay starting requesting USF for a UL TBF, hence the contention resolution in general takes more time and hence the PACCH assignment of the DL TBF takes more time too, so more DL data blocks are transmitted to the DL TBF assigned over PCH during that time. This patch improves the situation to at least recover the DL blocks transmitted if the DL TBF is freed (due to MS merge trigger by scenario mentioned above), where no DL ACK/NACK was ever received by the MS. Ideal solution would be to have complete tracking of which LLC PDUs from the llc_queues were completely ACKed at RLC/MAC level, but that really requires a lot of work and major refactoring, which are left as a future improvement. Change-Id: I9be4035fb2cf2b3ee56e91dcc12cc8c24028b4aa --- M src/gprs_ms.c M src/llc.c M src/llc.h M src/tbf_dl.cpp M src/tbf_dl.h M tests/llc/LlcTest.cpp M tests/tbf/TbfTest.err 7 files changed, 158 insertions(+), 21 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/25/33525/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33525 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I9be4035fb2cf2b3ee56e91dcc12cc8c24028b4aa Gerrit-Change-Number: 33525 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-pcu[master]: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted'
by pespin 29 Jun '23

29 Jun '23

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33530 ) Change subject: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted' ...................................................................... Patch Set 2: (1 comment) File src/tbf_dl_fsm.c: https://gerrit.osmocom.org/c/osmo-pcu/+/33530/comment/40a1d2c3_11086416 PS2, Line 227: "(CCCH was not requested on current assignment)\n"); > This sounds like it will not be ignored if CCCH is in state_flags, but it is also ignoring it there […] Yes, it's intentional. We ignore it because it's from a previous assignment, but it won't trigger the alarms of a user looking at the code or logging of running osmo-pcu since it will be an expected event (receiving PCUIF_CNF if it was assigned over CCCH). -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33530 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I042b0117552acae25c750e762f5cc254399da64f Gerrit-Change-Number: 33530 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 12:55:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-pcu[master]: Re-store last LLC frames never completely acked when freeing DL TBF
by osmith 29 Jun '23

29 Jun '23

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33525 ) Change subject: Re-store last LLC frames never completely acked when freeing DL TBF ...................................................................... Patch Set 2: (2 comments) File src/llc.c: https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/ebcb53ad_a47e31eb PS2, Line 368: OSMO_ASSERT(info); > If the queue is empty, msg=NULL and return in line 362 occurs, so this case you say cannot happen. Ack File src/tbf_dl.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/b72bb0eb_c8abb27d PS2, Line 546: if any > See my other comment, function doesn't assert. Ack -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33525 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I9be4035fb2cf2b3ee56e91dcc12cc8c24028b4aa Gerrit-Change-Number: 33525 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 12:53:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-pcu[master]: Re-store last LLC frames never completely acked when freeing DL TBF
by pespin 29 Jun '23

29 Jun '23

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33525 ) Change subject: Re-store last LLC frames never completely acked when freeing DL TBF ...................................................................... Patch Set 2: (2 comments) File src/llc.c: https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/4efed514_77842342 PS2, Line 368: OSMO_ASSERT(info); > maybe add a comment on top of the function that the queue must not be empty if out_info is set? If the queue is empty, msg=NULL and return in line 362 occurs, so this case you say cannot happen. File src/tbf_dl.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/6ea3ec69_c7f9e926 PS2, Line 546: if any > might be a bit nitpicky, but the "if any" part is dealt with above; now the function will assert if […] See my other comment, function doesn't assert. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33525 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I9be4035fb2cf2b3ee56e91dcc12cc8c24028b4aa Gerrit-Change-Number: 33525 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 12:52:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-pcu[master]: Re-store last LLC frames never completely acked when freeing DL TBF
by pespin 29 Jun '23

29 Jun '23

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33525 ) Change subject: Re-store last LLC frames never completely acked when freeing DL TBF ...................................................................... Patch Set 2: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/76b926aa_ba24bfa3 PS2, Line 30: improvement. > Is there an issue about improving this in the future? (If so, reference it here?) time and headaches, nothing special. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33525 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I9be4035fb2cf2b3ee56e91dcc12cc8c24028b4aa Gerrit-Change-Number: 33525 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 12:36:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted'
by osmith 29 Jun '23

29 Jun '23

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33530 ) Change subject: tbf_dl_fsm: Fix '{FLOW}: Event ASSIGN_PCUIF_CNF not permitted' ...................................................................... Patch Set 2: (1 comment) File src/tbf_dl_fsm.c: https://gerrit.osmocom.org/c/osmo-pcu/+/33530/comment/f6b2fa93_219432ba PS2, Line 227: "(CCCH was not requested on current assignment)\n"); This sounds like it will not be ignored if CCCH is in state_flags, but it is also ignoring it there (just not logging a message), right? Is this intentional? -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33530 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I042b0117552acae25c750e762f5cc254399da64f Gerrit-Change-Number: 33530 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 12:25:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in osmo-pcu[master]: Re-store last LLC frames never completely acked when freeing DL TBF
by osmith 29 Jun '23

29 Jun '23

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33525 ) Change subject: Re-store last LLC frames never completely acked when freeing DL TBF ...................................................................... Patch Set 2: Code-Review+1 (11 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/2006a7be_dd574a81 PS2, Line 9: Scenario: A DL TBF is assigned over PCH (CCCH) and we start transmitting DL data blocks blindly (would be easier to read with in gerrit with line breaks at 72 characters) https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/41f7f316_468bb119 PS2, Line 15: where were https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/53f49dd6_9a31e5fc PS2, Line 28: where were https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/295765ae_a89f323f PS2, Line 29: og of https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/0cff7b61_5ffb52a3 PS2, Line 30: improvement. Is there an issue about improving this in the future? (If so, reference it here?) Patchset: PS2: just some nitpicks about typos and comments, patch looks good otherwise! File src/llc.c: https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/b51d1ba0_b3526796 PS2, Line 368: OSMO_ASSERT(info); maybe add a comment on top of the function that the queue must not be empty if out_info is set? File src/tbf_dl.h: https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/f266a2f9_d0f26510 PS2, Line 86: lossing losing File src/tbf_dl.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/f124de00_0d5ed293 PS2, Line 546: if any might be a bit nitpicky, but the "if any" part is dealt with above; now the function will assert if at this point the queue is empty. maybe remove that part of the comment? https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/3f6faa02_2ba3b0ce PS2, Line 667: lossing losing https://gerrit.osmocom.org/c/osmo-pcu/+/33525/comment/b63b24a6_6259ea4c PS2, Line 1152: trasnmission transmission -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33525 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I9be4035fb2cf2b3ee56e91dcc12cc8c24028b4aa Gerrit-Change-Number: 33525 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 12:17:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: tbf_dl_fsm: Ignore DL_ACKNACK_MISS events in WAIT_{RELEASE,REUSE_TFI}...
by osmith 29 Jun '23

29 Jun '23

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33455 ) Change subject: tbf_dl_fsm: Ignore DL_ACKNACK_MISS events in WAIT_{RELEASE,REUSE_TFI} states ...................................................................... Patch Set 3: Code-Review-1 (1 comment) File src/tbf_dl_fsm.c: https://gerrit.osmocom.org/c/osmo-pcu/+/33455/comment/4a2396e5_68b0e6b7 PS3, Line 331: * fn-advance scheduling several DL blocks in advance. */ missing break -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33455 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ib0f23a9cc3c614fe428b682e01502930cd2e478f Gerrit-Change-Number: 33455 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 11:55:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-pcu[master]: tbf_ul: Avoid processing rx UL bocks for UL TBFs in RELEASING state
by osmith 29 Jun '23

29 Jun '23

Attention is currently required from: laforge, pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33415 ) Change subject: tbf_ul: Avoid processing rx UL bocks for UL TBFs in RELEASING state ...................................................................... Patch Set 4: Code-Review+1 (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-pcu/+/33415/comment/c6690433_e9c53d20 PS4, Line 7: bocks blocks -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33415 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: I1da9b665b9ed83a644ea798008d456d6298b7460 Gerrit-Change-Number: 33415 Gerrit-PatchSet: 4 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-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 29 Jun 2023 11:54:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0