On Fri, Jan 19, 2024 at 1:34 PM Song Liu <song@xxxxxxxxxx> wrote: > > On Fri, Jan 19, 2024 at 1:30 PM Andrii Nakryiko > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > On Fri, Jan 19, 2024 at 1:21 PM Andrii Nakryiko > > <andrii.nakryiko@xxxxxxxxx> wrote: > > > > > > On Fri, Jan 19, 2024 at 1:18 PM Song Liu <song@xxxxxxxxxx> wrote: > > > > > > > > On Fri, Jan 19, 2024 at 1:02 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > > > > > > > > > We've ran into issues with using dup2() API in production setting, where > > > > > libbpf is linked into large production environment and ends up calling > > > > > uninteded custom implementations of dup2(). These custom implementations > > > > > > > > typo: unintended > > > > > > oops, but probably doesn't warrant respinning > > > > > > > > > > > > don't provide atomic FD replacement guarantees of dup2() syscall, > > > > > leading to subtle and hard to debug issues. > > > > > > > > > > To prevent this in the future and guarantee that no libc implementation > > > > > will do their own custom non-atomic dup2() implementation, call dup2() > > > > > syscall directly with syscall(SYS_dup2). > > > > > > > > > > Note that some architectures don't seem to provide dup2 and have dup3 > > > > > instead. Try to detect and pick best syscall. > > > > > > > > I wonder whether we can just always use dup3(). > > > > > > dup3() (according to my git foo) was added in 4.17, which is more > > > modern than some other usable BPF, so I don't want to just randomly > > > bump the minimal supported (by libbpf) kernel for something like this. > > > > > I believe dup3() was added in 3.7. True, my git-foo isn't careful enough, 4.17 is when dup3 kernel refactoring happened. bpf() syscall was added in 3.17, right? In that case, yep, I could have just gone with __NR_dup3 directly, I suppose, but this version should work well anyways, so I wouldn't bother changing it. > > > > > Btw, this #ifdef check is the same as what glibc does for its > > implementation of dup2() (except for fd equality check which isn't > > necessary for libbpf), see [0] > > > > [0] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/dup2.c > > Yep, this looks good. > > Thanks, > Song
