[ovs-discuss] mf_value and mf_subvalue size restrictions

Madhu Challa challa at noironetworks.com
Sat Nov 8 19:41:13 UTC 2014


On Fri, Nov 7, 2014 at 5:14 PM, Jesse Gross <jesse at nicira.com> wrote:
> On Fri, Nov 7, 2014 at 3:06 PM, Madhu Challa <challa at noironetworks.com> wrote:
>> 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 guess the next question then is how to store/classify this set. I
> think that flows pushed down from the controller should be able to
> handle options in varying order, new optional attributes, etc. so that
> will probably require some new logic.

I was planning on accumulating all geneve class match entries in
nx_pull_raw and then post process the array indexed by type always in
the same order. Would that work ?

>
> One other issue that I was think about earlier is how to handle
> critical options. While I think we want to be able to handle
> unexpected non-critical options without additional flows, it should be
> possible to do the opposite as well - if a new option appears that is
> unexpected, it shouldn't be silently ignored.

Have not thought about this. If we get a critical option and no
matching flow we should just drop the packet and log something.

>
>> 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.
>
> I'm not really in favor of supporting this. It seems likely to
> introduce additional complexity and the extra overhead of additional
> OXM headers seems small in the context of a flow setup.

I agree it would keep things simpler this way.


Thanks.



More information about the discuss mailing list