nested structs parsing

nested structs parsing

[Johannes Berg]

Hi,

For a while I haven't looked at my documentation for 802.11, and now I
noticed I'm getting warnings due to the nested parsing.

However, something seems to be wrong? I have, for example, this (in
net/mac80211/sta_info.h)

struct sta_info {
	...
        struct {
                u64 packets[IEEE80211_NUM_ACS];
                u64 bytes[IEEE80211_NUM_ACS];
                struct ieee80211_tx_rate last_rate;
                u64 msdu[IEEE80211_NUM_TIDS + 1];
        } tx_stats;
};

but I'm getting the following warnings now, with only "@tx_stats" being
described in the documentation:

net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
net/mac80211/sta_info.h:590: warning: Function parameter or member 'msdu' not described in 'sta_info'

I can understand the first three of those, but not the last one? Why is
the last one not qualified?

If I change it to this:

        struct {
                u64 packets[IEEE80211_NUM_ACS];
                u64 bytes[IEEE80211_NUM_ACS];
                /**
                 * @last_rate: last TX rate
                 */
                struct ieee80211_tx_rate last_rate;
                /**
                 * @msdu: # of MSDUs per TID
                 */
                u64 msdu[IEEE80211_NUM_TIDS + 1];
        } tx_stats;

I still get a warning on "tx_stats.last_rate", but not on "msdu", which
is sort of obvious from the warning text, but also rather unexpected.

Normally I'd say that the "msdu" warning is originally wrong

However, I'd also argue that if I'm using inline declarations, I
shouldn't have to write it like this:

        struct {
                u64 packets[IEEE80211_NUM_ACS];
                u64 bytes[IEEE80211_NUM_ACS];
                /**
                 * @tx_stats.last_rate: last TX rate
                 */
                struct ieee80211_tx_rate last_rate;
	...
	} tx_stats;

since the comment is contained in the scope of tx_stats already, but
that seems to be what I'd have to do today?

At least fixing one of these to make it consistent would be good :-)

Thanks,
johannes

Re: nested structs parsing

[Johannes Berg]

On Thu, 2018-03-29 at 11:46 +0200, Johannes Berg wrote:
> Hi,
> 
> For a while I haven't looked at my documentation for 802.11, and now I
> noticed I'm getting warnings due to the nested parsing.
> 
> However, something seems to be wrong? I have, for example, this (in
> net/mac80211/sta_info.h)
> 
> struct sta_info {
> 	...
>         struct {
>                 u64 packets[IEEE80211_NUM_ACS];
>                 u64 bytes[IEEE80211_NUM_ACS];
>                 struct ieee80211_tx_rate last_rate;
>                 u64 msdu[IEEE80211_NUM_TIDS + 1];
>         } tx_stats;
> };
> 
> but I'm getting the following warnings now, with only "@tx_stats" being
> described in the documentation:
> 
> net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
> net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
> net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
> net/mac80211/sta_info.h:590: warning: Function parameter or member 'msdu' not described in 'sta_info'
> 
> I can understand the first three of those, but not the last one? Why is
> the last one not qualified?
> 
> If I change it to this:
> 
>         struct {
>                 u64 packets[IEEE80211_NUM_ACS];
>                 u64 bytes[IEEE80211_NUM_ACS];
>                 /**
>                  * @last_rate: last TX rate
>                  */
>                 struct ieee80211_tx_rate last_rate;
>                 /**
>                  * @msdu: # of MSDUs per TID
>                  */
>                 u64 msdu[IEEE80211_NUM_TIDS + 1];
>         } tx_stats;
> 
> I still get a warning on "tx_stats.last_rate", but not on "msdu", which
> is sort of obvious from the warning text, but also rather unexpected.
> 
> Normally I'd say that the "msdu" warning is originally wrong
> 
> However, I'd also argue that if I'm using inline declarations, I
> shouldn't have to write it like this:
> 
>         struct {
>                 u64 packets[IEEE80211_NUM_ACS];
>                 u64 bytes[IEEE80211_NUM_ACS];
>                 /**
>                  * @tx_stats.last_rate: last TX rate
>                  */
>                 struct ieee80211_tx_rate last_rate;
> 	...
> 	} tx_stats;

Whoops, sent a fraction of a second too early - this actually causes a
warning too (no idea why four times):

net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:                  * @tx_stats.last_rate: last TX rate
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:                  * @tx_stats.last_rate: last TX rate
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:                  * @tx_stats.last_rate: last TX rate
net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:                  * @tx_stats.last_rate: last TX rate

johannes

Re: nested structs parsing

[Mauro Carvalho Chehab]

Em Thu, 29 Mar 2018 11:47:07 +0200
Johannes Berg <johannes@sipsolutions.net> escreveu:

> On Thu, 2018-03-29 at 11:46 +0200, Johannes Berg wrote:
> > Hi,
> > 
> > For a while I haven't looked at my documentation for 802.11, and now I
> > noticed I'm getting warnings due to the nested parsing.
> > 
> > However, something seems to be wrong? I have, for example, this (in
> > net/mac80211/sta_info.h)
> > 
> > struct sta_info {
> > 	...
> >         struct {
> >                 u64 packets[IEEE80211_NUM_ACS];
> >                 u64 bytes[IEEE80211_NUM_ACS];
> >                 struct ieee80211_tx_rate last_rate;
> >                 u64 msdu[IEEE80211_NUM_TIDS + 1];
> >         } tx_stats;
> > };
> > 
> > but I'm getting the following warnings now, with only "@tx_stats" being
> > described in the documentation:
> > 
> > net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack' not described in 'sta_info'
> > net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.last_ack_signal' not described in 'sta_info'
> > net/mac80211/sta_info.h:590: warning: Function parameter or member 'status_stats.ack_signal_filled' not described in 'sta_info'
> > net/mac80211/sta_info.h:590: warning: Function parameter or member 'msdu' not described in 'sta_info'
> > 
> > I can understand the first three of those, but not the last one? Why is
> > the last one not qualified?
> > 
> > If I change it to this:
> > 
> >         struct {
> >                 u64 packets[IEEE80211_NUM_ACS];
> >                 u64 bytes[IEEE80211_NUM_ACS];
> >                 /**
> >                  * @last_rate: last TX rate
> >                  */
> >                 struct ieee80211_tx_rate last_rate;
> >                 /**
> >                  * @msdu: # of MSDUs per TID
> >                  */
> >                 u64 msdu[IEEE80211_NUM_TIDS + 1];
> >         } tx_stats;
> > 
> > I still get a warning on "tx_stats.last_rate", but not on "msdu", which
> > is sort of obvious from the warning text, but also rather unexpected.
> > 
> > Normally I'd say that the "msdu" warning is originally wrong
> > 
> > However, I'd also argue that if I'm using inline declarations, I
> > shouldn't have to write it like this:
> > 
> >         struct {
> >                 u64 packets[IEEE80211_NUM_ACS];
> >                 u64 bytes[IEEE80211_NUM_ACS];
> >                 /**
> >                  * @tx_stats.last_rate: last TX rate
> >                  */
> >                 struct ieee80211_tx_rate last_rate;
> > 	...
> > 	} tx_stats;  
> 
> Whoops, sent a fraction of a second too early - this actually causes a
> warning too (no idea why four times):
> 
> net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:                  * @tx_stats.last_rate: last TX rate
> net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:                  * @tx_stats.last_rate: last TX rate
> net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:                  * @tx_stats.last_rate: last TX rate
> net/mac80211/sta_info.h:560: warning: Incorrect use of kernel-doc format:                  * @tx_stats.last_rate: last TX rate

The original patchset for nested structs was supporting it only 
when not inlined. This should be fixed on this patchset:

	https://lkml.org/lkml/2018/2/19/387

Do you have those patches on your tree?

With regards to duplicated warnings, that use to happen if the same header
is included several times (with is a common pratice at the net subsystem).

I also submitted some patches to linux-next addressing it.

Could you please merge from docs-next and see if those problems
get solved?

It is located at:

	git://git.lwn.net/linux.git docs-next

Thanks,
Mauro

Re: nested structs parsing

[Johannes Berg]

Hi,

> The original patchset for nested structs was supporting it only 
> when not inlined. This should be fixed on this patchset:
> 
> 	https://lkml.org/lkml/2018/2/19/387
> 
> Do you have those patches on your tree?

No, looks like I don't have those yet. I'll wait for those then.

> With regards to duplicated warnings, that use to happen if the same header
> is included several times (with is a common pratice at the net subsystem).

Yeah, doesn't really matter anyway. I think I have to, in a sense,
because I'm getting lots of functions separately from the headers.

> Could you please merge from docs-next and see if those problems
> get solved?

No, that doesn't seem to address it fully:

net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
net/mac80211/sta_info.h:586: warning: Function parameter or member 'msdu' not described in 'sta_info'

You can reproduce this in

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master

(merging with docs-next) and running

  make SPHINXDIRS=driver-api/80211 htmldocs

johannes

Re: nested structs parsing

[Mauro Carvalho Chehab]

Em Thu, 29 Mar 2018 16:26:33 +0200
Johannes Berg <johannes@sipsolutions.net> escreveu:

> Hi,
> 
> > The original patchset for nested structs was supporting it only 
> > when not inlined. This should be fixed on this patchset:
> > 
> > 	https://lkml.org/lkml/2018/2/19/387
> > 
> > Do you have those patches on your tree?  
> 
> No, looks like I don't have those yet. I'll wait for those then.
> 
> > With regards to duplicated warnings, that use to happen if the same header
> > is included several times (with is a common pratice at the net subsystem).  
> 
> Yeah, doesn't really matter anyway. I think I have to, in a sense,
> because I'm getting lots of functions separately from the headers.
> 
> > Could you please merge from docs-next and see if those problems
> > get solved?  
> 
> No, that doesn't seem to address it fully:
> 
> net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.packets' not described in 'sta_info'
> net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.bytes' not described in 'sta_info'
> net/mac80211/sta_info.h:586: warning: Function parameter or member 'tx_stats.last_rate' not described in 'sta_info'
> net/mac80211/sta_info.h:586: warning: Function parameter or member 'msdu' not described in 'sta_info'
> 
> You can reproduce this in
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
> 
> (merging with docs-next) and running
> 
>   make SPHINXDIRS=driver-api/80211 htmldocs

No need to run it for checking the errors... you can run just:

	./scripts/kernel-doc -none net/mac80211/sta_info.h 

Applying the enclosed patch seems to work:

<patch>
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index f64eb86ca64b..d81cb6155e8d 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -477,6 +477,10 @@ struct ieee80211_sta_rx_stats {
  * @tdls_chandef: a TDLS peer can have a wider chandef that is compatible to
  *	the BSS one.
  * @tx_stats: TX statistics
+ * @tx_stats.packets: foo
+ * @tx_stats.last_rate: bar
+ * @tx_stats.bytes: foobar
+ * @tx_stats.msdu: foo
  * @rx_stats: RX statistics
  * @pcpu_rx_stats: per-CPU RX statistics, assigned only if the driver needs
  *	this (by advertising the USES_RSS hw flag)
</patch>

What's weird is that tx_stats.msdu field seems to be parsed wrong.
I'll take a look on it.

Thanks,
Mauro