Skip to content

Commit

Permalink
Simplify iptables-save parsing
Browse files Browse the repository at this point in the history 
We don't need to parse out the counter values from the iptables-save
output (since they are always 0 for the chains we care about). Just
parse the chain names themselves.

Also, all of the callers of GetChainLines() pass it input that
contains only a single table, so just assume that, rather than
carefully parsing only a single table's worth of the input.
  • Loading branch information
@danwinship
danwinship committed Jun 28, 2022
1 parent 4988699 commit 7c27cf0
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 182 deletions.
9 changes: 4 additions & 5 deletions pkg/proxy/iptables/proxier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ func CleanupLeftovers(ipt utiliptables.Interface) (encounteredError bool) {
klog.ErrorS(err, "Failed to execute iptables-save", "table", utiliptables.TableNAT)
encounteredError = true
} else {
existingNATChains := utiliptables.GetChainLines(utiliptables.TableNAT, iptablesData.Bytes())
existingNATChains := utiliptables.GetChainsFromTable(iptablesData.Bytes())
natChains := &utilproxy.LineBuffer{}
natRules := &utilproxy.LineBuffer{}
natChains.Write("*nat")
Expand Down Expand Up @@ -460,7 +460,7 @@ func CleanupLeftovers(ipt utiliptables.Interface) (encounteredError bool) {
klog.ErrorS(err, "Failed to execute iptables-save", "table", utiliptables.TableFilter)
encounteredError = true
} else {
existingFilterChains := utiliptables.GetChainLines(utiliptables.TableFilter, iptablesData.Bytes())
existingFilterChains := utiliptables.GetChainsFromTable(iptablesData.Bytes())
filterChains := &utilproxy.LineBuffer{}
filterRules := &utilproxy.LineBuffer{}
filterChains.Write("*filter")
Expand Down Expand Up @@ -890,14 +890,13 @@ func (proxier *Proxier) syncProxyRules() {

// Get iptables-save output so we can check for existing chains and rules.
// This will be a map of chain name to chain with rules as stored in iptables-save/iptables-restore
// IMPORTANT: existingNATChains may share memory with proxier.iptablesData.
existingNATChains := make(map[utiliptables.Chain][]byte)
existingNATChains := make(map[utiliptables.Chain]struct{})
proxier.iptablesData.Reset()
err := proxier.iptables.SaveInto(utiliptables.TableNAT, proxier.iptablesData)
if err != nil {
klog.ErrorS(err, "Failed to execute iptables-save: stale chains will not be deleted")
} else {
existingNATChains = utiliptables.GetChainLines(utiliptables.TableNAT, proxier.iptablesData.Bytes())
existingNATChains = utiliptables.GetChainsFromTable(proxier.iptablesData.Bytes())
}

// Reset all buffers used later.
Expand Down
104 changes: 17 additions & 87 deletions pkg/util/iptables/save_restore.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,102 +21,32 @@ import (
"fmt"
)

var (
commitBytes = []byte("COMMIT")
spaceBytes = []byte(" ")
)

// MakeChainLine return an iptables-save/restore formatted chain line given a Chain
func MakeChainLine(chain Chain) string {
return fmt.Sprintf(":%s - [0:0]", chain)
}

// GetChainLines parses a table's iptables-save data to find chains in the table.
// It returns a map of iptables.Chain to []byte where the []byte is the chain line
// from save (with counters etc.).
// Note that to avoid allocations memory is SHARED with save.
func GetChainLines(table Table, save []byte) map[Chain][]byte {
chainsMap := make(map[Chain][]byte)
tablePrefix := []byte("*" + string(table))
readIndex := 0
// find beginning of table
for readIndex < len(save) {
line, n := readLine(readIndex, save)
readIndex = n
if bytes.HasPrefix(line, tablePrefix) {
// GetChainsFromTable parses iptables-save data to find the chains that are defined. It
// assumes that save contains a single table's data, and returns a map with keys for every
// chain defined in that table.
func GetChainsFromTable(save []byte) map[Chain]struct{} {
chainsMap := make(map[Chain]struct{})

for {
i := bytes.Index(save, []byte("\n:"))
if i == -1 {
break
}
}
// parse table lines
for readIndex < len(save) {
line, n := readLine(readIndex, save)
readIndex = n
if len(line) == 0 {
continue
}
if bytes.HasPrefix(line, commitBytes) || line[0] == '*' {
start := i + 2
save = save[start:]
end := bytes.Index(save, []byte(" "))
if i == -1 {
// shouldn't happen, but...
break
} else if line[0] == '#' {
continue
} else if line[0] == ':' && len(line) > 1 {
// We assume that the <line> contains space - chain lines have 3 fields,
// space delimited. If there is no space, this line will panic.
spaceIndex := bytes.Index(line, spaceBytes)
if spaceIndex == -1 {
panic(fmt.Sprintf("Unexpected chain line in iptables-save output: %v", string(line)))
}
chain := Chain(line[1:spaceIndex])
chainsMap[chain] = line
}
chain := Chain(save[:end])
chainsMap[chain] = struct{}{}
save = save[end:]
}
return chainsMap
}

func readLine(readIndex int, byteArray []byte) ([]byte, int) {
currentReadIndex := readIndex

// consume left spaces
for currentReadIndex < len(byteArray) {
if byteArray[currentReadIndex] == ' ' {
currentReadIndex++
} else {
break
}
}

// leftTrimIndex stores the left index of the line after the line is left-trimmed
leftTrimIndex := currentReadIndex

// rightTrimIndex stores the right index of the line after the line is right-trimmed
// it is set to -1 since the correct value has not yet been determined.
rightTrimIndex := -1

for ; currentReadIndex < len(byteArray); currentReadIndex++ {
if byteArray[currentReadIndex] == ' ' {
// set rightTrimIndex
if rightTrimIndex == -1 {
rightTrimIndex = currentReadIndex
}
} else if (byteArray[currentReadIndex] == '\n') || (currentReadIndex == (len(byteArray) - 1)) {
// end of line or byte buffer is reached
if currentReadIndex <= leftTrimIndex {
return nil, currentReadIndex + 1
}
// set the rightTrimIndex
if rightTrimIndex == -1 {
rightTrimIndex = currentReadIndex
if currentReadIndex == (len(byteArray)-1) && (byteArray[currentReadIndex] != '\n') {
// ensure that the last character is part of the returned string,
// unless the last character is '\n'
rightTrimIndex = currentReadIndex + 1
}
}
// Avoid unnecessary allocation.
return byteArray[leftTrimIndex:rightTrimIndex], currentReadIndex + 1
} else {
// unset rightTrimIndex
rightTrimIndex = -1
}
}
return nil, currentReadIndex
}
118 changes: 28 additions & 90 deletions pkg/util/iptables/save_restore_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,73 +22,23 @@ import (
"github.com/lithammer/dedent"
)

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)
func checkChains(t *testing.T, save []byte, expected map[Chain]struct{}) {
chains := GetChainsFromTable(save)
for chain := range expected {
if _, exists := chains[chain]; !exists {
t.Errorf("GetChainsFromTable expected chain not present: %s", chain)
}
}

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 checkAllLines(t *testing.T, table Table, save []byte, expectedLines map[Chain]string) {
chainLines := GetChainLines(table, save)
for chain, lineBytes := range chainLines {
line := string(lineBytes)
if expected, exists := expectedLines[chain]; exists {
if expected != line {
t.Errorf("getChainLines expected chain line not present. For chain: %s Expected: %s Got: %s", chain, expected, line)
}
} else {
t.Errorf("getChainLines expected chain not present: %s", chain)
for chain := range chains {
if _, exists := expected[chain]; !exists {
t.Errorf("GetChainsFromTable chain unexpectedly present: %s", chain)
}
}
}

func TestGetChainLines(t *testing.T) {
iptablesSave := dedent.Dedent(
`# Generated by iptables-save v1.4.7 on Wed Oct 29 14:56:01 2014
*nat
:PREROUTING ACCEPT [2136997:197881818]
:POSTROUTING ACCEPT [4284525:258542680]
:OUTPUT ACCEPT [5901660:357267963]
-A PREROUTING -m addrtype --dst-type LOCAL -j DOCKER
COMMIT
# Completed on Wed Oct 29 14:56:01 2014`)
expected := map[Chain]string{
ChainPrerouting: ":PREROUTING ACCEPT [2136997:197881818]",
ChainPostrouting: ":POSTROUTING ACCEPT [4284525:258542680]",
ChainOutput: ":OUTPUT ACCEPT [5901660:357267963]",
}
checkAllLines(t, TableNAT, []byte(iptablesSave), expected)
}

func TestGetChainLinesMultipleTables(t *testing.T) {
iptablesSave := dedent.Dedent(
`# Generated by iptables-save v1.4.21 on Fri Aug 7 14:47:37 2015
func TestGetChainsFromTable(t *testing.T) {
iptablesSave := dedent.Dedent(`
# Generated by iptables-save v1.4.21 on Fri Aug 7 14:47:37 2015
*nat
:PREROUTING ACCEPT [2:138]
:INPUT ACCEPT [0:0]
Expand Down Expand Up @@ -126,35 +76,23 @@ func TestGetChainLinesMultipleTables(t *testing.T) {
-A KUBE-SVC-5555555555555555 -m comment --comment "default/kubernetes:" -j KUBE-SVC-4444444444444444
-A KUBE-SVC-6666666666666666 -m comment --comment "kube-system/kube-dns:dns" -j KUBE-SVC-1111111111111111
COMMIT
# Completed on Fri Aug 7 14:47:37 2015
# Generated by iptables-save v1.4.21 on Fri Aug 7 14:47:37 2015
*filter
:INPUT ACCEPT [17514:83115836]
:FORWARD ACCEPT [0:0]
:OUTPUT ACCEPT [8909:688225]
:DOCKER - [0:0]
-A FORWARD -o cbr0 -j DOCKER
-A FORWARD -o cbr0 -m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT
-A FORWARD -i cbr0 ! -o cbr0 -j ACCEPT
-A FORWARD -i cbr0 -o cbr0 -j ACCEPT
COMMIT
`)
expected := map[Chain]string{
ChainPrerouting: ":PREROUTING ACCEPT [2:138]",
Chain("INPUT"): ":INPUT ACCEPT [0:0]",
Chain("OUTPUT"): ":OUTPUT ACCEPT [0:0]",
ChainPostrouting: ":POSTROUTING ACCEPT [0:0]",
Chain("DOCKER"): ":DOCKER - [0:0]",
Chain("KUBE-NODEPORT-CONTAINER"): ":KUBE-NODEPORT-CONTAINER - [0:0]",
Chain("KUBE-NODEPORT-HOST"): ":KUBE-NODEPORT-HOST - [0:0]",
Chain("KUBE-PORTALS-CONTAINER"): ":KUBE-PORTALS-CONTAINER - [0:0]",
Chain("KUBE-PORTALS-HOST"): ":KUBE-PORTALS-HOST - [0:0]",
Chain("KUBE-SVC-1111111111111111"): ":KUBE-SVC-1111111111111111 - [0:0]",
Chain("KUBE-SVC-2222222222222222"): ":KUBE-SVC-2222222222222222 - [0:0]",
Chain("KUBE-SVC-3333333333333333"): ":KUBE-SVC-3333333333333333 - [0:0]",
Chain("KUBE-SVC-4444444444444444"): ":KUBE-SVC-4444444444444444 - [0:0]",
Chain("KUBE-SVC-5555555555555555"): ":KUBE-SVC-5555555555555555 - [0:0]",
Chain("KUBE-SVC-6666666666666666"): ":KUBE-SVC-6666666666666666 - [0:0]",
expected := map[Chain]struct{}{
ChainPrerouting: {},
Chain("INPUT"): {},
Chain("OUTPUT"): {},
ChainPostrouting: {},
Chain("DOCKER"): {},
Chain("KUBE-NODEPORT-CONTAINER"): {},
Chain("KUBE-NODEPORT-HOST"): {},
Chain("KUBE-PORTALS-CONTAINER"): {},
Chain("KUBE-PORTALS-HOST"): {},
Chain("KUBE-SVC-1111111111111111"): {},
Chain("KUBE-SVC-2222222222222222"): {},
Chain("KUBE-SVC-3333333333333333"): {},
Chain("KUBE-SVC-4444444444444444"): {},
Chain("KUBE-SVC-5555555555555555"): {},
Chain("KUBE-SVC-6666666666666666"): {},
}
checkAllLines(t, TableNAT, []byte(iptablesSave), expected)
checkChains(t, []byte(iptablesSave), expected)
}

0 comments on commit 7c27cf0

Please sign in to comment.