On Fri, 21 Feb 2025 10:03:28 +0800
Junlong Wang <wang.junlo...@zte.com.cn> wrote:
> V2:
> - modify CI some error results(checkpatches warnings、Wrong headline format)
> - fix warnings when enable extra warnings.
> - modify apply memcpy script for coccinelle and unnecessary init and
> unneccessary cast of void when use malloc.
>
> V1:
> - updated net zxdh driver.
> optimize init and some ops.
> provided csum/lro/tso 、extend stats、fw_version、module_info 、meter, etc.
>
> Junlong Wang (16):
> net/zxdh: optimize np dtb channel initialization
> net/zxdh: optimize queue res alloc/free process
> net/zxdh: optimize link update process
> net/zxdh: update Rx/Tx to latest
> net/zxdh: provided PF/VF msg intr callback
> net/zxdh: optimize MAC ops
> net/zxdh: optimize promisc ops
> net/zxdh: optimize VLAN filter/offload ops
> net/zxdh: optimize RSS/RETA hash config/update/get
> net/zxdh: optimize MTU set ops
> net/zxdh: optimize basic stats ops
> net/zxdh: provided CSUM/TSO/LRO config
> net/zxdh: provided rxq/txq info get implementations
> net/zxdh: provide extended stats ops implementations
> net/zxdh: provide ptypes FW version EEPROM ops
> net/zxdh: provide meter ops implementations
>
> doc/guides/nics/features/zxdh.ini | 11 +
> doc/guides/nics/zxdh.rst | 5 +
> drivers/net/zxdh/meson.build | 1 +
> drivers/net/zxdh/zxdh_common.c | 46 +-
> drivers/net/zxdh/zxdh_common.h | 3 +
> drivers/net/zxdh/zxdh_ethdev.c | 724 +++++++++++++---
> drivers/net/zxdh/zxdh_ethdev.h | 69 +-
> drivers/net/zxdh/zxdh_ethdev_ops.c | 916 ++++++++++++++++++---
> drivers/net/zxdh/zxdh_ethdev_ops.h | 53 +-
> drivers/net/zxdh/zxdh_msg.c | 985 +++++++++++++++++++++-
> drivers/net/zxdh/zxdh_msg.h | 175 +++-
> drivers/net/zxdh/zxdh_mtr.c | 1223 ++++++++++++++++++++++++++++
> drivers/net/zxdh/zxdh_mtr.h | 114 +++
> drivers/net/zxdh/zxdh_np.c | 811 ++++++++++++++++--
> drivers/net/zxdh/zxdh_np.h | 262 ++++++
> drivers/net/zxdh/zxdh_pci.c | 10 -
> drivers/net/zxdh/zxdh_queue.c | 132 +--
> drivers/net/zxdh/zxdh_queue.h | 118 +--
> drivers/net/zxdh/zxdh_rxtx.c | 695 +++++++++-------
> drivers/net/zxdh/zxdh_rxtx.h | 27 +
> drivers/net/zxdh/zxdh_tables.c | 378 +++++++--
> drivers/net/zxdh/zxdh_tables.h | 220 +++--
> 22 files changed, 6040 insertions(+), 938 deletions(-)
> create mode 100644 drivers/net/zxdh/zxdh_mtr.c
> create mode 100644 drivers/net/zxdh/zxdh_mtr.h
Still several checklist items, some of these are likely pre-existing conditions.
The one that matters are the build issue with RTE_ASSERT enabled.
Mark items with:
✔ passed
✘ Failed
o N/A
Basic hygiene
✔ Look at CI results in patchwork
✔ Merge cleanly with git am
✔ Run checkpatches
- bug in checkpatch around __rte_packed_begin macros
- warning from stub
✔ Run check-git-log
✔ Run check-symbol-maps.sh
✔ Run check-doc-vs-code
✔ Run check-spdk-tag
Builds
✔ Normal Gcc build; make sure driver is built!
✔ Use latest experimental Gcc 15 to catch new warnings
✔ Clang build using current version (clang-19)
✔ Doc build
o Build for 32 bit x86
o Cross build for Windows (if applicable)
✔ Debug build
x Enable asserts
[1594/3244] Compiling C object
drivers/libtmp_rte_net_zxdh.a.p/net_zxdh_zxdh_msg.c.o
FAILED: drivers/libtmp_rte_net_zxdh.a.p/net_zxdh_zxdh_msg.c.o
cc -Idrivers/libtmp_rte_net_zxdh.a.p -Idrivers -I../drivers -Idrivers/net/zxdh
-I../drivers/net/zxdh -Ilib/ethdev -I../lib/ethdev -I. -I.. -Iconfig
-I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include
-I../lib/eal/linux/include -Ilib/eal/x86/include -I../lib/eal/x86/include
-I../kernel/linux -Ilib/eal/common -I../lib/eal/common -Ilib/eal -I../lib/eal
-Ilib/kvargs -I../lib/kvargs -Ilib/log -I../lib/log -Ilib/metrics
-I../lib/metrics -Ilib/telemetry -I../lib/telemetry -Ilib/net -I../lib/net
-Ilib/mbuf -I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/ring
-I../lib/ring -Ilib/meter -I../lib/meter -Idrivers/bus/pci -I../drivers/bus/pci
-I../drivers/bus/pci/linux -Ilib/pci -I../lib/pci -Idrivers/bus/vdev
-I../drivers/bus/vdev -I/usr/include/x86_64-linux-gnu
-fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra
-std=c11 -O3 -include rte_config.h -Wvla -Wcast-qual -Wdeprecated -Wformat
-Wformat-nonliteral -Wformat-security -Wmissing-declarations
-Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith
-Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings
-Wno-packed-not-aligned -Wno-missing-field-initializers -D_GNU_SOURCE
-DRTE_ENABLE_ASSERT -DRTE_LIBRTE_ETHDEV_DEBUG=1 -fPIC -march=native -mrtm
-DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation
-Wno-address-of-packed-member -Wno-vla -DRTE_LOG_DEFAULT_LOGTYPE=pmd.net.zxdh
-MD -MQ drivers/libtmp_rte_net_zxdh.a.p/net_zxdh_zxdh_msg.c.o -MF
drivers/libtmp_rte_net_zxdh.a.p/net_zxdh_zxdh_msg.c.o.d -o
drivers/libtmp_rte_net_zxdh.a.p/net_zxdh_zxdh_msg.c.o -c
../drivers/net/zxdh/zxdh_msg.c
In file included from ../lib/eal/x86/include/rte_spinlock.h:11,
from ../drivers/net/zxdh/zxdh_msg.c:9:
../drivers/net/zxdh/zxdh_msg.c: In function ‘zxdh_vf_promisc_set’:
../drivers/net/zxdh/zxdh_msg.c:1450:41: error: ‘res_info’ undeclared (first use
in this function)
1450 | RTE_ASSERT(!cfg_data || !hw || !res_info || !res_len);
| ^~~~~~~~
../lib/eal/include/rte_branch_prediction.h:43:45: note: in definition of macro
‘unlikely’
43 | #define unlikely(x) __builtin_expect(!!(x), 0)
| ^
../lib/eal/include/rte_debug.h:47:25: note: in expansion of macro ‘RTE_VERIFY’
47 | #define RTE_ASSERT(exp) RTE_VERIFY(exp)
| ^~~~~~~~~~
../drivers/net/zxdh/zxdh_msg.c:1450:9: note: in expansion of macro ‘RTE_ASSERT’
1450 | RTE_ASSERT(!cfg_data || !hw || !res_info || !res_len);
| ^~~~~~~~~~
../drivers/net/zxdh/zxdh_msg.c:1450:41: note: each undeclared identifier is
reported only once for each function it appears in
1450 | RTE_ASSERT(!cfg_data || !hw || !res_info || !res_len);
| ^~~~~~~~
../lib/eal/include/rte_branch_prediction.h:43:45: note: in definition of macro
‘unlikely’
43 | #define unlikely(x) __builtin_expect(!!(x), 0)
| ^
../lib/eal/include/rte_debug.h:47:25: note: in expansion of macro ‘RTE_VERIFY’
47 | #define RTE_ASSERT(exp) RTE_VERIFY(exp)
| ^~~~~~~~~~
../drivers/net/zxdh/zxdh_msg.c:1450:9: note: in expansion of macro ‘RTE_ASSERT’
1450 | RTE_ASSERT(!cfg_data || !hw || !res_info || !res_len);
| ^~~~~~~~~~
[1603/3244] Compiling C object
drivers/libtmp_rte_net_virtio.a.p/net_virtio_virtio_rxtx.c.o
ninja: build stopped: subcommand failed.
✔ Test meson builds
Experimental builds:
✘ Enable address sanitizer for undefined checks
[1603/3244] Compiling C object
drivers/libtmp_rte_net_zxdh.a.p/net_zxdh_zxdh_np.c.o
In function ‘zxdh_np_dtb_smmu0_write_entry_data’,
inlined from ‘zxdh_np_dtb_se_smmu0_ind_write’ at
../drivers/net/zxdh/zxdh_np.c:1104:7,
inlined from ‘zxdh_np_dtb_eram_one_entry’ at
../drivers/net/zxdh/zxdh_np.c:1181:8,
inlined from ‘zxdh_np_dtb_table_entry_write’ at
../drivers/net/zxdh/zxdh_np.c:1452:9:
../drivers/net/zxdh/zxdh_np.c:75:12: warning: ‘rc’ may be used uninitialized
[-Wmaybe-uninitialized]
75 | if ((rc) != 0) {\
| ^
../drivers/net/zxdh/zxdh_np.c:1051:17: note: in expansion of macro
‘ZXDH_COMM_CHECK_RC_NO_ASSERT’
1051 | ZXDH_COMM_CHECK_RC_NO_ASSERT(rc,
"dpp_dtb_write_table_cmd");
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/zxdh/zxdh_np.c: In function ‘zxdh_np_dtb_table_entry_write’:
../drivers/net/zxdh/zxdh_np.c:983:26: note: ‘rc’ was declared here
983 | uint32_t rc;
| ^~
✘ Enable extra warnings (edit meson.build) for
-Wvla, -Wformat-truncation, -Waddress-of-packed-member
[1600/3244] Compiling C object
drivers/libtmp_rte_net_zxdh.a.p/net_zxdh_zxdh_queue.c.o
../drivers/net/zxdh/zxdh_queue.c: In function ‘zxdh_dev_rx_queue_setup_finish’:
../drivers/net/zxdh/zxdh_queue.c:357:24: warning: ISO C90 forbids variable
length array ‘new_pkts’ [-Wvla]
357 | struct rte_mbuf *new_pkts[free_cnt];
| ^~~~~~~~
[1603/3244] Compiling C object
drivers/libtmp_rte_net_zxdh.a.p/net_zxdh_zxdh_rxtx.c.o
../drivers/net/zxdh/zxdh_rxtx.c: In function ‘zxdh_recv_pkts_packed’:
../drivers/net/zxdh/zxdh_rxtx.c:903:24: warning: ISO C90 forbids variable
length array ‘new_pkts’ [-Wvla]
903 | struct rte_mbuf *new_pkts[free_cnt];
| ^~~~~~~~
[1616/3244] Compiling C object
drivers/libtmp_rte_net_zxdh.a.p/net_zxdh_zxdh_ethdev_ops.c.o
../drivers/net/zxdh/zxdh_ethdev_ops.c: In function ‘zxdh_dev_xstats_get_names’:
../drivers/net/zxdh/zxdh_ethdev_ops.c:1983:26: warning: ‘%s’ directive output
may be truncated writing up to 1359 bytes into a region of size 64
[-Wformat-truncation=]
1983 | "%s", zxdh_np_stat_strings[i].name);
| ^~
../drivers/net/zxdh/zxdh_ethdev_ops.c:1982:25: note: ‘snprintf’ output between
1 and 1360 bytes into a destination of size 64
1982 | snprintf(xstats_names[count].name,
sizeof(xstats_names[count].name),
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1983 | "%s", zxdh_np_stat_strings[i].name);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/zxdh/zxdh_ethdev_ops.c:1989:34: warning: ‘%s’ directive output
may be truncated writing up to 2991 bytes into a region of size 64
[-Wformat-truncation=]
1989 | "%s", zxdh_mac_stat_strings[i].name);
| ^~
../drivers/net/zxdh/zxdh_ethdev_ops.c:1988:33: note: ‘snprintf’ output between
1 and 2992 bytes into a destination of size 64
1988 | snprintf(xstats_names[count].name,
sizeof(xstats_names[count].name),
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1989 | "%s", zxdh_mac_stat_strings[i].name);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/zxdh/zxdh_ethdev_ops.c:1994:34: warning: ‘%s’ directive output
may be truncated writing up to 271 bytes into a region of size 64
[-Wformat-truncation=]
1994 | "%s", zxdh_mac_bytes_strings[i].name);
| ^~
../drivers/net/zxdh/zxdh_ethdev_ops.c:1993:33: note: ‘snprintf’ output between
1 and 272 bytes into a destination of size 64
1993 | snprintf(xstats_names[count].name,
sizeof(xstats_names[count].name),
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1994 | "%s", zxdh_mac_bytes_strings[i].name);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/zxdh/zxdh_ethdev_ops.c:2000:26: warning: ‘%s’ directive output
may be truncated writing up to 339 bytes into a region of size 64
[-Wformat-truncation=]
2000 | "%s", zxdh_vqm_stat_strings[i].name);
| ^~
../drivers/net/zxdh/zxdh_ethdev_ops.c:1999:25: note: ‘snprintf’ output between
1 and 340 bytes into a destination of size 64
1999 | snprintf(xstats_names[count].name,
sizeof(xstats_names[count].name),
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2000 | "%s", zxdh_vqm_stat_strings[i].name);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/zxdh/zxdh_ethdev_ops.c:2010:41: warning: ‘%s’ directive output
may be truncated writing up to 1359 bytes into a region of size between 54 and
58 [-Wformat-truncation=]
2010 | "rx_q%u_%s", i,
zxdh_rxq_stat_strings[t].name);
| ^~
../drivers/net/zxdh/zxdh_ethdev_ops.c:2009:33: note: ‘snprintf’ output between
7 and 1370 bytes into a destination of size 64
2009 | snprintf(xstats_names[count].name,
sizeof(xstats_names[count].name),
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2010 | "rx_q%u_%s", i,
zxdh_rxq_stat_strings[t].name);
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/net/zxdh/zxdh_ethdev_ops.c:2022:41: warning: ‘%s’ directive output
may be truncated writing up to 1291 bytes into a region of size between 54 and
58 [-Wformat-truncation=]
2022 | "tx_q%u_%s", i,
zxdh_txq_stat_strings[t].name);
| ^~
../drivers/net/zxdh/zxdh_ethdev_ops.c:2021:33: note: ‘snprintf’ output between
7 and 1302 bytes into a destination of size 64
2021 | snprintf(xstats_names[count].name,
sizeof(xstats_names[count].name),
|
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2022 | "tx_q%u_%s", i,
zxdh_txq_stat_strings[t].name);
|
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Look for anti-patterns:
✔ Do not put brackets on each switch case.
✔ Driver must not disable warnings with compiler flags or pragma's
✔ Driver must not use thread and signal
✔ Driver should not call abort() or assert() directly
✔ Review exposed symbol names
✔ Apply coccinelle scripts; look that for example null free checks
- Apply memcpy script for coccinelle
Lots of fixed size memcpy's that can get fixed later (see other patch
series)
✔ Review use of malloc
Several places could use rte_calloc instead of rte_zmalloc (low priority)
✘ Review use of memset
Extra memset and cast here:
static int32_t
zxdh_fill_common_msg(struct zxdh_hw *hw, struct zxdh_pci_bar_msg *desc,
uint8_t type, uint8_t field,
void *buff, uint16_t buff_size)
{
uint64_t msg_len = sizeof(struct zxdh_common_msg) + buff_size;
desc->payload_addr = rte_zmalloc(NULL, msg_len, 0);
if (unlikely(desc->payload_addr == NULL)) {
PMD_DRV_LOG(ERR, "Failed to allocate msg_data");
return -ENOMEM;
}
memset(desc->payload_addr, 0, msg_len); // zmalloc memory is already
zero!
desc->payload_len = msg_len;
// payload_addr is void *, cast not needed
struct zxdh_common_msg *msg_data = (struct zxdh_common_msg
*)desc->payload_addr;
