Skip to content

loader: Check if device has BPF prog before trying to detach it#13591

Merged
tklauser merged 1 commit intomasterfrom
pr/pchaigno/skip-tc-delete-if-no-filter
Oct 19, 2020
Merged

loader: Check if device has BPF prog before trying to detach it#13591
tklauser merged 1 commit intomasterfrom
pr/pchaigno/skip-tc-delete-if-no-filter

Conversation

@pchaigno
Copy link
Copy Markdown
Member

@pchaigno pchaigno commented Oct 15, 2020

When running Cilium with the devices set, but neither kube-proxy-free or the host firewall enabled, cilium-agent will attempt to remove the previous BPF program from the native devices' egress tc hook. If no program is attached there (i.e., Cilium was running without host firewall and kube-proxy replacement before restarting), the removal will fail with the following error.

level=error msg="Command execution failed" cmd="[tc filter delete dev enp0s8 egress]" error="exit status 2" subsys=datapath-loader
level=warning msg="Error: Parent Qdisc doesn't exists." subsys=datapath-loader
level=warning msg="We have an error talking to the kernel, -1" subsys=datapath-loader

This commit fixes this error by first checking that the device has a BPF program attached on egress.

I tested this in the dev. VM, by first starting Cilium with our kube-proxy-replacement, then without but keeping the devices configuration.

Fixes: #10994
Fixes: #13512
Updates: #11799

@pchaigno pchaigno added kind/bug This is a bug in the Cilium logic. area/loader Impacts the loading of BPF programs into the kernel. release-note/bug This PR fixes an issue in a previous release of Cilium. needs-backport/1.8 labels Oct 15, 2020
@pchaigno pchaigno requested a review from a team October 15, 2020 17:06
Copy link
Copy Markdown
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Two small nits, otherwise LGTM.

Comment thread pkg/datapath/loader/netlink.go Outdated
Comment thread pkg/datapath/loader/netlink.go Outdated
Copy link
Copy Markdown
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Two small nits, otherwise LGTM.

@pchaigno pchaigno force-pushed the pr/pchaigno/skip-tc-delete-if-no-filter branch from 9f76a69 to 55607ba Compare October 16, 2020 07:59
@pchaigno pchaigno requested a review from tklauser October 16, 2020 08:00
@tklauser
Copy link
Copy Markdown
Member

tklauser commented Oct 16, 2020

Comment thread pkg/datapath/loader/netlink.go Outdated
Comment thread pkg/datapath/loader/netlink.go Outdated
When running Cilium with the devices set, but neither kube-proxy-free or
the host firewall enabled, cilium-agent will attempt to remove the
previous BPF program from the native devices' egress tc hook. If no
program is attached there (i.e., Cilium was running without host
firewall and kube-proxy replacement before restarting), the removal will
fail with the following error.

    level=error msg="Command execution failed" cmd="[tc filter delete dev enp0s8 egress]" error="exit status 2" subsys=datapath-loader
    level=warning msg="Error: Parent Qdisc doesn't exists." subsys=datapath-loader
    level=warning msg="We have an error talking to the kernel, -1" subsys=datapath-loader

This commit fixes this error by first checking that the device has a BPF
program attached on egress.

I tested this in the dev. VM, by first starting Cilium with our
kube-proxy-replacement, then without but keeping the devices
configuration.

Fixes: a695f53 ("Endpoint for host")
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno force-pushed the pr/pchaigno/skip-tc-delete-if-no-filter branch from 55607ba to 6a1c3de Compare October 16, 2020 14:30
@pchaigno
Copy link
Copy Markdown
Member Author

test-me-please

@maintainer-s-little-helper maintainer-s-little-helper Bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 16, 2020
Comment thread pkg/datapath/loader/netlink.go
@tklauser tklauser merged commit 2e0539a into master Oct 19, 2020
@tklauser tklauser deleted the pr/pchaigno/skip-tc-delete-if-no-filter branch October 19, 2020 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/loader Impacts the loading of BPF programs into the kernel. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clear error messages that should be marked as default

8 participants