Intel PRO/Wireless 3945ABG (ipw3945)

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: Rafael Laufer <rlaufer@cs...> - 2009-08-21 00:18 This patch worked for me. Thanks Gábor Rafael Gábor Stefanik wrote: > Commit 1ccb84d87d04df3c76cd4352fe69786d8c7cf016 by Wey-Yi Guy > ("iwlwifi: clean up unused NL80211_IFTYPE_MONITOR for Monitor mode") > broke injection of non-broadcast frames to unassociated stations > (causing a SYSASSERT for all such injected frames), due to injected > frames no longer automatically getting a broadcast station ID assigned, > and instead ending up with invalid station IDs. > This patch restores the old behavior, fixing the aforementioned > regression. > > Signed-off-by: Gábor Stefanik <netrolller.3d@gmail.com> > --- > This fixes a regression introduced between 2.6.30 and 2.6.31-rc1. > Please apply to 2.6.31. > > drivers/net/wireless/iwlwifi/iwl-sta.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/wireless/iwlwifi/iwl-sta.c > b/drivers/net/wireless/iwlwifi/iwl-sta.c > index c6633fe..81a656f 100644 > --- a/drivers/net/wireless/iwlwifi/iwl-sta.c > +++ b/drivers/net/wireless/iwlwifi/iwl-sta.c > @@ -1054,7 +1054,8 @@ int iwl_get_sta_id(struct iwl_priv *priv, struct > ieee80211_hdr *hdr) > __le16 fc = hdr->frame_control; > > /* If this frame is broadcast or management, use broadcast station > id */ > - if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1)) > + if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1) || > + iwl_is_monitor_mode(priv)) /* Injected frames need broadcast > too */ > return priv->hw_params.bcast_sta_id; > > switch (priv->iw_mode) {

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: Johannes Berg <johannes@si...> - 2009-08-21 08:21 Attachments: Message as HTML      On Fri, 2009-08-21 at 00:08 +0200, Gábor Stefanik wrote: > /* If this frame is broadcast or management, use broadcast station id */ > - if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1)) > + if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1) || > + iwl_is_monitor_mode(priv)) /* Injected frames need broadcast too */ > return priv->hw_params.bcast_sta_id; Now you still can't transmit frames to another AP when connected to one, which will, for example, be required for 11r. IMHO that code should, in STATION mode, be checking the RA against the BSSID, and if they match use the AP ID, otherwise the bcast ID. And remove the warning in the default case, since that's what happens if in pure monitor mode afaict. johannes

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: Gábor Stefanik <netrolller.3d@gm...> - 2009-08-21 13:22 2009/8/21 Johannes Berg <johannes@sipsolutions.net>: > On Fri, 2009-08-21 at 00:08 +0200, Gábor Stefanik wrote: > >>       /* If this frame is broadcast or management, use broadcast station id */ >> -     if (!ieee80211_is_data(fc) ||  is_multicast_ether_addr(hdr->addr1)) >> +     if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1) || >> +         iwl_is_monitor_mode(priv)) /* Injected frames need broadcast too */ >>               return priv->hw_params.bcast_sta_id; > > Now you still can't transmit frames to another AP when connected to one, > which will, for example, be required for 11r. IMHO that code should, in > STATION mode, be checking the RA against the BSSID, and if they match > use the AP ID, otherwise the bcast ID. I'll probably do that, in a separate patch. This patch is a regression fix targeted at the next 2.6.31-rc (injection of non-broadcasts works fine in 2.6.30, but causes an ucode SYSASSERT in 2.6.31 and up). The problem you are describing is not a regression AFAIK, it's something that never worked; so it should be targeted @ 2.6.32. > And remove the warning in the > default case, since that's what happens if in pure monitor mode afaict. >From my limited testing, in monitor mode, the vif type will be IFTYPE_STATION, so the default case is never hit. (Is this a bug?) (BTW now that in mac80211, we internally use IFTYPE_MONITOR in monitor mode, as opposed to just checking for a missing STA - why can't we propagate this to the drivers, and have a much saner way of informing the driver of monitor mode? This is especially a problem for zd1211rw, where to get proper monitor mode, ZD_SNIFFER_ON needs to be turned on.) > > johannes > -- Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: Johannes Berg <johannes@si...> - 2009-08-21 13:28 Attachments: Message as HTML      On Fri, 2009-08-21 at 15:21 +0200, Gábor Stefanik wrote: > > And remove the warning in the > > default case, since that's what happens if in pure monitor mode afaict. > > From my limited testing, in monitor mode, the vif type will be > IFTYPE_STATION, so the default case is never hit. (Is this a bug?) It's the way iwlwifi handles things I guess. > (BTW now that in mac80211, we internally use IFTYPE_MONITOR in monitor > mode, as opposed to just checking for a missing STA - why can't we > propagate this to the drivers, and have a much saner way of informing > the driver of monitor mode? This is especially a problem for zd1211rw, > where to get proper monitor mode, ZD_SNIFFER_ON needs to be turned > on.) Think about it again (hint: multiple interfaces), and then ask the question again. johannes

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: reinette chatre <reinette.chatre@in...> - 2009-08-21 16:34 Hi Gábor, On Thu, 2009-08-20 at 15:08 -0700, Gábor Stefanik wrote: > Commit 1ccb84d87d04df3c76cd4352fe69786d8c7cf016 by Wey-Yi Guy > ("iwlwifi: clean up unused NL80211_IFTYPE_MONITOR for Monitor mode") > broke injection of non-broadcast frames to unassociated stations > (causing a SYSASSERT for all such injected frames), due to injected > frames no longer automatically getting a broadcast station ID assigned, > and instead ending up with invalid station IDs. > This patch restores the old behavior, fixing the aforementioned > regression. The patch you are quoting cannot be the culprit here. As that commit message indicates we never set NL80211_IFTYPE_MONITOR for the interface type, so when that code removed the test for this interface type in iwl_get_sta_id it essentially removed dead code. > /* If this frame is broadcast or management, use broadcast station id */ > - if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1)) > + if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1) || > + iwl_is_monitor_mode(priv)) /* Injected frames need broadcast too */ > return priv->hw_params.bcast_sta_id; I think my comment ties in with what Johannes said. When we are associated (whether in station, adhoc, or AP mode) we want this function to return the correct station ID. We also know that an interface can be in monitor mode while it is in any of these other modes. When this happens we do not want to return the broadcast station id ... we still want to return the station id. Your patch changes this behavior and will in this case always return the broadcast station id. Reinette

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: John W. Linville <linville@tu...> - 2009-08-21 16:45 On Fri, Aug 21, 2009 at 09:33:53AM -0700, reinette chatre wrote: > Hi Gábor, > > On Thu, 2009-08-20 at 15:08 -0700, Gábor Stefanik wrote: > > Commit 1ccb84d87d04df3c76cd4352fe69786d8c7cf016 by Wey-Yi Guy > > ("iwlwifi: clean up unused NL80211_IFTYPE_MONITOR for Monitor mode") > > broke injection of non-broadcast frames to unassociated stations > > (causing a SYSASSERT for all such injected frames), due to injected > > frames no longer automatically getting a broadcast station ID assigned, > > and instead ending up with invalid station IDs. > > This patch restores the old behavior, fixing the aforementioned > > regression. > > The patch you are quoting cannot be the culprit here. As that commit > message indicates we never set NL80211_IFTYPE_MONITOR for the interface > type, so when that code removed the test for this interface type in > iwl_get_sta_id it essentially removed dead code. > > > /* If this frame is broadcast or management, use broadcast station id */ > > - if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1)) > > + if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1) || > > + iwl_is_monitor_mode(priv)) /* Injected frames need broadcast too */ > > return priv->hw_params.bcast_sta_id; > > I think my comment ties in with what Johannes said. When we are > associated (whether in station, adhoc, or AP mode) we want this function > to return the correct station ID. We also know that an interface can be > in monitor mode while it is in any of these other modes. When this > happens we do not want to return the broadcast station id ... we still > want to return the station id. Your patch changes this behavior and will > in this case always return the broadcast station id. Dropping on the basis of this. Please repost when/if this is resolved. John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready.

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: Gábor Stefanik <netrolller.3d@gm...> - 2009-08-21 17:24 2009/8/21 reinette chatre <reinette.chatre@intel.com>: > Hi Gábor, > > On Thu, 2009-08-20 at 15:08 -0700, Gábor Stefanik wrote: >> Commit 1ccb84d87d04df3c76cd4352fe69786d8c7cf016 by Wey-Yi Guy >> ("iwlwifi: clean up unused NL80211_IFTYPE_MONITOR for Monitor mode") >> broke injection of non-broadcast frames to unassociated stations >> (causing a SYSASSERT for all such injected frames), due to injected >> frames no longer automatically getting a broadcast station ID assigned, >> and instead ending up with invalid station IDs. >> This patch restores the old behavior, fixing the aforementioned >> regression. > > The patch you are quoting cannot be the culprit here. As that commit > message indicates we never set NL80211_IFTYPE_MONITOR for the interface > type, so when that code removed the test for this interface type in > iwl_get_sta_id it essentially removed dead code. For some odd reason, reverting Wei-Yi Guy's patch makes the bug go away... should we do that instead for 2.6.31? (I'm all for it, if this patch is not the right thing to do, as Wey-Yi's patch was not a bug fix, just a cleanup.) My guess is that the "default to MONITOR mode" change is the culprit. > >>       /* If this frame is broadcast or management, use broadcast station id */ >> -     if (!ieee80211_is_data(fc) ||  is_multicast_ether_addr(hdr->addr1)) >> +     if (!ieee80211_is_data(fc) || is_multicast_ether_addr(hdr->addr1) || >> +         iwl_is_monitor_mode(priv)) /* Injected frames need broadcast too */ >>               return priv->hw_params.bcast_sta_id; > > I think my comment ties in with what Johannes said. When we are > associated (whether in station, adhoc, or AP mode) we want this function > to return the correct station ID. We also know that an interface can be > in monitor mode while it is in any of these other modes. When this > happens we do not want to return the broadcast station id ... we still > want to return the station id. Your patch changes this behavior and will > in this case always return the broadcast station id. Maybe we should check info->flags & IEEE80211_TX_CTL_INJECTED instead... is there a way to access the ieee80211_tx_info structure from this function (e.g. through priv)? > > Reinette > > > > > -- Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: reinette chatre <reinette.chatre@in...> - 2009-08-21 17:59 Hi Gábor, On Fri, 2009-08-21 at 10:24 -0700, Gábor Stefanik wrote: > For some odd reason, reverting Wei-Yi Guy's patch makes the bug go > away ah - now I see. The driver defaulted to monitor mode in iwl_mac_start. This is not correct and this patch rightly removed that code. > ... should we do that instead for 2.6.31? (I'm all for it, if this > patch is not the right thing to do, as Wey-Yi's patch was not a bug > fix, just a cleanup.) No, this patch was more than code cleanup - it changed the driver to behave correctly wrt monitor interface type. Unfortunately the workaround to get packet injection working was not apparent enough and was missed. > My guess is that the "default to MONITOR mode" > change is the culprit. yeah ... > Maybe we should check info->flags & IEEE80211_TX_CTL_INJECTED > instead... is there a way to access the ieee80211_tx_info structure > from this function (e.g. through priv)? No, but it may not be necessary. Why is is necessary to call this function in the first place if you know this is an injection packet? Specifically, in iwl_tx_skb and iwl3945_tx_skb (where ieee80211_tx_info) is known) there could just be a test if this is an injected packet, if it is, then do not call iwl_get_sta_id, but just use "bcast_sta_id" directly. Would this work? Is a test for monitor mode still needed in this case? Reinette

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: Gábor Stefanik <netrolller.3d@gm...> - 2009-08-21 18:10 2009/8/21 reinette chatre <reinette.chatre@intel.com>: > Hi Gábor, > > On Fri, 2009-08-21 at 10:24 -0700, Gábor Stefanik wrote: > >> For some odd reason, reverting Wei-Yi Guy's patch makes the bug go >> away > > ah - now I see. The driver defaulted to monitor mode in iwl_mac_start. > This is not correct and this patch rightly removed that code. > >> ... should we do that instead for 2.6.31? (I'm all for it, if this >> patch is not the right thing to do, as Wey-Yi's patch was not a bug >> fix, just a cleanup.) > > No, this patch was more than code cleanup - it changed the driver to > behave correctly wrt monitor interface type. Unfortunately the > workaround to get packet injection working was not apparent enough and > was missed. > > >> My guess is that the "default to MONITOR mode" >> change is the culprit. > > yeah ... > > >> Maybe we should check info->flags & IEEE80211_TX_CTL_INJECTED >> instead... is there a way to access the ieee80211_tx_info structure >> from this function (e.g. through priv)? > > No, but it may not be necessary. Why is is necessary to call this > function in the first place if you know this is an injection packet? > Specifically, in iwl_tx_skb and iwl3945_tx_skb (where ieee80211_tx_info) > is known) there could just be a test if this is an injected packet, if > it is, then do not call iwl_get_sta_id, but just use "bcast_sta_id" > directly. Would this work? Is a test for monitor mode still needed in > this case? That can be done, yes. It is also a good idea to convert iwl_is_monitor_mode(priv) calls with TX_CTL_INJECTED checks, as mac80211 will set IEEE80211_TX_CTL_INJECTED if and only if the packet arrived from a monitor interface. (However, when I submitted the first patches to iwlwifi to enable injection, they were rejected specifically because I checked INJECTED in iwl_tx_skb/iwl394_tx_skb, rather than adding a monitor mode case to iwl_get_sta_id... times change I guess.) I'll submit a patch for this. > > Reinette > > > -- Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: reinette chatre <reinette.chatre@in...> - 2009-08-21 18:24 Hi Gábor, On Fri, 2009-08-21 at 11:10 -0700, Gábor Stefanik wrote: > That can be done, yes. It is also a good idea to convert > iwl_is_monitor_mode(priv) calls with TX_CTL_INJECTED checks, as > mac80211 will set IEEE80211_TX_CTL_INJECTED if and only if the packet > arrived from a monitor interface. I don't think that is the intention of that function. iwl_is_monitor_mode() needs to return whether the interface is in monitor mode or not and being in monitor mode is specifically when some filter flags are set up. This is what is tested in this function. > (However, when I submitted the first > patches to iwlwifi to enable injection, they were rejected > specifically because I checked INJECTED in iwl_tx_skb/iwl394_tx_skb, > rather than adding a monitor mode case to iwl_get_sta_id... times > change I guess.) I'll submit a patch for this. Sure - you can move the check to iwl_get_sta_id. You will need to indicate to that function that the frame is being injected. Reinette

Re: [ipw3945-devel] [PATCH] iwlwifi: Make injection of non-broadcast frames work again From: Gábor Stefanik <netrolller.3d@gm...> - 2009-08-21 18:37 2009/8/21 reinette chatre <reinette.chatre@intel.com>: > Hi Gábor, > > On Fri, 2009-08-21 at 11:10 -0700, Gábor Stefanik wrote: > >> That can be done, yes. It is also a good idea to convert >> iwl_is_monitor_mode(priv) calls with TX_CTL_INJECTED checks, as >> mac80211 will set IEEE80211_TX_CTL_INJECTED if and only if the packet >> arrived from a monitor interface. > > I don't think that is the intention of that function. > iwl_is_monitor_mode() needs to return whether the interface is in > monitor mode or not and being in monitor mode is specifically when some > filter flags are set up. This is what is tested in this function. Thanks for the clarification. Will submit a corrected patch soon. > >>  (However, when I submitted the first >> patches to iwlwifi to enable injection, they were rejected >> specifically because I checked INJECTED in iwl_tx_skb/iwl394_tx_skb, >> rather than adding a monitor mode case to iwl_get_sta_id... times >> change I guess.) I'll submit a patch for this. > > Sure - you can move the check to iwl_get_sta_id. You will need to > indicate to that function that the frame is being injected. > > Reinette > > > > > > -- Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

[ipw3945-devel] [PATCH v2] iwlwifi: Make injection of non-broadcast frames work again From: Gábor Stefanik <netrolller.3d@gm...> - 2009-08-21 18:44 Commit 1ccb84d87d04df3c76cd4352fe69786d8c7cf016 by Wey-Yi Guy ("iwlwifi: clean up unused NL80211_IFTYPE_MONITOR for Monitor mode") broke injection of non-broadcast frames to unassociated stations (causing a SYSASSERT for all such injected frames), due to injected frames no longer automatically getting a broadcast station ID assigned. This patch restores the old behavior, fixing the aforementioned regression. Also, consistently check for IEEE80211_TX_CTL_INJECTED instead of iwl_is_monitor_mode in the TX path, as TX_CTL_INJECTED specifically means that a given packet is coming from a monitor interface, while iwl_is_monitor_mode only shows whether a monitor interface exists on the device. Signed-off-by: Gábor Stefanik <netrolller.3d@gmail.com> --- V2: Use a totally new approach to fixing this bug. Also fix a few more cases where iwl_is_monitor_mode was incorrectly used to check whether a given packet comes from a monitor interface. (For that, IEEE80211_TX_CTL_INJECTED should be used.) drivers/net/wireless/iwlwifi/iwl-tx.c | 10 ++++++---- drivers/net/wireless/iwlwifi/iwl3945-base.c | 9 ++++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c index 9b76bd4..8eba9e9 100644 --- a/drivers/net/wireless/iwlwifi/iwl-tx.c +++ b/drivers/net/wireless/iwlwifi/iwl-tx.c @@ -718,10 +718,9 @@ int iwl_tx_skb(struct iwl_priv *priv, struct sk_buff *skb) IWL_DEBUG_TX(priv, "Sending REASSOC frame\n"); #endif - /* drop all data frame if we are not associated */ + /* drop all non-injected data frame if we are not associated */ if (ieee80211_is_data(fc) && - (!iwl_is_monitor_mode(priv) || - !(info->flags & IEEE80211_TX_CTL_INJECTED)) && /* packet injection */ + !(info->flags & IEEE80211_TX_CTL_INJECTED) && (!iwl_is_associated(priv) || ((priv->iw_mode == NL80211_IFTYPE_STATION) && !priv->assoc_id) || !priv->assoc_station_added)) { @@ -732,7 +731,10 @@ int iwl_tx_skb(struct iwl_priv *priv, struct sk_buff *skb) hdr_len = ieee80211_hdrlen(fc); /* Find (or create) index into station table for destination station */ - sta_id = iwl_get_sta_id(priv, hdr); + if (info->flags & IEEE80211_TX_CTL_INJECTED) + sta_id = priv->hw_params.bcast_sta_id; + else + sta_id = iwl_get_sta_id(priv, hdr); if (sta_id == IWL_INVALID_STATION) { IWL_DEBUG_DROP(priv, "Dropping - INVALID STATION: %pM\n", hdr->addr1); diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c index e617411..d0debdf 100644 --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c @@ -518,9 +518,9 @@ static int iwl3945_tx_skb(struct iwl_priv *priv, struct sk_buff *skb) IWL_DEBUG_TX(priv, "Sending REASSOC frame\n"); #endif - /* drop all data frame if we are not associated */ + /* drop all non-injected data frame if we are not associated */ if (ieee80211_is_data(fc) && - (!iwl_is_monitor_mode(priv)) && /* packet injection */ + !(info->flags & IEEE80211_TX_CTL_INJECTED) && (!iwl_is_associated(priv) || ((priv->iw_mode == NL80211_IFTYPE_STATION) && !priv->assoc_id))) { IWL_DEBUG_DROP(priv, "Dropping - !iwl_is_associated\n"); @@ -532,7 +532,10 @@ static int iwl3945_tx_skb(struct iwl_priv *priv, struct sk_buff *skb) hdr_len = ieee80211_hdrlen(fc); /* Find (or create) index into station table for destination station */ - sta_id = iwl_get_sta_id(priv, hdr); + if (info->flags & IEEE80211_TX_CTL_INJECTED) + sta_id = priv->hw_params.bcast_sta_id; + else + sta_id = iwl_get_sta_id(priv, hdr); if (sta_id == IWL_INVALID_STATION) { IWL_DEBUG_DROP(priv, "Dropping - INVALID STATION: %pM\n", hdr->addr1); -- 1.6.2.4

Re: [ipw3945-devel] [PATCH v2] iwlwifi: Make injection of non-broadcast frames work again From: reinette chatre <reinette.chatre@in...> - 2009-08-21 20:49 On Fri, 2009-08-21 at 11:44 -0700, Gábor Stefanik wrote: > Commit 1ccb84d87d04df3c76cd4352fe69786d8c7cf016 by Wey-Yi Guy > ("iwlwifi: clean up unused NL80211_IFTYPE_MONITOR for Monitor mode") > broke injection of non-broadcast frames to unassociated stations > (causing a SYSASSERT for all such injected frames), due to injected > frames no longer automatically getting a broadcast station ID assigned. > This patch restores the old behavior, fixing the aforementioned > regression. > > Also, consistently check for IEEE80211_TX_CTL_INJECTED instead of > iwl_is_monitor_mode in the TX path, as TX_CTL_INJECTED specifically > means that a given packet is coming from a monitor interface, while > iwl_is_monitor_mode only shows whether a monitor interface exists > on the device. > > Signed-off-by: Gábor Stefanik <netrolller.3d@gmail.com> Acked-by: Reinette Chatre <reinette.chatre@intel.com> Thank you Reinette

2 messages have been excluded from this view by a project administrator.