On Fri, Nov 7, 2014 at 3:06 PM, Madhu Challa <cha...@noironetworks.com> wrote: > On Fri, Nov 7, 2014 at 2:42 PM, Jesse Gross <je...@nicira.com> wrote: >> On Thu, Nov 6, 2014 at 3:43 PM, Madhu Challa <cha...@noironetworks.com> >> wrote: >>> On Thu, Nov 6, 2014 at 2:37 PM, Jesse Gross <je...@nicira.com> wrote: >>>> >>>> On Thu, Nov 6, 2014 at 10:08 AM, Madhu Challa <cha...@noironetworks.com> >>>> wrote: >>>> > Jesse, >>>> > >>>> > Thanks for sharing your thoughts on this. >>>> > >>>> > On Thu, Nov 6, 2014 at 7:47 AM, Jesse Gross <je...@nicira.com> wrote: >>>> >> >>>> >> On Wed, Nov 5, 2014 at 10:03 AM, Madhu Challa <cha...@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. 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. > 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. _______________________________________________ discuss mailing list discuss@openvswitch.org http://openvswitch.org/mailman/listinfo/discuss
