Skip to content

Commit 1f31269

Browse files
champtarchamptar
committed
ipmasq: fix nftables backend
Rename SetupIPMasqForNetwork -> SetupIPMasqForNetworks TeardownIPMasqForNetwork -> TeardownIPMasqForNetworks and have them 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 9296c5f commit 1f31269

File tree

6 files changed

+101
-70
lines changed

6 files changed

+101
-70
lines changed

‎pkg/ip/ipmasq_iptables_linux.go‎

Lines changed: 27 additions & 8 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,16 +94,28 @@ 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.
99-
// Deprecated: This function only supports iptables. Use TeardownIPMasqForNetwork, which
118+
// Deprecated: This function only supports iptables. Use TeardownIPMasqForNetworks, which
100119
// supports both iptables and nftables.
101120
func TeardownIPMasq(ipn *net.IPNet, chain string, comment string) error {
102121
isV6 := ipn.IP.To4() == nil

‎pkg/ip/ipmasq_linux.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@ 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+
// SetupIPMasqForNetworks installs rules to masquerade traffic coming from ips of ipns and
28+
// going outside of ipns, using a chain name based on network, ifname, and containerID. The
2929
// backend can be either "iptables" or "nftables"; if it is nil, then a suitable default
3030
// implementation will be used.
31-
func SetupIPMasqForNetwork(backend *string, ipn *net.IPNet, network, ifname, containerID string) error {
31+
func SetupIPMasqForNetworks(backend *string, ipns []*net.IPNet, network, ifname, containerID string) error {
3232
if backend == nil {
3333
// Prefer iptables, unless only nftables is available
3434
defaultBackend := "iptables"
@@ -40,27 +40,27 @@ func SetupIPMasqForNetwork(backend *string, ipn *net.IPNet, network, ifname, con
4040

4141
switch *backend {
4242
case "iptables":
43-
return setupIPMasqIPTables(ipn, network, ifname, containerID)
43+
return setupIPMasqIPTables(ipns, network, ifname, containerID)
4444
case "nftables":
45-
return setupIPMasqNFTables(ipn, network, ifname, containerID)
45+
return setupIPMasqNFTables(ipns, network, ifname, containerID)
4646
default:
4747
return fmt.Errorf("unknown ipmasq backend %q", *backend)
4848
}
4949
}
5050

51-
// TeardownIPMasqForNetwork undoes the effects of SetupIPMasqForNetwork
52-
func TeardownIPMasqForNetwork(ipn *net.IPNet, network, ifname, containerID string) error {
51+
// TeardownIPMasqForNetworks undoes the effects of SetupIPMasqForNetworks
52+
func TeardownIPMasqForNetworks(ipns []*net.IPNet, network, ifname, containerID string) error {
5353
var errs []string
5454

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

58-
err := teardownIPMasqIPTables(ipn, network, ifname, containerID)
58+
err := teardownIPMasqIPTables(ipns, network, ifname, containerID)
5959
if err != nil && utils.SupportsIPTables() {
6060
errs = append(errs, err.Error())
6161
}
6262

63-
err = teardownIPMasqNFTables(ipn, network, ifname, containerID)
63+
err = teardownIPMasqNFTables(ipns, network, ifname, containerID)
6464
if err != nil && utils.SupportsNFTables() {
6565
errs = append(errs, err.Error())
6666
}

‎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: 25 additions & 17 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

@@ -31,43 +32,48 @@ func Test_setupIPMasqNFTables(t *testing.T) {
3132
network string
3233
ifname string
3334
containerID string
34-
addr string
35+
addrs []string
3536
}{
3637
{
3738
network: "unit-test",
3839
ifname: "eth0",
3940
containerID: "one",
40-
addr: "192.168.1.1/24",
41+
addrs: []string{"192.168.1.1/24"},
4142
},
4243
{
4344
network: "unit-test",
4445
ifname: "eth0",
4546
containerID: "two",
46-
addr: "192.168.1.2/24",
47+
addrs: []string{"192.168.1.2/24", "2001:db8::2/64"},
4748
},
4849
{
4950
network: "unit-test",
5051
ifname: "eth0",
5152
containerID: "three",
52-
addr: "192.168.99.5/24",
53+
addrs: []string{"192.168.99.5/24"},
5354
},
5455
{
5556
network: "alternate",
5657
ifname: "net1",
5758
containerID: "three",
58-
addr: "10.0.0.5/24",
59+
addrs: []string{"10.0.0.5/24", "2001:db8::5/64"},
5960
},
6061
}
6162

6263
for _, c := range containers {
63-
addr, err := netlink.ParseAddr(c.addr)
64-
if err != nil {
65-
t.Fatalf("failed to parse test addr: %v", err)
64+
ipns := []*net.IPNet{}
65+
for _, addr := range c.addrs {
66+
nladdr, err := netlink.ParseAddr(addr)
67+
if err != nil {
68+
t.Fatalf("failed to parse test addr: %v", err)
69+
}
70+
ipns = append(ipns, nladdr.IPNet)
6671
}
67-
err = setupIPMasqNFTablesWithInterface(nft, addr.IPNet, c.network, c.ifname, c.containerID)
72+
err := setupIPMasqNFTablesWithInterface(nft, ipns, c.network, c.ifname, c.containerID)
6873
if err != nil {
6974
t.Fatalf("error from setupIPMasqNFTables: %v", err)
7075
}
76+
7177
}
7278

7379
expected := strings.TrimSpace(`
@@ -76,8 +82,10 @@ add chain inet cni_plugins_masquerade masq_checks { comment "Masquerade traffic
7682
add chain inet cni_plugins_masquerade postrouting { type nat hook postrouting priority 100 ; }
7783
add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.1 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-287fc69eff0574a2, net: unit-test, if: eth0, id: one"
7884
add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.2 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-d750b2c8f0f25d5f, net: unit-test, if: eth0, id: two"
85+
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::2 ip6 daddr != 2001:db8::/64 masquerade comment "6fd94d501e58f0aa-d750b2c8f0f25d5f, net: unit-test, if: eth0, id: two"
7986
add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.99.5 ip daddr != 192.168.99.0/24 masquerade comment "6fd94d501e58f0aa-a4d4adb82b669cfe, net: unit-test, if: eth0, id: three"
8087
add rule inet cni_plugins_masquerade masq_checks ip saddr == 10.0.0.5 ip daddr != 10.0.0.0/24 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
88+
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::5 ip6 daddr != 2001:db8::/64 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
8189
add rule inet cni_plugins_masquerade postrouting ip daddr == 224.0.0.0/4 return
8290
add rule inet cni_plugins_masquerade postrouting ip6 daddr == ff00::/8 return
8391
add rule inet cni_plugins_masquerade postrouting goto masq_checks
@@ -88,22 +96,18 @@ add rule inet cni_plugins_masquerade postrouting goto masq_checks
8896
}
8997

9098
// Add a new container reusing "one"'s address, before deleting "one"
91-
addr, err := netlink.ParseAddr(containers[0].addr)
99+
c := containers[0]
100+
addr, err := netlink.ParseAddr(c.addrs[0])
92101
if err != nil {
93102
t.Fatalf("failed to parse test addr: %v", err)
94103
}
95-
err = setupIPMasqNFTablesWithInterface(nft, addr.IPNet, "unit-test", "eth0", "four")
104+
err = setupIPMasqNFTablesWithInterface(nft, []*net.IPNet{addr.IPNet}, "unit-test", "eth0", "four")
96105
if err != nil {
97106
t.Fatalf("error from setupIPMasqNFTables: %v", err)
98107
}
99108

100109
// Remove "one"
101-
c := containers[0]
102-
addr, err = netlink.ParseAddr(c.addr)
103-
if err != nil {
104-
t.Fatalf("failed to parse test addr: %v", err)
105-
}
106-
err = teardownIPMasqNFTablesWithInterface(nft, addr.IPNet, c.network, c.ifname, c.containerID)
110+
err = teardownIPMasqNFTablesWithInterface(nft, []*net.IPNet{addr.IPNet}, c.network, c.ifname, c.containerID)
107111
if err != nil {
108112
t.Fatalf("error from teardownIPMasqNFTables: %v", err)
109113
}
@@ -114,8 +118,10 @@ add table inet cni_plugins_masquerade { comment "Masquerading for plugins from g
114118
add chain inet cni_plugins_masquerade masq_checks { comment "Masquerade traffic from certain IPs to any (non-multicast) IP outside their subnet" ; }
115119
add chain inet cni_plugins_masquerade postrouting { type nat hook postrouting priority 100 ; }
116120
add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.2 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-d750b2c8f0f25d5f, net: unit-test, if: eth0, id: two"
121+
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::2 ip6 daddr != 2001:db8::/64 masquerade comment "6fd94d501e58f0aa-d750b2c8f0f25d5f, net: unit-test, if: eth0, id: two"
117122
add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.99.5 ip daddr != 192.168.99.0/24 masquerade comment "6fd94d501e58f0aa-a4d4adb82b669cfe, net: unit-test, if: eth0, id: three"
118123
add rule inet cni_plugins_masquerade masq_checks ip saddr == 10.0.0.5 ip daddr != 10.0.0.0/24 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
124+
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::5 ip6 daddr != 2001:db8::/64 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
119125
add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.1 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-e766de567ef6c543, net: unit-test, if: eth0, id: four"
120126
add rule inet cni_plugins_masquerade postrouting ip daddr == 224.0.0.0/4 return
121127
add rule inet cni_plugins_masquerade postrouting ip6 daddr == ff00::/8 return
@@ -150,8 +156,10 @@ add table inet cni_plugins_masquerade { comment "Masquerading for plugins from g
150156
add chain inet cni_plugins_masquerade masq_checks { comment "Masquerade traffic from certain IPs to any (non-multicast) IP outside their subnet" ; }
151157
add chain inet cni_plugins_masquerade postrouting { type nat hook postrouting priority 100 ; }
152158
add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.1.2 ip daddr != 192.168.1.0/24 masquerade comment "6fd94d501e58f0aa-d750b2c8f0f25d5f, net: unit-test, if: eth0, id: two"
159+
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::2 ip6 daddr != 2001:db8::/64 masquerade comment "6fd94d501e58f0aa-d750b2c8f0f25d5f, net: unit-test, if: eth0, id: two"
153160
add rule inet cni_plugins_masquerade masq_checks ip saddr == 192.168.99.5 ip daddr != 192.168.99.0/24 masquerade comment "6fd94d501e58f0aa-a4d4adb82b669cfe, net: unit-test, if: eth0, id: three"
154161
add rule inet cni_plugins_masquerade masq_checks ip saddr == 10.0.0.5 ip daddr != 10.0.0.0/24 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
162+
add rule inet cni_plugins_masquerade masq_checks ip6 saddr == 2001:db8::5 ip6 daddr != 2001:db8::/64 masquerade comment "82783ef24bdc7036-acb19d111858e348, net: alternate, if: net1, id: three"
155163
add rule inet cni_plugins_masquerade postrouting ip daddr == 224.0.0.0/4 return
156164
add rule inet cni_plugins_masquerade postrouting ip6 daddr == ff00::/8 return
157165
add rule inet cni_plugins_masquerade postrouting goto masq_checks

‎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)