Subject: Re: [RFC][PATCH net-next] ethtool: Allow drivers to select RX NFC rule locations

From: Dimitris Michailidis <[email-masked]>
Date: Fri, 16 Dec 2011 18:44:45 -0800

On 12/16/2011 05:18 PM, Ben Hutchings wrote:
> Define special location values for RX NFC that request the driver to
> select the actual rule location.  This allows for implementation on
> devices that use hash-based filter lookup, whereas currently the API is
> more suited to devices with TCAM lookup or linear search.
> 
> In ethtool_set_rxnfc() and the compat wrapper ethtool_ioctl(), copy
> the structure back to user-space after insertion so that the actual
> location is returned.
> 
> Signed-off-by: Ben Hutchings <[email-masked]>

I like this change.  One concern below.

> -	return dev->ethtool_ops->set_rxnfc(dev, &info);
> +	rc = dev->ethtool_ops->set_rxnfc(dev, &info);
> +	if (rc)
> +		return rc;
> +
> +	if (cmd == ETHTOOL_SRXCLSRLINS &&
> +	    copy_to_user(useraddr, &info, info_size))
> +		return -EFAULT;

Here we return failure but the rule has been added successfully and is in 
effect.  It may be better to return 0 and let user-space tell this last step 
failed by the fact that the location field is still special.