Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated backport of #3187: Fix error deleting TCPMSS clamp rule in route agent #3189

Merged
merged 2 commits into from
Oct 17, 2024
Merged

Automated backport of #3187: Fix error deleting TCPMSS clamp rule in route agent #3189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
265e25b
Fix error deleting TCPMSS clamp rule in route agent
tpantelis Oct 17, 2024
dbe6c01
Merge branch 'release-0.19' into automated-backport-of-#3187-upstream…
aswinsuryan Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 8 additions & 27 deletions pkg/packetfilter/iptables/iptables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ func mssClampToRuleSpec(ruleSpec *RuleSpec, clampType packetfilter.MssClampType,
switch clampType {
case packetfilter.UndefinedMSS:
case packetfilter.ToPMTU:
*ruleSpec = append(*ruleSpec, "-p", "tcp", "-m", "tcp", "--tcp-flags", "SYN,RST", "SYN", "--clamp-mss-to-pmtu")
*ruleSpec = append(*ruleSpec, "--clamp-mss-to-pmtu", "-p", "tcp", "-m", "tcp", "--tcp-flags", "SYN,RST", "SYN")
case packetfilter.ToValue:
*ruleSpec = append(*ruleSpec, "-p", "tcp", "-m", "tcp", "--tcp-flags", "SYN,RST", "SYN", "--set-mss", mssValue)
*ruleSpec = append(*ruleSpec, "--set-mss", mssValue, "-p", "tcp", "-m", "tcp", "--tcp-flags", "SYN,RST", "SYN")
}
}

Expand Down Expand Up @@ -322,6 +322,7 @@ func ToRuleSpec(rule *packetfilter.Rule) RuleSpec {
return ruleSpec
}

//nolint:gocyclo // This function has a lot of small case statements so ignore cyclomatic complexity.
func FromRuleSpec(spec []string) *packetfilter.Rule {
rule := &packetfilter.Rule{}

Expand Down Expand Up @@ -350,6 +351,11 @@ func FromRuleSpec(spec []string) *packetfilter.Rule {
rule.DPort, i = parseNextTerm(spec, i, noopParse)
case "--set-mark":
rule.MarkValue, i = parseNextTerm(spec, i, parseMark)
case "--clamp-mss-to-pmtu":
rule.ClampType = packetfilter.ToPMTU
case "--set-mss":
rule.MssValue, i = parseNextTerm(spec, i, noopParse)
rule.ClampType = packetfilter.ToValue
case "-j":
rule.Action, i = parseNextTerm(spec, i, parseAction)
if rule.Action == packetfilter.RuleActionJump {
Expand Down Expand Up @@ -400,31 +406,6 @@ func parseRuleMatch(spec []string, i int, rule *packetfilter.Rule) int {

i += 3
}
case "tcp":
// Parses the form: "-m", "tcp", "--tcp-flags", "SYN,RST", "SYN", "--clamp-mss-to-pmtu"
i = parseTCPSpec(spec, i, rule)
}

return i
}

func parseTCPSpec(spec []string, i int, rule *packetfilter.Rule) int {
i++
for i < len(spec) {
if spec[i] == "--clamp-mss-to-pmtu" {
rule.ClampType = packetfilter.ToPMTU
break
} else if spec[i] == "--set-mss" {
rule.MssValue, i = parseNextTerm(spec, i, noopParse)
rule.ClampType = packetfilter.ToValue

break
} else if !strings.HasPrefix(spec[i], "--") && strings.HasPrefix(spec[i], "-") {
i--
break
}

i++
}

return i
Expand Down
27 changes: 24 additions & 3 deletions pkg/packetfilter/iptables/iptables_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ limitations under the License.
package iptables_test

import (
"strings"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/submariner-io/submariner/pkg/packetfilter"
Expand All @@ -27,23 +29,31 @@ import (

var _ = Describe("Rule conversion", func() {
Specify("should correctly convert to and from a rule spec string", func() {
// -m set --match-set src-set src -m set --match-set dest-set dst -j TCPMSS -p tcp -m tcp --tcp-flags SYN,RST SYN --clamp-mss-to-pmtu
// -m set --match-set src-set src -m set --match-set dest-set dst -j TCPMSS --clamp-mss-to-pmtu -p tcp -m tcp --tcp-flags SYN,RST SYN
testRuleConversion(&packetfilter.Rule{
SrcSetName: "src-set",
DestSetName: "dest-set",
Action: packetfilter.RuleActionMss,
ClampType: packetfilter.ToPMTU,
})

// -m set --match-set src-set src -m set --match-set dest-set dst -j TCPMSS -p tcp -m tcp --tcp-flags SYN,RST SYN --set-mss mss-value
// -m set --match-set src-set src -m set --match-set dest-set dst -j TCPMSS --set-mss mss-value -p tcp -m tcp --tcp-flags SYN,RST SYN
testRuleConversion(&packetfilter.Rule{
SrcSetName: "src-set",
DestSetName: "dest-set",
MssValue: "mss-value",
MssValue: "1500",
Action: packetfilter.RuleActionMss,
ClampType: packetfilter.ToValue,
})

// -s 1.2.3.4/32 -j TCPMSS --set-mss mss-value -p tcp -m tcp --tcp-flags SYN,RST SYN
testRuleConversion(&packetfilter.Rule{
SrcCIDR: "1.2.3.4/32",
MssValue: "1500",
Action: packetfilter.RuleActionMss,
ClampType: packetfilter.ToValue,
})

// -p udp -m udp -s 171.254.1.0/24 -d 170.254.1.0/24 -o out-iface -i in-iface --dport d-port -j ACCEPT
testRuleConversion(&packetfilter.Rule{
Proto: packetfilter.RuleProtoUDP,
Expand Down Expand Up @@ -94,6 +104,17 @@ var _ = Describe("Rule conversion", func() {
TargetChain: "target-chain",
Action: packetfilter.RuleActionJump,
})

// The actual iptables command returns the TCPMSS rule parts in a different order than we write it out so ensure we
// can parse it correctly.
rs := "-s 1.2.3.4/32 -p tcp -m tcp --tcp-flags SYN,RST SYN -j TCPMSS --set-mss 1500"
parsed := iptables.FromRuleSpec(strings.Split(rs, " "))
Expect(parsed).To(Equal(&packetfilter.Rule{
SrcCIDR: "1.2.3.4/32",
Action: packetfilter.RuleActionMss,
ClampType: packetfilter.ToValue,
MssValue: "1500",
}))
})
})

Expand Down
Loading