2022-04-04 11:03 (UTC+0100), Bruce Richardson: [...] > Having EAL be the only one to create threads seems reasonable. However, I'm > a little uncertain about the scope of change and tying telemetry and EAL > together a lot more.
Scope is a strong valid argument. What do you think if telemetry would create threads using Win32 API just like on Unices it creates threads using pthread (external component that is always present)? > Another alternative might be to change telemetry to > use only one thread for all connections and using "select" or "poll" to > wait on input from all sockets simultaneously. Would that work on Windows? Yes, there are select() and "poll()" with slightly different naming. > > * The logging issue can be solved differently: > > > > a) by factoring logging into a library, either as Bruce suggested; > > b) by replacing logging with simple print in Windows shim; > > c) by integrating and using the new threading API [1, 2]. > > > > What is the issue with the current logging in telemetry on windows? pthread.h shim contains calls to rte_log(). > The > current implementation has EAL pass in the logging function to telemetry on > init, allowing use of EAL logging without requiring a link-time dependency. > Theoretically, the same solution could also be used for the threading, > though I'm wary about doing so as using the same solution multiple times > could lead to very awkward APIs. Thinking out of scope of this RFC, telemetry *library* API should be: - register handler - JSON helpers - create session - process command in a session - destroy session All IO should be outside. Otherwise it's not a library but a framework. rte_telemetry_init() and all IO it starts is a higher-level helper for the very common case that doesn't need to be in lib/telemetry itself and even doesn't need to be a public API (but can be). > Overall, I tend to think the proper solution for all this is to split EAL - > something that was discussed previously but never done because it's > difficult and the result may be uncertain. Theoretically, most low-level > services in EAL, such as logging, should be in one library, while the EAL > init code remains in a later, higher-level one. I'd even say: utilities (log, trace, UUID, ...), environment abstraction, and runtime (init, tasks, multiprocess). It would be interesting to have a topic on that. [...] > > * It is technically possible to accept remote connections. > > However, it's a new attack surface and a security risk. > > OTOH, there's no need to allow remote connections: > > anyone who needs this can setup a tunnel or something. > > Local socket must be used by default. > > > > Completely agree on not accepting non-local connections. However, I'd also > point out that another reason why a unix socket was chosen over other > TCP/UDP/etc socket options was that it also gave local-user protection. > Right now, if a DPDK process is run as root, no non-root user can connect > to the telemetry socket, unless root explicitly gives permissions. For TCP > or similar sockets, that would not be the case. Not sure how big a deal > that would be. Windows also provides named pipes: 1. Pipes are subject to access control. 2. Pipes can work in message mode, like SOCK_SEQPACKET. 3. Pipes are addressed by path of form \\.\pipe\NAME that is not visible in the file system, like abstract Unix sockets, but this namespace is shared, so collision issue remains. 4. Pipes can be accessed as files from Python. Message mode can be activated with a Win32 API call that can be done using ctypes and msvcrt bundled modules. 5. Pipes can be configured to reject remote connections and to automatically limit the number of clients if needed. Another advantage is that message framing will not be needed this time, which limits the scope. Drawbacks: 1. There will be less shared code between Windows and Unix. Unix domain sockets and named pipes provide the same "accept client, send message, receive message, disconnect" API, so it's easy to share all non-transport logic. dpdk-telemetry.py will need a few Windows-specific lines. 2. On Windows, select() and WSAPoll() are only applicable to sockets. If we decide to multiplex IO in lib/telemetry and use named pipes, multiplexers will be different.
