| Message ID | 1468468914-5225-2-git-send-email-aatteka@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
> On Jul 13, 2016, at 9:01 PM, Ansis Atteka <aatteka@ovn.org> wrote: > > There are two issues that this patch fixes: > 1. it was impossible to set skb mark at all through > NXM_NX_PKT_MARK register for tunnel packets; AND > 2. ipsec_xxx tunnels would not be marked with the default > IPsec mark (broken by d23df9a87 "lib/odp: Use masked set > actions."). > > This patch also adds anti-regression tests to prevent such > breakages in the future. > > Signed-off-by: Ansis Atteka <aatteka@ovn.org> > VMware-BZ: #1653178 > --- > ofproto/tunnel.c | 4 +- > tests/tunnel.at | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 122 insertions(+), 2 deletions(-) > > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c > index 9695c54..206299e 100644 > --- a/ofproto/tunnel.c > +++ b/ofproto/tunnel.c > @@ -357,7 +357,6 @@ tnl_process_ecn(struct flow *flow) > flow->nw_tos |= IP_ECN_CE; > } > > - flow->pkt_mark &= ~IPSEC_MARK; If we do not clear the IPSEC_MARK bit on input, then the bit will remain set also on output, since the change below will not change the mark if match.pkt_mark is 0. For example, we could have input from ipsec tunnel (mark has IPSEC_MARK set ?), which is then output to a non-IPSEC tunnel, but the combination of the changes above an below will fail to clear the mark. Would this work as intended? Jarno > return true; > } > > @@ -435,7 +434,8 @@ tnl_port_send(const struct ofport_dpif *ofport, struct flow *flow, > flow->tunnel.ipv6_dst = in6addr_any; > } > } > - flow->pkt_mark = tnl_port->match.pkt_mark; > + flow->pkt_mark |= tnl_port->match.pkt_mark; > + wc->masks.pkt_mark |= tnl_port->match.pkt_mark; > > if (!cfg->out_key_flow) { > flow->tunnel.tun_id = cfg->out_key; > diff --git a/tests/tunnel.at b/tests/tunnel.at > index 7f82785..15ae5cf 100644 > --- a/tests/tunnel.at > +++ b/tests/tunnel.at > @@ -142,6 +142,126 @@ AT_CHECK([tail -1 stdout], [0], > OVS_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([tunnel - unencrypted tunnel and not setting skb_mark]) > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > + options:key=5 ofport_request=1\ > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > + ofport_request=2 ofport_request=2]) > +AT_DATA([flows.txt], [dnl > +actions=output:1 > +]) > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) > +AT_CHECK([tail -1 stdout], [0], > + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),1 > +]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([tunnel - unencrypted tunnel and setting skb_mark to 1]) > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > + options:key=5 ofport_request=1\ > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > + ofport_request=2 ofport_request=2]) > +AT_DATA([flows.txt], [dnl > +actions=load:0x1->NXM_NX_PKT_MARK[[]],output:1 > +]) > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) > +AT_CHECK([tail -1 stdout], [0], > + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x1)),1 > +]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([tunnel - unencrypted tunnel and setting skb_mark to 2]) > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > + options:key=5 ofport_request=1\ > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > + ofport_request=2 ofport_request=2]) > +AT_DATA([flows.txt], [dnl > +actions=load:0x2->NXM_NX_PKT_MARK[[]],output:1 > +]) > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) > +AT_CHECK([tail -1 stdout], [0], > + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x2)),1 > +]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([tunnel - encrypted tunnel and not setting skb_mark]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > +AT_SKIP_IF([$non_ascii_cwd]) > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > + options:key=5 ofport_request=1\ > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > + ofport_request=2 ofport_request=2]) > +AT_DATA([flows.txt], [dnl > +actions=output:1 > +]) > +OVS_MONITOR_IPSEC_START > +AT_CHECK([ovs-vsctl set interface p1 type=ipsec_gre options:psk=1234567890]) > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) > +AT_CHECK([tail -1 stdout], [0], > + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x1/0x1)),1 > +]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([tunnel - encrypted tunnel and setting skb_mark to 1]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > +AT_SKIP_IF([$non_ascii_cwd]) > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > + options:key=5 ofport_request=1\ > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > + ofport_request=2 ofport_request=2]) > +AT_DATA([flows.txt], [dnl > +actions=load:0x1->NXM_NX_PKT_MARK[[]],output:1 > +]) > +OVS_MONITOR_IPSEC_START > +AT_CHECK([ovs-vsctl set interface p1 type=ipsec_gre options:psk=1234567890]) > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) > +AT_CHECK([tail -1 stdout], [0], > + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x1)),1 > +]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([tunnel - encrypted tunnel and setting skb_mark to 2]) > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > +AT_SKIP_IF([$non_ascii_cwd]) > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > + options:key=5 ofport_request=1\ > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > + ofport_request=2 ofport_request=2]) > +AT_DATA([flows.txt], [dnl > +actions=load:0x2->NXM_NX_PKT_MARK[[]],output:1 > +]) > +OVS_MONITOR_IPSEC_START > +AT_CHECK([ovs-vsctl set interface p1 type=ipsec_gre options:psk=1234567890]) > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) > +AT_CHECK([tail -1 stdout], [0], > + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x3)),1 > +]) > +OVS_VSWITCHD_STOP > +AT_CLEANUP > + > AT_SETUP([tunnel - ToS and TTL inheritance]) > OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > options:remote_ip=1.1.1.1 options:tos=inherit \ > -- > 2.7.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
On 14 July 2016 at 02:22, Jarno Rajahalme <jarno@ovn.org> wrote: > > > On Jul 13, 2016, at 9:01 PM, Ansis Atteka <aatteka@ovn.org> wrote: > > > > There are two issues that this patch fixes: > > 1. it was impossible to set skb mark at all through > > NXM_NX_PKT_MARK register for tunnel packets; AND > > 2. ipsec_xxx tunnels would not be marked with the default > > IPsec mark (broken by d23df9a87 "lib/odp: Use masked set > > actions."). > > > > This patch also adds anti-regression tests to prevent such > > breakages in the future. > > > > Signed-off-by: Ansis Atteka <aatteka@ovn.org> > > VMware-BZ: #1653178 > > --- > > ofproto/tunnel.c | 4 +- > > tests/tunnel.at | 120 > +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 122 insertions(+), 2 deletions(-) > > > > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c > > index 9695c54..206299e 100644 > > --- a/ofproto/tunnel.c > > +++ b/ofproto/tunnel.c > > @@ -357,7 +357,6 @@ tnl_process_ecn(struct flow *flow) > > flow->nw_tos |= IP_ECN_CE; > > } > > > > - flow->pkt_mark &= ~IPSEC_MARK; > > If we do not clear the IPSEC_MARK bit on input, then the bit will remain > set also on output, since the change below will not change the mark if > match.pkt_mark is 0. For example, we could have input from ipsec tunnel > (mark has IPSEC_MARK set ?), which is then output to a non-IPSEC tunnel, > but the combination of the changes above an below will fail to clear the > mark. Would this work as intended? > I think you are right. IPsec skb_mark Mark should be cleared when outputting packet to the next tunnel, because the next tunnel could be in unencrypted mode: 1) My patch behavior: recirc_id(0),tunnel(src=192.168.122.101,dst=192.168.122.13,flags(-df-csum)),in_port(2),skb_mark(0x1),eth(src=be:7a:9d:e2:4f:44,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.1.2,tip=1.1.1.3,op=1/0xff), packets:3, bytes:126, used:0.540s, actions:1,set(tunnel(dst=8.8.8.8,ttl=64,flags(df))),2 2) Behavior after your suggested changes: recirc_id(0),tunnel(src=192.168.122.13,dst=192.168.122.101,flags(-df-csum)),in_port(2),skb_mark(0x1),eth(src=ea:55:cf:a8:21:4c,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.1.1,tip=1.1.1.3,op=1/0xff), packets:4, bytes:168, used:0.900s, actions:set(skb_mark(0)),1,set(tunnel(dst=8.8.8.8,ttl=64,flags(df))),2 I sent PATCHv2 for that! Thanks for pointing this out. > Jarno > > > return true; > > } > > > > @@ -435,7 +434,8 @@ tnl_port_send(const struct ofport_dpif *ofport, > struct flow *flow, > > flow->tunnel.ipv6_dst = in6addr_any; > > } > > } > > - flow->pkt_mark = tnl_port->match.pkt_mark; > > + flow->pkt_mark |= tnl_port->match.pkt_mark; > > + wc->masks.pkt_mark |= tnl_port->match.pkt_mark; > > > > if (!cfg->out_key_flow) { > > flow->tunnel.tun_id = cfg->out_key; > > diff --git a/tests/tunnel.at b/tests/tunnel.at > > index 7f82785..15ae5cf 100644 > > --- a/tests/tunnel.at > > +++ b/tests/tunnel.at > > @@ -142,6 +142,126 @@ AT_CHECK([tail -1 stdout], [0], > > OVS_VSWITCHD_STOP > > AT_CLEANUP > > > > +AT_SETUP([tunnel - unencrypted tunnel and not setting skb_mark]) > > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > > + options:key=5 ofport_request=1\ > > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > > + ofport_request=2 ofport_request=2]) > > +AT_DATA([flows.txt], [dnl > > +actions=output:1 > > +]) > > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], > [0], [stdout]) > > +AT_CHECK([tail -1 stdout], [0], > > + [Datapath actions: > set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),1 > > +]) > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > + > > +AT_SETUP([tunnel - unencrypted tunnel and setting skb_mark to 1]) > > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > > + options:key=5 ofport_request=1\ > > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > > + ofport_request=2 ofport_request=2]) > > +AT_DATA([flows.txt], [dnl > > +actions=load:0x1->NXM_NX_PKT_MARK[[]],output:1 > > +]) > > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], > [0], [stdout]) > > +AT_CHECK([tail -1 stdout], [0], > > + [Datapath actions: > set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x1)),1 > > +]) > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > + > > +AT_SETUP([tunnel - unencrypted tunnel and setting skb_mark to 2]) > > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > > + options:key=5 ofport_request=1\ > > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > > + ofport_request=2 ofport_request=2]) > > +AT_DATA([flows.txt], [dnl > > +actions=load:0x2->NXM_NX_PKT_MARK[[]],output:1 > > +]) > > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], > [0], [stdout]) > > +AT_CHECK([tail -1 stdout], [0], > > + [Datapath actions: > set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x2)),1 > > +]) > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > + > > +AT_SETUP([tunnel - encrypted tunnel and not setting skb_mark]) > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > > +AT_SKIP_IF([$non_ascii_cwd]) > > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > > + options:key=5 ofport_request=1\ > > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > > + ofport_request=2 ofport_request=2]) > > +AT_DATA([flows.txt], [dnl > > +actions=output:1 > > +]) > > +OVS_MONITOR_IPSEC_START > > +AT_CHECK([ovs-vsctl set interface p1 type=ipsec_gre > options:psk=1234567890]) > > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], > [0], [stdout]) > > +AT_CHECK([tail -1 stdout], [0], > > + [Datapath actions: > set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x1/0x1)),1 > > +]) > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > + > > +AT_SETUP([tunnel - encrypted tunnel and setting skb_mark to 1]) > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > > +AT_SKIP_IF([$non_ascii_cwd]) > > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > > + options:key=5 ofport_request=1\ > > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > > + ofport_request=2 ofport_request=2]) > > +AT_DATA([flows.txt], [dnl > > +actions=load:0x1->NXM_NX_PKT_MARK[[]],output:1 > > +]) > > +OVS_MONITOR_IPSEC_START > > +AT_CHECK([ovs-vsctl set interface p1 type=ipsec_gre > options:psk=1234567890]) > > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], > [0], [stdout]) > > +AT_CHECK([tail -1 stdout], [0], > > + [Datapath actions: > set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x1)),1 > > +]) > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > + > > +AT_SETUP([tunnel - encrypted tunnel and setting skb_mark to 2]) > > +AT_SKIP_IF([test $HAVE_PYTHON = no]) > > +AT_SKIP_IF([$non_ascii_cwd]) > > +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > > + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ > > + options:key=5 ofport_request=1\ > > + -- add-port br0 p2 -- set Interface p2 type=dummy \ > > + ofport_request=2 ofport_request=2]) > > +AT_DATA([flows.txt], [dnl > > +actions=load:0x2->NXM_NX_PKT_MARK[[]],output:1 > > +]) > > +OVS_MONITOR_IPSEC_START > > +AT_CHECK([ovs-vsctl set interface p1 type=ipsec_gre > options:psk=1234567890]) > > +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP > > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) > > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy > 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], > [0], [stdout]) > > +AT_CHECK([tail -1 stdout], [0], > > + [Datapath actions: > set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x3)),1 > > +]) > > +OVS_VSWITCHD_STOP > > +AT_CLEANUP > > + > > AT_SETUP([tunnel - ToS and TTL inheritance]) > > OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ > > options:remote_ip=1.1.1.1 options:tos=inherit \ > > -- > > 2.7.4 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev > >
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c index 9695c54..206299e 100644 --- a/ofproto/tunnel.c +++ b/ofproto/tunnel.c @@ -357,7 +357,6 @@ tnl_process_ecn(struct flow *flow) flow->nw_tos |= IP_ECN_CE; } - flow->pkt_mark &= ~IPSEC_MARK; return true; } @@ -435,7 +434,8 @@ tnl_port_send(const struct ofport_dpif *ofport, struct flow *flow, flow->tunnel.ipv6_dst = in6addr_any; } } - flow->pkt_mark = tnl_port->match.pkt_mark; + flow->pkt_mark |= tnl_port->match.pkt_mark; + wc->masks.pkt_mark |= tnl_port->match.pkt_mark; if (!cfg->out_key_flow) { flow->tunnel.tun_id = cfg->out_key; diff --git a/tests/tunnel.at b/tests/tunnel.at index 7f82785..15ae5cf 100644 --- a/tests/tunnel.at +++ b/tests/tunnel.at @@ -142,6 +142,126 @@ AT_CHECK([tail -1 stdout], [0], OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([tunnel - unencrypted tunnel and not setting skb_mark]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ + options:key=5 ofport_request=1\ + -- add-port br0 p2 -- set Interface p2 type=dummy \ + ofport_request=2 ofport_request=2]) +AT_DATA([flows.txt], [dnl +actions=output:1 +]) +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),1 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([tunnel - unencrypted tunnel and setting skb_mark to 1]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ + options:key=5 ofport_request=1\ + -- add-port br0 p2 -- set Interface p2 type=dummy \ + ofport_request=2 ofport_request=2]) +AT_DATA([flows.txt], [dnl +actions=load:0x1->NXM_NX_PKT_MARK[[]],output:1 +]) +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x1)),1 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([tunnel - unencrypted tunnel and setting skb_mark to 2]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ + options:key=5 ofport_request=1\ + -- add-port br0 p2 -- set Interface p2 type=dummy \ + ofport_request=2 ofport_request=2]) +AT_DATA([flows.txt], [dnl +actions=load:0x2->NXM_NX_PKT_MARK[[]],output:1 +]) +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x2)),1 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([tunnel - encrypted tunnel and not setting skb_mark]) +AT_SKIP_IF([test $HAVE_PYTHON = no]) +AT_SKIP_IF([$non_ascii_cwd]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ + options:key=5 ofport_request=1\ + -- add-port br0 p2 -- set Interface p2 type=dummy \ + ofport_request=2 ofport_request=2]) +AT_DATA([flows.txt], [dnl +actions=output:1 +]) +OVS_MONITOR_IPSEC_START +AT_CHECK([ovs-vsctl set interface p1 type=ipsec_gre options:psk=1234567890]) +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x1/0x1)),1 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([tunnel - encrypted tunnel and setting skb_mark to 1]) +AT_SKIP_IF([test $HAVE_PYTHON = no]) +AT_SKIP_IF([$non_ascii_cwd]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ + options:key=5 ofport_request=1\ + -- add-port br0 p2 -- set Interface p2 type=dummy \ + ofport_request=2 ofport_request=2]) +AT_DATA([flows.txt], [dnl +actions=load:0x1->NXM_NX_PKT_MARK[[]],output:1 +]) +OVS_MONITOR_IPSEC_START +AT_CHECK([ovs-vsctl set interface p1 type=ipsec_gre options:psk=1234567890]) +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x1)),1 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([tunnel - encrypted tunnel and setting skb_mark to 2]) +AT_SKIP_IF([test $HAVE_PYTHON = no]) +AT_SKIP_IF([$non_ascii_cwd]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ + options:remote_ip=1.1.1.1 options:local_ip=2.2.2.2 \ + options:key=5 ofport_request=1\ + -- add-port br0 p2 -- set Interface p2 type=dummy \ + ofport_request=2 ofport_request=2]) +AT_DATA([flows.txt], [dnl +actions=load:0x2->NXM_NX_PKT_MARK[[]],output:1 +]) +OVS_MONITOR_IPSEC_START +AT_CHECK([ovs-vsctl set interface p1 type=ipsec_gre options:psk=1234567890]) +OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=6,tos=4,ttl=128,frag=no),tcp(src=8,dst=9)'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: set(tunnel(tun_id=0x5,src=2.2.2.2,dst=1.1.1.1,ttl=64,flags(df|key))),set(skb_mark(0x3)),1 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([tunnel - ToS and TTL inheritance]) OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \ options:remote_ip=1.1.1.1 options:tos=inherit \
There are two issues that this patch fixes: 1. it was impossible to set skb mark at all through NXM_NX_PKT_MARK register for tunnel packets; AND 2. ipsec_xxx tunnels would not be marked with the default IPsec mark (broken by d23df9a87 "lib/odp: Use masked set actions."). This patch also adds anti-regression tests to prevent such breakages in the future. Signed-off-by: Ansis Atteka <aatteka@ovn.org> VMware-BZ: #1653178 --- ofproto/tunnel.c | 4 +- tests/tunnel.at | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 2 deletions(-)