Hi Brian, > From: Brian Norris <briannorris@xxxxxxxxxxxx> > Sent: Friday, June 21, 2024 1:53 AM > To: David Lin <yu-hao.lin@xxxxxxx> > Cc: linux-wireless@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > kvalo@xxxxxxxxxx; francesco@xxxxxxxxxx; Pete Hsieh > <tsung-hsien.hsieh@xxxxxxx>; Francesco Dolcini > <francesco.dolcini@xxxxxxxxxxx> > 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@xxxxxxxxxxxx> > > > Sent: Saturday, May 25, 2024 6:55 AM > > > > > > On Fri, May 24, 2024 at 3:01 PM David Lin <yu-hao.lin@xxxxxxx> 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
