Re: [ethtool PATCH 4/4] v5 Add RX packet classification interface

[Dimitris Michailidis <dm@chelsio.com>]

On 05/04/2011 11:35 AM, Alexander Duyck wrote:
> On 5/4/2011 11:18 AM, Dimitris Michailidis wrote:
>> On 05/04/2011 10:41 AM, Alexander Duyck wrote:
>>> This is why I was suggesting that the best approach would be to update
>>> the kernel to add a separate ioctl for letting the driver setup the
>>> location.  We can just attempt to make that call and when we get the
>>> EOPNOTSUPP errno we know the device driver doesn't support it and can
>>> then let the rule manager take over.
>>
>> The problem with this is the location is dependent on the type of 
>> filter being
>> added.  I.e., the ioctl would need to get all the information the 
>> existing
>> ioctl carries making the new ioctl a small superset of the current one.
>> Additionally, if the driver only allocates a location in a separate 
>> ioctl how
>> does it know that the app is actually going to use it?
> 
> It doesn't know that the application is actually going to use it.  What 
> should happen is that the location should be verified by the driver when 
> it is used in the rule insertion call.  After all it is fully possible 
> for the user to specify a location out of range since the insert call 
> does no validation in ethtool if the user specified the location.  That 
> responsibility now lies with the driver.

That's not the problem I was pointing out.  Of course the driver will verify 
the location it is given.  The problem is if you have a separate ioctl call 
that only reserves a location (and it needs to reserve otherwise several of 
these calls can get the same value), if you don't use it you leave the driver 
with orphan reservations.  Imagine you hit ^C between the two ioctls in ethtool.