Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 07 Mar 2012 02:30:46 +0900 (JST)
From:      Mitsuru IWASAKI <iwasaki@jp.FreeBSD.org>
To:        bschmidt@freebsd.org, adrian@freebsd.org
Cc:        freebsd-current@freebsd.org, freebsd-wireless@freebsd.org
Subject:   Re: patches for if_iwi and wlan for WEP mode
Message-ID:  <20120307.023046.27956263.iwasaki@jp.FreeBSD.org>
In-Reply-To: <201203052314.22050.bschmidt@freebsd.org> <CAJ-Vmon%2BmHYTEZPP7x4R3uSwXV_=fJFmdTimEieEaPMvFqEmmA@mail.gmail.com>
References:  <20120306.024212.108736612.iwasaki@jp.FreeBSD.org> <201203052314.22050.bschmidt@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Thanks Bernhard and Adrian, I think the problem seems to be solved.

> > My patches set IEEE80211_NODE_ASSOCID bit only if ni->ni_associd
> > is set.  Any suggestions on this part are welcome.
> 
> Are you sure the net80211 part is correct? It looks to me as if you
> are just masking the real issue. The IEEE80211_NODE_ASSOCID flag is
> ment to be used to verify that an associd has actually been set, not
> doing so will break other things I guess. iwi(4) is a bit tricky in
> that regard, as it sets the associd itself, check iwi_checkforqos().
> I'd verify that function is actually called and if so if the parameters
> are correct. I fumbled around there once, might have wrong WEP..

As you suggested, iwi_checkforqos() has problems, wrong asresp
frame parsing.

----
@@ -1357,8 +1365,8 @@
 	frm += 2;
 
 	wme = NULL;
-	while (frm < efrm) {
-		IEEE80211_VERIFY_LENGTH(efrm - frm, frm[1], return);
+	while (efrm - frm > 1) {
+		IEEE80211_VERIFY_LENGTH(efrm - frm, frm[1] + 2, return);
 		switch (*frm) {
 		case IEEE80211_ELEMID_VENDOR:
 			if (iswmeoui(frm))
----

Bacause of the condition `while (frm < efrm)',
IEEE80211_VERIFY_LENGTH() was checking item length beyond the
ieee80211_frame region, and returned from iwi_checkforqos() without
setting flags, capinfo and associd!
I made above changes referring to net80211 code such as
ieee80211_sta.c.

Today's version of patches at:
http://people.freebsd.org/~iwasaki/iwi/iwi-20120306.diff

This one don't have changes on net80211 part at all.


> What's the reason behing adding if_qflush()/if_transmit()?

In RELENG_7, data frame is transmitted by iwi_tx_start() like this.

  ether_output
    ether_output_frame
      IFQ_HANDOFF/IFQ_HANDOFF_ADJ
        if_start
          iwi_start
            iwi_tx_start

After 8.0-RELEASE, device specific if_transmit() is called via net80211 layer.

  ether_output
    ether_output_frame
      if_transmit
        IFQ_HANDOFF/IFQ_HANDOFF_ADJ
          if_start
            ieee80211_start
              parent->if_transmit(ie. iwi_transmit())

There was not if_transmit method in iwi(4), so I add it.
On if_qflush(), CURRENT kernel complains that `transmit and qflush
must both either be set or both be NULL' from if.c.
I wrote iwi_qflush(), but actually never tested it...

From: Adrian Chadd <adrian@freebsd.org>
> Would you please open a PR with this particular issue and then attach
> the patch to it?

I prefer committing changes on iwi(4) by myself, because grimreaper@
keep giving pressure to me `Your src commit bit is still idle.' for
long time :)
I just want to stop it.

> I'd rather you not commit the net80211 change until I've verified that
> WEP works or doesn't work with ath(4).

Never mind, I think I don't need to touch on net80211.

Thanks!



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120307.023046.27956263.iwasaki>