Repository: kubernetes
Issue: nftables kube-proxy TODO
Labels: priority/important-soon,sig/network,triage/accepted

#### For 1.30:
- [x] decide on required kernel/`nft` versions (@danwinship, #124152)
  - discussion in the first half of #122296, plus some comments in this issue, and then #122743
- [ ] add some useful metrics (@aojea / @npinaeva ?)
  - we don't know exactly what these will be: [discussion in the KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#monitoring-requirements)
  - [x] clean up existing metrics stuff a bit at the same time? (@danwinship, #124557)
- [x] add performance job (@aojea https://github.com/kubernetes/test-infra/pull/32431)
  - Discussion on the KEP PR: https://github.com/kubernetes/enhancements/pull/3824#discussion_r1107584724, https://github.com/kubernetes/enhancements/pull/3824#discussion_r1328756780
- [x] change `--nodeport-addresses` behavior to default to "primary node IP(s) only" rather than "all node IPs". (@nayihz, https://github.com/kubernetes/kubernetes/pull/122724)
  - [discussion in the KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#the-iptables-mode-of-kube-proxy-has-grown-crufty)
  - Discussion in the KEP PR: https://github.com/kubernetes/enhancements/pull/3824#pullrequestreview-1284130849, https://github.com/kubernetes/enhancements/pull/3824#issuecomment-1418010527, https://github.com/kubernetes/enhancements/pull/3824#discussion_r1334533161
- [x] `reject` connections on invalid ports of service IPs (@aroradaman, https://github.com/kubernetes/kubernetes/pull/122692)
  - [Discussion in the KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#behavior-of-service-ips)
  - kube-proxy could watch [`ServiceCIDR` objects](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/1880-multiple-service-cidrs/README.md) to learn the full service CIDR(s) and reject connections on _all_ service IPs, not just currently-in-use-ones.
  - May involve some rewriting of the `@no-endpoints-services` / `@no-endpoints-nodeports` handling, in which case note that we're currently checking no-endpoints nodeports from more places than we need to be (#122687).
  - Resolve the `UNRESOLVED` section of the KEP after this is implemented.
- [X] add periodic and presubmit e2e tests (https://github.com/kubernetes/test-infra/pull/31525)
- [x] move `danwinship/knftables` to `kubernetes-sigs/knftables` (and eventually declare a v0.1.0 API) (@danwinship, https://github.com/kubernetes/org/issues/4673, https://github.com/kubernetes/kubernetes/pull/122920)
  - [x] add basic CI (unit tests, gofmt, ...?) to `kubernetes-sigs/knftables` (https://github.com/kubernetes-sigs/knftables/pull/3)
- [x] drop the `ct state invalid drop` rule (@aroradaman, https://github.com/kubernetes/kubernetes/pull/122663)
  - This wasn't discussed in the KEP, but it's fallout from other changes that were happening at the same time; we have a better way of dealing with this bug now (`--conntrack-tcp-be-liberal`) so we should remove the ugly hack from the nftables proxy and push people to use that instead if they need it.
- [x] ensure unit test parity with iptables. (I think all that's left here is adding unit tests for the packet tracing code in `helpers_test.go`.) (@npinaeva, https://github.com/kubernetes-sigs/knftables/pull/2, https://github.com/kubernetes-sigs/knftables/pull/6, #123389)
- [x] decide whether to change NodePorts vs LoadBalancerSourceRanges behavior, and if so, do it. (NO)
  - [discussion in iptables proxier source](https://github.com/kubernetes/kubernetes/blob/v1.29.0/pkg/proxy/iptables/proxier.go#L1167-L1169)
  - [discussion in the KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#the-iptables-mode-of-kube-proxy-has-grown-crufty)
- [x] decide if we are going to change anything about session affinity. (NO)
  - [Discussion in the KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#session-affinity). I'm not sure if anyone has suggested any particular benefits of the ipvs-like behavior vs the iptables-like behavior.
  - other discussion about changes to affinity behavior: #112806
  - also relevant on the LoadBalancer side (#123714); some LB implementations can't implement the current semantics.
  - Resolve the `UNRESOLVED` section of the KEP after a decision is made.
- [x] decide if we are using comments well in the ruleset. (I GUESS SO?)
  - We don't need many of the comments the iptables ruleset uses because our chain names are self-documenting.
  - In general I tried to put comments on objects (chains, sets, maps) rather than on individual rules, but there is some inconsistency. (But also, note that you won't see the chain/set/map comments with kernels < 5.10.)
  - Individual rules and set/map elements should have comments in some cases. E.g., [elements of `@no-endpoint-services` and `@no-endpoint-nodeports` have comments](https://github.com/kubernetes/kubernetes/blob/v1.29.0/pkg/proxy/nftables/proxier_test.go#L575-L577) so that you can see which IP/port goes with which service. But [elements of `@service-ips` and `@service-nodeports` don't](https://github.com/kubernetes/kubernetes/blob/v1.29.0/pkg/proxy/nftables/proxier_test.go#L570-L573) because you can already figure that out from the names of the chains that they jump to.

#### Things that depend on us having a perf job (and good metrics) first:
- [x] add iptables-style partial syncing (@npinaeva, https://github.com/kubernetes/kubernetes/pull/126013)
- [ ] optimize memory allocation:
  - reuse the buffers used for building the `nft -f -` input, [like iptables does](https://github.com/kubernetes/kubernetes/blob/v1.29.0/pkg/proxy/iptables/proxier.go#L185-L192). This could be done in a few ways. (Have `Proxier` keep buffers around like iptables does and pass a buffer to `nft.Run()`; have `knftables.realNFTables` keep buffers around itself and pass them to the `Transaction`; have `knftables.realNFTables` keep `Transaction`s around and reuse them (with the transaction storing its buffer); ...)
    - (@aroradaman, https://github.com/kubernetes-sigs/knftables/pull/9)
  - maybe reuse the `knftables.Transaction.operation` arrays somehow?
  - consider optimizing `knftables.Rule` generation wrt `knftables.Concat`. Some discussion [here](https://github.com/kubernetes/kubernetes/pull/121046#discussion_r1367736334).
- [ ] rewrite masquerading to use an nftables set rather than using the mark, [as discussed in the KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#masquerading)
- [ ] try out the alternative hairpin rule [suggested in the KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#future-improvements).

#### Additional metrics for iptables mode to help users figure out if they'd have trouble migrating:
- [x] Metric to indicate if you are using localhost nodeports. This could be done by making sure there is always a separate rule to catch 127.0.0.1:nodeport rather than letting it be caught by a generic rule, and then checking the counters on those rules and exporting some metric with the result. (@aroradaman, https://github.com/kubernetes/kubernetes/pull/125015)
- [ ] More generally, metric to indicate if you are using non-primary-node-IP nodeports. (Maybe the metric could just indicate every IP/interface that you're making use of nodeports on?)
- [x] Metric to indicate if you are relying on the `--ctstate INVALID -j DROP` rule (and should be using `--conntrack-tcp-be-liberal` instead) (@aroradaman, https://github.com/kubernetes/kubernetes/pull/122812)
- [ ] Possibly other metrics if we change other behavior
- [ ] (We could also add metrics to ipvs mode to help ipvs users, but that's not a blocker for eventually changing the default like the iptables metrics are.)

#### For 1.31/beta:
- [x] Generally better documentation (mostly in `content/en/docs/reference/networking/virtual-ips.md` in k/website)
  - Document all [changes from iptables](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#changes-from-the-iptables-kube-proxy-backend), including anything above not mentioned in the KEP.
  - Document different interaction with firewalls. ([Discussion in KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#forcing-traffic-on-healthchecknodeports-to-be-accepted))
  - https://github.com/kubernetes/website/pull/46541
- [ ] periodic e2e test of migration/rollback between iptables/ipvs and nftables
- [x] consider the suggestion of [making it possible to run multiple instances of kube-proxy](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#multiple-instances-of-kube-proxy) (@uablrek, https://github.com/kubernetes/kubernetes/pull/122814) (NO)
  - Discussion in the KEP PR: https://github.com/kubernetes/enhancements/pull/3824#issuecomment-1574741149, https://github.com/kubernetes/enhancements/pull/3824#issuecomment-1576820872
  - Resolve the `UNRESOLVED` section of the KEP after a decision is made.
- [ ] blog post (https://github.com/kubernetes/website/pull/46969)

#### For GA:
- [ ] Document the "API guarantees" for components interoperating with nftables kube-proxy.
  - [Discussion in KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-network/3866-nftables-proxy/README.md#defining-an-api-for-integration-with-admindebugthird-party-rules)
  - Resolve the `UNRESOLVED` section of the KEP when this is implemented.

/sig network
/priority important-soon
/triage accepted
cc @aojea @uablrek @aroradaman @tnqn