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@xxxxxxxxxxxxxxxx>
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
>
