mbox series

[v2,00/56] net: txgbe PMD

Message ID 20201005120910.189343-1-jiawenwu@trustnetic.com (mailing list archive)
Headers
Series net: txgbe PMD |

Message

Jiawen Wu Oct. 5, 2020, 12:08 p.m. UTC
  v2: re-order patches and fix some known problems
v1: introduce txgbe PMD

jiawenwu (56):
  net/txgbe: add build and doc infrastructure
  net/txgbe: add ethdev probe and remove
  net/txgbe: add device init and uninit
  net/txgbe: add error types and registers
  net/txgbe: add mac type and bus lan id
  net/txgbe: add HW infrastructure and dummy function
  net/txgbe: add EEPROM functions
  net/txgbe: add HW init and reset operation
  net/txgbe: add PHY init
  net/txgbe: add module identify
  net/txgbe: add PHY reset
  net/txgbe: add info get operation
  net/txgbe: add interrupt operation
  net/txgbe: add device configure operation
  net/txgbe: add link status change
  net/txgbe: add multi-speed link setup
  net/txgbe: add autoc read and write
  net/txgbe: add MAC address operations
  net/txgbe: add unicast hash bitmap
  net/txgbe: add RX and TX init
  net/txgbe: add RX and TX queues setup and release
  net/txgbe: add RX and TX start and stop
  net/txgbe: add packet type
  net/txgbe: fill simple transmit function
  net/txgbe: fill transmit function with hardware offload
  net/txgbe: fill TX prepare funtion
  net/txgbe: fill receive functions
  net/txgbe: add device start operation
  net/txgbe: add RX and TX data path start and stop
  net/txgbe: add device stop and close operations
  net/txgbe: support RX interrupt
  net/txgbe: add RX and TX queue info get
  net/txgbe: add device stats get
  net/txgbe: add device xstats get
  net/txgbe: add queue stats mapping
  net/txgbe: add VLAN handle support
  net/txgbe: add SWFW semaphore and lock
  net/txgbe: add PF module init and uninit for SRIOV
  net/txgbe: add process mailbox operation
  net/txgbe: add PF module configure for SRIOV
  net/txgbe: add VMDq configure
  net/txgbe: add RSS support
  net/txgbe: add DCB support
  net/txgbe: add flow control support
  net/txgbe: add FC auto negotiation support
  net/txgbe: add priority flow control support
  net/txgbe: add device promiscuous and allmulticast mode
  net/txgbe: add MTU set operation
  net/txgbe: add FW version get operation
  net/txgbe: add EEPROM info get operation
  net/txgbe: add register dump support
  net/txgbe: support device LED on and off
  net/txgbe: add mirror rule operations
  net/txgbe: add PTP support
  net/txgbe: add DCB info get operation
  net/txgbe: add Rx and Tx descriptor status

 MAINTAINERS                                 |    7 +
 doc/guides/nics/features/txgbe.ini          |   54 +
 doc/guides/nics/txgbe.rst                   |   50 +
 drivers/net/meson.build                     |    1 +
 drivers/net/txgbe/base/meson.build          |   26 +
 drivers/net/txgbe/base/txgbe.h              |   16 +
 drivers/net/txgbe/base/txgbe_dcb.c          |  360 ++
 drivers/net/txgbe/base/txgbe_dcb.h          |  113 +
 drivers/net/txgbe/base/txgbe_dcb_hw.c       |  283 ++
 drivers/net/txgbe/base/txgbe_dcb_hw.h       |   23 +
 drivers/net/txgbe/base/txgbe_devids.h       |   40 +
 drivers/net/txgbe/base/txgbe_dummy.h        |  739 +++
 drivers/net/txgbe/base/txgbe_eeprom.c       |  582 +++
 drivers/net/txgbe/base/txgbe_eeprom.h       |   62 +
 drivers/net/txgbe/base/txgbe_hw.c           | 3800 +++++++++++++++
 drivers/net/txgbe/base/txgbe_hw.h           |  112 +
 drivers/net/txgbe/base/txgbe_mbx.c          |  332 ++
 drivers/net/txgbe/base/txgbe_mbx.h          |   92 +
 drivers/net/txgbe/base/txgbe_mng.c          |  399 ++
 drivers/net/txgbe/base/txgbe_mng.h          |  175 +
 drivers/net/txgbe/base/txgbe_osdep.h        |  184 +
 drivers/net/txgbe/base/txgbe_phy.c          | 2239 +++++++++
 drivers/net/txgbe/base/txgbe_phy.h          |  375 ++
 drivers/net/txgbe/base/txgbe_regs.h         | 1890 ++++++++
 drivers/net/txgbe/base/txgbe_status.h       |  122 +
 drivers/net/txgbe/base/txgbe_type.h         |  710 +++
 drivers/net/txgbe/meson.build               |   19 +
 drivers/net/txgbe/rte_pmd_txgbe.c           |   11 +
 drivers/net/txgbe/rte_pmd_txgbe.h           |   37 +
 drivers/net/txgbe/rte_pmd_txgbe_version.map |    3 +
 drivers/net/txgbe/txgbe_ethdev.c            | 4273 +++++++++++++++++
 drivers/net/txgbe/txgbe_ethdev.h            |  413 ++
 drivers/net/txgbe/txgbe_logs.h              |   98 +
 drivers/net/txgbe/txgbe_pf.c                |  880 ++++
 drivers/net/txgbe/txgbe_ptypes.c            |  674 +++
 drivers/net/txgbe/txgbe_ptypes.h            |  351 ++
 drivers/net/txgbe/txgbe_regs_group.h        |   54 +
 drivers/net/txgbe/txgbe_rxtx.c              | 4671 +++++++++++++++++++
 drivers/net/txgbe/txgbe_rxtx.h              |  412 ++
 39 files changed, 24682 insertions(+)
 create mode 100644 doc/guides/nics/features/txgbe.ini
 create mode 100644 doc/guides/nics/txgbe.rst
 create mode 100644 drivers/net/txgbe/base/meson.build
 create mode 100644 drivers/net/txgbe/base/txgbe.h
 create mode 100644 drivers/net/txgbe/base/txgbe_dcb.c
 create mode 100644 drivers/net/txgbe/base/txgbe_dcb.h
 create mode 100644 drivers/net/txgbe/base/txgbe_dcb_hw.c
 create mode 100644 drivers/net/txgbe/base/txgbe_dcb_hw.h
 create mode 100644 drivers/net/txgbe/base/txgbe_devids.h
 create mode 100644 drivers/net/txgbe/base/txgbe_dummy.h
 create mode 100644 drivers/net/txgbe/base/txgbe_eeprom.c
 create mode 100644 drivers/net/txgbe/base/txgbe_eeprom.h
 create mode 100644 drivers/net/txgbe/base/txgbe_hw.c
 create mode 100644 drivers/net/txgbe/base/txgbe_hw.h
 create mode 100644 drivers/net/txgbe/base/txgbe_mbx.c
 create mode 100644 drivers/net/txgbe/base/txgbe_mbx.h
 create mode 100644 drivers/net/txgbe/base/txgbe_mng.c
 create mode 100644 drivers/net/txgbe/base/txgbe_mng.h
 create mode 100644 drivers/net/txgbe/base/txgbe_osdep.h
 create mode 100644 drivers/net/txgbe/base/txgbe_phy.c
 create mode 100644 drivers/net/txgbe/base/txgbe_phy.h
 create mode 100644 drivers/net/txgbe/base/txgbe_regs.h
 create mode 100644 drivers/net/txgbe/base/txgbe_status.h
 create mode 100644 drivers/net/txgbe/base/txgbe_type.h
 create mode 100644 drivers/net/txgbe/meson.build
 create mode 100644 drivers/net/txgbe/rte_pmd_txgbe.c
 create mode 100644 drivers/net/txgbe/rte_pmd_txgbe.h
 create mode 100644 drivers/net/txgbe/rte_pmd_txgbe_version.map
 create mode 100644 drivers/net/txgbe/txgbe_ethdev.c
 create mode 100644 drivers/net/txgbe/txgbe_ethdev.h
 create mode 100644 drivers/net/txgbe/txgbe_logs.h
 create mode 100644 drivers/net/txgbe/txgbe_pf.c
 create mode 100644 drivers/net/txgbe/txgbe_ptypes.c
 create mode 100644 drivers/net/txgbe/txgbe_ptypes.h
 create mode 100644 drivers/net/txgbe/txgbe_regs_group.h
 create mode 100644 drivers/net/txgbe/txgbe_rxtx.c
 create mode 100644 drivers/net/txgbe/txgbe_rxtx.h
  

Comments

Ferruh Yigit Oct. 6, 2020, 11:02 a.m. UTC | #1
On 10/5/2020 1:08 PM, Jiawen Wu wrote:
> v2: re-order patches and fix some known problems
> v1: introduce txgbe PMD
> 
> jiawenwu (56):
>    net/txgbe: add build and doc infrastructure
>    net/txgbe: add ethdev probe and remove
>    net/txgbe: add device init and uninit
>    net/txgbe: add error types and registers
>    net/txgbe: add mac type and bus lan id
>    net/txgbe: add HW infrastructure and dummy function
>    net/txgbe: add EEPROM functions
>    net/txgbe: add HW init and reset operation
>    net/txgbe: add PHY init
>    net/txgbe: add module identify
>    net/txgbe: add PHY reset
>    net/txgbe: add info get operation
>    net/txgbe: add interrupt operation
>    net/txgbe: add device configure operation
>    net/txgbe: add link status change
>    net/txgbe: add multi-speed link setup
>    net/txgbe: add autoc read and write
>    net/txgbe: add MAC address operations
>    net/txgbe: add unicast hash bitmap
>    net/txgbe: add RX and TX init
>    net/txgbe: add RX and TX queues setup and release
>    net/txgbe: add RX and TX start and stop
>    net/txgbe: add packet type
>    net/txgbe: fill simple transmit function
>    net/txgbe: fill transmit function with hardware offload
>    net/txgbe: fill TX prepare funtion
>    net/txgbe: fill receive functions
>    net/txgbe: add device start operation
>    net/txgbe: add RX and TX data path start and stop
>    net/txgbe: add device stop and close operations
>    net/txgbe: support RX interrupt
>    net/txgbe: add RX and TX queue info get
>    net/txgbe: add device stats get
>    net/txgbe: add device xstats get
>    net/txgbe: add queue stats mapping
>    net/txgbe: add VLAN handle support
>    net/txgbe: add SWFW semaphore and lock
>    net/txgbe: add PF module init and uninit for SRIOV
>    net/txgbe: add process mailbox operation
>    net/txgbe: add PF module configure for SRIOV
>    net/txgbe: add VMDq configure
>    net/txgbe: add RSS support
>    net/txgbe: add DCB support
>    net/txgbe: add flow control support
>    net/txgbe: add FC auto negotiation support
>    net/txgbe: add priority flow control support
>    net/txgbe: add device promiscuous and allmulticast mode
>    net/txgbe: add MTU set operation
>    net/txgbe: add FW version get operation
>    net/txgbe: add EEPROM info get operation
>    net/txgbe: add register dump support
>    net/txgbe: support device LED on and off
>    net/txgbe: add mirror rule operations
>    net/txgbe: add PTP support
>    net/txgbe: add DCB info get operation
>    net/txgbe: add Rx and Tx descriptor status
> 

Hi Jiawen,

Before going into more detailed reviews, the patchset conflicts with some recent 
changes in the main repo [1], can you please rebase on top of the latest head of 
the repo?

Also DPDK syntax/style check scripts are giving errors, can you please fix them 
too? You should run following to check:
./devtools/checkpatches.sh -n56
./devtools/check-git-log.sh -n56
(This one needs codespell package to show spelling errors)



[1] mainly the list is:
1) PMD close behavior change,
    - .dev_close changes
    - RTE_ETH_DEV_CLOSE_REMOVE flag removed

2) Some dev_ops moved to ethdev struct
    - .rx_queue_count
    - .rx_descriptor_done
    - .rx_descriptor_status
    - .tx_descriptor_status
  
Jiawen Wu Oct. 9, 2020, 3:03 a.m. UTC | #2
Hi Ferruh,

For the syntax/style check issue, should I fix all the errors and warnings or just fix the errors?
It seems to be a lot of warnings.

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Tuesday, October 6, 2020 7:03 PM
To: Jiawen Wu <jiawenwu@trustnetic.com>; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 00/56] net: txgbe PMD

On 10/5/2020 1:08 PM, Jiawen Wu wrote:
> v2: re-order patches and fix some known problems
> v1: introduce txgbe PMD
> 
> jiawenwu (56):
>    net/txgbe: add build and doc infrastructure
>    net/txgbe: add ethdev probe and remove
>    net/txgbe: add device init and uninit
>    net/txgbe: add error types and registers
>    net/txgbe: add mac type and bus lan id
>    net/txgbe: add HW infrastructure and dummy function
>    net/txgbe: add EEPROM functions
>    net/txgbe: add HW init and reset operation
>    net/txgbe: add PHY init
>    net/txgbe: add module identify
>    net/txgbe: add PHY reset
>    net/txgbe: add info get operation
>    net/txgbe: add interrupt operation
>    net/txgbe: add device configure operation
>    net/txgbe: add link status change
>    net/txgbe: add multi-speed link setup
>    net/txgbe: add autoc read and write
>    net/txgbe: add MAC address operations
>    net/txgbe: add unicast hash bitmap
>    net/txgbe: add RX and TX init
>    net/txgbe: add RX and TX queues setup and release
>    net/txgbe: add RX and TX start and stop
>    net/txgbe: add packet type
>    net/txgbe: fill simple transmit function
>    net/txgbe: fill transmit function with hardware offload
>    net/txgbe: fill TX prepare funtion
>    net/txgbe: fill receive functions
>    net/txgbe: add device start operation
>    net/txgbe: add RX and TX data path start and stop
>    net/txgbe: add device stop and close operations
>    net/txgbe: support RX interrupt
>    net/txgbe: add RX and TX queue info get
>    net/txgbe: add device stats get
>    net/txgbe: add device xstats get
>    net/txgbe: add queue stats mapping
>    net/txgbe: add VLAN handle support
>    net/txgbe: add SWFW semaphore and lock
>    net/txgbe: add PF module init and uninit for SRIOV
>    net/txgbe: add process mailbox operation
>    net/txgbe: add PF module configure for SRIOV
>    net/txgbe: add VMDq configure
>    net/txgbe: add RSS support
>    net/txgbe: add DCB support
>    net/txgbe: add flow control support
>    net/txgbe: add FC auto negotiation support
>    net/txgbe: add priority flow control support
>    net/txgbe: add device promiscuous and allmulticast mode
>    net/txgbe: add MTU set operation
>    net/txgbe: add FW version get operation
>    net/txgbe: add EEPROM info get operation
>    net/txgbe: add register dump support
>    net/txgbe: support device LED on and off
>    net/txgbe: add mirror rule operations
>    net/txgbe: add PTP support
>    net/txgbe: add DCB info get operation
>    net/txgbe: add Rx and Tx descriptor status
> 

Hi Jiawen,

Before going into more detailed reviews, the patchset conflicts with some recent changes in the main repo [1], can you please rebase on top of the latest head of the repo?

Also DPDK syntax/style check scripts are giving errors, can you please fix them too? You should run following to check:
./devtools/checkpatches.sh -n56
./devtools/check-git-log.sh -n56
(This one needs codespell package to show spelling errors)



[1] mainly the list is:
1) PMD close behavior change,
    - .dev_close changes
    - RTE_ETH_DEV_CLOSE_REMOVE flag removed

2) Some dev_ops moved to ethdev struct
    - .rx_queue_count
    - .rx_descriptor_done
    - .rx_descriptor_status
    - .tx_descriptor_status
  
Ferruh Yigit Oct. 9, 2020, 9:47 a.m. UTC | #3
On 10/9/2020 4:03 AM, jiawenwu@trustnetic.com wrote:
> Hi Ferruh,
> 
> For the syntax/style check issue, should I fix all the errors and warnings or just fix the errors?
> It seems to be a lot of warnings.
> 

[Please don't top post, it makes archives un-readable]

Please fix all, but beware that there may be false positive in the checkpatch 
warnings, so you need to process the output first.
This is a new PMD, if the syntax is not put correct at first place, very 
unlikely that it will be fixed later, so lets try to fix them as much as possible.

For some drivers, the base code is shared in multiple platforms, like Linux, 
FreeBSD, Windows etc..., for them we are more flexible and we allow to keep the 
original syntax of that shared code, *as long as it is consistent within 
itself*. Do you have similar case in the base folder files?

The code for the DPDK should follow the DPDK coding convention [1] and should 
have as less checkpatch warnings/errors as possible.

[1] https://doc.dpdk.org/guides/contributing/coding_style.html

Thanks,
ferruh


> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, October 6, 2020 7:03 PM
> To: Jiawen Wu <jiawenwu@trustnetic.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 00/56] net: txgbe PMD
> 
> On 10/5/2020 1:08 PM, Jiawen Wu wrote:
>> v2: re-order patches and fix some known problems
>> v1: introduce txgbe PMD
>>
>> jiawenwu (56):
>>     net/txgbe: add build and doc infrastructure
>>     net/txgbe: add ethdev probe and remove
>>     net/txgbe: add device init and uninit
>>     net/txgbe: add error types and registers
>>     net/txgbe: add mac type and bus lan id
>>     net/txgbe: add HW infrastructure and dummy function
>>     net/txgbe: add EEPROM functions
>>     net/txgbe: add HW init and reset operation
>>     net/txgbe: add PHY init
>>     net/txgbe: add module identify
>>     net/txgbe: add PHY reset
>>     net/txgbe: add info get operation
>>     net/txgbe: add interrupt operation
>>     net/txgbe: add device configure operation
>>     net/txgbe: add link status change
>>     net/txgbe: add multi-speed link setup
>>     net/txgbe: add autoc read and write
>>     net/txgbe: add MAC address operations
>>     net/txgbe: add unicast hash bitmap
>>     net/txgbe: add RX and TX init
>>     net/txgbe: add RX and TX queues setup and release
>>     net/txgbe: add RX and TX start and stop
>>     net/txgbe: add packet type
>>     net/txgbe: fill simple transmit function
>>     net/txgbe: fill transmit function with hardware offload
>>     net/txgbe: fill TX prepare funtion
>>     net/txgbe: fill receive functions
>>     net/txgbe: add device start operation
>>     net/txgbe: add RX and TX data path start and stop
>>     net/txgbe: add device stop and close operations
>>     net/txgbe: support RX interrupt
>>     net/txgbe: add RX and TX queue info get
>>     net/txgbe: add device stats get
>>     net/txgbe: add device xstats get
>>     net/txgbe: add queue stats mapping
>>     net/txgbe: add VLAN handle support
>>     net/txgbe: add SWFW semaphore and lock
>>     net/txgbe: add PF module init and uninit for SRIOV
>>     net/txgbe: add process mailbox operation
>>     net/txgbe: add PF module configure for SRIOV
>>     net/txgbe: add VMDq configure
>>     net/txgbe: add RSS support
>>     net/txgbe: add DCB support
>>     net/txgbe: add flow control support
>>     net/txgbe: add FC auto negotiation support
>>     net/txgbe: add priority flow control support
>>     net/txgbe: add device promiscuous and allmulticast mode
>>     net/txgbe: add MTU set operation
>>     net/txgbe: add FW version get operation
>>     net/txgbe: add EEPROM info get operation
>>     net/txgbe: add register dump support
>>     net/txgbe: support device LED on and off
>>     net/txgbe: add mirror rule operations
>>     net/txgbe: add PTP support
>>     net/txgbe: add DCB info get operation
>>     net/txgbe: add Rx and Tx descriptor status
>>
> 
> Hi Jiawen,
> 
> Before going into more detailed reviews, the patchset conflicts with some recent changes in the main repo [1], can you please rebase on top of the latest head of the repo?
> 
> Also DPDK syntax/style check scripts are giving errors, can you please fix them too? You should run following to check:
> ./devtools/checkpatches.sh -n56
> ./devtools/check-git-log.sh -n56
> (This one needs codespell package to show spelling errors)
> 
> 
> 
> [1] mainly the list is:
> 1) PMD close behavior change,
>      - .dev_close changes
>      - RTE_ETH_DEV_CLOSE_REMOVE flag removed
> 
> 2) Some dev_ops moved to ethdev struct
>      - .rx_queue_count
>      - .rx_descriptor_done
>      - .rx_descriptor_status
>      - .tx_descriptor_status
> 
> 
> 
>
  
Jiawen Wu Oct. 10, 2020, 9:45 a.m. UTC | #4
On 10/9/2020 5:47 PM, Ferruh Yigit wrote:
> On 10/9/2020 4:03 AM, jiawenwu@trustnetic.com wrote:
> > Hi Ferruh,
> >
> > For the syntax/style check issue, should I fix all the errors and warnings or
> just fix the errors?
> > It seems to be a lot of warnings.
> >
> 
> [Please don't top post, it makes archives un-readable]
> 
> Please fix all, but beware that there may be false positive in the checkpatch
> warnings, so you need to process the output first.
> This is a new PMD, if the syntax is not put correct at first place, very unlikely
> that it will be fixed later, so lets try to fix them as much as possible.
> 
> For some drivers, the base code is shared in multiple platforms, like Linux,
> FreeBSD, Windows etc..., for them we are more flexible and we allow to keep
> the original syntax of that shared code, *as long as it is consistent within itself*.
> Do you have similar case in the base folder files?
> 
> The code for the DPDK should follow the DPDK coding convention [1] and should
> have as less checkpatch warnings/errors as possible.
> 
> [1] https://doc.dpdk.org/guides/contributing/coding_style.html
> 
> Thanks,
> ferruh
> 
> 

There are some 'checks' show that macro argument reused will possible take side-effects.
Like this:

CHECK:MACRO_ARG_REUSE: Macro argument reuse 'y' - possible side-effects?
#56: FILE: drivers/net/txgbe/base/txgbe_regs.h:35:
+#define ROUND_UP(x, y)          (((x) + (y) - 1) / (y) * (y))

But the example given in the DPDK coding convention is:

#define MACRO(x, y) do {      \
		variable = (x) + (y);   \
		(y) += 2;            \
} while(0)

It seems to reuse argument, too.
Should I fix this 'check', or treat it as a false positive?


> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > Sent: Tuesday, October 6, 2020 7:03 PM
> > To: Jiawen Wu <jiawenwu@trustnetic.com>; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 00/56] net: txgbe PMD
> >
> > On 10/5/2020 1:08 PM, Jiawen Wu wrote:
> >> v2: re-order patches and fix some known problems
> >> v1: introduce txgbe PMD
> >>
> >> jiawenwu (56):
> >>     net/txgbe: add build and doc infrastructure
> >>     net/txgbe: add ethdev probe and remove
> >>     net/txgbe: add device init and uninit
> >>     net/txgbe: add error types and registers
> >>     net/txgbe: add mac type and bus lan id
> >>     net/txgbe: add HW infrastructure and dummy function
> >>     net/txgbe: add EEPROM functions
> >>     net/txgbe: add HW init and reset operation
> >>     net/txgbe: add PHY init
> >>     net/txgbe: add module identify
> >>     net/txgbe: add PHY reset
> >>     net/txgbe: add info get operation
> >>     net/txgbe: add interrupt operation
> >>     net/txgbe: add device configure operation
> >>     net/txgbe: add link status change
> >>     net/txgbe: add multi-speed link setup
> >>     net/txgbe: add autoc read and write
> >>     net/txgbe: add MAC address operations
> >>     net/txgbe: add unicast hash bitmap
> >>     net/txgbe: add RX and TX init
> >>     net/txgbe: add RX and TX queues setup and release
> >>     net/txgbe: add RX and TX start and stop
> >>     net/txgbe: add packet type
> >>     net/txgbe: fill simple transmit function
> >>     net/txgbe: fill transmit function with hardware offload
> >>     net/txgbe: fill TX prepare funtion
> >>     net/txgbe: fill receive functions
> >>     net/txgbe: add device start operation
> >>     net/txgbe: add RX and TX data path start and stop
> >>     net/txgbe: add device stop and close operations
> >>     net/txgbe: support RX interrupt
> >>     net/txgbe: add RX and TX queue info get
> >>     net/txgbe: add device stats get
> >>     net/txgbe: add device xstats get
> >>     net/txgbe: add queue stats mapping
> >>     net/txgbe: add VLAN handle support
> >>     net/txgbe: add SWFW semaphore and lock
> >>     net/txgbe: add PF module init and uninit for SRIOV
> >>     net/txgbe: add process mailbox operation
> >>     net/txgbe: add PF module configure for SRIOV
> >>     net/txgbe: add VMDq configure
> >>     net/txgbe: add RSS support
> >>     net/txgbe: add DCB support
> >>     net/txgbe: add flow control support
> >>     net/txgbe: add FC auto negotiation support
> >>     net/txgbe: add priority flow control support
> >>     net/txgbe: add device promiscuous and allmulticast mode
> >>     net/txgbe: add MTU set operation
> >>     net/txgbe: add FW version get operation
> >>     net/txgbe: add EEPROM info get operation
> >>     net/txgbe: add register dump support
> >>     net/txgbe: support device LED on and off
> >>     net/txgbe: add mirror rule operations
> >>     net/txgbe: add PTP support
> >>     net/txgbe: add DCB info get operation
> >>     net/txgbe: add Rx and Tx descriptor status
> >>
> >
> > Hi Jiawen,
> >
> > Before going into more detailed reviews, the patchset conflicts with some
> recent changes in the main repo [1], can you please rebase on top of the latest
> head of the repo?
> >
> > Also DPDK syntax/style check scripts are giving errors, can you please fix
> them too? You should run following to check:
> > ./devtools/checkpatches.sh -n56
> > ./devtools/check-git-log.sh -n56
> > (This one needs codespell package to show spelling errors)
> >
> >
> >
> > [1] mainly the list is:
> > 1) PMD close behavior change,
> >      - .dev_close changes
> >      - RTE_ETH_DEV_CLOSE_REMOVE flag removed
> >
> > 2) Some dev_ops moved to ethdev struct
> >      - .rx_queue_count
> >      - .rx_descriptor_done
> >      - .rx_descriptor_status
> >      - .tx_descriptor_status
> >
> >
> >
> >
  
Ferruh Yigit Oct. 12, 2020, 8:37 a.m. UTC | #5
On 10/10/2020 10:45 AM, Jiawen Wu wrote:
> On 10/9/2020 5:47 PM, Ferruh Yigit wrote:
>> On 10/9/2020 4:03 AM, jiawenwu@trustnetic.com wrote:
>>> Hi Ferruh,
>>>
>>> For the syntax/style check issue, should I fix all the errors and warnings or
>> just fix the errors?
>>> It seems to be a lot of warnings.
>>>
>>
>> [Please don't top post, it makes archives un-readable]
>>
>> Please fix all, but beware that there may be false positive in the checkpatch
>> warnings, so you need to process the output first.
>> This is a new PMD, if the syntax is not put correct at first place, very unlikely
>> that it will be fixed later, so lets try to fix them as much as possible.
>>
>> For some drivers, the base code is shared in multiple platforms, like Linux,
>> FreeBSD, Windows etc..., for them we are more flexible and we allow to keep
>> the original syntax of that shared code, *as long as it is consistent within itself*.
>> Do you have similar case in the base folder files?
>>
>> The code for the DPDK should follow the DPDK coding convention [1] and should
>> have as less checkpatch warnings/errors as possible.
>>
>> [1] https://doc.dpdk.org/guides/contributing/coding_style.html
>>
>> Thanks,
>> ferruh
>>
>>
> 
> There are some 'checks' show that macro argument reused will possible take side-effects.
> Like this:
> 
> CHECK:MACRO_ARG_REUSE: Macro argument reuse 'y' - possible side-effects?
> #56: FILE: drivers/net/txgbe/base/txgbe_regs.h:35:
> +#define ROUND_UP(x, y)          (((x) + (y) - 1) / (y) * (y))
> 
> But the example given in the DPDK coding convention is:
> 
> #define MACRO(x, y) do {      \
> 		variable = (x) + (y);   \
> 		(y) += 2;            \
> } while(0)
> 
> It seems to reuse argument, too.
> Should I fix this 'check', or treat it as a false positive?
> 

That is a check, please double check if the usage is safe, and if it is you can 
ignore it.


> 
>>> -----Original Message-----
>>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>> Sent: Tuesday, October 6, 2020 7:03 PM
>>> To: Jiawen Wu <jiawenwu@trustnetic.com>; dev@dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH v2 00/56] net: txgbe PMD
>>>
>>> On 10/5/2020 1:08 PM, Jiawen Wu wrote:
>>>> v2: re-order patches and fix some known problems
>>>> v1: introduce txgbe PMD
>>>>
>>>> jiawenwu (56):
>>>>      net/txgbe: add build and doc infrastructure
>>>>      net/txgbe: add ethdev probe and remove
>>>>      net/txgbe: add device init and uninit
>>>>      net/txgbe: add error types and registers
>>>>      net/txgbe: add mac type and bus lan id
>>>>      net/txgbe: add HW infrastructure and dummy function
>>>>      net/txgbe: add EEPROM functions
>>>>      net/txgbe: add HW init and reset operation
>>>>      net/txgbe: add PHY init
>>>>      net/txgbe: add module identify
>>>>      net/txgbe: add PHY reset
>>>>      net/txgbe: add info get operation
>>>>      net/txgbe: add interrupt operation
>>>>      net/txgbe: add device configure operation
>>>>      net/txgbe: add link status change
>>>>      net/txgbe: add multi-speed link setup
>>>>      net/txgbe: add autoc read and write
>>>>      net/txgbe: add MAC address operations
>>>>      net/txgbe: add unicast hash bitmap
>>>>      net/txgbe: add RX and TX init
>>>>      net/txgbe: add RX and TX queues setup and release
>>>>      net/txgbe: add RX and TX start and stop
>>>>      net/txgbe: add packet type
>>>>      net/txgbe: fill simple transmit function
>>>>      net/txgbe: fill transmit function with hardware offload
>>>>      net/txgbe: fill TX prepare funtion
>>>>      net/txgbe: fill receive functions
>>>>      net/txgbe: add device start operation
>>>>      net/txgbe: add RX and TX data path start and stop
>>>>      net/txgbe: add device stop and close operations
>>>>      net/txgbe: support RX interrupt
>>>>      net/txgbe: add RX and TX queue info get
>>>>      net/txgbe: add device stats get
>>>>      net/txgbe: add device xstats get
>>>>      net/txgbe: add queue stats mapping
>>>>      net/txgbe: add VLAN handle support
>>>>      net/txgbe: add SWFW semaphore and lock
>>>>      net/txgbe: add PF module init and uninit for SRIOV
>>>>      net/txgbe: add process mailbox operation
>>>>      net/txgbe: add PF module configure for SRIOV
>>>>      net/txgbe: add VMDq configure
>>>>      net/txgbe: add RSS support
>>>>      net/txgbe: add DCB support
>>>>      net/txgbe: add flow control support
>>>>      net/txgbe: add FC auto negotiation support
>>>>      net/txgbe: add priority flow control support
>>>>      net/txgbe: add device promiscuous and allmulticast mode
>>>>      net/txgbe: add MTU set operation
>>>>      net/txgbe: add FW version get operation
>>>>      net/txgbe: add EEPROM info get operation
>>>>      net/txgbe: add register dump support
>>>>      net/txgbe: support device LED on and off
>>>>      net/txgbe: add mirror rule operations
>>>>      net/txgbe: add PTP support
>>>>      net/txgbe: add DCB info get operation
>>>>      net/txgbe: add Rx and Tx descriptor status
>>>>
>>>
>>> Hi Jiawen,
>>>
>>> Before going into more detailed reviews, the patchset conflicts with some
>> recent changes in the main repo [1], can you please rebase on top of the latest
>> head of the repo?
>>>
>>> Also DPDK syntax/style check scripts are giving errors, can you please fix
>> them too? You should run following to check:
>>> ./devtools/checkpatches.sh -n56
>>> ./devtools/check-git-log.sh -n56
>>> (This one needs codespell package to show spelling errors)
>>>
>>>
>>>
>>> [1] mainly the list is:
>>> 1) PMD close behavior change,
>>>       - .dev_close changes
>>>       - RTE_ETH_DEV_CLOSE_REMOVE flag removed
>>>
>>> 2) Some dev_ops moved to ethdev struct
>>>       - .rx_queue_count
>>>       - .rx_descriptor_done
>>>       - .rx_descriptor_status
>>>       - .tx_descriptor_status
>>>
>>>
>>>
>>>
> 
> 
> 
>