Closed danwinship closed 8 years ago
@openshift/networking ptal
lgtm, for what that's worth
Looks good to me, but just to clarify:
cookie=0x0, table=6, priority=200,ip,nw_dst=10.1.0.1 actions=output:2
cookie=0x0, table=6, priority=150,ip,reg0=0,nw_dst=10.1.0.0/24 actions=goto_table:9
>>> cookie=0x0, table=6, priority=150,ip,nw_dst=10.1.0.0/24 actions=goto_table:7
cookie=0x0, table=6, priority=100,ip,nw_dst=10.1.0.0/16 actions=goto_table:8
cookie=0x0, table=6, priority=0, ip actions=output:2
there is no table 7 for flatsdn; but I guess that rule is just redundant with the rule above it, so we don't ever expect it to be matched right?
Just for reference, here's the new rule set from a node:
OFPST_FLOW reply (OF1.3) (xid=0x2):
cookie=0x0, table=0, tun_src=0.0.0.0 actions=goto_table:1
cookie=0xaf50204, table=0, tun_src=10.245.2.4 actions=goto_table:1
cookie=0x0, table=1, actions=learn(table=9,hard_timeout=900,priority=200,NXM_OF_ETH_DST[]=NXM_OF_ETH_SRC[],load:NXM_NX_TUN_IPV4_SRC[]->NXM_NX_TUN_IPV4_DST[],output:NXM_OF_IN_PORT[]),goto_table:2
cookie=0x0, table=2, priority=200,arp actions=goto_table:9
cookie=0x0, table=2, priority=100,in_port=1 actions=goto_table:3 # vxlan0
cookie=0x0, table=2, priority=100,in_port=2 actions=goto_table:6 # tun0
cookie=0x0, table=2, priority=100,in_port=3 actions=goto_table:6 # vovsbr
cookie=0x0, table=2, priority=0 actions=goto_table:4
cookie=0x0, table=3, priority=200,ip,nw_dst=10.1.0.1 actions=output:2
cookie=0x0, table=3, priority=100,ip,nw_dst=10.1.0.0/24 actions=goto_table:9
cookie=0x0, table=4, priority=100,ip,in_port=4,nw_src=10.1.0.2 actions=goto_table:6
cookie=0x0, table=6, priority=200,ip,nw_dst=10.1.0.1 actions=output:2
cookie=0x0, table=6, priority=150,ip,reg0=0,nw_dst=10.1.0.0/24 actions=goto_table:9
cookie=0x0, table=6, priority=150,ip,nw_dst=10.1.0.0/24 actions=goto_table:7
cookie=0x0, table=6, priority=100,ip,nw_dst=10.1.0.0/16 actions=goto_table:8
cookie=0x0, table=6, priority=0, ip actions=output:2
cookie=0xaf50204, table=8, priority=100,ip,nw_dst=10.1.1.0/24 actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:10.245.2.4->tun_dst,output:1
cookie=0x0, table=9, hard_timeout=900, priority=200,dl_dst=02:42:0a:01:00:02 actions=load:0->NXM_NX_TUN_IPV4_DST[],output:4
cookie=0xaf50204, table=9, priority=100,arp,arp_tpa=10.1.1.0/24 actions=move:NXM_NX_REG0[]->NXM_NX_TUN_ID[0..31],set_field:10.245.2.4->tun_dst,output:1
cookie=0x0, table=9, priority=0,arp actions=FLOOD
and here's the old ruleset:
OFPST_FLOW reply (OF1.3) (xid=0x2):
cookie=0x0, table=0, priority=100,arp,arp_tpa=10.1.0.1 actions=output:2
cookie=0x0, table=0, priority=100,ip,nw_dst=10.1.0.1 actions=output:2
cookie=0x0, table=0, priority=75,ip,nw_dst=10.1.0.0/24 actions=output:9
cookie=0x0, table=0, priority=75,arp,arp_tpa=10.1.0.0/24 actions=output:9
cookie=0xaf50204, table=0, priority=100,ip,nw_dst=10.1.1.0/24 actions=set_field:10.245.2.4->tun_dst,output:1
cookie=0xaf50204, table=0, priority=100,arp,arp_tpa=10.1.1.0/24 actions=set_field:10.245.2.4->tun_dst,output:1
cookie=0x0, table=0, priority=50 actions=output:2
(side-note, a lot less rules before; are we sure it's really faster as claimed?)
there is no table 7 for flatsdn; but I guess that rule is just redundant with the rule above it, so we don't ever expect it to be matched right?
Yes... maybe I should do that slightly different to be clearer.
(side-note, a lot less rules before; are we sure it's really faster as claimed?)
multitenant consistently beats ovs-subnet in the perf tests though I never got around to figuring out why... maybe the rules auto-added by learn() end up getting processed more efficiently than the ones manually added by ovs-subnet?
If you don't have any thoughts about how to do it more clearly, then punt for later since it has no actual effect to leave the rule. Branch LGTM, I'll leave open for a bit to see if anyone else has comments, like @rajatchopra ?
pushed an updated vxlan-filtering; you can diff against danwinship/vxlan-filtering-old. The new version adds a few more comments, and completely skips tables 5 and 7 when using single-tenant
OK, I figured out the details of how to test this (see the Trello card, https://trello.com/c/DSRGjbmc/) and confirmed that it works.
Diffs LGTM
I'm still working out how to test this, but (a) it doesn't break things it's not supposed to break, and (b) ovs-appctl ofproto/trace shows it working.
Rather than adding rules to subnet and multitenant separately, I just switched subnet over to using the multitenant rules (with one short-circuit around service isolation). Testing consistently shows that the multitenant rules are faster anyway, and this also pulls in one other security fix (making it so that containers can only send packets with their own IP address as source; previously multitenant enforced that but subnet did not).