On Tue, Nov 3, 2015 at 10:20 AM, Liu, Mengke <mengke....@intel.com> wrote: > Hi, Jesse > > For NSH implementation in OVS, we have two more topics want to discuss with > you: > Except for NSH encapsulation format of “VxLAN GPE + NSH”, we also want to > support another encapsulation mode defined in NSH SPEC: “Ethernet + NSH”, > which is to add only one L2 header out of NSH header. We have two proposals > to support it. > 1) We use an integer action code for action “push_nsh” to decide whether push > the Ethernet header or not: > a) “push_nsh:1” is used for pushing Ethernet header + NSH header > (Ethernet + NSH + original packet). > b) “push_nsh:0” is used for pushing only NSH header (NSH + original packet). > Then the sample for Ethernet+NSH encapsulation is as following: > ovs-ofctl add-flow br-int “priority=200, in_port=1, actions=push_nsh:1, > set_field:221->nsp, set_field:3->nsi, set_field:111->nshc1, > set_field:00:11:22:33:44:55->encap_eth_dst, output:${VXLAN_PORT}" > 2)Push NSH header action and push Ethernet action are decoupled. The action > “push_nsh“ is used for pushing NSH header and the action “push_eth” is used > for push Ethernet header. Then the encapsulation sample can be implemented > as following: > ovs-ofctl add-flow br-int “priority=200, in_port=1, actions=push_nsh, > set_field:221->nsp, set_field:3->nsi, set_field:111->nshc1, push_eth, > set_field:00:11:22:33:44:55->encap_eth_dst, output:${VXLAN_PORT}" > We prefer to take option 2. What do you think about it?
I agree, option 2 is better. >> > 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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev