Re: [PATCH v3] cfg80211/nl80211: Add support for NL80211_STA_INFO_RX_DURATION

[Mohammed Shafi Shajakhan <mohammed@codeaurora.org>]

Hi Johannes,

On Tue, Apr 05, 2016 at 12:04:10PM +0200, Johannes Berg wrote:
> Hi,
> 
> The implementation seems fine now, but I think the commit log needs some work.

[shafi] thanks for reviewing.

> 
> > Add support for new netlink attribute 'NL80211_STA_INFO_RX_DURATION'
> 
> I think it'd be worthwhile to describe the attribute a bit more,
> including why you're adding it.

[shafi] will update in v4

> 
> > This flag
> 
> There's no flag.

[shafi] i will replace it with the name 'NL80211_STA_INFO_RX_DURATION'

> 
> >  will be set when drivers can fill rx_duration (aggregate
> > PPDU duration(usecs) for all the frames from a peer) 
> 
> You have the description here, but putting it with the attribute would
> be better.

[shafi] ok

> 
> > via 'drv_sta_statistics' callback
> 
> drv_sta_statistics is a mac80211 detail, that's not relevant at
> cfg80211 level; mentioning that is just confusing. This can well used
> by non-mac80211 drivers.

[shafi] agreed, will remove it.

> 
> > Also make sta_info flags 'filled' as 64 bit to accommodate for new
> > per station stats. 
> 
> That sentence doesn't parse well.
> 
> > Extend 'PUT_SINFO' for supporting rx_duration
> > field and any new per sta information in future
> 
> That sentence I think should just be removed.

[shafi] ok.

-shafi