[ovs-dev] [PATCH V3 2/5] dpif-netdev: Implement the API functions to allow multiple handler threads read upcall.

Ben Pfaff blp at nicira.com
Wed Mar 12 00:11:50 UTC 2014


On Fri, Mar 07, 2014 at 06:04:39PM -0800, Alex Wang wrote:
> This commit implements the API functions to allow multiple handler
> threads read upcall.
> 
> Also, this commit removes the handling priority of DPIF_UC_MISS
> over DPIF_UC_ACTION.  So, both misses will be put to the same
> queue.  The decision is based on the fact that a lot has changed
> since the age when flow setup rate is most treasured and starving
> all actions in the presence of any flow misses doesn't seem like
> a sound balancing solution.
> 
> Thusly the current implementation will be put in testing and
> investigation for better balancing solution will continue if
> there is an issue.
> 
> Signed-off-by: Alex Wang <alexw at nicira.com>
> 
> ---
> V2 -> V3:
> - rebase.
> 
> PATCH -> V2:
> - explain the drop of upcall queueing priority in dpif-netdev.
> - use mhash to calculate the 5-tuple hash.

Why does dpif_netdev_recv_set() ignore its 'enable' argument?

dp_netdev_refresh_queues() has two callers, each of which is a wrapper
that takes the queue rwlock write-lock.  I guess that taking the lock
could be moved into dp_netdev_refresh_queues().

dp_netdev_init_queue() and dp_netdev_uninit_queue() are short
functions with only a single caller each.  I think that the code
would be clearer if they were integrated into their callers.

I would be more comfortable if dpif_netdev_recv() and
dpif_netdev_recv_wait() checked that the handler_id passed in was in
the currently valid range.

As written, flow_hash_5tuple() will incorporate ICMP type and code
into the hash (because those are stored into tp_src and tp_dst).  Is
that desirable?

Thanks,

Ben.



More information about the dev mailing list