Re: [tcpdump-workers] What's the correct new API to request pcap_linux to not open an eventfd
--- Begin Message --- Hi Denis, Thanks for pointing out the manpage update. I had old man pages (my work is being done in the context of the 1.10 release). What confused me is the asymmetry of the API. If you call pcap_setnonblock() on an un-activated socket, it sets a flag and doesn't return an error. But pcap_getnonblock() returns an error, so you can not check the value on an un-activated socket. This is not a flaw, necessarily, just confusing. Now that I understand this flow (I did not realize there were two different implementations of pcap_setnonblock(), because I was focused on pcap-linux.c and not on the important stuff in pcap.c) I think it's straightforward to defer the opening of the eventfd to pcap_activate(), so that we can avoid opening the eventfd altogether in nonblock mode. I'll see if I can update my changes accordingly. Thank you for pointing out the extra detail that helped me to understand. Bill --- End Message --- ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] What's the correct new API to request pcap_linux to not open an eventfd
--- Begin Message --- On Fri, 1 Jul 2022 13:55:30 -0400 Bill Fenner via tcpdump-workers wrote: > If we set > pcap_nonblock after pcap_create and before pcap_activate, we get -3 - > which I don't get at all, unless, -3 means "you didn't activate the > pcap yet". My naive reading of the Linux pcap_getnonblock code says > it'll return the integer value of a bool, and I don't know how that > can be -3. Hello Bill. The pcap_setnonblock(3PCAP) man page describes two functions: int pcap_setnonblock(pcap_t *p, int nonblock, char *errbuf); int pcap_getnonblock(pcap_t *p, char *errbuf); If your comments concern pcap_getnonblock(), what you describe is consistent with the documented behaviour: In pcap/pcap.h: #define PCAP_ERROR_NOT_ACTIVATED-3 /* the capture needs to be activated */ In the man page: RETURN VALUE pcap_getnonblock() returns the current ``non-blocking'' state of the capture descriptor; it always returns 0 on ``savefiles''. If called on a capture handle that has been created but not activated, PCAP_ERROR_NOT_ACTIVATED is returned. If there is another error, PCAP_ERROR is re‐ turned and errbuf is filled in with an appropriate error message. Did you mean there is an issue with pcap_setnonblock()? -- Denis Ovsienko --- End Message --- ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] What's the correct new API to request pcap_linux to not open an eventfd
--- Begin Message --- On Fri, May 20, 2022 at 6:10 PM Bill Fenner wrote: > On Fri, May 20, 2022 at 12:36 PM Guy Harris wrote: > >> If it's putting them in non-blocking mode, and using some >> select/poll/epoll/etc. mechanism in a single event loop, then the right >> name for the API is pcap_setnonblock(). There's no need for an eventfd to >> wake up the blocking poll() if there *is* no blocking poll(), so: >> >> if non-blocking mode is on before pcap_activate() is called, no >> eventfd should be opened, and poll_breakloop_fd should be set to -1; >> >> if non-blocking mode is turned on after pcap_activate() is >> called, the eventfd should be closed, and poll_breakloop_fd should be set >> to -1; >> >> if non-blocking mode is turned *off* afterwards, an eventfd >> should be opened, and poll_breakloop_fd should be set to it; >> >> if poll_breakloop_fd is -1, the poll() should only wait on the >> socket FD; >> >> so this can be handled without API changes. >> > > Thank you for the excellent observation, Guy. It is indeed setting > non-block before pcap_activate(). I'll work on this plan. > A slight variation of this plan is at https://github.com/the-tcpdump-group/libpcap/pull/1113 I wrote a test program that doesn't do much, but does demonstrate that the blocking-ness API on Linux is at least a little weird. If we set pcap_nonblock after pcap_create and before pcap_activate, we get -3 - which I don't get at all, unless, -3 means "you didn't activate the pcap yet". My naive reading of the Linux pcap_getnonblock code says it'll return the integer value of a bool, and I don't know how that can be -3. The sequence ends up being: pcap_create() -> open eventfd pcap_setnonblock() -> close eventfd pcap_activate() I didn't want to move the eventfd creation out of pcap_create without having a deeper understanding of the strangeness around the nonblock API. I ran 15,645 internal Arista tests using these changes, in an infrastructure that relies on nonblocking pcap for packet exchange, and they all passed. Obviously this doesn't really say much about its general applicability, and kind of is a no-brainer "when you don't need the eventfd you don't notice if it's not there", but at least it says that things aren't drastically broken. Bill --- End Message --- ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] What's the correct new API to request pcap_linux to not open an eventfd
--- Begin Message --- On Fri, May 20, 2022 at 6:10 PM Bill Fenner wrote: > On Fri, May 20, 2022 at 12:36 PM Guy Harris wrote: > >> If it's putting them in non-blocking mode, and using some >> select/poll/epoll/etc. mechanism in a single event loop, then the right >> name for the API is pcap_setnonblock(). There's no need for an eventfd to >> wake up the blocking poll() if there *is* no blocking poll(), so: >> >> if non-blocking mode is on before pcap_activate() is called, no >> eventfd should be opened, and poll_breakloop_fd should be set to -1; >> >> if non-blocking mode is turned on after pcap_activate() is >> called, the eventfd should be closed, and poll_breakloop_fd should be set >> to -1; >> >> if non-blocking mode is turned *off* afterwards, an eventfd >> should be opened, and poll_breakloop_fd should be set to it; >> >> if poll_breakloop_fd is -1, the poll() should only wait on the >> socket FD; >> >> so this can be handled without API changes. >> > > Thank you for the excellent observation, Guy. It is indeed setting > non-block before pcap_activate(). I'll work on this plan. > Actually, I confused myself. It turns out that pcap_linux is buggy when you set non-block before pcap_activate() -- it uses the handlep->timeout value to remember whether or not non-block was set, but, pcap_activate() unconditionally overwrites handlep->timeout. So it just comes down to, for now, we always open the eventfd and then can close it when non-blocking mode is turned on. This just means the first item on your list is not done, but the last 3 are enough. (I think that the right fix for setting nonblock before activate could be to add a bool to the handlep, to separate the nonblock status from the timeout, but that can be a separate fix.) Bill --- End Message --- ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] What's the correct new API to request pcap_linux to not open an eventfd
--- Begin Message --- On Fri, May 20, 2022 at 12:36 PM Guy Harris wrote: > If it's putting them in non-blocking mode, and using some > select/poll/epoll/etc. mechanism in a single event loop, then the right > name for the API is pcap_setnonblock(). There's no need for an eventfd to > wake up the blocking poll() if there *is* no blocking poll(), so: > > if non-blocking mode is on before pcap_activate() is called, no > eventfd should be opened, and poll_breakloop_fd should be set to -1; > > if non-blocking mode is turned on after pcap_activate() is called, > the eventfd should be closed, and poll_breakloop_fd should be set to -1; > > if non-blocking mode is turned *off* afterwards, an eventfd should > be opened, and poll_breakloop_fd should be set to it; > > if poll_breakloop_fd is -1, the poll() should only wait on the > socket FD; > > so this can be handled without API changes. > Thank you for the excellent observation, Guy. It is indeed setting non-block before pcap_activate(). I'll work on this plan. Bill --- End Message --- ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
Re: [tcpdump-workers] What's the correct new API to request pcap_linux to not open an eventfd
--- Begin Message --- On May 20, 2022, at 10:56 AM, Bill Fenner via tcpdump-workers wrote: > I'm helping to debug a system that uses many many pcap handles, and never > calls pcap_loop - only ever pcap_next. Both pcap_loop() and pcap_next() ultimately go to the same place. Note, BTW, that pcap_next() sucks; it's impossible to know whether it returns NULL because of an error or because the timeout expired and no packets had arrived during that time. pcap_next_ex() doesn't have that problem. (On Linux, the turbopacket timer doesn't expire if no packets have arrived, so, *on Linux*, NULL should, as far as I know, be returned only on errors.) > We've found that each pcap handle has an associated eventfd, which is used to > make sure to wake up when > pcap_breakloop() is called. Since this code doesn't call pcap_loop or > pcap_breakloop, the eventfd is unneeded. If it called pcap_breakloop(), the eventfd would still be needed; otherwise, a call could remain indefinitely stuck in pcap_next() until a packet finally arrives. Only the lack of a pcap_breakloop() call renders the eventfd unnecessary. So how is this system handling those pcap handles? If it's putting them in non-blocking mode, and using some select/poll/epoll/etc. mechanism in a single event loop, then the right name for the API is pcap_setnonblock(). There's no need for an eventfd to wake up the blocking poll() if there *is* no blocking poll(), so: if non-blocking mode is on before pcap_activate() is called, no eventfd should be opened, and poll_breakloop_fd should be set to -1; if non-blocking mode is turned on after pcap_activate() is called, the eventfd should be closed, and poll_breakloop_fd should be set to -1; if non-blocking mode is turned *off* afterwards, an eventfd should be opened, and poll_breakloop_fd should be set to it; if poll_breakloop_fd is -1, the poll() should only wait on the socket FD; so this can be handled without API changes. If it's doing something else, e.g. using multiple threads, then: > I'm willing to write and test the code that skips creating the breakloop_fd > - but, I wanted to discuss what the API should be. Should there be a > pcap.c "pcap_breakloop_not_needed( pcap_t * )" that dispatches to the > implementation, or should there be a linux-specific > "pcap_linux_dont_create_eventfd( pcap_t * )"? ...it should be called pcap_breakloop_not_needed() (or something such as that), with a per-type implementation, and a *default* implementation that does nothing, so only implementations that need to do something different would need to do so. --- End Message --- ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
[tcpdump-workers] What's the correct new API to request pcap_linux to not open an eventfd
--- Begin Message --- I'm helping to debug a system that uses many many pcap handles, and never calls pcap_loop - only ever pcap_next. We've found that each pcap handle has an associated eventfd, which is used to make sure to wake up when pcap_breakloop() is called. Since this code doesn't call pcap_loop or pcap_breakloop, the eventfd is unneeded. I'm willing to write and test the code that skips creating the breakloop_fd - but, I wanted to discuss what the API should be. Should there be a pcap.c "pcap_breakloop_not_needed( pcap_t * )" that dispatches to the implementation, or should there be a linux-specific "pcap_linux_dont_create_eventfd( pcap_t * )"? For this use case, portability is not important, so I would be fine with either. I'd also be fine with less ridiculous name suggestions :-) Thanks, Bill --- End Message --- ___ tcpdump-workers mailing list tcpdump-workers@lists.tcpdump.org https://lists.sandelman.ca/mailman/listinfo/tcpdump-workers
