| Message ID | 20250217172308.3291739-7-marcus.wichelmann@hetzner-cloud.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | XDP metadata support for tun driver | expand |
Marcus Wichelmann wrote: > The open_tuntap helper function uses open() to get a file descriptor for > /dev/net/tun. > > The open(2) manpage writes this about its return value: > > On success, open(), openat(), and creat() return the new file > descriptor (a nonnegative integer). On error, -1 is returned and > errno is set to indicate the error. > > This means that the fd > 0 assertion in the open_tuntap helper is > incorrect and should rather check for fd >= 0. > > When running the BPF selftests locally, this incorrect assertion was not > an issue, but the BPF kernel-patches CI failed because of this: > > open_tuntap:FAIL:open(/dev/net/tun) unexpected open(/dev/net/tun): > actual 0 <= expected 0 Wow. What kind of environment is this that 0 is not assigned stdin. > Signed-off-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de> The code makes sense. I suppose that if this condition can hit, then it can also affect existing lwt_* tests and thus should be a fix to commit 43a7c3ef8a15 ("selftests/bpf: Add lwt_xmit tests for BPF_REDIRECT"), sent separately to bpf (not bpf-next)? Since it's a test and no failure was reported so far, maybe fine to just merge as part of this bpf-next series, not my call. > --- > tools/testing/selftests/bpf/network_helpers.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c > index e1cfa1b37754..9b59bfd5d912 100644 > --- a/tools/testing/selftests/bpf/network_helpers.c > +++ b/tools/testing/selftests/bpf/network_helpers.c > @@ -571,7 +571,7 @@ int open_tuntap(const char *dev_name, bool need_mac) > struct ifreq ifr; > int fd = open("/dev/net/tun", O_RDWR); > > - if (!ASSERT_GT(fd, 0, "open(/dev/net/tun)")) > + if (!ASSERT_GE(fd, 0, "open(/dev/net/tun)")) > return -1; > > ifr.ifr_flags = IFF_NO_PI | (need_mac ? IFF_TAP : IFF_TUN); > -- > 2.43.0 >
Am 18.02.25 um 02:56 schrieb Willem de Bruijn: > Marcus Wichelmann wrote: >> [...] >> When running the BPF selftests locally, this incorrect assertion was not >> an issue, but the BPF kernel-patches CI failed because of this: >> >> open_tuntap:FAIL:open(/dev/net/tun) unexpected open(/dev/net/tun): >> actual 0 <= expected 0 > > Wow. What kind of environment is this that 0 is not assigned stdin. > >> Signed-off-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de> > > The code makes sense. > > I suppose that if this condition can hit, then it can also affect > existing lwt_* tests and thus should be a fix to commit 43a7c3ef8a15 > ("selftests/bpf: Add lwt_xmit tests for BPF_REDIRECT"), sent > separately to bpf (not bpf-next)? > > Since it's a test and no failure was reported so far, maybe fine > to just merge as part of this bpf-next series, not my call. I'm not sure why this only became an issue after I added the xdp_context_tuntap test and never before. This may have to do with the order of test execution. If nobody speaks up, I'll leave it in this patch series for now. Marcus
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c index e1cfa1b37754..9b59bfd5d912 100644 --- a/tools/testing/selftests/bpf/network_helpers.c +++ b/tools/testing/selftests/bpf/network_helpers.c @@ -571,7 +571,7 @@ int open_tuntap(const char *dev_name, bool need_mac) struct ifreq ifr; int fd = open("/dev/net/tun", O_RDWR); - if (!ASSERT_GT(fd, 0, "open(/dev/net/tun)")) + if (!ASSERT_GE(fd, 0, "open(/dev/net/tun)")) return -1; ifr.ifr_flags = IFF_NO_PI | (need_mac ? IFF_TAP : IFF_TUN);
The open_tuntap helper function uses open() to get a file descriptor for /dev/net/tun. The open(2) manpage writes this about its return value: On success, open(), openat(), and creat() return the new file descriptor (a nonnegative integer). On error, -1 is returned and errno is set to indicate the error. This means that the fd > 0 assertion in the open_tuntap helper is incorrect and should rather check for fd >= 0. When running the BPF selftests locally, this incorrect assertion was not an issue, but the BPF kernel-patches CI failed because of this: open_tuntap:FAIL:open(/dev/net/tun) unexpected open(/dev/net/tun): actual 0 <= expected 0 Signed-off-by: Marcus Wichelmann <marcus.wichelmann@hetzner-cloud.de> --- tools/testing/selftests/bpf/network_helpers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)