[PATCH] mac80211: silently accept deletion of non-existant key

[PATCH] mac80211: silently accept deletion of non-existant key

[John W. Linville]

From: John W. Linville <linville@linville-t41.local>

Otherwise, 'iwconfig wlan0 key off' with no key set results in:

	Error for wireless request "Set Encode" (8B2A) :
	    SET failed on device wlan0 ; No such file or directory.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
Is it really worth returning an error if someone tries to delete an
non-existant key?  Does hostapd make some use of that?

Would it be worth having a debug message in the non-existant key case?

 net/mac80211/ieee80211_ioctl.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index 1d91575..7d6d52d 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -55,9 +55,6 @@ static int ieee80211_set_encryption(struct net_device *dev, u8 *sta_addr,
 			key = sta->key;
 		}
 
-		if (!key)
-			return -ENOENT;
-
 		ieee80211_key_free(key);
 		return 0;
 	} else {
-- 
1.5.4.1

Re: [PATCH] mac80211: silently accept deletion of non-existant key

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 512 bytes --]


On Tue, 2008-03-25 at 11:43 -0400, John W. Linville wrote:
> From: John W. Linville <linville@linville-t41.local>
> 
> Otherwise, 'iwconfig wlan0 key off' with no key set results in:
> 
> 	Error for wireless request "Set Encode" (8B2A) :
> 	    SET failed on device wlan0 ; No such file or directory.

And what is the problem with us telling iwconfig that there was no key?
You should argue for iwconfig ignoring that particular problem, but I
don't think we should do so in the kernel.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] mac80211: silently accept deletion of non-existant key

[John W. Linville]

On Tue, Mar 25, 2008 at 05:22:05PM +0100, Johannes Berg wrote:
> 
> On Tue, 2008-03-25 at 11:43 -0400, John W. Linville wrote:
> > From: John W. Linville <linville@linville-t41.local>
> > 
> > Otherwise, 'iwconfig wlan0 key off' with no key set results in:
> > 
> > 	Error for wireless request "Set Encode" (8B2A) :
> > 	    SET failed on device wlan0 ; No such file or directory.
> 
> And what is the problem with us telling iwconfig that there was no key?
> You should argue for iwconfig ignoring that particular problem, but I
> don't think we should do so in the kernel.

Why is it a problem?  How does it hurt anything?  How is it useful
to return an error?

FWIW, other drivers seem to accept it.  I don't see why we need to
complain.

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] mac80211: silently accept deletion of non-existant key

[Michael Buesch]

On Tuesday 25 March 2008 17:37:51 John W. Linville wrote:
> On Tue, Mar 25, 2008 at 05:22:05PM +0100, Johannes Berg wrote:
> > 
> > On Tue, 2008-03-25 at 11:43 -0400, John W. Linville wrote:
> > > From: John W. Linville <linville@linville-t41.local>
> > > 
> > > Otherwise, 'iwconfig wlan0 key off' with no key set results in:
> > > 
> > > 	Error for wireless request "Set Encode" (8B2A) :
> > > 	    SET failed on device wlan0 ; No such file or directory.
> > 
> > And what is the problem with us telling iwconfig that there was no key?
> > You should argue for iwconfig ignoring that particular problem, but I
> > don't think we should do so in the kernel.
> 
> Why is it a problem?  How does it hurt anything?  How is it useful
> to return an error?
> 
> FWIW, other drivers seem to accept it.  I don't see why we need to
> complain.

Well, it makes sense to return an error in this case, but if common
practice is to ignore it in old WE based drivers, we should adhere to that
to preserve userspace ABI compatibility.

So the real question is: Is there any userspace program that relies on
this ABI detail?

-- 
Greetings Michael.

Re: [PATCH] mac80211: silently accept deletion of non-existant key

[John W. Linville]

On Tue, Mar 25, 2008 at 06:12:45PM +0100, Michael Buesch wrote:
> On Tuesday 25 March 2008 17:37:51 John W. Linville wrote:
> > On Tue, Mar 25, 2008 at 05:22:05PM +0100, Johannes Berg wrote:
> > > 
> > > On Tue, 2008-03-25 at 11:43 -0400, John W. Linville wrote:
> > > > From: John W. Linville <linville@linville-t41.local>
> > > > 
> > > > Otherwise, 'iwconfig wlan0 key off' with no key set results in:
> > > > 
> > > > 	Error for wireless request "Set Encode" (8B2A) :
> > > > 	    SET failed on device wlan0 ; No such file or directory.
> > > 
> > > And what is the problem with us telling iwconfig that there was no key?
> > > You should argue for iwconfig ignoring that particular problem, but I
> > > don't think we should do so in the kernel.
> > 
> > Why is it a problem?  How does it hurt anything?  How is it useful
> > to return an error?
> > 
> > FWIW, other drivers seem to accept it.  I don't see why we need to
> > complain.
> 
> Well, it makes sense to return an error in this case, but if common
> practice is to ignore it in old WE based drivers, we should adhere to that
> to preserve userspace ABI compatibility.
> 
> So the real question is: Is there any userspace program that relies on
> this ABI detail?

That seems unlikely.  The only times I recall seeing this reported
is when a mac80211-based driver is used.  If there was something
depending on it, other drivers would be doing it, and I/we would see
this "error" reported elsewhere.

FWIW, the iwconfig semantic is "disabling encryption", not "deleting
the key".  Disabling encryption that is already disabled should be
treated as a no-op, not as an error.

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] mac80211: silently accept deletion of non-existant key

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 322 bytes --]


> FWIW, the iwconfig semantic is "disabling encryption", not "deleting
> the key".  Disabling encryption that is already disabled should be
> treated as a no-op, not as an error.

I wish semantics were actually documented ;) I guess you can see it that
way, feel free to apply the patch based on that.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] mac80211: silently accept deletion of non-existant key

[John W. Linville]

On Wed, Mar 26, 2008 at 01:47:54PM +0100, Johannes Berg wrote:
> 
> > FWIW, the iwconfig semantic is "disabling encryption", not "deleting
> > the key".  Disabling encryption that is already disabled should be
> > treated as a no-op, not as an error.
> 
> I wish semantics were actually documented ;) I guess you can see it that
> way, feel free to apply the patch based on that.

Just going by the man page...still, I know what you mean! :-)

I'll apply it...thanks!

John
-- 
John W. Linville
linville@tuxdriver.com