Bug fix. Incoming UDP packets not reach newly deployed services #32561

zreigz · 2016-09-13T08:54:33Z

What this PR does / why we need it:

Incoming UDP packets not reach newly deployed services when old connection's state in conntrack is not cleared. When a packet arrives, it will not go through NAT table again, because it is not "the first" packet. The PR fix the issue

Which issue this PR fixes
Fixes #31983
xref moby/moby#8795

This change is

k8s-ci-robot · 2016-09-13T08:54:35Z

Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "ok to test" on its own line.

Regular contributors should join the org to skip this step.

While we transition away from the Jenkins GitHub PR Builder plugin, "ok to test" commenters will need to be on the admin list defined in this file.

zreigz · 2016-09-26T12:36:05Z

cc @kubernetes/sig-network

zreigz · 2016-10-12T08:41:39Z

cc @kubernetes/sig-network PTAL

zreigz · 2016-11-03T11:39:11Z

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

thockin · 2016-11-21T05:26:23Z

@k8s-bot ok to test

thockin · 2016-11-21T05:24:10Z

pkg/proxy/iptables/proxier.go

@@ -1075,6 +1075,7 @@ func (proxier *Proxier) syncProxyRules() {
 				glog.V(4).Infof("Port %s was open before and is still needed", lp.String())
 				replacementPortsMap[lp] = proxier.portsMap[lp]
 			} else {
+


This is a dead line - remove

thockin · 2016-11-21T05:38:21Z

pkg/proxy/iptables/proxier.go

+			// https://github.com/kubernetes/kubernetes/issues/31983
+			if lp.protocol == "udp" {
+				glog.V(2).Infof("Deleting conntrack entries for udp connections")
+				err := proxier.execConntrackTool("-D", "-p", "udp", "--dport", strconv.Itoa(lp.port))


This doesn't only happen once - this is going to happen periodically, unless I am mis-readng. I think you want to do this up around line 1077 (OpenLocalPort), when you know that this is a new nodePort, rather than a refresh. You might also do it way at the end of this function when we close nodeports we don't want any more.

But even that is not 100% correct. I think there's a bug here where, if the same nodeport were to be release and reused inside a single refresh loop it would fail. I think that will self-repair, though.

zreigz · 2016-11-21T07:17:42Z

@thockin Done. I followed your comments and I've made necessary changes. I've tested it and works
PTAL

k8s-ci-robot · 2016-11-21T07:42:25Z

Jenkins verification failed for commit f53a2c50e093044d9c6ddc13522b2a021fa749cf. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-11-21T09:30:57Z

Jenkins unit/integration failed for commit cddfd2f. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

thockin · 2016-11-30T05:41:54Z

ok for post 1.5.0

dims · 2016-11-30T13:17:34Z

Is this a real error or a flake?
https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/32561/pull-kubernetes-unit/5897/

dims · 2016-11-30T23:56:02Z

moving this to post 1.5.0 == 1.6 :)

thockin

I am going to LGTM and see if you can fix it before the bot merges it. If not, please send a followup :)

thockin · 2016-12-01T07:40:56Z

pkg/proxy/iptables/proxier.go

+	glog.V(2).Infof("Deleting conntrack entries for udp connections")
+	if port > 0 {
+		err = proxier.execConntrackTool("-D", "-p", "udp", "--dport", strconv.Itoa(port))
+	} else {


We don't really need this fallback path any more - would be safer to just log.

zreigz · 2016-12-01T07:54:19Z

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

Comments from Reviewable

thockin · 2016-12-01T07:55:16Z

LGTM!

zreigz · 2016-12-01T07:55:47Z

Thanks

zreigz · 2016-12-01T07:56:55Z

Reviewed 1 of 1 files at r5.
Review status: all files reviewed at latest revision, 5 unresolved discussions.

Comments from Reviewable

k8s-github-robot · 2016-12-01T22:20:59Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2016-12-02T00:02:03Z

Automatic merge from submit-queue

jfoy · 2017-01-13T23:19:09Z

I'm confused - how is this different from #22573 ?

bjoernhaeuser · 2017-02-01T20:38:07Z

Will this be cherry-picked for 1.5.?

thockin · 2017-02-06T05:32:56Z

Given that 1.6 is freezing in O(weeks) we weren't planning on back-patching to 1.5

…

On Wed, Feb 1, 2017 at 12:38 PM, Björn Häuser ***@***.***> wrote: Will this be cherry-picked for 1.5.? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#32561 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFVgVNTZQp072aQo-xFZDRRM0GkSShN-ks5rYO1JgaJpZM4J7b8n> .

timchenxiaoyu · 2019-08-22T08:51:42Z

why TCP doesn't this problem ？

googlebot added the cla: yes label Sep 13, 2016

k8s-github-robot assigned ArtfulCoder Sep 13, 2016

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Sep 13, 2016

zreigz mentioned this pull request Sep 13, 2016

Incoming UDP packets not reach newly deployed services #31983

Closed

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 1, 2016

zreigz force-pushed the fix-incoming-udp branch from 453dadd to aeab3e6 Compare October 12, 2016 08:41

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 12, 2016

zreigz force-pushed the fix-incoming-udp branch from aeab3e6 to 53bc8f3 Compare November 3, 2016 11:38

zreigz unassigned ArtfulCoder Nov 3, 2016

k8s-github-robot assigned thockin Nov 3, 2016

k8s-github-robot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Nov 10, 2016

apelisse removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Nov 11, 2016

thockin added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Nov 21, 2016

thockin changed the title ~~Bug fix. Incoming UDP packets not reach newly deployed services~~ Clear stale conntrack for incoming UDP packets on nodePorts Nov 21, 2016

thockin reviewed Nov 21, 2016

View reviewed changes

zreigz force-pushed the fix-incoming-udp branch from 53bc8f3 to f53a2c5 Compare November 21, 2016 07:13

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 21, 2016

zreigz force-pushed the fix-incoming-udp branch 2 times, most recently from dffaf3f to cddfd2f Compare November 21, 2016 09:13

zreigz changed the title ~~Clear stale conntrack for incoming UDP packets on nodePorts~~ Bug fix. Incoming UDP packets not reach newly deployed services Nov 21, 2016

thockin self-assigned this Nov 30, 2016

thockin added this to the v1.5 milestone Nov 30, 2016

dims modified the milestones: v1.6, v1.5 Nov 30, 2016

zreigz force-pushed the fix-incoming-udp branch from cddfd2f to b70f4df Compare December 1, 2016 06:02

thockin approved these changes Dec 1, 2016

View reviewed changes

thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 1, 2016

Bug fix. Incoming UDP packets not reach newly deployed services

dc54a8d

zreigz force-pushed the fix-incoming-udp branch from b70f4df to dc54a8d Compare December 1, 2016 07:52

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 1, 2016

thockin added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 1, 2016

k8s-github-robot merged commit 9defe2c into kubernetes:master Dec 2, 2016

efficks mentioned this pull request Feb 6, 2017

Failed to receive UDP traffic after container restart moby/moby#8795

Closed

rfay mentioned this pull request Mar 31, 2017

UDP memberlist checks always timing out hashicorp/consul#2200

Closed

cmluciano mentioned this pull request Dec 6, 2017

KubeProxy does not delete connections for traffic to NodePort Service if the backend changes. #42473

Closed

andrewsykim mentioned this pull request Oct 31, 2019

kube-proxy clean udp conntrack after node network resumed #84438

Closed

aojea mentioned this pull request Jun 14, 2020

WIP improve kube-proxy conntrack resilience #92122

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug fix. Incoming UDP packets not reach newly deployed services #32561

Bug fix. Incoming UDP packets not reach newly deployed services #32561

zreigz commented Sep 13, 2016 •

edited by thockin

Loading

k8s-ci-robot commented Sep 13, 2016

zreigz commented Sep 26, 2016

zreigz commented Oct 12, 2016

zreigz commented Nov 3, 2016

thockin commented Nov 21, 2016

thockin Nov 21, 2016

thockin Nov 21, 2016

zreigz commented Nov 21, 2016

k8s-ci-robot commented Nov 21, 2016

k8s-ci-robot commented Nov 21, 2016

thockin commented Nov 30, 2016

dims commented Nov 30, 2016

dims commented Nov 30, 2016

thockin left a comment

thockin Dec 1, 2016

zreigz Dec 1, 2016

zreigz commented Dec 1, 2016

thockin commented Dec 1, 2016

zreigz commented Dec 1, 2016

zreigz commented Dec 1, 2016

k8s-github-robot commented Dec 1, 2016

k8s-github-robot commented Dec 2, 2016

jfoy commented Jan 13, 2017

bjoernhaeuser commented Feb 1, 2017

thockin commented Feb 6, 2017 via email

timchenxiaoyu commented Aug 22, 2019

Bug fix. Incoming UDP packets not reach newly deployed services #32561

Bug fix. Incoming UDP packets not reach newly deployed services #32561

Conversation

zreigz commented Sep 13, 2016 • edited by thockin Loading

k8s-ci-robot commented Sep 13, 2016

zreigz commented Sep 26, 2016

zreigz commented Oct 12, 2016

zreigz commented Nov 3, 2016

thockin commented Nov 21, 2016

thockin Nov 21, 2016

Choose a reason for hiding this comment

thockin Nov 21, 2016

Choose a reason for hiding this comment

zreigz commented Nov 21, 2016

k8s-ci-robot commented Nov 21, 2016

k8s-ci-robot commented Nov 21, 2016

thockin commented Nov 30, 2016

dims commented Nov 30, 2016

dims commented Nov 30, 2016

thockin left a comment

Choose a reason for hiding this comment

thockin Dec 1, 2016

Choose a reason for hiding this comment

zreigz Dec 1, 2016

Choose a reason for hiding this comment

zreigz commented Dec 1, 2016

thockin commented Dec 1, 2016

zreigz commented Dec 1, 2016

zreigz commented Dec 1, 2016

k8s-github-robot commented Dec 1, 2016

k8s-github-robot commented Dec 2, 2016

jfoy commented Jan 13, 2017

bjoernhaeuser commented Feb 1, 2017

thockin commented Feb 6, 2017 via email

timchenxiaoyu commented Aug 22, 2019

zreigz commented Sep 13, 2016 •

edited by thockin

Loading