[ovs-dev] [PATCH 1/2] datapath: Mega flow implementation

Jesse Gross jesse at nicira.com
Thu Jun 20 01:31:11 UTC 2013


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 <jesse at 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 <azhou at 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 <jesse at 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 <azhou at 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 <jesse at nicira.com> wrote:
>>> >>
>>> >> On Tue, Jun 18, 2013 at 4:15 PM, Andy Zhou <azhou at 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 <pshelar at nicira.com>
>>> >> > Signed-off-by: Pravin B Shelar <pshelar at nicira.com>
>>> >> > Signed-off-by: Andy Zhou <azhou at 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.
>>> >
>>> >
>>
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Mask-deletion.patch
Type: application/octet-stream
Size: 5395 bytes
Desc: not available
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130619/9dde4114/attachment-0005.obj>


More information about the dev mailing list