Hi Antonio, Thanks for the review. I will fix all the editorials in v3. A few more answers inline. Thanks, Jan
> -----Original Message----- > From: Fischetti, Antonio [mailto:antonio.fische...@intel.com] > Sent: Tuesday, 19 July, 2016 17:23 > > Hi Jan, > I added some comments prefixed with [Antonio F]. > > Thanks, > Antonio > > > > +#define DPCLS_OPTIMIZATION_INTERVAL 1000 > > [Antonio F] I'd prefer to add a comment to specify if these are sec, msec,.. OK > > @@ -3893,6 +3975,7 @@ fast_path_processing(struct > > dp_netdev_pmd_thread *pmd, > > } > > > > dp_netdev_count_packet(pmd, DP_STAT_MASKED_HIT, cnt - miss_cnt); > > + dp_netdev_count_packet(pmd, DP_STAT_LOOKUP_HIT, lookup_cnt); > > [Antonio F] Should we divide lookup_cnt by the processed packets, > I mean > dp_netdev_count_packet(pmd, DP_STAT_LOOKUP_HIT, > lookup_cnt/cnt); > ? It is faster to just add up the absolute numbers in the loop and do the division only at the time of printing. Also, I am interested in the average number of subtable lookups per masked hit. dpcls misses would distort the picture as they always have to visit all subtables in the dpcls instance. > > +/* Periodically sort the dpcls subtable vectors according to hit > > counts */ > > +static inline void > > +dpcls_optimize(struct dpcls *cls) > > [Antonio F] Why not calling this function 'dpcls_sort_subtables' > or 'dpcls_upd_subtbl_priority' ? I will rename to dpcls_sort_subtables. > > + subtable->hit_cnt++; > > [Antonio F] I think we can be enough confident that hit_cnt does not > overflow. > It's a uint32_d var and the ranking refresh it's every 1000 msec. > Even if a port is hitting the same subtable at the line-rate - say 14 Mpps - > it's enough less than 2^32-1. > So we don't need to check if hit_cnt overflows, or am I missing something? > If that's the case, maybe a comment here would help? I think you are right. Even in the best case a single PMD cannot handle much more than 10 Mpps. This would be the upper bound for any subtable in one of its dpcls instances. A 32-bit counter cannot wrap in one second. Will add a comment. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
