[PATCH] iwlwifi regression in 20111205 merge

[PATCH] iwlwifi regression in 20111205 merge

[Nikolay Martynov]

  It looks like the regression was introduced between 20111202 and
20111205 (linux-next tree). Symptoms: connection to AP seem to be
established, but no data goes though it in any way. Tested on intel
5300.
  Peek at the changes have shown that it looks like at least part of
the code wasn't merged properly. It was originally committed into
iwl_agn.c but code in question was moved to iwl-mac80211.c.
  This patch puts code in place and my card works again.
  I hope this patch will be reviewed by people developing iwl
driver. Also, it is possible that some other iwl code didn't make it
though 20111205 merge as well, so this probably should be checked too.
  Please let me know if I can provide any more info.
  Thanks!

Signed-off-by: Nikolay Martynov <mar.kolya@gmail.com>
---
 drivers/net/wireless/iwlwifi/iwl-mac80211.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
index 794b735..55308b8 100644
--- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
+++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
@@ -517,6 +517,17 @@ static int iwlagn_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		return -EOPNOTSUPP;
 	}
 
+	switch (key->cipher) {
+	case WLAN_CIPHER_SUITE_TKIP:
+		key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
+		/* fall through */
+	case WLAN_CIPHER_SUITE_CCMP:
+		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
+		break;
+	default:
+		break;
+	}
+
 	/*
 	 * We could program these keys into the hardware as well, but we
 	 * don't expect much multicast traffic in IBSS and having keys
-- 
1.7.4.1

Re: [PATCH] iwlwifi regression in 20111205 merge

[Guy, Wey-Yi]

Hi Nikolay,

I believe I know what happen,

this change happen after the function being move from iwl-agn.c to
iwl-mac80211.c

Author: Johannes Berg <johannes.berg@intel.com>
Date:   Fri Dec 2 08:19:17 2011 -0800

    iwlagn: fix HW crypto for TX-only keys
    
    Group keys in IBSS or AP mode are not programmed
    into the device since we give the key to it with
    every TX packet. However, we do need mac80211 to
    create the MMIC & PN in all cases. Move the code
    around to set the key flags all the time. We set
    them even when the key is removed again but that
    is obviously harmless.
    
    Cc: stable@vger.kernel.org
    Reported-by: Reinette Chatre <reinette.chatre@intel.com>
    Signed-off-by: Johannes Berg <johannes.berg@intel.com>
    Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
    Signed-off-by: John W. Linville <linville@tuxdriver.com>

and the changes was made in iwl-mac80211.c

but since this patch need to be backport to stable kernel.
so there is a separated patch sent for the changes in iwl-agn.c against
stable

because all these, for some reason the changes just got lost.
Thank you for catch this

Wey


On Thu, 2011-12-08 at 18:43 -0800, Nikolay Martynov wrote:
> It looks like the regression was introduced between 20111202 and
> 20111205 (linux-next tree). Symptoms: connection to AP seem to be
> established, but no data goes though it in any way. Tested on intel
> 5300.
>   Peek at the changes have shown that it looks like at least part of
> the code wasn't merged properly. It was originally committed into
> iwl_agn.c but code in question was moved to iwl-mac80211.c.
>   This patch puts code in place and my card works again.
>   I hope this patch will be reviewed by people developing iwl
> driver. Also, it is possible that some other iwl code didn't make it
> though 20111205 merge as well, so this probably should be checked too.
>   Please let me know if I can provide any more info.
>   Thanks!
> 
> Signed-off-by: Nikolay Martynov <mar.kolya@gmail.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-mac80211.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> index 794b735..55308b8 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> @@ -517,6 +517,17 @@ static int iwlagn_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	switch (key->cipher) {
> +	case WLAN_CIPHER_SUITE_TKIP:
> +		key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
> +		/* fall through */
> +	case WLAN_CIPHER_SUITE_CCMP:
> +		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
> +		break;
> +	default:
> +		break;
> +	}
> +
>  	/*
>  	 * We could program these keys into the hardware as well, but we
>  	 * don't expect much multicast traffic in IBSS and having keys

Re: [PATCH] iwlwifi regression in 20111205 merge

[Guy, Wey-Yi]

John,

what is the correct method to handle this case?
the patch need to add a piece of code in iwl-mac80211.c, and also need
to backport to stable
Since the function was recently move from iwl-agn.c to iwl-mac80211.c,
so separated patch need for "wireless-next" and "wireless". But look
like portion of patch just lost because the function was move.

Thanks
Wey

On Thu, 2011-12-08 at 19:03 -0800, Guy, Wey-Yi wrote:
> Hi Nikolay,
> 
> I believe I know what happen,
> 
> this change happen after the function being move from iwl-agn.c to
> iwl-mac80211.c
> 
> Author: Johannes Berg <johannes.berg@intel.com>
> Date:   Fri Dec 2 08:19:17 2011 -0800
> 
>     iwlagn: fix HW crypto for TX-only keys
>     
>     Group keys in IBSS or AP mode are not programmed
>     into the device since we give the key to it with
>     every TX packet. However, we do need mac80211 to
>     create the MMIC & PN in all cases. Move the code
>     around to set the key flags all the time. We set
>     them even when the key is removed again but that
>     is obviously harmless.
>     
>     Cc: stable@vger.kernel.org
>     Reported-by: Reinette Chatre <reinette.chatre@intel.com>
>     Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>     Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
>     Signed-off-by: John W. Linville <linville@tuxdriver.com>
> 
> and the changes was made in iwl-mac80211.c
> 
> but since this patch need to be backport to stable kernel.
> so there is a separated patch sent for the changes in iwl-agn.c against
> stable
> 
> because all these, for some reason the changes just got lost.
> Thank you for catch this
> 
> Wey
> 
> 
> On Thu, 2011-12-08 at 18:43 -0800, Nikolay Martynov wrote:
> > It looks like the regression was introduced between 20111202 and
> > 20111205 (linux-next tree). Symptoms: connection to AP seem to be
> > established, but no data goes though it in any way. Tested on intel
> > 5300.
> >   Peek at the changes have shown that it looks like at least part of
> > the code wasn't merged properly. It was originally committed into
> > iwl_agn.c but code in question was moved to iwl-mac80211.c.
> >   This patch puts code in place and my card works again.
> >   I hope this patch will be reviewed by people developing iwl
> > driver. Also, it is possible that some other iwl code didn't make it
> > though 20111205 merge as well, so this probably should be checked too.
> >   Please let me know if I can provide any more info.
> >   Thanks!
> > 
> > Signed-off-by: Nikolay Martynov <mar.kolya@gmail.com>
> > ---
> >  drivers/net/wireless/iwlwifi/iwl-mac80211.c |   11 +++++++++++
> >  1 files changed, 11 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > index 794b735..55308b8 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > @@ -517,6 +517,17 @@ static int iwlagn_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> >  		return -EOPNOTSUPP;
> >  	}
> >  
> > +	switch (key->cipher) {
> > +	case WLAN_CIPHER_SUITE_TKIP:
> > +		key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
> > +		/* fall through */
> > +	case WLAN_CIPHER_SUITE_CCMP:
> > +		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> >  	/*
> >  	 * We could program these keys into the hardware as well, but we
> >  	 * don't expect much multicast traffic in IBSS and having keys
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] iwlwifi regression in 20111205 merge

[John W. Linville]

Are the bits in the wireless tree correct?  If so, then please just
send a patch to fix the wireless-next tree.

Now that the kernel.org mess is sorted, I guess we/I need to go back
to pulling your trees instead of applying patches...

John

On Thu, Dec 08, 2011 at 07:15:28PM -0800, Guy, Wey-Yi wrote:
> John,
> 
> what is the correct method to handle this case?
> the patch need to add a piece of code in iwl-mac80211.c, and also need
> to backport to stable
> Since the function was recently move from iwl-agn.c to iwl-mac80211.c,
> so separated patch need for "wireless-next" and "wireless". But look
> like portion of patch just lost because the function was move.
> 
> Thanks
> Wey
> 
> On Thu, 2011-12-08 at 19:03 -0800, Guy, Wey-Yi wrote:
> > Hi Nikolay,
> > 
> > I believe I know what happen,
> > 
> > this change happen after the function being move from iwl-agn.c to
> > iwl-mac80211.c
> > 
> > Author: Johannes Berg <johannes.berg@intel.com>
> > Date:   Fri Dec 2 08:19:17 2011 -0800
> > 
> >     iwlagn: fix HW crypto for TX-only keys
> >     
> >     Group keys in IBSS or AP mode are not programmed
> >     into the device since we give the key to it with
> >     every TX packet. However, we do need mac80211 to
> >     create the MMIC & PN in all cases. Move the code
> >     around to set the key flags all the time. We set
> >     them even when the key is removed again but that
> >     is obviously harmless.
> >     
> >     Cc: stable@vger.kernel.org
> >     Reported-by: Reinette Chatre <reinette.chatre@intel.com>
> >     Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> >     Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> >     Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > 
> > and the changes was made in iwl-mac80211.c
> > 
> > but since this patch need to be backport to stable kernel.
> > so there is a separated patch sent for the changes in iwl-agn.c against
> > stable
> > 
> > because all these, for some reason the changes just got lost.
> > Thank you for catch this
> > 
> > Wey
> > 
> > 
> > On Thu, 2011-12-08 at 18:43 -0800, Nikolay Martynov wrote:
> > > It looks like the regression was introduced between 20111202 and
> > > 20111205 (linux-next tree). Symptoms: connection to AP seem to be
> > > established, but no data goes though it in any way. Tested on intel
> > > 5300.
> > >   Peek at the changes have shown that it looks like at least part of
> > > the code wasn't merged properly. It was originally committed into
> > > iwl_agn.c but code in question was moved to iwl-mac80211.c.
> > >   This patch puts code in place and my card works again.
> > >   I hope this patch will be reviewed by people developing iwl
> > > driver. Also, it is possible that some other iwl code didn't make it
> > > though 20111205 merge as well, so this probably should be checked too.
> > >   Please let me know if I can provide any more info.
> > >   Thanks!
> > > 
> > > Signed-off-by: Nikolay Martynov <mar.kolya@gmail.com>
> > > ---
> > >  drivers/net/wireless/iwlwifi/iwl-mac80211.c |   11 +++++++++++
> > >  1 files changed, 11 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > > index 794b735..55308b8 100644
> > > --- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > > +++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > > @@ -517,6 +517,17 @@ static int iwlagn_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> > >  		return -EOPNOTSUPP;
> > >  	}
> > >  
> > > +	switch (key->cipher) {
> > > +	case WLAN_CIPHER_SUITE_TKIP:
> > > +		key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
> > > +		/* fall through */
> > > +	case WLAN_CIPHER_SUITE_CCMP:
> > > +		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
> > > +		break;
> > > +	default:
> > > +		break;
> > > +	}
> > > +
> > >  	/*
> > >  	 * We could program these keys into the hardware as well, but we
> > >  	 * don't expect much multicast traffic in IBSS and having keys
> > 
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] iwlwifi regression in 20111205 merge

[Guy, Wey-Yi]

On Fri, 2011-12-09 at 07:44 -0800, John W. Linville wrote:
> Are the bits in the wireless tree correct?  If so, then please just
> send a patch to fix the wireless-next tree.
> 
> Now that the kernel.org mess is sorted, I guess we/I need to go back
> to pulling your trees instead of applying patches...
> 
"wireless" tree is correct, the piece is missing in iwl-mac80211.c on
"wireless-next" tree. Nikolay's patch fix it.

The question I have is what is the right way to deal with this. the
orig. patch need to be backport to stable, but the function was move to
different file short before the patch. That is the reason cause this
mess :-)

Thanks
Wey

> 
> On Thu, Dec 08, 2011 at 07:15:28PM -0800, Guy, Wey-Yi wrote:
> > John,
> > 
> > what is the correct method to handle this case?
> > the patch need to add a piece of code in iwl-mac80211.c, and also need
> > to backport to stable
> > Since the function was recently move from iwl-agn.c to iwl-mac80211.c,
> > so separated patch need for "wireless-next" and "wireless". But look
> > like portion of patch just lost because the function was move.
> > 
> > Thanks
> > Wey
> > 
> > On Thu, 2011-12-08 at 19:03 -0800, Guy, Wey-Yi wrote:
> > > Hi Nikolay,
> > > 
> > > I believe I know what happen,
> > > 
> > > this change happen after the function being move from iwl-agn.c to
> > > iwl-mac80211.c
> > > 
> > > Author: Johannes Berg <johannes.berg@intel.com>
> > > Date:   Fri Dec 2 08:19:17 2011 -0800
> > > 
> > >     iwlagn: fix HW crypto for TX-only keys
> > >     
> > >     Group keys in IBSS or AP mode are not programmed
> > >     into the device since we give the key to it with
> > >     every TX packet. However, we do need mac80211 to
> > >     create the MMIC & PN in all cases. Move the code
> > >     around to set the key flags all the time. We set
> > >     them even when the key is removed again but that
> > >     is obviously harmless.
> > >     
> > >     Cc: stable@vger.kernel.org
> > >     Reported-by: Reinette Chatre <reinette.chatre@intel.com>
> > >     Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> > >     Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > >     Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > > 
> > > and the changes was made in iwl-mac80211.c
> > > 
> > > but since this patch need to be backport to stable kernel.
> > > so there is a separated patch sent for the changes in iwl-agn.c against
> > > stable
> > > 
> > > because all these, for some reason the changes just got lost.
> > > Thank you for catch this
> > > 
> > > Wey
> > > 
> > > 
> > > On Thu, 2011-12-08 at 18:43 -0800, Nikolay Martynov wrote:
> > > > It looks like the regression was introduced between 20111202 and
> > > > 20111205 (linux-next tree). Symptoms: connection to AP seem to be
> > > > established, but no data goes though it in any way. Tested on intel
> > > > 5300.
> > > >   Peek at the changes have shown that it looks like at least part of
> > > > the code wasn't merged properly. It was originally committed into
> > > > iwl_agn.c but code in question was moved to iwl-mac80211.c.
> > > >   This patch puts code in place and my card works again.
> > > >   I hope this patch will be reviewed by people developing iwl
> > > > driver. Also, it is possible that some other iwl code didn't make it
> > > > though 20111205 merge as well, so this probably should be checked too.
> > > >   Please let me know if I can provide any more info.
> > > >   Thanks!
> > > > 
> > > > Signed-off-by: Nikolay Martynov <mar.kolya@gmail.com>
> > > > ---
> > > >  drivers/net/wireless/iwlwifi/iwl-mac80211.c |   11 +++++++++++
> > > >  1 files changed, 11 insertions(+), 0 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/wireless/iwlwifi/iwl-mac80211.c b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > > > index 794b735..55308b8 100644
> > > > --- a/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > > > +++ b/drivers/net/wireless/iwlwifi/iwl-mac80211.c
> > > > @@ -517,6 +517,17 @@ static int iwlagn_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> > > >  		return -EOPNOTSUPP;
> > > >  	}
> > > >  
> > > > +	switch (key->cipher) {
> > > > +	case WLAN_CIPHER_SUITE_TKIP:
> > > > +		key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
> > > > +		/* fall through */
> > > > +	case WLAN_CIPHER_SUITE_CCMP:
> > > > +		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
> > > > +		break;
> > > > +	default:
> > > > +		break;
> > > > +	}
> > > > +
> > > >  	/*
> > > >  	 * We could program these keys into the hardware as well, but we
> > > >  	 * don't expect much multicast traffic in IBSS and having keys
> > > 
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> > 
> 

Re: [PATCH] iwlwifi regression in 20111205 merge

[John W. Linville]

On Fri, Dec 09, 2011 at 07:15:27AM -0800, Guy, Wey-Yi wrote:
> On Fri, 2011-12-09 at 07:44 -0800, John W. Linville wrote:
> > Are the bits in the wireless tree correct?  If so, then please just
> > send a patch to fix the wireless-next tree.
> > 
> > Now that the kernel.org mess is sorted, I guess we/I need to go back
> > to pulling your trees instead of applying patches...
> > 
> "wireless" tree is correct, the piece is missing in iwl-mac80211.c on
> "wireless-next" tree. Nikolay's patch fix it.
> 
> The question I have is what is the right way to deal with this. the
> orig. patch need to be backport to stable, but the function was move to
> different file short before the patch. That is the reason cause this
> mess :-)

Once it has made it to Linus, you can send a patch for
stable@vger.kernel.org referencing the commit ID in Linus' tree.

If you mean that the change needs to go to wireless but the function
has moved in wireless-next, then post the patch for wireless.  Usually
I can figure-out the merge (although apparently not in this case).
If you think the merge will be tricky, you can send a reference patch
for wireless-next for me to use as a guide when fixing-up the merge.
Or you can pull your fixes tree into your -next tree, resolve the
merge conflict, and ask me to pull.  Or you can wait until the merge
breakage happens, and send a patch. :-)

Hth!

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] iwlwifi regression in 20111205 merge

[Guy, Wey-Yi]

On Fri, 2011-12-09 at 08:51 -0800, John W. Linville wrote:
> On Fri, Dec 09, 2011 at 07:15:27AM -0800, Guy, Wey-Yi wrote:
> > On Fri, 2011-12-09 at 07:44 -0800, John W. Linville wrote:
> > > Are the bits in the wireless tree correct?  If so, then please just
> > > send a patch to fix the wireless-next tree.
> > > 
> > > Now that the kernel.org mess is sorted, I guess we/I need to go back
> > > to pulling your trees instead of applying patches...
> > > 
> > "wireless" tree is correct, the piece is missing in iwl-mac80211.c on
> > "wireless-next" tree. Nikolay's patch fix it.
> > 
> > The question I have is what is the right way to deal with this. the
> > orig. patch need to be backport to stable, but the function was move to
> > different file short before the patch. That is the reason cause this
> > mess :-)
> 
> Once it has made it to Linus, you can send a patch for
> stable@vger.kernel.org referencing the commit ID in Linus' tree.
> 
> If you mean that the change needs to go to wireless but the function
> has moved in wireless-next, then post the patch for wireless.  Usually
> I can figure-out the merge (although apparently not in this case).
> If you think the merge will be tricky, you can send a reference patch
> for wireless-next for me to use as a guide when fixing-up the merge.
> Or you can pull your fixes tree into your -next tree, resolve the
> merge conflict, and ask me to pull.  Or you can wait until the merge
> breakage happens, and send a patch. :-)
> 

Thank you for the advise and I will try my best to make sure not happen
again :-)

Thanks
Wey