[ovs-dev] [PATCH] datapath: Change the single ENOENT return value to ENODEV...

Jesse Gross jesse at nicira.com
Wed Jan 9 22:26:51 UTC 2013

On Wed, Jan 9, 2013 at 12:16 PM, Jarno Rajahalme
<jarno.rajahalme at nsn.com> wrote:
> On Jan 9, 2013, at 20:19 , ext Jesse Gross wrote:
>> On Wed, Jan 9, 2013 at 2:33 AM, Jarno Rajahalme <jarno.rajahalme at nsn.com> wrote:
>>> On Jan 8, 2013, at 21:07 , ext Jesse Gross wrote:
>>>> On Tue, Jan 8, 2013 at 3:41 AM, Jarno Rajahalme <jarno.rajahalme at nsn.com> wrote:
>>>>> Userspace code will need to keep on checking for both ENODEV and ENOENT as
>>>>> long as older kernel modules are around.
>>>>> Signed-off-by: Jarno Rajahalme <jarno.rajahalme at nsn.com>
>>>> OVS userspace actually uses this distinction to determine whether it
>>>> is the datapath or the port that doesn't exist.  For example, see
>>>> netdev_vport_set_config().  In general, it's also usually helpful to
>>>> have more information about the cause of the error as long as it
>>>> doesn't expose implementation details.
>>> Actually, it seems to me that netdev_vport_set_config() does the OVS_VPORT_CMD_SET based on the vport name only. In the "name" branch of the lookup_vport() ENODEV is returned if a matching vport cannot be found, i.e. ENOENT would not be returned to netdev_vport_set_config(). Also, it is the ENODEV value netdev_vport_set_config() looks for, not ENOENT. If netdev_vport_set_config() were to use ODP port number to refer to the vport, it would also need to check for ENOENT in addition to ENODEV.
>>> It is the mismatch between the name and port_no branches of lookup_vport() that triggered me to propose the change. Currently:
>>> If name is given,
>>>  and no such port is found, return ENODEV
>>>  if dp_ifindex is given but it does not match, return ENODEV
>>> else if port_no is given
>>>  and dp (via dp_ifindex) cannot be found, return ENODEV
>>>  if the dp has no port_no, return ENOENT
>>> To me this does not look right. A different value for similar error case ("valid datapath, no such port") is returned depending on whether the query is done with a name or a port_no.
>> OK, that seems fair.
>> In the commit message, you mentioned that userspace will need to keep
>> checking both ENOENT and ENODEV.  However, I only see one use of
>> querying by number (in dpif-linux.c:report_loss() which doesn't care
>> about the actual error value) and only one use of ENOENT (in
>> dpif-linux.c:dpif_linux_is_internal_device() which queries by name).
>> Neither of these seem to be affected by this change or would require
>> any special care going forward.  Is there any other case that you
>> noticed or were you only talking about future uses?
> No, my thinking was that if someone at userspace makes a difference
> between the two, they will need keep on doing it, since they might be
> talking with an older kernel module. Now that you checked that no-one
> actually really depends on the that behavior, the latter part of my commit
> message is moot.

OK, applied, thanks.

More information about the dev mailing list