[ovs-discuss] mf_value and mf_subvalue size restrictions

[Madhu Challa]

On Fri, Nov 7, 2014 at 2:42 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Thu, Nov 6, 2014 at 3:43 PM, Madhu Challa <challa at noironetworks.com> wrote:
>> On Thu, Nov 6, 2014 at 2:37 PM, Jesse Gross <jesse at nicira.com> wrote:
>>>
>>> On Thu, Nov 6, 2014 at 10:08 AM, Madhu Challa <challa at noironetworks.com> wrote:
>>> > Jesse,
>>> >
>>> > Thanks for sharing your thoughts on this.
>>> >
>>> > On Thu, Nov 6, 2014 at 7:47 AM, Jesse Gross <jesse at nicira.com> wrote:
>>> >>
>>> >> On Wed, Nov 5, 2014 at 10:03 AM, Madhu Challa <challa at noironetworks.com>
>>> >> wrote:
>>> >> > Thanks Ben. I will debug and get back to you. I will check with Jesse in
>>> >> > the
>>> >> > upcoming ovs conference if he has other thoughts on implementing this.
>>> >>
>>> >> I haven't had too much time to make progress on this so I don't have
>>> >> much in the way of additional thoughts at this point. The main one is
>>> >> that my goal is to not implement support for specific TLVs in OVS but
>>> >> to expose the full flexibility outwards so that anyone can introduce
>>> >> new metadata without having to modify OVS (the only possible exception
>>> >> being things that have to happen autonomously or on a per-packet basis
>>> >> in OVS).
>>> >
>>> >
>>> > I was thinking along same lines. I was planning on having a new
>>> > MFF_TUN_METADATA  that is basically parsed as a raw OXM of length between 4
>>> > and 128 bytes. The match logic would parse multiple of these OXMs from a
>>> > FlowMod.
>>> >
>>> > From the struct match perspective we need to extend struct flow_tnl to carry
>>> > this metadata. This is the difficult part because struct flow is already 200
>>> > bytes and the sparse representation only allows an addition of 52 bytes. I
>>> > feel we could instead have a reference to tnl metadata from flow_tnl. I have
>>> > not scoped out all the changes to do this, however if you have any thoughts
>>> > or an alternative that would be great.
>>>
>>> I agree that we will need to extend some infrastructure here. I
>>> haven't thought too much about this yet.
>>>
>>> >>
>>> >> One issue that comes up when doing this is that the TLVs in both
>>> >> Geneve and OXM are exactly the same size so mapping them directly
>>> >> would consume the entire OXM space just for Geneve. There was a
>>> >> suggestion to use experimenter OXMs since they are larger but I
>>> >> haven't had a chance to look into this yet.
>>> >
>>> >
>>> > Yep I am using experimenter OXMs.
>>>
>>> I'm curious how you ended up laying this out. The OpenFlow spec says
>>> that the extra space should be used as an vendor ID in the form of an
>>> OUI. How did you reconcile this?
>>
>>
>> Good that you brought this up. I had different thoughts in mind. One
>> was to use the Nicira vendor id 0x002320 and a unique oxm_field to
>> represent MFF_TUN_METADATA. I was thinking we could overlay the Geneve
>> tunnel options header right after the oxm header, so it looks
>> something like this.
>>
>>        3                   2                   1                   0
>>      1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>    |           oxm_class           |   oxm_field |M|   oxm_length  |
>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>    |                            Vendor id                          |
>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>    |          Option Class         |      Type     |R|R|R| Length  |
>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>    |                      Variable Option Data                     |
>>    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>
>> oxm_class = 0xffff
>> vendor_id = 0x002320
>>
>> I was also thinking if it would be possible to reserve a separate
>> class for tunnel encap metadata.
>
> Would you then parse the option class and type as a further part of
> the match header (as opposed to the payload to be matched)? It seems
> like you would have to, otherwise you could have multiple OXMs trying
> to match multiple options and it's not clear how you would match them
> up.

Sorry I was not clear earlier. I was planning on taking the later
approach i.e one oxm per geneve option type so that we can support the
full length options 128 bytes. During the parse I was planning on
bundling all the oxms of type MFF_TUN_METADATA, we would have multiple
of these and when these get stored in the flow_tnl.metadata they would
contain a set of <4 byte geneve options header, variable length value>

I do not see why we cannot support multiple Geneve options per OXM if
the option length is small, it would save us some packet memory that
the 8 byte OXM header would take.

However we would still need to be able to support multiple OXMs of
same type. So to keep things simple in the poc I am assuming one OXM
per geneve options.

Does that make sense ?

Thanks.