Skip to content

Commit

Permalink
option: move checking of map size parameters to own function
Browse files Browse the repository at this point in the history 
Refactoring in preparation for introducing dynamic map sizes. Also add
some tests.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
  • Loading branch information
@tklauser
tklauser authored and Michal Rostecki committed Apr 14, 2020
1 parent 4c12742 commit 6f98435
Show file tree
Hide file tree
Showing 2 changed files with 259 additions and 42 deletions.
93 changes: 51 additions & 42 deletions pkg/option/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2156,48 +2156,8 @@ func (c *DaemonConfig) Validate() error {
}
}

if c.CTMapEntriesGlobalTCP < LimitTableMin || c.CTMapEntriesGlobalAny < LimitTableMin {
return fmt.Errorf("specified CT tables values %d/%d must exceed minimum %d",
c.CTMapEntriesGlobalTCP, c.CTMapEntriesGlobalAny, LimitTableMin)
}
if c.CTMapEntriesGlobalTCP > LimitTableMax || c.CTMapEntriesGlobalAny > LimitTableMax {
return fmt.Errorf("specified CT tables values %d/%d must not exceed maximum %d",
c.CTMapEntriesGlobalTCP, c.CTMapEntriesGlobalAny, LimitTableMax)
}
if c.NATMapEntriesGlobal < LimitTableMin {
return fmt.Errorf("specified NAT table size %d must exceed minimum %d",
c.NATMapEntriesGlobal, LimitTableMin)
}
if c.NATMapEntriesGlobal > LimitTableMax {
return fmt.Errorf("specified NAT tables size %d must not exceed maximum %d",
c.NATMapEntriesGlobal, LimitTableMax)
}
if c.NATMapEntriesGlobal > c.CTMapEntriesGlobalTCP+c.CTMapEntriesGlobalAny {
if c.NATMapEntriesGlobal == NATMapEntriesGlobalDefault {
// Auto-size for the case where CT table size was adapted but NAT still on default
c.NATMapEntriesGlobal = int((c.CTMapEntriesGlobalTCP + c.CTMapEntriesGlobalAny) * 2 / 3)
} else {
return fmt.Errorf("specified NAT tables size %d must not exceed maximum CT table size %d",
c.NATMapEntriesGlobal, c.CTMapEntriesGlobalTCP+c.CTMapEntriesGlobalAny)
}
}

if c.PolicyMapEntries < PolicyMapMin {
return fmt.Errorf("specified PolicyMap max entries %d must exceed minimum %d",
c.PolicyMapEntries, PolicyMapMin)
}
if c.PolicyMapEntries > PolicyMapMax {
return fmt.Errorf("specified PolicyMap max entries %d must not exceed maximum %d",
c.PolicyMapEntries, PolicyMapMax)
}

if c.FragmentsMapEntries < FragmentsMapMin {
return fmt.Errorf("specified fragments tracking map max entries %d must exceed minimum %d",
c.FragmentsMapEntries, FragmentsMapMin)
}
if c.FragmentsMapEntries > FragmentsMapMax {
return fmt.Errorf("specified fragments tracking map max entries %d must not exceed maximum %d",
c.FragmentsMapEntries, FragmentsMapMax)
if err := c.checkMapSizeLimits(); err != nil {
return err
}

// Validate that the KVStore Lease TTL value lies between a particular range.
Expand Down Expand Up @@ -2745,6 +2705,55 @@ func (c *DaemonConfig) populateHostServicesProtos() error {
return nil
}

func (c *DaemonConfig) checkMapSizeLimits() error {
if c.CTMapEntriesGlobalTCP < LimitTableMin || c.CTMapEntriesGlobalAny < LimitTableMin {
return fmt.Errorf("specified CT tables values %d/%d must exceed minimum %d",
c.CTMapEntriesGlobalTCP, c.CTMapEntriesGlobalAny, LimitTableMin)
}
if c.CTMapEntriesGlobalTCP > LimitTableMax || c.CTMapEntriesGlobalAny > LimitTableMax {
return fmt.Errorf("specified CT tables values %d/%d must not exceed maximum %d",
c.CTMapEntriesGlobalTCP, c.CTMapEntriesGlobalAny, LimitTableMax)
}

if c.NATMapEntriesGlobal < LimitTableMin {
return fmt.Errorf("specified NAT table size %d must exceed minimum %d",
c.NATMapEntriesGlobal, LimitTableMin)
}
if c.NATMapEntriesGlobal > LimitTableMax {
return fmt.Errorf("specified NAT tables size %d must not exceed maximum %d",
c.NATMapEntriesGlobal, LimitTableMax)
}
if c.NATMapEntriesGlobal > c.CTMapEntriesGlobalTCP+c.CTMapEntriesGlobalAny {
if c.NATMapEntriesGlobal == NATMapEntriesGlobalDefault {
// Auto-size for the case where CT table size was adapted but NAT still on default
c.NATMapEntriesGlobal = int((c.CTMapEntriesGlobalTCP + c.CTMapEntriesGlobalAny) * 2 / 3)
} else {
return fmt.Errorf("specified NAT tables size %d must not exceed maximum CT table size %d",
c.NATMapEntriesGlobal, c.CTMapEntriesGlobalTCP+c.CTMapEntriesGlobalAny)
}
}

if c.PolicyMapEntries < PolicyMapMin {
return fmt.Errorf("specified PolicyMap max entries %d must exceed minimum %d",
c.PolicyMapEntries, PolicyMapMin)
}
if c.PolicyMapEntries > PolicyMapMax {
return fmt.Errorf("specified PolicyMap max entries %d must not exceed maximum %d",
c.PolicyMapEntries, PolicyMapMax)
}

if c.FragmentsMapEntries < FragmentsMapMin {
return fmt.Errorf("specified fragments tracking map max entries %d must exceed minimum %d",
c.FragmentsMapEntries, FragmentsMapMin)
}
if c.FragmentsMapEntries > FragmentsMapMax {
return fmt.Errorf("specified fragments tracking map max entries %d must not exceed maximum %d",
c.FragmentsMapEntries, FragmentsMapMax)
}

return nil
}

func sanitizeIntParam(paramName string, paramDefault int) int {
intParam := viper.GetInt(paramName)
if intParam <= 0 {
Expand Down
208 changes: 208 additions & 0 deletions pkg/option/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"testing"

"github.com/cilium/cilium/pkg/defaults"
"github.com/google/go-cmp/cmp"
flag "github.com/spf13/pflag"
"github.com/spf13/viper"
. "gopkg.in/check.v1"
Expand Down Expand Up @@ -259,6 +260,213 @@ func (s *OptionSuite) TestEndpointStatusIsEnabled(c *C) {
c.Assert(d.EndpointStatusIsEnabled(EndpointStatusLog), Equals, false)
}

func TestCheckMapSizeLimits(t *testing.T) {
type sizes struct {
CTMapEntriesGlobalTCP int
CTMapEntriesGlobalAny int
NATMapEntriesGlobal int
PolicyMapEntries int
FragmentsMapEntries int
WantErr bool
}
tests := []struct {
name string
d *DaemonConfig
want sizes
}{
{
name: "default map sizes",
d: &DaemonConfig{
CTMapEntriesGlobalTCP: CTMapEntriesGlobalTCPDefault,
CTMapEntriesGlobalAny: CTMapEntriesGlobalAnyDefault,
NATMapEntriesGlobal: NATMapEntriesGlobalDefault,
PolicyMapEntries: defaults.PolicyMapEntries,
FragmentsMapEntries: defaults.FragmentsMapEntries,
},
want: sizes{
CTMapEntriesGlobalTCP: CTMapEntriesGlobalTCPDefault,
CTMapEntriesGlobalAny: CTMapEntriesGlobalAnyDefault,
NATMapEntriesGlobal: NATMapEntriesGlobalDefault,
PolicyMapEntries: defaults.PolicyMapEntries,
FragmentsMapEntries: defaults.FragmentsMapEntries,
WantErr: false,
},
},
{
name: "arbitrary map sizes within range",
d: &DaemonConfig{
CTMapEntriesGlobalTCP: 20000,
CTMapEntriesGlobalAny: 18000,
NATMapEntriesGlobal: 2048,
PolicyMapEntries: 512,
FragmentsMapEntries: 2 << 14,
},
want: sizes{
CTMapEntriesGlobalTCP: 20000,
CTMapEntriesGlobalAny: 18000,
NATMapEntriesGlobal: 2048,
PolicyMapEntries: 512,
FragmentsMapEntries: 2 << 14,
WantErr: false,
},
},
{
name: "CT TCP map size below range",
d: &DaemonConfig{
CTMapEntriesGlobalTCP: LimitTableMin - 1,
},
want: sizes{
CTMapEntriesGlobalTCP: LimitTableMin - 1,
WantErr: true,
},
},
{
name: "CT TCP map size above range",
d: &DaemonConfig{
CTMapEntriesGlobalTCP: LimitTableMax + 1,
},
want: sizes{
CTMapEntriesGlobalTCP: LimitTableMax + 1,
WantErr: true,
},
},
{
name: "CT Any map size below range",
d: &DaemonConfig{
CTMapEntriesGlobalAny: LimitTableMin - 1,
},
want: sizes{
CTMapEntriesGlobalAny: LimitTableMin - 1,
WantErr: true,
},
},
{
name: "CT Any map size above range",
d: &DaemonConfig{
CTMapEntriesGlobalAny: LimitTableMax + 1,
},
want: sizes{
CTMapEntriesGlobalAny: LimitTableMax + 1,
WantErr: true,
},
},
{
name: "NAT map size below range",
d: &DaemonConfig{
NATMapEntriesGlobal: LimitTableMin - 1,
},
want: sizes{
NATMapEntriesGlobal: LimitTableMin - 1,
WantErr: true,
},
},
{
name: "NAT map size above range",
d: &DaemonConfig{
NATMapEntriesGlobal: LimitTableMax + 1,
},
want: sizes{
NATMapEntriesGlobal: LimitTableMax + 1,
WantErr: true,
},
},
{
name: "NAT map auto sizing with default size",
d: &DaemonConfig{
CTMapEntriesGlobalTCP: 2048,
CTMapEntriesGlobalAny: 4096,
NATMapEntriesGlobal: NATMapEntriesGlobalDefault,
PolicyMapEntries: defaults.PolicyMapEntries,
FragmentsMapEntries: defaults.FragmentsMapEntries,
},
want: sizes{
CTMapEntriesGlobalTCP: 2048,
CTMapEntriesGlobalAny: 4096,
NATMapEntriesGlobal: (2048 + 4096) * 2 / 3,
PolicyMapEntries: defaults.PolicyMapEntries,
FragmentsMapEntries: defaults.FragmentsMapEntries,
WantErr: false,
},
},
{
name: "NAT map auto sizing outside of range",
d: &DaemonConfig{
CTMapEntriesGlobalTCP: 2048,
CTMapEntriesGlobalAny: 4096,
NATMapEntriesGlobal: 8192,
},
want: sizes{
CTMapEntriesGlobalTCP: 2048,
CTMapEntriesGlobalAny: 4096,
NATMapEntriesGlobal: 8192,
WantErr: true,
},
},
{
name: "Policy map size below range",
d: &DaemonConfig{
PolicyMapEntries: PolicyMapMin - 1,
},
want: sizes{
PolicyMapEntries: PolicyMapMin - 1,
WantErr: true,
},
},
{
name: "Policy map size above range",
d: &DaemonConfig{
PolicyMapEntries: PolicyMapMax + 1,
},
want: sizes{
PolicyMapEntries: PolicyMapMax + 1,
WantErr: true,
},
},
{
name: "Fragments map size below range",
d: &DaemonConfig{
FragmentsMapEntries: FragmentsMapMin - 1,
},
want: sizes{
FragmentsMapEntries: FragmentsMapMin - 1,
WantErr: true,
},
},
{
name: "Fragments map size above range",
d: &DaemonConfig{
FragmentsMapEntries: FragmentsMapMax + 1,
},
want: sizes{
FragmentsMapEntries: FragmentsMapMax + 1,
WantErr: true,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := tt.d.checkMapSizeLimits()

got := sizes{
CTMapEntriesGlobalTCP: tt.d.CTMapEntriesGlobalTCP,
CTMapEntriesGlobalAny: tt.d.CTMapEntriesGlobalAny,
NATMapEntriesGlobal: tt.d.NATMapEntriesGlobal,
PolicyMapEntries: tt.d.PolicyMapEntries,
FragmentsMapEntries: tt.d.FragmentsMapEntries,
WantErr: err != nil,
}

if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("DaemonConfig.checkMapSizeLimits mismatch (-want +got):\n%s", diff)

if err != nil {
t.Error(err)
}
}
})
}
}

func Test_populateNodePortRange(t *testing.T) {
type want struct {
wantMin int
Expand Down

0 comments on commit 6f98435

Please sign in to comment.