Merge pull request #421 from yanjianbo1983/improve-agent-iptables

Improve agent iptables
FabEdge · Oct 19, 2023 · ce31ff0 · ce31ff0
2 parents 33c76b6 + 80fdb51
commit ce31ff0
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 92 deletions.
diff --git a/pkg/agent/iptables.go b/pkg/agent/iptables.go
@@ -15,8 +15,11 @@
 package agent
 
 import (
+	"bytes"
 	"strings"
+	"text/template"
 
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 
 	"github.com/fabedge/fabedge/pkg/util/ipset"
@@ -28,57 +31,39 @@ type IPSet struct {
 	EntrySet sets.String
 }
 
-var jumpChains = []iptables.JumpChain{
-	{Table: iptables.TableFilter, SrcChain: iptables.ChainForward, DstChain: iptables.ChainFabEdgeForward, Position: iptables.Append},
-	{Table: iptables.TableNat, SrcChain: iptables.ChainPostRouting, DstChain: iptables.ChainFabEdgeNatOutgoing, Position: iptables.Prepend},
-}
-
-func buildRuleData(ipsetName string, subnets []string) []byte {
-	var builder strings.Builder
-	builder.WriteString(`
+var tmpl = template.Must(template.New("iptables").Parse(`
 *filter
 :FABEDGE-FORWARD - [0:0]
-
-`)
-
-	for _, subnet := range subnets {
-		builder.WriteString("-A FABEDGE-FORWARD -s ")
-		builder.WriteString(subnet)
-		builder.WriteString(" -j ACCEPT\n")
-
-		builder.WriteString("-A FABEDGE-FORWARD -d ")
-		builder.WriteString(subnet)
-		builder.WriteString(" -j ACCEPT\n")
-	}
-
-	builder.WriteString(`COMMIT
+{{- range $cidr := .edgePodCIDRs }}
+-A FABEDGE-FORWARD -s {{ $cidr }} -j ACCEPT
+-A FABEDGE-FORWARD -d {{ $cidr }} -j ACCEPT
+{{- end }}
+COMMIT
 
 *nat
-:FABEDGE-NAT-OUTGOING - [0:0]
+:FABEDGE-POSTROUTING - [0:0]
+{{- range $cidr := .edgePodCIDRs }}
+-A FABEDGE-POSTROUTING -s {{ $cidr }} -m set --match-set {{ $.ipsetName }} dst -j RETURN
+-A FABEDGE-POSTROUTING -s {{ $cidr }} -d {{ $cidr }} -j RETURN
+-A FABEDGE-POSTROUTING -s {{ $cidr }} -j MASQUERADE
+{{- end }}
+COMMIT
+`))
 
-`)
+var jumpChains = []iptables.JumpChain{
+	{Table: iptables.TableFilter, SrcChain: iptables.ChainForward, DstChain: iptables.ChainFabEdgeForward, Position: iptables.Append},
+	{Table: iptables.TableNat, SrcChain: iptables.ChainPostRouting, DstChain: iptables.ChainFabEdgePostRouting, Position: iptables.Prepend},
+}
 
-	for _, subnet := range subnets {
-		builder.WriteString("-A FABEDGE-NAT-OUTGOING -s ")
-		builder.WriteString(subnet)
-		builder.WriteString(" -m set --match-set ")
-		builder.WriteString(ipsetName)
-		builder.WriteString(" dst -j RETURN\n")
-
-		builder.WriteString("-A FABEDGE-NAT-OUTGOING -s ")
-		builder.WriteString(subnet)
-		builder.WriteString(" -d ")
-		builder.WriteString(subnet)
-		builder.WriteString(" -j RETURN\n")
-
-		builder.WriteString("-A FABEDGE-NAT-OUTGOING -s ")
-		builder.WriteString(subnet)
-		builder.WriteString(" -j MASQUERADE\n")
-	}
+func buildRuleData(ipsetName string, edgePodCIDRs []string) []byte {
+	buf := bytes.NewBuffer(nil)
 
-	builder.WriteString("COMMIT\n")
+	_ = tmpl.Execute(buf, map[string]interface{}{
+		"ipsetName":    ipsetName,
+		"edgePodCIDRs": edgePodCIDRs,
+	})
 
-	return []byte(builder.String())
+	return buf.Bytes()
 }
 
 func (m *Manager) ensureIPTablesRules() error {
@@ -87,66 +72,44 @@ func (m *Manager) ensureIPTablesRules() error {
 	peerIPSet4, peerIPSet6 := m.getAllPeerCIDRs()
 	subnetsIP4, subnetsIP6 := classifySubnets(current.Subnets)
 
+	if !areSubnetsEqual(current.Subnets, m.lastSubnets) {
+		m.ipt = iptables.NewApplierCleaner(iptables.ProtocolIPv4, jumpChains, buildRuleData(IPSetFabEdgePeerCIDR, subnetsIP4))
+		m.ipt6 = iptables.NewApplierCleaner(iptables.ProtocolIPv6, jumpChains, buildRuleData(IPSetFabEdgePeerCIDR6, subnetsIP6))
+		m.lastSubnets = current.Subnets
+	}
+
 	configs := []struct {
-		peerIPSet     IPSet
-		loopbackIPSet IPSet
-		subnets       []string
-		helper        iptables.ApplierCleaner
+		name       string
+		hashFamily string
+		peerIPSet  sets.String
+		ipt        iptables.ApplierCleaner
 	}{
-		{
-			peerIPSet: IPSet{
-				IPSet: &ipset.IPSet{
-					Name:       IPSetFabEdgePeerCIDR,
-					SetType:    ipset.HashNet,
-					HashFamily: ipset.ProtocolFamilyIPV4,
-				},
-				EntrySet: peerIPSet4,
-			},
-			subnets: subnetsIP4,
-			helper:  iptables.NewApplierCleaner(iptables.ProtocolIPv4, jumpChains, buildRuleData(IPSetFabEdgePeerCIDR, subnetsIP4)),
-		},
-		{
-			peerIPSet: IPSet{
-				IPSet: &ipset.IPSet{
-					Name:       IPSetFabEdgePeerCIDR6,
-					SetType:    ipset.HashNet,
-					HashFamily: ipset.ProtocolFamilyIPV6,
-				},
-				EntrySet: peerIPSet6,
-			},
-			subnets: subnetsIP6,
-			helper:  iptables.NewApplierCleaner(iptables.ProtocolIPv6, jumpChains, buildRuleData(IPSetFabEdgePeerCIDR6, subnetsIP6)),
-		},
+		{IPSetFabEdgePeerCIDR, ipset.ProtocolFamilyIPV4, peerIPSet4, m.ipt},
+		{IPSetFabEdgePeerCIDR6, ipset.ProtocolFamilyIPV6, peerIPSet6, m.ipt6},
 	}
 
+	var errors []error
 	for _, c := range configs {
-		if err := m.ipset.EnsureIPSet(c.peerIPSet.IPSet, c.peerIPSet.EntrySet); err != nil {
-			m.log.Error(err, "failed to sync ipset", "ipsetName", c.peerIPSet.IPSet.Name)
-			return err
+		ipSet := &ipset.IPSet{
+			Name:       c.name,
+			HashFamily: c.hashFamily,
+			SetType:    ipset.HashNet,
 		}
 
-		if err := c.helper.Apply(); err != nil {
+		if err := m.ipset.EnsureIPSet(ipSet, c.peerIPSet); err != nil {
+			m.log.Error(err, "failed to sync ipset", "ipsetName", c.name)
+			errors = append(errors, err)
+		}
+
+		if err := c.ipt.Apply(); err != nil {
 			m.log.Error(err, "failed to sync iptables rules")
+			errors = append(errors, err)
 		} else {
 			m.log.V(5).Info("iptables rules is synced")
 		}
 	}
 
-	return nil
-}
-
-func (m *Manager) areSubnetsEqual(sa1, sa2 []string) bool {
-	if len(sa1) != len(sa2) {
-		return false
-	}
-
-	for i := range sa1 {
-		if sa1[i] != sa2[i] {
-			return false
-		}
-	}
-
-	return true
+	return utilerrors.NewAggregate(errors)
 }
 
 func (m *Manager) getAllPeerCIDRs() (cidrSet4, cidrSet6 sets.String) {
@@ -188,3 +151,17 @@ func classifySubnets(subnets []string) (ipv4, ipv6 []string) {
 func isIPv6(addr string) bool {
 	return strings.Index(addr, ":") != -1
 }
+
+func areSubnetsEqual(sa1, sa2 []string) bool {
+	if len(sa1) != len(sa2) {
+		return false
+	}
+
+	for i := range sa1 {
+		if sa1[i] != sa2[i] {
+			return false
+		}
+	}
+
+	return true
+}
diff --git a/pkg/agent/manager.go b/pkg/agent/manager.go
@@ -30,6 +30,7 @@ import (
 	apis "github.com/fabedge/fabedge/pkg/apis/v1alpha1"
 	"github.com/fabedge/fabedge/pkg/tunnel"
 	"github.com/fabedge/fabedge/pkg/util/ipset"
+	"github.com/fabedge/fabedge/pkg/util/iptables"
 	netutil "github.com/fabedge/fabedge/pkg/util/net"
 	routeutil "github.com/fabedge/fabedge/pkg/util/route"
 	"github.com/fabedge/fabedge/third_party/ipvs"
@@ -42,9 +43,12 @@ const (
 
 type Manager struct {
 	Config
+
 	netLink ipvs.NetLinkHandle
 	ipvs    ipvs.Interface
 	ipset   ipset.Interface
+	ipt     iptables.ApplierCleaner
+	ipt6    iptables.ApplierCleaner
 
 	tm  tunnel.Manager
 	log logr.Logger
@@ -55,7 +59,7 @@ type Manager struct {
 	// endpointLock is used to protect currentEndpoint and peerEndpoints
 	endpointLock sync.RWMutex
 
-	// lastSubnets is used to determine whether to clear chain FABEDGE-NAT-OUTGOING
+	// lastSubnets is used to determine if current node's pod CIDR are changed
 	lastSubnets []string
 
 	events   chan struct{}

diff --git a/pkg/util/iptables/iptables.go b/pkg/util/iptables/iptables.go
@@ -46,7 +46,6 @@ const (
 	ChainFabEdgeInput       Chain = "FABEDGE-INPUT"
 	ChainFabEdgeForward     Chain = "FABEDGE-FORWARD"
 	ChainFabEdgePostRouting Chain = "FABEDGE-POSTROUTING"
-	ChainFabEdgeNatOutgoing Chain = "FABEDGE-NAT-OUTGOING"
 )
 
 type RulePosition = utiliptables.RulePosition