<div dir="ltr"><div>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. <br><br></div><div>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, <br>
<br></div><div>I will continue to think about how to merge these two cases. Please let me know if you have any suggestions. <br><br></div><div>--andy <br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">
On Wed, Jun 19, 2013 at 4:09 PM, Jesse Gross <span dir="ltr"><<a href="mailto:jesse@nicira.com" target="_blank">jesse@nicira.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
What is the reason need for iterating through the mask list itself<br>
when destroying the table instead of doing it as the flows are<br>
deleted? Doing it that way would both avoid the need to have multiple<br>
list deletion functions and the differences between the RCU and<br>
non-RCU versions of ovs_flow_free().<br>
<br>
One issue that comes to mind is the RCU barrier when the module is<br>
unloaded but there are other ways to fix that.<br>
<div class="HOEnZb"><div class="h5"><br>
On Wed, Jun 19, 2013 at 2:09 PM, Andy Zhou <<a href="mailto:azhou@nicira.com">azhou@nicira.com</a>> wrote:<br>
> Jesse,<br>
><br>
> The following patch is an incremental patch on top of yours.<br>
> -- Fix a few small bugs introduced by the cleanup patch.<br>
> -- Make mask list per table.<br>
><br>
> I believe this implementation will address the rcu lock issues we discussed<br>
> off line.<br>
><br>
> Would you please review?<br>
><br>
> Thanks,<br>
><br>
> --andy<br>
><br>
> On Tue, Jun 18, 2013 at 4:35 PM, Jesse Gross <<a href="mailto:jesse@nicira.com">jesse@nicira.com</a>> wrote:<br>
>><br>
>> On Tue, Jun 18, 2013 at 4:15 PM, Andy Zhou <<a href="mailto:azhou@nicira.com">azhou@nicira.com</a>> wrote:<br>
>> > Add wildcarded flow support in kernel datapath.<br>
>> ><br>
>> > Wildcarded flow can improve OVS flow set up performance by avoid sending<br>
>> > matching new flows to the user space program. The exact performance<br>
>> > boost<br>
>> > will largely dependent on wildcarded flow hit rate.<br>
>> ><br>
>> > In case all new flows hits wildcard flows, the flow set up rate is<br>
>> > within 5% of that of linux bridge module.<br>
>> ><br>
>> > Pravin has made significant contributions to this patch. Including API<br>
>> > clean ups and bug fixes.<br>
>> ><br>
>> > Co-authored-by: Pravin B Shelar <<a href="mailto:pshelar@nicira.com">pshelar@nicira.com</a>><br>
>> > Signed-off-by: Pravin B Shelar <<a href="mailto:pshelar@nicira.com">pshelar@nicira.com</a>><br>
>> > Signed-off-by: Andy Zhou <<a href="mailto:azhou@nicira.com">azhou@nicira.com</a>><br>
>><br>
>> I made some incremental changes, can you take a look to see if they<br>
>> seem reasonable to you?<br>
>><br>
>> Here are the main blocks:<br>
>> - Additional interface documentation.<br>
>> - Fix memory leak when installing a new flow if there is an<br>
>> allocation failure for the mask.<br>
>> - Require Ethernet addresses to be present in the key, as before.<br>
>> - Make SNAP handling a little more integrated with the rest of the<br>
>> parsing and validation code.<br>
>> - Make ovs_flow_free() and ovs_deferred_flow_free() have exactly the<br>
>> same behavior, just with the addition of RCU, to avoid possible<br>
>> confusion.<br>
>> - Return errors directly rather than through an intermediate variable<br>
>> in ovs_flow_extract() and children.<br>
>> - Enforce an exact match for the outer EtherType for vlan packets,<br>
>> similar to what we do for other protocols.<br>
>> - Miscellaneous style fixes.<br>
><br>
><br>
</div></div></blockquote></div><br></div>