Hello, Joan Lledó, le ven. 10 oct. 2025 23:26:42 +0200, a ecrit: > On 10/10/25 23:10, [email protected] wrote: > > Here are the changes for libpcap. Basically this implements the > > `selectable_fd` pcap feature for the Hurd. This is needed because > > dhcpcd needs a fd to insert in an event loop. The event loop keeps > > waiting on poll(), so we need to provide it with a descriptor able > > to be sent to poll(). When the fd is signaled, dhcpcd calls the read > > function to get the data from a BPF filter. > > > > In this patch, my approach is to make the data pass through a pipe, > > which read end is going to be the selectable_fd for dhcpcd. When capture > > starts, a new thread reads packets from gnumach bpf and pushes them to > > the pipe. When the pipe has data, dhcpcd is signaled and it will call > > the read function, which I modified to read from the pipe and return data > > to dhcpcd. This is working fine in my local tests, but I'd appreciate a > > review.
That doesn't look like an efficient way :) But for gnumach devices, we don't really have another way to get it through poll(), so it's actually the most efficient way (more than using some separate translator on /dev/eth0). Perhaps > From f8b5cd5401da28d4ddd7f6ba08d6e20f800353bf Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Joan=20Lled=C3=B3?= <[email protected]> > Date: Sun, 5 Oct 2025 13:34:34 +0200 > Subject: [PATCH] Hurd: implement `selectable_fd`. > > --- > pcap-hurd.c | 145 ++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 130 insertions(+), 15 deletions(-) > > diff --git a/pcap-hurd.c b/pcap-hurd.c > index 134a109b..08a7773f 100644 > --- a/pcap-hurd.c > +++ b/pcap-hurd.c > @@ -7,16 +7,18 @@ > > #include <fcntl.h> > #include <hurd.h> > -#include <mach.h> > #include <time.h> > #include <errno.h> > +#include <pthread.h> > #include <stdio.h> > #include <stddef.h> > #include <stdlib.h> > #include <string.h> > +#include <unistd.h> > #include <device/device.h> > #include <device/device_types.h> > #include <device/net_status.h> > +#include <hurd/ports.h> > #include <net/if_ether.h> > > #include "pcap-int.h" > @@ -26,6 +28,9 @@ struct pcap_hurd { > device_t mach_dev; > mach_port_t rcv_port; > int filtering_in_kernel; > + pthread_t pipe_thread_id; > + int pipefd[2]; > + volatile int thread_should_stop; thread_should_stop doesn't seem used? Usually you want to avoid volatile variables anyway, they're so easy to get completely wrong. > }; > > /* Accept all packets. */ > @@ -126,11 +131,11 @@ pcap_read_hurd(pcap_t *p, int cnt _U_, pcap_handler > callback, u_char *user) > struct pcap_hurd *ph; > struct pcap_pkthdr h; > struct timespec ts; > - int wirelen, caplen; > + int wirelen, caplen, rpipe, ret; > u_char *pkt; > - kern_return_t kr; > > ph = p->priv; > + rpipe = ph->pipefd[0]; > msg = (struct net_rcv_msg *)p->buffer; > > retry: > @@ -139,19 +144,20 @@ retry: > return PCAP_ERROR_BREAK; > } > > - kr = mach_msg(&msg->msg_hdr, MACH_RCV_MSG | MACH_RCV_INTERRUPT, 0, > - p->bufsize, ph->rcv_port, MACH_MSG_TIMEOUT_NONE, > - MACH_PORT_NULL); > - clock_gettime(CLOCK_REALTIME, &ts); > - > - if (kr) { > - if (kr == MACH_RCV_INTERRUPTED) > - goto retry; > - > - pcapint_fmt_errmsg_for_kern_return_t(p->errbuf, > PCAP_ERRBUF_SIZE, kr, > - "mach_msg"); > + ret = read(rpipe, &msg->msg_hdr, p->bufsize); > + if (ret < 0) { > + /* Only write to errbuf if this is an error, > + * not during termination */ > + if (errno != EBADF) { Does EBADF happen? That's worrying, we shouldn't be getting it if we properly manage fds. > + pcapint_fmt_errmsg_for_kern_return_t(p->errbuf, > + PCAP_ERRBUF_SIZE, errno, "read"); > + } > return PCAP_ERROR; > } > + if (ret == 0) > + /* Pipe closed, 0 packets read */ > + return 0; > + clock_gettime(CLOCK_REALTIME, &ts); > > ph->stat.ps_recv++; > > @@ -212,7 +218,7 @@ pcap_inject_hurd(pcap_t *p, const void *buf, int size) > if (kr) { > pcapint_fmt_errmsg_for_kern_return_t(p->errbuf, > PCAP_ERRBUF_SIZE, kr, > "device_write"); > - return -1; > + return PCAP_ERROR; > } > > return count; > @@ -232,9 +238,34 @@ static void > pcap_cleanup_hurd(pcap_t *p) > { > struct pcap_hurd *ph; > + int err; > > ph = p->priv; > > + /* Cancel the thread */ > + if (ph->pipe_thread_id != 0) { > + pthread_cancel(ph->pipe_thread_id); > + > + err = pthread_join(ph->pipe_thread_id, NULL); > + if (err != 0) { > + pcapint_fmt_errmsg_for_errno(p->errbuf, > + PCAP_ERRBUF_SIZE, err, "pthread_join"); > + } > + ph->pipe_thread_id = 0; > + } > + > + /* Close the pipe ends */ > + if (ph->pipefd[1] != 0) { > + close(ph->pipefd[1]); > + ph->pipefd[1] = 0; Better make it -1, since 0 is a valid fd. > + } > + > + if (ph->pipefd[0] != 0) { > + close(ph->pipefd[0]); > + ph->pipefd[0] = 0; > + } > + > + /* Release remaining resources */ > if (ph->rcv_port != MACH_PORT_NULL) { > mach_port_deallocate(mach_task_self(), ph->rcv_port); > ph->rcv_port = MACH_PORT_NULL; > @@ -242,12 +273,86 @@ pcap_cleanup_hurd(pcap_t *p) > > if (ph->mach_dev != MACH_PORT_NULL) { > device_close(ph->mach_dev); > + mach_port_deallocate(mach_task_self(), ph->mach_dev); > ph->mach_dev = MACH_PORT_NULL; > } > > pcapint_cleanup_live_common(p); > } > > +static void* > +pipe_write_thread(void *arg) { > + pcap_t *p; > + struct pcap_hurd *ph; > + int wpipe, ret; > + struct net_rcv_msg msg; > + u_int msgsize; > + kern_return_t kr; > + mach_msg_timeout_t timeout_ms; > + sigset_t set; > + > + pthread_setname_np (pthread_self(), "pcap_hurd_pipe_thread"); > + > + /* Block SIGPIPE for this thread */ > + sigemptyset(&set); > + sigaddset(&set, SIGPIPE); > + pthread_sigmask(SIG_BLOCK, &set, NULL); > + > + p = (pcap_t *)arg; > + ph = p->priv; > + wpipe = ph->pipefd[1]; > + msgsize = sizeof(struct net_rcv_msg); > + timeout_ms = 100; > + > + while (1) { > + kr = mach_msg(&msg.msg_hdr, > + MACH_RCV_MSG | MACH_RCV_INTERRUPT| MACH_RCV_TIMEOUT, > + 0, msgsize, ph->rcv_port, timeout_ms, > + MACH_PORT_NULL); > + > + if (kr) { > + if (kr == MACH_RCV_TIMED_OUT || kr == > MACH_RCV_INTERRUPTED) { > + pthread_testcancel(); Yes, that's a good way to get cancelling working fine easily. > + continue; > + } > + > + pcapint_fmt_errmsg_for_kern_return_t(p->errbuf, > + PCAP_ERRBUF_SIZE, kr, "mach_msg"); > + > + return NULL; > + } > + > + ret = write(wpipe, &msg, msgsize); > + if (ret < 0) { > + /* Only write to errbuf if this is an error, not during > termination */ > + if (errno != EPIPE && errno != EBADF) { Similarly here, we shouldn't be getting EBADF. > + pcapint_fmt_errmsg_for_errno(p->errbuf, > + PCAP_ERRBUF_SIZE, errno, "write"); > + } > + return NULL; > + } > + } > + > + return NULL; > +} > + > +static int > +init_pipe(pcap_t *p) { > + int err; > + struct pcap_hurd *ph; > + > + ph = p->priv; > + err = pipe(ph->pipefd); > + if (err < 0) > + return errno; > + > + err = pthread_create(&ph->pipe_thread_id, NULL, pipe_write_thread, p); > + if (err != 0) > + return err; > + > + return 0; > +} > + > static int > pcap_activate_hurd(pcap_t *p) > { > @@ -310,6 +415,15 @@ pcap_activate_hurd(pcap_t *p) > goto error; > } > > + ret = init_pipe(p); > + if (ret != 0) { > + pcapint_fmt_errmsg_for_errno(p->errbuf, PCAP_ERRBUF_SIZE, > + ret, "init_pipe"); > + goto error; > + } > + > + p->selectable_fd = ph->pipefd[0]; > + > /* > * XXX Ethernet only currently > * > @@ -332,6 +446,7 @@ pcap_activate_hurd(pcap_t *p) > p->inject_op = pcap_inject_hurd; > p->setfilter_op = pcap_setfilter_hurd; > p->stats_op = pcap_stats_hurd; > + p->cleanup_op = pcap_cleanup_hurd; > > return 0; > > -- > 2.50.1 > -- Samuel <T> csp.tar.gz: ascii text -+- #ens-mim - vive les browsers qui prennent des initiatives à la con -+-
