Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Closed #1288. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#event-1558135695___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Closing the issue even when xflags as currently implemented do not behave the same way as flags and thus cannot be used as their replacement. -- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-378851744___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes: > The purpose of that t_flush_flags() is documented: > > * > https://www.kamailio.org/docs/modules/stable/modules/tmx.html#tmx.f.t_flush_flags > > The role was to push the flags set after t_newtran() to > transaction. If that is not happening/is not needed for what so ever > reason, needs to be sorted out in a way or another. Yes, and that is the topic of #1490. > The behaviour can stay as it is, with or without any new parameter -- > this is going to be discussed. But again, it doesn't belong here, the > issue we discuss is with flushing the flags after t_newtrans() and > should be done in a single place. The xflags will follow whatever is > decided for t_flush_flags(), but right now xflags works as it is > expected based on docs for t_flush_flags(). > Let's discuss further on #1490 and get the two work the same in a > properly documented behaviour. OK, I'll close this issue with the note that the current semantics of xflags does not correspond to that of flags. -- Juha ___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
The purpose of that t_flush_flags() is documented: * https://www.kamailio.org/docs/modules/stable/modules/tmx.html#tmx.f.t_flush_flags The role was to push the flags set after t_newtran() to transaction. If that is not happening/is not needed for what so ever reason, needs to be sorted out in a way or another. The behaviour can stay as it is, with or without any new parameter -- this is going to be discussed. But again, it doesn't belong here, the issue we discuss is with flushing the flags after t_newtrans() and should be done in a single place. The xflags will follow whatever is decided for t_flush_flags(), but right now xflags works as it is expected based on docs for t_flush_flags(). Let's discuss further on #1490 and get the two work the same in a properly documented behaviour. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-378844680___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes: > As I wrote on the mailing list, flags should not be migrated to the > transaction after t_newtran() unless one uses t_flush_flags() -- that > was the desired functionality, otherwise t_flush_flags() has no > purpose. I have used flags for many years and for me it has been desirable that they are available after t_newtrans() in failure and onreply routes without a need to call t_flush_flags(). If someone wants to change that behavior and make t_flush_flags() purposeful, then a tm variable can be introduced to achieve that new behavior. > You opened another item on this tracker related to this unexpected > behaviour (#1490), and I think it is better to sort this out there, > because that is the unexpected behaviour. xflags behave as > expected. Whatever will be the conclusion of #1490 will be applied to > both flags and xflags, but makes no sense to track an issue with two > separate items. I opened this issue, because I needed more that 32 flags, i.e., flags that work the same way as before, but more of them. It is OK to replace flags with xflags if my current configuration file keeps on working when I textually change current flag function calls with xflag function calls, but it is not OK if I need to start adding t_flush_xflags() calls to it. Issue "Is t_flush_flags() really needed? #1490" asks about the purpose of t_flush_flags() function. I have not been able to figure out one, but perhaps there is some. If there is no purpose, then the function should be removed. -- Juha -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-378838094___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
As I wrote on the mailing list, flags should not be migrated to the transaction after t_newtran() unless one uses t_flush_flags() -- that was the desired functionality, otherwise t_flush_flags() has no purpose. You opened another item on this tracker related to this unexpected behaviour (#1490), and I think it is better to sort this out there, because that is the unexpected behaviour. xflags behave as expected. Whatever will be the conclusion of #1490 will be applied to both flags and xflags, but makes no sense to track an issue with two separate items. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-378834818___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Reopened #1288. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#event-1558010774___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
As long as xflags do not work the same way as flags, I do not consider this issue closed. Flags do not require call of t_flush_flags() after t_newtrans() in order to make flags available in failure and onreply routes, but xflags does require call of t_flush_xflags(). This makes xflags cumbersome to use, since I would need to make t_flush_xflags() call before every t_relay() call. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-378833596___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Closing, xflags being implemented. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-378830173___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Closed #1288. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#event-1557989344___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
The functions were added by mistake inside a disabled ifdef zone. It should be fixed now in master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-374664629___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes: > Just added support for extended flags (xflags) with a capacity of 64 > flags. The functions to manage them are exported by corex module. No > time to test them, so feedback if all is fine or not is appreciated. I was going to give corex a try, but loading of the module failed like this: ar 21 01:39:35 trout sip-proxy[13013]: 0(13064) ERROR: [core/sr_module.c:582]: load_module(): could not open module : /usr/lib/x86_64-linux-gnu/sip-proxy/modules/corex.so: undefined symbol: setxflag -- Juha -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-374583220___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Just added support for extended flags (xflags) with a capacity of 64 flags. The functions to manage them are exported by corex module. No time to test them, so feedback if all is fine or not is appreciated. I will also add t_fush_xflags() soon in tmx module. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-372998360___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
How about using an array of unsigned ints, but functions have only one parameter? Array index is param / 32 and flag number at the index is param % 32. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-340836695___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes: > Another way could be adding a new field, extended flags, with a > different set of functions to handle them like > setflagx()/resetflagx(...)/isflagxset(...), so the current behaviour > is not affected at all. That would be fine with me. > view I think using an array (static size) is better than single value > field. Initially it can be of uint32_t[2] to give access to 64 more > flags, but in the future it can be changed if needed. The functions > will take two parameters, bit index and array index. That would be too complex to use. A flag should be a single integer. -- Juha -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-340545188___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
I wish that changing a structure is something simple, but is not. The main constraint is having the config interpreter able to deal only with integer numbers as values (static or via variables), and flags field stay behind a variable. But there could be also memory alignment problems when fields size changes. Another way could be adding a new field, extended flags, with a different set of functions to handle them like setflagx()/resetflagx(...)/isflagxset(...), so the current behaviour is not affected at all. At this moment, from extensibility point of view I think using an array (static size) is better than single value field. Initially it can be of uint32_t[2] to give access to 64 more flags, but in the future it can be changed if needed. The functions will take two parameters, bit index and array index. Just some ideas, it can be another way, but I would prefer not to affect that much existing code... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-340431612___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
My proposal is to change flag_t to unsigned long long after master is opened for new features. Then there is lots of time to fix code that might get broken. -- Juha -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-340398676___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
Daniel-Constantin Mierla writes: > This has implications on other parts, such as variables where they can > be accessed and set via $mf (iirc). Other parts of the code should not assume anything about the actual length of flag_t. If they do, that code needs to be fixed. > As an alternative for now would be using an avp or xavps, and do > bitwise operations (`|` and `&`) to set or test. Use of avps for storing bit flags is too slow. -- Juha -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-340397304___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
Re: [sr-dev] [kamailio/kamailio] define flag_t as unsigned long (#1288)
This has implications on other parts, such as variables where they can be accessed and set via $mf (iirc). On the other hand, I don't think having a variable with different lengths is a good approach, because it can break easily config behaviour when moving from one server to another. It should be same size on 32b or 64b, so if a change to increase the size is to be done, then should be `long long` to ensure it is 64 on all archs. An alternative would be to make the field inside sip_msg_t an array of flag_t values, so can be one or more, even beyond 64b. But this change will have also impact in other parts, therefore would need careful review. As an alternative for now would be using an avp or xavps, and do bitwise operations (`|` and `&`) to set or test. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/kamailio/kamailio/issues/1288#issuecomment-340394835___ Kamailio (SER) - Development Mailing List sr-dev@lists.kamailio.org https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
