Re: [dpdk-dev] [PATCH v2] windows: build warnings with clang
On Wed, 2020-12-09 at 09:30 +, Nick Connolly wrote: > Copying sta...@dpdk.org - given that 20.11 has now been released I > assume this needs to be applied there as well. This patch was ignored due to the backport scanning scripts check commit message from upstream git repo, not mail thread. Remember to add "Cc: sta...@dpdk.org" in message body, or a "Fixes" tag. Applied to 20.11.4 now. > > On 07/12/2020 20:34, Thomas Monjalon wrote: > > 03/12/2020 01:02, Dmitry Kozlyuk: > > > On Sun, 29 Nov 2020 16:00:24 +, Nick Connolly wrote: > > > > Microsoft CRT defines Windows-specific secure alternatives to > > > > standard library functions and triggers warnings when "insecure" > > > > functions are used [1]. However, calling code already has all > > > > necessary checks around those functions, so these warnings are not > > > > useful for DPDK. MinGW provides its own CRT without this issue. > > > > > > > > [1]: > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fcpp%2Fc-runtime-library%2Fsecurity-features-in-the-crt%3Fview%3Dmsvc-160&data=04%7C01%7Cxuemingl%40nvidia.com%7C93c0adf5be2246c07dcf08d89c2532f5%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637431030883808802%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=LJVdh0nUgUEXT9vukPmwPffPugpiDFhe7EWp2rU7DuQ%3D&reserved=0 > > > > > > > > Disable this by defining -D_CRT_SECURE_NO_WARNINGS. > > > > > > > > Signed-off-by: Nick Connolly > > > > --- > > > > v2: > > > > * Adjust commit wording as per Dmitry Kozlyuk > > > > > > > > Note: It is unclear exactly which commit this fixes. It is probably a > > > > consequence of building more libraries for Windows. > > > Acked-by: Dmitry Kozlyuk > > Applied, thanks > > > > >
[Bug 900] [19.11.11-rc1] net/nfp build failure with clang 13
https://bugs.dpdk.org/show_bug.cgi?id=900
Bug ID: 900
Summary: [19.11.11-rc1] net/nfp build failure with clang 13
Product: DPDK
Version: 20.11
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: normal
Priority: Normal
Component: ethdev
Assignee: dev@dpdk.org
Reporter: alia...@nvidia.com
Target Milestone: ---
"""
$ meson --werror --buildtype=debugoptimized build && ninja-build -C build
[..]
drivers/net/nfp/nfp_net.c:3037:24: error: variable 'totlen' set but not used
[-Werror,-Wunused-but-set-variable] size_t count, curlen, totlen = 0;
drivers/net/nfp/nfp_net.c:3139:24: error: variable 'totlen' set but not used
[-Werror,-Wunused-but-set-variable]
size_t count, curlen, totlen = 0;
"""
clang: 13.0.0 (Fedora 13.0.0-3.fc35)
This was recently fixed by 9465a5d5a065 ("net/nfp: remove unused message
length").
--
You are receiving this mail because:
You are the assignee for the bug.
[Bug 901] [19.11.11-rc1] net/i40e build failure with make and clang 13
https://bugs.dpdk.org/show_bug.cgi?id=901 Bug ID: 901 Summary: [19.11.11-rc1] net/i40e build failure with make and clang 13 Product: DPDK Version: 19.11 Hardware: All OS: All Status: UNCONFIRMED Severity: normal Priority: Normal Component: ethdev Assignee: dev@dpdk.org Reporter: alia...@nvidia.com Target Milestone: --- """ make config MAKE_PAUSE=n O=/tmp/build-x86_64-native-linuxapp-clang T=x86_64-native-linuxapp-clang make -j64 MAKE_PAUSE=n O=/tmp/build-x86_64-native-linuxapp-clang drivers/net/i40e/base/i40e_common.c:5558:6: error: variable 'pe_cntx_size' set but not used [-Werror,-Wunused-but-set-variable] u32 pe_cntx_size, pe_filt_size; ^ drivers/net/i40e/base/i40e_common.c:5558:20: error: variable 'pe_filt_size' set but not used [-Werror,-Wunused-but-set-variable] u32 pe_cntx_size, pe_filt_size; """ clang: 13.0.0 (Fedora 13.0.0-3.fc35) -- You are receiving this mail because: You are the assignee for the bug.
[Bug 902] [19.11.11-rc1] net/ice build failure with make and clang 13
https://bugs.dpdk.org/show_bug.cgi?id=902 Bug ID: 902 Summary: [19.11.11-rc1] net/ice build failure with make and clang 13 Product: DPDK Version: 19.11 Hardware: All OS: All Status: UNCONFIRMED Severity: normal Priority: Normal Component: ethdev Assignee: dev@dpdk.org Reporter: alia...@nvidia.com Target Milestone: --- """ $ make config MAKE_PAUSE=n O=/tmp/build-x86_64-native-linuxapp-clang T=x86_64-native-linuxapp-clang $ make -j64 MAKE_PAUSE=n O=/tmp/build-x86_64-native-linuxapp-clang [..] drivers/net/ice/base/ice_common.c:2409:18: error: variable 'hw' set but not used [-Werror,-Wunused-but-set-variable] struct ice_hw *hw; [..] """ clang: 13.0.0 (Fedora 13.0.0-3.fc35) -- You are receiving this mail because: You are the assignee for the bug.
[Bug 903] [19.11.11-rc1] net/ixgbe build failure with make and clang 13
https://bugs.dpdk.org/show_bug.cgi?id=903 Bug ID: 903 Summary: [19.11.11-rc1] net/ixgbe build failure with make and clang 13 Product: DPDK Version: 19.11 Hardware: All OS: All Status: UNCONFIRMED Severity: normal Priority: Normal Component: ethdev Assignee: dev@dpdk.org Reporter: alia...@nvidia.com Target Milestone: --- """ $ make config MAKE_PAUSE=n O=/tmp/build-x86_64-native-linuxapp-clang T=x86_64-native-linuxapp-clang $ make -j64 MAKE_PAUSE=n O=/tmp/build-x86_64-native-linuxapp-clang [..] drivers/net/ixgbe/base/ixgbe_82598.c:53:6: error: variable 'pcie_devctl2' set but not used [-Werror,-Wunused-but-set-variable] u16 pcie_devctl2; drivers/net/ixgbe/base/ixgbe_common.c:3645:6: error: variable 'pcie_offset' set but not used [-Werror,-Wunused-but-set-variable] u16 pcie_offset; [..] """ clang: 13.0.0 (Fedora 13.0.0-3.fc35) -- You are receiving this mail because: You are the assignee for the bug.
RE: [PATCH v3] net/ixgbe: add vector Rx parameter check
> -Original Message-
> From: Bin Zheng
> Sent: Friday, December 10, 2021 4:22 PM
> To: dev@dpdk.org
> Cc: Wang, Haiyue ; lian...@liangbit.com;
> sta...@dpdk.org; Rong, Leyi ; Bin Zheng
> ; jia@intel.com
> Subject: [PATCH v3] net/ixgbe: add vector Rx parameter check
>
> Under the circumstance that `rx_tail` wrap back to zero and the advance speed
> of `rx_tail` is greater than `rxrearm_start`, `rx_tail` will catch up with
> `rxrearm_start` and surpass it.
> This may cause some mbufs be reused by application.
>
> So we need to make some restrictions to ensure that `rx_tail` will not exceed
> `rxrearm_start`.
>
> e.g.
>
> RDH: 972 RDT: 991 rxrearm_nb: 991 rxrearm_start: 992 rx_tail: 959
> RDH: 1004 RDT: 1023 rxrearm_nb: 991 rxrearm_start: 0 rx_tail: 991
> RDH: 12 RDT: 31 rxrearm_nb: 991 rxrearm_start: 32 rx_tail: 1023
> RDH: 31 RDT: 63 rxrearm_nb: 960 rxrearm_start: 64 rx_tail: 0
> RDH: 95 RDT: 95 rxrearm_nb: 1016 rxrearm_start: 96 rx_tail: 88
> RDH: 95 RDT: 127 rxrearm_nb: 991 rxrearm_start: 128 rx_tail: 95 ...
> RDH: 908 RDT: 927 rxrearm_nb: 991 rxrearm_start: 928 rx_tail: 895
> RDH: 940 RDT: 959 rxrearm_nb: 991 rxrearm_start: 960 rx_tail: 927
> RDH: 980 RDT: 991 rxrearm_nb: 991 rxrearm_start: 992 rx_tail: 959
> RDH: 991 RDT: 991 rxrearm_nb: 1026 rxrearm_start: 992 rx_tail: 994
>
> when `rx_tail` catches up with `rxrearm_start`,
> 2(994 - 992) mbufs be reused by application !
>
> Bugzilla ID: 882
> Fixes: 5a3cca342417 ("net/ixgbe: fix vector Rx")
> Cc: jia@intel.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Bin Zheng
> ---
> drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> index 1eed949495..4654d0adec 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> @@ -364,6 +364,17 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq,
> struct rte_mbuf **rx_pkts,
> uint8_t vlan_flags;
> uint16_t udp_p_flag = 0; /* Rx Descriptor UDP header present */
>
> + /*
> + * Under the circumstance that `rx_tail` wrap back to zero
> + * and the advance speed of `rx_tail` is greater than `rxrearm_start`,
> + * `rx_tail` will catch up with `rxrearm_start` and surpass it.
> + * This may cause some mbufs be reused by application.
> + *
> + * So we need to make some restrictions to ensure that
> + * `rx_tail` will not exceed `rxrearm_start`.
> + */
> + nb_pkts = RTE_MIN(nb_pkts, RTE_IXGBE_RXQ_REARM_THRESH);
> +
> /* nb_pkts has to be floor-aligned to RTE_IXGBE_DESCS_PER_LOOP */
> nb_pkts = RTE_ALIGN_FLOOR(nb_pkts, RTE_IXGBE_DESCS_PER_LOOP);
>
> --
> 2.25.1
Acked-by: Leyi Rong
Re: 19.11.11 patches review and test
Hi Christian, We still see KNI build failure on SLES15 SP3 with dpdk-stable-19.11.11-rc1. Is there a plan to address that on 19.11.11 LTS? Regards, Kalesh On Thu, Dec 9, 2021 at 8:13 PM wrote: > Hi all, > > Here is a list of patches targeted for stable release 19.11.11. > > The planned date for the final release is 7th January 2021. > > Please help with testing and validation of your use cases and report > any issues/results with reply-all to this mail. For the final release > the fixes and reported validations will be added to the release notes. > > A release candidate tarball can be found at: > > https://dpdk.org/browse/dpdk-stable/tag/?id=v19.11.11-rc1 > > These patches are located at branch 19.11 of dpdk-stable repo: > https://dpdk.org/browse/dpdk-stable/ > > Thanks. > > Christian Ehrhardt > > --- > Ajit Khaparde (3): > net/bnxt: fix Tx queue startup state > net/bnxt: fix memzone free for Tx and Rx rings > net/bnxt: fix tunnel port accounting > > Alexander Bechikov (1): > mbuf: fix dump of dynamic fields and flags > > Alexander Kozyrev (2): > net/mlx5: fix GENEVE and VXLAN-GPE flow item matching > net/mlx5: fix GRE flow item matching > > Alvin Zhang (1): > net/i40e: fix Rx packet statistics > > Aman Singh (1): > kni: fix build for SLES15-SP3 > > Anatoly Burakov (2): > vfio: fix FreeBSD clear group stub > vfio: fix FreeBSD documentation > > Anoob Joseph (2): > test/crypto: skip plain text compare for null cipher > common/cpt: fix KASUMI input length > > Arek Kusztal (2): > crypto/qat: fix status in RSA decryption > crypto/qat: fix uncleared cookies after operation > > Ben Pfaff (1): > doc: fix numbers power of 2 in LPM6 guide > > Bing Zhao (2): > net/mlx5: fix flow tables double release > net/mlx5: fix RETA update without stopping device > > Bruce Richardson (3): > eal/freebsd: lock memory device to prevent conflicts > test/mem: fix memory autotests on FreeBSD > eal/freebsd: ignore in-memory option > > Chengchang Tang (2): > net/bonding: fix dedicated queue mode in vector burst > net/bonding: fix RSS key length > > Chengfeng Ye (1): > net/axgbe: fix unreleased lock in I2C transfer > > Chengwen Feng (1): > kni: check error code of allmulticast mode switch > > Cian Ferriter (1): > ring: fix Doxygen comment of internal function > > Ciara Loftus (1): > net/af_xdp: disable secondary process support > > Ciara Power (3): > crypto/openssl: fix CCM processing 0 length source > examples/fips_validation: remove unused allocation > test/crypto: fix unnecessary stats retrieval > > Conor Walsh (8): > net/hinic/base: remove some unused variables > bus/fslmc: remove unused device count > event/sw: remove unused inflight events count > net/liquidio: remove unused counter > net/qede/base: remove unused message size > net/vmxnet3: fix build with clang 13 > test/distributor: remove unused counter > examples/performance-thread: remove unused hits count > > Dapeng Yu (1): > net/ice: save rule on switch filter creation > > Dariusz Sosnowski (2): > net/mlx5: fix MPLS tunnel outer layer overwrite > doc: fix typo in coding style > > David Christensen (3): > eal/ppc: ignore GCC 10 stringop-overflow warnings > config/ppc: ignore GCC 11 psabi warnings > test/atomic: fix 128-bit atomic test with many cores > > David Hunt (1): > test/power: fix CPU frequency when turbo enabled > > David Marchand (10): > net/virtio: avoid unneeded link interrupt configuration > net/virtio-user: fix Rx interrupts with multi-queue > bus/vmbus: fix leak on device scan > test/latency: fix loop boundary > eal/x86: fix some CPU extended features definitions > mbuf: enforce no option for dynamic fields and flags > doc: fix default mempool option in guides > eal/linux: fix uevent message parsing > test/red: fix typo in test description > examples/ntb: fix build dependency > > Eli Britstein (4): > net: avoid cast-align warning in VLAN insert function > mbuf: avoid cast-align warning in data offset macro > eal/x86: avoid cast-align warning in memcpy functions > app/testpmd: fix packet burst spreading stats > > Eugenio Pérez (1): > vhost: clean IOTLB cache on vring stop > > Fan Zhang (1): > examples/fips_validation: fix device start > > Feifei Wang (2): > drivers/net: fix typo in vector Rx comment > drivers/net: fix vector Rx comments > > Ferruh Yigit (3): > net/softnic: fix useless address check > net/i40e: fix 32-bit build > ethdev: fix crash on owner delete > > Gagandeep Singh (3): > common/dpaax/caamflib: fix IV for short MAC-I in SNOW3G > common/dpaax: fix physical address conversion > drivers/crypto: fix IPsec TTL decrement option > > Gaoxian
[PATCH 1/3] vdpa/ifc: fix log info mismatch
fix log info mismatch.
Signed-off-by: Andy Pei
---
drivers/vdpa/ifc/base/ifcvf.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
index 721cb1d..d10c1fd 100644
--- a/drivers/vdpa/ifc/base/ifcvf.c
+++ b/drivers/vdpa/ifc/base/ifcvf.c
@@ -94,12 +94,14 @@
return -1;
}
- DEBUGOUT("capability mapping:\ncommon cfg: %p\n"
- "notify base: %p\nisr cfg: %p\ndevice cfg: %p\n"
- "multiplier: %u\n",
- hw->common_cfg, hw->dev_cfg,
- hw->isr, hw->notify_base,
- hw->notify_off_multiplier);
+ DEBUGOUT("capability mapping:\n"
+"common cfg: %p\n"
+"notify base: %p\n"
+"isr cfg: %p\n"
+"device cfg: %p\n"
+"multiplier: %u\n",
+hw->common_cfg, hw->notify_base, hw->isr, hw->dev_cfg,
+hw->notify_off_multiplier);
return 0;
}
--
1.8.3.1
[PATCH 2/3] vdpa/ifc: check lm_cfg is not NULL before use lm_cfg
check lm_cfg is not NULL before use lm_cfg.
when init hardware, if lm_cfg is NULL, output some debug information.
Signed-off-by: Andy Pei
---
drivers/vdpa/ifc/base/ifcvf.c | 32
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
index d10c1fd..b9b061f 100644
--- a/drivers/vdpa/ifc/base/ifcvf.c
+++ b/drivers/vdpa/ifc/base/ifcvf.c
@@ -87,6 +87,8 @@
}
hw->lm_cfg = hw->mem_resource[4].addr;
+ if (!hw->lm_cfg)
+ DEBUGOUT("HW mem_resource[4] is NULL, so lm_cfg is NULL.\n");
if (hw->common_cfg == NULL || hw->notify_base == NULL ||
hw->isr == NULL || hw->dev_cfg == NULL) {
@@ -218,10 +220,13 @@
&cfg->queue_used_hi);
IFCVF_WRITE_REG16(hw->vring[i].size, &cfg->queue_size);
- *(u32 *)(lm_cfg + IFCVF_LM_RING_STATE_OFFSET +
- (i / 2) * IFCVF_LM_CFG_SIZE + (i % 2) * 4) =
- (u32)hw->vring[i].last_avail_idx |
- ((u32)hw->vring[i].last_used_idx << 16);
+ if (lm_cfg != NULL) {
+ *(u32 *)(lm_cfg + IFCVF_LM_RING_STATE_OFFSET +
+ (i / 2) * IFCVF_LM_CFG_SIZE +
+ (i % 2) * 4) =
+ (u32)hw->vring[i].last_avail_idx |
+ ((u32)hw->vring[i].last_used_idx << 16);
+ }
IFCVF_WRITE_REG16(i + 1, &cfg->queue_msix_vector);
if (IFCVF_READ_REG16(&cfg->queue_msix_vector) ==
@@ -254,10 +259,14 @@
IFCVF_WRITE_REG16(i, &cfg->queue_select);
IFCVF_WRITE_REG16(0, &cfg->queue_enable);
IFCVF_WRITE_REG16(IFCVF_MSI_NO_VECTOR, &cfg->queue_msix_vector);
- ring_state = *(u32 *)(hw->lm_cfg + IFCVF_LM_RING_STATE_OFFSET +
- (i / 2) * IFCVF_LM_CFG_SIZE + (i % 2) * 4);
- hw->vring[i].last_avail_idx = (u16)(ring_state >> 16);
- hw->vring[i].last_used_idx = (u16)(ring_state >> 16);
+ if (hw->lm_cfg != NULL) {
+ ring_state = *(u32 *)(hw->lm_cfg +
+ IFCVF_LM_RING_STATE_OFFSET +
+ (i / 2) * IFCVF_LM_CFG_SIZE +
+ (i % 2) * 4);
+ hw->vring[i].last_avail_idx = (u16)(ring_state >> 16);
+ hw->vring[i].last_used_idx = (u16)(ring_state >> 16);
+ }
}
}
@@ -292,6 +301,9 @@
lm_cfg = hw->lm_cfg;
+ if (lm_cfg == NULL)
+ return;
+
*(u32 *)(lm_cfg + IFCVF_LM_BASE_ADDR_LOW) =
log_base & IFCVF_32_BIT_MASK;
@@ -313,6 +325,10 @@
u8 *lm_cfg;
lm_cfg = hw->lm_cfg;
+
+ if (lm_cfg == NULL)
+ return;
+
*(u32 *)(lm_cfg + IFCVF_LM_LOGGING_CTRL) = IFCVF_LM_DISABLE;
}
--
1.8.3.1
[PATCH 3/3] vhost: add some log for vhost message VHOST_USER_SET_VRING_BASE
Usually the last avail index and last used index is 0, but for target device of live migration, the last avail index and last used index is not 0. So I think some log is helpful. Signed-off-by: Andy Pei --- lib/vhost/vhost_user.c | 5 + 1 file changed, 5 insertions(+) diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index a781346..3cb13fb 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -973,6 +973,11 @@ vq->last_avail_idx = msg->payload.state.num; } + VHOST_LOG_CONFIG(INFO, + "vring base idx:%d last_used_idx:%u last_avail_idx:%u.\n", + msg->payload.state.index, vq->last_used_idx, + vq->last_avail_idx); + return RTE_VHOST_MSG_RESULT_OK; } -- 1.8.3.1
RE: [EXT] Re: [dpdk-stable] [PATCH v2] test: avoid hang if queues are full and Tx fails
> -Original Message-
> From: Thomas Monjalon
> Sent: Monday, November 29, 2021 2:44 PM
> To: Rakesh Kudurumalla ; Jerin Jacob
> Kollanukkaran
> Cc: sta...@dpdk.org; dev@dpdk.org; david.march...@redhat.com;
> ferruh.yi...@intel.com; andrew.rybche...@oktetlabs.ru;
> ajit.khapa...@broadcom.com
> Subject: Re: [EXT] Re: [dpdk-stable] [PATCH v2] test: avoid hang if queues are
> full and Tx fails
>
> 29/11/2021 09:52, Rakesh Kudurumalla:
> > From: Thomas Monjalon
> > > 22/11/2021 08:59, Rakesh Kudurumalla:
> > > > From: Thomas Monjalon
> > > > > 20/07/2021 18:50, Rakesh Kudurumalla:
> > > > > > Current pmd_perf_autotest() in continuous mode tries to
> > > > > > enqueue MAX_TRAFFIC_BURST completely before starting the test.
> > > > > > Some drivers cannot accept complete MAX_TRAFFIC_BURST even
> > > > > > though
> > > rx+tx
> > > > > > desc
> > > > > count
> > > > > > can fit it.
> > > > >
> > > > > Which driver is failing to do so?
> > > > > Why it cannot enqueue 32 packets?
> > > >
> > > > Octeontx2 driver is failing to enqueue because hardware buffers
> > > > are full
> > > before test.
>
> Aren't you stopping the support of octeontx2?
> Why do you care now?
> yes we are not supporting octeontx2,but this issue is observed in cnxk
> driver ,current patch fixes the same
> > >
> > > Why hardware buffers are full?
> > Hardware buffers are full because number of number of descriptors in
> > continuous mode Is less than MAX_TRAFFIC_BURST, so if enque fails ,
> > there is no way hardware can drop the Packets . pmd_per_autotest
> > application evaluates performance after enqueueing packets Initially.
> > >
> > > > pmd_perf_autotest() in continuous mode tries to enqueue
> > > > MAX_TRAFFIC_BURST (2048) before starting the test.
> > > >
> > > > > > This patch changes behaviour to stop enqueuing after few retries.
> > > > >
> > > > > If there is a real limitation, there will be issues in more
> > > > > places than this test program.
> > > > > I feel it should be addressed either in the driver or at ethdev level.
> > > > >
> > > > > [...]
> > > > > > @@ -480,10 +483,19 @@ main_loop(__rte_unused void *args)
> > > > > > nb_tx = RTE_MIN(MAX_PKT_BURST, num);
> > > > > > nb_tx = rte_eth_tx_burst(portid, 0,
> > > > > > &tx_burst[idx],
> nb_tx);
> > > > > > + if (nb_tx == 0)
> > > > > > + retry_cnt++;
> > > > > > num -= nb_tx;
> > > > > > idx += nb_tx;
> > > > > > + if (retry_cnt == MAX_RETRY_COUNT) {
> > > > > > + retry_cnt = 0;
> > > > > > + break;
> > > > > > + }
>
>
RE: [PATCH 1/3] vdpa/ifc: fix log info mismatch
Hi Andy,
Thanks for the patch.
You need to add the "Fixes: " line.
BRs,
Xiao
> -Original Message-
> From: Pei, Andy
> Sent: Monday, December 13, 2021 12:29 PM
> To: dev@dpdk.org
> Cc: Pei, Andy ; Xia, Chenbo ;
> Wang, Xiao W
> Subject: [PATCH 1/3] vdpa/ifc: fix log info mismatch
>
> fix log info mismatch.
>
> Signed-off-by: Andy Pei
> ---
> drivers/vdpa/ifc/base/ifcvf.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
> index 721cb1d..d10c1fd 100644
> --- a/drivers/vdpa/ifc/base/ifcvf.c
> +++ b/drivers/vdpa/ifc/base/ifcvf.c
> @@ -94,12 +94,14 @@
> return -1;
> }
>
> - DEBUGOUT("capability mapping:\ncommon cfg: %p\n"
> - "notify base: %p\nisr cfg: %p\ndevice cfg: %p\n"
> - "multiplier: %u\n",
> - hw->common_cfg, hw->dev_cfg,
> - hw->isr, hw->notify_base,
> - hw->notify_off_multiplier);
> + DEBUGOUT("capability mapping:\n"
> + "common cfg: %p\n"
> + "notify base: %p\n"
> + "isr cfg: %p\n"
> + "device cfg: %p\n"
> + "multiplier: %u\n",
> + hw->common_cfg, hw->notify_base, hw->isr, hw->dev_cfg,
> + hw->notify_off_multiplier);
>
> return 0;
> }
> --
> 1.8.3.1
RE: [PATCH 2/3] vdpa/ifc: check lm_cfg is not NULL before use lm_cfg
Hi,
Comments inline.
BRs,
Xiao
> -Original Message-
> From: Pei, Andy
> Sent: Monday, December 13, 2021 12:29 PM
> To: dev@dpdk.org
> Cc: Pei, Andy ; Xia, Chenbo ;
> Wang, Xiao W
> Subject: [PATCH 2/3] vdpa/ifc: check lm_cfg is not NULL before use lm_cfg
>
> check lm_cfg is not NULL before use lm_cfg.
> when init hardware, if lm_cfg is NULL, output some debug information.
1. We need to capitalize the first letter in a sentence.
2. If lm_cfg is null, then I assume device doesn't support HW LM feature. But
in below code change, many places just return silently, is it an issue?
>
> Signed-off-by: Andy Pei
> ---
> drivers/vdpa/ifc/base/ifcvf.c | 32
> 1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
> index d10c1fd..b9b061f 100644
> --- a/drivers/vdpa/ifc/base/ifcvf.c
> +++ b/drivers/vdpa/ifc/base/ifcvf.c
> @@ -87,6 +87,8 @@
> }
>
> hw->lm_cfg = hw->mem_resource[4].addr;
> + if (!hw->lm_cfg)
> + DEBUGOUT("HW mem_resource[4] is NULL, so lm_cfg is
> NULL.\n");
We need to make the debug message more human readable.
>
> if (hw->common_cfg == NULL || hw->notify_base == NULL ||
> hw->isr == NULL || hw->dev_cfg == NULL) {
> @@ -218,10 +220,13 @@
> &cfg->queue_used_hi);
> IFCVF_WRITE_REG16(hw->vring[i].size, &cfg->queue_size);
>
> - *(u32 *)(lm_cfg + IFCVF_LM_RING_STATE_OFFSET +
> - (i / 2) * IFCVF_LM_CFG_SIZE + (i % 2) * 4) =
> - (u32)hw->vring[i].last_avail_idx |
> - ((u32)hw->vring[i].last_used_idx << 16);
> + if (lm_cfg != NULL) {
> + *(u32 *)(lm_cfg + IFCVF_LM_RING_STATE_OFFSET +
> + (i / 2) * IFCVF_LM_CFG_SIZE +
> + (i % 2) * 4) =
> + (u32)hw->vring[i].last_avail_idx |
> + ((u32)hw->vring[i].last_used_idx << 16);
> + }
>
> IFCVF_WRITE_REG16(i + 1, &cfg->queue_msix_vector);
> if (IFCVF_READ_REG16(&cfg->queue_msix_vector) ==
> @@ -254,10 +259,14 @@
> IFCVF_WRITE_REG16(i, &cfg->queue_select);
> IFCVF_WRITE_REG16(0, &cfg->queue_enable);
> IFCVF_WRITE_REG16(IFCVF_MSI_NO_VECTOR, &cfg-
> >queue_msix_vector);
> - ring_state = *(u32 *)(hw->lm_cfg +
> IFCVF_LM_RING_STATE_OFFSET +
> - (i / 2) * IFCVF_LM_CFG_SIZE + (i % 2) * 4);
> - hw->vring[i].last_avail_idx = (u16)(ring_state >> 16);
> - hw->vring[i].last_used_idx = (u16)(ring_state >> 16);
> + if (hw->lm_cfg != NULL) {
> + ring_state = *(u32 *)(hw->lm_cfg +
> + IFCVF_LM_RING_STATE_OFFSET +
> + (i / 2) * IFCVF_LM_CFG_SIZE +
> + (i % 2) * 4);
> + hw->vring[i].last_avail_idx = (u16)(ring_state >> 16);
> + hw->vring[i].last_used_idx = (u16)(ring_state >> 16);
> + }
> }
> }
>
> @@ -292,6 +301,9 @@
>
> lm_cfg = hw->lm_cfg;
>
> + if (lm_cfg == NULL)
> + return;
> +
> *(u32 *)(lm_cfg + IFCVF_LM_BASE_ADDR_LOW) =
> log_base & IFCVF_32_BIT_MASK;
>
> @@ -313,6 +325,10 @@
> u8 *lm_cfg;
>
> lm_cfg = hw->lm_cfg;
> +
> + if (lm_cfg == NULL)
> + return;
> +
> *(u32 *)(lm_cfg + IFCVF_LM_LOGGING_CTRL) = IFCVF_LM_DISABLE;
> }
>
> --
> 1.8.3.1
Re: [PATCH] build/eal: add OS defines for C conditional checks
On Fri, Dec 10, 2021 at 8:23 PM Bruce Richardson
wrote:
>
> Define a set of macros in the build configuration to allow C runtime
> code to check the current OS environment. This saves the user having to
> use ifdefs for e.g. disabling particular tests on Windows. See included
> documentation changes for usage examples.
>
> Signed-off-by: Bruce Richardson
Acked-by: Jerin Jacob
> ---
> doc/guides/contributing/coding_style.rst | 42 ++--
> lib/eal/meson.build | 7
> 2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/doc/guides/contributing/coding_style.rst
> b/doc/guides/contributing/coding_style.rst
> index 0ec37c019b..e52ecb2b60 100644
> --- a/doc/guides/contributing/coding_style.rst
> +++ b/doc/guides/contributing/coding_style.rst
> @@ -136,6 +136,12 @@ For example:
> Conditional Compilation
> ~~~
>
> +.. note::
> +
> + Conditional compilation should be used only when absolutely necessary, as
> it increases the number of target binaries that need to be built and tested.
> + See below for details of some utility macros/defines available to allow
> ifdefs/macros to be replaced by C conditional in some cases.
> +
> +
> * When code is conditionally compiled using ``#ifdef`` or ``#if``, a comment
> may be added following the matching
>``#endif`` or ``#else`` to permit the reader to easily discern where
> conditionally compiled code regions end.
> * This comment should be used only for (subjectively) long regions, regions
> greater than 20 lines, or where a series of nested ``#ifdef``'s may be
> confusing to the reader.
> @@ -165,9 +171,41 @@ Conditional Compilation
> /* Or here. */
> #endif /* !COMPAT_43 */
>
> -.. note::
> +Defines to Avoid Conditional Compilation
> +
> +
> +In many cases in DPDK, one wants to optionally compile code based on the
> target platform,
> +or runtime environment.
> +While this can be done using the conditional compilation directives,
> +e.g. ``#ifdef RTE_EXEC_ENV_LINUX``, present in DPDK for many releases,
> +this can also be done in many cases using regular ``if`` statements and the
> following defines:
> +
> +* ``RTE_ENV_FREEBSD``, ``RTE_ENV_LINUX``, ``RTE_ENV_WINDOWS`` - these define
> ids for each operating system environment.
> +* ``RTE_EXEC_ENV`` - this defines the id of the current environment, i.e.
> one of the items in list above.
> +* ``RTE_EXEC_ENV_IS_FREEBSD``, ``RTE_EXEC_ENV_IS_LINUX``,
> ``RTE_EXEC_ENV_IS_WINDOWS`` - 0/1 values indicating if the current
> environment is that specified,
> + shortcuts for checking e.g. ``RTE_EXEC_ENV == RTE_ENV_WINDOWS``
> +
> +Examples of use:
> +
> +.. code-block:: c
> +
> + /* report a unit tests as unsupported on Windows */
> + if (RTE_EXEC_ENV_IS_WINDOWS)
> + return TEST_SKIPPED;
> +
> + /* set different default values depending on OS Environment */
> + switch (RTE_EXEC_ENV) {
> + case RTE_ENV_FREEBSD:
> + default = x;
> + break;
> + case RTE_ENV_LINUX:
> + default = y;
> + break;
> + case RTE_ENV_WINDOWS:
> + default = z;
> + break;
> + }
>
> - Conditional compilation should be used only when absolutely necessary, as
> it increases the number of target binaries that need to be built and tested.
>
> C Types
> ---
> diff --git a/lib/eal/meson.build b/lib/eal/meson.build
> index 1722924f67..056beb9461 100644
> --- a/lib/eal/meson.build
> +++ b/lib/eal/meson.build
> @@ -10,6 +10,13 @@ if not is_windows
> subdir('unix')
> endif
>
> +exec_envs = {'freebsd': 0, 'linux': 1, 'windows': 2}
> +foreach env, id:exec_envs
> +dpdk_conf.set('RTE_ENV_' + env.to_upper(), id)
> +dpdk_conf.set10('RTE_EXEC_ENV_IS_' + env.to_upper(), (exec_env == env))
> +endforeach
> +dpdk_conf.set('RTE_EXEC_ENV', exec_envs[exec_env])
> +
> dpdk_conf.set('RTE_EXEC_ENV_' + exec_env.to_upper(), 1)
> subdir(exec_env)
>
> --
> 2.32.0
>
[PATCH v2] vdpa/ifc: fix log info mismatch
fix log info mismatch.
Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
Cc: xiao.w.w...@intel.com
Signed-off-by: Andy Pei
---
drivers/vdpa/ifc/base/ifcvf.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
index 721cb1d..d10c1fd 100644
--- a/drivers/vdpa/ifc/base/ifcvf.c
+++ b/drivers/vdpa/ifc/base/ifcvf.c
@@ -94,12 +94,14 @@
return -1;
}
- DEBUGOUT("capability mapping:\ncommon cfg: %p\n"
- "notify base: %p\nisr cfg: %p\ndevice cfg: %p\n"
- "multiplier: %u\n",
- hw->common_cfg, hw->dev_cfg,
- hw->isr, hw->notify_base,
- hw->notify_off_multiplier);
+ DEBUGOUT("capability mapping:\n"
+"common cfg: %p\n"
+"notify base: %p\n"
+"isr cfg: %p\n"
+"device cfg: %p\n"
+"multiplier: %u\n",
+hw->common_cfg, hw->notify_base, hw->isr, hw->dev_cfg,
+hw->notify_off_multiplier);
return 0;
}
--
1.8.3.1
RE: [PATCH 1/3] vdpa/ifc: fix log info mismatch
Hi Xiao,
"Fixes: " line will be added in V2.
Thanks for reply.
-Original Message-
From: Wang, Xiao W
Sent: Monday, December 13, 2021 2:55 PM
To: Pei, Andy ; dev@dpdk.org
Cc: Xia, Chenbo
Subject: RE: [PATCH 1/3] vdpa/ifc: fix log info mismatch
Hi Andy,
Thanks for the patch.
You need to add the "Fixes: " line.
BRs,
Xiao
> -Original Message-
> From: Pei, Andy
> Sent: Monday, December 13, 2021 12:29 PM
> To: dev@dpdk.org
> Cc: Pei, Andy ; Xia, Chenbo
> ; Wang, Xiao W
> Subject: [PATCH 1/3] vdpa/ifc: fix log info mismatch
>
> fix log info mismatch.
>
> Signed-off-by: Andy Pei
> ---
> drivers/vdpa/ifc/base/ifcvf.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/vdpa/ifc/base/ifcvf.c
> b/drivers/vdpa/ifc/base/ifcvf.c index 721cb1d..d10c1fd 100644
> --- a/drivers/vdpa/ifc/base/ifcvf.c
> +++ b/drivers/vdpa/ifc/base/ifcvf.c
> @@ -94,12 +94,14 @@
> return -1;
> }
>
> - DEBUGOUT("capability mapping:\ncommon cfg: %p\n"
> - "notify base: %p\nisr cfg: %p\ndevice cfg: %p\n"
> - "multiplier: %u\n",
> - hw->common_cfg, hw->dev_cfg,
> - hw->isr, hw->notify_base,
> - hw->notify_off_multiplier);
> + DEBUGOUT("capability mapping:\n"
> + "common cfg: %p\n"
> + "notify base: %p\n"
> + "isr cfg: %p\n"
> + "device cfg: %p\n"
> + "multiplier: %u\n",
> + hw->common_cfg, hw->notify_base, hw->isr, hw->dev_cfg,
> + hw->notify_off_multiplier);
>
> return 0;
> }
> --
> 1.8.3.1
RE: [PATCH v2] vdpa/ifc: fix log info mismatch
Hi Andy,
BRs,
Xiao
> -Original Message-
> From: Pei, Andy
> Sent: Monday, December 13, 2021 2:36 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo ; Wang, Xiao W
>
> Subject: [PATCH v2] vdpa/ifc: fix log info mismatch
>
> fix log info mismatch.
Use "Fix".
>
> Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
> Cc: xiao.w.w...@intel.com
For fix patch, you need to Cc "sta...@dpdk.org", not me.
>
> Signed-off-by: Andy Pei
> ---
> drivers/vdpa/ifc/base/ifcvf.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
> index 721cb1d..d10c1fd 100644
> --- a/drivers/vdpa/ifc/base/ifcvf.c
> +++ b/drivers/vdpa/ifc/base/ifcvf.c
> @@ -94,12 +94,14 @@
> return -1;
> }
>
> - DEBUGOUT("capability mapping:\ncommon cfg: %p\n"
> - "notify base: %p\nisr cfg: %p\ndevice cfg: %p\n"
> - "multiplier: %u\n",
> - hw->common_cfg, hw->dev_cfg,
> - hw->isr, hw->notify_base,
> - hw->notify_off_multiplier);
> + DEBUGOUT("capability mapping:\n"
> + "common cfg: %p\n"
> + "notify base: %p\n"
> + "isr cfg: %p\n"
> + "device cfg: %p\n"
> + "multiplier: %u\n",
> + hw->common_cfg, hw->notify_base, hw->isr, hw->dev_cfg,
> + hw->notify_off_multiplier);
>
> return 0;
> }
> --
> 1.8.3.1
RE: [PATCH v2] vdpa/ifc: fix log info mismatch
Hi Xiao,
Thanks for your reply.
I will send a V3 patch to address this.
-Original Message-
From: Wang, Xiao W
Sent: Monday, December 13, 2021 3:30 PM
To: Pei, Andy ; dev@dpdk.org
Cc: Xia, Chenbo
Subject: RE: [PATCH v2] vdpa/ifc: fix log info mismatch
Hi Andy,
BRs,
Xiao
> -Original Message-
> From: Pei, Andy
> Sent: Monday, December 13, 2021 2:36 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo ; Wang, Xiao W
>
> Subject: [PATCH v2] vdpa/ifc: fix log info mismatch
>
> fix log info mismatch.
Use "Fix".
>
> Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
> Cc: xiao.w.w...@intel.com
For fix patch, you need to Cc "sta...@dpdk.org", not me.
>
> Signed-off-by: Andy Pei
> ---
> drivers/vdpa/ifc/base/ifcvf.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/vdpa/ifc/base/ifcvf.c
> b/drivers/vdpa/ifc/base/ifcvf.c index 721cb1d..d10c1fd 100644
> --- a/drivers/vdpa/ifc/base/ifcvf.c
> +++ b/drivers/vdpa/ifc/base/ifcvf.c
> @@ -94,12 +94,14 @@
> return -1;
> }
>
> - DEBUGOUT("capability mapping:\ncommon cfg: %p\n"
> - "notify base: %p\nisr cfg: %p\ndevice cfg: %p\n"
> - "multiplier: %u\n",
> - hw->common_cfg, hw->dev_cfg,
> - hw->isr, hw->notify_base,
> - hw->notify_off_multiplier);
> + DEBUGOUT("capability mapping:\n"
> + "common cfg: %p\n"
> + "notify base: %p\n"
> + "isr cfg: %p\n"
> + "device cfg: %p\n"
> + "multiplier: %u\n",
> + hw->common_cfg, hw->notify_base, hw->isr, hw->dev_cfg,
> + hw->notify_off_multiplier);
>
> return 0;
> }
> --
> 1.8.3.1
[PATCH v3] vdpa/ifc: fix log info mismatch
Fix log info mismatch.
Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
Cc: sta...@dpdk.org
Signed-off-by: Andy Pei
---
drivers/vdpa/ifc/base/ifcvf.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/vdpa/ifc/base/ifcvf.c b/drivers/vdpa/ifc/base/ifcvf.c
index 721cb1d..d10c1fd 100644
--- a/drivers/vdpa/ifc/base/ifcvf.c
+++ b/drivers/vdpa/ifc/base/ifcvf.c
@@ -94,12 +94,14 @@
return -1;
}
- DEBUGOUT("capability mapping:\ncommon cfg: %p\n"
- "notify base: %p\nisr cfg: %p\ndevice cfg: %p\n"
- "multiplier: %u\n",
- hw->common_cfg, hw->dev_cfg,
- hw->isr, hw->notify_base,
- hw->notify_off_multiplier);
+ DEBUGOUT("capability mapping:\n"
+"common cfg: %p\n"
+"notify base: %p\n"
+"isr cfg: %p\n"
+"device cfg: %p\n"
+"multiplier: %u\n",
+hw->common_cfg, hw->notify_base, hw->isr, hw->dev_cfg,
+hw->notify_off_multiplier);
return 0;
}
--
1.8.3.1
