[ovs-dev] [PATCHv6 13/14] dpif: Minimize memory copy for revalidation.

Joe Stringer joestringer at nicira.com
Wed Oct 1 07:44:58 UTC 2014


On 30 September 2014 10:24, Ben Pfaff <blp at nicira.com> wrote:

> I suspect that check_recirc() and check_uid() should pass
> DPIF_FP_MODIFY as well as DPIF_FP_CREATE, just in case a previous run
> of OVS was killed at just the right time to leave a straggler probe
> flow in the datapath.
>

I checked on this, and it used to use DPIF_FP_MODIFY, but this was changed
in commit a7d1bbdcfe49e8c7a5575c9ab46b2ac9e5642ef1, log below.

The probes already check for whether the flow exists, and count this as a
success case, then delete the flows, so I don't think this should be a
problem.


    ofproto-dpif: Use DPIF_FP_CREATE but not DPIF_FP_MODIFY.

    A dpif reports EEXIST if a flow put operation that should create a new
flow
    instead attempts to modify an existing flow, or ENOENT if a flow put
would
    create a flow that overlaps some existing flow.  The latter does not
always
    indicate a bug in OVS userspace, because it can also mean that two
    userspace OVS packet handler threads received packets that generated
    the same megaflow for different microflows.  Until now, userspace has
    logged this, which confuses users by making them suspect a bug.  We
could
    simply not log ENOENT in userspace, but that would suppress logging for
    genuine bugs too.  Instead, this commit drops DPIF_FP_MODIFY from flow
    put operations in ofproto-dpif, which transforms this particular
    problem into EEXIST, which userspace already logs at "debug" level (see
    flow_message_log_level()), effectively suppressing the logging in normal
    circumstances.

    It appears that in practice ofproto-dpif doesn't actually ever need to
    modify flows in the datapath, only create and delete them, so this
    shouldn't cause problems.



More information about the dev mailing list