On Mon, Jul 03, 2017 at 01:22:30PM +0000, Jan Scheurich wrote: > 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.
I agree that's probably a better idea to keep them in the same place. > > Zoltan has proposed a similar change earlier and will send a > corresponding patch soon. I will keep an eye out for it. I'm currently holding a series to support layer3 tunnels (VXLAN-GPE, GRE) with the kernel datapath. This is one of the fixes required. Thanks. Eric. > > 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
