Re: [dpdk-dev] [PATCH v4 00/16] net/mlx5: support Sub-Function
> -Original Message- > From: Thomas Monjalon > Sent: Thursday, July 22, 2021 6:24 AM > To: Xueming(Steven) Li > Cc: Slava Ovsiienko ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v4 00/16] net/mlx5: support Sub-Function > > 21/07/2021 16:37, Xueming Li: > > Sub-Function [1] is a portion of the PCI device, a SF netdev has its > > own dedicated queues(txq, rxq). A SF shares PCI level resources with > > other SFs and/or with its parent PCI function. Auxiliary bus is the > > fundamental of SF. > > > > This patch set introduces Sub-Function support for mlx5 PMD driver > > including class net, regex, vdpa and compress. > > Applied, thanks. > > Fixup note: a transient per-patch compilation issue was fixed, and new common > symbols are made exported for Windows. > Thanks very much!
Re: [dpdk-dev] [PATCH 1/4] ethdev: fix max Rx packet length
在 2021/7/21 23:29, Ferruh Yigit 写道:
On 7/19/2021 4:35 AM, Huisong Li wrote:
Hi, Ferruh
Hi Huisong,
Thanks for the review.
在 2021/7/10 1:29, Ferruh Yigit 写道:
There is a confusion on setting max Rx packet length, this patch aims to
clarify it.
'rte_eth_dev_configure()' API accepts max Rx packet size via
'uint32_t max_rx_pkt_len' filed of the config struct 'struct
rte_eth_conf'.
Also 'rte_eth_dev_set_mtu()' API can be used to set the MTU, and result
stored into '(struct rte_eth_dev)->data->mtu'.
These two APIs are related but they work in a disconnected way, they
store the set values in different variables which makes hard to figure
out which one to use, also two different related method is confusing for
the users.
Other issues causing confusion is:
* maximum transmission unit (MTU) is payload of the Ethernet frame. And
'max_rx_pkt_len' is the size of the Ethernet frame. Difference is
Ethernet frame overhead, but this may be different from device to
device based on what device supports, like VLAN and QinQ.
* 'max_rx_pkt_len' is only valid when application requested jumbo frame,
which adds additional confusion and some APIs and PMDs already
discards this documented behavior.
* For the jumbo frame enabled case, 'max_rx_pkt_len' is an mandatory
field, this adds configuration complexity for application.
As solution, both APIs gets MTU as parameter, and both saves the result
in same variable '(struct rte_eth_dev)->data->mtu'. For this
'max_rx_pkt_len' updated as 'mtu', and it is always valid independent
from jumbo frame.
For 'rte_eth_dev_configure()', 'dev->data->dev_conf.rxmode.mtu' is user
request and it should be used only within configure function and result
should be stored to '(struct rte_eth_dev)->data->mtu'. After that point
both application and PMD uses MTU from this variable.
When application doesn't provide an MTU during 'rte_eth_dev_configure()'
default 'RTE_ETHER_MTU' value is used.
As additional clarification, MTU is used to configure the device for
physical Rx/Tx limitation. Other related issue is size of the buffer to
store Rx packets, many PMDs use mbuf data buffer size as Rx buffer size.
And compares MTU against Rx buffer size to decide enabling scattered Rx
or not, if PMD supports it. If scattered Rx is not supported by device,
MTU bigger than Rx buffer size should fail.
Signed-off-by: Ferruh Yigit
<...>
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index e51512560e15..8bccdeddb2f7 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2379,20 +2379,11 @@ hns3_refresh_mtu(struct rte_eth_dev *dev, struct
rte_eth_conf *conf)
{
struct hns3_adapter *hns = dev->data->dev_private;
struct hns3_hw *hw = &hns->hw;
- uint32_t max_rx_pkt_len;
- uint16_t mtu;
- int ret;
-
- if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME))
- return 0;
+ uint32_t max_rx_pktlen;
- /*
- * If jumbo frames are enabled, MTU needs to be refreshed
- * according to the maximum RX packet length.
- */
- max_rx_pkt_len = conf->rxmode.max_rx_pkt_len;
- if (max_rx_pkt_len > HNS3_MAX_FRAME_LEN ||
- max_rx_pkt_len <= HNS3_DEFAULT_FRAME_LEN) {
+ max_rx_pktlen = conf->rxmode.mtu + HNS3_ETH_OVERHEAD;
+ if (max_rx_pktlen > HNS3_MAX_FRAME_LEN ||
+ max_rx_pktlen <= HNS3_DEFAULT_FRAME_LEN) {
hns3_err(hw, "maximum Rx packet length must be greater than %u "
"and no more than %u when jumbo frame enabled.",
(uint16_t)HNS3_DEFAULT_FRAME_LEN,
The preceding check for the maximum frame length was based on the scenario where
jumbo frames are enabled.
Since there is no offload of jumbo frames in this patchset, the maximum frame
length does not need to be checked and only ensure conf->rxmode.mtu is valid.
These should be guaranteed by dev_configure() in the framework .
Got it, agree that 'HNS3_DEFAULT_FRAME_LEN' check is now wrong, and as you said
these checks are becoming redundant, so I will remove them.
In that case 'hns3_refresh_mtu()' becomes just wrapper to 'hns3_dev_mtu_set()',
I will remove function too.
<...>
ok
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c
b/drivers/net/hns3/hns3_ethdev_vf.c
index e582503f529b..ca839fa55fa0 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -784,8 +784,7 @@ hns3vf_dev_configure(struct rte_eth_dev *dev)
uint16_t nb_rx_q = dev->data->nb_rx_queues;
uint16_t nb_tx_q = dev->data->nb_tx_queues;
struct rte_eth_rss_conf rss_conf;
- uint32_t max_rx_pkt_len;
- uint16_t mtu;
+ uint32_t max_rx_pktlen;
bool gro_en;
int ret;
@@ -825,29 +824,21 @@ hns3vf_dev_configure(struct rte_eth_dev *dev)
goto cfg_err;
}
- /*
- * If jumbo frames are enabled, MTU needs to be refreshed
- * according to the maximum RX packet length.
- */
- if (conf->rxmode.offloads &
Re: [dpdk-dev] [PATCH] net/virtio: report maximum MTU in device info
On 7/21/21 11:22 AM, Andrew Rybchenko wrote:
> From: Ivan Ilchenko
>
> Fix the driver to report maximum MTU obtained from config if
> VIRTIO_NET_F_MTU is supported or calculated based on maximum
> Rx packet length.
>
> Fixes: ad97ceece12c ("ethdev: add min/max MTU to device info")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Ivan Ilchenko
> Signed-off-by: Andrew Rybchenko
> ---
> drivers/net/virtio/virtio_ethdev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> index 6d6e105960..af6305e9d8 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -2502,6 +2502,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct
> rte_eth_dev_info *dev_info)
> dev_info->min_rx_bufsize = VIRTIO_MIN_RX_BUFSIZE;
> dev_info->max_rx_pktlen = VIRTIO_MAX_RX_PKTLEN;
> dev_info->max_mac_addrs = VIRTIO_MAX_MAC_ADDRS;
> + dev_info->max_mtu = hw->max_mtu;
>
> host_features = VIRTIO_OPS(hw)->get_features(hw);
> dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
>
Reviewed-by: Maxime Coquelin
Thanks,
Maxime
Re: [dpdk-dev] [PATCH v3] net/virtio: fix Rx scatter offload
On 7/21/21 11:29 AM, Andrew Rybchenko wrote:
> On 7/20/21 7:19 PM, Maxime Coquelin wrote:
>>
>>
>> On 7/20/21 9:54 AM, Andrew Rybchenko wrote:
>>> From: Ivan Ilchenko
>>>
>>> Report Rx scatter offload capability depending on
>>> VIRTIO_NET_F_MRG_RXBUF.
>>>
>>> If Rx scatter is not requested, ensure that provided Rx buffers on
>>> each Rx queue are big enough to fit Rx packets up to configured MTU.
>>>
>>> Fixes: ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
>>> Cc: sta...@dpdk.org
>>>
>>> Signed-off-by: Ivan Ilchenko
>>> Signed-off-by: Andrew Rybchenko
>>> Reviewed-by: Maxime Coquelin
>>> ---
>>> v3:
>>> - fix segfault on MTU set if an Rx queue is not setup
>>>
>>> v2:
>>> - do not overwrite Rx offloads when Rx scatter is added
>>>
>>> drivers/net/virtio/virtio.h | 2 +
>>> drivers/net/virtio/virtio_ethdev.c | 65 ++
>>> drivers/net/virtio/virtio_ethdev.h | 5 +++
>>> drivers/net/virtio/virtio_rxtx.c | 10 +
>>> 4 files changed, 82 insertions(+)
>>>
>>
>> Thanks for the fix.
>> I see my R-by is already there, but I confirm this is good to me.
>
> It was inherited from v1, since changes from v1 to v3 are really minor
> fixes.
>
Yes, no problem. I was just to let Chenbo know, so that he can add it to
the PR.
Maxime
Re: [dpdk-dev] [EXT] Re: [PATCH v2] crypto/mvsam: IPSec full offload support
> You should implement checks for crypto doc in devtools/check-doc-vs- > code.sh Ok will look into it.
Re: [dpdk-dev] [EXT] Re: [PATCH v2 1/2] drivers: add octeontx crypto adapter framework
> 20/07/2021 14:14, David Marchand:
> > On Tue, Jul 20, 2021 at 1:59 PM Akhil Goyal wrote:
> > >
> > > Hi David,
> > > >
> > > > > > deps += ['common_octeontx', 'mempool_octeontx', 'bus_vdev',
> > > > > 'net_octeontx']
> > > > > > +deps += ['crypto_octeontx']
> > > > >
> > > > > This extra dependency resulted in disabling the event/octeontx driver
> > > > > in FreeBSD, since crypto/octeontx only builds on Linux.
> > > > > Removing hw support triggers a ABI failure for FreeBSD.
> > > > >
> > > > >
> > > > > - This had been reported by UNH CI:
> > > > > https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__mails.dpdk.org_archives_test-2Dreport_2021-
> 2DJune_200637.html&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2
> wl_PRwpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m=zikYn88P-
> Q3H517Go0NWLsokSeUCheJhQyY-Rh-
> DAWQ&s=v6vmJJNBDxjoA81J4rpuxvgPhR8DCT6qizgAkXauZIY&e=
> > > > > It seems the result has been ignored but it should have at least
> > > > > raised some discussion.
> > > > >
> > > > This was highlighted to CI ML
> > > > https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__patches.dpdk.org_project_dpdk_patch_0686a7c3fb3a22e37378a8545b
> &d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9TWey3eu6
> 8gBzn7DkPwuqhd6WNyo&m=zikYn88P-Q3H517Go0NWLsokSeUCheJhQyY-
> Rh-DAWQ&s=68Xkwo5J0d3BngYD0gxM0JKIgDzd58pypXyJrprGIgA&e=
> > > > c37bce04f4c391.1624481225.git.sthot...@marvell.com/
> > > >
> > > > but I think I missed to take the follow up with Brandon and applied the
> patch
> > > > as it did not look an issue to me as octeon drivers are not currently
> > > > built
> on
> > > > FreeBSD.
> > > > Not sure why event driver is getting built there.
> > > >
> > > > >
> > > > > - I asked UNH to stop testing FreeBSD abi for now, waiting to get the
> > > > > main branch fixed.
> > > > >
> > > > > I don't have the time to look at this, please can you work on it?
> > > > >
> > > > > Several options:
> > > > > * crypto/octeontx is made so that it compiles on FreeBSD,
> > > > > * the abi check is extended to have exceptions per OS,
> > > > > * the FreeBSD abi reference is regenerated at UNH not to have those
> > > > > drivers in it (not sure it is doable),
> > > >
> > > > Thanks for the suggestions, we are working on it to resolve this as soon
> as
> > > > possible.
> > > > We may need to add exception in ABI checking so that it does not shout
> if a
> > > > PMD
> > > > is not compiled.
> > > Can we have below change? Will it work to disable compilation of
> > > event/octeontx2 for FreeBSD? I believe this was done by mistake earlier
> > > as all other octeontx2 drivers are compiled off on platforms other than
> Linux.
> > >
> > > diff --git a/drivers/event/octeontx2/meson.build
> b/drivers/event/octeontx2/meson.build
> > > index 96ebb1f2e7..1ebc51f73f 100644
> > > --- a/drivers/event/octeontx2/meson.build
> > > +++ b/drivers/event/octeontx2/meson.build
> > > @@ -2,7 +2,7 @@
> > > # Copyright(C) 2019 Marvell International Ltd.
> > > #
> > >
> > > -if not dpdk_conf.get('RTE_ARCH_64')
> > > +if not is_linux or not dpdk_conf.get('RTE_ARCH_64')
> > > build = false
> > > reason = 'only supported on 64-bit'
> > > subdir_done()
> >
> > I did not suggest this possibility.
> > That's the same as for other octeon drivers, such change has been
> > deferred to 21.11.
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__patches.dpdk.org_project_dpdk_list_-3Fseries-
> 3D15885&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9T
> Wey3eu68gBzn7DkPwuqhd6WNyo&m=zikYn88P-
> Q3H517Go0NWLsokSeUCheJhQyY-Rh-
> DAWQ&s=A5fHouoeBcH2sL_xt5dtzRwfA8Fq__eBUYc-J9ANBIg&e=
> >
> > >
> > > Or of this does not work, then we would need to add exception in ABI
> checking.
> > > Any suggestions how to do this?
> >
> > Sorry, no good idea from me.
>
> We would need to revert the change breaking the ABI test.
> But I don't understand why it seems passing in recent CI runs?
>
It is passing because FreeBSD is currently skipped. Right David?
BTW, no need to revert, we would be sending a patch to enable compilation
of crypto/octeontx
[dpdk-dev] [PATCH] net/sfc: fix broken build with clang 3.4.x
Old clanng requires libatomic as well as gcc. Avoid compiler name and
version based checks. Add custom test for 16-byte atomic operations
to find out if libatomic is required to build.
Bugzilla ID: 760
Signed-off-by: Andrew Rybchenko
---
drivers/net/sfc/meson.build | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/sfc/meson.build b/drivers/net/sfc/meson.build
index 4625859077..a1ad792b80 100644
--- a/drivers/net/sfc/meson.build
+++ b/drivers/net/sfc/meson.build
@@ -40,8 +40,20 @@ foreach flag: extra_flags
endif
endforeach
-# for gcc compiles we need -latomic for 128-bit atomic ops
-if cc.get_id() == 'gcc'
+# for gcc and old Clang compiles we need -latomic for 128-bit atomic ops
+atomic_check_code = '''
+int main(void)
+{
+__int128 a = 0;
+__int128 b;
+
+b = __atomic_load_n(&a, __ATOMIC_RELAXED);
+__atomic_store(&b, &a, __ATOMIC_RELAXED);
+__atomic_store_n(&b, a, __ATOMIC_RELAXED);
+return 0;
+}
+'''
+if not cc.links(atomic_check_code)
libatomic_dep = cc.find_library('atomic', required: false)
if not libatomic_dep.found()
build = false
@@ -51,11 +63,7 @@ if cc.get_id() == 'gcc'
# libatomic could be half-installed when above check finds it but
# linkage fails
-atomic_link_code = '''
-#include
-void main() { printf("libatomic link check\n"); }
-'''
-if not cc.links(atomic_link_code, dependencies: libatomic_dep)
+if not cc.links(atomic_check_code, dependencies: libatomic_dep)
build = false
reason = 'broken dependency, "libatomic"'
subdir_done()
--
2.30.2
[dpdk-dev] [PATCH v2] net/sfc: fix broken build with clang 3.4.x
Old clanng requires libatomic as well as gcc. Avoid compiler name and
version based checks. Add custom test for 16-byte atomic operations
to find out if libatomic is required to build.
Fixes: 96fd2bd69b58 ("net/sfc: support flow action count in transfer rules")
Bugzilla ID: 760
Signed-off-by: Andrew Rybchenko
---
drivers/net/sfc/meson.build | 22 +++---
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/sfc/meson.build b/drivers/net/sfc/meson.build
index 4625859077..a1ad792b80 100644
--- a/drivers/net/sfc/meson.build
+++ b/drivers/net/sfc/meson.build
@@ -40,8 +40,20 @@ foreach flag: extra_flags
endif
endforeach
-# for gcc compiles we need -latomic for 128-bit atomic ops
-if cc.get_id() == 'gcc'
+# for gcc and old Clang compiles we need -latomic for 128-bit atomic ops
+atomic_check_code = '''
+int main(void)
+{
+__int128 a = 0;
+__int128 b;
+
+b = __atomic_load_n(&a, __ATOMIC_RELAXED);
+__atomic_store(&b, &a, __ATOMIC_RELAXED);
+__atomic_store_n(&b, a, __ATOMIC_RELAXED);
+return 0;
+}
+'''
+if not cc.links(atomic_check_code)
libatomic_dep = cc.find_library('atomic', required: false)
if not libatomic_dep.found()
build = false
@@ -51,11 +63,7 @@ if cc.get_id() == 'gcc'
# libatomic could be half-installed when above check finds it but
# linkage fails
-atomic_link_code = '''
-#include
-void main() { printf("libatomic link check\n"); }
-'''
-if not cc.links(atomic_link_code, dependencies: libatomic_dep)
+if not cc.links(atomic_check_code, dependencies: libatomic_dep)
build = false
reason = 'broken dependency, "libatomic"'
subdir_done()
--
2.30.2
[dpdk-dev] [PATCH] net/iavf: fix tx thresh check issue
Function check_tx_thresh is called with wrong parameter. If the
check fails, tx_queue_setup should return error not keep going.
iThis patch fixes above issues.
Fixes: 69dd4c3d0898 ("net/avf: enable queue and device")
Cc: sta...@dpdk.org
Signed-off-by: Xiaoyun Li
---
drivers/net/iavf/iavf_rxtx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c
index d61b32fcee..e33fe4576b 100644
--- a/drivers/net/iavf/iavf_rxtx.c
+++ b/drivers/net/iavf/iavf_rxtx.c
@@ -708,7 +708,8 @@ iavf_dev_tx_queue_setup(struct rte_eth_dev *dev,
tx_conf->tx_rs_thresh : DEFAULT_TX_RS_THRESH);
tx_free_thresh = (uint16_t)((tx_conf->tx_free_thresh) ?
tx_conf->tx_free_thresh : DEFAULT_TX_FREE_THRESH);
- check_tx_thresh(nb_desc, tx_rs_thresh, tx_rs_thresh);
+ if (check_tx_thresh(nb_desc, tx_rs_thresh, tx_free_thresh) != 0)
+ return -EINVAL;
/* Free memory if needed. */
if (dev->data->tx_queues[queue_idx]) {
--
2.25.1
[dpdk-dev] [PATCH v2] ifpga/base/meson: fix looking for librt
From: Mohamad Noor Alim Hussin
Finding with "librt" keyword would give the output with full path of librt such
as /usr/lib/gcc/x86_64-linux-gnu/7/../../../x86_64-linux-gnu/librt.so
instead of -lrt in libdpdk.pc pkg-config file.
Assume find_library() will prepend "lib", thus remove "lib" from "librt"
keyword. The output will shows as -lrt.
This will cause an issue when compile DPDK app with static library as the
path of librt has been hard-coded in the libdpdk.pc file.
Fixes: e41856b515ce ("raw/ifpga/base: enhance driver reliability in
multi-process")
Cc: tianfei.zh...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Mohamad Noor Alim Hussin
---
drivers/raw/ifpga/base/meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/raw/ifpga/base/meson.build
b/drivers/raw/ifpga/base/meson.build
index da2d6e33c..949f7f127 100644
--- a/drivers/raw/ifpga/base/meson.build
+++ b/drivers/raw/ifpga/base/meson.build
@@ -25,7 +25,7 @@ sources = [
rtdep = dependency('librt', required: false)
if not rtdep.found()
- rtdep = cc.find_library('librt', required: false)
+ rtdep = cc.find_library('rt', required: false)
endif
if not rtdep.found()
build = false
--
2.32.0
[dpdk-dev] [PATCH] maintainers: update for crypto API
Claim ownership for crypto API layer. Have been reviewing patches from quite some time. Signed-off-by: Akhil Goyal --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index c68acbcd06..8b5a3f0249 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -420,6 +420,7 @@ F: examples/bbdev_app/ F: doc/guides/sample_app_ug/bbdev_app.rst Crypto API +M: Akhil Goyal M: Declan Doherty T: git://dpdk.org/next/dpdk-next-crypto F: lib/cryptodev/ -- 2.25.1
[dpdk-dev] [PATCH] crypto/octeontx: enable build on non Linux OS
Enabled build of Octeontx crypto PMD on non linux OS. Other Octeontx PMDs are enabled already. This is to avoid ABI test failure on an OS once we add dependency between a driver which is built to another which is not. Signed-off-by: Shijith Thotton --- drivers/crypto/octeontx/meson.build | 4 1 file changed, 4 deletions(-) diff --git a/drivers/crypto/octeontx/meson.build b/drivers/crypto/octeontx/meson.build index 3ae6729e8f..244b16230e 100644 --- a/drivers/crypto/octeontx/meson.build +++ b/drivers/crypto/octeontx/meson.build @@ -1,9 +1,5 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2018 Cavium, Inc -if not is_linux -build = false -reason = 'only supported on Linux' -endif deps += ['bus_pci'] deps += ['bus_vdev'] -- 2.25.1
Re: [dpdk-dev] [PATCH 03/11] ethdev: fix docs of functions getting xstats by IDs
On 7/20/21 7:25 PM, Ferruh Yigit wrote:
On 6/4/2021 3:42 PM, Andrew Rybchenko wrote:
From: Ivan Ilchenko
Document valid combinations of input arguments in accordance with
current implementation in ethdev.
Fixes: 79c913a42f0 ("ethdev: retrieve xstats by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
lib/ethdev/rte_ethdev.h | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index faf3bd901d..1f63118544 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2873,12 +2873,15 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* The port identifier of the Ethernet device.
* @param xstats_names
* An rte_eth_xstat_name array of at least *size* elements to
- * be filled. If set to NULL, the function returns the required number
- * of elements.
+ * be filled. Must not be NULL if @p ids are specified (not NULL).
Removed part is also valid. If both 'ids' & 'xstats_names' are NULL, API returns
number of all elements.
Yes, but it is an excessive information. The trigger to return number
of all elements is 'ids == NULL'. Here we are talking about
'xstats_names' parameter. If the parameter is NULL, but ids is not
null, it does not trigger number of all elements return. It is an
invalid input parameters. That's what a new description says.
Addition part looks good.
* @param ids
- * IDs array given by app to retrieve specific statistics
+ * IDs array given by app to retrieve specific statistics. May be NULL
+ * to retrieve all available statistics.
ack
* @param size
- * The size of the xstats_names array (number of elements).
+ * If @p ids is not NULL, number of elements in the array with requested IDs
+ * and number of elements in @p xstats_names to put names in. If @p ids is
+ * NULL, number of elements in @p xstats_names to put all available
statistics
+ * names in.
ack
* @return
* - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
@@ -2886,7 +2889,7 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - A negative value on error (invalid port id).
+ * - A negative value on error.
ack
The 'eth_dev_get_xstats_count()' API is flexible but it makes API unnecessarily
complex, not for this patch but for future perhaps we can update the API and it
can return error if either 'ids' or 'xstats_names' is NULL. Remove support to
get all elements or getting number of elements support, these already supported
by non _id version of API.
I'm not sure that it is a right direction. The support allows
application to use less number of functions and depend on less
number of function prototypes.
And as a note for future, if we ever consider updating these _by_id APIs, we can
consider making the parameter order same for both, currently it is:
"rte_eth_xstats_get_names_by_id(port_id, values, size, ids)"
" rte_eth_xstats_get_by_id(port_id, ids, values, size)"
+1, current difference is terribly bad
*/
int
rte_eth_xstats_get_names_by_id(uint16_t port_id,
@@ -2900,13 +2903,15 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id,
* The port identifier of the Ethernet device.
* @param ids
* A pointer to an ids array passed by application. This tells which
- * statistics values function should retrieve. This parameter
- * can be set to NULL if size is 0. In this case function will retrieve
+ * statistics values function should retrieve. May be NULL to retrieve
* all available statistics.
Update is good. But what do you think to make it exact same in the both APIs
('rte_eth_xstats_get_names_by_id()' & 'rte_eth_xstats_get_by_id()')? Since it is
used for same purpose and exact same way in both APIs, no need to have slightly
different description.
I agree. I'll fix in v2.
* @param values
* A pointer to a table to be filled with device statistics values.
+ * Must not be NULL if ids are specified (not NULL).
Same comment on making description similar in both APIs.
OK
Also both 'ids' & 'values' being NULL returns number of all elements should be
addressed.
I think it is excessibe. It is sufficient to say so for ids==NULL which
is a trigger to get all elements.
* @param size
- * The size of the ids array (number of elements).
+ * If @p ids is not NULL, number of elements in the array with requested IDs
+ * and number of elements in values to put statistics in. If @p ids is NULL,
+ * number of elements in values to put all available statistics in.
ack
* @return
* - A positive value lower or equa
Re: [dpdk-dev] [PATCH v2] net/sfc: fix broken build with clang 3.4.x
Hi Andrew,
On Thu, Jul 22, 2021 at 9:49 AM Andrew Rybchenko
wrote:
>
> Old clanng requires libatomic as well as gcc. Avoid compiler name and
s/nn/n/
> version based checks. Add custom test for 16-byte atomic operations
> to find out if libatomic is required to build.
>
> Bugzilla ID: 760
> Fixes: 96fd2bd69b58 ("net/sfc: support flow action count in transfer rules")
>
> Signed-off-by: Andrew Rybchenko
> ---
> drivers/net/sfc/meson.build | 22 +++---
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/sfc/meson.build b/drivers/net/sfc/meson.build
> index 4625859077..a1ad792b80 100644
> --- a/drivers/net/sfc/meson.build
> +++ b/drivers/net/sfc/meson.build
> @@ -40,8 +40,20 @@ foreach flag: extra_flags
> endif
> endforeach
>
> -# for gcc compiles we need -latomic for 128-bit atomic ops
> -if cc.get_id() == 'gcc'
> +# for gcc and old Clang compiles we need -latomic for 128-bit atomic ops
> +atomic_check_code = '''
> +int main(void)
> +{
> +__int128 a = 0;
> +__int128 b;
> +
> +b = __atomic_load_n(&a, __ATOMIC_RELAXED);
> +__atomic_store(&b, &a, __ATOMIC_RELAXED);
> +__atomic_store_n(&b, a, __ATOMIC_RELAXED);
> +return 0;
> +}
> +'''
> +if not cc.links(atomic_check_code)
Nice.
> libatomic_dep = cc.find_library('atomic', required: false)
> if not libatomic_dep.found()
> build = false
> @@ -51,11 +63,7 @@ if cc.get_id() == 'gcc'
>
> # libatomic could be half-installed when above check finds it but
> # linkage fails
> -atomic_link_code = '''
> -#include
> -void main() { printf("libatomic link check\n"); }
> -'''
> -if not cc.links(atomic_link_code, dependencies: libatomic_dep)
> +if not cc.links(atomic_check_code, dependencies: libatomic_dep)
> build = false
> reason = 'broken dependency, "libatomic"'
> subdir_done()
> --
> 2.30.2
>
Such a check will have its place in a common place if another
component in DPDK starts to depend on libatomic in the future.
But for now, this patch lgtm.
Thanks.
Acked-by: David Marchand
--
David Marchand
Re: [dpdk-dev] [PATCH] crypto/octeontx: enable build on non Linux OS
> Enabled build of Octeontx crypto PMD on non linux OS. Other Octeontx
> PMDs are enabled already.
>
> This is to avoid ABI test failure on an OS once we add dependency
> between a driver which is built to another which is not.
Fixes: 8dc6c2f12ecf ("crypto/octeontx: add crypto adapter framework")
>
Reported-by: David Marchand
> Signed-off-by: Shijith Thotton
Acked-by: Akhil Goyal
Thomas/David: please pick this patch directly on main to fix build on CI for
FreeBSD.
Re: [dpdk-dev] imissed drop with mellanox connectx5
Hi Yaron Freeing mbufs from a different lcore than the original lcore allocated them causes cache miss in the mempool cache of the original lcore per mbuf allocation - all the time the PMD will get non-hot mbufs to work with. It can be one of the reasons for the earlier drops you see. Matan From: Yaron Illouz > Hi > > We try to read from 100G NIC Mellanox ConnectX-5 without drop at nic. > All thread are with core pinning and cpu isolation. > We use dpdk 19.11 > I tried to apply all configuration that are in > https://fast.dpdk.org/doc/perf/DPDK_19_08_Mellanox_NIC_performance_r > eport.pdf > > We have a strange behavior, 1 thread can receive receive 20 Gbps/12 Mpps > and free mbuf without dropps, but when trying to pass these mbuf to > another thread that only free them there are drops, even when trying to > work with more threads. > > When running 1 thread that only read from port (no multi queue) and free > mbuf in the same thread, there are no dropp with traffic up to 21 Gbps 12.4 > Mpps. > When running 6 thread that only read from port (with multi queue) and free > mbuf in the same threads, there are no dropp with traffic up to 21 Gbps 12.4 > Mpps. > > When running 1 to 6 thread that only read from port and pass them to > another 6 thread that only read from ring and free mbuf, there are dropp in > nic (imissed counter) with traffic over to 10 Gbps 5.2 Mpps.(Here receive > thread were pinned to cpu 1-6 and additional thread from 7-12 each thread > on a single cpu) Each receive thread send to one thread that free the buffer. > > Configurations: > > We use rings of size 32768 between the threads. Ring are initialized with > SP/SC, Write are done with bulk of 512 with rte_ring_enqueue_burst. > Port is initialized with rte_eth_rx_queue_setup nb_rx_desc=8192 > rte_eth_rxconf - rx_conf.rx_thresh.pthresh = DPDK_NIC_RX_PTHRESH; > //ring prefetch threshold > rx_conf.rx_thresh.hthresh = > DPDK_NIC_RX_HTHRESH; //ring > host threshold > rx_conf.rx_thresh.wthresh = > DPDK_NIC_RX_WTHRESH; > //ring writeback threshold > rx_conf.rx_free_thresh = > DPDK_NIC_RX_FREE_THRESH; rss - > > ETH_RSS_IP | ETH_RSS_UDP | ETH_RSS_TCP; > > > We tried to work with and without hyperthreading. > > > > Network devices using kernel driver > === > :37:00.0 'MT27800 Family [ConnectX-5] 1017' if=ens2f0 drv=mlx5_core > unused=igb_uio > :37:00.1 'MT27800 Family [ConnectX-5] 1017' if=ens2f1 drv=mlx5_core > unused=igb_uio > > > > ethtool -i ens2f0 > driver: mlx5_core > version: 5.3-1.0.0 > firmware-version: 16.30.1004 (HPE09) > expansion-rom-version: > bus-info: :37:00.0 > supports-statistics: yes > supports-test: yes > supports-eeprom-access: no > supports-register-dump: no > supports-priv-flags: yes > > > > uname -a > Linux localhost.localdomain 3.10.0-1160.el7.x86_64 #1 SMP Mon Oct 19 > 16:18:59 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux > > > > lscpu | grep -e Socket -e Core -e Thread > Thread(s) per core:1 > Core(s) per socket:24 > Socket(s): 2 > > > cat /sys/devices/system/node/node0/cpulist > 0-23 > > From /proc/cpuinfo > > processor : 0 > vendor_id : GenuineIntel > cpu family : 6 > model : 85 > model name : Intel(R) Xeon(R) Gold 5220R CPU @ 2.20GHz > stepping: 7 > microcode : 0x5003003 > cpu MHz : 2200.000 > > > > python /home/cpu_layout.py > == > > Core and Socket Information (as reported by '/sys/devices/system/cpu') > == > > > cores = [0, 1, 2, 3, 4, 5, 6, 8, 9, 10, 11, 12, 13, 16, 17, 18, 19, 20, 21, > 25, 26, 27, > 28, 29, 24] sockets = [0, 1] > > Socket 0Socket 1 > > Core 0 [0] [24] > Core 1 [1] [25] > Core 2 [2] [26] > Core 3 [3] [27] > Core 4 [4] [28] > Core 5 [5] [29] > Core 6 [6] [30] > Core 8 [7] > Core 9 [8] [31] > Core 10 [9] [32] > Core 11 [10][33] > Core 12 [11][34] > Core 13 [12][35] > Core 16 [13][36] > Core 17 [14][37] > Core 18 [15][38] > Core 19 [16][39] > Core 20 [17][40] > Core 21 [18][41] > Core 25 [19][43] > Core 26 [20][44] > Core 27 [21][45] > Core 28 [22][46] > Core 29 [23][47] > Core 24 [42]
Re: [dpdk-dev] [PATCH 04/11] ethdev: fix docs of drivers callbacks getting xstats by IDs
On 7/20/21 7:51 PM, Ferruh Yigit wrote:
On 6/4/2021 3:42 PM, Andrew Rybchenko wrote:
From: Ivan Ilchenko
Update xstats by IDs callbacks documentation in accordance with
ethdev usage of these callbacks. Document valid combinations of
input arguments to make driver implementation simpler.
Fixes: 79c913a42f0 ("ethdev: retrieve xstats by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
lib/ethdev/ethdev_driver.h | 43 --
1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 40e474aa7e..fd5b7ca550 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -187,11 +187,28 @@ typedef int (*eth_xstats_get_t)(struct rte_eth_dev *dev,
struct rte_eth_xstat *stats, unsigned int n);
/**< @internal Get extended stats of an Ethernet device. */
+/**
+ * @internal
+ * Get extended stats of an Ethernet device.
Should it mention _by_id detail?
Yes, will fix in v2.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param ids
+ * IDs array to retrieve specific statistics. Must not be NULL.
+ * @param values
+ * A pointer to a table to be filled with device statistics values.
+ * Must not be NULL.
+ * @param n
+ * Element count in @p ids and @p values
+ *
+ * @return
+ * - A number of filled in stats.
+ * - A negative value on error.
+ */
typedef int (*eth_xstats_get_by_id_t)(struct rte_eth_dev *dev,
const uint64_t *ids,
uint64_t *values,
unsigned int n);
-/**< @internal Get extended stats of an Ethernet device. */
/**
* @internal
@@ -218,10 +235,32 @@ typedef int (*eth_xstats_get_names_t)(struct rte_eth_dev
*dev,
struct rte_eth_xstat_name *xstats_names, unsigned int size);
/**< @internal Get names of extended stats of an Ethernet device. */
+/**
+ * @internal
+ * Get names of extended stats of an Ethernet device.
Should it mention _by_id detail?
Yes, will fix in v2.
+ * For name count, set @p xstats_names and @p ids to NULL.
+ *
isn't the 'count' part handled in the API? I think in the devops both should not
be NULL.
No, eth_dev_get_xstats_count() uses the callback with NULL, NULL, 0.
+ * @param dev
+ * ethdev handle of port.
+ * @param xstats_names
+ * An rte_eth_xstat_name array of at least *size* elements to
+ * be filled. Can be NULL together with @p ids to retrieve number of
+ * available statistics.
As far as I understand both _by_id APIs and devops behave same, so argument
descriptions/behavior should be same.
In fact no, it is slightly different. For example, devops is never
called with NULL ids and not NULL names or non-zero size. It allows to
check less in drivers.
+ * @param ids
+ * IDs array to retrieve specific statistics. Can be NULL together
+ * with @p xstats_names to retrieve number of available statistics.
+ * @param size
+ * Size of ids and xstats_names arrays.
+ * Element count in @p ids and @p xstats_names
+ *
+ * @return
+ * - A number of filled in stats if both xstats_names and ids are not NULL.
+ * - A number of available stats if both xstats_names and ids are NULL.
Again as far as I can see these covered by API, not devops, am I missing
something.
See eth_dev_get_xstats_count()
+ * - A negative value on error.
+ */
typedef int (*eth_xstats_get_names_by_id_t)(struct rte_eth_dev *dev,
struct rte_eth_xstat_name *xstats_names, const uint64_t *ids,
unsigned int size);
-/**< @internal Get names of extended stats of an Ethernet device. */
typedef int (*eth_queue_stats_mapping_set_t)(struct rte_eth_dev *dev,
uint16_t queue_id,
[dpdk-dev] [PATCH v2 00/11] net/sfc: provide Rx/Tx doorbells stats
Rx/Tx doorbells stats are essential for performance investigation. On the way fix ethdev documenation to refine requirements on driver callback. It allows to make these callbacks a bit simpler. Add testpmd option to show specified xstats periodically or upon request, for example: * --display-xstats rx_good_packets,tx_good_packets --stats-period 1 Port statistics NIC statistics for port 0 RX-packets: 14102808 RX-missed: 0 RX-bytes: 7164239264 RX-errors: 0 RX-nombuf: 0 TX-packets: 14102789 TX-errors: 0 TX-bytes: 7164226028 Throughput (since last show) Rx-pps: 2349577 Rx-bps: 9548682392 Tx-pps: 2349576 Tx-bps: 9548682408 ValueRate (since last show) rx_good_packets 14103280 2349575 tx_good_packets 14103626 2349573 * -i --display-xstats tx_good_packets,vadapter_rx_overflow testpmd> port start 0 ... No xstat 'vadapter_rx_overflow' on port 0 - skip it ... testpmd> start tx_first testpmd> show port stats all ValueRate (since last show) tx_good_packets 132545336 1420439 v2: - address Ferruh review notes on ethdev patches Ivan Ilchenko (11): net/sfc: fix get xstats by ID callback to use MAC stats lock net/sfc: fix reading adapter state without locking ethdev: fix docs of functions getting xstats by IDs ethdev: fix docs of drivers callbacks getting xstats by IDs net/sfc: fix xstats by ID callbacks according to ethdev net/sfc: fix accessing xstats by an unsorted list of IDs net/sfc: fix MAC stats update to work for stopped device net/sfc: simplify getting of available xstats case net/sfc: prepare to add more xstats net/sfc: add xstats for Rx/Tx doorbells app/testpmd: add option to display extended statistics app/test-pmd/cmdline.c| 56 +++ app/test-pmd/config.c | 66 +++ app/test-pmd/parameters.c | 18 + app/test-pmd/testpmd.c| 122 ++ app/test-pmd/testpmd.h| 21 + doc/guides/testpmd_app_ug/run_app.rst | 5 + drivers/net/sfc/meson.build | 1 + drivers/net/sfc/sfc.c | 16 + drivers/net/sfc/sfc.h | 18 +- drivers/net/sfc/sfc_dp.h | 10 + drivers/net/sfc/sfc_ef10.h| 3 +- drivers/net/sfc/sfc_ef100_rx.c| 1 + drivers/net/sfc/sfc_ef100_tx.c| 1 + drivers/net/sfc/sfc_ef10_essb_rx.c| 3 +- drivers/net/sfc/sfc_ef10_rx.c | 3 +- drivers/net/sfc/sfc_ef10_tx.c | 1 + drivers/net/sfc/sfc_ethdev.c | 185 + drivers/net/sfc/sfc_port.c| 127 +- drivers/net/sfc/sfc_rx.c | 1 + drivers/net/sfc/sfc_sw_stats.c| 572 ++ drivers/net/sfc/sfc_sw_stats.h| 49 +++ drivers/net/sfc/sfc_tx.c | 4 +- lib/ethdev/ethdev_driver.h| 43 +- lib/ethdev/rte_ethdev.h | 30 +- 24 files changed, 1246 insertions(+), 110 deletions(-) create mode 100644 drivers/net/sfc/sfc_sw_stats.c create mode 100644 drivers/net/sfc/sfc_sw_stats.h -- 2.30.2
[dpdk-dev] [PATCH v2 01/11] net/sfc: fix get xstats by ID callback to use MAC stats lock
From: Ivan Ilchenko
Add MAC stats lock in get xstats by id callback before reading
number of supported MAC stats.
Fixes: 73280c1e4ff ("net/sfc: support xstats retrieval by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
drivers/net/sfc/sfc_ethdev.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 88896db1f8..d4ac61ff76 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -789,12 +789,14 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
int ret;
int rc;
- if (unlikely(values == NULL) ||
- unlikely((ids == NULL) && (n < port->mac_stats_nb_supported)))
- return port->mac_stats_nb_supported;
-
rte_spinlock_lock(&port->mac_stats_lock);
+ if (unlikely(values == NULL) ||
+ unlikely(ids == NULL && n < port->mac_stats_nb_supported)) {
+ ret = port->mac_stats_nb_supported;
+ goto unlock;
+ }
+
rc = sfc_port_update_mac_stats(sa);
if (rc != 0) {
SFC_ASSERT(rc > 0);
--
2.30.2
[dpdk-dev] [PATCH v2 02/11] net/sfc: fix reading adapter state without locking
From: Ivan Ilchenko
Update MAC stats function reads adapter state with MAC stats locking
but without adapter locking. Add adapter locking before calling this
function and remove MAC stats locking since there's no point to have
it together with adapter locking. The second place MAC stats locking
is used is MAC stats reset function. It's called with adapter being
already locked so there's no point to use MAC stats locking anymore.
Fixes: 1caab2f1e68 ("net/sfc: add basic statistics")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
drivers/net/sfc/sfc.h| 1 -
drivers/net/sfc/sfc_ethdev.c | 28
drivers/net/sfc/sfc_port.c | 9 +++--
3 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 546739bd4a..c7b0e5a30d 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -130,7 +130,6 @@ struct sfc_port {
unsigned intnb_mcast_addrs;
uint8_t *mcast_addrs;
- rte_spinlock_t mac_stats_lock;
uint64_t*mac_stats_buf;
unsigned intmac_stats_nb_supported;
efsys_mem_t mac_stats_dma_mem;
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index d4ac61ff76..d5417e5e65 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -613,7 +613,7 @@ sfc_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats
*stats)
uint64_t *mac_stats;
int ret;
- rte_spinlock_lock(&port->mac_stats_lock);
+ sfc_adapter_lock(sa);
ret = sfc_port_update_mac_stats(sa);
if (ret != 0)
@@ -686,7 +686,7 @@ sfc_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats
*stats)
}
unlock:
- rte_spinlock_unlock(&port->mac_stats_lock);
+ sfc_adapter_unlock(sa);
SFC_ASSERT(ret >= 0);
return -ret;
}
@@ -698,12 +698,15 @@ sfc_stats_reset(struct rte_eth_dev *dev)
struct sfc_port *port = &sa->port;
int rc;
+ sfc_adapter_lock(sa);
+
if (sa->state != SFC_ADAPTER_STARTED) {
/*
* The operation cannot be done if port is not started; it
* will be scheduled to be done during the next port start
*/
port->mac_stats_reset_pending = B_TRUE;
+ sfc_adapter_unlock(sa);
return 0;
}
@@ -711,6 +714,8 @@ sfc_stats_reset(struct rte_eth_dev *dev)
if (rc != 0)
sfc_err(sa, "failed to reset statistics (rc = %d)", rc);
+ sfc_adapter_unlock(sa);
+
SFC_ASSERT(rc >= 0);
return -rc;
}
@@ -726,7 +731,7 @@ sfc_xstats_get(struct rte_eth_dev *dev, struct
rte_eth_xstat *xstats,
unsigned int i;
int nstats = 0;
- rte_spinlock_lock(&port->mac_stats_lock);
+ sfc_adapter_lock(sa);
rc = sfc_port_update_mac_stats(sa);
if (rc != 0) {
@@ -748,7 +753,7 @@ sfc_xstats_get(struct rte_eth_dev *dev, struct
rte_eth_xstat *xstats,
}
unlock:
- rte_spinlock_unlock(&port->mac_stats_lock);
+ sfc_adapter_unlock(sa);
return nstats;
}
@@ -789,7 +794,7 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
int ret;
int rc;
- rte_spinlock_lock(&port->mac_stats_lock);
+ sfc_adapter_lock(sa);
if (unlikely(values == NULL) ||
unlikely(ids == NULL && n < port->mac_stats_nb_supported)) {
@@ -819,7 +824,7 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
ret = nb_written;
unlock:
- rte_spinlock_unlock(&port->mac_stats_lock);
+ sfc_adapter_unlock(sa);
return ret;
}
@@ -835,9 +840,14 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
unsigned int nb_written = 0;
unsigned int i;
+ sfc_adapter_lock(sa);
+
if (unlikely(xstats_names == NULL) ||
- unlikely((ids == NULL) && (size < port->mac_stats_nb_supported)))
- return port->mac_stats_nb_supported;
+ unlikely((ids == NULL) && (size < port->mac_stats_nb_supported))) {
+ nb_supported = port->mac_stats_nb_supported;
+ sfc_adapter_unlock(sa);
+ return nb_supported;
+ }
for (i = 0; (i < EFX_MAC_NSTATS) && (nb_written < size); ++i) {
if (!EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i))
@@ -853,6 +863,8 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
++nb_supported;
}
+ sfc_adapter_unlock(sa);
+
return nb_written;
}
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index ac117f9c48..cdc0f94f19 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -43,7 +43,7 @@ sfc_port_update_mac_sta
[dpdk-dev] [PATCH v2 03/11] ethdev: fix docs of functions getting xstats by IDs
From: Ivan Ilchenko
Document valid combinations of input arguments in accordance with
current implementation in ethdev.
Fixes: 79c913a42f0 ("ethdev: retrieve xstats by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
lib/ethdev/rte_ethdev.h | 30 +-
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c351f..80c42d2f08 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2872,13 +2872,16 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* @param port_id
* The port identifier of the Ethernet device.
* @param xstats_names
- * An rte_eth_xstat_name array of at least *size* elements to
- * be filled. If set to NULL, the function returns the required number
- * of elements.
+ * An array of at least @p size elements to be filled in.
+ * Must not be NULL if @p ids are specified (not NULL) or @p size is not 0.
* @param ids
- * IDs array given by app to retrieve specific statistics
+ * IDs array given by app to retrieve specific statistics names.
+ * May be NULL to retrieve all available statistics names.
* @param size
- * The size of the xstats_names array (number of elements).
+ * If @p ids is not NULL, number of elements in the array with requested IDs
+ * and number of elements in @p xstats_names to put names in. If @p ids is
+ * NULL, number of elements in @p xstats_names to put all available
statistics
+ * names in.
* @return
* - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
@@ -2886,7 +2889,7 @@ int rte_eth_xstats_get(uint16_t port_id, struct
rte_eth_xstat *xstats,
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - A negative value on error (invalid port id).
+ * - A negative value on error.
*/
int
rte_eth_xstats_get_names_by_id(uint16_t port_id,
@@ -2899,14 +2902,15 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id,
* @param port_id
* The port identifier of the Ethernet device.
* @param ids
- * A pointer to an ids array passed by application. This tells which
- * statistics values function should retrieve. This parameter
- * can be set to NULL if size is 0. In this case function will retrieve
- * all available statistics.
+ * IDs array given by app to retrieve specific statistics.
+ * May be NULL to retrieve all available statistics.
* @param values
- * A pointer to a table to be filled with device statistics values.
+ * An array of at least @p size elements to be filled in.
+ * Must not be NULL if @p ids are specified (not NULL) or @p size is not 0.
* @param size
- * The size of the ids array (number of elements).
+ * If @p ids is not NULL, number of elements in the array with requested IDs
+ * and number of elements in values to put statistics in. If @p ids is NULL,
+ * number of elements in values to put all available statistics in.
* @return
* - A positive value lower or equal to size: success. The return value
* is the number of entries filled in the stats table.
@@ -2914,7 +2918,7 @@ rte_eth_xstats_get_names_by_id(uint16_t port_id,
* is too small. The return value corresponds to the size that should
* be given to succeed. The entries in the table are not valid and
* shall not be used by the caller.
- * - A negative value on error (invalid port id).
+ * - A negative value on error.
*/
int rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids,
uint64_t *values, unsigned int size);
--
2.30.2
[dpdk-dev] [PATCH v2 04/11] ethdev: fix docs of drivers callbacks getting xstats by IDs
From: Ivan Ilchenko
Update xstats by IDs callbacks documentation in accordance with
ethdev usage of these callbacks. Document valid combinations of
input arguments to make driver implementation simpler.
Fixes: 79c913a42f0 ("ethdev: retrieve xstats by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
lib/ethdev/ethdev_driver.h | 43 --
1 file changed, 41 insertions(+), 2 deletions(-)
diff --git a/lib/ethdev/ethdev_driver.h b/lib/ethdev/ethdev_driver.h
index 40e474aa7e..e934be9285 100644
--- a/lib/ethdev/ethdev_driver.h
+++ b/lib/ethdev/ethdev_driver.h
@@ -187,11 +187,28 @@ typedef int (*eth_xstats_get_t)(struct rte_eth_dev *dev,
struct rte_eth_xstat *stats, unsigned int n);
/**< @internal Get extended stats of an Ethernet device. */
+/**
+ * @internal
+ * Get extended stats specified by IDs of an Ethernet device.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param ids
+ * IDs array to retrieve specific statistics. Must not be NULL.
+ * @param values
+ * A pointer to a table to be filled with device statistics values.
+ * Must not be NULL.
+ * @param n
+ * Element count in @p ids and @p values
+ *
+ * @return
+ * - A number of filled in stats.
+ * - A negative value on error.
+ */
typedef int (*eth_xstats_get_by_id_t)(struct rte_eth_dev *dev,
const uint64_t *ids,
uint64_t *values,
unsigned int n);
-/**< @internal Get extended stats of an Ethernet device. */
/**
* @internal
@@ -218,10 +235,32 @@ typedef int (*eth_xstats_get_names_t)(struct rte_eth_dev
*dev,
struct rte_eth_xstat_name *xstats_names, unsigned int size);
/**< @internal Get names of extended stats of an Ethernet device. */
+/**
+ * @internal
+ * Get names of extended stats specified by IDs of an Ethernet device.
+ * For name count, set @p xstats_names and @p ids to NULL.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param xstats_names
+ * An rte_eth_xstat_name array of at least *size* elements to
+ * be filled. Can be NULL together with @p ids to retrieve number of
+ * available statistics.
+ * @param ids
+ * IDs array to retrieve specific statistics. Can be NULL together
+ * with @p xstats_names to retrieve number of available statistics.
+ * @param size
+ * Size of ids and xstats_names arrays.
+ * Element count in @p ids and @p xstats_names
+ *
+ * @return
+ * - A number of filled in stats if both xstats_names and ids are not NULL.
+ * - A number of available stats if both xstats_names and ids are NULL.
+ * - A negative value on error.
+ */
typedef int (*eth_xstats_get_names_by_id_t)(struct rte_eth_dev *dev,
struct rte_eth_xstat_name *xstats_names, const uint64_t *ids,
unsigned int size);
-/**< @internal Get names of extended stats of an Ethernet device. */
typedef int (*eth_queue_stats_mapping_set_t)(struct rte_eth_dev *dev,
uint16_t queue_id,
--
2.30.2
[dpdk-dev] [PATCH v2 05/11] net/sfc: fix xstats by ID callbacks according to ethdev
From: Ivan Ilchenko
Fix xstats by ID callbacks according to ethdev usage.
Handle combinations of input arguments that are required by ethdev
and sanity check and reject other combinations on callback entry.
Fixes: 73280c1e4ff ("net/sfc: support xstats retrieval by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
drivers/net/sfc/sfc_ethdev.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index d5417e5e65..fca3f524a1 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -794,13 +794,10 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
int ret;
int rc;
- sfc_adapter_lock(sa);
+ if (unlikely(ids == NULL || values == NULL))
+ return -EINVAL;
- if (unlikely(values == NULL) ||
- unlikely(ids == NULL && n < port->mac_stats_nb_supported)) {
- ret = port->mac_stats_nb_supported;
- goto unlock;
- }
+ sfc_adapter_lock(sa);
rc = sfc_port_update_mac_stats(sa);
if (rc != 0) {
@@ -815,7 +812,7 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
if (!EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i))
continue;
- if ((ids == NULL) || (ids[nb_written] == nb_supported))
+ if (ids[nb_written] == nb_supported)
values[nb_written++] = mac_stats[i];
++nb_supported;
@@ -840,10 +837,13 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
unsigned int nb_written = 0;
unsigned int i;
+ if (unlikely(xstats_names == NULL && ids != NULL) ||
+ unlikely(xstats_names != NULL && ids == NULL))
+ return -EINVAL;
+
sfc_adapter_lock(sa);
- if (unlikely(xstats_names == NULL) ||
- unlikely((ids == NULL) && (size < port->mac_stats_nb_supported))) {
+ if (unlikely(xstats_names == NULL && ids == NULL)) {
nb_supported = port->mac_stats_nb_supported;
sfc_adapter_unlock(sa);
return nb_supported;
@@ -853,7 +853,7 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
if (!EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i))
continue;
- if ((ids == NULL) || (ids[nb_written] == nb_supported)) {
+ if (ids[nb_written] == nb_supported) {
char *name = xstats_names[nb_written++].name;
strlcpy(name, efx_mac_stat_name(sa->nic, i),
--
2.30.2
[dpdk-dev] [PATCH v2 06/11] net/sfc: fix accessing xstats by an unsorted list of IDs
From: Ivan Ilchenko
Device may support only some MAC stats. Add mapping from ids to subset
of supported MAC stats for each port.
Fixes: 73280c1e4ff ("net/sfc: support xstats retrieval by ID")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
drivers/net/sfc/sfc.h| 2 ++
drivers/net/sfc/sfc_ethdev.c | 44 ++--
drivers/net/sfc/sfc_port.c | 29 ++--
3 files changed, 46 insertions(+), 29 deletions(-)
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index c7b0e5a30d..972d32606d 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -141,6 +141,8 @@ struct sfc_port {
uint32_tmac_stats_mask[EFX_MAC_STATS_MASK_NPAGES];
+ unsigned intmac_stats_by_id[EFX_MAC_NSTATS];
+
uint64_tipackets;
};
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index fca3f524a1..ae9304f90f 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -788,8 +788,6 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
struct sfc_port *port = &sa->port;
uint64_t *mac_stats;
- unsigned int nb_supported = 0;
- unsigned int nb_written = 0;
unsigned int i;
int ret;
int rc;
@@ -808,17 +806,19 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
mac_stats = port->mac_stats_buf;
- for (i = 0; (i < EFX_MAC_NSTATS) && (nb_written < n); ++i) {
- if (!EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i))
- continue;
-
- if (ids[nb_written] == nb_supported)
- values[nb_written++] = mac_stats[i];
+ SFC_ASSERT(port->mac_stats_nb_supported <=
+ RTE_DIM(port->mac_stats_by_id));
- ++nb_supported;
+ for (i = 0; i < n; i++) {
+ if (ids[i] < port->mac_stats_nb_supported) {
+ values[i] = mac_stats[port->mac_stats_by_id[ids[i]]];
+ } else {
+ ret = i;
+ goto unlock;
+ }
}
- ret = nb_written;
+ ret = n;
unlock:
sfc_adapter_unlock(sa);
@@ -833,8 +833,7 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
{
struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
struct sfc_port *port = &sa->port;
- unsigned int nb_supported = 0;
- unsigned int nb_written = 0;
+ unsigned int nb_supported;
unsigned int i;
if (unlikely(xstats_names == NULL && ids != NULL) ||
@@ -849,23 +848,24 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
return nb_supported;
}
- for (i = 0; (i < EFX_MAC_NSTATS) && (nb_written < size); ++i) {
- if (!EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i))
- continue;
-
- if (ids[nb_written] == nb_supported) {
- char *name = xstats_names[nb_written++].name;
+ SFC_ASSERT(port->mac_stats_nb_supported <=
+ RTE_DIM(port->mac_stats_by_id));
- strlcpy(name, efx_mac_stat_name(sa->nic, i),
+ for (i = 0; i < size; i++) {
+ if (ids[i] < port->mac_stats_nb_supported) {
+ strlcpy(xstats_names[i].name,
+ efx_mac_stat_name(sa->nic,
+port->mac_stats_by_id[ids[i]]),
sizeof(xstats_names[0].name));
+ } else {
+ sfc_adapter_unlock(sa);
+ return i;
}
-
- ++nb_supported;
}
sfc_adapter_unlock(sa);
- return nb_written;
+ return size;
}
static int
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index cdc0f94f19..bb9e01d96b 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -157,6 +157,27 @@ sfc_port_phy_caps_to_max_link_speed(uint32_t phy_caps)
#endif
+static void
+sfc_port_fill_mac_stats_info(struct sfc_adapter *sa)
+{
+ unsigned int mac_stats_nb_supported = 0;
+ struct sfc_port *port = &sa->port;
+ unsigned int stat_idx;
+
+ efx_mac_stats_get_mask(sa->nic, port->mac_stats_mask,
+ sizeof(port->mac_stats_mask));
+
+ for (stat_idx = 0; stat_idx < EFX_MAC_NSTATS; ++stat_idx) {
+ if (!EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, stat_idx))
+ continue;
+
+ port->mac_stats_by_id[mac_stats_nb_supported] = stat_idx;
+ mac_stats_nb_supported++;
+ }
+
+ port->mac_stats_nb_supported = mac_stats_nb_supported;
+}
+
int
sfc_port_start(struct
[dpdk-dev] [PATCH v2 07/11] net/sfc: fix MAC stats update to work for stopped device
From: Ivan Ilchenko
Fixes: 1caab2f1e68 ("net/sfc: add basic statistics")
Cc: sta...@dpdk.org
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
drivers/net/sfc/sfc.h| 2 +-
drivers/net/sfc/sfc_ethdev.c | 6 +++---
drivers/net/sfc/sfc_port.c | 11 +++
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 972d32606d..1594f934ba 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -422,7 +422,7 @@ int sfc_port_start(struct sfc_adapter *sa);
void sfc_port_stop(struct sfc_adapter *sa);
void sfc_port_link_mode_to_info(efx_link_mode_t link_mode,
struct rte_eth_link *link_info);
-int sfc_port_update_mac_stats(struct sfc_adapter *sa);
+int sfc_port_update_mac_stats(struct sfc_adapter *sa, boolean_t manual_update);
int sfc_port_reset_mac_stats(struct sfc_adapter *sa);
int sfc_set_rx_mode(struct sfc_adapter *sa);
int sfc_set_rx_mode_unchecked(struct sfc_adapter *sa);
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index ae9304f90f..bbc22723f6 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -615,7 +615,7 @@ sfc_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats
*stats)
sfc_adapter_lock(sa);
- ret = sfc_port_update_mac_stats(sa);
+ ret = sfc_port_update_mac_stats(sa, B_FALSE);
if (ret != 0)
goto unlock;
@@ -733,7 +733,7 @@ sfc_xstats_get(struct rte_eth_dev *dev, struct
rte_eth_xstat *xstats,
sfc_adapter_lock(sa);
- rc = sfc_port_update_mac_stats(sa);
+ rc = sfc_port_update_mac_stats(sa, B_FALSE);
if (rc != 0) {
SFC_ASSERT(rc > 0);
nstats = -rc;
@@ -797,7 +797,7 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
sfc_adapter_lock(sa);
- rc = sfc_port_update_mac_stats(sa);
+ rc = sfc_port_update_mac_stats(sa, B_FALSE);
if (rc != 0) {
SFC_ASSERT(rc > 0);
ret = -rc;
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index bb9e01d96b..8c432c15f5 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -26,7 +26,8 @@
/**
* Update MAC statistics in the buffer.
*
- * @param sa Adapter
+ * @param sa Adapter
+ * @param force_uploadFlag to upload MAC stats in any case
*
* @return Status code
* @retval 0 Success
@@ -34,7 +35,7 @@
* @retval ENOMEM Memory allocation failure
*/
int
-sfc_port_update_mac_stats(struct sfc_adapter *sa)
+sfc_port_update_mac_stats(struct sfc_adapter *sa, boolean_t force_upload)
{
struct sfc_port *port = &sa->port;
efsys_mem_t *esmp = &port->mac_stats_dma_mem;
@@ -46,14 +47,14 @@ sfc_port_update_mac_stats(struct sfc_adapter *sa)
SFC_ASSERT(sfc_adapter_is_locked(sa));
if (sa->state != SFC_ADAPTER_STARTED)
- return EINVAL;
+ return 0;
/*
* If periodic statistics DMA'ing is off or if not supported,
* make a manual request and keep an eye on timer if need be
*/
if (!port->mac_stats_periodic_dma_supported ||
- (port->mac_stats_update_period_ms == 0)) {
+ (port->mac_stats_update_period_ms == 0) || force_upload) {
if (port->mac_stats_update_period_ms != 0) {
uint64_t timestamp = sfc_get_system_msecs();
@@ -367,6 +368,8 @@ sfc_port_stop(struct sfc_adapter *sa)
(void)efx_mac_stats_periodic(sa->nic, &sa->port.mac_stats_dma_mem,
0, B_FALSE);
+ sfc_port_update_mac_stats(sa, B_TRUE);
+
efx_port_fini(sa->nic);
efx_filter_fini(sa->nic);
--
2.30.2
[dpdk-dev] [PATCH v2 08/11] net/sfc: simplify getting of available xstats case
From: Ivan Ilchenko
There is no point to recalculate number of available xstats on
each request. The number is calculated once on device start
and may be returned on subsequent calls.
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
drivers/net/sfc/sfc_ethdev.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index bbc22723f6..f0567a71d0 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -733,6 +733,11 @@ sfc_xstats_get(struct rte_eth_dev *dev, struct
rte_eth_xstat *xstats,
sfc_adapter_lock(sa);
+ if (unlikely(xstats == NULL)) {
+ nstats = port->mac_stats_nb_supported;
+ goto unlock;
+ }
+
rc = sfc_port_update_mac_stats(sa, B_FALSE);
if (rc != 0) {
SFC_ASSERT(rc > 0);
@@ -744,7 +749,7 @@ sfc_xstats_get(struct rte_eth_dev *dev, struct
rte_eth_xstat *xstats,
for (i = 0; i < EFX_MAC_NSTATS; ++i) {
if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {
- if (xstats != NULL && nstats < (int)xstats_count) {
+ if (nstats < (int)xstats_count) {
xstats[nstats].id = nstats;
xstats[nstats].value = mac_stats[i];
}
@@ -768,9 +773,16 @@ sfc_xstats_get_names(struct rte_eth_dev *dev,
unsigned int i;
unsigned int nstats = 0;
+ if (unlikely(xstats_names == NULL)) {
+ sfc_adapter_lock(sa);
+ nstats = port->mac_stats_nb_supported;
+ sfc_adapter_unlock(sa);
+ return nstats;
+ }
+
for (i = 0; i < EFX_MAC_NSTATS; ++i) {
if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {
- if (xstats_names != NULL && nstats < xstats_count)
+ if (nstats < xstats_count)
strlcpy(xstats_names[nstats].name,
efx_mac_stat_name(sa->nic, i),
sizeof(xstats_names[0].name));
--
2.30.2
[dpdk-dev] [PATCH v2 09/11] net/sfc: prepare to add more xstats
From: Ivan Ilchenko
Move getting MAC stats code that involves locking to separate functions
to simplify addition of new xstats.
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
drivers/net/sfc/sfc.h| 4 ++
drivers/net/sfc/sfc_ethdev.c | 73
drivers/net/sfc/sfc_port.c | 80
3 files changed, 92 insertions(+), 65 deletions(-)
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 1594f934ba..58b8c2c2ad 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -423,6 +423,10 @@ void sfc_port_stop(struct sfc_adapter *sa);
void sfc_port_link_mode_to_info(efx_link_mode_t link_mode,
struct rte_eth_link *link_info);
int sfc_port_update_mac_stats(struct sfc_adapter *sa, boolean_t manual_update);
+int sfc_port_get_mac_stats(struct sfc_adapter *sa, struct rte_eth_xstat
*xstats,
+ unsigned int xstats_count, unsigned int *nb_written);
+int sfc_port_get_mac_stats_by_id(struct sfc_adapter *sa, const uint64_t *ids,
+uint64_t *values, unsigned int n);
int sfc_port_reset_mac_stats(struct sfc_adapter *sa);
int sfc_set_rx_mode(struct sfc_adapter *sa);
int sfc_set_rx_mode_unchecked(struct sfc_adapter *sa);
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index f0567a71d0..dd7e5c253a 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -726,41 +726,17 @@ sfc_xstats_get(struct rte_eth_dev *dev, struct
rte_eth_xstat *xstats,
{
struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
struct sfc_port *port = &sa->port;
- uint64_t *mac_stats;
- int rc;
- unsigned int i;
- int nstats = 0;
-
- sfc_adapter_lock(sa);
+ unsigned int nb_written = 0;
+ unsigned int nb_supp;
if (unlikely(xstats == NULL)) {
- nstats = port->mac_stats_nb_supported;
- goto unlock;
- }
-
- rc = sfc_port_update_mac_stats(sa, B_FALSE);
- if (rc != 0) {
- SFC_ASSERT(rc > 0);
- nstats = -rc;
- goto unlock;
- }
-
- mac_stats = port->mac_stats_buf;
-
- for (i = 0; i < EFX_MAC_NSTATS; ++i) {
- if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {
- if (nstats < (int)xstats_count) {
- xstats[nstats].id = nstats;
- xstats[nstats].value = mac_stats[i];
- }
- nstats++;
- }
+ sfc_adapter_lock(sa);
+ nb_supp = port->mac_stats_nb_supported;
+ sfc_adapter_unlock(sa);
+ return nb_supp;
}
-unlock:
- sfc_adapter_unlock(sa);
-
- return nstats;
+ return sfc_port_get_mac_stats(sa, xstats, xstats_count, &nb_written);
}
static int
@@ -798,44 +774,11 @@ sfc_xstats_get_by_id(struct rte_eth_dev *dev, const
uint64_t *ids,
uint64_t *values, unsigned int n)
{
struct sfc_adapter *sa = sfc_adapter_by_eth_dev(dev);
- struct sfc_port *port = &sa->port;
- uint64_t *mac_stats;
- unsigned int i;
- int ret;
- int rc;
if (unlikely(ids == NULL || values == NULL))
return -EINVAL;
- sfc_adapter_lock(sa);
-
- rc = sfc_port_update_mac_stats(sa, B_FALSE);
- if (rc != 0) {
- SFC_ASSERT(rc > 0);
- ret = -rc;
- goto unlock;
- }
-
- mac_stats = port->mac_stats_buf;
-
- SFC_ASSERT(port->mac_stats_nb_supported <=
- RTE_DIM(port->mac_stats_by_id));
-
- for (i = 0; i < n; i++) {
- if (ids[i] < port->mac_stats_nb_supported) {
- values[i] = mac_stats[port->mac_stats_by_id[ids[i]]];
- } else {
- ret = i;
- goto unlock;
- }
- }
-
- ret = n;
-
-unlock:
- sfc_adapter_unlock(sa);
-
- return ret;
+ return sfc_port_get_mac_stats_by_id(sa, ids, values, n);
}
static int
diff --git a/drivers/net/sfc/sfc_port.c b/drivers/net/sfc/sfc_port.c
index 8c432c15f5..f6689a17c0 100644
--- a/drivers/net/sfc/sfc_port.c
+++ b/drivers/net/sfc/sfc_port.c
@@ -636,3 +636,83 @@ sfc_port_link_mode_to_info(efx_link_mode_t link_mode,
link_info->link_autoneg = ETH_LINK_AUTONEG;
}
+
+int
+sfc_port_get_mac_stats(struct sfc_adapter *sa, struct rte_eth_xstat *xstats,
+ unsigned int xstats_count, unsigned int *nb_written)
+{
+ struct sfc_port *port = &sa->port;
+ uint64_t *mac_stats;
+ unsigned int i;
+ int nstats = 0;
+ int ret;
+
+ sfc_adapter_lock(sa);
+
+ ret = sfc_port_update_mac_stats(sa, B_FALSE);
+ if (ret != 0) {
+ SFC_ASSERT(re
[dpdk-dev] [PATCH v2 10/11] net/sfc: add xstats for Rx/Tx doorbells
From: Ivan Ilchenko
Rx/Tx doorbells statistics are collected in software and
available per queue. These stats are useful for performance
investigation.
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
Reviewed-by: Andy Moreton
---
drivers/net/sfc/meson.build| 1 +
drivers/net/sfc/sfc.c | 16 +
drivers/net/sfc/sfc.h | 9 +
drivers/net/sfc/sfc_dp.h | 10 +
drivers/net/sfc/sfc_ef10.h | 3 +-
drivers/net/sfc/sfc_ef100_rx.c | 1 +
drivers/net/sfc/sfc_ef100_tx.c | 1 +
drivers/net/sfc/sfc_ef10_essb_rx.c | 3 +-
drivers/net/sfc/sfc_ef10_rx.c | 3 +-
drivers/net/sfc/sfc_ef10_tx.c | 1 +
drivers/net/sfc/sfc_ethdev.c | 124 +--
drivers/net/sfc/sfc_port.c | 10 +-
drivers/net/sfc/sfc_rx.c | 1 +
drivers/net/sfc/sfc_sw_stats.c | 572 +
drivers/net/sfc/sfc_sw_stats.h | 49 +++
drivers/net/sfc/sfc_tx.c | 4 +-
16 files changed, 772 insertions(+), 36 deletions(-)
create mode 100644 drivers/net/sfc/sfc_sw_stats.c
create mode 100644 drivers/net/sfc/sfc_sw_stats.h
diff --git a/drivers/net/sfc/meson.build b/drivers/net/sfc/meson.build
index 4625859077..a912cdccfa 100644
--- a/drivers/net/sfc/meson.build
+++ b/drivers/net/sfc/meson.build
@@ -70,6 +70,7 @@ sources = files(
'sfc.c',
'sfc_mcdi.c',
'sfc_sriov.c',
+'sfc_sw_stats.c',
'sfc_intr.c',
'sfc_ev.c',
'sfc_port.c',
diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index 4097cf39de..274a98e228 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -24,6 +24,7 @@
#include "sfc_tx.h"
#include "sfc_kvargs.h"
#include "sfc_tweak.h"
+#include "sfc_sw_stats.h"
int
@@ -636,10 +637,17 @@ sfc_configure(struct sfc_adapter *sa)
if (rc != 0)
goto fail_tx_configure;
+ rc = sfc_sw_xstats_configure(sa);
+ if (rc != 0)
+ goto fail_sw_xstats_configure;
+
sa->state = SFC_ADAPTER_CONFIGURED;
sfc_log_init(sa, "done");
return 0;
+fail_sw_xstats_configure:
+ sfc_tx_close(sa);
+
fail_tx_configure:
sfc_rx_close(sa);
@@ -666,6 +674,7 @@ sfc_close(struct sfc_adapter *sa)
SFC_ASSERT(sa->state == SFC_ADAPTER_CONFIGURED);
sa->state = SFC_ADAPTER_CLOSING;
+ sfc_sw_xstats_close(sa);
sfc_tx_close(sa);
sfc_rx_close(sa);
sfc_port_close(sa);
@@ -891,6 +900,10 @@ sfc_attach(struct sfc_adapter *sa)
sfc_flow_init(sa);
+ rc = sfc_sw_xstats_init(sa);
+ if (rc != 0)
+ goto fail_sw_xstats_init;
+
/*
* Create vSwitch to be able to use VFs when PF is not started yet
* as DPDK port. VFs should be able to talk to each other even
@@ -906,6 +919,9 @@ sfc_attach(struct sfc_adapter *sa)
return 0;
fail_sriov_vswitch_create:
+ sfc_sw_xstats_close(sa);
+
+fail_sw_xstats_init:
sfc_flow_fini(sa);
sfc_mae_detach(sa);
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 58b8c2c2ad..331e06bac6 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -217,6 +217,14 @@ struct sfc_counter_rxq {
struct rte_mempool *mp;
};
+struct sfc_sw_xstats {
+ uint64_t*reset_vals;
+
+ rte_spinlock_t queues_bitmap_lock;
+ void*queues_bitmap_mem;
+ struct rte_bitmap *queues_bitmap;
+};
+
/* Adapter private data */
struct sfc_adapter {
/*
@@ -249,6 +257,7 @@ struct sfc_adapter {
struct sfc_sriovsriov;
struct sfc_intr intr;
struct sfc_port port;
+ struct sfc_sw_xstatssw_xstats;
struct sfc_filter filter;
struct sfc_mae mae;
diff --git a/drivers/net/sfc/sfc_dp.h b/drivers/net/sfc/sfc_dp.h
index 61c1a3fbac..7fd8f34b0f 100644
--- a/drivers/net/sfc/sfc_dp.h
+++ b/drivers/net/sfc/sfc_dp.h
@@ -42,6 +42,16 @@ enum sfc_dp_type {
/** Datapath queue run-time information */
struct sfc_dp_queue {
+ /*
+* Typically the structure is located at the end of Rx/Tx queue
+* data structure and not used on datapath. So, it is not a
+* problem to have extra fields even if not used. However,
+* put stats at top of the structure to be closer to fields
+* used on datapath or reap to have more chances to be cache-hot.
+*/
+ uint32_trx_dbells;
+ uint32_ttx_dbells;
+
uint16_tport_id;
uint16_tqueue_id;
struct rte_pci_addr pci_addr;
diff --git a/drivers/net/sfc/sfc_ef10.h b/drivers/net/sfc/sfc_ef10.h
index ad4c1fdbef..e9bb72e28b 100644
--- a/drivers/net/sfc/sfc_ef10.h
+++ b/dri
[dpdk-dev] [PATCH v2 11/11] app/testpmd: add option to display extended statistics
From: Ivan Ilchenko
Add 'display-xstats' option for using in accompanying with Rx/Tx statistics
(i.e. 'stats-period' option or 'show port stats' interactive command) to
display specified list of extended statistics.
Signed-off-by: Ivan Ilchenko
Signed-off-by: Andrew Rybchenko
---
app/test-pmd/cmdline.c| 56
app/test-pmd/config.c | 66 ++
app/test-pmd/parameters.c | 18
app/test-pmd/testpmd.c| 122 ++
app/test-pmd/testpmd.h| 21 +
doc/guides/testpmd_app_ug/run_app.rst | 5 ++
6 files changed, 288 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 8468018cf3..baffef1642 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -3609,6 +3609,62 @@ cmdline_parse_inst_t cmd_stop = {
/* *** SET CORELIST and PORTLIST CONFIGURATION *** */
+int
+parse_xstats_list(char *in_str, struct rte_eth_xstat_name **xstats,
+ unsigned int *xstats_num)
+{
+ int max_names_nb, names_nb;
+ int stringlen;
+ char **names;
+ char *str;
+ int ret;
+ int i;
+
+ names = NULL;
+ str = strdup(in_str);
+ if (str == NULL) {
+ ret = ENOMEM;
+ goto out;
+ }
+ stringlen = strlen(str);
+
+ for (i = 0, max_names_nb = 1; str[i] != '\0'; i++) {
+ if (str[i] == ',')
+ max_names_nb++;
+ }
+
+ names = calloc(max_names_nb, sizeof(*names));
+ if (names == NULL) {
+ ret = ENOMEM;
+ goto out;
+ }
+
+ names_nb = rte_strsplit(str, stringlen, names, max_names_nb, ',');
+ printf("max names is %d\n", max_names_nb);
+ if (names_nb < 0) {
+ ret = EINVAL;
+ goto out;
+ }
+
+ *xstats = calloc(names_nb, sizeof(**xstats));
+ if (*xstats == NULL) {
+ ret = ENOMEM;
+ goto out;
+ }
+
+ for (i = 0; i < names_nb; i++)
+ rte_strscpy((*xstats)[i].name, names[i],
+ sizeof((*xstats)[i].name));
+
+ *xstats_num = names_nb;
+ ret = 0;
+
+out:
+ free(names);
+ free(str);
+ return ret;
+}
+
unsigned int
parse_item_list(const char *str, const char *item_name, unsigned int max_items,
unsigned int *parsed_items, int check_unique_values)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 04ae0feb58..6d604145bd 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -173,6 +173,70 @@ print_ethaddr(const char *name, struct rte_ether_addr
*eth_addr)
printf("%s%s", name, buf);
}
+static void
+nic_xstats_display_periodic(portid_t port_id)
+{
+ struct xstat_display_info *xstats_info;
+ uint64_t *prev_values, *curr_values;
+ uint64_t diff_value, value_rate;
+ uint64_t *ids, *ids_supp;
+ struct timespec cur_time;
+ unsigned int i, i_supp;
+ size_t ids_supp_sz;
+ uint64_t diff_ns;
+ int rc;
+
+ xstats_info = &xstats_per_port[port_id];
+
+ ids_supp_sz = xstats_info->ids_supp_sz;
+ if (xstats_display_num == 0 || ids_supp_sz == 0)
+ return;
+
+ printf("\n");
+
+ ids = xstats_info->ids;
+ ids_supp = xstats_info->ids_supp;
+ prev_values = xstats_info->prev_values;
+ curr_values = xstats_info->curr_values;
+
+ rc = rte_eth_xstats_get_by_id(port_id, ids_supp, curr_values,
+ ids_supp_sz);
+ if (rc != (int)ids_supp_sz) {
+ fprintf(stderr, "%s: Failed to get values of %zu supported
xstats for port %u - return code %d\n",
+ __func__, ids_supp_sz, port_id, rc);
+ return;
+ }
+
+ diff_ns = 0;
+ if (clock_gettime(CLOCK_TYPE_ID, &cur_time) == 0) {
+ uint64_t ns;
+
+ ns = cur_time.tv_sec * NS_PER_SEC;
+ ns += cur_time.tv_nsec;
+
+ if (xstats_info->prev_ns != 0)
+ diff_ns = ns - xstats_info->prev_ns;
+ xstats_info->prev_ns = ns;
+ }
+
+ printf("%-31s%-17s%s\n", " ", "Value", "Rate (since last show)");
+ for (i = i_supp = 0; i < xstats_display_num; i++) {
+ if (ids[i] == XSTAT_ID_INVALID)
+ continue;
+
+ diff_value = (curr_values[i_supp] > prev_values[i]) ?
+(curr_values[i_supp] - prev_values[i]) : 0;
+ prev_values[i] = curr_values[i_supp];
+ value_rate = diff_ns > 0 ?
+ (double)diff_value / diff_ns * NS_PER_SEC : 0;
+
+ printf(" %-25s%12"PRIu64" %15"PRIu64"\n",
+ xstats_display[i].name, curr_values[i_supp], value_rate);
+
+ i_supp++;
+ }
+}
+
void
nic_stats_display(port
Re: [dpdk-dev] [PATCH 1/4] ethdev: fix max Rx packet length
On 7/22/2021 8:21 AM, Huisong Li wrote:
>
> 在 2021/7/21 23:29, Ferruh Yigit 写道:
>> On 7/19/2021 4:35 AM, Huisong Li wrote:
>>> Hi, Ferruh
>>>
>> Hi Huisong,
>>
>> Thanks for the review.
>>
>>> 在 2021/7/10 1:29, Ferruh Yigit 写道:
There is a confusion on setting max Rx packet length, this patch aims to
clarify it.
'rte_eth_dev_configure()' API accepts max Rx packet size via
'uint32_t max_rx_pkt_len' filed of the config struct 'struct
rte_eth_conf'.
Also 'rte_eth_dev_set_mtu()' API can be used to set the MTU, and result
stored into '(struct rte_eth_dev)->data->mtu'.
These two APIs are related but they work in a disconnected way, they
store the set values in different variables which makes hard to figure
out which one to use, also two different related method is confusing for
the users.
Other issues causing confusion is:
* maximum transmission unit (MTU) is payload of the Ethernet frame. And
'max_rx_pkt_len' is the size of the Ethernet frame. Difference is
Ethernet frame overhead, but this may be different from device to
device based on what device supports, like VLAN and QinQ.
* 'max_rx_pkt_len' is only valid when application requested jumbo frame,
which adds additional confusion and some APIs and PMDs already
discards this documented behavior.
* For the jumbo frame enabled case, 'max_rx_pkt_len' is an mandatory
field, this adds configuration complexity for application.
As solution, both APIs gets MTU as parameter, and both saves the result
in same variable '(struct rte_eth_dev)->data->mtu'. For this
'max_rx_pkt_len' updated as 'mtu', and it is always valid independent
from jumbo frame.
For 'rte_eth_dev_configure()', 'dev->data->dev_conf.rxmode.mtu' is user
request and it should be used only within configure function and result
should be stored to '(struct rte_eth_dev)->data->mtu'. After that point
both application and PMD uses MTU from this variable.
When application doesn't provide an MTU during 'rte_eth_dev_configure()'
default 'RTE_ETHER_MTU' value is used.
As additional clarification, MTU is used to configure the device for
physical Rx/Tx limitation. Other related issue is size of the buffer to
store Rx packets, many PMDs use mbuf data buffer size as Rx buffer size.
And compares MTU against Rx buffer size to decide enabling scattered Rx
or not, if PMD supports it. If scattered Rx is not supported by device,
MTU bigger than Rx buffer size should fail.
Signed-off-by: Ferruh Yigit
>> <...>
>>
diff --git a/drivers/net/hns3/hns3_ethdev.c
b/drivers/net/hns3/hns3_ethdev.c
index e51512560e15..8bccdeddb2f7 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2379,20 +2379,11 @@ hns3_refresh_mtu(struct rte_eth_dev *dev, struct
rte_eth_conf *conf)
{
struct hns3_adapter *hns = dev->data->dev_private;
struct hns3_hw *hw = &hns->hw;
- uint32_t max_rx_pkt_len;
- uint16_t mtu;
- int ret;
-
- if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME))
- return 0;
+ uint32_t max_rx_pktlen;
- /*
- * If jumbo frames are enabled, MTU needs to be refreshed
- * according to the maximum RX packet length.
- */
- max_rx_pkt_len = conf->rxmode.max_rx_pkt_len;
- if (max_rx_pkt_len > HNS3_MAX_FRAME_LEN ||
- max_rx_pkt_len <= HNS3_DEFAULT_FRAME_LEN) {
+ max_rx_pktlen = conf->rxmode.mtu + HNS3_ETH_OVERHEAD;
+ if (max_rx_pktlen > HNS3_MAX_FRAME_LEN ||
+ max_rx_pktlen <= HNS3_DEFAULT_FRAME_LEN) {
hns3_err(hw, "maximum Rx packet length must be greater than %u "
"and no more than %u when jumbo frame enabled.",
(uint16_t)HNS3_DEFAULT_FRAME_LEN,
>>> The preceding check for the maximum frame length was based on the scenario
>>> where
>>> jumbo frames are enabled.
>>>
>>> Since there is no offload of jumbo frames in this patchset, the maximum
>>> frame
>>> length does not need to be checked and only ensure conf->rxmode.mtu is
>>> valid.
>>>
>>> These should be guaranteed by dev_configure() in the framework .
>>>
>> Got it, agree that 'HNS3_DEFAULT_FRAME_LEN' check is now wrong, and as you
>> said
>> these checks are becoming redundant, so I will remove them.
>>
>> In that case 'hns3_refresh_mtu()' becomes just wrapper to
>> 'hns3_dev_mtu_set()',
>> I will remove function too.
>>
>> <...>
> ok
>>
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c
b/drivers/net/hns3/hns3_ethdev_vf.c
index e582503f529b..ca839fa55fa0 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -784,8 +784,7 @@ hns3vf_dev_configure(struct rte_eth_d
Re: [dpdk-dev] [PATCH 1/4] ethdev: fix max Rx packet length
On 7/22/21 1:12 PM, Ferruh Yigit wrote:
On 7/22/2021 8:21 AM, Huisong Li wrote:
在 2021/7/21 23:29, Ferruh Yigit 写道:
On 7/19/2021 4:35 AM, Huisong Li wrote:
Hi, Ferruh
Hi Huisong,
Thanks for the review.
在 2021/7/10 1:29, Ferruh Yigit 写道:
There is a confusion on setting max Rx packet length, this patch aims to
clarify it.
'rte_eth_dev_configure()' API accepts max Rx packet size via
'uint32_t max_rx_pkt_len' filed of the config struct 'struct
rte_eth_conf'.
Also 'rte_eth_dev_set_mtu()' API can be used to set the MTU, and result
stored into '(struct rte_eth_dev)->data->mtu'.
These two APIs are related but they work in a disconnected way, they
store the set values in different variables which makes hard to figure
out which one to use, also two different related method is confusing for
the users.
Other issues causing confusion is:
* maximum transmission unit (MTU) is payload of the Ethernet frame. And
'max_rx_pkt_len' is the size of the Ethernet frame. Difference is
Ethernet frame overhead, but this may be different from device to
device based on what device supports, like VLAN and QinQ.
* 'max_rx_pkt_len' is only valid when application requested jumbo frame,
which adds additional confusion and some APIs and PMDs already
discards this documented behavior.
* For the jumbo frame enabled case, 'max_rx_pkt_len' is an mandatory
field, this adds configuration complexity for application.
As solution, both APIs gets MTU as parameter, and both saves the result
in same variable '(struct rte_eth_dev)->data->mtu'. For this
'max_rx_pkt_len' updated as 'mtu', and it is always valid independent
from jumbo frame.
For 'rte_eth_dev_configure()', 'dev->data->dev_conf.rxmode.mtu' is user
request and it should be used only within configure function and result
should be stored to '(struct rte_eth_dev)->data->mtu'. After that point
both application and PMD uses MTU from this variable.
When application doesn't provide an MTU during 'rte_eth_dev_configure()'
default 'RTE_ETHER_MTU' value is used.
As additional clarification, MTU is used to configure the device for
physical Rx/Tx limitation. Other related issue is size of the buffer to
store Rx packets, many PMDs use mbuf data buffer size as Rx buffer size.
And compares MTU against Rx buffer size to decide enabling scattered Rx
or not, if PMD supports it. If scattered Rx is not supported by device,
MTU bigger than Rx buffer size should fail.
Signed-off-by: Ferruh Yigit
<...>
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index e51512560e15..8bccdeddb2f7 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2379,20 +2379,11 @@ hns3_refresh_mtu(struct rte_eth_dev *dev, struct
rte_eth_conf *conf)
{
struct hns3_adapter *hns = dev->data->dev_private;
struct hns3_hw *hw = &hns->hw;
- uint32_t max_rx_pkt_len;
- uint16_t mtu;
- int ret;
-
- if (!(conf->rxmode.offloads & DEV_RX_OFFLOAD_JUMBO_FRAME))
- return 0;
+ uint32_t max_rx_pktlen;
- /*
- * If jumbo frames are enabled, MTU needs to be refreshed
- * according to the maximum RX packet length.
- */
- max_rx_pkt_len = conf->rxmode.max_rx_pkt_len;
- if (max_rx_pkt_len > HNS3_MAX_FRAME_LEN ||
- max_rx_pkt_len <= HNS3_DEFAULT_FRAME_LEN) {
+ max_rx_pktlen = conf->rxmode.mtu + HNS3_ETH_OVERHEAD;
+ if (max_rx_pktlen > HNS3_MAX_FRAME_LEN ||
+ max_rx_pktlen <= HNS3_DEFAULT_FRAME_LEN) {
hns3_err(hw, "maximum Rx packet length must be greater than %u "
"and no more than %u when jumbo frame enabled.",
(uint16_t)HNS3_DEFAULT_FRAME_LEN,
The preceding check for the maximum frame length was based on the scenario where
jumbo frames are enabled.
Since there is no offload of jumbo frames in this patchset, the maximum frame
length does not need to be checked and only ensure conf->rxmode.mtu is valid.
These should be guaranteed by dev_configure() in the framework .
Got it, agree that 'HNS3_DEFAULT_FRAME_LEN' check is now wrong, and as you said
these checks are becoming redundant, so I will remove them.
In that case 'hns3_refresh_mtu()' becomes just wrapper to 'hns3_dev_mtu_set()',
I will remove function too.
<...>
ok
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c
b/drivers/net/hns3/hns3_ethdev_vf.c
index e582503f529b..ca839fa55fa0 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -784,8 +784,7 @@ hns3vf_dev_configure(struct rte_eth_dev *dev)
uint16_t nb_rx_q = dev->data->nb_rx_queues;
uint16_t nb_tx_q = dev->data->nb_tx_queues;
struct rte_eth_rss_conf rss_conf;
- uint32_t max_rx_pkt_len;
- uint16_t mtu;
+ uint32_t max_rx_pktlen;
bool gro_en;
int ret;
@@ -825,29 +824,21 @@ hns3vf_dev_configure(struct rte_eth_dev *dev)
goto cfg_err;
}
- /*
- * If jumbo frames are enabled, MTU needs to be
Re: [dpdk-dev] [PATCH 1/4] ethdev: fix max Rx packet length
On 7/22/2021 2:31 AM, Ajit Khaparde wrote:
>
>
>
> > [snip]
> >
> >> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> >> index faf3bd901d75..9f288f98329c 100644
> >> --- a/lib/ethdev/rte_ethdev.h
> >> +++ b/lib/ethdev/rte_ethdev.h
> >> @@ -410,7 +410,7 @@ enum rte_eth_tx_mq_mode {
> >> struct rte_eth_rxmode {
> >> /** The multi-queue packet distribution mode to be used, e.g.
> RSS. */
> >> enum rte_eth_rx_mq_mode mq_mode;
> >> - uint32_t max_rx_pkt_len; /**< Only used if JUMBO_FRAME enabled.
> */
> >> + uint32_t mtu; /**< Requested MTU. */
> >
> > Maximum Transmit Unit looks a bit confusing in Rx mode
> > structure.
> >
>
> True, but I think it is already used for Rx already as concept, I believe
> the
> intention will be clear enough. Do you think will be more clear if we
> pick a
> DPDK specific variable name?
>
> Maybe use MRU - Max Receive Unit.
>
It can be an option, but this patch unifies 'max_rx_pkt_len' & 'mtu' => mtu,
if we switch to 'mru', we should switch all usage to 'mru', including
'rte_eth_dev_set_mtu()' API name change, to not cause a new confusion between
'mru' & 'mtu' difference.
Does 'mtu' really cause this much confusion to do all this change?
>
> >> /** Maximum allowed size of LRO aggregated packet. */
> >> uint32_t max_lro_pkt_size;
> >> uint16_t split_hdr_size; /**< hdr buf size (header_split
> enabled).*/
> >
> > [snip]
> >
>
Re: [dpdk-dev] [PATCH 1/4] doc: clarify RTE flow behaviour on port stop/start
On 7/21/21 6:55 PM, Martin Havlik wrote: It is now clearly stated that RTE flow rules can be created only after the port is started. Signed-off-by: Martin Havlik --- doc/guides/nics/mlx5.rst | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index f5b727c1ee..119d537adf 100644 --- a/doc/guides/nics/mlx5.rst +++ b/doc/guides/nics/mlx5.rst @@ -1790,21 +1790,25 @@ Notes for rte_flow -- Flows are not cached in the driver. When stopping a device port, all the flows created on this port from the application will be flushed automatically in the background. After stopping the device port, all flows on this port become invalid and not represented in the system. All references to these flows held by the application should be discarded directly but neither destroyed nor flushed. -The application should re-create the flows as required after the port restart. +The application should re-create the flows as required after the port is +started again. + +Creating flows before port start is not permitted. All flows the application +wants to create have to be created after the port is started. I'm not 100% sure that it is always OK for applications, but in an attempt to make it OK we should: - mention isolated mode if application dislikes default flow rules and would like to control it - mention what happens if restart happens internally, e.g. in order to recover from broken state. I guess in this case we need an event and application must register callback and handle it. Notes for testpmd - Compared to librte_net_mlx4 that implements a single RSS configuration per port, librte_net_mlx5 supports per-protocol RSS configuration. Since ``testpmd`` defaults to IP RSS mode and there is currently no command-line parameter to enable additional protocols (UDP and TCP as well as IP), the following commands must be entered from its CLI to get the same
Re: [dpdk-dev] [PATCH 2/4] doc: specify RTE flow create behaviour
On 7/21/21 9:16 PM, Stephen Hemminger wrote: On Wed, 21 Jul 2021 17:58:14 +0200 Martin Havlik wrote: The ability to create RTE flow rules, depending on port status, can and does differ between PMDs. Now the doc reflects that. Signed-off-by: Martin Havlik --- doc/guides/prog_guide/rte_flow.rst | 4 1 file changed, 4 insertions(+) diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst index 2b42d5ec8c..2988e3328a 100644 --- a/doc/guides/prog_guide/rte_flow.rst +++ b/doc/guides/prog_guide/rte_flow.rst @@ -3097,6 +3097,10 @@ actually created and a handle returned. const struct rte_flow_action *actions[], struct rte_flow_error *error); +The ability to create a flow rule may depend on the status (started/stopped) +of the port for which the rule is being created. This behaviour is +PMD specific. Seek relevant PMD documentation for details. Any PMD specific behavior in DPDK is an anathema to application developers and should be considered a design flaw! +1
Re: [dpdk-dev] imissed drop with mellanox connectx5
Hi Matan We work with mbuf in all threads and lcores, We pass them from one thread to another through the dpdk ring before releasing them. There are drops in 10K to 100K pps, we can't stay with these drops. The drops are in the imissed counter from rte_eth_stats_get, so I thought that the drops are at the port level and not drop at mempool level From what I see number of mbuf in pool is stable( and close to the total/original number of mbuf in pool), the rings are empty, Traffic is well balanced between threads, All threads are running in pool from port and from ring. And from perf top profiler there doesn't seem to be any unexpected function taking cpu. So the only possible architecture would be to implement all logic in the threads that read from port, and to launch hundreds of threads in multiqueue mode that read from port? I don't think this is a viable solution ( In the following link for example they show an example of application that pass packet from one core/thread to another https://doc.dpdk.org/guides-16.04/sample_app_ug/qos_scheduler.html ) Thank you answer -Original Message- From: Matan Azrad Sent: Thursday, July 22, 2021 8:19 AM To: Yaron Illouz ; us...@dpdk.org Cc: dev@dpdk.org Subject: RE: imissed drop with mellanox connectx5 Hi Yaron Freeing mbufs from a different lcore than the original lcore allocated them causes cache miss in the mempool cache of the original lcore per mbuf allocation - all the time the PMD will get non-hot mbufs to work with. It can be one of the reasons for the earlier drops you see. Matan From: Yaron Illouz > Hi > > We try to read from 100G NIC Mellanox ConnectX-5 without drop at nic. > All thread are with core pinning and cpu isolation. > We use dpdk 19.11 > I tried to apply all configuration that are in > https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Ffast > .dpdk.org%2Fdoc%2Fperf%2FDPDK_19_08_Mellanox_NIC_performance_r&dat > a=04%7C01%7C%7Cdcbb2d8246be4dc456c508d94cd038a7%7C0eb9e2d98763412e9709 > 3f539e9e25bc%7C0%7C0%7C637625279453292671%7CUnknown%7CTWFpbGZsb3d8eyJW > IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000& > amp;sdata=KMBFyIMEFV4B0JqxQE%2BiMXJ2p9qE8lEOpUWRsFhD0gM%3D&reserve > d=0 > eport.pdf > > We have a strange behavior, 1 thread can receive receive 20 Gbps/12 > Mpps and free mbuf without dropps, but when trying to pass these mbuf > to another thread that only free them there are drops, even when > trying to work with more threads. > > When running 1 thread that only read from port (no multi queue) and > free mbuf in the same thread, there are no dropp with traffic up to 21 > Gbps 12.4 Mpps. > When running 6 thread that only read from port (with multi queue) and > free mbuf in the same threads, there are no dropp with traffic up to > 21 Gbps 12.4 Mpps. > > When running 1 to 6 thread that only read from port and pass them to > another 6 thread that only read from ring and free mbuf, there are > dropp in nic (imissed counter) with traffic over to 10 Gbps 5.2 > Mpps.(Here receive thread were pinned to cpu 1-6 and additional thread > from 7-12 each thread on a single cpu) Each receive thread send to one thread > that free the buffer. > > Configurations: > > We use rings of size 32768 between the threads. Ring are initialized > with SP/SC, Write are done with bulk of 512 with rte_ring_enqueue_burst. > Port is initialized with rte_eth_rx_queue_setup nb_rx_desc=8192 > rte_eth_rxconf - rx_conf.rx_thresh.pthresh = DPDK_NIC_RX_PTHRESH; > //ring prefetch threshold > rx_conf.rx_thresh.hthresh = > DPDK_NIC_RX_HTHRESH; //ring host threshold > rx_conf.rx_thresh.wthresh = > DPDK_NIC_RX_WTHRESH; //ring writeback threshold > rx_conf.rx_free_thresh = > DPDK_NIC_RX_FREE_THRESH; rss - > > ETH_RSS_IP | ETH_RSS_UDP | ETH_RSS_TCP; > > > We tried to work with and without hyperthreading. > > > > Network devices using kernel driver > === > :37:00.0 'MT27800 Family [ConnectX-5] 1017' if=ens2f0 > drv=mlx5_core unused=igb_uio > :37:00.1 'MT27800 Family [ConnectX-5] 1017' if=ens2f1 > drv=mlx5_core unused=igb_uio > > > > ethtool -i ens2f0 > driver: mlx5_core > version: 5.3-1.0.0 > firmware-version: 16.30.1004 (HPE09) > expansion-rom-version: > bus-info: :37:00.0 > supports-statistics: yes > supports-test: yes > supports-eeprom-access: no > supports-register-dump: no > supports-priv-flags: yes > > > > uname -a > Linux localhost.localdomain 3.10.0-1160.el7.x86_64 #1 SMP Mon Oct 19 > 16:18:59 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux > > > > lscpu | grep -e Socket -e Core -e Thread > Thread(s) per core:1 > Core(s) per socket:24 > Socket(s): 2 > >
Re: [dpdk-dev] [PATCH 1/4] ethdev: fix max Rx packet length
On 7/22/21 1:27 PM, Ferruh Yigit wrote:
On 7/22/2021 2:31 AM, Ajit Khaparde wrote:
> [snip]
>
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index faf3bd901d75..9f288f98329c 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -410,7 +410,7 @@ enum rte_eth_tx_mq_mode {
>> struct rte_eth_rxmode {
>> /** The multi-queue packet distribution mode to be used, e.g. RSS.
*/
>> enum rte_eth_rx_mq_mode mq_mode;
>> - uint32_t max_rx_pkt_len; /**< Only used if JUMBO_FRAME enabled. */
>> + uint32_t mtu; /**< Requested MTU. */
>
> Maximum Transmit Unit looks a bit confusing in Rx mode
> structure.
>
True, but I think it is already used for Rx already as concept, I believe
the
intention will be clear enough. Do you think will be more clear if we pick
a
DPDK specific variable name?
Maybe use MRU - Max Receive Unit.
It can be an option, but this patch unifies 'max_rx_pkt_len' & 'mtu' => mtu,
if we switch to 'mru', we should switch all usage to 'mru', including
'rte_eth_dev_set_mtu()' API name change, to not cause a new confusion between
'mru' & 'mtu' difference.
Does 'mtu' really cause this much confusion to do all this change?
Reconsidering it I see no better options. Yes, mtu is a bit confusing
in Rx configuration, but just a bit.
>> /** Maximum allowed size of LRO aggregated packet. */
>> uint32_t max_lro_pkt_size;
>> uint16_t split_hdr_size; /**< hdr buf size (header_split
enabled).*/
>
> [snip]
>
[dpdk-dev] [PATCH v3] app/procinfo: add device registers dump
From: Chengchang Tang
This patch add support for dump the device registers from a running
application. It can help developers locate the problem.
Signed-off-by: Chengchang Tang
Signed-off-by: Min Hu (Connor)
---
v3:
* delete memset of dev_info.
v2:
* some logs are adjusted and error string are printed after
file operation fails.
---
app/proc-info/main.c | 92 +++-
1 file changed, 91 insertions(+), 1 deletion(-)
diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index b9587f7..e85d997 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -94,6 +94,9 @@ static char *mempool_name;
/**< Enable iter mempool. */
static uint32_t enable_iter_mempool;
static char *mempool_iter_name;
+/**< Enable dump regs. */
+static uint32_t enable_dump_regs;
+static char *dump_regs_file_prefix;
/**< display usage */
static void
@@ -119,7 +122,8 @@ proc_info_usage(const char *prgname)
" --show-crypto: to display crypto information\n"
" --show-ring[=name]: to display ring information\n"
" --show-mempool[=name]: to display mempool information\n"
- " --iter-mempool=name: iterate mempool elements to display
content\n",
+ " --iter-mempool=name: iterate mempool elements to display
content\n"
+ " --dump-regs=file-prefix: dump reg to file with the
file-prefix\n",
prgname);
}
@@ -226,6 +230,7 @@ proc_info_parse_args(int argc, char **argv)
{"show-ring", optional_argument, NULL, 0},
{"show-mempool", optional_argument, NULL, 0},
{"iter-mempool", required_argument, NULL, 0},
+ {"dump-regs", required_argument, NULL, 0},
{NULL, 0, 0, 0}
};
@@ -288,6 +293,10 @@ proc_info_parse_args(int argc, char **argv)
"iter-mempool", MAX_LONG_OPT_SZ)) {
enable_iter_mempool = 1;
mempool_iter_name = optarg;
+ } else if (!strncmp(long_option[option_index].name,
+ "dump-regs", MAX_LONG_OPT_SZ)) {
+ enable_dump_regs = 1;
+ dump_regs_file_prefix = optarg;
}
break;
case 1:
@@ -1349,6 +1358,85 @@ iter_mempool(char *name)
}
}
+static void
+dump_regs(char *file_prefix)
+{
+#define MAX_FILE_NAME_SZ (MAX_LONG_OPT_SZ + 10)
+ char file_name[MAX_FILE_NAME_SZ];
+ struct rte_dev_reg_info reg_info;
+ struct rte_eth_dev_info dev_info;
+ unsigned char *buf_data;
+ size_t buf_size;
+ FILE *fp_regs;
+ uint16_t i;
+ int ret;
+
+ snprintf(bdr_str, MAX_STRING_LEN, " dump - Port REG");
+ STATS_BDR_STR(10, bdr_str);
+
+ RTE_ETH_FOREACH_DEV(i) {
+ /* Skip if port is not in mask */
+ if ((enabled_port_mask & (1ul << i)) == 0)
+ continue;
+
+ snprintf(bdr_str, MAX_STRING_LEN, " Port (%u)", i);
+ STATS_BDR_STR(5, bdr_str);
+
+ ret = rte_eth_dev_info_get(i, &dev_info);
+ if (ret) {
+ printf("Error getting device info: %d\n", ret);
+ continue;
+ }
+
+ memset(®_info, 0, sizeof(reg_info));
+ ret = rte_eth_dev_get_reg_info(i, ®_info);
+ if (ret) {
+ printf("Error getting device reg info: %d\n", ret);
+ continue;
+ }
+
+ buf_size = reg_info.length * reg_info.width;
+ buf_data = malloc(buf_size);
+ if (buf_data == NULL) {
+ printf("Error allocating %zu bytes buffer\n", buf_size);
+ continue;
+ }
+
+ reg_info.data = buf_data;
+ reg_info.length = 0;
+ ret = rte_eth_dev_get_reg_info(i, ®_info);
+ if (ret) {
+ printf("Error getting regs from device: %d\n", ret);
+ free(buf_data);
+ continue;
+ }
+
+ snprintf(file_name, MAX_FILE_NAME_SZ, "%s-port%u",
+ file_prefix, i);
+ fp_regs = fopen(file_name, "wb");
+ if (fp_regs == NULL) {
+ printf("Error during opening '%s' for writing: %s\n",
+ file_name, strerror(errno));
+ } else {
+ size_t nr_written;
+
+ nr_written = fwrite(buf_data, 1, buf_size, fp_regs);
+ if (nr_written != buf_size)
+ printf("Error during writing %s: %s\n",
+ file_prefix, strer
[dpdk-dev] [PATCH v2] net/mlx5: fix indexed pools allocation
Currently, the flow indexed pools are allocated per port, the allocation
was missing in Windows code.
Allocate indexed pool for the Windows case too.
Fixes: b4edeaf3efd5 ("net/mlx5: replace flow list with indexed pool")
Signed-off-by: Suanming Mou
Acked-by: Tal Shnaiderman
Acked-by: Matan Azrad
Tested-by: Odi Assli
---
v2: commit message updated.
---
drivers/net/mlx5/windows/mlx5_os.c | 47 ++
1 file changed, 47 insertions(+)
diff --git a/drivers/net/mlx5/windows/mlx5_os.c
b/drivers/net/mlx5/windows/mlx5_os.c
index 5da362a9d5..a31fafc90d 100644
--- a/drivers/net/mlx5/windows/mlx5_os.c
+++ b/drivers/net/mlx5/windows/mlx5_os.c
@@ -35,6 +35,44 @@ static const char *MZ_MLX5_PMD_SHARED_DATA =
"mlx5_pmd_shared_data";
/* Spinlock for mlx5_shared_data allocation. */
static rte_spinlock_t mlx5_shared_data_lock = RTE_SPINLOCK_INITIALIZER;
+/* rte flow indexed pool configuration. */
+static struct mlx5_indexed_pool_config icfg[] = {
+ {
+ .size = sizeof(struct rte_flow),
+ .trunk_size = 64,
+ .need_lock = 1,
+ .release_mem_en = 0,
+ .malloc = mlx5_malloc,
+ .free = mlx5_free,
+ .per_core_cache = 0,
+ .type = "ctl_flow_ipool",
+ },
+ {
+ .size = sizeof(struct rte_flow),
+ .trunk_size = 64,
+ .grow_trunk = 3,
+ .grow_shift = 2,
+ .need_lock = 1,
+ .release_mem_en = 0,
+ .malloc = mlx5_malloc,
+ .free = mlx5_free,
+ .per_core_cache = 1 << 14,
+ .type = "rte_flow_ipool",
+ },
+ {
+ .size = sizeof(struct rte_flow),
+ .trunk_size = 64,
+ .grow_trunk = 3,
+ .grow_shift = 2,
+ .need_lock = 1,
+ .release_mem_en = 0,
+ .malloc = mlx5_malloc,
+ .free = mlx5_free,
+ .per_core_cache = 0,
+ .type = "mcp_flow_ipool",
+ },
+};
+
/**
* Initialize shared data between primary and secondary process.
*
@@ -317,6 +355,7 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
char name[RTE_ETH_NAME_MAX_LEN];
int own_domain_id = 0;
uint16_t port_id;
+ int i;
/* Build device name. */
strlcpy(name, dpdk_dev->name, sizeof(name));
@@ -584,6 +623,14 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
mlx5_set_min_inline(spawn, config);
/* Store device configuration on private structure. */
priv->config = *config;
+ for (i = 0; i < MLX5_FLOW_TYPE_MAXI; i++) {
+ icfg[i].release_mem_en = !!config->reclaim_mode;
+ if (config->reclaim_mode)
+ icfg[i].per_core_cache = 0;
+ priv->flows[i] = mlx5_ipool_create(&icfg[i]);
+ if (!priv->flows[i])
+ goto error;
+ }
/* Create context for virtual machine VLAN workaround. */
priv->vmwa_context = NULL;
if (config->dv_flow_en) {
--
2.25.1
Re: [dpdk-dev] [dpdk-stable] [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash offload
On 7/19/21 7:18 PM, Ferruh Yigit wrote:
On 7/19/2021 10:55 AM, Wang, Jie1X wrote:
-Original Message-
From: Yigit, Ferruh
Sent: Friday, July 16, 2021 4:52 PM
To: Li, Xiaoyun ; Wang, Jie1X ;
dev@dpdk.org
Cc: andrew.rybche...@oktetlabs.ru; sta...@dpdk.org
Subject: Re: [dpdk-stable] [PATCH v4] app/testpmd: fix testpmd doesn't show
RSS hash offload
On 7/16/2021 9:30 AM, Li, Xiaoyun wrote:
-Original Message-
From: stable On Behalf Of Li, Xiaoyun
Sent: Thursday, July 15, 2021 12:54
To: Wang, Jie1X ; dev@dpdk.org
Cc: andrew.rybche...@oktetlabs.ru; sta...@dpdk.org
Subject: Re: [dpdk-stable] [PATCH v4] app/testpmd: fix testpmd
doesn't show RSS hash offload
-Original Message-
From: Wang, Jie1X
Sent: Thursday, July 15, 2021 19:57
To: dev@dpdk.org
Cc: Li, Xiaoyun ;
andrew.rybche...@oktetlabs.ru; Wang, Jie1X ;
sta...@dpdk.org
Subject: [PATCH v4] app/testpmd: fix testpmd doesn't show RSS hash
offload
The driver may change offloads info into dev->data->dev_conf in
dev_configure which may cause port->dev_conf and port->rx_conf
contain
outdated values.
This patch updates the offloads info if it changes to fix this issue.
Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: sta...@dpdk.org
Signed-off-by: Jie Wang
---
v4: delete the whitespace at the end of the line.
v3:
- check and update the "offloads" of "port->dev_conf.rx/txmode".
- update the commit log.
v2: copy "rx/txmode.offloads", instead of copying the entire struct
"dev->data-
dev_conf.rx/txmode".
---
app/test-pmd/testpmd.c | 27 +++
1 file changed, 27 insertions(+)
Acked-by: Xiaoyun Li
Although I gave my ack, app shouldn't touch rte_eth_devices which this patch
does. Usually, testpmd should only call function like
eth_dev_info_get_print_err().
But dev_info doesn't contain the info dev->data->dev_conf which the driver
modifies.
Probably we need a better fix.
Agree, an application accessing directly to 'rte_eth_devices' is sign of
something
missing/wrong.
In this case there is no way for application to know what is the configured
offload settings per port and queue. Which is missing part I think.
As you said normally we get data from PMD mainly via 'rte_eth_dev_info_get()',
which is an overloaded function, it provides many different things, like driver
default values, limitations, current config/status, capabilities etc...
So I think we can do a few things:
1) Add current offload configuration to 'rte_eth_dev_info_get()', so application
can get it and use it.
The advantage is this API already called many places, many times, so there is a
big chance that application already have this information when it needs.
Disadvantage is, as mentioned above the API already big and messy, making it
bigger makes more error prone and makes easier to break ABI.
I prefer to choose the 1st suggestion.
Normally PMD gets data via 'rte_eth_dev_info_get()'. When we add offloads
configuration
to it, we can get offloads as same as getting other info.
Most probably it is easier to implement 1), I see your point but as said before
I think 'rte_eth_dev_info_get()' is already messy and I am worried to make it
even bigger.
IMHO, (1) is not an option.
I prefer option 2).
I'm not sure that API function for each config parameter is an option as
well. We should find a balance. May be I'd add something like
rte_eth_dev_get_conf(uint16_t port_id, const struct rte_eth_conf **conf)
which returns a pointer to up-to-date configuration. I.e. option (3).
The tricky part here is to ensure that all specific API which modifies
various bits of the configuration updates dev_conf.
@Thomas, @Andrew, what do you think?
2) Add a new API to get configured offload information, so a specific API for
it.
3) Get a more generic API to get configured config (dev_conf) which will cover
offloads too.
Disadvantage can be leaking out too many internal config to user
unintentionally.
I don't understand it. dev_conf is provided by user on
rte_eth_dev_configure().
Re: [dpdk-dev] [PATCH 1/4] doc: clarify RTE flow behaviour on port stop/start
2021-07-22 13:32 (UTC+0300), Andrew Rybchenko: > On 7/21/21 6:55 PM, Martin Havlik wrote: > > It is now clearly stated that RTE flow rules can be > > created only after the port is started. > > > > Signed-off-by: Martin Havlik > > --- > > doc/guides/nics/mlx5.rst | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst > > index f5b727c1ee..119d537adf 100644 > > --- a/doc/guides/nics/mlx5.rst > > +++ b/doc/guides/nics/mlx5.rst > > @@ -1790,21 +1790,25 @@ Notes for rte_flow > > -- > > > > Flows are not cached in the driver. > > When stopping a device port, all the flows created on this port from the > > application will be flushed automatically in the background. > > After stopping the device port, all flows on this port become invalid and > > not represented in the system. > > All references to these flows held by the application should be discarded > > directly but neither destroyed nor flushed. > > > > -The application should re-create the flows as required after the port > > restart. > > +The application should re-create the flows as required after the port is > > +started again. > > + > > +Creating flows before port start is not permitted. All flows the > > application > > +wants to create have to be created after the port is started. > > I'm not 100% sure that it is always OK for applications, but in an > attempt to make it OK we should: > - mention isolated mode if application dislikes default flow rules and > would like to control it > - mention what happens if restart happens internally, e.g. in order to > recover from broken state. I guess in this case we need an event and > application must register callback and handle it. I think this callback would be an unnecessary complication. What is the notion of internal restart or recovery event, in the first place? Port can move to some "inconsistent state" (from rte_flow_flush description). What this means is unspecified, I guess in this state the port can only be stopped or closed if it's not started. Which brings the port to a consistent state where the behavior is already specified.
Re: [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5
21/07/2021 10:54, Bing Zhao: > When creating a meter policy, the actions for yellow color can be > specified together with green color. The mlx5 PMD now supports to > set the policy actions for yellow color. > > The actions list that is supported for yellow is the same as that > for green. > > Acked-by: Matan Azrad > > Bing Zhao (7): > net/mlx5: handle yellow case in default meter policy > net/mlx5: enable meter bucket overflow for yellow color > net/mlx5: added support for yellow policy rules > net/mlx5: split policies handling of colors > net/mlx5: support yellow in meter policy validation > net/mlx5: check consistency of meter policy and profile > net/mlx5: add meter support for trTCM profiles Applied, thanks
[dpdk-dev] [PATCH] net/mlx5: add check for pop and push VLAN actions
For CX6 in FDB domain, pop and push VLAN on both ingress and
egress directions are supported.
For CX6 in NIC domain, and CX5 in both FWD add NIC domain, pop
VLAN is only supported on ingress direction, push VLAN is only
supported on egress direction.
Signed-off-by: Dong Zhou
Acked-by: Matan Azrad
---
drivers/common/mlx5/mlx5_devx_cmds.c | 2 ++
drivers/common/mlx5/mlx5_devx_cmds.h | 1 +
drivers/common/mlx5/mlx5_prm.h | 7 -
drivers/net/mlx5/linux/mlx5_os.c | 2 ++
drivers/net/mlx5/mlx5.h | 2 ++
drivers/net/mlx5/mlx5_flow_dv.c | 38
6 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c
b/drivers/common/mlx5/mlx5_devx_cmds.c
index 10a02d13ee..56407cc332 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.c
+++ b/drivers/common/mlx5/mlx5_devx_cmds.c
@@ -819,6 +819,8 @@ mlx5_devx_cmd_query_hca_attr(void *ctx,
attr->roce = MLX5_GET(cmd_hca_cap, hcattr, roce);
attr->rq_ts_format = MLX5_GET(cmd_hca_cap, hcattr, rq_ts_format);
attr->sq_ts_format = MLX5_GET(cmd_hca_cap, hcattr, sq_ts_format);
+ attr->steering_format_version =
+ MLX5_GET(cmd_hca_cap, hcattr, steering_format_version);
attr->regex = MLX5_GET(cmd_hca_cap, hcattr, regexp);
attr->regexp_num_of_engines = MLX5_GET(cmd_hca_cap, hcattr,
regexp_num_of_engines);
diff --git a/drivers/common/mlx5/mlx5_devx_cmds.h
b/drivers/common/mlx5/mlx5_devx_cmds.h
index c11ca6586f..e576e30f24 100644
--- a/drivers/common/mlx5/mlx5_devx_cmds.h
+++ b/drivers/common/mlx5/mlx5_devx_cmds.h
@@ -141,6 +141,7 @@ struct mlx5_hca_attr {
uint32_t roce:1;
uint32_t rq_ts_format:2;
uint32_t sq_ts_format:2;
+ uint32_t steering_format_version:4;
uint32_t qp_ts_format:2;
uint32_t regex:1;
uint32_t reg_c_preserve:1;
diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h
index 7950070976..915fcb66a2 100644
--- a/drivers/common/mlx5/mlx5_prm.h
+++ b/drivers/common/mlx5/mlx5_prm.h
@@ -1317,6 +1317,10 @@ enum {
#define MLX5_HCA_FLEX_ICMP_ENABLED (1UL << 8)
#define MLX5_HCA_FLEX_ICMPV6_ENABLED (1UL << 9)
+/* The device steering logic format. */
+#define MLX5_STEERING_LOGIC_FORMAT_CONNECTX_5 0x0
+#define MLX5_STEERING_LOGIC_FORMAT_CONNECTX_6DX 0x1
+
struct mlx5_ifc_cmd_hca_cap_bits {
u8 reserved_at_0[0x30];
u8 vhca_id[0x10];
@@ -1585,7 +1589,8 @@ struct mlx5_ifc_cmd_hca_cap_bits {
u8 general_obj_types[0x40];
u8 sq_ts_format[0x2];
u8 rq_ts_format[0x2];
- u8 reserved_at_444[0x1C];
+ u8 steering_format_version[0x4];
+ u8 reserved_at_448[0x18];
u8 reserved_at_460[0x8];
u8 aes_xts[0x1];
u8 crypto[0x1];
diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index aa5210fa45..1dbb51da0c 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -1357,6 +1357,8 @@ mlx5_dev_spawn(struct rte_device *dpdk_dev,
}
sh->rq_ts_format = config->hca_attr.rq_ts_format;
sh->sq_ts_format = config->hca_attr.sq_ts_format;
+ sh->steering_format_version =
+ config->hca_attr.steering_format_version;
sh->qp_ts_format = config->hca_attr.qp_ts_format;
/* Check for LRO support. */
if (config->dest_tir && config->hca_attr.lro_cap &&
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 94618e10fa..61898e8ea1 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1113,6 +1113,8 @@ struct mlx5_dev_ctx_shared {
uint32_t flow_hit_aso_en:1; /* Flow Hit ASO is supported. */
uint32_t rq_ts_format:2; /* RQ timestamp formats supported. */
uint32_t sq_ts_format:2; /* SQ timestamp formats supported. */
+ uint32_t steering_format_version:4;
+ /* Indicates the device steering logic format. */
uint32_t qp_ts_format:2; /* QP timestamp formats supported. */
uint32_t meter_aso_en:1; /* Flow Meter ASO is supported. */
uint32_t ct_aso_en:1; /* Connection Tracking ASO is supported. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index d250486950..3f9726d1c3 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2790,20 +2790,30 @@ flow_dv_validate_action_pop_vlan(struct rte_eth_dev
*dev,
struct rte_flow_error *error)
{
const struct mlx5_priv *priv = dev->data->dev_private;
+ struct mlx5_dev_ctx_shared *sh = priv->sh;
+ bool direction_error = false;
- (void)action;
- (void)attr;
if (!priv->sh->pop_vlan_action)
return rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
Re: [dpdk-dev] [PATCH v2 1/4] regex/mlx5: fix size of setup constants
01/07/2021 08:39, Michael Baum:
> The constant representing the size of the metadata is defined as a
> unsigned int variable with 32-bit.
> Similarly the constant representing the maximal output is also defined
> as a unsigned int variable with 32-bit.
>
> There is potentially overflowing expression when those constants are
> evaluated using 32-bit arithmetic, and then used in a context that
> expects an expression of type size_t that might be 64 bit.
>
> Change the size of the above constants to size_t.
>
> Fixes: 30d604bb1504 ("regex/mlx5: fix type of setup constants")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Michael Baum
> Acked-by: Matan Azrad
Series applied, thanks.
Re: [dpdk-dev] [PATCH_v4 0/3] regex/mlx5: some independent fixes
> Michael Baum (3): > regex/mlx5: fix memory region unregistration > regex/mlx5: fix leak in PCI remove function > regex/mlx5: fix redundancy in PCI remove function Applied, thanks
Re: [dpdk-dev] [PATCH] net/mlx5: add check for pop and push VLAN actions
22/07/2021 09:48, Dong Zhou: > For CX6 in FDB domain, pop and push VLAN on both ingress and > egress directions are supported. > > For CX6 in NIC domain, and CX5 in both FWD add NIC domain, pop > VLAN is only supported on ingress direction, push VLAN is only > supported on egress direction. > > Signed-off-by: Dong Zhou > Acked-by: Matan Azrad Applied with title "net/mlx5: check VLAN push/pop support", thanks.
[dpdk-dev] Question about hardware error handling policy
Hi, all I notice ethdev support dev_reset ops, which could be used to recover from errors, and only 13+ drivers support this function. And also there is event for reset: RTE_ETH_EVENT_INTR_RESET, and only 6 drivers support it (most of them are VF). This provides users with two ways to handle hardware errors: a. driver report RTE_ETH_EVENT_INTR_RESET, and application do reset ops. b. application detect errors (the detection method is unclear), and call reset ops to recover. According to the design of this API, error handling is assigned to the application, and the driver is only responsible for reporting events. This simplifies the driver design (for example, the driver does not need to maintain mutex locks). As we know, many modern NICs come with firmware, have PCIE interfaces, support SR-IOV, the hardware errors can have: firmware reboot/PF reset/ VF reset/FLR, but these errors(particularly firmware/PF) are not addressed in most drivers. Question 1: what do we think of these errors(particularly firmware/PF)? Do we think that the probability is very low and that there is no need to deal with them? Question 2: I prefer to put error handling in the application layer, because doing it in the driver can make the driver complex, but there is no app to register the INTR_RESET event handler. I think we can build a standard handler in testpmd, What do you think? Thanks
Re: [dpdk-dev] [PATCH] net/mlx5: fix invalid Rx/Tx queue checks
20/07/2021 09:53, Dmitry Kozlyuk:
> When device configuration was interrupted by a signal,
> mlx5_rxq/txq_release() could access yet unitinialized array
> and crash the application. Add checks whether queue array
> is initialized.
>
> Fixes: a1366b1a2be3 ("net/mlx5: add reference counter on DPDK Rx queues")
> Fixes: 6e78005a9b30 ("net/mlx5: add reference counter on DPDK Tx queues")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Dmitry Kozlyuk
> Acked-by: Matan Azrad
Applied, thanks.
PS: no need fo "invalid" word after "fix".
Re: [dpdk-dev] [dpdk-stable] [PATCH] net/mlx5: fix indirect action modify rollback
21/07/2021 14:51, Dmitry Kozlyuk:
> mlx5_ind_table_obj_modify() first references queues from the new list,
> then applies the new list to HW. In case of apply failure the function
> dereferenced queues from the old list, while it should be the new list.
>
> Fixes: fa7ad49e96b5 ("net/mlx5: fix shared RSS action update")
> Cc: andr...@nvidia.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Dmitry Kozlyuk
> Acked-by: Matan Azrad
Applied, thanks
Re: [dpdk-dev] [PATCH] net/mlx5: fix use after free in mlx5_dma_unmap
10/07/2021 12:35, wangyunjian:
> From: Yunjian Wang
>
> This patch fixes the use-after-free bug which was reported by Coverity
> Scan in the mlx5_dma_unmap function.
>
> Coverity issue: 371679
> Fixes: 992e6df3dafe ("common/mlx5: free MR resource on device DMA unmap")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Yunjian Wang
> ---
> LIST_REMOVE(mr, mr);
> - mlx5_mr_free(mr, sh->share_cache.dereg_mr_cb);
> DRV_LOG(DEBUG, "port %u remove MR(%p) from list", dev->data->port_id,
> (void *)mr);
> + mlx5_mr_free(mr, sh->share_cache.dereg_mr_cb);
> mlx5_mr_rebuild_cache(&sh->share_cache);
Sorry, it seems this fix has been integrated as part of this patch
(for no good reason):
https://git.dpdk.org/dpdk/commit/?id=a7f34989e9
Re: [dpdk-dev] [PATCH] net/mlx5: fix indexed pools allocate on Windows
21/07/2021 10:43, Matan Azrad: > Better title: > net/mlx5/windows: fix indexed pools allocation even better: keep the "on Windows" at the end.
Re: [dpdk-dev] [PATCH v2] net/mlx5: fix indexed pools allocation
22/07/2021 08:59, Suanming Mou:
> Currently, the flow indexed pools are allocated per port, the allocation
> was missing in Windows code.
>
> Allocate indexed pool for the Windows case too.
>
> Fixes: b4edeaf3efd5 ("net/mlx5: replace flow list with indexed pool")
>
> Signed-off-by: Suanming Mou
> Acked-by: Tal Shnaiderman
> Acked-by: Matan Azrad
> Tested-by: Odi Assli
Applied, thanks.
Re: [dpdk-dev] [PATCH v2] net/mlx5: fix meta register conversion for extensive mode
20/07/2021 09:51, Alexander Kozyrev:
> Register C is used in the extensive metadata mode number 1 and its
> width can vary from 0 to 32 bits depending on the kernel usage of it.
>
> There are several issues associated with this mode (dv_xmeta_en=1):
> 1. The metadata setting assumes that the width is always 16 bits,
> which is the most common case in this mode. Use the proper mask.
> 2. The same is true for the modify_field Flow API. 16-bits width
> is hardcoded for dv_xmeta_en=1. Switch to the register C mask width.
> 3. Metadata is stored in the most significant bits in CQE in this
> mode because the registers copy code was not updated during the
> metadata conversion to the big-endian format. Update this code to
> avoid shifting the metadata in the datapath.
>
> Fixes: b57e414b48 ("net/mlx5: convert meta register to big-endian")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Alexander Kozyrev
> Acked-by: Viacheslav Ovsiienko
Applied, thanks
Re: [dpdk-dev] [PATCH v2] net/mlx5: do not allow copy to mark via modify field
16/07/2021 12:47, Slava Ovsiienko: > > -Original Message- > > From: Alexander Kozyrev > > Sent: Friday, July 16, 2021 11:43 > > To: dev@dpdk.org > > Cc: Raslan Darawsheh ; Matan Azrad > > ; Slava Ovsiienko > > Subject: [PATCH v2] net/mlx5: do not allow copy to mark via modify field > > > > The Mark action is a two-stage process in the Mellanox driver. > > First, a hardware register is filled with the required value, then this > > value is > > registered in the software resource table. > > > > The MODIFY_FIELD action can instruct a Mellanox NIC to copy some value > > from an arbitrary packet header field into the hardware register, associated > > with the Mark item. But there is no way NIC can modify the software > > resource table as well. > > > > Due to these driver limitations the copying of arbitrary value to the MARK > > can > > not be supported and should be rejected in the MODIFY_FIELD action. > > > Thank you, Alexander > > > Signed-off-by: Alexander Kozyrev > Acked-by: Viacheslav Ovsiienko Applied, thanks.
Re: [dpdk-dev] [PATCH 1/4] ethdev: fix max Rx packet length
On Thu, 22 Jul 2021 13:15:04 +0300 Andrew Rybchenko wrote: > > I don't think we care about type of transmission in this level, I assume we > > define min MTU mainly for the HW limitation and configuration. That is why > > it > > makes sense to me to use Ethernet frame lenght limitation (not IPv4 one). > > +1 Also it is important that DPDK follow the conventions of other software such as Linux and BSD. Cisco and Juniper already disagree about whether header should be included in what is defined as MTU; i.e Cisco says 1514 and Juniper says 1500.
Re: [dpdk-dev] [dpdk-stable] [PATCH] net/mlx5: fix ROCE LAG bond device probing
21/07/2021 10:31, Viacheslav Ovsiienko:
> The ROCE LAG bond device requires neither E-Switch nor SR-IOV
> configurations. It means the ROCE LAG bond device might be
> presented as a single port Infiniband device.
>
> The mlx5 PMD wrongly recognized standalone ROCE LAG bond device
> as E-Switch configuration, this triggered the calls of E-Switch
> ports related API and the latter failed (over the new OFED kernel
> driver, starting since 5.4.1), causing the overall device probe
> failure.
>
> If there is a single port Infiniband bond device found the
> E-Switch related flags must be cleared indicating standalone
> configuration.
>
> Also, it is not true anymore the bond device can exist
> over E-Switch configurations only (as it was claimed for VF LAG
> bond devices). The related checks are not relevant anymore
> and removed.
>
> Fixes: 790164ce1d2d ("net/mlx5: check kernel support for VF LAG bonding")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Viacheslav Ovsiienko
> Acked-by: Matan Azrad
Applied, thanks
Re: [dpdk-dev] [PATCH] net/mlx5: fix SubFunction representor probe in isolate mode
> > Representor failed to probe in isolated mode due to callback of retrieving
> > representor info missing. This patch adds it back.
> >
> > Fixes: cb95feefdd03 ("net/mlx5: support sub-function representor")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Xueming Li
> Acked-by: Viacheslav Ovsiienko
Applied, thanks.
Re: [dpdk-dev] [PATCH v1] net/mlx5: fix RSS expansion for GTP
18/07/2021 13:15, Lior Margalit:
> The flow did not expand correctly when it included a GTP item.
>
> Added GTP node to the expansion graph as possible next node
> after IPv4/IPv6 UDP node.
>
> Fixes: 592f05b29a25 ("net/mlx5: add RSS flow action")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Lior Margalit
> Acked-by: Matan Azrad
Applied, thanks.
Re: [dpdk-dev] [PATCH v1] net/mlx5: fix ETH validation for GTP
20/07/2021 17:17, Lior Margalit:
> The user is able to create a flow rule pattern with ETH after GTP
> although it is not supported by the flex-parser configuration.
>
> Failed the rule validation in such case with proper error message.
>
> Fixes: 23c1d42c7138 ("net/mlx5: split flow validation to dedicated function")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Lior Margalit
> Acked-by: Matan Azrad
Applied, thanks.
Re: [dpdk-dev] [PATCH] net/mlx5: export rte_pmd_mlx5 header
18/07/2021 12:29, Liang Ma:
> From: Liang Ma
>
> rte prefix header should be exported in meson.build
>
> Fixes: 23f627e0ed28 (net/mlx5: add flow sync API)
Fixes: efa79e68c8cd ("net/mlx5: support fine grain dynamic flag")
Cc: sta...@dpdk.org
> Signed-off-by: Liang Ma
Applied with some rewords, thanks.
Re: [dpdk-dev] Question about hardware error handling policy
22/07/2021 15:50, fengchengwen: > Hi, all > > I notice ethdev support dev_reset ops, which could be used to recover from > errors, and only 13+ drivers support this function. > And also there is event for reset: RTE_ETH_EVENT_INTR_RESET, and only 6 > drivers support it (most of them are VF). > > This provides users with two ways to handle hardware errors: > a. driver report RTE_ETH_EVENT_INTR_RESET, and application do reset ops. > b. application detect errors (the detection method is unclear), and call > reset ops to recover. > > According to the design of this API, error handling is assigned to the > application, and the driver is only responsible for reporting events. This > simplifies the driver design (for example, the driver does not need to > maintain > mutex locks). > > As we know, many modern NICs come with firmware, have PCIE interfaces, > support SR-IOV, the hardware errors can have: firmware reboot/PF reset/ > VF reset/FLR, but these errors(particularly firmware/PF) are not addressed in > most drivers. > > Question 1: what do we think of these errors(particularly firmware/PF)? Do > we think that the probability is very low and that there is no need to deal > with > them? Even rare errors must be managed. > Question 2: I prefer to put error handling in the application layer, > because > doing it in the driver can make the driver complex, but there is no app to > register the INTR_RESET event handler. I think we can build a standard handler > in testpmd, What do you think? Absolutely. As any ethdev API, it must be tested with testpmd.
Re: [dpdk-dev] [PATCH v7 0/5] vhost: handle memory hotplug for async vhost
22/07/2021 07:07, Xia, Chenbo:
> From: Jiang, Cheng1
> > When the guest memory is hotplugged, the vhost application which
> > enables DMA acceleration must stop DMA transfers before the vhost
> > re-maps the guest memory.
> >
> > This patch set is to provide an unsafe API to drain inflight pkts
> > which are submitted to DMA engine in vhost async data path, and
> > notify the vhost application of stopping DMA transfers. And enable it
> > in vhost example.
>
> Series applied to next-virtio/main. Thanks
I cannot pull this series in main branch.
There is a compilation error seen on Arm cross-compilation:
examples/vhost/main.c:1493:51: error: assignment to 'int32_t (*)(int,
uint16_t, struct rte_vhost_async_desc *, struct rte_vhost_async_status *,
uint16_t)' {aka 'int (*)(int, short unsigned int, struct rte_vhost_async_desc
*, struct rte_vhost_async_status *, short unsigned int)'} from incompatible
pointer type 'uint32_t (*)(int, uint16_t, struct rte_vhost_async_desc *,
struct rte_vhost_async_status *, uint16_t)' {aka 'unsigned int (*)(int, short
unsigned int, struct rte_vhost_async_desc *, struct rte_vhost_async_status *,
short unsigned int)'} [-Werror=incompatible-pointer-types]
1493 | channel_ops.transfer_data =
ioat_transfer_data_cb;
| ^
Other comments about the last patch:
- it is updating doc out of the original patch doing the code changes
- there is not even a reference to the code patch (Fixes: line)
- the addition in the release notes is not sorted
Last question while at it, why having the API documentation
in the vhost guide (rst file)?
Doxygen is not enough to describe the functions?
[dpdk-dev] [PATCH v2 2/6] ethdev: move jumbo frame offload check to library
Setting MTU bigger than RTE_ETHER_MTU requires the jumbo frame support,
and application should enable the jumbo frame offload support for it.
When jumbo frame offload is not enabled by application, but MTU bigger
than RTE_ETHER_MTU is requested there are two options, either fail or
enable jumbo frame offload implicitly.
Enabling jumbo frame offload implicitly is selected by many drivers
since setting a big MTU value already implies it, and this increases
usability.
This patch moves this logic from drivers to the library, both to reduce
the duplicated code in the drivers and to make behaviour more visible.
Signed-off-by: Ferruh Yigit
Reviewed-by: Andrew Rybchenko
Reviewed-by: Rosen Xu
Acked-by: Ajit Khaparde
---
drivers/net/axgbe/axgbe_ethdev.c| 9 ++---
drivers/net/bnxt/bnxt_ethdev.c | 9 ++---
drivers/net/cnxk/cnxk_ethdev_ops.c | 5 -
drivers/net/cxgbe/cxgbe_ethdev.c| 8
drivers/net/dpaa/dpaa_ethdev.c | 7 ---
drivers/net/dpaa2/dpaa2_ethdev.c| 7 ---
drivers/net/e1000/em_ethdev.c | 9 ++---
drivers/net/e1000/igb_ethdev.c | 9 ++---
drivers/net/enetc/enetc_ethdev.c| 7 ---
drivers/net/hinic/hinic_pmd_ethdev.c| 7 ---
drivers/net/hns3/hns3_ethdev.c | 8
drivers/net/hns3/hns3_ethdev_vf.c | 6 --
drivers/net/i40e/i40e_ethdev.c | 5 -
drivers/net/i40e/i40e_ethdev_vf.c | 5 -
drivers/net/iavf/iavf_ethdev.c | 7 ---
drivers/net/ice/ice_ethdev.c| 5 -
drivers/net/igc/igc_ethdev.c| 9 ++---
drivers/net/ipn3ke/ipn3ke_representor.c | 5 -
drivers/net/ixgbe/ixgbe_ethdev.c| 7 ++-
drivers/net/liquidio/lio_ethdev.c | 7 ---
drivers/net/nfp/nfp_net.c | 6 --
drivers/net/octeontx/octeontx_ethdev.c | 5 -
drivers/net/octeontx2/otx2_ethdev_ops.c | 5 -
drivers/net/qede/qede_ethdev.c | 4
drivers/net/sfc/sfc_ethdev.c| 9 -
drivers/net/thunderx/nicvf_ethdev.c | 6 --
drivers/net/txgbe/txgbe_ethdev.c| 6 --
lib/ethdev/rte_ethdev.c | 18 +-
28 files changed, 29 insertions(+), 171 deletions(-)
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 76aeec077f2b..2960834b4539 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -1492,15 +1492,10 @@ static int axgb_mtu_set(struct rte_eth_dev *dev,
uint16_t mtu)
dev->data->port_id);
return -EBUSY;
}
- if (mtu > RTE_ETHER_MTU) {
- dev->data->dev_conf.rxmode.offloads |=
- DEV_RX_OFFLOAD_JUMBO_FRAME;
+ if (mtu > RTE_ETHER_MTU)
val = 1;
- } else {
- dev->data->dev_conf.rxmode.offloads &=
- ~DEV_RX_OFFLOAD_JUMBO_FRAME;
+ else
val = 0;
- }
AXGMAC_IOWRITE_BITS(pdata, MAC_RCR, JE, val);
return 0;
}
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index e27720e71645..18511b28e4a3 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -3022,15 +3022,10 @@ int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev,
uint16_t new_mtu)
return -EINVAL;
}
- if (new_mtu > RTE_ETHER_MTU) {
+ if (new_mtu > RTE_ETHER_MTU)
bp->flags |= BNXT_FLAG_JUMBO;
- bp->eth_dev->data->dev_conf.rxmode.offloads |=
- DEV_RX_OFFLOAD_JUMBO_FRAME;
- } else {
- bp->eth_dev->data->dev_conf.rxmode.offloads &=
- ~DEV_RX_OFFLOAD_JUMBO_FRAME;
+ else
bp->flags &= ~BNXT_FLAG_JUMBO;
- }
/* Is there a change in mtu setting? */
if (eth_dev->data->mtu == new_mtu)
diff --git a/drivers/net/cnxk/cnxk_ethdev_ops.c
b/drivers/net/cnxk/cnxk_ethdev_ops.c
index 695d0d6fd3e2..349896f6a1bf 100644
--- a/drivers/net/cnxk/cnxk_ethdev_ops.c
+++ b/drivers/net/cnxk/cnxk_ethdev_ops.c
@@ -439,11 +439,6 @@ cnxk_nix_mtu_set(struct rte_eth_dev *eth_dev, uint16_t mtu)
plt_err("Failed to max Rx frame length, rc=%d", rc);
goto exit;
}
-
- if (mtu > RTE_ETHER_MTU)
- dev->rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
- else
- dev->rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
exit:
return rc;
}
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 8cf61f12a8d6..0c9cc2f5bb3f 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -313,14 +313,6 @@ int cxgbe_dev_mtu_set(struct rte_eth_dev *eth_dev,
uint16_t mtu)
if (mtu < RTE_ETHER_MIN_MTU || new_mtu > dev_info.max_rx_pktlen)
return -EINVAL;
- /* set t
[dpdk-dev] [PATCH v2 3/6] ethdev: move check to library for MTU set
Move requested MTU value check to the API to prevent the duplicated
code.
Signed-off-by: Ferruh Yigit
Reviewed-by: Andrew Rybchenko
Reviewed-by: Rosen Xu
---
drivers/net/axgbe/axgbe_ethdev.c| 15 ---
drivers/net/bnxt/bnxt_ethdev.c | 2 +-
drivers/net/cxgbe/cxgbe_ethdev.c| 13 +
drivers/net/dpaa/dpaa_ethdev.c | 2 --
drivers/net/dpaa2/dpaa2_ethdev.c| 4
drivers/net/e1000/em_ethdev.c | 10 --
drivers/net/e1000/igb_ethdev.c | 11 ---
drivers/net/enetc/enetc_ethdev.c| 4
drivers/net/hinic/hinic_pmd_ethdev.c| 8 +---
drivers/net/i40e/i40e_ethdev.c | 17 -
drivers/net/i40e/i40e_ethdev_vf.c | 17 -
drivers/net/iavf/iavf_ethdev.c | 10 ++
drivers/net/ice/ice_ethdev.c| 14 +++---
drivers/net/igc/igc_ethdev.c| 5 -
drivers/net/ipn3ke/ipn3ke_representor.c | 6 --
drivers/net/liquidio/lio_ethdev.c | 10 --
drivers/net/nfp/nfp_net.c | 4
drivers/net/octeontx/octeontx_ethdev.c | 4
drivers/net/octeontx2/otx2_ethdev_ops.c | 5 -
drivers/net/qede/qede_ethdev.c | 12
drivers/net/thunderx/nicvf_ethdev.c | 6 --
drivers/net/txgbe/txgbe_ethdev.c| 10 --
lib/ethdev/rte_ethdev.c | 9 +
23 files changed, 29 insertions(+), 169 deletions(-)
diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 2960834b4539..c36cd7b1d2f0 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -1478,25 +1478,18 @@ axgbe_dev_supported_ptypes_get(struct rte_eth_dev *dev)
static int axgb_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
{
- struct rte_eth_dev_info dev_info;
struct axgbe_port *pdata = dev->data->dev_private;
- uint32_t frame_size = mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
- unsigned int val = 0;
- axgbe_dev_info_get(dev, &dev_info);
- /* check that mtu is within the allowed range */
- if (mtu < RTE_ETHER_MIN_MTU || frame_size > dev_info.max_rx_pktlen)
- return -EINVAL;
+ unsigned int val;
+
/* mtu setting is forbidden if port is start */
if (dev->data->dev_started) {
PMD_DRV_LOG(ERR, "port %d must be stopped before configuration",
dev->data->port_id);
return -EBUSY;
}
- if (mtu > RTE_ETHER_MTU)
- val = 1;
- else
- val = 0;
+ val = mtu > RTE_ETHER_MTU ? 1 : 0;
AXGMAC_IOWRITE_BITS(pdata, MAC_RCR, JE, val);
+
return 0;
}
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 18511b28e4a3..2c58f7f681c6 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2995,7 +2995,7 @@ int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t
new_mtu)
uint32_t overhead = BNXT_MAX_PKT_LEN - BNXT_MAX_MTU;
struct bnxt *bp = eth_dev->data->dev_private;
uint32_t new_pkt_size;
- uint32_t rc = 0;
+ uint32_t rc;
uint32_t i;
rc = is_bnxt_in_error(bp);
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index 0c9cc2f5bb3f..70b879fed100 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -301,21 +301,10 @@ int cxgbe_dev_mtu_set(struct rte_eth_dev *eth_dev,
uint16_t mtu)
{
struct port_info *pi = eth_dev->data->dev_private;
struct adapter *adapter = pi->adapter;
- struct rte_eth_dev_info dev_info;
- int err;
uint16_t new_mtu = mtu + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
- err = cxgbe_dev_info_get(eth_dev, &dev_info);
- if (err != 0)
- return err;
-
- /* Must accommodate at least RTE_ETHER_MIN_MTU */
- if (mtu < RTE_ETHER_MIN_MTU || new_mtu > dev_info.max_rx_pktlen)
- return -EINVAL;
-
- err = t4_set_rxmode(adapter, adapter->mbox, pi->viid, new_mtu, -1, -1,
+ return t4_set_rxmode(adapter, adapter->mbox, pi->viid, new_mtu, -1, -1,
-1, -1, true);
- return err;
}
/*
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index a444f749bb96..60dd4f67fc26 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -167,8 +167,6 @@ dpaa_mtu_set(struct rte_eth_dev *dev, uint16_t mtu)
PMD_INIT_FUNC_TRACE();
- if (mtu < RTE_ETHER_MIN_MTU || frame_size > DPAA_MAX_RX_PKT_LEN)
- return -EINVAL;
/*
* Refuse mtu that requires the support of scattered packets
* when this feature has not been enabled before.
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index febe3d0b754e..7bb309691ce2 100644
--
[dpdk-dev] [PATCH v2 4/6] ethdev: remove jumbo offload flag
Removing 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag. Instead of drivers announce this capability, application can deduct the capability by checking reported 'dev_info.max_mtu' or 'dev_info.max_rx_pktlen'. And instead of application explicitly set this flag to enable jumbo frames, this can be deducted by driver by comparing requested 'mtu' to 'RTE_ETHER_MTU'. Removing this additional configuration for simplification. Suggested-by: Konstantin Ananyev Signed-off-by: Ferruh Yigit Acked-by: Andrew Rybchenko Reviewed-by: Rosen Xu --- app/test-eventdev/test_pipeline_common.c | 2 - app/test-pmd/cmdline.c| 2 +- app/test-pmd/config.c | 24 +- app/test-pmd/testpmd.c| 46 +-- app/test-pmd/testpmd.h| 2 +- doc/guides/howto/debug_troubleshoot.rst | 2 - doc/guides/nics/bnxt.rst | 1 - doc/guides/nics/features.rst | 3 +- drivers/net/atlantic/atl_ethdev.c | 1 - drivers/net/axgbe/axgbe_ethdev.c | 1 - drivers/net/bnx2x/bnx2x_ethdev.c | 1 - drivers/net/bnxt/bnxt.h | 1 - drivers/net/bnxt/bnxt_ethdev.c| 10 +--- drivers/net/bonding/rte_eth_bond_pmd.c| 8 drivers/net/cnxk/cnxk_ethdev.h| 5 +- drivers/net/cnxk/cnxk_ethdev_ops.c| 1 - drivers/net/cxgbe/cxgbe.h | 1 - drivers/net/cxgbe/cxgbe_ethdev.c | 8 drivers/net/cxgbe/sge.c | 5 +- drivers/net/dpaa/dpaa_ethdev.c| 2 - drivers/net/dpaa2/dpaa2_ethdev.c | 2 - drivers/net/e1000/e1000_ethdev.h | 4 +- drivers/net/e1000/em_ethdev.c | 4 +- drivers/net/e1000/em_rxtx.c | 19 +++- drivers/net/e1000/igb_rxtx.c | 3 +- drivers/net/ena/ena_ethdev.c | 2 - drivers/net/enetc/enetc_ethdev.c | 3 +- drivers/net/enic/enic_res.c | 1 - drivers/net/failsafe/failsafe_ops.c | 2 - drivers/net/fm10k/fm10k_ethdev.c | 1 - drivers/net/hinic/hinic_pmd_ethdev.c | 1 - drivers/net/hns3/hns3_ethdev.c| 1 - drivers/net/hns3/hns3_ethdev_vf.c | 1 - drivers/net/i40e/i40e_ethdev.c| 1 - drivers/net/i40e/i40e_ethdev_vf.c | 3 +- drivers/net/i40e/i40e_rxtx.c | 2 +- drivers/net/iavf/iavf_ethdev.c| 3 +- drivers/net/ice/ice_dcf_ethdev.c | 3 +- drivers/net/ice/ice_dcf_vf_representor.c | 1 - drivers/net/ice/ice_ethdev.c | 1 - drivers/net/ice/ice_rxtx.c| 3 +- drivers/net/igc/igc_ethdev.h | 1 - drivers/net/igc/igc_txrx.c| 2 +- drivers/net/ionic/ionic_ethdev.c | 1 - drivers/net/ipn3ke/ipn3ke_representor.c | 3 +- drivers/net/ixgbe/ixgbe_ethdev.c | 5 +- drivers/net/ixgbe/ixgbe_pf.c | 9 +--- drivers/net/ixgbe/ixgbe_rxtx.c| 3 +- drivers/net/mlx4/mlx4_rxq.c | 1 - drivers/net/mlx5/mlx5_rxq.c | 1 - drivers/net/mvneta/mvneta_ethdev.h| 3 +- drivers/net/mvpp2/mrvl_ethdev.c | 1 - drivers/net/nfp/nfp_net.c | 6 +-- drivers/net/octeontx/octeontx_ethdev.h| 1 - drivers/net/octeontx2/otx2_ethdev.h | 1 - drivers/net/octeontx_ep/otx_ep_ethdev.c | 3 +- drivers/net/octeontx_ep/otx_ep_rxtx.c | 6 --- drivers/net/qede/qede_ethdev.c| 1 - drivers/net/sfc/sfc_rx.c | 2 - drivers/net/thunderx/nicvf_ethdev.h | 1 - drivers/net/txgbe/txgbe_rxtx.c| 1 - drivers/net/virtio/virtio_ethdev.c| 1 - drivers/net/vmxnet3/vmxnet3_ethdev.c | 1 - examples/ip_fragmentation/main.c | 3 +- examples/ip_reassembly/main.c | 3 +- examples/ipsec-secgw/ipsec-secgw.c| 2 - examples/ipv4_multicast/main.c| 1 - examples/kni/main.c | 5 -- examples/l3fwd-acl/main.c | 4 +- examples/l3fwd-graph/main.c | 4 +- examples/l3fwd-power/main.c | 4 +- examples/l3fwd/main.c | 4 +- .../performance-thread/l3fwd-thread/main.c| 4 +- examples/vhost/main.c | 2 - lib/ethdev/rte_ethdev.c | 26 +-- lib/ethdev/rte_ethdev.h | 1 - 76 files changed, 47 insertions(+), 257 deletions(-) diff --git a/app/test-eventdev/test_pipeline_common.c b/app/test-eventdev/test_pipeline_common.c index 5fcea74b4d43..2775e72c580d 100644 --- a/app/test-eventdev/test_pipeline_common.c +++ b/app/test-eventdev/te
[dpdk-dev] [PATCH v2 5/6] ethdev: unify MTU checks
Both 'rte_eth_dev_configure()' & 'rte_eth_dev_set_mtu()' sets MTU but
have slightly different checks. Like one checks min MTU against
RTE_ETHER_MIN_MTU and other RTE_ETHER_MIN_LEN.
Checks moved into common function to unify the checks. Also this has
benefit to have common error logs.
Suggested-by: Huisong Li
Signed-off-by: Ferruh Yigit
---
lib/ethdev/rte_ethdev.c | 82 ++---
lib/ethdev/rte_ethdev.h | 2 +-
2 files changed, 54 insertions(+), 30 deletions(-)
diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 97d5c7d42d3b..1957fdec46a7 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1337,6 +1337,47 @@ eth_dev_get_overhead_len(uint32_t max_rx_pktlen,
uint16_t max_mtu)
return overhead_len;
}
+/* rte_eth_dev_info_get() should be called prior to this function */
+static int
+eth_dev_validate_mtu(uint16_t port_id, struct rte_eth_dev_info *dev_info,
+ uint16_t mtu)
+{
+ uint16_t overhead_len;
+ uint32_t frame_size;
+
+ if (mtu < dev_info->min_mtu) {
+ RTE_ETHDEV_LOG(ERR,
+ "MTU (%u) < device min MTU (%u) for port_id %u\n",
+ mtu, dev_info->min_mtu, port_id);
+ return -EINVAL;
+ }
+ if (mtu > dev_info->max_mtu) {
+ RTE_ETHDEV_LOG(ERR,
+ "MTU (%u) > device max MTU (%u) for port_id %u\n",
+ mtu, dev_info->max_mtu, port_id);
+ return -EINVAL;
+ }
+
+ overhead_len = eth_dev_get_overhead_len(dev_info->max_rx_pktlen,
+ dev_info->max_mtu);
+ frame_size = mtu + overhead_len;
+ if (frame_size < RTE_ETHER_MIN_LEN) {
+ RTE_ETHDEV_LOG(ERR,
+ "Frame size (%u) < min frame size (%u) for port_id
%u\n",
+ frame_size, RTE_ETHER_MIN_LEN, port_id);
+ return -EINVAL;
+ }
+
+ if (frame_size > dev_info->max_rx_pktlen) {
+ RTE_ETHDEV_LOG(ERR,
+ "Frame size (%u) > device max frame size (%u) for
port_id %u\n",
+ frame_size, dev_info->max_rx_pktlen, port_id);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
int
rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
const struct rte_eth_conf *dev_conf)
@@ -1464,26 +1505,13 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t
nb_rx_q, uint16_t nb_tx_q,
goto rollback;
}
- /*
-* Check that the maximum RX packet length is supported by the
-* configured device.
-*/
if (dev_conf->rxmode.mtu == 0)
dev->data->dev_conf.rxmode.mtu = RTE_ETHER_MTU;
- max_rx_pktlen = dev->data->dev_conf.rxmode.mtu + overhead_len;
- if (max_rx_pktlen > dev_info.max_rx_pktlen) {
- RTE_ETHDEV_LOG(ERR,
- "Ethdev port_id=%u max_rx_pktlen %u > max valid value
%u\n",
- port_id, max_rx_pktlen, dev_info.max_rx_pktlen);
- ret = -EINVAL;
- goto rollback;
- } else if (max_rx_pktlen < RTE_ETHER_MIN_LEN) {
- RTE_ETHDEV_LOG(ERR,
- "Ethdev port_id=%u max_rx_pktlen %u < min valid value
%u\n",
- port_id, max_rx_pktlen, RTE_ETHER_MIN_LEN);
- ret = -EINVAL;
+
+ ret = eth_dev_validate_mtu(port_id, &dev_info,
+ dev->data->dev_conf.rxmode.mtu);
+ if (ret != 0)
goto rollback;
- }
dev->data->mtu = dev->data->dev_conf.rxmode.mtu;
@@ -1492,6 +1520,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q,
uint16_t nb_tx_q,
* size is supported by the configured device.
*/
if (dev_conf->rxmode.offloads & DEV_RX_OFFLOAD_TCP_LRO) {
+ overhead_len = eth_dev_get_overhead_len(dev_info.max_rx_pktlen,
+ dev_info.max_mtu);
+ max_rx_pktlen = dev->data->dev_conf.rxmode.mtu + overhead_len;
if (dev_conf->rxmode.max_lro_pkt_size == 0)
dev->data->dev_conf.rxmode.max_lro_pkt_size =
max_rx_pktlen;
ret = eth_dev_check_lro_pkt_size(port_id,
@@ -3438,7 +3469,8 @@ rte_eth_dev_info_get(uint16_t port_id, struct
rte_eth_dev_info *dev_info)
dev_info->rx_desc_lim = lim;
dev_info->tx_desc_lim = lim;
dev_info->device = dev->device;
- dev_info->min_mtu = RTE_ETHER_MIN_MTU;
+ dev_info->min_mtu = RTE_ETHER_MIN_LEN - RTE_ETHER_HDR_LEN -
+ RTE_ETHER_CRC_LEN;
dev_info->max_mtu = UINT16_MAX;
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP);
@@ -3644,21 +3676,13 @@ rte_eth_dev_set_mtu(uint16_t port_id, uint16_t mtu)
* which relies on dev->dev_ops->dev_infos_get.
*/
if (*dev->dev_ops->dev
[dpdk-dev] [PATCH v2 6/6] examples/ip_reassembly: remove unused parameter
Remove 'max-pkt-len' parameter.
Signed-off-by: Ferruh Yigit
---
examples/ip_reassembly/main.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index b92f0e460178..5c1b951c0d80 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -512,7 +512,6 @@ static void
print_usage(const char *prgname)
{
printf("%s [EAL options] -- -p PORTMASK [-q NQ]"
- " [--max-pkt-len PKTLEN]"
" [--maxflows=] [--flowttl=[(s|ms)]]\n"
" -p PORTMASK: hexadecimal bitmask of ports to configure\n"
" -q NQ: number of RX queues per lcore\n"
@@ -614,7 +613,6 @@ parse_args(int argc, char **argv)
int option_index;
char *prgname = argv[0];
static struct option lgopts[] = {
- {"max-pkt-len", 1, 0, 0},
{"maxflows", 1, 0, 0},
{"flowttl", 1, 0, 0},
{NULL, 0, 0, 0}
--
2.31.1
Re: [dpdk-dev] [dpdk-stable] [PATCH v4] build: check for broken AVX-512 compiler support
20/07/2021 15:36, Liang Ma: > From: Liang Ma > > GCC 6.3.0 has a known bug which related to _mm512_extracti64x4_epi64. > Please reference https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82887 > > Some DPDK PMD avx512 version heavily use _mm512_extracti64x4_epi6, > which cause building failure with debug buildtype. > > Therefore, it's helpful to check if compiler work with > _mm512_extracti64x4_epi6. > > This patch check the compiler compile result against the test code > snippet. If the checking is failed then disable avx512. > > Bugzilla ID: 717 > Fixes: e6a6a138919f (net/i40e: add AVX512 vector path) > Fixes: 808a17b3c1e6 (net/ice: add Rx AVX512 offload path) > Fixes: 4b64ccb328c9 (net/iavf: fix VLAN extraction in AVX512 path) > Cc: sta...@dpdk.org > > Reported-by: Liang Ma > Signed-off-by: Liang Ma > Acked-by: Bruce richardson Applied, thanks.
Re: [dpdk-dev] [PATCH] net/mlx5: fix SubFunction representor probe in isolate mode
> -Original Message-
> From: Xueming(Steven) Li
> Sent: Wednesday, July 7, 2021 14:53
> Cc: dev@dpdk.org; Xueming(Steven) Li ;
> sta...@dpdk.org; Matan Azrad ; Shahaf Shuler
> ; Slava Ovsiienko
> Subject: [PATCH] net/mlx5: fix SubFunction representor probe in isolate
> mode
>
> Representor failed to probe in isolated mode due to callback of retrieving
> representor info missing. This patch adds it back.
>
> Fixes: cb95feefdd03 ("net/mlx5: support sub-function representor")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Xueming Li
Acked-by: Viacheslav Ovsiienko
Re: [dpdk-dev] [PATCH] crypto/octeontx: enable build on non Linux OS
22/07/2021 11:17, Akhil Goyal:
> > Enabled build of Octeontx crypto PMD on non linux OS. Other Octeontx
> > PMDs are enabled already.
> >
> > This is to avoid ABI test failure on an OS once we add dependency
> > between a driver which is built to another which is not.
>
> Fixes: 8dc6c2f12ecf ("crypto/octeontx: add crypto adapter framework")
> >
>
> Reported-by: David Marchand
>
> > Signed-off-by: Shijith Thotton
>
> Acked-by: Akhil Goyal
>
> Thomas/David: please pick this patch directly on main to fix build on CI for
> FreeBSD.
Applied, thanks.
Re: [dpdk-dev] [PATCH] crypto/octeontx: enable build on non Linux OS
22/07/2021 21:06, Thomas Monjalon:
> 22/07/2021 11:17, Akhil Goyal:
> > > Enabled build of Octeontx crypto PMD on non linux OS. Other Octeontx
> > > PMDs are enabled already.
> > >
> > > This is to avoid ABI test failure on an OS once we add dependency
> > > between a driver which is built to another which is not.
> >
> > Fixes: 8dc6c2f12ecf ("crypto/octeontx: add crypto adapter framework")
> > >
> >
> > Reported-by: David Marchand
> >
> > > Signed-off-by: Shijith Thotton
> >
> > Acked-by: Akhil Goyal
> >
> > Thomas/David: please pick this patch directly on main to fix build on CI
> > for FreeBSD.
>
> Applied, thanks.
Please could you re-test the ABI on FreeBSD
and re-enable in the CI if the test is passing?
Thank you
Re: [dpdk-dev] [PATCH] maintainers: update for crypto API
22/07/2021 10:37, Akhil Goyal: > Claim ownership for crypto API layer. > Have been reviewing patches from quite some time. > > Signed-off-by: Akhil Goyal > --- > Crypto API > +M: Akhil Goyal > M: Declan Doherty > T: git://dpdk.org/next/dpdk-next-crypto > F: lib/cryptodev/ Applied, thanks, I thought you were already maintainer of this API.
Re: [dpdk-dev] [PATCH] bus/vmbus: Fix crash when handling packets in secondary process
21/07/2021 02:17, Long Li: > From: Stephen Hemminger > > > > Looks good, minor comment. You don't have to check for NULL before calling > > rte_free(). > > Rte_free(NULL) is a NOP like free(NULL). > > > > Sorry for top posting; but if you send to my Microsoft account you are stuck > > with what Outlook can do... > > > From: jerb > > > > Have secondary processes construct their own copy of primary channel with > > own mappings. > > > > Remove vmbus_channel primary ptr from struct mapped_vmbus_resource as > > its not used. > > > > Populate virtual memory address "addr" in struct rte_mem_resource for > > secondary processes as netvsc will attempt to reference it thus causing a > > crash. > > It was initialized for primary processes but not for secondary. > > > > Signed-off-by: jerb Please give your complete name. > Looks good. > > This should also go to stable. Please send a v2 with Fixes and Cc: sta...@dpdk.org lines as documented in the contributor's guide. If you don't know how to do, the maintainers can help. Thank you
Re: [dpdk-dev] [PATCH v1 1/1] power: fix multi-queue scale mode for pmd mgmt
21/07/2021 16:39, David Hunt:
> On 21/7/2021 3:26 PM, Anatoly Burakov wrote:
> > Currently in scale mode, multi-queue initialization will attempt to
> > initialize and de-initialize the per-lcore power library structures
> > multiple times. Fix it to only do this whenever we either enabling
> > first queue or disabling last queue.
> >
> > Fixes: 5dff9a72b0ef ("power: support callbacks for multiple Rx queues")
> >
> > Signed-off-by: Anatoly Burakov
>
> Fix looks good. Previous to this patch, was failing on adding second
> queue to a core, now with this patch, succeeds.
>
> Tested-by: David Hunt
Applied, thanks
Re: [dpdk-dev] [PATCH v1 1/1] power: check freq count before filling the freqs array
21/07/2021 11:27, Richael Zhuang: > The freqs array size is RTE_MAX_LCORE_FREQS. Before filling the > array with num_freqs elements, restrict the total num to > RTE_MAX_LCORE_FREQS. This fix aims to fix the coverity scan issue > like: > Overrunning array "pi->freqs" of 256 bytes by passing it to a > function which accesses it at byte offset 464. > > Coverity issue: 371913 > > Signed-off-by: Richael Zhuang Please provide "Fixes:" lines.
Re: [dpdk-dev] [PATCH v2 11/11] app/testpmd: add option to display extended statistics
On Thu, Jul 22, 2021 at 11:55 AM Andrew Rybchenko wrote: > > From: Ivan Ilchenko > > Add 'display-xstats' option for using in accompanying with Rx/Tx statistics > (i.e. 'stats-period' option or 'show port stats' interactive command) to > display specified list of extended statistics. > > Signed-off-by: Ivan Ilchenko > Signed-off-by: Andrew Rybchenko $ build/app/dpdk-testpmd -c 3 --no-huge -m 20 -a 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=2048 --stats-period 1 --display-xstats a, EAL: Detected 8 lcore(s) EAL: Detected 1 NUMA nodes EAL: Detected static linkage of DPDK EAL: Multi-process socket /run/user/114840/dpdk/rte/mp_socket EAL: Selected IOVA mode 'VA' max names is 2 ^^ Useless printf debug. I can remove it when applying. testpmd: create a new mbuf pool : n=2048, size=2176, socket=0 testpmd: preferred mempool ops selected: ring_mp_mc Configuring Port 0 (socket 0) Port 0: 26:45:E7:40:23:6E Configuring Port 1 (socket 0) Port 1: BA:5B:47:48:E6:AC Checking link statuses... No xstat 'a' on port 0 - skip it No xstat 'a' on port 1 - skip it Done No xstat 'a' on port 0 - skip it No xstat 'a' on port 1 - skip it xstats are resolved twice (at least?) per port. This is harmless afaics. Can you double check? No commandline core given, start packet forwarding io packet forwarding - ports=2 - cores=1 - streams=2 - NUMA support enabled, MP allocation mode: native -- David Marchand
Re: [dpdk-dev] [PATCH v2] net/sfc: fix broken build with clang 3.4.x
On Thu, Jul 22, 2021 at 11:12 AM David Marchand
wrote:
> On Thu, Jul 22, 2021 at 9:49 AM Andrew Rybchenko
> wrote:
> >
> > Old clang requires libatomic as well as gcc. Avoid compiler name and
> > version based checks. Add custom test for 16-byte atomic operations
> > to find out if libatomic is required to build.
> >
> > Bugzilla ID: 760
> > Fixes: 96fd2bd69b58 ("net/sfc: support flow action count in transfer rules")
> >
> > Signed-off-by: Andrew Rybchenko
> Acked-by: David Marchand
Applied, thanks.
--
David Marchand
Re: [dpdk-dev] [PATCH 1/3] bitrate: change reg implementation to match API description
09/07/2021 17:19, Kevin Traynor:
> rte_stats_bitrate_reg() API states it returns 'Zero on success'.
>
> However, the implementation directly returns the return of
> rte_metrics_reg_names() which may be zero or positive on success,
> with a positive value also indicating the index.
>
> The user of rte_stats_bitrate_reg() should not care about the
> index as it is stored in the opaque rte_stats_bitrates struct.
>
> Change the implementation of rte_stats_bitrate_reg() to match
> the API description by always returning zero on success.
>
> Fixes: 2ad7ba9a6567 ("bitrate: add bitrate statistics library")
>
> Signed-off-by: Kevin Traynor
Does it require a deprecation notice?
At least I suggest a release note in API section.
What is the target for this series? 21.11?
Re: [dpdk-dev] [PATCH v2] net: prepare the outer ipv4 hdr for checksum
07/07/2021 11:14, Mohsin Kazmi:
> On Wed, Jun 30, 2021 at 3:09 PM Olivier Matz wrote:
> > > + if (ol_flags & (PKT_TX_OUTER_IPV4 | PKT_TX_OUTER_IPV6)) {
> > > inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
> > > + /*
> > > + * prepare outer ipv4 header checksum by setting it to 0,
> > > + * in order to be computed by hardware NICs.
> > > + */
> > > + if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
> > > + ipv4_hdr = rte_pktmbuf_mtod_offset(m,
> > > + struct rte_ipv4_hdr *,
> > m->outer_l2_len);
> > > + ipv4_hdr->hdr_checksum = 0;
> > > + }
> > > + }
> >
> > What about outer L4 checksum? Does it requires the same than inner?
> >
> I am using XL710 for my testing with i40e dpdk driver. AFAIK, It doesn't
> support outer l4 checksum. I am not sure if other Intel NICs support it.
This function is used by a lot of drivers.
Try git grep rte_net_intel_cksum_prepare
I think we need more reviews on the v3.
Given it is far from being a new bug, I suggest to wait the next release
in order to have more feedbacks.
Re: [dpdk-dev] [dpdk-stable] [PATCH v3] net: fix Intel-specific Prepare the outer ipv4 hdr for checksum
+Cc more people for reviews.
07/07/2021 11:40, Mohsin Kazmi:
> Preparation the headers for the hardware offload
> misses the outer ipv4 checksum offload.
> It results in bad checksum computed by hardware NIC.
>
> This patch fixes the issue by setting the outer ipv4
> checksum field to 0.
nit: please write "IPv4" here and below.
> Fixes: 4fb7e803eb1a ("ethdev: add Tx preparation")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Mohsin Kazmi
> Acked-by: Qi Zhang
[...]
> @@ -125,11 +125,22 @@ rte_net_intel_cksum_flags_prepare(struct rte_mbuf *m,
> uint64_t ol_flags)
>* Mainly it is required to avoid fragmented headers check if
>* no offloads are requested.
>*/
> - if (!(ol_flags & (PKT_TX_IP_CKSUM | PKT_TX_L4_MASK | PKT_TX_TCP_SEG)))
> + if (!(ol_flags & (PKT_TX_IP_CKSUM | PKT_TX_L4_MASK | PKT_TX_TCP_SEG |
> + PKT_TX_OUTER_IP_CKSUM)))
> return 0;
>
> - if (ol_flags & (PKT_TX_OUTER_IPV4 | PKT_TX_OUTER_IPV6))
> + if (ol_flags & (PKT_TX_OUTER_IPV4 | PKT_TX_OUTER_IPV6)) {
> inner_l3_offset += m->outer_l2_len + m->outer_l3_len;
> + /*
> + * prepare outer ipv4 header checksum by setting it to 0,
> + * in order to be computed by hardware NICs.
> + */
> + if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
> + ipv4_hdr = rte_pktmbuf_mtod_offset(m,
> + struct rte_ipv4_hdr *, m->outer_l2_len);
> + ipv4_hdr->hdr_checksum = 0;
> + }
> + }
Re: [dpdk-dev] [PATCH v2] eal/windows: enforce alarm APIs parameter check
21/07/2021 17:28, Dmitry Kozlyuk:
> 2021-07-07 13:25 (UTC-0700), Jie Zhou:
> > eal/windows alarm APIs rte_eal_alarm_set and rte_eal_alarm_cancel
> > did not check parameters to fail fast for invalid parameters, which
> > caught by DPDK UT alarm_autotest.
> >
> > Enforce eal/windows alarm APIs parameter check to fail fast for
> > invalid parameters.
> >
> > Fixes: f4cbdbc7fbd2 ("eal/windows: implement alarm API")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Jie Zhou
>
> Acked-by: Dmitry Kozlyuk
Applied with title "eal/windows: check callback parameter of alarm functions"
Re: [dpdk-dev] [PATCH v2 11/11] app/testpmd: add option to display extended statistics
On Thu, Jul 22, 2021 at 11:55 AM Andrew Rybchenko
wrote:
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 8468018cf3..baffef1642 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -3609,6 +3609,62 @@ cmdline_parse_inst_t cmd_stop = {
>
> /* *** SET CORELIST and PORTLIST CONFIGURATION *** */
>
> +int
> +parse_xstats_list(char *in_str, struct rte_eth_xstat_name **xstats,
> + unsigned int *xstats_num)
I had saved this comment as it seemed harmless, but in_str can be constified.
And well, mingw build seems picky about it:
[5/20] Compiling C object app/dpdk-testpmd.exe.p/test-pmd_parameters.c.obj
FAILED: app/dpdk-testpmd.exe.p/test-pmd_parameters.c.obj
x86_64-w64-mingw32-gcc -Iapp/dpdk-testpmd.exe.p -Iapp -I../../dpdk/app
-Ilib/ethdev -I../../dpdk/lib/ethdev -I. -I../../dpdk -Iconfig
-I../../dpdk/config -Ilib/eal/include -I../../dpdk/lib/eal/include
-Ilib/eal/windows/include -I../../dpdk/lib/eal/windows/include
-Ilib/eal/x86/include -I../../dpdk/lib/eal/x86/include
-Ilib/eal/common -I../../dpdk/lib/eal/common -Ilib/eal
-I../../dpdk/lib/eal -Ilib/kvargs -I../../dpdk/lib/kvargs -Ilib/net
-I../../dpdk/lib/net -Ilib/mbuf -I../../dpdk/lib/mbuf -Ilib/mempool
-I../../dpdk/lib/mempool -Ilib/ring -I../../dpdk/lib/ring -Ilib/meter
-I../../dpdk/lib/meter -Ilib/metrics -I../../dpdk/lib/metrics
-Ilib/telemetry -I../../dpdk/lib/telemetry -Ilib/gro
-I../../dpdk/lib/gro -Ilib/gso -I../../dpdk/lib/gso -Ilib/cmdline
-I../../dpdk/lib/cmdline -Idrivers/bus/pci
-I../../dpdk/drivers/bus/pci -I../../dpdk/drivers/bus/pci/windows
-Ilib/pci -I../../dpdk/lib/pci -Ilib/bitratestats
-I../../dpdk/lib/bitratestats -Ilib/pdump -I../../dpdk/lib/pdump
-Ilib/latencystats -I../../dpdk/lib/latencystats -Idrivers/net/i40e
-I../../dpdk/drivers/net/i40e -Idrivers/net/i40e/base
-I../../dpdk/drivers/net/i40e/base -Idrivers/bus/vdev
-I../../dpdk/drivers/bus/vdev -Ilib/hash -I../../dpdk/lib/hash
-Ilib/rcu -I../../dpdk/lib/rcu -fdiagnostics-color=always -pipe
-D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Werror -O2 -g -include
rte_config.h -Wextra -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-address-of-packed-member -Wno-packed-not-aligned
-Wno-missing-field-initializers -D_GNU_SOURCE -D_WIN32_WINNT=0x0A00
-D__USE_MINGW_ANSI_STDIO -mno-avx512f -march=native
-DALLOW_EXPERIMENTAL_API -Wno-deprecated-declarations -MD -MQ
app/dpdk-testpmd.exe.p/test-pmd_parameters.c.obj -MF
app/dpdk-testpmd.exe.p/test-pmd_parameters.c.obj.d -o
app/dpdk-testpmd.exe.p/test-pmd_parameters.c.obj -c
../../dpdk/app/test-pmd/parameters.c
In file included from ../../dpdk/app/test-pmd/parameters.c:6:
../../dpdk/app/test-pmd/parameters.c: In function 'launch_args_parse':
../../dpdk/lib/eal/windows/include/getopt.h:38:16: error: passing
argument 1 of 'parse_xstats_list' discards 'const' qualifier from
pointer target type [-Werror=discarded-qualifiers]
38 | #define optarg usual_optarg
|^~~~
../../dpdk/app/test-pmd/parameters.c:699:28: note: in expansion of
macro 'optarg'
699 | rc = parse_xstats_list(optarg, &xstats_display,
|^~
In file included from ../../dpdk/app/test-pmd/parameters.c:46:
../../dpdk/app/test-pmd/testpmd.h:787:29: note: expected 'char *' but
argument is of type 'const char *'
787 | int parse_xstats_list(char *in_str, struct rte_eth_xstat_name **xstats,
| ~~^~
cc1: all warnings being treated as errors
[19/20] Compiling C object app/dpdk-testpmd.exe.p/test-pmd_cmdline.c.obj
--
David Marchand
Re: [dpdk-dev] [PATCH] build: enable to build on power10 or newer for ppc
On Wed, Jul 21, 2021 at 11:14 PM Thinh Tran wrote:
>
> A older version of complier would fail to generate code for new Power
compiler
> CPUs when it uses "-mcpu=native" argument.
> This patch will test if the compiler supports the current Power CPU type
> then proceeds with "-mcpu=native" argument, else it tries with older type.
> Limit to two older CPU type levels.
Such a change seems a bit late for 21.08, and is broken (see below).
In any case, I would need a review from ppc maintainer.
>
> Signed-off-by: Thinh Tran
> ---
> config/ppc/check_cpu_platform.sh | 2 ++
> config/ppc/meson.build | 40 +---
> 2 files changed, 34 insertions(+), 8 deletions(-)
> create mode 100644 config/ppc/check_cpu_platform.sh
>
> diff --git a/config/ppc/check_cpu_platform.sh
> b/config/ppc/check_cpu_platform.sh
> new file mode 100644
> index 00..cdea24561b
> --- /dev/null
> +++ b/config/ppc/check_cpu_platform.sh
> @@ -0,0 +1,2 @@
> +#! /bin/sh
> +LD_SHOW_AUXV=1 /bin/true | awk '/AT_PLATFORM/ {print $2}'|sed 's/\power//'
> diff --git a/config/ppc/meson.build b/config/ppc/meson.build
> index adf49e1f42..05aa860cfd 100644
> --- a/config/ppc/meson.build
> +++ b/config/ppc/meson.build
> @@ -7,16 +7,40 @@ endif
> dpdk_conf.set('RTE_ARCH', 'ppc_64')
> dpdk_conf.set('RTE_ARCH_PPC_64', 1)
>
> -# RHEL 7.x uses gcc 4.8.X which doesn't generate code for Power 9 CPUs,
> -# though it will detect a Power 9 CPU when the "-mcpu=native" argument
> -# is used, resulting in a build failure.
> -power9_supported = cc.has_argument('-mcpu=power9')
> -if not power9_supported
> -cpu_instruction_set = 'power8'
> -machine_args = ['-mcpu=power8', '-mtune=power8']
> -dpdk_conf.set('RTE_MACHINE','power8')
> +# Checking compiler for supporting Power CPU platform
> +# For newer Power(N) System that current gcc may not supoort it yet,
> +# it falls back and try N-1 and N-2
double space unneeded.
Plus, wording reads odd to me.
> +check_cpu = find_program(join_paths(meson.current_source_dir(),
> + 'check_cpu_platform.sh'))
Why do you need a separate script?
The value it returns is constant on a given system.
Looking at the script itself, this breaks cross compilation.
Compiler for C supports arguments -Wno-missing-field-initializers
-Wmissing-field-initializers: YES (cached)
Program /home/dmarchan/dpdk/config/ppc/check_cpu_platform.sh found: YES
../../dpdk/config/ppc/meson.build:18:0: ERROR: String 'x86_64' cannot
be converted to int
A full log can be found at
/home/dmarchan/builds/build-ppc64le-power8/meson-logs/meson-log.txt
FAILED: build.ninja
/usr/bin/meson --internal regenerate /home/dmarchan/dpdk
/home/dmarchan/builds/build-ppc64le-power8 --backend ninja
ninja: error: rebuilding 'build.ninja': subcommand failed
> +
> +target_cpu = run_command(check_cpu.path()).stdout().strip()
> +
> +cpu_int = target_cpu.to_int()
> +cpu_flag = '-mcpu=power@0@'
> +tune_flag = '-mtune=power@0@'
> +machine_type = 'power@0@'
> +debug = 'configure the compiler to build DPDK for POWER@0@ platform'
> +
> +if cc.has_argument(cpu_flag.format(cpu_int))
> +
> + # target system cpu is supported by the compiler, use '-mcpu=native'
> + message(debug.format(target_cpu+'_native'))
> + machine_args = ['-mcpu=native']
> + dpdk_conf.set('RTE_MACHINE',machine_type.format(cpu_int))
> +elif cc.has_argument(cpu_flag.format(cpu_int-1))
> + message(debug.format(cpu_int-1))
> + machine_args = [cpu_flag.format(cpu_int-1),tune_flag.format(cpu_int-1)]
> + dpdk_conf.set('RTE_MACHINE',machine_type.format(cpu_int-1))
> +elif cc.has_argument(cpu_flag.format(cpu_int-2))
> + message(debug.format(cpu_int-2))
> + machine_args = [cpu_flag.format(cpu_int-2),tune_flag.format(cpu_int-2)]
> + dpdk_conf.set('RTE_MACHINE',machine_type.format(cpu_int-2))
> +else
> + error('The compiler does not support POWER@0@ platform' .format(cpu_int))
> endif
>
> +
> +
One line is enough.
> # Certain POWER9 systems can scale as high as 1536 LCORES, but setting such a
> # high value can waste memory, cause timeouts in time limited autotests, and
> is
> # unlikely to be used in many production situations. Similarly, keeping the
> --
> 2.17.1
>
--
David Marchand
Re: [dpdk-dev] [PATCH] crypto/octeontx: enable build on non Linux OS
On Thu, Jul 22, 2021 at 3:08 PM Thomas Monjalon wrote:
>
> 22/07/2021 21:06, Thomas Monjalon:
> > 22/07/2021 11:17, Akhil Goyal:
> > > > Enabled build of Octeontx crypto PMD on non linux OS. Other Octeontx
> > > > PMDs are enabled already.
> > > >
> > > > This is to avoid ABI test failure on an OS once we add dependency
> > > > between a driver which is built to another which is not.
> > >
> > > Fixes: 8dc6c2f12ecf ("crypto/octeontx: add crypto adapter framework")
> > > >
> > >
> > > Reported-by: David Marchand
> > >
> > > > Signed-off-by: Shijith Thotton
> > >
> > > Acked-by: Akhil Goyal
> > >
> > > Thomas/David: please pick this patch directly on main to fix build on CI
> > > for FreeBSD.
> >
> > Applied, thanks.
>
> Please could you re-test the ABI on FreeBSD
> and re-enable in the CI if the test is passing?
>
> Thank you
I ran a couple test runs on FreeBSD 13 to ensure that the patch
compiles successfully, and I enabled reporting.
FreeBSD 13 should start to appear in the ABI test results of newer
tarballs with the patch.
Thanks,
Brandon
--
Brandon Lo
UNH InterOperability Laboratory
21 Madbury Rd, Suite 100, Durham, NH 03824
b...@iol.unh.edu
www.iol.unh.edu
[dpdk-dev] DPDK Release Status Meeting 22/07/2021
Release Dates - * v21.08 - Proposal/V1:Wednesday, 2 June (completed) - rc1:Saturday, 10 July (completed) - rc2:Friday,23 July - rc3:Thursday, 29 July - rc4:Wednesday, 4 August - Release:Friday, 6 August Subtrees * next-net - Bug with libatomic in clang, fixed today. * next-crypto - Pulled yesterday. - Only deprecation notices left for this release. - ABI check on FreeBSD: fixed today. * next-eventdev - Few patches for -rc3. * next-virtio - Pulled yesterday. - One more series to look at (was rejected later). - Change on async experimental code - candidate for -rc3 * next-net-brcm - No update. * next-net-intel - No update. * next-net-mlx - Integration in progress * next-net-mrvl - Few patches for -rc3. LTS --- DPDK 19.11.9 released on Monday by Christian. Call for help for 19.11.x to fix issues with new toolchains, kernels, etc.
Re: [dpdk-dev] [PATCH 1/3] bitrate: change reg implementation to match API description
On 22/07/2021 20:46, Thomas Monjalon wrote:
> 09/07/2021 17:19, Kevin Traynor:
>> rte_stats_bitrate_reg() API states it returns 'Zero on success'.
>>
>> However, the implementation directly returns the return of
>> rte_metrics_reg_names() which may be zero or positive on success,
>> with a positive value also indicating the index.
>>
>> The user of rte_stats_bitrate_reg() should not care about the
>> index as it is stored in the opaque rte_stats_bitrates struct.
>>
>> Change the implementation of rte_stats_bitrate_reg() to match
>> the API description by always returning zero on success.
>>
>> Fixes: 2ad7ba9a6567 ("bitrate: add bitrate statistics library")
>>
>> Signed-off-by: Kevin Traynor
>
> Does it require a deprecation notice?
I'm not certain, but I don't think it does. It is fixing the
implementation so it behaves as the API is documented to.
> At least I suggest a release note in API section.
>
> What is the target for this series? 21.11?
>
No urgency, 21.11 is fine for this set.
>
>
Re: [dpdk-dev] [dpdk-users] [DISCUSSION] code snippet documentation
15/07/2021 09:01, Asaf Penso:
> Hello DPDK community,
>
> I would like to bring up a discussion about a way to have code snippets as an
> example for proper usage.
> The DPDK tree is filled with great pieces of code that are well documented
> and maintained in high quality.
> I feel we are a bit behind when we talk about usage examples.
>
> One way, whenever we implement a new feature, is to extend one of the test-*
> under the "app" folder.
> This, however, provides means to test but doesn't provide a good usage
> example.
>
> Another way is to check the content of the "example" folder and whenever we
> have a BIG new feature it seems like a good place.
> This, however, doesn't provide a good option when we talk about small
> features.
> If, for example, we extend rte_flow with an extra action then providing a
> full-blown example application is somewhat an entry barrier.
>
> A third option could be to document it in one of the .rst files we have.
> Obviously, this requires high maintenance and no option to assure it still
> compiles.
>
> I'd like to propose another approach that will address the main two issues:
> remove the entry barrier and assure compilation.
> In this approach, inside the "examples" folder we'll create another folder
> for "snippets".
> Inside "snippets" we'll have several files per category, for example,
> rte_flow_snippets.c
> Each .c file will include a main function that calls the different use cases
> we want to give as an example.
> The purpose is not to generate traffic nor read rx/tx packets from the DPDK
> ports.
> The purpose is to have a good example that compiles properly.
>
> Taking the rte_flow_snippets.c as an example its main function would look
> like this:
>
> int
> main(int argc, char **argv)
> {
> rte_flow_snippet_match_5tuple_and_drop();
> rte_flow_snippet_match_geneve_ope_and_rss();
> ...
> Return 0;
> }
I think we need to have a policy or justification about which snippet
is worth to have.
My thought is to avoid creating snippets which have no other value
than showing a function call.
I think there is a value if the context is not simple.
Please could you provide a more complex example?
Re: [dpdk-dev] [PATCH v3] eal: allow hugetlbfs sub-directories
On Thu, Jul 8, 2021 at 1:00 PM John Levon wrote: > > get_hugepage_dir() was implemented in such a way that a --huge-dir > option had to exactly match the mountpoint, but there's no reason for > this restriction. Fix the implementation to allow a sub-directory within > a suitable hugetlbfs mountpoint to be specified, preferring the closest > match. > > Signed-off-by: John Levon This change in EAL hugetlbfs discovery is too dangerous to be taken after -rc1. Could you give some usecases/examples on why this change is needed? Updating the documentation and the unit test also seem necessary. -- David Marchand
Re: [dpdk-dev] [PATCH] crypto/octeontx: enable build on non Linux OS
22/07/2021 22:20, Brandon Lo:
> On Thu, Jul 22, 2021 at 3:08 PM Thomas Monjalon wrote:
> >
> > 22/07/2021 21:06, Thomas Monjalon:
> > > 22/07/2021 11:17, Akhil Goyal:
> > > > > Enabled build of Octeontx crypto PMD on non linux OS. Other Octeontx
> > > > > PMDs are enabled already.
> > > > >
> > > > > This is to avoid ABI test failure on an OS once we add dependency
> > > > > between a driver which is built to another which is not.
> > > >
> > > > Fixes: 8dc6c2f12ecf ("crypto/octeontx: add crypto adapter framework")
> > > > >
> > > >
> > > > Reported-by: David Marchand
> > > >
> > > > > Signed-off-by: Shijith Thotton
> > > >
> > > > Acked-by: Akhil Goyal
> > > >
> > > > Thomas/David: please pick this patch directly on main to fix build on
> > > > CI for FreeBSD.
> > >
> > > Applied, thanks.
> >
> > Please could you re-test the ABI on FreeBSD
> > and re-enable in the CI if the test is passing?
> >
> > Thank you
>
> I ran a couple test runs on FreeBSD 13 to ensure that the patch
> compiles successfully, and I enabled reporting.
> FreeBSD 13 should start to appear in the ABI test results of newer
> tarballs with the patch.
Thanks a lot Brandon, well managed.
Re: [dpdk-dev] [PATCH v3] eal: allow hugetlbfs sub-directories
On Thu, Jul 22, 2021 at 10:29:45PM +0200, David Marchand wrote: > On Thu, Jul 8, 2021 at 1:00 PM John Levon wrote: > > > > get_hugepage_dir() was implemented in such a way that a --huge-dir > > option had to exactly match the mountpoint, but there's no reason for > > this restriction. Fix the implementation to allow a sub-directory within > > a suitable hugetlbfs mountpoint to be specified, preferring the closest > > match. > > > > Signed-off-by: John Levon > > This change in EAL hugetlbfs discovery is too dangerous to be taken after > -rc1. Sure. > Could you give some usecases/examples on why this change is needed? Would you like me to expand the commit message? I had hoped it was clear enough, but I suppose not. Simply put, DPDK above is assuming its the only user of hugepages on the system - including clear_hugedir(). That is certainly not the case for our use cases. > Updating the documentation https://doc.dpdk.org/guides/linux_gsg/linux_eal_parameters.html """ --huge-dir Use specified hugetlbfs directory instead of autodetected ones. """ That is, it already says "directory", not "mount". You'd like something additional saying it can be below a mount point? > and the unit test also seem necessary. You're talking about app/test/test_eal_flags.c or something else? thanks, john
Re: [dpdk-dev] DPDK Packet drop/Out of sequence issue with Jumbo frames on MLX ConnectX-4 Lx NIC
Can you tell what's the mbuf size? Max packet Len? MTU size?
Regards,
Asaf Penso
From: Balbeer Tiwari
Sent: Tuesday, July 20, 2021 6:17:50 PM
To: Asaf Penso ; dev@dpdk.org
Subject: RE: [dpdk-dev] DPDK Packet drop/Out of sequence issue with Jumbo
frames on MLX ConnectX-4 Lx NIC
Hi Asaf,
Please see details, let me know if more details required.
What DPDK version do you use?
DPDK-20.05
What EAL and devargs value are being used?
Following are the argv at eal_init
1005 if (rte_eal_init(argc, argv) < 0)
(gdb) p argv
$7 = {0x72bce60 "fast_pkt_app", 0x72bce90 "-c 6c", 0x72bcec0 "-m 1024",
0x72bcef0 "-n 1", 0x72bcf20 "--file-prefix=gnb_du_rte_config", 0x72bcf50
"--proc-type=auto", 0x72bcf80 "-w:c1:00.0",
0x72bcfb0 "-w:c1:00.1", 0x72bcfe0 "", 0x72bd010 ""}
(gdb)
Regards,
Balbeer
-Original Message-
From: Asaf Penso
Sent: Tuesday, July 20, 2021 8:03 PM
To: Balbeer Tiwari ; dev@dpdk.org
Subject: RE: [dpdk-dev] DPDK Packet drop/Out of sequence issue with Jumbo
frames on MLX ConnectX-4 Lx NIC
The e-mail below is from an external source. Please do not open attachments or
click links from an unknown or suspicious origin.
Hello Balbeer,
Can you please provide more details?
What DPDK version do you use?
What EAL and devargs value are being used?
Regards,
Asaf Penso
>-Original Message-
>From: dev mailto:dev-boun...@dpdk.org>> On Behalf Of
>Balbeer Tiwari
>Sent: Tuesday, July 20, 2021 4:54 PM
>To: dev@dpdk.org
>Subject: [dpdk-dev] DPDK Packet drop/Out of sequence issue with Jumbo
>frames on MLX ConnectX-4 Lx NIC
>
>Hi There,
>
>I am using MLX ConnectX-4 Lx with DPDK and with this I am observing
>packet drops at receiver if size greater than ~8k(jumbo) frames.
>Smaller packets go through fine. Its being run directly on baremetal
>with Centos 8.3. Please can you share if there is any fix for this.
>
>Regards,
>Balbeer
[dpdk-dev] [PATCH v2] eal: fix argument to rte_bsf32_safe
The first argument to rte_bsf32_safe was incorrectly declared as
a 64 bit value. The code only works on 32 bit values and the underlying
function rte_bsf32 only accepts 32 bit values. This was a mistake
introduced when the safe version was added and probaly cause
by copy/paste from the 64 bit version.
The bug passed silently under the radar until some other code was
built with -Wall and -Wextra in C++ and C++ complains about the
missing cast.
Yes, this is a API signature change, but the original code was wrong.
It is an inline so not an ABI change.
Fixes: 4e261f551986 ("eal: add 64-bit bsf and 32-bit safe bsf functions")
Cc: anatoly.bura...@intel.com
Signed-off-by: Stephen Hemminger
Acked-By: Tyler Retzlaff
---
v2 - add suggested release note
doc/guides/rel_notes/release_21_08.rst | 4
lib/eal/include/rte_common.h | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/doc/guides/rel_notes/release_21_08.rst
b/doc/guides/rel_notes/release_21_08.rst
index e2c5ccbf7d90..148405891fcb 100644
--- a/doc/guides/rel_notes/release_21_08.rst
+++ b/doc/guides/rel_notes/release_21_08.rst
@@ -196,6 +196,10 @@ API Changes
to be thread safe; all Rx queues affected by the API will now need to be
stopped before making any changes to the power management scheme.
+* eal: ``rte_bsf32_safe`` now takes a 32 bit value for its first
+ argument. This fixes warnings about loss of precision when used
+ with some compilers settings.
+
ABI Changes
---
diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index d5a32c66a5fe..99eb5f1820ae 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -623,7 +623,7 @@ rte_bsf32(uint32_t v)
* Returns 0 if ``v`` was 0, otherwise returns 1.
*/
static inline int
-rte_bsf32_safe(uint64_t v, uint32_t *pos)
+rte_bsf32_safe(uint32_t v, uint32_t *pos)
{
if (v == 0)
return 0;
--
2.30.2
Re: [dpdk-dev] [PATCH v1 1/1] power: check freq count before filling the freqs array
> -Original Message- > From: Thomas Monjalon > Sent: Friday, July 23, 2021 3:43 AM > To: Richael Zhuang > Cc: dev@dpdk.org; David Hunt > Subject: Re: [dpdk-dev] [PATCH v1 1/1] power: check freq count before filling > the freqs array > > 21/07/2021 11:27, Richael Zhuang: > > The freqs array size is RTE_MAX_LCORE_FREQS. Before filling the array > > with num_freqs elements, restrict the total num to > > RTE_MAX_LCORE_FREQS. This fix aims to fix the coverity scan issue > > like: > > Overrunning array "pi->freqs" of 256 bytes by passing it to a function > > which accesses it at byte offset 464. > > > > Coverity issue: 371913 > > > > Signed-off-by: Richael Zhuang > > Please provide "Fixes:" lines. > > Thanks, I will add it.
[dpdk-dev] [PATCH v2 0/1] power: check freq count before filling the freqs
v1: add check for freq count v2: add "Fixes" tag in commit message Richael Zhuang (1): power: check freq count before filling the freqs array lib/power/power_cppc_cpufreq.c | 5 + lib/power/power_pstate_cpufreq.c | 5 + 2 files changed, 10 insertions(+) -- 2.20.1
[dpdk-dev] [PATCH v2 1/1] power: check freq count before filling the freqs array
The freqs array size is RTE_MAX_LCORE_FREQS. Before filling the
array with num_freqs elements, restrict the total num to
RTE_MAX_LCORE_FREQS. This fix aims to fix the coverity scan issue
like:
Overrunning array "pi->freqs" of 256 bytes by passing it to a
function which accesses it at byte offset 464.
Coverity issue: 371913
Fixes: 82432c45d631 ("power: check freq count before filling")
Cc: richael.zhu...@arm.com
Cc: sta...@dpdk.org
Signed-off-by: Richael Zhuang
---
lib/power/power_cppc_cpufreq.c | 5 +
lib/power/power_pstate_cpufreq.c | 5 +
2 files changed, 10 insertions(+)
diff --git a/lib/power/power_cppc_cpufreq.c b/lib/power/power_cppc_cpufreq.c
index e92973ab54..db63c2cc10 100644
--- a/lib/power/power_cppc_cpufreq.c
+++ b/lib/power/power_cppc_cpufreq.c
@@ -246,6 +246,11 @@ power_get_available_freqs(struct cppc_power_info *pi)
pi->nominal_perf * UNIT_DIFF : pi->nominal_perf;
num_freqs = (nominal_perf - scaling_min_freq) / BUS_FREQ + 1 +
pi->turbo_available;
+ if (num_freqs >= RTE_MAX_LCORE_FREQS) {
+ RTE_LOG(ERR, POWER, "Too many available frequencies : %d\n",
+ num_freqs);
+ goto out;
+ }
/* Generate the freq bucket array. */
for (i = 0, pi->nb_freqs = 0; i < num_freqs; i++) {
diff --git a/lib/power/power_pstate_cpufreq.c b/lib/power/power_pstate_cpufreq.c
index 3b607515fd..619090c8d1 100644
--- a/lib/power/power_pstate_cpufreq.c
+++ b/lib/power/power_pstate_cpufreq.c
@@ -419,6 +419,11 @@ power_get_available_freqs(struct pstate_power_info *pi)
*/
num_freqs = (base_max_freq - sys_min_freq) / BUS_FREQ + 1 +
pi->turbo_available;
+ if (num_freqs >= RTE_MAX_LCORE_FREQS) {
+ RTE_LOG(ERR, POWER, "Too many available frequencies : %d\n",
+ num_freqs);
+ goto out;
+ }
/* Generate the freq bucket array.
* If turbo is available the freq bucket[0] value is base_max +1
--
2.20.1
Re: [dpdk-dev] Question about hardware error handling policy
On 2021/7/22 23:46, Thomas Monjalon wrote: > 22/07/2021 15:50, fengchengwen: >> Hi, all >> >> I notice ethdev support dev_reset ops, which could be used to recover >> from >> errors, and only 13+ drivers support this function. >> And also there is event for reset: RTE_ETH_EVENT_INTR_RESET, and only 6 >> drivers support it (most of them are VF). >> >> This provides users with two ways to handle hardware errors: >> a. driver report RTE_ETH_EVENT_INTR_RESET, and application do reset ops. >> b. application detect errors (the detection method is unclear), and call >> reset ops to recover. >> >> According to the design of this API, error handling is assigned to the >> application, and the driver is only responsible for reporting events. This >> simplifies the driver design (for example, the driver does not need to >> maintain >> mutex locks). >> >> As we know, many modern NICs come with firmware, have PCIE interfaces, >> support SR-IOV, the hardware errors can have: firmware reboot/PF reset/ >> VF reset/FLR, but these errors(particularly firmware/PF) are not addressed in >> most drivers. >> >> Question 1: what do we think of these errors(particularly firmware/PF)? >> Do >> we think that the probability is very low and that there is no need to deal >> with >> them? > > Even rare errors must be managed. Because intel and mlx NIC are widely used, I look at i40e/mlx5 driver code, and found: 1) i40e PF driver, it only show logs when detect global reset and other error: if (icr0 & I40E_PFINT_ICR0_GRST_MASK) PMD_DRV_LOG(INFO, "ICR0: global reset requested"); if (icr0 & I40E_PFINT_ICR0_PCI_EXCEPTION_MASK) PMD_DRV_LOG(INFO, "ICR0: PCI exception activated"); if (icr0 & I40E_PFINT_ICR0_STORM_DETECT_MASK) PMD_DRV_LOG(INFO, "ICR0: a change in the storm control state"); @Beilei Why not report RESET_EVENT in these cases ? or these error are very rarely so just report it. And also, the application may still send or recv packet, These resets, if not handled correctly, do not cause the hardware/driver to hang ? 2) mlx5 PF driver, I notice there is a mlx5_dev_interrupt_device_fatal which will remove the device. @Matan Why not report RESET_EVENT in these cases ? so the application can be recovered quickly. > >> Question 2: I prefer to put error handling in the application layer, >> because >> doing it in the driver can make the driver complex, but there is no app to >> register the INTR_RESET event handler. I think we can build a standard >> handler >> in testpmd, What do you think? > > Absolutely. As any ethdev API, it must be tested with testpmd. > > > . >
[dpdk-dev] [PATCH v3 0/1] power: check freq count before filling the freqs array
v1: add check for freq count v2: add "Fixes" tag in commit message v3: update commit message Richael Zhuang (1): power: check freq count before filling the freqs array lib/power/power_cppc_cpufreq.c | 5 + lib/power/power_pstate_cpufreq.c | 5 + 2 files changed, 10 insertions(+) -- 2.20.1
[dpdk-dev] [PATCH v3 1/1] power: check freq count before filling the freqs array
The freqs array size is RTE_MAX_LCORE_FREQS. Before filling the
array with num_freqs elements, restrict the total num to
RTE_MAX_LCORE_FREQS. This fix aims to fix the coverity scan issue
like:
Overrunning array "pi->freqs" of 256 bytes by passing it to a
function which accesses it at byte offset 464.
Coverity issue: 371913
Fixes: ef1cc88f1837 ("power: support cppc_cpufreq driver")
Cc: richael.zhu...@arm.com
Cc: sta...@dpdk.org
Signed-off-by: Richael Zhuang
---
lib/power/power_cppc_cpufreq.c | 5 +
lib/power/power_pstate_cpufreq.c | 5 +
2 files changed, 10 insertions(+)
diff --git a/lib/power/power_cppc_cpufreq.c b/lib/power/power_cppc_cpufreq.c
index e92973ab54..db63c2cc10 100644
--- a/lib/power/power_cppc_cpufreq.c
+++ b/lib/power/power_cppc_cpufreq.c
@@ -246,6 +246,11 @@ power_get_available_freqs(struct cppc_power_info *pi)
pi->nominal_perf * UNIT_DIFF : pi->nominal_perf;
num_freqs = (nominal_perf - scaling_min_freq) / BUS_FREQ + 1 +
pi->turbo_available;
+ if (num_freqs >= RTE_MAX_LCORE_FREQS) {
+ RTE_LOG(ERR, POWER, "Too many available frequencies : %d\n",
+ num_freqs);
+ goto out;
+ }
/* Generate the freq bucket array. */
for (i = 0, pi->nb_freqs = 0; i < num_freqs; i++) {
diff --git a/lib/power/power_pstate_cpufreq.c b/lib/power/power_pstate_cpufreq.c
index 3b607515fd..619090c8d1 100644
--- a/lib/power/power_pstate_cpufreq.c
+++ b/lib/power/power_pstate_cpufreq.c
@@ -419,6 +419,11 @@ power_get_available_freqs(struct pstate_power_info *pi)
*/
num_freqs = (base_max_freq - sys_min_freq) / BUS_FREQ + 1 +
pi->turbo_available;
+ if (num_freqs >= RTE_MAX_LCORE_FREQS) {
+ RTE_LOG(ERR, POWER, "Too many available frequencies : %d\n",
+ num_freqs);
+ goto out;
+ }
/* Generate the freq bucket array.
* If turbo is available the freq bucket[0] value is base_max +1
--
2.20.1
Re: [dpdk-dev] [PATCH] net/iavf: fix tx thresh check issue
> -Original Message-
> From: Li, Xiaoyun
> Sent: Thursday, July 22, 2021 3:56 PM
> To: dev@dpdk.org; Wu, Jingjing ; Xing, Beilei
>
> Cc: Li, Xiaoyun ; sta...@dpdk.org
> Subject: [PATCH] net/iavf: fix tx thresh check issue
>
> Function check_tx_thresh is called with wrong parameter. If the check fails,
> tx_queue_setup should return error not keep going.
> iThis patch fixes above issues.
Typo: This
Except that,
Acked-by: Beilei Xing
>
> Fixes: 69dd4c3d0898 ("net/avf: enable queue and device")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Xiaoyun Li
> ---
> drivers/net/iavf/iavf_rxtx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/iavf/iavf_rxtx.c b/drivers/net/iavf/iavf_rxtx.c index
> d61b32fcee..e33fe4576b 100644
> --- a/drivers/net/iavf/iavf_rxtx.c
> +++ b/drivers/net/iavf/iavf_rxtx.c
> @@ -708,7 +708,8 @@ iavf_dev_tx_queue_setup(struct rte_eth_dev *dev,
> tx_conf->tx_rs_thresh : DEFAULT_TX_RS_THRESH);
> tx_free_thresh = (uint16_t)((tx_conf->tx_free_thresh) ?
> tx_conf->tx_free_thresh : DEFAULT_TX_FREE_THRESH);
> - check_tx_thresh(nb_desc, tx_rs_thresh, tx_rs_thresh);
> + if (check_tx_thresh(nb_desc, tx_rs_thresh, tx_free_thresh) != 0)
> + return -EINVAL;
>
> /* Free memory if needed. */
> if (dev->data->tx_queues[queue_idx]) {
> --
> 2.25.1
[dpdk-dev] [PATCH v1 0/4] fix note error
Fix drivers/net note error and do some optimization for i40e NEON path. Feifei Wang (4): drivers/net: remove redundant phrases drivers/net: fix note error for Rx vector net/i40e: reorder Rx NEON code for better readability net/i40e: change code order to reduce L1 cache misses drivers/net/fm10k/fm10k_rxtx_vec.c | 6 +- drivers/net/i40e/i40e_rxtx_vec_altivec.c | 10 +-- drivers/net/i40e/i40e_rxtx_vec_neon.c| 101 ++- drivers/net/i40e/i40e_rxtx_vec_sse.c | 6 +- drivers/net/iavf/iavf_rxtx_vec_sse.c | 12 +-- drivers/net/ice/ice_rxtx_vec_sse.c | 6 +- drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 6 +- 7 files changed, 68 insertions(+), 79 deletions(-) -- 2.25.1
[dpdk-dev] [PATCH v1 1/4] drivers/net: remove redundant phrases
For the note of Rx vec path,when extract and record EOP bit, the code
note should be "as the count of dd bits doesn't care", remove the
redundant "count".
fm10k:
Fixes: 7092be8437bd ("fm10k: add vector Rx")
Cc: jing.d.c...@intel.com
i40e-altive:
Fixes: c3def6a8724c ("net/i40e: implement vector PMD for altivec")
Cc: gowrishanka...@linux.vnet.ibm.com
i40e-neon:
Fixes: ae0eb310f253 ("net/i40e: implement vector PMD for ARM")
i40e-sse:
Fixes: 9ed94e5bb04e ("i40e: add vector Rx")
Cc: zhe@intel.com
iavf:
Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx")
Cc: jingjing...@intel.com
Fixes: 1162f5a0ef31 ("net/iavf: support flexible Rx descriptor in SSE path")
Cc: leyi.r...@intel.com
ice:
Fixes: c68a52b8b38c ("net/ice: support vector SSE in Rx")
Cc: wenzhuo...@intel.com
ixgbe:
Fixes: cf4b4708a88a ("ixgbe: improve slow-path perf with vector scattered Rx")
Cc: bruce.richard...@intel.com
Cc: sta...@dpdk.org
Signed-off-by: Feifei Wang
Reviewed-by: Ruifeng Wang
---
drivers/net/fm10k/fm10k_rxtx_vec.c | 2 +-
drivers/net/i40e/i40e_rxtx_vec_altivec.c | 2 +-
drivers/net/i40e/i40e_rxtx_vec_neon.c| 2 +-
drivers/net/i40e/i40e_rxtx_vec_sse.c | 2 +-
drivers/net/iavf/iavf_rxtx_vec_sse.c | 4 ++--
drivers/net/ice/ice_rxtx_vec_sse.c | 2 +-
drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c | 2 +-
7 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/fm10k/fm10k_rxtx_vec.c
b/drivers/net/fm10k/fm10k_rxtx_vec.c
index 39e3cdac1f..cae5322d48 100644
--- a/drivers/net/fm10k/fm10k_rxtx_vec.c
+++ b/drivers/net/fm10k/fm10k_rxtx_vec.c
@@ -544,7 +544,7 @@ fm10k_recv_raw_pkts_vec(void *rx_queue, struct rte_mbuf
**rx_pkts,
/* and with mask to extract bits, flipping 1-0 */
__m128i eop_bits = _mm_andnot_si128(staterr, eop_check);
/* the staterr values are not in order, as the count
-* count of dd bits doesn't care. However, for end of
+* of dd bits doesn't care. However, for end of
* packet tracking, we do care, so shuffle. This also
* compresses the 32-bit values to 8-bit
*/
diff --git a/drivers/net/i40e/i40e_rxtx_vec_altivec.c
b/drivers/net/i40e/i40e_rxtx_vec_altivec.c
index 1ad74646d6..edaa462ac8 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_altivec.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_altivec.c
@@ -398,7 +398,7 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *rxq, struct
rte_mbuf **rx_pkts,
(vector unsigned char)vec_nor(staterr, staterr),
(vector unsigned char)eop_check);
/* the staterr values are not in order, as the count
-* count of dd bits doesn't care. However, for end of
+* of dd bits doesn't care. However, for end of
* packet tracking, we do care, so shuffle. This also
* compresses the 32-bit values to 8-bit
*/
diff --git a/drivers/net/i40e/i40e_rxtx_vec_neon.c
b/drivers/net/i40e/i40e_rxtx_vec_neon.c
index 1f5539bda8..32336fdb80 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_neon.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_neon.c
@@ -387,7 +387,7 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *__rte_restrict rxq,
eop_bits = vmvnq_u8(vreinterpretq_u8_u16(staterr));
eop_bits = vandq_u8(eop_bits, eop_check);
/* the staterr values are not in order, as the count
-* count of dd bits doesn't care. However, for end of
+* of dd bits doesn't care. However, for end of
* packet tracking, we do care, so shuffle. This also
* compresses the 32-bit values to 8-bit
*/
diff --git a/drivers/net/i40e/i40e_rxtx_vec_sse.c
b/drivers/net/i40e/i40e_rxtx_vec_sse.c
index bfa5aff48d..03a0320353 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_sse.c
+++ b/drivers/net/i40e/i40e_rxtx_vec_sse.c
@@ -557,7 +557,7 @@ _recv_raw_pkts_vec(struct i40e_rx_queue *rxq, struct
rte_mbuf **rx_pkts,
/* and with mask to extract bits, flipping 1-0 */
__m128i eop_bits = _mm_andnot_si128(staterr, eop_check);
/* the staterr values are not in order, as the count
-* count of dd bits doesn't care. However, for end of
+* of dd bits doesn't care. However, for end of
* packet tracking, we do care, so shuffle. This also
* compresses the 32-bit values to 8-bit
*/
diff --git a/drivers/net/iavf/iavf_rxtx_vec_sse.c
b/drivers/net/iavf/iavf_rxtx_vec_sse.c
index bf87696fa4..b813d96ef4 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_sse.c
+++ b/drivers/net/iavf/iavf_rxtx_vec_sse.c
@@ -59
