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 <vyanits...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-Attention: fixeria <vyanits...@sysmocom.de> Gerrit-Comment-Date: Fri, 10 May 2024 15:30:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
