Metric to track conntrack state invalid packets dropped by iptables #122812

aroradaman · 2024-01-16T17:26:11Z

Metric to track number of packets dropped by iptables which were marked INVALID by conntrack.

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Subtask of #122572

Does this PR introduce a user-facing change?

The iptables mode of kube-proxy now tracks packets that are wrongfully marked invalid by conntrack and subsequently dropped by introducing `kubeproxy_iptables_ct_state_invalid_dropped_packets_total` metric

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2024-01-16T17:26:14Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

aroradaman · 2024-01-16T17:26:33Z

/sig network
/cc. @danwinship

aroradaman · 2024-01-16T17:27:01Z

/test pull-kubernetes-verify
/test pull-kubernetes-unit
/test pull-kubernetes-e2e-kind

aroradaman · 2024-01-16T17:27:27Z

Tested locally on kind

root@kind-worker:/# curl --silent 127.0.0.1:10249/metrics | grep kubeproxy_iptables_ct_invalid_dropped_packets
# HELP kubeproxy_iptables_ct_invalid_dropped_packets [ALPHA] Cumulative invalid packets dropped by iptables
# TYPE kubeproxy_iptables_ct_invalid_dropped_packets counter
kubeproxy_iptables_ct_invalid_dropped_packets 0

root@kind-worker:/# curl --silent 127.0.0.1:10249/metrics | grep kubeproxy_iptables_ct_invalid_dropped_packets
# HELP kubeproxy_iptables_ct_invalid_dropped_packets [ALPHA] Cumulative invalid packets dropped by iptables
# TYPE kubeproxy_iptables_ct_invalid_dropped_packets counter
kubeproxy_iptables_ct_invalid_dropped_packets 240

root@kind-worker:/# curl --silent 127.0.0.1:10249/metrics | grep kubeproxy_iptables_ct_invalid_dropped_packets
# HELP kubeproxy_iptables_ct_invalid_dropped_packets [ALPHA] Cumulative invalid packets dropped by iptables
# TYPE kubeproxy_iptables_ct_invalid_dropped_packets counter
kubeproxy_iptables_ct_invalid_dropped_packets 614

root@kind-worker:/# curl --silent 127.0.0.1:10249/metrics | grep kubeproxy_iptables_ct_invalid_dropped_packets
# HELP kubeproxy_iptables_ct_invalid_dropped_packets [ALPHA] Cumulative invalid packets dropped by iptables
# TYPE kubeproxy_iptables_ct_invalid_dropped_packets counter
kubeproxy_iptables_ct_invalid_dropped_packets 967

aojea · 2024-01-16T22:15:14Z

love this

danwinship

nice

pkg/proxy/iptables/proxier.go

pkg/util/iptables/save_restore.go

pkg/util/iptables/testing/fake.go

aroradaman · 2024-01-17T17:19:24Z

Tried preserving the counters.

Added the drop rule in FORWARD instead of KUBE-FORWARD to prevent counter resets. Now the rule is ensured just like the base-chain jump rules.

pkg/proxy/iptables/number_generated_rules_test.go

aojea · 2024-04-23T15:10:04Z

/kind feature

@aroradaman please add a release note

aojea · 2024-04-23T15:15:45Z

pkg/proxy/util/nfacct/nfacct_linux.go

+	// NFNL_MSG_ACCT_NEW
+	cmdNew = 0
+	// NFNL_MSG_ACCT_GET
+	cmdGet = 1


I can see 3 commands here https://nftables.org/projects/libnetfilter_acct/doxygen/html/group__nlmsg.html , are this related?

Yes. We don't need the third one NFNL_MSG_ACCT_GET_CTRZERO, it does get + atomic reset to zero.

aojea · 2024-04-23T15:23:18Z

pkg/proxy/metrics/metrics.go

@@ -264,3 +275,40 @@ func RegisterMetrics() {
 func SinceInSeconds(start time.Time) float64 {
 	return time.Since(start).Seconds()
 }
+
+var _ metrics.StableCollector = &ctStateInvalidPacketsCollector{}


I´m not familiar with this stuff, @dgrisonnet do you mind to take a look to the metrics part?

oh it seems was already reviewed by instrumentation #122812 (comment)

This also removed the metric sync dependency on proxier sync loop 😃

aojea · 2024-04-23T15:26:24Z

@aroradaman you have to rebase, I don't understand the minor details but it seems they were thoroughly reviewed, the INVALID DROP rules for conntrack now also uses the accounting system of iptables to store the value with the number of packets that match and report them via metrics. To do this a new low level netlink library was added

LGTM

/assign @danwinship @thockin

pkg/proxy/metrics/metrics.go

thockin · 2024-04-23T20:57:22Z

/approve

Approve for go.mod

k8s-ci-robot · 2024-04-23T20:57:29Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aroradaman, thockin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [thockin]
~~pkg/proxy/OWNERS~~ [thockin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

danwinship · 2024-04-25T12:41:56Z

lgtm but needs rebase (yay post-unfreeze mergefest)

also needs a release note... doesn't need to be super-detailed since there will eventually be a larger nftables release note and upgrading-from-iptables documentation, but at least the general idea (new metric to help iptables kube-proxy users know if they are depending on iptables-kube-proxy-specific features)

as discussed here there's no e2e test because this particular rule doesn't get hit in all cluster types, but we have plans for other nfacct-based metrics and will add an e2e for one of them.

aroradaman · 2024-04-25T16:06:21Z

rebased, release-note added and addressed #122812 (comment).

nfacct is netfilter's accounting subsystem. This utility allows interactions with the subsystem using lower level netlink API. Signed-off-by: Daman Arora <aroradaman@gmail.com>

Track packets dropped by proxy which were marked invalid by conntrack using nfacct netfilter extended accounting infrastructure. Signed-off-by: Daman Arora <aroradaman@gmail.com>

danwinship · 2024-04-27T12:21:58Z

/lgtm

k8s-ci-robot · 2024-04-27T12:22:04Z

LGTM label has been added.

Git tree hash: 769d1550d33e4411eafefeaca3bafac35a3e25d2

k8s-ci-robot added sig/network Categorizes an issue or PR as relevant to SIG Network. area/ipvs area/kube-proxy and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 16, 2024

k8s-ci-robot requested review from andrewsykim and danwinship January 16, 2024 17:27

aroradaman force-pushed the ct-invalid-drop-metric branch from 63e4154 to 4bbeeaf Compare January 16, 2024 18:16

danwinship reviewed Jan 16, 2024

View reviewed changes

danwinship mentioned this pull request Jan 17, 2024

nftables kube-proxy TODO #122572

Open

27 tasks

aroradaman force-pushed the ct-invalid-drop-metric branch from 4bbeeaf to 1a393ad Compare January 17, 2024 17:17

aroradaman marked this pull request as ready for review January 17, 2024 17:25

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 17, 2024

k8s-ci-robot requested review from bowei and MrHohn January 17, 2024 17:25

aroradaman commented Jan 17, 2024

View reviewed changes

pkg/proxy/iptables/number_generated_rules_test.go Outdated Show resolved Hide resolved

aroradaman marked this pull request as draft January 22, 2024 21:11

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. labels Apr 23, 2024

aojea reviewed Apr 23, 2024

View reviewed changes

k8s-ci-robot assigned danwinship and thockin Apr 23, 2024

aroradaman force-pushed the ct-invalid-drop-metric branch from f515d84 to 823983c Compare April 23, 2024 16:37

dgrisonnet reviewed Apr 23, 2024

View reviewed changes

pkg/proxy/metrics/metrics.go Outdated Show resolved Hide resolved

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 23, 2024

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 25, 2024

aroradaman force-pushed the ct-invalid-drop-metric branch from 823983c to aa95396 Compare April 25, 2024 15:56

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Apr 25, 2024

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 26, 2024

aroradaman added 2 commits April 27, 2024 12:17

pkg/proxy/util/nfacct: utility to interact with nfacct subsystem

6b52916

nfacct is netfilter's accounting subsystem. This utility allows interactions with the subsystem using lower level netlink API. Signed-off-by: Daman Arora <aroradaman@gmail.com>

pkg/proxy/iptables: track ct state invalid dropped packets

3363ec4

Track packets dropped by proxy which were marked invalid by conntrack using nfacct netfilter extended accounting infrastructure. Signed-off-by: Daman Arora <aroradaman@gmail.com>

aroradaman force-pushed the ct-invalid-drop-metric branch from aa95396 to 3363ec4 Compare April 27, 2024 07:09

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 27, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 27, 2024

k8s-ci-robot merged commit de9a932 into kubernetes:master Apr 27, 2024
16 checks passed

k8s-ci-robot added this to the v1.31 milestone Apr 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metric to track conntrack state invalid packets dropped by iptables #122812

Metric to track conntrack state invalid packets dropped by iptables #122812

aroradaman commented Jan 16, 2024 •

edited

k8s-ci-robot commented Jan 16, 2024

aroradaman commented Jan 16, 2024

aroradaman commented Jan 16, 2024

aroradaman commented Jan 16, 2024

aojea commented Jan 16, 2024

danwinship left a comment

aroradaman commented Jan 17, 2024

aojea commented Apr 23, 2024

aojea Apr 23, 2024

aroradaman Apr 23, 2024 •

edited

aojea Apr 23, 2024

aojea Apr 23, 2024

aroradaman Apr 23, 2024

aroradaman Apr 23, 2024

aojea commented Apr 23, 2024 •

edited

thockin commented Apr 23, 2024

k8s-ci-robot commented Apr 23, 2024

danwinship commented Apr 25, 2024

aroradaman commented Apr 25, 2024 •

edited

danwinship commented Apr 27, 2024

k8s-ci-robot commented Apr 27, 2024

Metric to track conntrack state invalid packets dropped by iptables #122812

Metric to track conntrack state invalid packets dropped by iptables #122812

Conversation

aroradaman commented Jan 16, 2024 • edited

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Jan 16, 2024

aroradaman commented Jan 16, 2024

aroradaman commented Jan 16, 2024

aroradaman commented Jan 16, 2024

aojea commented Jan 16, 2024

danwinship left a comment

Choose a reason for hiding this comment

aroradaman commented Jan 17, 2024

aojea commented Apr 23, 2024

aojea Apr 23, 2024

Choose a reason for hiding this comment

aroradaman Apr 23, 2024 • edited

Choose a reason for hiding this comment

aojea Apr 23, 2024

Choose a reason for hiding this comment

aojea Apr 23, 2024

Choose a reason for hiding this comment

aroradaman Apr 23, 2024

Choose a reason for hiding this comment

aroradaman Apr 23, 2024

Choose a reason for hiding this comment

aojea commented Apr 23, 2024 • edited

thockin commented Apr 23, 2024

k8s-ci-robot commented Apr 23, 2024

danwinship commented Apr 25, 2024

aroradaman commented Apr 25, 2024 • edited

danwinship commented Apr 27, 2024

k8s-ci-robot commented Apr 27, 2024

aroradaman commented Jan 16, 2024 •

edited

aroradaman Apr 23, 2024 •

edited

aojea commented Apr 23, 2024 •

edited

aroradaman commented Apr 25, 2024 •

edited