* [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM @ 2015-08-31 19:19 Ondrej Zary 2015-08-31 19:19 ` [PATCH 2/2] airo: Implement netif_carrier_on/off Ondrej Zary 2015-08-31 20:44 ` [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM Dan Williams 0 siblings, 2 replies; 8+ messages in thread From: Ondrej Zary @ 2015-08-31 19:19 UTC (permalink / raw) To: netdev; +Cc: Kernel development list Handle IW_AUTH_ALG_OPEN_SYSTEM in set_auth. This allows wpa_supplicant (and thus NetworkManager) to work with open APs. Signed-off-by: Ondrej Zary <linux@rainbow-software.org> --- drivers/net/wireless/airo.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c index d0c97c2..2066a1f 100644 --- a/drivers/net/wireless/airo.c +++ b/drivers/net/wireless/airo.c @@ -6670,10 +6670,9 @@ static int airo_set_auth(struct net_device *dev, break; case IW_AUTH_80211_AUTH_ALG: { - /* FIXME: What about AUTH_OPEN? This API seems to - * disallow setting our auth to AUTH_OPEN. - */ - if (param->value & IW_AUTH_ALG_SHARED_KEY) { + if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { + local->config.authType = AUTH_OPEN; + } else if (param->value & IW_AUTH_ALG_SHARED_KEY) { local->config.authType = AUTH_SHAREDKEY; } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { local->config.authType = AUTH_ENCRYPT; -- Ondrej Zary ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] airo: Implement netif_carrier_on/off 2015-08-31 19:19 [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM Ondrej Zary @ 2015-08-31 19:19 ` Ondrej Zary 2015-08-31 20:44 ` [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM Dan Williams 1 sibling, 0 replies; 8+ messages in thread From: Ondrej Zary @ 2015-08-31 19:19 UTC (permalink / raw) To: netdev; +Cc: Kernel development list Add calls to netif_carrier_on and netif_carrier_off Signed-off-by: Ondrej Zary <linux@rainbow-software.org> --- drivers/net/wireless/airo.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c index 2066a1f..1a9ddcd 100644 --- a/drivers/net/wireless/airo.c +++ b/drivers/net/wireless/airo.c @@ -3266,6 +3266,7 @@ static void airo_handle_link(struct airo_info *ai) wake_up_interruptible(&ai->thr_wait); } else airo_send_event(ai->dev); + netif_carrier_on(ai->dev); } else if (!scan_forceloss) { if (auto_wep && !ai->expires) { ai->expires = RUN_AT(3*HZ); @@ -3276,7 +3277,9 @@ static void airo_handle_link(struct airo_info *ai) eth_zero_addr(wrqu.ap_addr.sa_data); wrqu.ap_addr.sa_family = ARPHRD_ETHER; wireless_send_event(ai->dev, SIOCGIWAP, &wrqu, NULL); - } + netif_carrier_off(ai->dev); + } else + netif_carrier_off(ai->dev); } static void airo_handle_rx(struct airo_info *ai) @@ -3612,6 +3615,7 @@ static void disable_MAC( struct airo_info *ai, int lock ) { return; if (test_bit(FLAG_ENABLED, &ai->flags)) { + netif_carrier_off(ai->dev); memset(&cmd, 0, sizeof(cmd)); cmd.cmd = MAC_DISABLE; // disable in case already enabled issuecommand(ai, &cmd, &rsp); -- Ondrej Zary ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM 2015-08-31 19:19 [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM Ondrej Zary 2015-08-31 19:19 ` [PATCH 2/2] airo: Implement netif_carrier_on/off Ondrej Zary @ 2015-08-31 20:44 ` Dan Williams 2015-08-31 22:12 ` Ondrej Zary 1 sibling, 1 reply; 8+ messages in thread From: Dan Williams @ 2015-08-31 20:44 UTC (permalink / raw) To: Ondrej Zary; +Cc: netdev, Kernel development list On Mon, 2015-08-31 at 21:19 +0200, Ondrej Zary wrote: > Handle IW_AUTH_ALG_OPEN_SYSTEM in set_auth. > This allows wpa_supplicant (and thus NetworkManager) to work with open APs. > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org> > --- > drivers/net/wireless/airo.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c > index d0c97c2..2066a1f 100644 > --- a/drivers/net/wireless/airo.c > +++ b/drivers/net/wireless/airo.c > @@ -6670,10 +6670,9 @@ static int airo_set_auth(struct net_device *dev, > break; > > case IW_AUTH_80211_AUTH_ALG: { > - /* FIXME: What about AUTH_OPEN? This API seems to > - * disallow setting our auth to AUTH_OPEN. > - */ > - if (param->value & IW_AUTH_ALG_SHARED_KEY) { > + if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { > + local->config.authType = AUTH_OPEN; > + } else if (param->value & IW_AUTH_ALG_SHARED_KEY) { > local->config.authType = AUTH_SHAREDKEY; > } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { > local->config.authType = AUTH_ENCRYPT; NAK; there are two problems with this patch. First, there's already an if test for OPEN_SYSTEM which sets authType to AUTH_ENCRYPT. Second, AUTH_OPEN means to disable encryption entirely. The decision being made here is whether to use Shared Key or Open authentication, not whether encryption is being used or not. Thus this patch would appear to break most WEP APs? Airo really wants to know the auth type *and* whether encryption will actually be used at the same time, and we don't have that information here. I guess the only thing you can do here is call get_wep_key() for all the indexes and see if any keys are set, and if any keys are set, use AUTH_ENCRYPT. If get_wep_key() returns -1 for all 4 indexes, use AUTH_OPEN. But you have to make sure that this all gets protected by local->wep_capable and that you're not checking indexes above ai->max_wep_idx. Yay airo! Dan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM 2015-08-31 20:44 ` [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM Dan Williams @ 2015-08-31 22:12 ` Ondrej Zary 2015-09-01 0:04 ` Dan Williams 0 siblings, 1 reply; 8+ messages in thread From: Ondrej Zary @ 2015-08-31 22:12 UTC (permalink / raw) To: Dan Williams; +Cc: netdev, Kernel development list On Monday 31 August 2015 22:44:54 Dan Williams wrote: > On Mon, 2015-08-31 at 21:19 +0200, Ondrej Zary wrote: > > Handle IW_AUTH_ALG_OPEN_SYSTEM in set_auth. > > This allows wpa_supplicant (and thus NetworkManager) to work with open > > APs. > > > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org> > > --- > > drivers/net/wireless/airo.c | 7 +++---- > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c > > index d0c97c2..2066a1f 100644 > > --- a/drivers/net/wireless/airo.c > > +++ b/drivers/net/wireless/airo.c > > @@ -6670,10 +6670,9 @@ static int airo_set_auth(struct net_device *dev, > > break; > > > > case IW_AUTH_80211_AUTH_ALG: { > > - /* FIXME: What about AUTH_OPEN? This API seems to > > - * disallow setting our auth to AUTH_OPEN. > > - */ > > - if (param->value & IW_AUTH_ALG_SHARED_KEY) { > > + if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { > > + local->config.authType = AUTH_OPEN; > > + } else if (param->value & IW_AUTH_ALG_SHARED_KEY) { > > local->config.authType = AUTH_SHAREDKEY; > > } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { > > local->config.authType = AUTH_ENCRYPT; > > NAK; there are two problems with this patch. First, there's already an > if test for OPEN_SYSTEM which sets authType to AUTH_ENCRYPT. Second, > AUTH_OPEN means to disable encryption entirely. The decision being made > here is whether to use Shared Key or Open authentication, not whether > encryption is being used or not. Thus this patch would appear to break > most WEP APs? > > Airo really wants to know the auth type *and* whether encryption will > actually be used at the same time, and we don't have that information > here. I guess the only thing you can do here is call get_wep_key() for > all the indexes and see if any keys are set, and if any keys are set, > use AUTH_ENCRYPT. If get_wep_key() returns -1 for all 4 indexes, use > AUTH_OPEN. But you have to make sure that this all gets protected by > local->wep_capable and that you're not checking indexes above > ai->max_wep_idx. Yay airo! Sorry, I got confused (and it worked with WEP with a test AP, although there's no open system/shared key setting in the firmware). Reading the wpa_supplicant code, it uses IW_AUTH_ALG_OPEN_SYSTEM for WEP open system and also as a default value - which gets used when encryption is disabled: static int wpa_driver_wext_set_auth_alg(void *priv, int auth_alg) { struct wpa_driver_wext_data *drv = priv; int algs = 0, res; if (auth_alg & WPA_AUTH_ALG_OPEN) algs |= IW_AUTH_ALG_OPEN_SYSTEM; if (auth_alg & WPA_AUTH_ALG_SHARED) algs |= IW_AUTH_ALG_SHARED_KEY; if (auth_alg & WPA_AUTH_ALG_LEAP) algs |= IW_AUTH_ALG_LEAP; if (algs == 0) { /* at least one algorithm should be set */ algs = IW_AUTH_ALG_OPEN_SYSTEM; } res = wpa_driver_wext_set_auth_param(drv, IW_AUTH_80211_AUTH_ALG, algs); drv->auth_alg_fallback = res == -2; return res; } However, when SIOCSIWAUTH fails with EOPNOTSUPP, it tries SIOCSIWENCODE. This patch seems to work too with my AP: diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c index d0c97c2..2610fe3 100644 --- a/drivers/net/wireless/airo.c +++ b/drivers/net/wireless/airo.c @@ -6670,14 +6670,17 @@ static int airo_set_auth(struct net_device *dev, break; case IW_AUTH_80211_AUTH_ALG: { - /* FIXME: What about AUTH_OPEN? This API seems to - * disallow setting our auth to AUTH_OPEN. + /* + * IW_AUTH_ALG_OPEN_SYSTEM is ambiguous here for WEP as + * wpa_supplicant uses it for both no encryption and + * WEP open system. So we return -EOPNOTSUPP and + * wpa_supplicant will use SIOCSIWENCODE instead. */ - if (param->value & IW_AUTH_ALG_SHARED_KEY) { + if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) + return -EOPNOTSUPP; + if (param->value & IW_AUTH_ALG_SHARED_KEY) local->config.authType = AUTH_SHAREDKEY; - } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { - local->config.authType = AUTH_ENCRYPT; - } else + else return -EINVAL; /* Commit the changes to flags if needed */ -- Ondrej Zary ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM 2015-08-31 22:12 ` Ondrej Zary @ 2015-09-01 0:04 ` Dan Williams 2015-09-01 22:21 ` Ondrej Zary 0 siblings, 1 reply; 8+ messages in thread From: Dan Williams @ 2015-09-01 0:04 UTC (permalink / raw) To: Ondrej Zary; +Cc: netdev, Kernel development list On Tue, 2015-09-01 at 00:12 +0200, Ondrej Zary wrote: > > On Monday 31 August 2015 22:44:54 Dan Williams wrote: > > On Mon, 2015-08-31 at 21:19 +0200, Ondrej Zary wrote: > > > Handle IW_AUTH_ALG_OPEN_SYSTEM in set_auth. > > > This allows wpa_supplicant (and thus NetworkManager) to work with open > > > APs. > > > > > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org> > > > --- > > > drivers/net/wireless/airo.c | 7 +++---- > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c > > > index d0c97c2..2066a1f 100644 > > > --- a/drivers/net/wireless/airo.c > > > +++ b/drivers/net/wireless/airo.c > > > @@ -6670,10 +6670,9 @@ static int airo_set_auth(struct net_device *dev, > > > break; > > > > > > case IW_AUTH_80211_AUTH_ALG: { > > > - /* FIXME: What about AUTH_OPEN? This API seems to > > > - * disallow setting our auth to AUTH_OPEN. > > > - */ > > > - if (param->value & IW_AUTH_ALG_SHARED_KEY) { > > > + if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { > > > + local->config.authType = AUTH_OPEN; > > > + } else if (param->value & IW_AUTH_ALG_SHARED_KEY) { > > > local->config.authType = AUTH_SHAREDKEY; > > > } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { > > > local->config.authType = AUTH_ENCRYPT; > > > > NAK; there are two problems with this patch. First, there's already an > > if test for OPEN_SYSTEM which sets authType to AUTH_ENCRYPT. Second, > > AUTH_OPEN means to disable encryption entirely. The decision being made > > here is whether to use Shared Key or Open authentication, not whether > > encryption is being used or not. Thus this patch would appear to break > > most WEP APs? > > > > Airo really wants to know the auth type *and* whether encryption will > > actually be used at the same time, and we don't have that information > > here. I guess the only thing you can do here is call get_wep_key() for > > all the indexes and see if any keys are set, and if any keys are set, > > use AUTH_ENCRYPT. If get_wep_key() returns -1 for all 4 indexes, use > > AUTH_OPEN. But you have to make sure that this all gets protected by > > local->wep_capable and that you're not checking indexes above > > ai->max_wep_idx. Yay airo! > > Sorry, I got confused (and it worked with WEP with a test AP, although there's > no open system/shared key setting in the firmware). > > Reading the wpa_supplicant code, it uses IW_AUTH_ALG_OPEN_SYSTEM for WEP open > system and also as a default value - which gets used when encryption is > disabled: I think you're still confusing the relationship between Open System and WEP in the code and comments here. 802.11 always uses an "auth method" regardless of whether encryption is used or not. There are 3 possible settings here: Auth Encryption ------------------ OPEN NONE OPEN WEP SHARED WEP The problem here is that: 1) the WEXT SIWAUTH call only sets authentication, but says nothing about encryption 2) that airo is currently structured such that it wants both auth & encryption specified at the same time. It tracks the states in the table above with the authType variable, but at the point of SIWAUTH there is no information about whether the requested mode is OPEN+NONE or OPEN+WEP. The patches might work for some cases, but they are ignoring others where clients might send WEXT calls in different order. WEXT doesn't specify any kind of ordering, which is one reason it's been deprecated and replaced with nl80211. So in your case, the supplicant does [IWAUTH + IWENCODE] but that's just how the supplicant decided to implement it. If some other client does a perfectly legal [IWENCODE + IWAUTH] then your original patch will turn off WEP, when the client wanted OPEN + WEP. > static int wpa_driver_wext_set_auth_alg(void *priv, int auth_alg) > { > struct wpa_driver_wext_data *drv = priv; > int algs = 0, res; > > if (auth_alg & WPA_AUTH_ALG_OPEN) > algs |= IW_AUTH_ALG_OPEN_SYSTEM; > if (auth_alg & WPA_AUTH_ALG_SHARED) > algs |= IW_AUTH_ALG_SHARED_KEY; > if (auth_alg & WPA_AUTH_ALG_LEAP) > algs |= IW_AUTH_ALG_LEAP; > if (algs == 0) { > /* at least one algorithm should be set */ > algs = IW_AUTH_ALG_OPEN_SYSTEM; > } > > res = wpa_driver_wext_set_auth_param(drv, IW_AUTH_80211_AUTH_ALG, > algs); > drv->auth_alg_fallback = res == -2; > return res; > } > > > However, when SIOCSIWAUTH fails with EOPNOTSUPP, it tries SIOCSIWENCODE. This > patch seems to work too with my AP: > > diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c > index d0c97c2..2610fe3 100644 > --- a/drivers/net/wireless/airo.c > +++ b/drivers/net/wireless/airo.c > @@ -6670,14 +6670,17 @@ static int airo_set_auth(struct net_device *dev, > break; > > case IW_AUTH_80211_AUTH_ALG: { > - /* FIXME: What about AUTH_OPEN? This API seems to > - * disallow setting our auth to AUTH_OPEN. > + /* > + * IW_AUTH_ALG_OPEN_SYSTEM is ambiguous here for WEP as > + * wpa_supplicant uses it for both no encryption and > + * WEP open system. So we return -EOPNOTSUPP and > + * wpa_supplicant will use SIOCSIWENCODE instead. It's not really the supplicant that's ambiguous, the supplicant is doing stuff that makes sense. Unfortunately the WEXT API is what is ambiguous here. Plus, even though wpa_supplicant is the de-facto standard, the kernel should treat the supplicant specially and we shouldn't add hacks for specific programs. Let's see if a general solution can be found. > */ > - if (param->value & IW_AUTH_ALG_SHARED_KEY) { > + if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) > + return -EOPNOTSUPP; While it works due to wpa_supplicant behavior, it's a hack. It's perfectly legal to set OPEN_SYSTEM mode in SIWAUTH. It's just that airo is so simple in how it handles WEXT that it doesn't have enough information to do the right thing. What ipw2200 does is cache values in the driver struct so it has everything it needs all the time. So what I'm saying is that your fix here isn't really a complete fix. A complete fix would work for all these scenarios: a) SIWAUTH(open), SIWENCODE(enable wep) = WEP + open system b) SIWENCODE(enable wep), SIWAUTH(open) = WEP + open system c) SIWAUTH(open), SIWENCODE(disable WEP) = unencrypted + open system d) SIWENCODE(disable WEP), SIWAUTH(open) = unencrypted + open system and that complete fix might well be caching the IW_AUTH_ALG value in a couple places (in the SIWAUTH handler and the SIWENCODE and SIWENCODEEXT handlers) and also whether WEP is enabled or not, and then using both of those values to set authType in SIWAUTH. Dan > + if (param->value & IW_AUTH_ALG_SHARED_KEY) > local->config.authType = AUTH_SHAREDKEY; > - } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { > - local->config.authType = AUTH_ENCRYPT; > - } else > + else > return -EINVAL; > > /* Commit the changes to flags if needed */ > > > > -- > Ondrej Zary > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM 2015-09-01 0:04 ` Dan Williams @ 2015-09-01 22:21 ` Ondrej Zary 0 siblings, 0 replies; 8+ messages in thread From: Ondrej Zary @ 2015-09-01 22:21 UTC (permalink / raw) To: Dan Williams; +Cc: netdev, Kernel development list On Tuesday 01 September 2015 02:04:43 Dan Williams wrote: > On Tue, 2015-09-01 at 00:12 +0200, Ondrej Zary wrote: > > On Monday 31 August 2015 22:44:54 Dan Williams wrote: > > > On Mon, 2015-08-31 at 21:19 +0200, Ondrej Zary wrote: > > > > Handle IW_AUTH_ALG_OPEN_SYSTEM in set_auth. > > > > This allows wpa_supplicant (and thus NetworkManager) to work with > > > > open APs. > > > > > > > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org> > > > > --- > > > > drivers/net/wireless/airo.c | 7 +++---- > > > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/net/wireless/airo.c > > > > b/drivers/net/wireless/airo.c index d0c97c2..2066a1f 100644 > > > > --- a/drivers/net/wireless/airo.c > > > > +++ b/drivers/net/wireless/airo.c > > > > @@ -6670,10 +6670,9 @@ static int airo_set_auth(struct net_device > > > > *dev, break; > > > > > > > > case IW_AUTH_80211_AUTH_ALG: { > > > > - /* FIXME: What about AUTH_OPEN? This API seems to > > > > - * disallow setting our auth to AUTH_OPEN. > > > > - */ > > > > - if (param->value & IW_AUTH_ALG_SHARED_KEY) { > > > > + if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { > > > > + local->config.authType = AUTH_OPEN; > > > > + } else if (param->value & IW_AUTH_ALG_SHARED_KEY) { > > > > local->config.authType = AUTH_SHAREDKEY; > > > > } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { > > > > local->config.authType = AUTH_ENCRYPT; > > > > > > NAK; there are two problems with this patch. First, there's already an > > > if test for OPEN_SYSTEM which sets authType to AUTH_ENCRYPT. Second, > > > AUTH_OPEN means to disable encryption entirely. The decision being > > > made here is whether to use Shared Key or Open authentication, not > > > whether encryption is being used or not. Thus this patch would appear > > > to break most WEP APs? > > > > > > Airo really wants to know the auth type *and* whether encryption will > > > actually be used at the same time, and we don't have that information > > > here. I guess the only thing you can do here is call get_wep_key() for > > > all the indexes and see if any keys are set, and if any keys are set, > > > use AUTH_ENCRYPT. If get_wep_key() returns -1 for all 4 indexes, use > > > AUTH_OPEN. But you have to make sure that this all gets protected by > > > local->wep_capable and that you're not checking indexes above > > > ai->max_wep_idx. Yay airo! > > > > Sorry, I got confused (and it worked with WEP with a test AP, although > > there's no open system/shared key setting in the firmware). > > > > Reading the wpa_supplicant code, it uses IW_AUTH_ALG_OPEN_SYSTEM for WEP > > open system and also as a default value - which gets used when encryption > > is disabled: > > I think you're still confusing the relationship between Open System and > WEP in the code and comments here. 802.11 always uses an "auth method" > regardless of whether encryption is used or not. There are 3 possible > settings here: > > Auth Encryption > ------------------ > OPEN NONE > OPEN WEP > SHARED WEP > > The problem here is that: > > 1) the WEXT SIWAUTH call only sets authentication, but says nothing > about encryption > > 2) that airo is currently structured such that it wants both auth & > encryption specified at the same time. It tracks the states in the > table above with the authType variable, but at the point of SIWAUTH > there is no information about whether the requested mode is OPEN+NONE or > OPEN+WEP. > > The patches might work for some cases, but they are ignoring others > where clients might send WEXT calls in different order. WEXT doesn't > specify any kind of ordering, which is one reason it's been deprecated > and replaced with nl80211. So in your case, the supplicant does [IWAUTH > + IWENCODE] but that's just how the supplicant decided to implement it. > If some other client does a perfectly legal [IWENCODE + IWAUTH] then > your original patch will turn off WEP, when the client wanted OPEN + > WEP. > > > static int wpa_driver_wext_set_auth_alg(void *priv, int auth_alg) > > { > > struct wpa_driver_wext_data *drv = priv; > > int algs = 0, res; > > > > if (auth_alg & WPA_AUTH_ALG_OPEN) > > algs |= IW_AUTH_ALG_OPEN_SYSTEM; > > if (auth_alg & WPA_AUTH_ALG_SHARED) > > algs |= IW_AUTH_ALG_SHARED_KEY; > > if (auth_alg & WPA_AUTH_ALG_LEAP) > > algs |= IW_AUTH_ALG_LEAP; > > if (algs == 0) { > > /* at least one algorithm should be set */ > > algs = IW_AUTH_ALG_OPEN_SYSTEM; > > } > > > > res = wpa_driver_wext_set_auth_param(drv, IW_AUTH_80211_AUTH_ALG, > > algs); > > drv->auth_alg_fallback = res == -2; > > return res; > > } > > > > > > However, when SIOCSIWAUTH fails with EOPNOTSUPP, it tries SIOCSIWENCODE. > > This patch seems to work too with my AP: > > > > diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c > > index d0c97c2..2610fe3 100644 > > --- a/drivers/net/wireless/airo.c > > +++ b/drivers/net/wireless/airo.c > > @@ -6670,14 +6670,17 @@ static int airo_set_auth(struct net_device *dev, > > break; > > > > case IW_AUTH_80211_AUTH_ALG: { > > - /* FIXME: What about AUTH_OPEN? This API seems to > > - * disallow setting our auth to AUTH_OPEN. > > + /* > > + * IW_AUTH_ALG_OPEN_SYSTEM is ambiguous here for WEP as > > + * wpa_supplicant uses it for both no encryption and > > + * WEP open system. So we return -EOPNOTSUPP and > > + * wpa_supplicant will use SIOCSIWENCODE instead. > > It's not really the supplicant that's ambiguous, the supplicant is doing > stuff that makes sense. Unfortunately the WEXT API is what is ambiguous > here. Plus, even though wpa_supplicant is the de-facto standard, the > kernel should treat the supplicant specially and we shouldn't add hacks > for specific programs. Let's see if a general solution can be found. > > > */ > > - if (param->value & IW_AUTH_ALG_SHARED_KEY) { > > + if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) > > + return -EOPNOTSUPP; > > While it works due to wpa_supplicant behavior, it's a hack. It's > perfectly legal to set OPEN_SYSTEM mode in SIWAUTH. It's just that airo > is so simple in how it handles WEXT that it doesn't have enough > information to do the right thing. What ipw2200 does is cache values in > the driver struct so it has everything it needs all the time. > > So what I'm saying is that your fix here isn't really a complete fix. A > complete fix would work for all these scenarios: > > a) SIWAUTH(open), SIWENCODE(enable wep) = WEP + open system > b) SIWENCODE(enable wep), SIWAUTH(open) = WEP + open system > c) SIWAUTH(open), SIWENCODE(disable WEP) = unencrypted + open system > d) SIWENCODE(disable WEP), SIWAUTH(open) = unencrypted + open system > > and that complete fix might well be caching the IW_AUTH_ALG value in a > couple places (in the SIWAUTH handler and the SIWENCODE and SIWENCODEEXT > handlers) and also whether WEP is enabled or not, and then using both of > those values to set authType in SIWAUTH. Is this enough? It seems to work. diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c index d0c97c2..a8f2767 100644 --- a/drivers/net/wireless/airo.c +++ b/drivers/net/wireless/airo.c @@ -1237,6 +1237,7 @@ struct airo_info { int wep_capable; int max_wep_idx; + int last_auth; /* WPA-related stuff */ unsigned int bssListFirst; @@ -3863,6 +3864,7 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock) } ai->config.opmode = adhoc ? MODE_STA_IBSS : MODE_STA_ESS; ai->config.authType = AUTH_OPEN; + ai->last_auth = AUTH_OPEN; ai->config.modulation = MOD_CCK; if (le16_to_cpu(cap_rid.len) >= sizeof(cap_rid) && @@ -6370,6 +6372,7 @@ static int airo_set_encode(struct net_device *dev, if((index == current_index) && (key.len > 0) && (local->config.authType == AUTH_OPEN)) { local->config.authType = AUTH_ENCRYPT; + local->last_auth = AUTH_ENCRYPT; } } else { /* Do we want to just set the transmit key index ? */ @@ -6389,12 +6392,16 @@ static int airo_set_encode(struct net_device *dev, } } /* Read the flags */ - if(dwrq->flags & IW_ENCODE_DISABLED) + if (dwrq->flags & IW_ENCODE_DISABLED) { local->config.authType = AUTH_OPEN; // disable encryption + local->last_auth = AUTH_OPEN; + } if(dwrq->flags & IW_ENCODE_RESTRICTED) local->config.authType = AUTH_SHAREDKEY; // Only Both - if(dwrq->flags & IW_ENCODE_OPEN) + if (dwrq->flags & IW_ENCODE_OPEN) { local->config.authType = AUTH_ENCRYPT; // Only Wep + local->last_auth = AUTH_ENCRYPT; + } /* Commit the changes to flags if needed */ if (local->config.authType != currentAuthType) set_bit (FLAG_COMMIT, &local->flags); @@ -6549,12 +6556,16 @@ static int airo_set_encodeext(struct net_device *dev, } /* Read the flags */ - if(encoding->flags & IW_ENCODE_DISABLED) + if (encoding->flags & IW_ENCODE_DISABLED) { local->config.authType = AUTH_OPEN; // disable encryption + local->last_auth = AUTH_OPEN; + } if(encoding->flags & IW_ENCODE_RESTRICTED) local->config.authType = AUTH_SHAREDKEY; // Only Both - if(encoding->flags & IW_ENCODE_OPEN) + if (encoding->flags & IW_ENCODE_OPEN) { local->config.authType = AUTH_ENCRYPT; // Only Wep + local->last_auth = AUTH_ENCRYPT; + } /* Commit the changes to flags if needed */ if (local->config.authType != currentAuthType) set_bit (FLAG_COMMIT, &local->flags); @@ -6658,10 +6669,13 @@ static int airo_set_auth(struct net_device *dev, case IW_AUTH_DROP_UNENCRYPTED: if (param->value) { /* Only change auth type if unencrypted */ - if (currentAuthType == AUTH_OPEN) + if (currentAuthType == AUTH_OPEN) { local->config.authType = AUTH_ENCRYPT; + local->last_auth = AUTH_ENCRYPT; + } } else { local->config.authType = AUTH_OPEN; + local->last_auth = AUTH_OPEN; } /* Commit the changes to flags if needed */ @@ -6670,13 +6684,14 @@ static int airo_set_auth(struct net_device *dev, break; case IW_AUTH_80211_AUTH_ALG: { - /* FIXME: What about AUTH_OPEN? This API seems to - * disallow setting our auth to AUTH_OPEN. - */ if (param->value & IW_AUTH_ALG_SHARED_KEY) { local->config.authType = AUTH_SHAREDKEY; } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { - local->config.authType = AUTH_ENCRYPT; + /* We don't know here if WEP open system or + * unencrypted mode was requested - so use the + * last mode (of these two) used last time + */ + local->config.authType = local->last_auth; } else return -EINVAL; -- Ondrej Zary ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/2 v2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM @ 2015-09-15 15:18 Ondrej Zary 2015-09-15 15:18 ` [PATCH 2/2] airo: Implement netif_carrier_on/off Ondrej Zary 0 siblings, 1 reply; 8+ messages in thread From: Ondrej Zary @ 2015-09-15 15:18 UTC (permalink / raw) To: Dan Williams; +Cc: netdev, Kernel development list IW_AUTH_ALG_OPEN_SYSTEM is ambiguous in set_auth for WEP as wpa_supplicant uses it for both no encryption and WEP open system. Cache the last mode set (only of these two) and use it here. This allows wpa_supplicant to work with unencrypted APs. Signed-off-by: Ondrej Zary <linux@rainbow-software.org> --- drivers/net/wireless/airo.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c index d0c97c2..a8f2767 100644 --- a/drivers/net/wireless/airo.c +++ b/drivers/net/wireless/airo.c @@ -1237,6 +1237,7 @@ struct airo_info { int wep_capable; int max_wep_idx; + int last_auth; /* WPA-related stuff */ unsigned int bssListFirst; @@ -3863,6 +3864,7 @@ static u16 setup_card(struct airo_info *ai, u8 *mac, int lock) } ai->config.opmode = adhoc ? MODE_STA_IBSS : MODE_STA_ESS; ai->config.authType = AUTH_OPEN; + ai->last_auth = AUTH_OPEN; ai->config.modulation = MOD_CCK; if (le16_to_cpu(cap_rid.len) >= sizeof(cap_rid) && @@ -6370,6 +6372,7 @@ static int airo_set_encode(struct net_device *dev, if((index == current_index) && (key.len > 0) && (local->config.authType == AUTH_OPEN)) { local->config.authType = AUTH_ENCRYPT; + local->last_auth = AUTH_ENCRYPT; } } else { /* Do we want to just set the transmit key index ? */ @@ -6389,12 +6392,16 @@ static int airo_set_encode(struct net_device *dev, } } /* Read the flags */ - if(dwrq->flags & IW_ENCODE_DISABLED) + if (dwrq->flags & IW_ENCODE_DISABLED) { local->config.authType = AUTH_OPEN; // disable encryption + local->last_auth = AUTH_OPEN; + } if(dwrq->flags & IW_ENCODE_RESTRICTED) local->config.authType = AUTH_SHAREDKEY; // Only Both - if(dwrq->flags & IW_ENCODE_OPEN) + if (dwrq->flags & IW_ENCODE_OPEN) { local->config.authType = AUTH_ENCRYPT; // Only Wep + local->last_auth = AUTH_ENCRYPT; + } /* Commit the changes to flags if needed */ if (local->config.authType != currentAuthType) set_bit (FLAG_COMMIT, &local->flags); @@ -6549,12 +6556,16 @@ static int airo_set_encodeext(struct net_device *dev, } /* Read the flags */ - if(encoding->flags & IW_ENCODE_DISABLED) + if (encoding->flags & IW_ENCODE_DISABLED) { local->config.authType = AUTH_OPEN; // disable encryption + local->last_auth = AUTH_OPEN; + } if(encoding->flags & IW_ENCODE_RESTRICTED) local->config.authType = AUTH_SHAREDKEY; // Only Both - if(encoding->flags & IW_ENCODE_OPEN) + if (encoding->flags & IW_ENCODE_OPEN) { local->config.authType = AUTH_ENCRYPT; // Only Wep + local->last_auth = AUTH_ENCRYPT; + } /* Commit the changes to flags if needed */ if (local->config.authType != currentAuthType) set_bit (FLAG_COMMIT, &local->flags); @@ -6658,10 +6669,13 @@ static int airo_set_auth(struct net_device *dev, case IW_AUTH_DROP_UNENCRYPTED: if (param->value) { /* Only change auth type if unencrypted */ - if (currentAuthType == AUTH_OPEN) + if (currentAuthType == AUTH_OPEN) { local->config.authType = AUTH_ENCRYPT; + local->last_auth = AUTH_ENCRYPT; + } } else { local->config.authType = AUTH_OPEN; + local->last_auth = AUTH_OPEN; } /* Commit the changes to flags if needed */ @@ -6670,13 +6684,14 @@ static int airo_set_auth(struct net_device *dev, break; case IW_AUTH_80211_AUTH_ALG: { - /* FIXME: What about AUTH_OPEN? This API seems to - * disallow setting our auth to AUTH_OPEN. - */ if (param->value & IW_AUTH_ALG_SHARED_KEY) { local->config.authType = AUTH_SHAREDKEY; } else if (param->value & IW_AUTH_ALG_OPEN_SYSTEM) { - local->config.authType = AUTH_ENCRYPT; + /* We don't know here if WEP open system or + * unencrypted mode was requested - so use the + * last mode (of these two) used last time + */ + local->config.authType = local->last_auth; } else return -EINVAL; -- Ondrej Zary ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] airo: Implement netif_carrier_on/off 2015-09-15 15:18 [PATCH 1/2 v2] " Ondrej Zary @ 2015-09-15 15:18 ` Ondrej Zary 2015-09-16 14:30 ` Sergei Shtylyov 0 siblings, 1 reply; 8+ messages in thread From: Ondrej Zary @ 2015-09-15 15:18 UTC (permalink / raw) To: Dan Williams; +Cc: netdev, Kernel development list Add calls to netif_carrier_on and netif_carrier_off Signed-off-by: Ondrej Zary <linux@rainbow-software.org> --- drivers/net/wireless/airo.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c index a8f2767..629245c 100644 --- a/drivers/net/wireless/airo.c +++ b/drivers/net/wireless/airo.c @@ -3267,6 +3267,7 @@ static void airo_handle_link(struct airo_info *ai) wake_up_interruptible(&ai->thr_wait); } else airo_send_event(ai->dev); + netif_carrier_on(ai->dev); } else if (!scan_forceloss) { if (auto_wep && !ai->expires) { ai->expires = RUN_AT(3*HZ); @@ -3277,7 +3278,9 @@ static void airo_handle_link(struct airo_info *ai) eth_zero_addr(wrqu.ap_addr.sa_data); wrqu.ap_addr.sa_family = ARPHRD_ETHER; wireless_send_event(ai->dev, SIOCGIWAP, &wrqu, NULL); - } + netif_carrier_off(ai->dev); + } else + netif_carrier_off(ai->dev); } static void airo_handle_rx(struct airo_info *ai) @@ -3613,6 +3616,7 @@ static void disable_MAC( struct airo_info *ai, int lock ) { return; if (test_bit(FLAG_ENABLED, &ai->flags)) { + netif_carrier_off(ai->dev); memset(&cmd, 0, sizeof(cmd)); cmd.cmd = MAC_DISABLE; // disable in case already enabled issuecommand(ai, &cmd, &rsp); -- Ondrej Zary ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] airo: Implement netif_carrier_on/off 2015-09-15 15:18 ` [PATCH 2/2] airo: Implement netif_carrier_on/off Ondrej Zary @ 2015-09-16 14:30 ` Sergei Shtylyov 0 siblings, 0 replies; 8+ messages in thread From: Sergei Shtylyov @ 2015-09-16 14:30 UTC (permalink / raw) To: Ondrej Zary, Dan Williams; +Cc: netdev, Kernel development list Hello. On 9/15/2015 6:18 PM, Ondrej Zary wrote: > Add calls to netif_carrier_on and netif_carrier_off > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org> > --- > drivers/net/wireless/airo.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c > index a8f2767..629245c 100644 > --- a/drivers/net/wireless/airo.c > +++ b/drivers/net/wireless/airo.c [...] > @@ -3277,7 +3278,9 @@ static void airo_handle_link(struct airo_info *ai) > eth_zero_addr(wrqu.ap_addr.sa_data); > wrqu.ap_addr.sa_family = ARPHRD_ETHER; > wireless_send_event(ai->dev, SIOCGIWAP, &wrqu, NULL); > - } > + netif_carrier_off(ai->dev); > + } else > + netif_carrier_off(ai->dev); Need {} in all branches, according the the Documentation/CodingStyle. > } > > static void airo_handle_rx(struct airo_info *ai) [...] MBR, Sergei ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-09-16 14:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-31 19:19 [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM Ondrej Zary 2015-08-31 19:19 ` [PATCH 2/2] airo: Implement netif_carrier_on/off Ondrej Zary 2015-08-31 20:44 ` [PATCH 1/2] airo: fix IW_AUTH_ALG_OPEN_SYSTEM Dan Williams 2015-08-31 22:12 ` Ondrej Zary 2015-09-01 0:04 ` Dan Williams 2015-09-01 22:21 ` Ondrej Zary -- strict thread matches above, loose matches on Subject: below -- 2015-09-15 15:18 [PATCH 1/2 v2] " Ondrej Zary 2015-09-15 15:18 ` [PATCH 2/2] airo: Implement netif_carrier_on/off Ondrej Zary 2015-09-16 14:30 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).