netdev - Re: [PATCH net-next] sfc: correct kernel-doc complaints

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]

Message-ID: <3f391457-d43c-4bd2-bd96-a5701a08e9eb@gmail.com>
Date: Fri, 5 Dec 2025 17:19:40 +0000
From: Edward Cree <ecree.xilinx@...il.com>
To: Randy Dunlap <rdunlap@...radead.org>, netdev@...r.kernel.org
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>, linux-net-drivers@....com
Subject: Re: [PATCH net-next] sfc: correct kernel-doc complaints

On 29/11/2025 22:03, Randy Dunlap wrote:
> Fix kernel-doc warnings by adding 3 missing struct member descriptions
> in struct efx_ef10_nic_data and removing preprocessor directives (which
> are not handled by kernel-doc).
> 
> Fixes these 5 warnings:
> Warning: drivers/net/ethernet/sfc/nic.h:158 bad line: #ifdef CONFIG_SFC_SRIOV
> Warning: drivers/net/ethernet/sfc/nic.h:160 bad line: #endif

Does kernel-doc not complain if a member is documented but the actual
 declaration is ifdefed out?  Normal practice seems to be to move the
 doc into another comment adjacent to the declaration so it's under
 the same ifdef; is that unnecessary?

> Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'port_id'
>  not described in 'efx_ef10_nic_data'
> Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'vf_index'
>  not described in 'efx_ef10_nic_data'
> Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'licensed_features'
>  not described in 'efx_ef10_nic_data'
> 
> Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
> ---
> Cc: Edward Cree <ecree.xilinx@...il.com>
> Cc: Andrew Lunn <andrew+netdev@...n.ch>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Eric Dumazet <edumazet@...gle.com>
> Cc: Jakub Kicinski <kuba@...nel.org>
> Cc: Paolo Abeni <pabeni@...hat.com>
> Cc: Simon Horman <horms@...nel.org>
> Cc: linux-net-drivers@....com
> ---
>  drivers/net/ethernet/sfc/nic.h |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> --- linux-next-20251128.orig/drivers/net/ethernet/sfc/nic.h
> +++ linux-next-20251128/drivers/net/ethernet/sfc/nic.h
> @@ -156,9 +156,10 @@ enum {
>   * @tx_dpcpu_fw_id: Firmware ID of the TxDPCPU
>   * @must_probe_vswitching: Flag: vswitching has yet to be setup after MC reboot
>   * @pf_index: The number for this PF, or the parent PF if this is a VF
> -#ifdef CONFIG_SFC_SRIOV
> + * @port_id: port id (Ethernet address) if !CONFIG_SFC_SRIOV;
> + *   for CONFIG_SFC_SRIOV, the VF port id

I think this is always the PF's MAC address, and is used by
 ndo_get_phys_port_id.  On EF100 that method only exists for PFs
 (the vfrep's ndo_get_port_parent_id also shows the PF's port_id),
 whereas on EF10 VFs also have a phys_port_id with the PF's MAC
 address.
I guess the best way to summarise this for the kerneldoc comment
 would be:
 * @port_id: Ethernet address of owning PF, used for phys_port_id
In our local tree we just have "@port_id: Physical port identity".

> + * @vf_index: Index of particular VF in the VF data structure

This isn't quite right; this field is the index of this VF more
 generally within the PF's set of VFs; it's provided by firmware,
 and passed back to firmware in various requests.
And when it's used as an index into the VF data structure array,
 it's the _parent PF's_ nic_data->vf that is indexed by the VF's
 nic_data->vf_index.  (The VF's nic_data->vf is %NULL afaik.)
Not really sure how to summarise this, other than just following
 the pattern of @pf_index above:
 * @vf_index: The number for this VF, or 0xFFFF if this is a VF
 which isn't greatly informative, but we could add more to @vf:

>   * @vf: Pointer to VF data structure

 * @vf: for a PF, array of VF data structures indexed by VF's
	@vf_index

> -#endif
>   * @vport_mac: The MAC address on the vport, only for PFs; VFs will be zero
>   * @vlan_list: List of VLANs added over the interface. Serialised by vlan_lock.
>   * @vlan_lock: Lock to serialize access to vlan_list.
> @@ -166,6 +167,7 @@ enum {
>   * @udp_tunnels_dirty: flag indicating a reboot occurred while pushing
>   *	@udp_tunnels to hardware and thus the push must be re-done.
>   * @udp_tunnels_lock: Serialises writes to @udp_tunnels and @udp_tunnels_dirty.
> + * @licensed_features: used to enable features if the adapter is licensed for it

In our local tree we have:
 * @licensed_features: Flags for licensed firmware features.
 which might be better as it doesn't give the impression that the
 driver can change this ('enable' things) — it's a bitmask that
 comes directly from firmware.

-ed