On Thu Oct 3, 2024 at 4:47 PM CEST, Benjamin Berg wrote: > On Thu, 2024-10-03 at 15:59 +0200, Nicolas Escande wrote: > > Hello Benjamin, > > > > On Thu Sep 26, 2024 at 5:34 PM CEST, Benjamin Berg wrote: > > > From: Benjamin Berg <benjamin.berg@xxxxxxxxx> > > > > > > Both of these implementations are broken by design currently and will > > > not work correctly with any socket that needs to be registered for > > > multiple event types (reading, writing, exception). > > > > > > The reason for this is that the eloop_register_sock API can only take > > > one event at a time resulting in multiple entries for the FD. However, > > > epoll_ctl() will refuse to register the FD a second time and kqueue() > > > will happily overwrite the earlier information. As such, neither > > > implementation is working correctly. > > > > I never even realized that sometimes hostapd registers a socket for writing. > > I think in the DBus case it is exceptions and either reading or > writing. There are also the DPP/HTTP/Radius clients that register > writing, but that might work just fine as long as only one event type > is active at a time. > > > > Simply remove both, I doubt anyone ever tried to use them as at least > > > DBus is broken. Also, I don't expect that hostapd/wpa_supplicant has a > > > lot of FDs/sockets making this a micro-optimization which is likely > > > pointless in the first place. > > > > > Actually, in our product we heavily use the epoll implementation, but with a > > very light config / feature set (no dbus or dpp...) so we never encountered a > > problem of that nature. > > > > Instead of removing the code, we could $(error ) or # error for incompatible > > configs ? But it seems hard to maintain in the long run when we add new stuff. > > Either way, I don't really care as long as the poll implementationworks, we'll > > switch to it if need be. > > To be honest, one motivation for me to remove it entirely was because > it seemed unnecessarily hard to work with the code (I would have a hard > time testing kqueue at least). And I was wondering whether it might be > worthwhile to add a prioritization feature to eloop as that could make > some event handling code more predictable. For example, one could > ensure all nl80211 events are processed before the next timeout is > fired. I just wanted to point out that in specific configs epoll was not broken. So maybe, if the only motivation was "it's broken, lets not anybody be deceived by a silently broken build, lets remove it", we could have gone the error way instead. But it's more work, and I don't mind removing epoll as long as poll works instead. kqueue is alien for me, I don't care for it at all. > > All that said, do you maybe have an idea if it is actually making a > relevant performance difference? No, not at all. I don't really care about performance, we want reliability first and foremost. > > Benjamin Nicolas _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
