netdev - Re: [PATCH net-next v4 00/15][pull request] Introduce Intel IDPF driver

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: <e1beeb14-fbb5-216c-f661-2bb9a84ba724@intel.com>
Date: Tue, 8 Aug 2023 17:35:27 -0700
From: "Linga, Pavan Kumar" <pavan.kumar.linga@...el.com>
To: Jakub Kicinski <kuba@...nel.org>, Tony Nguyen <anthony.l.nguyen@...el.com>
CC: <davem@...emloft.net>, <pabeni@...hat.com>, <edumazet@...gle.com>,
	<netdev@...r.kernel.org>, <emil.s.tantilov@...el.com>,
	<jesse.brandeburg@...el.com>, <sridhar.samudrala@...el.com>,
	<shiraz.saleem@...el.com>, <sindhu.devale@...el.com>, <willemb@...gle.com>,
	<decot@...gle.com>, <andrew@...n.ch>, <leon@...nel.org>, <mst@...hat.com>,
	<simon.horman@...igine.com>, <shannon.nelson@....com>,
	<stephen@...workplumber.org>
Subject: Re: [PATCH net-next v4 00/15][pull request] Introduce Intel IDPF
 driver



On 8/8/2023 1:32 PM, Jakub Kicinski wrote:
> On Mon,  7 Aug 2023 17:34:01 -0700 Tony Nguyen wrote:
>> This patch series introduces the Intel Infrastructure Data Path Function
>> (IDPF) driver. It is used for both physical and virtual functions. Except
>> for some of the device operations the rest of the functionality is the
>> same for both PF and VF. IDPF uses virtchnl version2 opcodes and
>> structures defined in the virtchnl2 header file which helps the driver
>> to learn the capabilities and register offsets from the device
>> Control Plane (CP) instead of assuming the default values.
> 
> Patches 4 and 10 add kdoc warnings, please fix those.
> And double check all the checkpatch warning about lines > 80 chars.

Thanks for the feedback.

Will review the warnings regarding 80char limit. Are you wanting them 
all removed or is it okay to leave the ones that help readability?

 > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value 
'csum_caps' not described in enum 'idpf_cap_field'
 > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value 
'seg_caps' not described in enum 'idpf_cap_field'
 > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value 
'rss_caps' not described in enum 'idpf_cap_field'
 > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value 
'hsplit_caps' not described in enum 'idpf_cap_field'
 > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value 
'rsc_caps' not described in enum 'idpf_cap_field'
 > drivers/net/ethernet/intel/idpf/idpf.h:123: warning: Enum value 
'other_caps' not described in enum 'idpf_cap_field'
 > drivers/net/ethernet/intel/idpf/idpf_txrx.h:153: warning: Function 
parameter or member 'DEFINE_DMA_UNMAP_ADDR(dma' not described in 
'idpf_tx_buf'
 > drivers/net/ethernet/intel/idpf/idpf_txrx.h:153: warning: Function 
parameter or member 'DEFINE_DMA_UNMAP_LEN(len' not described in 
'idpf_tx_buf'

/**
  * enum idpf_cap_field - Offsets into capabilities struct for specific caps
  * @IDPF_BASE_CAPS: generic base capabilities
  * @IDPF_CSUM_CAPS: checksum offload capabilities
...
  */
enum idpf_cap_field {
          IDPF_BASE_CAPS          = -1,
          IDPF_CSUM_CAPS          = offsetof(struct 
virtchnl2_get_capabilities,
                                             csum_caps),
          IDPF_SEG_CAPS           = offsetof(struct
...
}


/**
  * struct idpf_tx_buf
  * @next_to_watch: Next descriptor to clean
  * @skb: Pointer to the skb
  * @dma: DMA address
  * @len: DMA length
...
  */
struct idpf_tx_buf {
         void *next_to_watch;
         struct sk_buff *skb;
         DEFINE_DMA_UNMAP_ADDR(dma);
         DEFINE_DMA_UNMAP_LEN(len);
...
}

The script is parsing the offsetof() argument as part of the enum, which 
is not true. I believe it to be a false positive. Same for the second 
one where it parses 'DEFINE_DMA_UNMAP_ADDR(dma'. Is it okay to use 'dma' 
and 'len' in the kdoc header as-is or please suggest if you prefer 
something?

Best Regards,
Pavan