On Jan 9, 2013, at 20:19 , ext Jesse Gross wrote: > On Wed, Jan 9, 2013 at 2:33 AM, Jarno Rajahalme <[email protected]> > wrote: >> >> On Jan 8, 2013, at 21:07 , ext Jesse Gross wrote: >> >>> On Tue, Jan 8, 2013 at 3:41 AM, Jarno Rajahalme <[email protected]> >>> 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 <[email protected]> >>> >>> 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. Jarno _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
