linux-kernel - RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 21 Jun 2024 04:36:23 +0000
From: David Lin <yu-hao.lin@....com>
To: Brian Norris <briannorris@...omium.org>
CC: "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kvalo@...nel.org" <kvalo@...nel.org>, "francesco@...cini.it"
	<francesco@...cini.it>, Pete Hsieh <tsung-hsien.hsieh@....com>, Francesco
 Dolcini <francesco.dolcini@...adex.com>
Subject: RE: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client
 mode

Hi Brian,

> From: Brian Norris <briannorris@...omium.org>
> Sent: Friday, June 21, 2024 1:53 AM
> To: David Lin <yu-hao.lin@....com>
> Cc: linux-wireless@...r.kernel.org; linux-kernel@...r.kernel.org;
> kvalo@...nel.org; francesco@...cini.it; Pete Hsieh
> <tsung-hsien.hsieh@....com>; Francesco Dolcini
> <francesco.dolcini@...adex.com>
> Subject: Re: [EXT] Re: [PATCH v10 1/2] wifi: mwifiex: add host mlme for client
> mode
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Hi David,
> 
> On Sat, May 25, 2024 at 12:50:59AM +0000, David Lin wrote:
> >
> > > From: Brian Norris <briannorris@...omium.org>
> > > Sent: Saturday, May 25, 2024 6:55 AM
> > >
> > > On Fri, May 24, 2024 at 3:01 PM David Lin <yu-hao.lin@....com> wrote:
> > > > I think it needs time to support probe client. Can we put your
> > > > suggested comments to the code used to hook probe_client() and add
> > > >
> > > > "TODO: support probe client" to mwifiex_cfg80211_probe_client().
> > >
> > > Are you suggesting that you plan to actually implement proper
> > > probe_client support? Did you already do what I suggested, and
> > > understand why hostapd needs probe_client support? This seems to be
> > > a common pattern -- that reviewers are asking for you to do your
> > > research, and it takes several requests before you actually do it.
> > >
> > > Now that I've tried to do that research for you ... it looks like
> > > hostapd uses probe_client to augment TX MGMT acks, as a proxy for
> > > station presence / inactivity. If a station is inactive and
> > > non-responsive, we disconnect it eventually. So that looks to me
> > > like probe_client support should actually be optional, if your
> > > driver reports TX status? And in that case, I'd still recommend you try to fix
> hostapd.
> > >
> > > But if you're really planning to implement proper probe_client
> > > support, then I suppose the TODO approach is also OK.
> > >
> > > I'd also request that you please actually do your research when
> > > reviewers ask questions. I'm frankly not sure why I'm spending my
> > > time on the above research, when the onus should be on the submitter
> > > to explain why they're doing what they're doing.
> >
> > Yes. I know when aging time of station is out, hostapd will use probe_client
> to check if station is still there before really disconnect it.
> >
> > Without this feature, it won't really affect mayor function of hostapd.
> 
> I'm glad *you* know all about the above behavior, but *I* didn't know about it
> until I went and researched what this API does, and how hostapd is using it. But
> that isn't my job -- it's your job, as the code submitter, to explain your
> reasoning and reduce the amount of work that readers/reviewers/maintainers
> have to do to understand your code and agree that it is the right thing to do.
> 
> It's not clear to me that you've really learned the above lesson, and it's really
> affecting the rate at which I review your code. This is by far not the first time
> that you've placed the burden on the reader. And if you're going to make the
> job difficult, then I'll prioritize enjoying my free time, or stuff that actually pays
> me at $DAY_JOB, or ...

I will keep this in mind.

> 
> > That is the reason that I suggest that we put comments and TODO to the
> code.
> 
> OK, I suppose that works for me.
> 
> Brian

I suggest that we just put your comments and prepare patch v11.

Thanks,
David