[ovs-dev] [PATCH V2 1/3] ofproto-dpif: Move send_packet() to ofproto-dpif-xlate module.

[Alex Wang]

On Mon, Sep 30, 2013 at 1:21 PM, Ethan Jackson <ethan at nicira.com> wrote:

> I think this is on the right track, just some minor comments.
>
> You aren't planning to have anyone call xlate_actions_unsafe() in
> future patches right?  If that's true I'd rather keep the public API
> the same and do the safe/unsafe split internally.  I.E:
>
> Rename xlate_actions_safe() to xlate_actions().
>
> Remove xlate_actions_unsafe() entirely, and simply call
> xlate_actions__() directly when necessary.  Alternatively you could
> rename xlate_actions__() xlate_actions_unsafe() instead, that's a
> matter of personal preference.
>


This makes sense, I'll adjust accordingly,



> I doubt it matters much in practice, but I'd prefer we release the
> xlate_rwlock before we call dpif_execute() in case that takes a long
> time.  You don't need the xlate_rwlock to call xlate_out_uninit(), so
> it should be a simple matter of moving the call before the
> dpif_execute() call.
>
> Along those same lines, there's a lot of working setting up the xin
> which could be pulled out of the critical section.  We could do the
> flow_extract() beforehand for example.  Also initializing the the
> output ofpact.
>


I'll shorten the critical section.


Are you planning to shove a lock around ofproto->stats?  Updating it
> in ofproto_dpif_send_packet() isn't thread safe.
>


Yes, the lock is added in patch 2/3
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130930/76a60318/attachment-0003.html>