diff mbox

[ovs-dev,PATCHv2,2/2] tunneling: get skb marking to work properly with tunnels

Message ID 1468963555-19323-2-git-send-email-aatteka@ovn.org
State Accepted
Headers show

Commit Message

Ansis Atteka July 19, 2016, 9:25 p.m. UTC
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 |   3 +-
 tests/tunnel.at  | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+), 1 deletion(-)

Comments

Jarno Rajahalme July 21, 2016, 11:07 a.m. UTC | #1
Looks good to me:

Acked-by: Jarno Rajahalme <jarno@ovn.org>

> On Jul 19, 2016, at 2:25 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 |   3 +-
> tests/tunnel.at  | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 122 insertions(+), 1 deletion(-)
> 
> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> index 9695c54..85a485c 100644
> --- a/ofproto/tunnel.c
> +++ b/ofproto/tunnel.c
> @@ -435,7 +435,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
Ansis July 22, 2016, 2:28 a.m. UTC | #2
On Jul 21, 2016 2:08 PM, "Jarno Rajahalme" <jarno@ovn.org> wrote:
>
> Looks good to me:
>
> Acked-by: Jarno Rajahalme <jarno@ovn.org>
>

Thanks. Can you also review patch 1/2?
> > On Jul 19, 2016, at 2:25 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 |   3 +-
> > tests/tunnel.at  | 120
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 122 insertions(+), 1 deletion(-)
> >
> > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> > index 9695c54..85a485c 100644
> > --- a/ofproto/tunnel.c
> > +++ b/ofproto/tunnel.c
> > @@ -435,7 +435,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
>
Jarno Rajahalme July 22, 2016, 4:42 a.m. UTC | #3
I already tested and pushed these to master.

  Jarno

> On Jul 22, 2016, at 05:28, Ansis Atteka <ansisatteka@gmail.com> wrote:
> 
> On Jul 21, 2016 2:08 PM, "Jarno Rajahalme" <jarno@ovn.org> wrote:
> >
> > Looks good to me:
> >
> > Acked-by: Jarno Rajahalme <jarno@ovn.org>
> >
> 
> Thanks. Can you also review patch 1/2?
> > > On Jul 19, 2016, at 2:25 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 |   3 +-
> > > tests/tunnel.at  | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 122 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> > > index 9695c54..85a485c 100644
> > > --- a/ofproto/tunnel.c
> > > +++ b/ofproto/tunnel.c
> > > @@ -435,7 +435,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 mbox

Patch

diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 9695c54..85a485c 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -435,7 +435,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 \