Re: patches for if_iwi and wlan for WEP mode

.. except that the default if_transmit handling breaks fragments. Sigh.

So we're going to have to implement if_transmit for all net80211
drivers soon and fix fragment handling.


Adrian

On 6 March 2012 11:05, Bernhard Schmidt <bschmidt@xxxxxxxxxxx> wrote:
On Tuesday 06 March 2012 18:30:46 Mitsuru IWASAKI wrote:
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.

Looks good to me, please get that into the tree.

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...

Hmm, it still is the case for >= 8 afaik, there is a default
if_transmit() which is used for all wireless drivers which seems to
work pretty well. That's why I'm wondering, iwi(4) would be the first
driver to have it's own if_transmit() function. I'm not aware of any
technical reason for adding one, or did I miss something? If not I'd
rather not have one added, for sake of consistency.

From: Adrian Chadd <adrian@xxxxxxxxxxx>
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.

;)

--
Bernhard
_______________________________________________
freebsd-current@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscribe@xxxxxxxxxxx"