Hi Jan, Thanks for letting me know, and sorry that I did not aware of this patch [1]. I take a look at patch [1], and it seems like the common part of [1] and [2] are 1) Defining OF 1.5 packet out format 2) Decoding and encoding OF 1.5 packet out message
On the other hand, [1] focuses on supporting the new packet_type field, and [2] is more on all the other pipeline fields. We can see that from the testcases in [1] and [2]. The testcases in the two patches are actually quite independent, and they test different part of the OF 1.5 protocol. I am thinking about to pull out the common part of both [1] and [2] in a separate patch in my v2, and we can both contribute our work from there. How do you think? [1] https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331032.html [2] https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332066.html Thanks, -Yi-Hung On Mon, May 8, 2017 at 4:40 AM, Jan Scheurich <jan.scheur...@ericsson.com> wrote: > Hi Yi-Hung, > > We have already started to add support for the OpenFlow 1.5 Packet Out > message in our patch series for "packet type-aware pipeline: > https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331023.html > https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/331032.html > > Our focus was to add support for the new packet_type pipeline field, and I'm > not sure we have covered all necessary efforts to support the rest of the > pipeline fields. Please have a look at our patch and make sure that our > efforts are aligned. > > Thanks, Jan > >> -----Original Message----- >> From: ovs-dev-boun...@openvswitch.org >> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Ben Pfaff >> Sent: Saturday, 06 May, 2017 06:40 >> To: Yi-Hung Wei <yihung....@gmail.com> >> Cc: d...@openvswitch.org >> Subject: Re: [ovs-dev] [PATCH 0/3] Support OpenFlow 1.5 packet-out >> >> On Thu, May 04, 2017 at 04:12:17PM -0700, Yi-Hung Wei wrote: >> > This patch series add support to OpenFlow 1.5 packet-out message that >> > enables setting pipeline fields. While there are six pipeline match >> > fields as listed in OpenFlow spec 1.5.1, this patch series implements >> > three of them (OXM_OF_IN_PORT, OXM_OF_METADATA, and OXM_OF_TUNNEL_ID), >> > since the rest of them (OXM_OF_IN_PHY_PORT, OXM_OF_ACTSET_OUTPUT, and >> > OXM_OF_PACKET_TYPE) does not fit into the ovs. >> > >> > Yi-Hung Wei (3): >> > ofp-util: Add flow metadata to ofputil_packet_out >> > ofproto: Add OpenFlow 1.5 packet-out support >> > ofp-parse: Parse pipeline fields in OF1.5 packet-out >> >> Thanks for working on this feature. I'm looking forward to being able >> to say that OVS supports all the features that OpenFlow 1.5 requires. I >> think that with this and extensible flow entry statistics (which we've >> already seen a draft of from, I believe, a TCS developer) we'll be >> there. >> >> This is a high quality series. Thank you! I have only a few comments. >> >> First, I see that this series represents metadata as a "struct match". >> Certainly, this works, but I wonder about the alternatives. The most >> obvious one is "struct flow", which has all the same features as struct >> match, without the masks. To me, it looks like the masks in struct >> match aren't even used, at least not consistently; for example, this >> code in parse_ofp_packet_out_str__() initializes in_port without its >> mask: >> >> *po = (struct ofputil_packet_out) { >> .buffer_id = UINT32_MAX, >> .flow_metadata.flow.in_port.ofp_port = OFPP_CONTROLLER, >> }; >> >> The other possible data structure for metadata is struct pkt_metadata, >> which is designed specifically for metadata. However it's currently >> used mostly in packet handling rather than in OpenFlow, so it would >> probably be a second choice. >> >> Second, this series seems to take the literal definition of "pipeline >> fields" from OpenFlow, only allowing those fields actually mentioned in >> OpenFlow to be used in "packet-out"s. But I think that we should >> include OVS extension pipeline fields, too, such as the various fields >> with tunnel information. I also think that the implementation misses >> some fields that OpenFlow itself defines as pipeline fields, such as the >> OpenFlow packet register pipeline fields. >> >> Can you think through these issues and write up a v2? >> >> Thanks, >> >> Ben. >> _______________________________________________ >> dev mailing list >> d...@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
