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: [email protected] > [mailto:[email protected]] On Behalf Of Ben Pfaff > Sent: Saturday, 06 May, 2017 06:40 > To: Yi-Hung Wei <[email protected]> > Cc: [email protected] > 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 > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
