linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stanislaw Gruszka <sgruszka@redhat.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Wey-Yi Guy <wey-yi.w.guy@intel.com>,
	Intel Linux Wireless <ilw@linux.intel.com>,
	linux-wireless@vger.kernel.org
Subject: Re: [PATCH] iwlwifi: do not nulify ctx->vif on reset
Date: Fri, 16 Mar 2012 15:37:31 +0100	[thread overview]
Message-ID: <20120316143730.GA11278@redhat.com> (raw)
In-Reply-To: <20120314081424.GC2788@redhat.com>

On Wed, Mar 14, 2012 at 09:14:24AM +0100, Stanislaw Gruszka wrote:
> On Wed, Mar 14, 2012 at 08:07:48AM +0100, Stanislaw Gruszka wrote:
> > I'll update and recheck again.
> 
> On updated tree (head 035364916f75151b4b91ea53968c6beba7545317) devices
> stop working here on any forced reset during TX as well. There are
> messages like below in dmesg:
> 
> wlan3: dropped data frame to not associated station 00:00:00:00:00:00
> 
> And one time crash happened too, in:
> 
> (gdb) l *(iwl_remove_dynamic_key+0x1f0)
> 0x15e20 is in iwl_remove_dynamic_key (drivers/net/wireless/iwlwifi/iwl-agn-sta.c:1105).
> 1100		/*
> 1101		 * The device expects GTKs for station interfaces to be
> 1102		 * installed as GTKs for the AP station. If we have no
> 1103		 * station ID, then use the ap_sta_id in that case.
> 1104		 */
> 1105		if (vif->type == NL80211_IFTYPE_STATION && vif_priv->ctx)
> 1106			return vif_priv->ctx->ap_sta_id;
> 1107	
> 1108		return IWL_INVALID_STATION;
> 1109	}
> 
> To reproduce problems, I'm doing "ping -f 192.168.1.1" on one console
> and run script [1] on other console.

So what we do here? Patch fix possible crash on firmware restart.
We have a few places where ctx->vif is dereferences without a NULL
check. Perhaps those places should be verified instead. What protects
to dereference ctx->vif after iwl_mac_remove_interface() ?

Regarding restart, this apparently is broken - hung the adapter such the
module reload is needed. To mitigate the problems You just disable
watchdog on various adapters.

I do not think restart should be fixed. I think better would be disable
restart by default (set fw_restart=0) and start to solve real problems :-)

This is of course not easy, but perhaps could be done using tracing. It
offer a nice feature that allow to enable and disable logging on
runtime, (tracing_on(), tracing_off() functions). So offer a compile
option to make IWL_DEBUG use trace_printk(), start tracing on module
load, disable it on microcode error or queue stuck. All of that should
help with debugging inimitable problems, as ftrace log data into memory
and use ring buffer, hence allow to see (lot of) latest actions before
the problem occurs. I could write the patches, but since you are
constantly refactoring the iwlwifi driver better if you will do this.
How about that?

Stanislaw


  reply	other threads:[~2012-03-16 15:08 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-13 15:10 [PATCH] iwlwifi: do not nulify ctx->vif on reset Stanislaw Gruszka
2012-03-13 15:15 ` Johannes Berg
2012-03-14  6:25   ` Stanislaw Gruszka
2012-03-14  6:58     ` Johannes Berg
2012-03-14  7:07       ` Stanislaw Gruszka
2012-03-14  8:14         ` Stanislaw Gruszka
2012-03-16 14:37           ` Stanislaw Gruszka [this message]
2012-03-16 15:16             ` Johannes Berg
2012-03-26  9:08           ` Johannes Berg
2012-03-26 14:30             ` Guy, Wey-Yi
2012-03-26 18:42               ` John W. Linville
2012-03-26 18:51                 ` Guy, Wey-Yi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120316143730.GA11278@redhat.com \
    --to=sgruszka@redhat.com \
    --cc=ilw@linux.intel.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=wey-yi.w.guy@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).