On Apr 28, 2014, at 7:07 PM, Ethan Jackson <et...@nicira.com> wrote:
> The comment of miniflow_and_mask_matches_flow() need to be reworded. > > Do we really need that function? Is it really that much faster? > Having two functions which do the exact same thing is a bit confusing. The almost equivalent function miniflow_equal_flow_in_minimask() does not require the miniflow and minimask to have the same map, and it is slower for it. However, it is only used by the test-classifier.c, so one option to reduce the confusion would be to move it there. What do you think? > > So I suppose the only reason for a non zero inline_values array is for > those callers who can't allocate something that's variable sized, but > still want to avoid the extra memory allocation. However, I don't > think there are any of those callers which are performance critical. > I'm wondering if we could simplify this significantly by always having > offline values passed in by the caller. For those callers which can > make it variable sized, they would, everyone else would make it > FLOW_U32s sized? To rephrase: now that the performance critical users always allocate the right amount of storage, we should decide what to do with the size of the inline_values array. I see two options: 1) Minimize it (i.e., make it 2, as the offline_values would take that much space anyway). 2) Maximize it (i.e., make it FLOWU32S) with the thought that we could get rid of offline_values altogether and thus simplify the code and speed it up a bit. I think this is an interesting idea. The only downside I see is that we still include cls_rule in struct rule, and the size of it could still be an issue. Using a pointer instead could solve that problem. I suggest we work on this as on work-in-progress, and check the current series in to get a stable basis for that work. Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
