.. 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_at_freebsd.org> 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. >> >> ---- >> _at__at_ -1357,8 +1365,8 _at__at_ >> 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_at_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_at_ >> keep giving pressure to me `Your src commit bit is still idle.' for >> long time :) >> I just want to stop it. > > ;) > > -- > BernhardReceived on Tue Mar 06 2012 - 19:12:57 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:24 UTC