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

Reply via email to