diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 7f315262b92..db5fce2065c 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -56,38 +56,6 @@ func checkAllLines(t *testing.T, table utiliptables.Table, save []byte, expected } } -func TestReadLinesFromByteBuffer(t *testing.T) { - testFn := func(byteArray []byte, expected []string) { - index := 0 - readIndex := 0 - for ; readIndex < len(byteArray); index++ { - line, n := utiliptables.ReadLine(readIndex, byteArray) - readIndex = n - if expected[index] != line { - t.Errorf("expected:%q, actual:%q", expected[index], line) - } - } // for - if readIndex < len(byteArray) { - t.Errorf("Byte buffer was only partially read. Buffer length is:%d, readIndex is:%d", len(byteArray), readIndex) - } - if index < len(expected) { - t.Errorf("All expected strings were not compared. expected arr length:%d, matched count:%d", len(expected), index-1) - } - } - - byteArray1 := []byte("\n Line 1 \n\n\n L ine4 \nLine 5 \n \n") - expected1 := []string{"", "Line 1", "", "", "L ine4", "Line 5", ""} - testFn(byteArray1, expected1) - - byteArray1 = []byte("") - expected1 = []string{} - testFn(byteArray1, expected1) - - byteArray1 = []byte("\n\n") - expected1 = []string{"", ""} - testFn(byteArray1, expected1) -} - func TestGetChainLines(t *testing.T) { iptablesSave := `# Generated by iptables-save v1.4.7 on Wed Oct 29 14:56:01 2014 *nat diff --git a/pkg/util/iptables/BUILD b/pkg/util/iptables/BUILD index 790aa79f17d..3e58721fe55 100644 --- a/pkg/util/iptables/BUILD +++ b/pkg/util/iptables/BUILD @@ -36,7 +36,10 @@ go_library( go_test( name = "go_default_test", - srcs = ["iptables_test.go"], + srcs = [ + "iptables_test.go", + "save_restore_test.go", + ], embed = [":go_default_library"], deps = select({ "@io_bazel_rules_go//go/platform:linux": [ diff --git a/pkg/util/iptables/save_restore.go b/pkg/util/iptables/save_restore.go index 6f4eacacab0..08671839f2d 100644 --- a/pkg/util/iptables/save_restore.go +++ b/pkg/util/iptables/save_restore.go @@ -17,10 +17,15 @@ limitations under the License. package iptables import ( + "bytes" "fmt" "strings" ) +var ( + commitBytes = []byte("COMMIT") +) + // MakeChainLine return an iptables-save/restore formatted chain line given a Chain func MakeChainLine(chain Chain) string { return fmt.Sprintf(":%s - [0:0]", chain) @@ -30,38 +35,41 @@ func MakeChainLine(chain Chain) string { // It returns a map of iptables.Chain to string where the string is the chain line from the save (with counters etc). func GetChainLines(table Table, save []byte) map[Chain]string { chainsMap := make(map[Chain]string) - tablePrefix := "*" + string(table) + tablePrefix := []byte("*" + string(table)) readIndex := 0 // find beginning of table for readIndex < len(save) { - line, n := ReadLine(readIndex, save) + line, n := readLine(readIndex, save) readIndex = n - if strings.HasPrefix(line, tablePrefix) { + if bytes.HasPrefix(line, tablePrefix) { break } } // parse table lines for readIndex < len(save) { - line, n := ReadLine(readIndex, save) + line, n := readLine(readIndex, save) readIndex = n if len(line) == 0 { continue } - if strings.HasPrefix(line, "COMMIT") || strings.HasPrefix(line, "*") { + if bytes.HasPrefix(line, commitBytes) || line[0] == '*' { break - } else if strings.HasPrefix(line, "#") { + } else if line[0] == '#' { continue - } else if strings.HasPrefix(line, ":") && len(line) > 1 { + } else if line[0] == ':' && len(line) > 1 { // We assume that the contains space - chain lines have 3 fields, // space delimited. If there is no space, this line will panic. - chain := Chain(line[1:strings.Index(line, " ")]) - chainsMap[chain] = line + // + // TODO: Try to avoid these allocations by reusing memory with the input. + lineString := string(line) + chain := Chain(line[1:strings.Index(lineString, " ")]) + chainsMap[chain] = lineString } } return chainsMap } -func ReadLine(readIndex int, byteArray []byte) (string, int) { +func readLine(readIndex int, byteArray []byte) ([]byte, int) { currentReadIndex := readIndex // consume left spaces @@ -89,7 +97,7 @@ func ReadLine(readIndex int, byteArray []byte) (string, int) { } else if (byteArray[currentReadIndex] == '\n') || (currentReadIndex == (len(byteArray) - 1)) { // end of line or byte buffer is reached if currentReadIndex <= leftTrimIndex { - return "", currentReadIndex + 1 + return nil, currentReadIndex + 1 } // set the rightTrimIndex if rightTrimIndex == -1 { @@ -100,11 +108,12 @@ func ReadLine(readIndex int, byteArray []byte) (string, int) { rightTrimIndex = currentReadIndex + 1 } } - return string(byteArray[leftTrimIndex:rightTrimIndex]), currentReadIndex + 1 + // Avoid unnecessary allocation. + return byteArray[leftTrimIndex:rightTrimIndex], currentReadIndex + 1 } else { // unset rightTrimIndex rightTrimIndex = -1 } } - return "", currentReadIndex + return nil, currentReadIndex } diff --git a/pkg/util/iptables/save_restore_test.go b/pkg/util/iptables/save_restore_test.go new file mode 100644 index 00000000000..d7c89d3e42f --- /dev/null +++ b/pkg/util/iptables/save_restore_test.go @@ -0,0 +1,53 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package iptables + +import ( + "testing" +) + +func TestReadLinesFromByteBuffer(t *testing.T) { + testFn := func(byteArray []byte, expected []string) { + index := 0 + readIndex := 0 + for ; readIndex < len(byteArray); index++ { + line, n := readLine(readIndex, byteArray) + readIndex = n + if expected[index] != string(line) { + t.Errorf("expected:%q, actual:%q", expected[index], line) + } + } // for + if readIndex < len(byteArray) { + t.Errorf("Byte buffer was only partially read. Buffer length is:%d, readIndex is:%d", len(byteArray), readIndex) + } + if index < len(expected) { + t.Errorf("All expected strings were not compared. expected arr length:%d, matched count:%d", len(expected), index-1) + } + } + + byteArray1 := []byte("\n Line 1 \n\n\n L ine4 \nLine 5 \n \n") + expected1 := []string{"", "Line 1", "", "", "L ine4", "Line 5", ""} + testFn(byteArray1, expected1) + + byteArray1 = []byte("") + expected1 = []string{} + testFn(byteArray1, expected1) + + byteArray1 = []byte("\n\n") + expected1 = []string{"", ""} + testFn(byteArray1, expected1) +}