> -----Original Message-----
> From: Ilya Maximets [mailto:[email protected]]
> Sent: Friday, July 12, 2019 11:50 AM
> To: Van Haaren, Harry <[email protected]>
> Cc: [email protected]; Stokes, Ian <[email protected]>; Michal Orsák
> <[email protected]>; [email protected]
> Subject: Re: [PATCH v10 0/5] dpcls func ptrs & optimizations
<snip> lots of chatter
> >> All the flows fits into 5+1 case, i.e. optimized function
> >> dpcls_subtable_lookup_mf_u0w5_u1w1 used.
> >>
> >> Most interesting observation:
> >>
> >> * New version of dpcls lookup outperforms SMC in this setup even on
> >> relatively small number of flows. With 8K flows dpcls faster than SMC
> >> by 1.5% and by 5.7% with 256K flows.
> >> Of course, SMC is 10% faster than dpcls with 8 flows, but it's not very
> >> interesting because no-one can beat EMC in this area.
> >>
> >> I'd like to read the code more carefully tomorrow and probably give some
> >> more feedback.
> >>
> >> Best regards, Ilya Maximets.
> >
> > Thanks for your comments - please do prioritize feedback ASAP, because as
> > you know the 2.12 soft-freeze is already in effect.
> >
> > I'll work on Ian's comments on v10, but hold off sending v11 until there
> > is some feedback from you too :)
Thanks for the prompt review;
> Few comments:
>
> 1. I don't really like that new dpcls depends on the internal structure of
> 'struct flowmap'. At least, we need to add a build assert to notify
> anyone who will try to change it.
Agreed - adding a build-time assert is a good idea, will include in v11.
> 2. No need to expose so many internal stuff to every module that includes
> 'dpif-netdev.h'. Can we create 'dpif-netdev-private.h' instead and move
> all the subtable related stuff there?
Good idea - keeps things a bit cleaner, will do for v11.
> 3. IMHO, it's better to apply some preprocessor optimizations to make it
> easier to add new optimized functions and decrease code duplication.
Yes - had intended to MACRO them out when adding more - V11 will include,
thanks!
> 4. Please, add a space between 'ULLONG_FOR_EACH_1' and '('.
Sure - I see you've sent checkpatch improvements - nice one.
> 5. A lot of comments needs to be re-formatted according to coding style,
> i.e. first letter capitalized + period at the end.
Yes - unfortunately I'm trained & hard-coded to DPDK standards... Will fix
as much as I see while rebasing.
> 6. "generic optimized" sounds weird.
I think it’s the best description of what it is that I can think of.
Generic in that it will work for any subtable & packet combination
Optimized in that it is specialized for a specific use-case at compile time.
[Pulling in from other email]
>> Ilya wrote:
>> One more thing. Could you, please, rename patches like:
>> dpif-netdev: Implement function pointers/subtable.
Yes will do.
> Here is some incremental I'm suggesting which covers comments 1, 3 and 4:
Thanks for the suggestions - working on it.
<snip> actual code changes (see previous email on mailing list archive).
Regards, -Harry
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
