On Thu, Dec 10, 2015 at 6:55 PM, Liu, Mengke <mengke....@intel.com> wrote: >> -----Original Message----- >> From: jgr...@nicira.com [mailto:jgr...@nicira.com] On Behalf Of Jesse >> Gross >> Sent: Tuesday, December 8, 2015 12:32 AM >> To: Liu, Mengke <mengke....@intel.com> >> Cc: dev@openvswitch.org; Pritesh Kothari (pritkoth) <pritk...@cisco.com>; >> Zhou, Danny <danny.z...@intel.com>; Li, Ricky <ricky...@intel.com>; >> pa...@cisco.com >> Subject: Re: [ovs-dev] [PATCH 0/7] Enable NSH based Service Function >> Chaining support in OVS >> >> On Fri, Nov 6, 2015 at 4:23 PM, Jesse Gross <je...@nicira.com> wrote: >> > On Tue, Nov 3, 2015 at 10:20 AM, Liu, Mengke <mengke....@intel.com> >> wrote: >> >>> > So for NSH header, we need to add the TLV support. For the fixed >> >>> > fields of NSH header like NSI, NSP, we’d like to add specific >> >>> > meta-flow fields for them, for example: >> >>> > >> >>> > MFF_NSP Type: NXM_NX_NSP (105), Length:4 bytes >> >>> > >> >>> > MFF_NSI Type: NXM_NX_NSI (106), Length:1 bytes >> >>> > >> >>> > But for the variable length context headers of NSH, we’d like to >> >>> > use fields like “tun_metadata” (tnl->metadata) to support it. We >> >>> > also have two >> >>> options: >> >>> > >> >>> > 1) Reuse the “tun_metadata” for NSH variable context header, it’s >> >>> > similar to current Geneve TLV support. But it’s a little wield >> >>> > because the NSH header is already an independent protocol layer >> >>> > but not belong to the tunnel layer. >> >>> > >> >>> > 2) Define a new “nsh_metadata” fields for NSH variable context >> header. >> >>> > >> >>> > Which one do you prefer? Please tell us for you inputs on our >> >>> > modification plan. >> >>> >> >>> I would definitely like to reuse the same set of fields as were used >> >>> for Geneve since there are a large number of them and have a second >> >>> set seems wasteful. I don't think there is anything that inherently >> >>> ties them to tunneling, so if you have a different name that is more >> >>> generic we can still change them as long as it is before OVS 2.5 is >> released. >> >>> >> >>> By the way, there are several OpenFlow commands that were added >> >>> support mapping TLVs to fields. These are currently specific to >> >>> Geneve because they validate some protocol specific aspects. NSH >> >>> actually uses the same TLV format as Geneve, so in theory they could >> >>> be shared (and it would be nice to avoid duplicating these). The >> >>> main thing that concerns me about this is the possibility that the >> >>> protocols will diverge in the future or some other protocol that >> >>> does not have the same format will want to use the same thing. In >> >>> any case, it would be nice to think about how this could be made useable >> by everybody before OVS 2.5. >> >> >> >> For NSH TLV implementation, We agree that we should reuse the same >> set of fields as were used for Geneve. As NSH in MD-Type 2 use same TLV >> format as Geneve, they can share the pool of 64 NXMs which can be mapped >> on Geneve TLVs or NSH TLVs. It may be better to make the command name >> more generic. For example, we can rename “add-geneve-map” to “add-tlv- >> map”. >> >> An example in our initial design for NSH MD-type 2 support: >> >> ovs-ofctl add-tlv-map br0 {class=0xffff,type=0,len=4}->tun_metadata0 >> >> ovs-ofctl add-flow br0 in_port=LOCAL, actions=push_nsh, >> >> set_field:221->nsp, set_field:3->nsi, set_field:2- >> >nsh_md_type,set_field:111->tun_metadata0, 1 What do you think about >> this proposal for NSH TLV support? >> > >> > I think it's basically fine although I worry a bit that "add-tlv-map" >> > isn't overly descriptive - it isn't obvious that it is referring to >> > this type of metadata or there could be TLVs in other formats. >> >> I just wanted to point out that OVS 2.5 has branched at this point and >> contains the current (Geneve-centric) names of these commands/fields. >> If you want to change them, I would still apply a patch to help with forward >> compatibility. However, the window of time to do that is rapidly closing, so >> speak now or forever hold your peace. > > We strongly agree that the command "add-geneve-map" should be renamed to > "add-tlv-map" or the other generic name for NSH TLV support and other > protocol in future. We plan to add NSH MD-Type 2 support in the first quarter > of 2016. > > About this patch for renaming the command name, if you are busy, we are > pleased to submit it for review. Thanks.
Please submit a patch to do the renaming ASAP so that it can be applied to branch-2.5. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
