Skip to content

v2: ebpf: replace deprecated prog.Attach/prog.Detach and fix closer#192

Merged
estesp merged 1 commit into
containerd:masterfrom
thaJeztah:ebpf_deprecated
Apr 14, 2021
Merged

v2: ebpf: replace deprecated prog.Attach/prog.Detach and fix closer#192
estesp merged 1 commit into
containerd:masterfrom
thaJeztah:ebpf_deprecated

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

similar to opencontainers/runc#2903

this was added in 33207b4 (#116), but had the same issue as the code in runc.

Caught by golangci-lint when enabling golint:

v2/ebpf.go:45:12: SA1019: prog.Attach is deprecated: use link.RawAttachProgram instead. (staticcheck)
    if err := prog.Attach(dirFD, ebpf.AttachCGroupDevice, unix.BPF_F_ALLOW_MULTI); err != nil {
              ^
v2/ebpf.go:49:13: SA1019: prog.Detach is deprecated: use link.RawDetachProgram instead. (staticcheck)
    if err := prog.Detach(dirFD, ebpf.AttachCGroupDevice, unix.BPF_F_ALLOW_MULTI); err != nil {
              ^

worth noting that we currently call prog.Detach() with unix.BPF_F_ALLOW_MULTI;
https://github.com/golang/sys/blob/22da62e12c0cd9c1da93581e1113ca4d82a5be14/unix/zerrors_linux.go#L178

BPF_F_ALLOW_MULTI = 0x2

Looking at the source code for prog.Detach(); https://github.com/cilium/ebpf/blob/v0.4.0/prog.go#L579-L581,
this would always produce an error:

if flags != 0 {
    return errors.New("flags must be zero")
}

Note that the flags parameter is not used (except for that validation)

Signed-off-by: Sebastiaan van Stijn github@gone.nl

Caught by golangci-lint when enabling golint:

    v2/ebpf.go:45:12: SA1019: prog.Attach is deprecated: use link.RawAttachProgram instead. (staticcheck)
        if err := prog.Attach(dirFD, ebpf.AttachCGroupDevice, unix.BPF_F_ALLOW_MULTI); err != nil {
                  ^
    v2/ebpf.go:49:13: SA1019: prog.Detach is deprecated: use link.RawDetachProgram instead. (staticcheck)
        if err := prog.Detach(dirFD, ebpf.AttachCGroupDevice, unix.BPF_F_ALLOW_MULTI); err != nil {
                  ^

worth noting that we currently call prog.Detach() with unix.BPF_F_ALLOW_MULTI;
https://github.com/golang/sys/blob/22da62e12c0cd9c1da93581e1113ca4d82a5be14/unix/zerrors_linux.go#L178

    BPF_F_ALLOW_MULTI = 0x2

Looking at the source code for prog.Detach(); https://github.com/cilium/ebpf/blob/v0.4.0/prog.go#L579-L581,
this would _always_ produce an error:

    if flags != 0 {
        return errors.New("flags must be zero")
    }

Note that the flags parameter is not used (except for that validation)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Copy Markdown
Member Author

@AkihiroSuda @Zyqsempai @estesp ptal

Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@estesp estesp merged commit 21be173 into containerd:master Apr 14, 2021
@thaJeztah thaJeztah deleted the ebpf_deprecated branch April 14, 2021 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants