Skip to content

Commit

Permalink
Configure rp_filter for primary interface based on env variable.
Browse files Browse the repository at this point in the history
  • Loading branch information
@SaranBalaji90
SaranBalaji90 committed Apr 7, 2020
1 parent 21bd4ac commit 750e8bd
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 89 deletions.
46 changes: 30 additions & 16 deletions pkg/networkutils/network.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ const (
// sent over the main ENI.
envConnmark = "AWS_VPC_K8S_CNI_CONNMARK"

// This environment variable indicates if ipamd should configure rp filter for primary interface. Default value is
// true. If set to false, then rp filter should be configured through init container.
envConfigureRpfilter = "AWS_VPC_K8S_CNI_CONFIGURE_RPFILTER"

// defaultConnmark is the default value for the connmark described above. Note: the mark space is a little crowded,
// - kube-proxy uses 0x0000c000
// - Calico uses 0xffff0000.
Expand Down Expand Up @@ -125,12 +129,13 @@ type NetworkAPIs interface {
}

type linuxNetwork struct {
useExternalSNAT bool
excludeSNATCIDRs []string
typeOfSNAT snatType
nodePortSupportEnabled bool
connmark uint32
mtu int
useExternalSNAT bool
excludeSNATCIDRs []string
typeOfSNAT snatType
nodePortSupportEnabled bool
shouldConfigureRpFilter bool
connmark uint32
mtu int

netLink netlinkwrapper.NetLink
ns nswrapper.NS
Expand Down Expand Up @@ -163,12 +168,13 @@ const (
// New creates a linuxNetwork object
func New() NetworkAPIs {
return &linuxNetwork{
useExternalSNAT: useExternalSNAT(),
excludeSNATCIDRs: getExcludeSNATCIDRs(),
typeOfSNAT: typeOfSNAT(),
nodePortSupportEnabled: nodePortSupportEnabled(),
mainENIMark: getConnmark(),
mtu: GetEthernetMTU(""),
useExternalSNAT: useExternalSNAT(),
excludeSNATCIDRs: getExcludeSNATCIDRs(),
typeOfSNAT: typeOfSNAT(),
nodePortSupportEnabled: nodePortSupportEnabled(),
shouldConfigureRpFilter: shouldConfigureRpFilter(),
mainENIMark: getConnmark(),
mtu: GetEthernetMTU(""),

netLink: netlinkwrapper.NewNetLink(),
ns: nswrapper.NewNS(),
Expand Down Expand Up @@ -243,10 +249,14 @@ func (n *linuxNetwork) SetupHostNetwork(vpcCIDR *net.IPNet, vpcCIDRs []*string,
primaryIntfRPFilter := "net/ipv4/conf/" + primaryIntf + "/rp_filter"
const rpFilterLoose = "2"

log.Debugf("Setting RPF for primary interface: %s", primaryIntfRPFilter)
err = n.procSys.Set(primaryIntfRPFilter, rpFilterLoose)
if err != nil {
return errors.Wrapf(err, "failed to configure %s RPF check", primaryIntf)
if n.shouldConfigureRpFilter {
log.Debugf("Setting RPF for primary interface: %s", primaryIntfRPFilter)
err = n.procSys.Set(primaryIntfRPFilter, rpFilterLoose)
if err != nil {
return errors.Wrapf(err, "failed to configure %s RPF check", primaryIntf)
}
} else {
log.Infof("Skip updating RPF for primary interface: %s", primaryIntfRPFilter)
}
}

Expand Down Expand Up @@ -596,6 +606,10 @@ func nodePortSupportEnabled() bool {
return getBoolEnvVar(envNodePortSupport, true)
}

func shouldConfigureRpFilter() bool {
return getBoolEnvVar(envConfigureRpfilter, true)
}

func getBoolEnvVar(name string, defaultValue bool) bool {
if strValue := os.Getenv(name); strValue != "" {
parsedValue, err := strconv.ParseBool(strValue)
Expand Down
144 changes: 71 additions & 73 deletions pkg/networkutils/network_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,11 @@ func TestSetupHostNetworkNodePortEnabled(t *testing.T) {
defer ctrl.Finish()

ln := &linuxNetwork{
useExternalSNAT: true,
nodePortSupportEnabled: true,
mainENIMark: defaultConnmark,
mtu: testMTU,
useExternalSNAT: true,
nodePortSupportEnabled: true,
shouldConfigureRpFilter: true,
mainENIMark: defaultConnmark,
mtu: testMTU,

netLink: mockNetLink,
ns: mockNS,
Expand All @@ -285,16 +286,7 @@ func TestSetupHostNetworkNodePortEnabled(t *testing.T) {
procSys: mockProcSys,
}

mockPrimaryInterfaceLookup(ctrl, mockNetLink)
mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)

var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)
setupNetLinkMocks(ctrl, mockNetLink)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

Expand Down Expand Up @@ -349,11 +341,12 @@ func TestSetupHostNetworkWithExcludeSNATCIDRs(t *testing.T) {
defer ctrl.Finish()

ln := &linuxNetwork{
useExternalSNAT: false,
excludeSNATCIDRs: []string{"10.12.0.0/16", "10.13.0.0/16"},
nodePortSupportEnabled: true,
mainENIMark: defaultConnmark,
mtu: testMTU,
useExternalSNAT: false,
excludeSNATCIDRs: []string{"10.12.0.0/16", "10.13.0.0/16"},
nodePortSupportEnabled: true,
shouldConfigureRpFilter: true,
mainENIMark: defaultConnmark,
mtu: testMTU,

netLink: mockNetLink,
ns: mockNS,
Expand All @@ -363,16 +356,7 @@ func TestSetupHostNetworkWithExcludeSNATCIDRs(t *testing.T) {
procSys: mockProcSys,
}

mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)
setupNetLinkMocks(ctrl, mockNetLink)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

Expand Down Expand Up @@ -403,11 +387,12 @@ func TestSetupHostNetworkCleansUpStaleSNATRules(t *testing.T) {
defer ctrl.Finish()

ln := &linuxNetwork{
useExternalSNAT: false,
excludeSNATCIDRs: nil,
nodePortSupportEnabled: true,
mainENIMark: defaultConnmark,
mtu: testMTU,
useExternalSNAT: false,
excludeSNATCIDRs: nil,
nodePortSupportEnabled: true,
shouldConfigureRpFilter: true,
mainENIMark: defaultConnmark,
mtu: testMTU,

netLink: mockNetLink,
ns: mockNS,
Expand All @@ -416,16 +401,7 @@ func TestSetupHostNetworkCleansUpStaleSNATRules(t *testing.T) {
},
procSys: mockProcSys,
}
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)
setupNetLinkMocks(ctrl, mockNetLink)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

Expand Down Expand Up @@ -464,11 +440,12 @@ func TestSetupHostNetworkExcludedSNATCIDRsIdempotent(t *testing.T) {
defer ctrl.Finish()

ln := &linuxNetwork{
useExternalSNAT: false,
excludeSNATCIDRs: []string{"10.12.0.0/16", "10.13.0.0/16"},
nodePortSupportEnabled: true,
mainENIMark: defaultConnmark,
mtu: testMTU,
useExternalSNAT: false,
excludeSNATCIDRs: []string{"10.12.0.0/16", "10.13.0.0/16"},
nodePortSupportEnabled: true,
shouldConfigureRpFilter: true,
mainENIMark: defaultConnmark,
mtu: testMTU,

netLink: mockNetLink,
ns: mockNS,
Expand All @@ -477,16 +454,7 @@ func TestSetupHostNetworkExcludedSNATCIDRsIdempotent(t *testing.T) {
},
procSys: mockProcSys,
}
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)
setupNetLinkMocks(ctrl, mockNetLink)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

Expand Down Expand Up @@ -525,10 +493,11 @@ func TestSetupHostNetworkMultipleCIDRs(t *testing.T) {
defer ctrl.Finish()

ln := &linuxNetwork{
useExternalSNAT: true,
nodePortSupportEnabled: true,
mainENIMark: defaultConnmark,
mtu: testMTU,
useExternalSNAT: true,
nodePortSupportEnabled: true,
shouldConfigureRpFilter: true,
mainENIMark: defaultConnmark,
mtu: testMTU,

netLink: mockNetLink,
ns: mockNS,
Expand All @@ -537,16 +506,7 @@ func TestSetupHostNetworkMultipleCIDRs(t *testing.T) {
},
procSys: mockProcSys,
}
mockPrimaryInterfaceLookup(ctrl, mockNetLink)

mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)
var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)
setupNetLinkMocks(ctrl, mockNetLink)

mockProcSys.EXPECT().Set("net/ipv4/conf/lo/rp_filter", "2").Return(nil)

Expand Down Expand Up @@ -582,6 +542,44 @@ func TestIncrementIPv4Addr(t *testing.T) {
}
}

func TestSetupHostNetworkIgnoringRpFilterUpdate(t *testing.T) {
ctrl, mockNetLink, _, mockNS, mockIptables, mockProcSys := setup(t)
defer ctrl.Finish()

ln := &linuxNetwork{
useExternalSNAT: true,
nodePortSupportEnabled: true,
shouldConfigureRpFilter: false,
mainENIMark: defaultConnmark,
mtu: testMTU,

netLink: mockNetLink,
ns: mockNS,
newIptables: func() (iptablesIface, error) {
return mockIptables, nil
},
procSys: mockProcSys,
}
setupNetLinkMocks(ctrl, mockNetLink)

var vpcCIDRs []*string
err := ln.SetupHostNetwork(testENINetIPNet, vpcCIDRs, loopback, &testENINetIP)
assert.NoError(t, err)
}

func setupNetLinkMocks(ctrl *gomock.Controller, mockNetLink *mock_netlinkwrapper.MockNetLink) {
mockPrimaryInterfaceLookup(ctrl, mockNetLink)
mockNetLink.EXPECT().LinkSetMTU(gomock.Any(), testMTU).Return(nil)

var hostRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&hostRule)
mockNetLink.EXPECT().RuleDel(&hostRule)
var mainENIRule netlink.Rule
mockNetLink.EXPECT().NewRule().Return(&mainENIRule)
mockNetLink.EXPECT().RuleDel(&mainENIRule)
mockNetLink.EXPECT().RuleAdd(&mainENIRule)
}

type mockIptables struct {
// dataplaneState is a map from table name to chain name to slice of rulespecs
dataplaneState map[string]map[string][][]string
Expand Down

0 comments on commit 750e8bd

Please sign in to comment.