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

Jesse Gross jesse at nicira.com
Wed Jan 9 18:19:00 UTC 2013


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?



More information about the dev mailing list