Attention is currently required from: fixeria.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-msc/+/36760?usp=email )
Change subject: libmsc: add X5 timer for delaying LU transactions
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
While this approach may work in practice, I find it a bit inelegant.
Man criticism:
- what guarantees that a pending SMS is sent within that 5 second wait time?
- I'd also prefer if we don't generally delay all LU for 5 seconds, most most most
of them without reason.
It would make more sense to me to run a pending-SMS-round for the subscriber before
put()ing the LU use count token.
There should be some mechanism like this to trigger SMS sending immediately on Complete
Layer 3 somewhere, at the very least for rx'ing a Paging Response. I can't seem to
find it now, only looked briefly... I found sms_queue.c sms_send_next(), but that is not
the right one AFAICT. Maybe you can find the right API trigger...?
If an SMS is pending, by starting the SMS operation, a new use token will be taken for the
conn, i.e. a new trans starts and picks up a use count on the conn. Then we can release
the LU use count immediately; the SMS will continue and keep the conn open until the SMS
done. If none was pending, releasing the LU token will immediately release without delay,
as usual. That's what I'd favor =)
IOW, when waiting for 5 seconds, is it *guaranteed* to run an sms queue in that time? how
does the pending SMS get sent? So can we run that SMS triggering loop once before
releasing the LU token (and hence have no need clean up a pending timer).
I dimly remember that in ttcn3 tests, when a previous test has put an SMS in the queue for
that IMSI, then subsequent tests get unexpected SMS signalling and the tests get messed
up. I thought this also happens during LU, but maybe it was non-LU compl3 only...? I'm
trying to say that, for this feature, we "just" want to make this "adverse
effect" that we already have more aggressive / more consistent.
Re Pau's remark, if you keep this patch with the timer delay instead of running the
SMS queue like I am suggesting, some of msc_a_fsm_cleanup() or trans_free() should
osmo_timer_del() the timer, to make very sure that the timer cb does not try to access a
freed pointer.
(A consideration: if we ever remove the SMS queue implementation from osmo-msc to a
separate SMSC, there should be some async operation to ask for a pending SMS before
releasing a conn. So, that would take out another use token for
"query-pending-SMS" and release it when the SMSC has responded. Maybe I'm
wrong because I'm not aware of how SMSC procedures work, just maybe something to keep
in mind as a general direction towards a distant future, when implementing this.)
--
To view, visit
https://gerrit.osmocom.org/c/osmo-msc/+/36760?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ic519cab55d65e47b2636124427dab1a1d80fab78
Gerrit-Change-Number: 36760
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 10 May 2024 15:30:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment