From owner-freebsd-current@FreeBSD.ORG Tue Mar 6 20:12:57 2012 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 1595A1065679; Tue, 6 Mar 2012 20:12:57 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-we0-f182.google.com (mail-we0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 475878FC29; Tue, 6 Mar 2012 20:12:55 +0000 (UTC) Received: by werl4 with SMTP id l4so4389245wer.13 for ; Tue, 06 Mar 2012 12:12:55 -0800 (PST) Received-SPF: pass (google.com: domain of adrian.chadd@gmail.com designates 10.180.85.69 as permitted sender) client-ip=10.180.85.69; Authentication-Results: mr.google.com; spf=pass (google.com: domain of adrian.chadd@gmail.com designates 10.180.85.69 as permitted sender) smtp.mail=adrian.chadd@gmail.com; dkim=pass header.i=adrian.chadd@gmail.com Received: from mr.google.com ([10.180.85.69]) by 10.180.85.69 with SMTP id f5mr26625069wiz.18.1331064775395 (num_hops = 1); Tue, 06 Mar 2012 12:12:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=h9UBg5vjAHop1HuH9oml5K0R3u9LxjGSxcFWluMGNGI=; b=k7EyCV0J73py7V9VKDJ8nYrkx5iwArvBKsj8PolvJr3b/yUcAauYrKC5sgq18UkOkf EMZUK2q40kpwKhyKFRCj+sZOe0J8rHso6p3GYktJX/WjwnEX+CevZ87EsN9+2Ifbpy2Q pMu1Qsn7q0SrKvBW/EDcBOFIGTbnftzt1kO8KDYiwPJOCT/SXxq4zheQewWRziGTWe1v v0p6z9NJs0qg7duTZqdpPnhz3C3dqvU3rXDyEreUCw53mHqARb7MTe9T3uBBaKXkwf+L J36Q4BT1+9MvPrEr3FcuBOpgNvxJOsNFGqJpOrE7mTMC7Uo02vzwJ16powTxnTphBPyo svgg== MIME-Version: 1.0 Received: by 10.180.85.69 with SMTP id f5mr21087563wiz.18.1331064775193; Tue, 06 Mar 2012 12:12:55 -0800 (PST) Sender: adrian.chadd@gmail.com Received: by 10.216.198.81 with HTTP; Tue, 6 Mar 2012 12:12:55 -0800 (PST) In-Reply-To: <201203062005.15276.bschmidt@freebsd.org> References: <20120306.024212.108736612.iwasaki@jp.FreeBSD.org> <201203052314.22050.bschmidt@freebsd.org> <20120307.023046.27956263.iwasaki@jp.FreeBSD.org> <201203062005.15276.bschmidt@freebsd.org> Date: Tue, 6 Mar 2012 12:12:55 -0800 X-Google-Sender-Auth: PM46MTeTZNVa05vAfUDYlOfrzWg Message-ID: From: Adrian Chadd To: Bernhard Schmidt Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-current@freebsd.org, freebsd-wireless@freebsd.org Subject: Re: patches for if_iwi and wlan for WEP mode X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Mar 2012 20:12:57 -0000 .. 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 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. =A0Any 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 parameter= s >> > 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 @@ >> =A0 =A0 =A0 frm +=3D 2; >> >> =A0 =A0 =A0 wme =3D NULL; >> - =A0 =A0 while (frm < efrm) { >> - =A0 =A0 =A0 =A0 =A0 =A0 IEEE80211_VERIFY_LENGTH(efrm - frm, frm[1], re= turn); >> + =A0 =A0 while (efrm - frm > 1) { >> + =A0 =A0 =A0 =A0 =A0 =A0 IEEE80211_VERIFY_LENGTH(efrm - frm, frm[1] + 2= , return); >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 switch (*frm) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 case IEEE80211_ELEMID_VENDOR: >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 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. >> >> =A0 ether_output >> =A0 =A0 ether_output_frame >> =A0 =A0 =A0 IFQ_HANDOFF/IFQ_HANDOFF_ADJ >> =A0 =A0 =A0 =A0 if_start >> =A0 =A0 =A0 =A0 =A0 iwi_start >> =A0 =A0 =A0 =A0 =A0 =A0 iwi_tx_start >> >> After 8.0-RELEASE, device specific if_transmit() is called via net80211 = layer. >> >> =A0 ether_output >> =A0 =A0 ether_output_frame >> =A0 =A0 =A0 if_transmit >> =A0 =A0 =A0 =A0 IFQ_HANDOFF/IFQ_HANDOFF_ADJ >> =A0 =A0 =A0 =A0 =A0 if_start >> =A0 =A0 =A0 =A0 =A0 =A0 ieee80211_start >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 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 >=3D 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 >> > 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