[ovs-dev] [PATCH v2] dpif: Return ENODEV from dpif_port_query_by_name() if there's no port.

Daniele Di Proietto diproiettod at vmware.com
Fri Jan 6 20:43:03 UTC 2017






On 06/01/2017 11:34, "Ben Pfaff" <blp at ovn.org> wrote:

>On Fri, Jan 06, 2017 at 10:59:07AM -0800, Daniele Di Proietto wrote:
>> bridge_delete_or_reconfigure() deletes every interface that's not dumped
>> by OFPROTO_PORT_FOR_EACH().  ofproto_dpif.c:port_dump_next(), used by
>> OFPROTO_PORT_FOR_EACH, checks if the ofport is in the datapath by
>> calling port_query_by_name().  If port_query_by_name() returns an error,
>> the dump is interrupted.  If port_query_by_name() returns ENODEV, the
>> device doesn't exist and the dump can continue.
>> 
>> port_query_by_name() for the userspace datapath returns ENOENT instead
>> of ENODEV.  This is expected by dpif_port_query_by_name(), but it's not
>> handled correctly by port_dump_next().
>
>Should port_query_by_name() for the userspace datapath return ENODEV,
>instead?

Sorry to waste your time on this.  Yes, that seems the more appropriate solution.

I decided to handle both ENODEV and ENOENT to be consistent with what we did in
the past, e.g bee6b8bc16b1("dpif: Don't log warning for ENOENT with
dpif_port_exists().").

I suspected that ENOENT could only come from the userspace datapath, but I wasn't
too sure about that.

After looking at vport_cmd_get() and testing it I couldn't find any ENOENT, so,
probably, we added all those special cases just for the userspace datapath.

How about the following v3?

https://mail.openvswitch.org/pipermail/ovs-dev/2017-January/327323.html

Thanks,

Daniele

>
>> This commit fixes the problem by translating ENOENT in ENODEV in
>> dpif_port_query_by_name().
>> 
>> dpif-netdev handles reconfiguration errors for an interface by deleting
>> it from the datapath, so it's possible that a device is missing. When this
>> happens we must make sure that port_dump_next() continues to dump other
>> devices, otherwise they will be deleted and the two layers will have an
>> inconsistent view.
>> 
>> The problem was found while developing new code.
>> 
>> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>> ---
>> v2:
>> * Translate ENOENT into ENODEV in dpif_port_query_by_name(), instead of
>>   handling both in port_dump_next().
>
>Acked-by: Ben Pfaff <blp at ovn.org>


More information about the dev mailing list