Sorry to insist, but can you guys (Jesse/Pravin/Jarno) please look at this and give your opinion so Thomas and I know how to proceed?
-Lori On 01/23/2015 02:45 PM, Lori Jakab wrote: > On 1/20/15 12:52 PM, Thomas Morin wrote: >> Hi Jarno, Lori, >> >> On 12/10/2014 02:45 AM, Jarno Rajahalme wrote: >>> We should add a new metadata key field OVS_KEY_ATTR_PACKET_ETHERTYPE, >>> that contains the ethertype of the associated packet attribute. While >>> this would be strictly needed only for L3 packets, it would be >>> cleaner to include it with all packets in packet misses. Then it >>> could be used in flow setup and packet execution as well. >>> >>> Other packet type namespaces (like IP protocol) could be later >>> supported defining new netlink attributes. >>> >>> A corresponding new field packet_ethertype needs to be added in >>> struct pkt_metadata. This needs to be kept upto date in userspace >>> code pushing and popping headers, so that the correct value gets >>> passed to kernel execution. >> >> Having the tunneled payload type be passed between kernel and >> userspace via OVS_KEY_ATTR_PACKET_ETHERTYPE is something needed for >> MPLS/GRE too (see [1] below). >> >> With some hints from Jesse back in November, I've been working in the >> past weeks to see how much needed to be adapted based on your patch to >> add support for l3 GRE tunnel ports, and have this work for >> MPLS-over-GRE payloads. I've not everything covered yet, but I have >> the basic stuff working (with a kernel dataplane). >> >> Here is an outline of the things I did to support MPLS/GRE based on >> the current l3 port patch (l3_v9), and that I think that these are >> also relevant to simplify the code in the IP/non-MPLS case : >> - kernel: adapt ovs_nla_put_flow to include >> OVS_KEY_ATTR_PACKET_ETHERTYPE in the noethernet case (not done in the >> current l3_v9 patch, in which the kernel datapath consume a value >> given by userspace for a flow put or a command execute, but does not >> provides this info on a flow miss) >> - user: have odp_key_to_pkt_metadata determine md->packet_ethertype >> based on OVS_KEY_ATTR_PACKET_ETHERTYPE, rather than base on the >> presence of OVS_KEY_ATTR_IPV4 or OVS_KEY_ATTR_IPV6) >> - user: similarly for parse_ethertype, to determine the ethertype >> (based on OVS_KEY_ATTR_PACKET_ETHERTYPE rather than base on the >> presence of OVS_KEY_ATTR_IPV4 or OVS_KEY_ATTR_IPV6) >> - user: have miniflow_extract rely on md->packet_ethertype for layer3 >> frames and do not use get_l3_eth_type anymore (which was guessing >> ethertype based on the version field of the IP header) >> - kernel: have ovs_key_from_nlattrs use OVS_KEY_ATTR_PACKET_ETHERTYPE, >> if present, to determine the ethertype >> >> Comments ? > > I will defer judgement to Jesse, Pravin, and Jarno. I don't mind > changing the kernel/userspace API to this direction, it looks cleaner to > me, although I don't know if it has significant performance or > compatibility implications. > >> >> >> Lori, I have the above working on my tree and will share code if we >> agree that this is the right direction. > > If there is consensus to use this approach I will pull your work into my > tree and submit it along with my existing patches. Looking forward to > feedback from the guys who reviewed my patches so far. > > -Lori > >> >> Best, >> >> -Thomas >> >> [1] because MPLS can be used with two ethertypes (0x8847,0x8848) and a >> different semantic can be given to an MPLS label depending on the >> ethtype. This contrasts with IP, for which the ethertype can be >> guessed/derived from the presence of OVS_KEY_ATTR_IPV(4|6) or the >> version field of the IP header. >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
