On Oct 31, 2014, at 2:10 PM, Ben Pfaff <b...@nicira.com> wrote: > On Fri, Oct 31, 2014 at 01:08:37PM -0700, Jarno Rajahalme wrote: >> >> On Oct 30, 2014, at 4:43 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote: >> >>> >>> On Oct 30, 2014, at 3:38 PM, Ben Pfaff <b...@nicira.com> wrote: >>> >>>> On Tue, Oct 28, 2014 at 07:05:01PM -0700, Jarno Rajahalme wrote: >>>>> >>>>>> On Oct 28, 2014, at 4:36 PM, Ben Pfaff <b...@nicira.com> wrote: >>>>>> >>>>>>> On Fri, Oct 24, 2014 at 01:36:40PM -0700, Jarno Rajahalme wrote: >>>>>>> Previously, accurate iteration required writers to be excluded during >>>>>>> iteration. This patch changes the structure of the classifier by >>>>>>> moving the list of rules from struct cls_match to struct cls_subtable. >>>>>>> The list element is also moved from the struct cls_match to struct >>>>>>> cls_rule, which makes iteration more straightforward, and allows the >>>>>>> iterators to remain ignorant of the internals of the cls_match. These >>>>>>> changes allow iteration of rules in the classifier by traversing the >>>>>>> RCU-friendly subtables vector, and the rculist of rules in each >>>>>>> subtable. Classifier modifications may be performed concurrently, but >>>>>>> whether or not the concurrent iterator sees those changes depends on >>>>>>> the timing of change. This is similar to having writers excluded by a >>>>>>> mutex, where visibility of changes depends on the timing of mutex >>>>>>> acquisition. >>>>>>> >>>>>>> The subtable's rculist also allows to make >>>>>>> classifier_find_rule_exactly() and classifier_rule_overlaps() >>>>>>> lockless. >>>>>>> >>>>>>> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> >>>>>> >>>>>> I *think* I follow what's going on here, but just to be sure, let me >>>>>> try to explain it. After this patch, the subtable has a list of every >>>>>> rule in the subtable, as 'rules_list'. The rules in the list are in >>>>>> no particular order, except that rules with identical match criteria >>>>>> are in subsequent positions. Is that correct? >>>>> >>>>> Yes :-) >>>> >>>> The conjunctive match series uses the list of lower-priority rules in >>>> lookup. I think that this patch, as-is, would make that a lot more >>>> expensive, because it's no longer cheap to tell whether the next rule >>>> in the list has the same match. I guess one could still mark >>>> boundaries somehow; do you have an idea? >>> >>> Will have to think about this. However, I totally missed this in >>> my review of the conjunctive match; as the lower-priority rules >>> list is not yet RCU, it cannot be safely used on lookups. It >>> should be sufficient to convert from struct list in struct >>> cls_match to struct rculist, tough. >>> >> >> My assumption with this patch was that the list entry in cls_match >> is not used in lookup, and hence it can be moved to cls_rule. Now, >> since conjunctive match is going to use the list in lookup, not only >> does the list be an rculist, but moving it to cls_rule will add >> unnecessary memory indirections. >> >> How about simply having two rculist nodes, one in cls_match (like >> before) and another in cls_rule for iteration? I kind of liked the >> fact that this patch did not add to the memory footprint at all, but >> I guess more robust iteration is worth it, even if we add a new >> rculist node. > > I agree. > > When I stopped by your desk, it sounded like you're working on a > patch, so I'll wait for it.
Just sent them out. Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
