Hi Jarno, This is breaking the windows build (https://ci.appveyor.com/project/blp/ovs/build/1.0.459): lib/classifier.c(1355) : error C2229: struct '<unnamed-tag>' has an illegal zero-sized array.
The problem is the following (https://msdn.microsoft.com/en-us/library/0scy7z2d.aspx ): https://github.com/openvswitch/ovs/commit/8fd4792403254f868398a89fb5625830ee5a08eb#diff-b87c023d7e733f983131f231a672e71eR389 Could you please take a look into it? Thanks, Alin. -----Mesaj original----- De la: dev [mailto:dev-boun...@openvswitch.org] În numele Jarno Rajahalme Trimis: Thursday, July 16, 2015 1:59 AM Către: Ben Pfaff Cc: dev@openvswitch.org Subiect: Re: [ovs-dev] [RFC PATCH v2 3/7] flow: Always inline miniflows. Thanks for the review, pushed to master. Jarno > On Jul 15, 2015, at 3:05 PM, Ben Pfaff <b...@nicira.com> wrote: > > On Wed, Jul 15, 2015 at 01:11:54PM -0700, Jarno Rajahalme wrote: >> >>> On Jul 14, 2015, at 4:50 PM, Ben Pfaff <b...@nicira.com> wrote: >>> >>> On Fri, Jul 10, 2015 at 10:35:21AM -0700, Jarno Rajahalme wrote: >>>> Now that performance critical code already inlines miniflows and >>>> minimasks, we can simplify struct miniflow by always dynamically >>>> allocating miniflows and minimasks to the correct size. This >>>> changes the struct minimatch to always contain pointers to its >>>> miniflow and minimask. >>>> >>>> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> >>> >>> I like simplifying things so that two possibilities become one. >>> It's nice. >>> >>> I see two possible ways to simplify this, each with its own tradeoffs: >>> >>> 1. The way that you did it, which means that one must always >>> either dynamically allocate struct miniflow to a proper size >>> or ensure that there's enough space for the maximum size flow >>> immediately following struct miniflow. >>> >>> 2. Change struct miniflow to: >>> >>> struct miniflow { >>> uint64_t map; >>> uint64_t *values; >>> }; >>> >>> With this strategy, there's no need to dynamically allocate >>> struct miniflow itself, so it can be directly embedded in >>> larger structures instead of via pointer. The usual time cost >>> of accessing miniflow data is again one pointer dereference >>> (of 'values'). >>> >>> This can be kind of nice because you're not fighting with the >>> compiler to put data in the right place. >>> >>> Either way, the usual time cost of accessing miniflow data is one >>> pointer dereference (in case #1, from whatever contains the >>> miniflow; in case #2, dereference of 'values'), and the usual space >>> cost of embedding a miniflow is one pointer (although in case #1 the >>> pointer is in what contains the miniflow and in case #2 the pointer >>> is in the miniflow itself). >>> >>> Anyway, I wanted to make sure that you considered both of these >>> solutions and decided that #1 was better. If so: >>> >> >> I agree that case #2 is syntactically nicer in the generic case, >> where one pointer dereference is always needed. However, we already >> embed miniflows to other data structures (flow keys and masks in >> classifier and userspace datapath), where avoid the dereference and >> hence make accessing the miniflow faster. With case #2 we’d need to >> define another “struct inline_miniflow” (basically the case #1) to >> avoid the dereference. IMO it is better to use the case #1 everywhere >> than defining two different miniflow structures. > > OK, I'm glad that you considered the possibility then. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
