Search Linux Wireless

RE: [PATCH 3/3] mwifiex: fix kernel warning in ibss start/join path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Hi Johannes,

> On Thu, 2012-05-17 at 11:28 -0700, Bing Zhao wrote:
> > > Don't touch it. I knew I should have hidden these fields from driver
> > > view, but it wasn't easily possible.
> > >
> > > If you're using the API wrong then you should fix that instead. In
> > > particular, you should only call cfg80211_ibss_joined() when you
> > > actually know the BSSID of the IBSS you've joined. You can't possibly
> > > know this at this point since you can't even have scanned for peers yet.
> >
> > In .join_ibss handler, mwifiex driver sends a command to firmware to start/join an IBSS network.
> > This command is a synchronous call. When the command response comes
> > back we have started/joined it successfully, otherwise an error code
> > will be returned. However, we are still inside .join_ibss handler at
> > this moment. Calling cfg80211_ibss_joined() gives us the warning
> > because ssid_len hasn't been set by cfg80211 yet.
> Hmm. Interesting, because I would think that could potentially take
> quite a while?

It does take a while, at least for a scan. The issue is that, no matter how long it takes the driver stays in the join_ibss handler. The function call isn't returned until the AD_HOC_JOIN command response is received.

> It's interesting though because it points out that even if you do call
> it asynchronously, there's no guarantee that it actually happens before
> the function call returns.

By doing it asynchronously we can probably use join_ibss handler as a trigger for ssid scan and ibss start/join. Here the join_ibss function call will be retuned right away. Later after driver finishes scan and gets "ibss started/joined" response from firmware, cfg80211_ibss_joined() is called.

In this case, returning cfg80211_join_ibss() call doesn't mean that we have joined successfully. Instead, cfg80211_ibss_joined() call guarantees that.

> Maybe we should just move the assignment in cfg80211 instead?

Moving wdev->ssid and wdev->ssid_len assignments from __cfg80211_join_ibss() to cfg80211_ibss_joined() makes more sense to me. Of course, "CFG80211_DEV_WARN_ON(!wdev->ssid_len)" check won’t be needed any more.



[Linux Kernel]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]     [Free Dating]     [M2M Wireless]

Add to Google Powered by Linux