Skip to content

Commit 6084c3a

Browse files
champtarchamptar
committed
ipmasq: fix nftables backend
Add SetupIPMasqForNetworks and TeardownIPMasqForNetworks functions that take []*net.IPNet instead of *net.IPNet. This allow the nftables backend to cleanup stale rules and recreate all needed rules in a single transaction, where previously the stale rules cleanup was breaking all but the last IPNet. Fixes 61d0786 Signed-off-by: Etienne Champetier <[email protected]>
1 parent fec2d62 commit 6084c3a

File tree

6 files changed

+88
-54
lines changed

6 files changed

+88
-54
lines changed

‎pkg/ip/ipmasq_iptables_linux.go‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,33 @@
1515
package ip
1616

1717
import (
18+
"errors"
1819
"fmt"
1920
"net"
21+
"strings"
2022

2123
"github.com/coreos/go-iptables/iptables"
2224

2325
"github.com/containernetworking/cni/pkg/types"
2426
"github.com/containernetworking/plugins/pkg/utils"
2527
)
2628

27-
// setupIPMasqIPTables is the iptables-based implementation of SetupIPMasqForNetwork
28-
func setupIPMasqIPTables(ipn *net.IPNet, network, _, containerID string) error {
29+
// setupIPMasqIPTables is the iptables-based implementation of SetupIPMasqForNetworks
30+
func setupIPMasqIPTables(ipns []*net.IPNet, network, _, containerID string) error {
2931
// Note: for historical reasons, the iptables implementation ignores ifname.
3032
chain := utils.FormatChainName(network, containerID)
3133
comment := utils.FormatComment(network, containerID)
32-
return SetupIPMasq(ipn, chain, comment)
34+
for _, ip := range ipns {
35+
if err := SetupIPMasq(ip, chain, comment); err != nil {
36+
return err
37+
}
38+
}
39+
return nil
3340
}
3441

3542
// SetupIPMasq installs iptables rules to masquerade traffic
3643
// coming from ip of ipn and going outside of ipn.
37-
// Deprecated: This function only supports iptables. Use SetupIPMasqForNetwork, which
44+
// Deprecated: This function only supports iptables. Use SetupIPMasqForNetworks, which
3845
// supports both iptables and nftables.
3946
func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error {
4047
isV6 := ipn.IP.To4() == nil
@@ -87,12 +94,24 @@ func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error {
8794
return ipt.AppendUnique("nat", "POSTROUTING", "-s", ipn.IP.String(), "-j", chain, "-m", "comment", "--comment", comment)
8895
}
8996

90-
// teardownIPMasqIPTables is the iptables-based implementation of TeardownIPMasqForNetwork
91-
func teardownIPMasqIPTables(ipn *net.IPNet, network, _, containerID string) error {
97+
// teardownIPMasqIPTables is the iptables-based implementation of TeardownIPMasqForNetworks
98+
func teardownIPMasqIPTables(ipns []*net.IPNet, network, _, containerID string) error {
9299
// Note: for historical reasons, the iptables implementation ignores ifname.
93100
chain := utils.FormatChainName(network, containerID)
94101
comment := utils.FormatComment(network, containerID)
95-
return TeardownIPMasq(ipn, chain, comment)
102+
103+
var errs []string
104+
for _, ipn := range ipns {
105+
err := TeardownIPMasq(ipn, chain, comment)
106+
if err != nil {
107+
errs = append(errs, err.Error())
108+
}
109+
}
110+
111+
if errs == nil {
112+
return nil
113+
}
114+
return errors.New(strings.Join(errs, "\n"))
96115
}
97116

98117
// TeardownIPMasq undoes the effects of SetupIPMasq.

‎pkg/ip/ipmasq_linux.go‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,16 @@ import (
2424
"github.com/containernetworking/plugins/pkg/utils"
2525
)
2626

27-
// SetupIPMasqForNetwork installs rules to masquerade traffic coming from ip of ipn and
28-
// going outside of ipn, using a chain name based on network, ifname, and containerID. The
27+
// Deprecated: use SetupIPMasqForNetworks
28+
func SetupIPMasqForNetwork(backend *string, ipn *net.IPNet, network, ifname, containerID string) error {
29+
return SetupIPMasqForNetworks(backend, []*net.IPNet{ipn}, network, ifname, containerID)
30+
}
31+
32+
// SetupIPMasqForNetworks installs rules to masquerade traffic coming from ips of ipns and
33+
// going outside of ipns, using a chain name based on network, ifname, and containerID. The
2934
// backend can be either "iptables" or "nftables"; if it is nil, then a suitable default
3035
// implementation will be used.
31-
func SetupIPMasqForNetwork(backend *string, ipn *net.IPNet, network, ifname, containerID string) error {
36+
func SetupIPMasqForNetworks(backend *string, ipns []*net.IPNet, network, ifname, containerID string) error {
3237
if backend == nil {
3338
// Prefer iptables, unless only nftables is available
3439
defaultBackend := "iptables"
@@ -40,27 +45,32 @@ func SetupIPMasqForNetwork(backend *string, ipn *net.IPNet, network, ifname, con
4045

4146
switch *backend {
4247
case "iptables":
43-
return setupIPMasqIPTables(ipn, network, ifname, containerID)
48+
return setupIPMasqIPTables(ipns, network, ifname, containerID)
4449
case "nftables":
45-
return setupIPMasqNFTables(ipn, network, ifname, containerID)
50+
return setupIPMasqNFTables(ipns, network, ifname, containerID)
4651
default:
4752
return fmt.Errorf("unknown ipmasq backend %q", *backend)
4853
}
4954
}
5055

51-
// TeardownIPMasqForNetwork undoes the effects of SetupIPMasqForNetwork
56+
// Deprecated: use TeardownIPMasqForNetworks
5257
func TeardownIPMasqForNetwork(ipn *net.IPNet, network, ifname, containerID string) error {
58+
return TeardownIPMasqForNetworks([]*net.IPNet{ipn}, network, ifname, containerID)
59+
}
60+
61+
// TeardownIPMasqForNetworks undoes the effects of SetupIPMasqForNetworks
62+
func TeardownIPMasqForNetworks(ipns []*net.IPNet, network, ifname, containerID string) error {
5363
var errs []string
5464

5565
// Do both the iptables and the nftables cleanup, since the pod may have been
5666
// created with a different version of this plugin or a different configuration.
5767

58-
err := teardownIPMasqIPTables(ipn, network, ifname, containerID)
68+
err := teardownIPMasqIPTables(ipns, network, ifname, containerID)
5969
if err != nil && utils.SupportsIPTables() {
6070
errs = append(errs, err.Error())
6171
}
6272

63-
err = teardownIPMasqNFTables(ipn, network, ifname, containerID)
73+
err = teardownIPMasqNFTables(ipns, network, ifname, containerID)
6474
if err != nil && utils.SupportsNFTables() {
6575
errs = append(errs, err.Error())
6676
}

‎pkg/ip/ipmasq_nftables_linux.go‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,16 @@ func commentForInstance(network, ifname, containerID string) string {
7272
return comment
7373
}
7474

75-
// setupIPMasqNFTables is the nftables-based implementation of SetupIPMasqForNetwork
76-
func setupIPMasqNFTables(ipn *net.IPNet, network, ifname, containerID string) error {
75+
// setupIPMasqNFTables is the nftables-based implementation of SetupIPMasqForNetworks
76+
func setupIPMasqNFTables(ipns []*net.IPNet, network, ifname, containerID string) error {
7777
nft, err := knftables.New(knftables.InetFamily, ipMasqTableName)
7878
if err != nil {
7979
return err
8080
}
81-
return setupIPMasqNFTablesWithInterface(nft, ipn, network, ifname, containerID)
81+
return setupIPMasqNFTablesWithInterface(nft, ipns, network, ifname, containerID)
8282
}
8383

84-
func setupIPMasqNFTablesWithInterface(nft knftables.Interface, ipn *net.IPNet, network, ifname, containerID string) error {
84+
func setupIPMasqNFTablesWithInterface(nft knftables.Interface, ipns []*net.IPNet, network, ifname, containerID string) error {
8585
staleRules, err := findRules(nft, hashForInstance(network, ifname, containerID))
8686
if err != nil {
8787
return err
@@ -128,37 +128,39 @@ func setupIPMasqNFTablesWithInterface(nft knftables.Interface, ipn *net.IPNet, n
128128
for _, rule := range staleRules {
129129
tx.Delete(rule)
130130
}
131-
ip := "ip"
132-
if ipn.IP.To4() == nil {
133-
ip = "ip6"
134-
}
135-
136-
// e.g. if ipn is "192.168.1.4/24", then dstNet is "192.168.1.0/24"
137-
dstNet := &net.IPNet{IP: ipn.IP.Mask(ipn.Mask), Mask: ipn.Mask}
131+
for _, ipn := range ipns {
132+
ip := "ip"
133+
if ipn.IP.To4() == nil {
134+
ip = "ip6"
135+
}
138136

139-
tx.Add(&knftables.Rule{
140-
Chain: ipMasqChainName,
141-
Rule: knftables.Concat(
142-
ip, "saddr", "==", ipn.IP,
143-
ip, "daddr", "!=", dstNet,
144-
"masquerade",
145-
),
146-
Comment: knftables.PtrTo(commentForInstance(network, ifname, containerID)),
147-
})
137+
// e.g. if ipn is "192.168.1.4/24", then dstNet is "192.168.1.0/24"
138+
dstNet := &net.IPNet{IP: ipn.IP.Mask(ipn.Mask), Mask: ipn.Mask}
139+
140+
tx.Add(&knftables.Rule{
141+
Chain: ipMasqChainName,
142+
Rule: knftables.Concat(
143+
ip, "saddr", "==", ipn.IP,
144+
ip, "daddr", "!=", dstNet,
145+
"masquerade",
146+
),
147+
Comment: knftables.PtrTo(commentForInstance(network, ifname, containerID)),
148+
})
149+
}
148150

149151
return nft.Run(context.TODO(), tx)
150152
}
151153

152-
// teardownIPMasqNFTables is the nftables-based implementation of TeardownIPMasqForNetwork
153-
func teardownIPMasqNFTables(ipn *net.IPNet, network, ifname, containerID string) error {
154+
// teardownIPMasqNFTables is the nftables-based implementation of TeardownIPMasqForNetworks
155+
func teardownIPMasqNFTables(ipns []*net.IPNet, network, ifname, containerID string) error {
154156
nft, err := knftables.New(knftables.InetFamily, ipMasqTableName)
155157
if err != nil {
156158
return err
157159
}
158-
return teardownIPMasqNFTablesWithInterface(nft, ipn, network, ifname, containerID)
160+
return teardownIPMasqNFTablesWithInterface(nft, ipns, network, ifname, containerID)
159161
}
160162

161-
func teardownIPMasqNFTablesWithInterface(nft knftables.Interface, _ *net.IPNet, network, ifname, containerID string) error {
163+
func teardownIPMasqNFTablesWithInterface(nft knftables.Interface, _ []*net.IPNet, network, ifname, containerID string) error {
162164
rules, err := findRules(nft, hashForInstance(network, ifname, containerID))
163165
if err != nil {
164166
return err

‎pkg/ip/ipmasq_nftables_linux_test.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
package ip
1616

1717
import (
18+
"net"
1819
"strings"
1920
"testing"
2021

@@ -64,7 +65,7 @@ func Test_setupIPMasqNFTables(t *testing.T) {
6465
if err != nil {
6566
t.Fatalf("failed to parse test addr: %v", err)
6667
}
67-
err = setupIPMasqNFTablesWithInterface(nft, addr.IPNet, c.network, c.ifname, c.containerID)
68+
err = setupIPMasqNFTablesWithInterface(nft, []*net.IPNet{addr.IPNet}, c.network, c.ifname, c.containerID)
6869
if err != nil {
6970
t.Fatalf("error from setupIPMasqNFTables: %v", err)
7071
}
@@ -92,7 +93,7 @@ add rule inet cni_plugins_masquerade postrouting goto masq_checks
9293
if err != nil {
9394
t.Fatalf("failed to parse test addr: %v", err)
9495
}
95-
err = setupIPMasqNFTablesWithInterface(nft, addr.IPNet, "unit-test", "eth0", "four")
96+
err = setupIPMasqNFTablesWithInterface(nft, []*net.IPNet{addr.IPNet}, "unit-test", "eth0", "four")
9697
if err != nil {
9798
t.Fatalf("error from setupIPMasqNFTables: %v", err)
9899
}
@@ -103,7 +104,7 @@ add rule inet cni_plugins_masquerade postrouting goto masq_checks
103104
if err != nil {
104105
t.Fatalf("failed to parse test addr: %v", err)
105106
}
106-
err = teardownIPMasqNFTablesWithInterface(nft, addr.IPNet, c.network, c.ifname, c.containerID)
107+
err = teardownIPMasqNFTablesWithInterface(nft, []*net.IPNet{addr.IPNet}, c.network, c.ifname, c.containerID)
107108
if err != nil {
108109
t.Fatalf("error from teardownIPMasqNFTables: %v", err)
109110
}

‎plugins/main/bridge/bridge.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -668,10 +668,12 @@ func cmdAdd(args *skel.CmdArgs) error {
668668
}
669669

670670
if n.IPMasq {
671+
ipns := []*net.IPNet{}
671672
for _, ipc := range result.IPs {
672-
if err = ip.SetupIPMasqForNetwork(n.IPMasqBackend, &ipc.Address, n.Name, args.IfName, args.ContainerID); err != nil {
673-
return err
674-
}
673+
ipns = append(ipns, &ipc.Address)
674+
}
675+
if err = ip.SetupIPMasqForNetworks(n.IPMasqBackend, ipns, n.Name, args.IfName, args.ContainerID); err != nil {
676+
return err
675677
}
676678
}
677679
} else if !n.DisableContainerInterface {
@@ -807,10 +809,8 @@ func cmdDel(args *skel.CmdArgs) error {
807809
}
808810

809811
if isLayer3 && n.IPMasq {
810-
for _, ipn := range ipnets {
811-
if err := ip.TeardownIPMasqForNetwork(ipn, n.Name, args.IfName, args.ContainerID); err != nil {
812-
return err
813-
}
812+
if err := ip.TeardownIPMasqForNetworks(ipnets, n.Name, args.IfName, args.ContainerID); err != nil {
813+
return err
814814
}
815815
}
816816

‎plugins/main/ptp/ptp.go‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,12 @@ func cmdAdd(args *skel.CmdArgs) error {
229229
}
230230

231231
if conf.IPMasq {
232+
ipns := []*net.IPNet{}
232233
for _, ipc := range result.IPs {
233-
if err = ip.SetupIPMasqForNetwork(conf.IPMasqBackend, &ipc.Address, conf.Name, args.IfName, args.ContainerID); err != nil {
234-
return err
235-
}
234+
ipns = append(ipns, &ipc.Address)
235+
}
236+
if err = ip.SetupIPMasqForNetworks(conf.IPMasqBackend, ipns, conf.Name, args.IfName, args.ContainerID); err != nil {
237+
return err
236238
}
237239
}
238240

@@ -291,8 +293,8 @@ func cmdDel(args *skel.CmdArgs) error {
291293
}
292294

293295
if len(ipnets) != 0 && conf.IPMasq {
294-
for _, ipn := range ipnets {
295-
err = ip.TeardownIPMasqForNetwork(ipn, conf.Name, args.IfName, args.ContainerID)
296+
if err := ip.TeardownIPMasqForNetworks(ipnets, conf.Name, args.IfName, args.ContainerID); err != nil {
297+
return err
296298
}
297299
}
298300

0 commit comments

Comments
 (0)