Skip to content

Commit 542f3c6

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #78547 from MikeSpreitzer/fix-76699
Make iptables and ipvs modes of kube-proxy MASQUERADE --random-fully if possible
2 parents 975d073 + d86d1de commit 542f3c6

File tree

8 files changed

+123
-8
lines changed

8 files changed

+123
-8
lines changed

‎pkg/kubelet/dockershim/network/hostport/fake_iptables.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,3 +347,7 @@ func (f *fakeIPTables) isBuiltinChain(tableName utiliptables.Table, chainName ut
347347
}
348348
return false
349349
}
350+
351+
func (f *fakeIPTables) HasRandomFully() bool {
352+
return false
353+
}

‎pkg/kubelet/kubelet_network_linux.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,21 @@ func (kl *Kubelet) syncNetworkUtil() {
9696
klog.Errorf("Failed to ensure that %s chain %s jumps to %s: %v", utiliptables.TableNAT, utiliptables.ChainPostrouting, KubePostroutingChain, err)
9797
return
9898
}
99-
if _, err := kl.iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubePostroutingChain,
99+
// Establish the masquerading rule.
100+
// NB: THIS MUST MATCH the corresponding code in the iptables and ipvs
101+
// modes of kube-proxy
102+
masqRule := []string{
100103
"-m", "comment", "--comment", "kubernetes service traffic requiring SNAT",
101-
"-m", "mark", "--mark", masqueradeMark, "-j", "MASQUERADE"); err != nil {
104+
"-m", "mark", "--mark", masqueradeMark,
105+
"-j", "MASQUERADE",
106+
}
107+
if kl.iptClient.HasRandomFully() {
108+
masqRule = append(masqRule, "--random-fully")
109+
klog.V(3).Info("Using `--random-fully` in the MASQUERADE rule for iptables")
110+
} else {
111+
klog.V(2).Info("Not using `--random-fully` in the MASQUERADE rule for iptables because the local version of iptables does not support it")
112+
}
113+
if _, err := kl.iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubePostroutingChain, masqRule...); err != nil {
102114
klog.Errorf("Failed to ensure SNAT rule for packets marked by %v in %v chain %v: %v", KubeMarkMasqChain, utiliptables.TableNAT, KubePostroutingChain, err)
103115
return
104116
}

‎pkg/proxy/iptables/proxier.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -780,12 +780,20 @@ func (proxier *Proxier) syncProxyRules() {
780780
// Install the kubernetes-specific postrouting rules. We use a whole chain for
781781
// this so that it is easier to flush and change, for example if the mark
782782
// value should ever change.
783-
writeLine(proxier.natRules, []string{
783+
// NB: THIS MUST MATCH the corresponding code in the kubelet
784+
masqRule := []string{
784785
"-A", string(kubePostroutingChain),
785786
"-m", "comment", "--comment", `"kubernetes service traffic requiring SNAT"`,
786787
"-m", "mark", "--mark", proxier.masqueradeMark,
787788
"-j", "MASQUERADE",
788-
}...)
789+
}
790+
if proxier.iptables.HasRandomFully() {
791+
masqRule = append(masqRule, "--random-fully")
792+
klog.V(3).Info("Using `--random-fully` in the MASQUERADE rule for iptables")
793+
} else {
794+
klog.V(2).Info("Not using `--random-fully` in the MASQUERADE rule for iptables because the local version of iptables does not support it")
795+
}
796+
writeLine(proxier.natRules, masqRule...)
789797

790798
// Install the kubernetes-specific masquerade mark rule. We use a whole chain for
791799
// this so that it is easier to flush and change, for example if the mark

‎pkg/proxy/iptables/proxier_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,15 @@ func hasSrcType(rules []iptablestest.Rule, srcType string) bool {
438438
return false
439439
}
440440

441+
func hasMasqRandomFully(rules []iptablestest.Rule) bool {
442+
for _, r := range rules {
443+
if r[iptablestest.Masquerade] == "--random-fully" {
444+
return true
445+
}
446+
}
447+
return false
448+
}
449+
441450
func TestHasJump(t *testing.T) {
442451
testCases := map[string]struct {
443452
rules []iptablestest.Rule
@@ -784,6 +793,25 @@ func TestNodePort(t *testing.T) {
784793
}
785794
}
786795

796+
func TestMasqueradeRule(t *testing.T) {
797+
for _, testcase := range []bool{false, true} {
798+
ipt := iptablestest.NewFake().SetHasRandomFully(testcase)
799+
fp := NewFakeProxier(ipt, false)
800+
makeServiceMap(fp)
801+
makeEndpointsMap(fp)
802+
fp.syncProxyRules()
803+
804+
postRoutingRules := ipt.GetRules(string(kubePostroutingChain))
805+
if !hasJump(postRoutingRules, "MASQUERADE", "", 0) {
806+
errorf(fmt.Sprintf("Failed to find -j MASQUERADE in %s chain", kubePostroutingChain), postRoutingRules, t)
807+
}
808+
if hasMasqRandomFully(postRoutingRules) != testcase {
809+
probs := map[bool]string{false: "found", true: "did not find"}
810+
errorf(fmt.Sprintf("%s --random-fully in -j MASQUERADE rule in %s chain when HasRandomFully()==%v", probs[testcase], kubePostroutingChain, testcase), postRoutingRules, t)
811+
}
812+
}
813+
}
814+
787815
func TestExternalIPsReject(t *testing.T) {
788816
ipt := iptablestest.NewFake()
789817
fp := NewFakeProxier(ipt, false)

‎pkg/proxy/ipvs/proxier.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1710,12 +1710,20 @@ func (proxier *Proxier) createAndLinkeKubeChain() {
17101710
// Install the kubernetes-specific postrouting rules. We use a whole chain for
17111711
// this so that it is easier to flush and change, for example if the mark
17121712
// value should ever change.
1713-
writeLine(proxier.natRules, []string{
1713+
// NB: THIS MUST MATCH the corresponding code in the kubelet
1714+
masqRule := []string{
17141715
"-A", string(kubePostroutingChain),
17151716
"-m", "comment", "--comment", `"kubernetes service traffic requiring SNAT"`,
17161717
"-m", "mark", "--mark", proxier.masqueradeMark,
17171718
"-j", "MASQUERADE",
1718-
}...)
1719+
}
1720+
if proxier.iptables.HasRandomFully() {
1721+
masqRule = append(masqRule, "--random-fully")
1722+
klog.V(3).Info("Using `--random-fully` in the MASQUERADE rule for iptables")
1723+
} else {
1724+
klog.V(2).Info("Not using `--random-fully` in the MASQUERADE rule for iptables because the local version of iptables does not support it")
1725+
}
1726+
writeLine(proxier.natRules, masqRule...)
17191727

17201728
// Install the kubernetes-specific masquerade mark rule. We use a whole chain for
17211729
// this so that it is easier to flush and change, for example if the mark

‎pkg/proxy/ipvs/proxier_test.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1120,6 +1120,27 @@ func TestClusterIP(t *testing.T) {
11201120
}
11211121
}
11221122

1123+
func TestMasqueradeRule(t *testing.T) {
1124+
for _, testcase := range []bool{false, true} {
1125+
ipt := iptablestest.NewFake().SetHasRandomFully(testcase)
1126+
ipvs := ipvstest.NewFake()
1127+
ipset := ipsettest.NewFake(testIPSetVersion)
1128+
fp := NewFakeProxier(ipt, ipvs, ipset, nil, nil, false)
1129+
makeServiceMap(fp)
1130+
makeEndpointsMap(fp)
1131+
fp.syncProxyRules()
1132+
1133+
postRoutingRules := ipt.GetRules(string(kubePostroutingChain))
1134+
if !hasJump(postRoutingRules, "MASQUERADE", "") {
1135+
t.Errorf("Failed to find -j MASQUERADE in %s chain", kubePostroutingChain)
1136+
}
1137+
if hasMasqRandomFully(postRoutingRules) != testcase {
1138+
probs := map[bool]string{false: "found", true: "did not find"}
1139+
t.Errorf("%s --random-fully in -j MASQUERADE rule in %s chain for HasRandomFully()=%v", probs[testcase], kubePostroutingChain, testcase)
1140+
}
1141+
}
1142+
}
1143+
11231144
func TestExternalIPsNoEndpoint(t *testing.T) {
11241145
ipt := iptablestest.NewFake()
11251146
ipvs := ipvstest.NewFake()
@@ -3112,6 +3133,15 @@ func hasJump(rules []iptablestest.Rule, destChain, ipSet string) bool {
31123133
return false
31133134
}
31143135

3136+
func hasMasqRandomFully(rules []iptablestest.Rule) bool {
3137+
for _, r := range rules {
3138+
if r[iptablestest.Masquerade] == "--random-fully" {
3139+
return true
3140+
}
3141+
}
3142+
return false
3143+
}
3144+
31153145
// checkIptabless to check expected iptables chain and rules
31163146
func checkIptables(t *testing.T, ipt *iptablestest.FakeIPTables, epIpt netlinktest.ExpectedIptablesChain) {
31173147
for epChain, epRules := range epIpt {

‎pkg/util/iptables/iptables.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ type Interface interface {
6969
AddReloadFunc(reloadFunc func())
7070
// Destroy cleans up resources used by the Interface
7171
Destroy()
72+
// HasRandomFully reveals whether `-j MASQUERADE` takes the
73+
// `--random-fully` option. This is helpful to work around a
74+
// Linux kernel bug that sometimes causes multiple flows to get
75+
// mapped to the same IP:PORT and consequently some suffer packet
76+
// drops.
77+
HasRandomFully() bool
7278
}
7379

7480
type Protocol byte
@@ -121,6 +127,8 @@ const NoFlushTables FlushFlag = false
121127
// (test whether a rule exists).
122128
var MinCheckVersion = utilversion.MustParseGeneric("1.4.11")
123129

130+
var RandomFullyMinVersion = utilversion.MustParseGeneric("1.6.2")
131+
124132
// Minimum iptables versions supporting the -w and -w<seconds> flags
125133
var WaitMinVersion = utilversion.MustParseGeneric("1.4.20")
126134
var WaitSecondsMinVersion = utilversion.MustParseGeneric("1.4.22")
@@ -139,6 +147,7 @@ type runner struct {
139147
protocol Protocol
140148
hasCheck bool
141149
hasListener bool
150+
hasRandomFully bool
142151
waitFlag []string
143152
restoreWaitFlag []string
144153
lockfilePath string
@@ -166,6 +175,7 @@ func newInternal(exec utilexec.Interface, dbus utildbus.Interface, protocol Prot
166175
protocol: protocol,
167176
hasCheck: version.AtLeast(MinCheckVersion),
168177
hasListener: false,
178+
hasRandomFully: version.AtLeast(RandomFullyMinVersion),
169179
waitFlag: getIPTablesWaitFlag(version),
170180
restoreWaitFlag: getIPTablesRestoreWaitFlag(version),
171181
lockfilePath: lockfilePath,
@@ -632,6 +642,10 @@ func (runner *runner) reload() {
632642
}
633643
}
634644

645+
func (runner *runner) HasRandomFully() bool {
646+
return runner.hasRandomFully
647+
}
648+
635649
var iptablesNotFoundStrings = []string{
636650
// iptables-legacy [-A|-I] BAD-CHAIN [...]
637651
// iptables-legacy [-C|-D] GOOD-CHAIN [...non-matching rule...]

‎pkg/util/iptables/testing/fake.go‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,19 +35,26 @@ const (
3535
Recent = "recent "
3636
MatchSet = "--match-set "
3737
SrcType = "--src-type "
38+
Masquerade = "MASQUERADE "
3839
)
3940

4041
type Rule map[string]string
4142

4243
// no-op implementation of iptables Interface
4344
type FakeIPTables struct {
44-
Lines []byte
45+
hasRandomFully bool
46+
Lines []byte
4547
}
4648

4749
func NewFake() *FakeIPTables {
4850
return &FakeIPTables{}
4951
}
5052

53+
func (f *FakeIPTables) SetHasRandomFully(can bool) *FakeIPTables {
54+
f.hasRandomFully = can
55+
return f
56+
}
57+
5158
func (*FakeIPTables) EnsureChain(table iptables.Table, chain iptables.Chain) (bool, error) {
5259
return true, nil
5360
}
@@ -110,7 +117,7 @@ func (f *FakeIPTables) GetRules(chainName string) (rules []Rule) {
110117
for _, l := range strings.Split(string(f.Lines), "\n") {
111118
if strings.Contains(l, fmt.Sprintf("-A %v", chainName)) {
112119
newRule := Rule(map[string]string{})
113-
for _, arg := range []string{Destination, Source, DPort, Protocol, Jump, ToDest, Recent, MatchSet, SrcType} {
120+
for _, arg := range []string{Destination, Source, DPort, Protocol, Jump, ToDest, Recent, MatchSet, SrcType, Masquerade} {
114121
tok := getToken(l, arg)
115122
if tok != "" {
116123
newRule[arg] = tok
@@ -122,4 +129,8 @@ func (f *FakeIPTables) GetRules(chainName string) (rules []Rule) {
122129
return
123130
}
124131

132+
func (f *FakeIPTables) HasRandomFully() bool {
133+
return f.hasRandomFully
134+
}
135+
125136
var _ = iptables.Interface(&FakeIPTables{})

0 commit comments

Comments
 (0)