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