[ovs-dev] [PATCH] packet_type: Force _ETHERTYPE mask in netlink messages
Jan Scheurich
jan.scheurich at ericsson.com
Mon Jul 3 13:22:30 UTC 2017
Hi Eric,
Thanks for the catch. We seem to have overlooked this side effect on the kernel datapath when removing the conceptually incorrect match on eth_type in the datapath when packet_type != PT_ETH.
Instead of re-introducing the unconditional match on eth_type in all megaflow entries, we would rather suggest to handle this (hopefully temporary) mapping of packet_type != PT_ETH to dl_type for the kernel datapath in the function put_exclude_packet_type() in dpif-netlink.c, which already now filters the packet_type attribute out.
Thus we can keep the ofproto translation logic clean and limit the kernel datapath specialties to a single place in the code.
Zoltan has proposed a similar change earlier and will send a corresponding patch soon.
BR, Jan
> -----Original Message-----
> From: Eric Garver [mailto:e at erig.me]
> Sent: Friday, 30 June, 2017 22:30
> To: Jan Scheurich <jan.scheurich at ericsson.com>; dev at openvswitch.org
> Subject: [PATCH] packet_type: Force _ETHERTYPE mask in netlink messages
>
> The kernel does not understand packet_type so it's implicitly encoded by
> the absence of _ETHERNET and presence of _ETHERTYPE. For non-Ethernet we
> need to force the mask for _ETHERTYPE otherwise the kernel's
> match_validate() will complain.
>
> The mask use to always be set in xlate_wc_init() and xlate_wc_finish(),
> but that changed for non-Ethernet frames with the commit listed in
> Fixes.
>
> Fixes: 3d4b2e6eb74e ("userspace: Add OXM field MFF_PACKET_TYPE")
> Signed-off-by: Eric Garver <e at erig.me>
> ---
> lib/odp-util.c | 11 ++++++++++-
> tests/packet-type-aware.at | 4 ++--
> tests/system-userspace-packet-type-aware.at | 8 ++++----
> 3 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index f4c0b665068f..956d9608b220 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -4588,7 +4588,16 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms *parms,
> goto unencap;
> }
>
> - nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, data->dl_type);
> + if (export_mask && flow->packet_type != htonl(PT_ETH)) {
> + /* The kernel does not yet explicitly support packet_type. However, it
> + * implicitly supports it by the absence of _ETHERNET and the presence
> + * of _ETHERTYPE. This mean we must force an exact match on the
> + * _ETHERTYPE.
> + */
> + nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
> + } else {
> + nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, data->dl_type);
> + }
>
> if (eth_type_vlan(flow->dl_type)) {
> goto unencap;
> diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
> index 110407857786..43756eba3041 100644
> --- a/tests/packet-type-aware.at
> +++ b/tests/packet-type-aware.at
> @@ -407,8 +407,8 @@ AT_CHECK([
> recirc_id(0),in_port(br-
> p2),packet_type(ns=0,id=0),eth(dst=aa:55:00:00:00:01),eth_type(0x0800),ipv4(src=20.0.0.2,dst=20.0.0.1,proto=47,frag=no),
> packets:1, bytes:122, used:0.0s, actions:set(ipv4(src=10.0.0.2,dst=10.0.0.1)),tnl_pop(gre_sys)
> recirc_id(0),in_port(br-
> p3),packet_type(ns=0,id=0),eth(dst=aa:55:00:00:00:02),eth_type(0x0800),ipv4(src=30.0.0.3,dst=30.0.0.2,proto=47,frag=no),
> packets:1, bytes:122, used:0.0s, actions:set(ipv4(src=20.0.0.3,dst=20.0.0.2)),tnl_pop(gre_sys)
> recirc_id(0),in_port(n3),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(dst=192.168.10.10,tos=0/0x3,frag=no), packets:1, bytes:98,
> used:0.0s,
> actions:pop_eth,tnl_push(tnl_port(gre_sys),header(size=38,type=3,eth(dst=aa:55:00:00:00:02,src=aa:55:00:00:00:03,dl_type=0x0800)
> ,ipv4(src=30.0.0.3,dst=30.0.0.2,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x800))),out_port(br-p3))
> -tunnel(src=10.0.0.2,dst=10.0.0.1,flags(-df-
> csum)),recirc_id(0),in_port(gre_sys),packet_type(ns=1,id=0x800),ipv4(dst=192.168.10.10,frag=no), packets:1, bytes:84, used:0.0s,
> actions:push_eth(src=00:00:00:00:00:00,dst=aa:55:aa:55:00:01),n1
> -tunnel(src=20.0.0.3,dst=20.0.0.2,flags(-df-
> csum)),recirc_id(0),in_port(gre_sys),packet_type(ns=1,id=0x800),ipv4(dst=192.168.10.10,tos=0/0x3,frag=no), packets:1, bytes:84,
> used:0.0s,
> actions:tnl_push(tnl_port(gre_sys),header(size=38,type=3,eth(dst=aa:55:00:00:00:01,src=aa:55:00:00:00:02,dl_type=0x0800),ipv4(src
> =20.0.0.2,dst=20.0.0.1,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x800))),out_port(br-p2))
> +tunnel(src=10.0.0.2,dst=10.0.0.1,flags(-df-
> csum)),recirc_id(0),in_port(gre_sys),packet_type(ns=1,id=0x800),eth_type(0x0800),ipv4(dst=192.168.10.10,frag=no), packets:1,
> bytes:84, used:0.0s, actions:push_eth(src=00:00:00:00:00:00,dst=aa:55:aa:55:00:01),n1
> +tunnel(src=20.0.0.3,dst=20.0.0.2,flags(-df-
> csum)),recirc_id(0),in_port(gre_sys),packet_type(ns=1,id=0x800),eth_type(0x0800),ipv4(dst=192.168.10.10,tos=0/0x3,frag=no),
> packets:1, bytes:84, used:0.0s,
> actions:tnl_push(tnl_port(gre_sys),header(size=38,type=3,eth(dst=aa:55:00:00:00:01,src=aa:55:00:00:00:02,dl_type=0x0800),ipv4(src
> =20.0.0.2,dst=20.0.0.1,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x800))),out_port(br-p2))
> ])
>
> # Clear up megaflow cache
> diff --git a/tests/system-userspace-packet-type-aware.at b/tests/system-userspace-packet-type-aware.at
> index 65d81ce8ecdd..b405b2520edd 100644
> --- a/tests/system-userspace-packet-type-aware.at
> +++ b/tests/system-userspace-packet-type-aware.at
> @@ -355,8 +355,8 @@ recirc_id(0),in_port(6),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), p
> recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:244, used:0.0s, actions:10
>
> recirc_id(0),in_port(8),packet_type(ns=0,id=0),eth(dst=aa:55:00:00:00:01),eth_type(0x0800),ipv4(src=20.0.0.2,dst=20.0.0.1,proto=47,
> frag=no), packets:2, bytes:244, used:0.0s, actions:set(ipv4(src=10.0.0.2,dst=10.0.0.1)),tnl_pop(14)
>
> recirc_id(0),in_port(9),packet_type(ns=0,id=0),eth(dst=aa:55:00:00:00:02),eth_type(0x0800),ipv4(src=30.0.0.3,dst=30.0.0.2,proto=47,
> frag=no), packets:2, bytes:244, used:0.0s, actions:set(ipv4(src=20.0.0.3,dst=20.0.0.2)),tnl_pop(14)
> -tunnel(src=10.0.0.2,dst=10.0.0.1,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=1,id=0x800),ipv4(dst=192.168.10.10,frag=no), packets:2, bytes:168, used:0.0s,
> actions:push_eth(src=00:00:00:00:00:00,dst=aa:55:aa:55:00:01),15
> -tunnel(src=20.0.0.3,dst=20.0.0.2,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=1,id=0x800),ipv4(dst=192.168.10.10,tos=0/0x3,frag=no), packets:2, bytes:168,
> used:0.0s,
> actions:tnl_push(tnl_port(14),header(size=38,type=3,eth(dst=aa:55:00:00:00:01,src=aa:55:00:00:00:02,dl_type=0x0800),ipv4(src=20.0.
> 0.2,dst=20.0.0.1,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x800))),out_port(6))
> +tunnel(src=10.0.0.2,dst=10.0.0.1,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=1,id=0x800),eth_type(0x0800),ipv4(dst=192.168.10.10,frag=no), packets:2, bytes:168,
> used:0.0s, actions:push_eth(src=00:00:00:00:00:00,dst=aa:55:aa:55:00:01),15
> +tunnel(src=20.0.0.3,dst=20.0.0.2,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=1,id=0x800),eth_type(0x0800),ipv4(dst=192.168.10.10,tos=0/0x3,frag=no), packets:2,
> bytes:168, used:0.0s,
> actions:tnl_push(tnl_port(14),header(size=38,type=3,eth(dst=aa:55:00:00:00:01,src=aa:55:00:00:00:02,dl_type=0x0800),ipv4(src=20.0.
> 0.2,dst=20.0.0.1,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x800))),out_port(6))
> tunnel(src=30.0.0.1,dst=30.0.0.3,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=0,id=0),eth(dst=aa:55:aa:55:00:03),eth_type(0x0800),ipv4(dst=192.168.10.30,frag=no
> ), packets:2, bytes:196, used:0.0s, actions:17
> ])
>
> @@ -409,8 +409,8 @@ recirc_id(0),in_port(6),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), p
> recirc_id(0),in_port(7),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), packets:2, bytes:244, used:0.0s, actions:10
>
> recirc_id(0),in_port(8),packet_type(ns=0,id=0),eth(dst=aa:55:00:00:00:01),eth_type(0x0800),ipv4(src=20.0.0.2,dst=20.0.0.1,proto=47,
> frag=no), packets:2, bytes:244, used:0.0s, actions:set(ipv4(src=10.0.0.2,dst=10.0.0.1)),tnl_pop(14)
>
> recirc_id(0),in_port(9),packet_type(ns=0,id=0),eth(dst=aa:55:00:00:00:02),eth_type(0x0800),ipv4(src=30.0.0.3,dst=30.0.0.2,proto=47,
> frag=no), packets:2, bytes:244, used:0.0s, actions:set(ipv4(src=20.0.0.3,dst=20.0.0.2)),tnl_pop(14)
> -tunnel(src=10.0.0.2,dst=10.0.0.1,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=1,id=0x800),ipv4(dst=192.168.10.10,frag=no), packets:2, bytes:168, used:0.0s,
> actions:push_eth(src=00:00:00:00:00:00,dst=aa:55:aa:55:00:01),15
> -tunnel(src=20.0.0.3,dst=20.0.0.2,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=1,id=0x800),ipv4(dst=192.168.10.10,tos=0/0x3,frag=no), packets:2, bytes:168,
> used:0.0s,
> actions:tnl_push(tnl_port(14),header(size=38,type=3,eth(dst=aa:55:00:00:00:01,src=aa:55:00:00:00:02,dl_type=0x0800),ipv4(src=20.0.
> 0.2,dst=20.0.0.1,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x800))),out_port(6))
> +tunnel(src=10.0.0.2,dst=10.0.0.1,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=1,id=0x800),eth_type(0x0800),ipv4(dst=192.168.10.10,frag=no), packets:2, bytes:168,
> used:0.0s, actions:push_eth(src=00:00:00:00:00:00,dst=aa:55:aa:55:00:01),15
> +tunnel(src=20.0.0.3,dst=20.0.0.2,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=1,id=0x800),eth_type(0x0800),ipv4(dst=192.168.10.10,tos=0/0x3,frag=no), packets:2,
> bytes:168, used:0.0s,
> actions:tnl_push(tnl_port(14),header(size=38,type=3,eth(dst=aa:55:00:00:00:01,src=aa:55:00:00:00:02,dl_type=0x0800),ipv4(src=20.0.
> 0.2,dst=20.0.0.1,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x800))),out_port(6))
> tunnel(src=30.0.0.1,dst=30.0.0.3,flags(-df-
> csum)),recirc_id(0),in_port(14),packet_type(ns=0,id=0),eth(dst=aa:55:aa:55:00:03),eth_type(0x0800),ipv4(dst=192.168.10.30,frag=no
> ), packets:2, bytes:196, used:0.0s, actions:17
> ])
>
> --
> 2.12.0
More information about the dev
mailing list