Lori, The refinements proposed by Thomas seem sensible to me. Please proceed as planned below.
Regards, Jarno On Feb 2, 2015, at 10:15 PM, Lori Jakab <loja...@cisco.com> wrote: > 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
