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. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
