Skip to content

Commit 6ba1067

Browse files
danwinshipdanwinship
committed
Use cookies in HostSubnet-related OVS flows to avoid misdeletions
1 parent 022e2de commit 6ba1067

File tree

2 files changed

+155
-9
lines changed

2 files changed

+155
-9
lines changed

‎pkg/network/node/ovscontroller.go

+18-9
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package node
44

55
import (
6+
"crypto/sha256"
67
"fmt"
78
"net"
89
"sort"
@@ -34,7 +35,7 @@ const (
3435
Vxlan0 = "vxlan0"
3536

3637
// rule versioning; increment each time flow rules change
37-
ruleVersion = 6
38+
ruleVersion = 7
3839

3940
ruleVersionTable = 253
4041
)
@@ -503,26 +504,34 @@ func (oc *ovsController) UpdateEgressNetworkPolicyRules(policies []networkapi.Eg
503504
}
504505
}
505506

507+
func hostSubnetCookie(subnet *networkapi.HostSubnet) uint32 {
508+
hash := sha256.Sum256([]byte(subnet.UID))
509+
return (uint32(hash[0]) << 24) | (uint32(hash[1]) << 16) | (uint32(hash[2]) << 8) | uint32(hash[3])
510+
}
511+
506512
func (oc *ovsController) AddHostSubnetRules(subnet *networkapi.HostSubnet) error {
513+
cookie := hostSubnetCookie(subnet)
507514
otx := oc.ovs.NewTransaction()
508515

509-
otx.AddFlow("table=10, priority=100, tun_src=%s, actions=goto_table:30", subnet.HostIP)
516+
otx.AddFlow("table=10, priority=100, cookie=0x%08x, tun_src=%s, actions=goto_table:30", cookie, subnet.HostIP)
510517
if vnid, ok := subnet.Annotations[networkapi.FixedVNIDHostAnnotation]; ok {
511-
otx.AddFlow("table=50, priority=100, arp, nw_dst=%s, actions=load:%s->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, vnid, subnet.HostIP)
512-
otx.AddFlow("table=90, priority=100, ip, nw_dst=%s, actions=load:%s->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, vnid, subnet.HostIP)
518+
otx.AddFlow("table=50, priority=100, cookie=0x%08x, arp, nw_dst=%s, actions=load:%s->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", cookie, subnet.Subnet, vnid, subnet.HostIP)
519+
otx.AddFlow("table=90, priority=100, cookie=0x%08x, ip, nw_dst=%s, actions=load:%s->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", cookie, subnet.Subnet, vnid, subnet.HostIP)
513520
} else {
514-
otx.AddFlow("table=50, priority=100, arp, nw_dst=%s, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, subnet.HostIP)
515-
otx.AddFlow("table=90, priority=100, ip, nw_dst=%s, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", subnet.Subnet, subnet.HostIP)
521+
otx.AddFlow("table=50, priority=100, cookie=0x%08x, arp, nw_dst=%s, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", cookie, subnet.Subnet, subnet.HostIP)
522+
otx.AddFlow("table=90, priority=100, cookie=0x%08x, ip, nw_dst=%s, actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:%s->tun_dst,output:1", cookie, subnet.Subnet, subnet.HostIP)
516523
}
517524

518525
return otx.EndTransaction()
519526
}
520527

521528
func (oc *ovsController) DeleteHostSubnetRules(subnet *networkapi.HostSubnet) error {
529+
cookie := hostSubnetCookie(subnet)
530+
522531
otx := oc.ovs.NewTransaction()
523-
otx.DeleteFlows("table=10, tun_src=%s", subnet.HostIP)
524-
otx.DeleteFlows("table=50, arp, nw_dst=%s", subnet.Subnet)
525-
otx.DeleteFlows("table=90, ip, nw_dst=%s", subnet.Subnet)
532+
otx.DeleteFlows("table=10, cookie=0x%08x", cookie)
533+
otx.DeleteFlows("table=50, cookie=0x%08x", cookie)
534+
otx.DeleteFlows("table=90, cookie=0x%08x", cookie)
526535
return otx.EndTransaction()
527536
}
528537

‎pkg/network/node/subnets_test.go

+137
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,140 @@ func TestHostSubnetWatcher(t *testing.T) {
200200
}
201201
}
202202

203+
func TestHostSubnetReassignment(t *testing.T) {
204+
hsw, flows := setupHostSubnetWatcher(t)
205+
206+
hs1orig := makeHostSubnet("node1", "192.168.0.2", "10.128.0.0/23")
207+
hs2orig := makeHostSubnet("node2", "192.168.1.2", "10.129.0.0/23")
208+
209+
// Create original HostSubnets
210+
211+
err := hsw.updateHostSubnet(hs1orig)
212+
if err != nil {
213+
t.Fatalf("Unexpected error adding HostSubnet: %v", err)
214+
}
215+
err = hsw.updateHostSubnet(hs2orig)
216+
if err != nil {
217+
t.Fatalf("Unexpected error adding HostSubnet: %v", err)
218+
}
219+
220+
err = assertHostSubnetFlowChanges(hsw, &flows,
221+
flowChange{
222+
kind: flowAdded,
223+
match: []string{"table=10", "tun_src=192.168.0.2"},
224+
},
225+
flowChange{
226+
kind: flowAdded,
227+
match: []string{"table=10", "tun_src=192.168.1.2"},
228+
},
229+
flowChange{
230+
kind: flowAdded,
231+
match: []string{"table=50", "arp", "arp_tpa=10.128.0.0/23", "192.168.0.2->tun_dst"},
232+
},
233+
flowChange{
234+
kind: flowAdded,
235+
match: []string{"table=50", "arp", "arp_tpa=10.129.0.0/23", "192.168.1.2->tun_dst"},
236+
},
237+
flowChange{
238+
kind: flowAdded,
239+
match: []string{"table=90", "ip", "nw_dst=10.128.0.0/23", "192.168.0.2->tun_dst"},
240+
},
241+
flowChange{
242+
kind: flowAdded,
243+
match: []string{"table=90", "ip", "nw_dst=10.129.0.0/23", "192.168.1.2->tun_dst"},
244+
},
245+
flowChange{
246+
kind: flowRemoved,
247+
match: []string{"table=111", "goto_table:120"},
248+
noMatch: []string{"->tun_dst"},
249+
},
250+
flowChange{
251+
kind: flowAdded,
252+
match: []string{"table=111", "192.168.0.2->tun_dst", "192.168.1.2->tun_dst"},
253+
},
254+
)
255+
if err != nil {
256+
t.Fatalf("%v", err)
257+
}
258+
259+
// Now both nodes go offline (without their Node objects being deleted), reboot and
260+
// get assigned the opposite IPs after reboot. They reregister with the master, which
261+
// updates their Node IPs, which causes the SDN master to update their HostSubnets.
262+
// After the first update, we'll have two HostSubnets with the same HostIP, which
263+
// used to cause us to break things when we got the second update.
264+
265+
hs1new := hs1orig.DeepCopy()
266+
hs1new.HostIP = hs2orig.HostIP
267+
hs2new := hs2orig.DeepCopy()
268+
hs2new.HostIP = hs1orig.HostIP
269+
270+
err = hsw.updateHostSubnet(hs1new)
271+
if err != nil {
272+
t.Fatalf("Unexpected error adding HostSubnet: %v", err)
273+
}
274+
err = hsw.updateHostSubnet(hs2new)
275+
if err != nil {
276+
t.Fatalf("Unexpected error adding HostSubnet: %v", err)
277+
}
278+
279+
err = assertHostSubnetFlowChanges(hsw, &flows,
280+
// (We have to check for these table=10 removes+adds because they're not
281+
// actually identical; the cookies will have changed.)
282+
flowChange{
283+
kind: flowRemoved,
284+
match: []string{"table=10", "tun_src=192.168.0.2"},
285+
},
286+
flowChange{
287+
kind: flowAdded,
288+
match: []string{"table=10", "tun_src=192.168.0.2"},
289+
},
290+
flowChange{
291+
kind: flowRemoved,
292+
match: []string{"table=10", "tun_src=192.168.1.2"},
293+
},
294+
flowChange{
295+
kind: flowAdded,
296+
match: []string{"table=10", "tun_src=192.168.1.2"},
297+
},
298+
299+
flowChange{
300+
kind: flowRemoved,
301+
match: []string{"table=50", "arp", "arp_tpa=10.128.0.0/23", "192.168.0.2->tun_dst"},
302+
},
303+
flowChange{
304+
kind: flowAdded,
305+
match: []string{"table=50", "arp", "arp_tpa=10.128.0.0/23", "192.168.1.2->tun_dst"},
306+
},
307+
308+
flowChange{
309+
kind: flowRemoved,
310+
match: []string{"table=50", "arp", "arp_tpa=10.129.0.0/23", "192.168.1.2->tun_dst"},
311+
},
312+
flowChange{
313+
kind: flowAdded,
314+
match: []string{"table=50", "arp", "arp_tpa=10.129.0.0/23", "192.168.0.2->tun_dst"},
315+
},
316+
317+
flowChange{
318+
kind: flowRemoved,
319+
match: []string{"table=90", "ip", "nw_dst=10.128.0.0/23", "192.168.0.2->tun_dst"},
320+
},
321+
flowChange{
322+
kind: flowAdded,
323+
match: []string{"table=90", "ip", "nw_dst=10.128.0.0/23", "192.168.1.2->tun_dst"},
324+
},
325+
326+
flowChange{
327+
kind: flowRemoved,
328+
match: []string{"table=90", "ip", "nw_dst=10.129.0.0/23", "192.168.1.2->tun_dst"},
329+
},
330+
flowChange{
331+
kind: flowAdded,
332+
match: []string{"table=90", "ip", "nw_dst=10.129.0.0/23", "192.168.0.2->tun_dst"},
333+
},
334+
)
335+
if err != nil {
336+
t.Fatalf("%v", err)
337+
}
338+
}
339+

0 commit comments

Comments
 (0)