OK, that idea turned out to be a bad one since masks really do have a lifetime that is separate from an individual flow. However, here's a patch that does the unification a different way.
What do you think? On Wed, Jun 19, 2013 at 4:32 PM, Jesse Gross <je...@nicira.com> wrote: > I think at this point, the use of a separate RCU callback for the > masks is hurting more than it is helping and it would be easier to > just piggyback off the flow/table destroy RCU. I'll try to write a > patch to see if I can get it to work. > > On Wed, Jun 19, 2013 at 4:23 PM, Andy Zhou <az...@nicira.com> wrote: >> When table is destroyed, there is really no need to keep track of the mask's >> reference counting -- The masks will have to be destroyed any ways. >> >> During run time, the mask_list deletion needs to be protected by the >> ovs_lock, But we should not require ovs_lock to be held during table >> destroy. right? So it seems we will have two ways of doing ovs_flow_free() >> any ways, >> >> I will continue to think about how to merge these two cases. Please let me >> know if you have any suggestions. >> >> --andy >> >> >> On Wed, Jun 19, 2013 at 4:09 PM, Jesse Gross <je...@nicira.com> wrote: >>> >>> What is the reason need for iterating through the mask list itself >>> when destroying the table instead of doing it as the flows are >>> deleted? Doing it that way would both avoid the need to have multiple >>> list deletion functions and the differences between the RCU and >>> non-RCU versions of ovs_flow_free(). >>> >>> One issue that comes to mind is the RCU barrier when the module is >>> unloaded but there are other ways to fix that. >>> >>> On Wed, Jun 19, 2013 at 2:09 PM, Andy Zhou <az...@nicira.com> wrote: >>> > Jesse, >>> > >>> > The following patch is an incremental patch on top of yours. >>> > -- Fix a few small bugs introduced by the cleanup patch. >>> > -- Make mask list per table. >>> > >>> > I believe this implementation will address the rcu lock issues we >>> > discussed >>> > off line. >>> > >>> > Would you please review? >>> > >>> > Thanks, >>> > >>> > --andy >>> > >>> > On Tue, Jun 18, 2013 at 4:35 PM, Jesse Gross <je...@nicira.com> wrote: >>> >> >>> >> On Tue, Jun 18, 2013 at 4:15 PM, Andy Zhou <az...@nicira.com> wrote: >>> >> > Add wildcarded flow support in kernel datapath. >>> >> > >>> >> > Wildcarded flow can improve OVS flow set up performance by avoid >>> >> > sending >>> >> > matching new flows to the user space program. The exact performance >>> >> > boost >>> >> > will largely dependent on wildcarded flow hit rate. >>> >> > >>> >> > In case all new flows hits wildcard flows, the flow set up rate is >>> >> > within 5% of that of linux bridge module. >>> >> > >>> >> > Pravin has made significant contributions to this patch. Including >>> >> > API >>> >> > clean ups and bug fixes. >>> >> > >>> >> > Co-authored-by: Pravin B Shelar <pshe...@nicira.com> >>> >> > Signed-off-by: Pravin B Shelar <pshe...@nicira.com> >>> >> > Signed-off-by: Andy Zhou <az...@nicira.com> >>> >> >>> >> I made some incremental changes, can you take a look to see if they >>> >> seem reasonable to you? >>> >> >>> >> Here are the main blocks: >>> >> - Additional interface documentation. >>> >> - Fix memory leak when installing a new flow if there is an >>> >> allocation failure for the mask. >>> >> - Require Ethernet addresses to be present in the key, as before. >>> >> - Make SNAP handling a little more integrated with the rest of the >>> >> parsing and validation code. >>> >> - Make ovs_flow_free() and ovs_deferred_flow_free() have exactly the >>> >> same behavior, just with the addition of RCU, to avoid possible >>> >> confusion. >>> >> - Return errors directly rather than through an intermediate variable >>> >> in ovs_flow_extract() and children. >>> >> - Enforce an exact match for the outer EtherType for vlan packets, >>> >> similar to what we do for other protocols. >>> >> - Miscellaneous style fixes. >>> > >>> > >> >>
0001-Mask-deletion.patch
Description: Binary data
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
