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:[email protected]] > Sent: Friday, 30 June, 2017 22:30 > To: Jan Scheurich <[email protected]>; [email protected] > 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 <[email protected]> > --- > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
