Re: [PATCH v1 1/3] ethdev: introduce maximum Rx buffer size
On 8/15/2023 12:10 PM, Huisong Li wrote:
> The Rx buffer size stands for the size hardware supported to receive
> packets in one mbuf. The "min_rx_bufsize" is the minimum buffer hardware
> supported in Rx.
Hi Huisong,
I guess I understand the intention, but above description is not accurate,
ethdev Rx buffer sizes are not per mbuf. Device internal Rx buffer can
be bigger and received packet can be represented by multiple
descriptors. Each descriptor maps to an mbuf.
Like device can support 8K packets, this is informed with
'max_rx_pktlen', and if you provide 1K mbufs, device can receives a 8K
buffer and chains 8 mbufs to represent packet.
> Actually, some engines also have the maximum buffer
> specification, like, hns3. For these engines, the available data size
> of one mbuf in Rx also depends on the maximum buffer hardware supported.
> So introduce maximum Rx buffer size in struct rte_eth_dev_info to report
> user to avoid memory waste. And driver should accept it and just pass
> maximum buffer hardware supported to hardware if application specifies
> the Rx buffer size is greater than the maximum buffer.
>
If I understand correctly, you want to notify user about per descriptor
max buffer size, and I can see that is 4K for hns3.
Which means even if device can receive larger packets, each
descriptor/mbuf can't have more than 4K.
So user allocating pktmbuf pool with mbuf bigger than 4K is wasting
memory, and you are trying to prevent it, is this understanding correct?
No objection device sharing this information, but we should make it
clear what it is to not confuse users, please check below comments.
> Signed-off-by: Huisong Li
> ---
> lib/ethdev/rte_ethdev.c | 7 +++
> lib/ethdev/rte_ethdev.h | 6 ++
> 2 files changed, 13 insertions(+)
>
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 0840d2b594..9985bd3049 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -2068,6 +2068,7 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t
> rx_queue_id,
> struct rte_eth_dev *dev;
> struct rte_eth_dev_info dev_info;
> struct rte_eth_rxconf local_conf;
> + uint32_t vld_bufsize;
>
> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> dev = &rte_eth_devices[port_id];
> @@ -2105,6 +2106,11 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t
> rx_queue_id,
> return ret;
>
> mbp_buf_size = rte_pktmbuf_data_room_size(mp);
> + vld_bufsize = mbp_buf_size - RTE_PKTMBUF_HEADROOM;
> + if (vld_bufsize > dev_info.max_rx_bufsize)
> + RTE_ETHDEV_LOG(WARNING,
> + "Ethdev port_id=%u Rx buffer size (%u) is
> greater than the maximum buffer size (%u) driver supported.\n",
> + port_id, vld_bufsize, dev_info.max_rx_bufsize);
>
I am not sure about this check in the ethdev level, application already
getting this value, intention is optimization but a warning message can
be confusing to the user,
even if we keep the log, at least log level shouldn't be warning,
nothing wrong to warn, maybe info or debug level, and I think message
should be updated, this is not about Rx buffer size but max size per mbuf.
What do you think to move this log to testpmd, it can be also a sample
how to use this new field in application level?
> } else if (rx_conf != NULL && rx_conf->rx_nseg > 0) {
> const struct rte_eth_rxseg_split *rx_seg;
> uint16_t n_seg;
> @@ -3689,6 +3695,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct
> rte_eth_dev_info *dev_info)
> dev_info->min_mtu = RTE_ETHER_MIN_LEN - RTE_ETHER_HDR_LEN -
> RTE_ETHER_CRC_LEN;
> dev_info->max_mtu = UINT16_MAX;
> + dev_info->max_rx_bufsize = UINT32_MAX;
>
> if (*dev->dev_ops->dev_infos_get == NULL)
> return -ENOTSUP;
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 04a2564f22..9fdf2c75ee 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1724,6 +1724,12 @@ struct rte_eth_dev_info {
> uint16_t max_mtu; /**< Maximum MTU allowed */
> const uint32_t *dev_flags; /**< Device flags */
> uint32_t min_rx_bufsize; /**< Minimum size of Rx buffer. */
> + /**
> + * Maximum size of Rx buffer. Driver should accept it and just pass
> + * this value to HW if application specifies the Rx buffer size is
> + * greater than this value.
> + */
Above comment can be simplified, as something like:
"
Maximum buffer size supported per Rx descriptor by HW.
The value is not enforced, information only to application to optimize
mbuf size.
"
> + uint32_t max_rx_bufsize;
>
What about renaming variable, to something like:
"max_desc_bufsize" /**< Maximum buffer size per Rx descriptor. */
> uint32_t max_rx_pktlen; /**< Maximum configurable length of Rx pkt. */
> /** Maximum configurab
Re: [PATCH] doc: add note about VMXNET3 on AMD with ESXI.
On 8/14/2023 12:55 PM, Igor de Paula wrote: > This patch notes the requirements for ESXI version 7.0.1 > or higher when using IOVA as VA. > > Signed-off-by: Igor de Paula > Thanks Igor for the contribution. Reviewed-by: Ferruh Yigit Applied to dpdk-next-net/main, thanks.
Re: [PATCH v2] app/testpmd: add flush multicast MAC address command
On 8/2/2023 7:23 AM, Dengdui Huang wrote:
> Add command to flush multicast MAC address
> Usage:
> mcast_addr flush :
> flush multicast MAC address on port_id
>
> Signed-off-by: Dengdui Huang
> ---
> app/test-pmd/cmdline.c | 43 +
> app/test-pmd/config.c | 18 +
> app/test-pmd/testpmd.h | 1 +
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 8
> 4 files changed, 70 insertions(+)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0d0723f659..d5a3bd2287 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -516,6 +516,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> "set allmulti (port_id|all) (on|off)\n"
> "Set the allmulti mode on port_id, or all.\n\n"
>
> + "mcast_addr flush (port_id)\n"
> + "To flush the set of multicast addresses.\n\n"
> +>
I assume 'set of multicast address' comes from the API getting
'mc_addr_set' as parameter, but using 'set' is not useful in the command
description, what about:
"Flush all multicast MAC addresses on port_id."
Similarly there are logs/documents below refers as "set of multicast
addresses", does it make sense to replace all as "all multicast MAC
addresses"?
And relating to the ordering, as you said "mcast_addr add|remove ..." is
missing, which is contributing to the confusion.
Can you please add this first (in a separate patch) to just below
"mac_addr .*" group, later put "mcast_addr flush .*' below it in this patch?
> "set flow_ctrl rx (on|off) tx (on|off) (high_water)"
> " (low_water) (pause_time) (send_xon)
> mac_ctrl_frame_fwd"
> " (on|off) autoneg (on|off) (port_id)\n"
> @@ -8561,6 +8564,45 @@ static cmdline_parse_inst_t cmd_mcast_addr = {
> },
> };
>
> +/* *** FLUSH MULTICAST MAC ADDRESS ON PORT *** */
> +struct cmd_mcast_addr_flush_result {
> + cmdline_fixed_string_t mcast_addr_cmd;
> + cmdline_fixed_string_t what;
> + uint16_t port_num;
> +};
> +
> +static void cmd_mcast_addr_flush_parsed(void *parsed_result,
> + __rte_unused struct cmdline *cl,
> + __rte_unused void *data)
> +{
> + struct cmd_mcast_addr_flush_result *res = parsed_result;
> +
> + mcast_addr_flush(res->port_num);
> +}
> +
> +static cmdline_parse_token_string_t cmd_mcast_addr_flush_cmd =
> + TOKEN_STRING_INITIALIZER(struct cmd_mcast_addr_result,
> + mcast_addr_cmd, "mcast_addr");
> +static cmdline_parse_token_string_t cmd_mcast_addr_flush_what =
> + TOKEN_STRING_INITIALIZER(struct cmd_mcast_addr_result, what,
> + "flush");
> +static cmdline_parse_token_num_t cmd_mcast_addr_flush_portnum =
> + TOKEN_NUM_INITIALIZER(struct cmd_mcast_addr_result, port_num,
> + RTE_UINT16);
> +
> +static cmdline_parse_inst_t cmd_mcast_addr_flush = {
> + .f = cmd_mcast_addr_flush_parsed,
> + .data = (void *)0,
> + .help_str = "mcast_addr flush : "
> + "flush multicast MAC address on port_id",>
What about:
"Flush all multicast MAC addresses on port_id."
> + .tokens = {
> + (void *)&cmd_mcast_addr_flush_cmd,
> + (void *)&cmd_mcast_addr_flush_what,
> + (void *)&cmd_mcast_addr_flush_portnum,
> + NULL,
> + },
> +};
> +
> /* vf vlan anti spoof configuration */
>
> /* Common result structure for vf vlan anti spoof */
> @@ -12929,6 +12971,7 @@ static cmdline_parse_ctx_t builtin_ctx[] = {
> (cmdline_parse_inst_t *)&cmd_set_port_meter_stats_mask,
> (cmdline_parse_inst_t *)&cmd_show_port_meter_stats,
> (cmdline_parse_inst_t *)&cmd_mcast_addr,
> + (cmdline_parse_inst_t *)&cmd_mcast_addr_flush,
> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_anti_spoof,
> (cmdline_parse_inst_t *)&cmd_set_vf_mac_anti_spoof,
> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_stripq,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 11f3a22048..d66d1db37c 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -6802,6 +6802,24 @@ mcast_addr_remove(portid_t port_id, struct
> rte_ether_addr *mc_addr)
> mcast_addr_pool_append(port, mc_addr);
> }
>
> +void
> +mcast_addr_flush(portid_t port_id)
> +{
> + int ret;
> +
> + if (port_id_is_invalid(port_id, ENABLED_WARN))
> + return;
> +
> + ret = rte_eth_dev_set_mc_addr_list(port_id, NULL, 0);
> + if (ret != 0) {
> + fprintf(stderr,
> + "Failed to flush the set of filtered addresses on port
> %u\n",
> + port_id);
> + return;
> + }
> + mcast_addr_pool_destroy(port_id);
> +}
> +
> void
> port_dcb_info_display(portid_t port_id)
> {
> diff --git a/app/test-pmd/testpmd.h b/
Re: [PATCH] net/netvsc: increase VSP response timeout to 60 seconds
On 7/26/2023 11:29 PM, lon...@linuxonhyperv.com wrote: > From: Long Li > > The current timeout is set to 5 seconds. In Azure, tests show that it may > take up to 15 seconds for VSP to respond on busy nodes. The VSP schedules > unbounded work to process VSC resquest, there is no upper limit on how long > it takes to send response back to VSC. > > In the NETVSC kernel mode driver, it waits forever for VSP response. While in > DPDK we can't wait forever, setting the timeout to 60 seconds. > > Cc: sta...@dpdk.org > Signed-off-by: Long Li > Applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/mana: use %m for fscanf to read mac address
On 7/25/2023 10:16 PM, lon...@linuxonhyperv.com wrote: > From: Long Li > > Use %m through fscanf to allocate mac dynamically is safer than using > mac[20], this guarantees there is no overflow on mac[]. > > Signed-off-by: Long Li > Acked-by: Ferruh Yigit Applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/mana: add support to update MTU
On 7/25/2023 10:16 PM, lon...@linuxonhyperv.com wrote: > From: Long Li > > Add support for updating MTU for MANA. MTU is updated in kernel through > socket interface. > > Signed-off-by: Long Li > Applied to dpdk-next-net/main, thanks.
Re: [RFC] ethdev: clarify device queue state after start and stop
On 7/24/2023 2:43 AM, lihuisong (C) wrote:
>
> 在 2023/7/22 0:04, Ferruh Yigit 写道:
>> Drivers start/stop device queues on port start/stop, but not all drivers
>> update queue state accordingly.
>>
>> This becomes more visible if a specific queue stopped explicitly and
>> port stopped/started later, in this case although all queues are
>> started, the state of that specific queue is stopped and it is
>> misleading.
>>
>> Misrepresentation of queue state became a defect with commit [1] that
>> does forwarding decision based on queue state and commit [2] that gets
>> up to date queue state from ethdev/device before forwarding.
>>
>> This patch documents that status of all queues of a device should be
>> `RTE_ETH_QUEUE_STATE_STOPPED` after port stop and their status should
>> be`RTE_ETH_QUEUE_STATE_STARTED` after port start.
>>
>> Also an unit test added to verify drivers.
>>
>> Signed-off-by: Ferruh Yigit
>> ---
>> Cc: Jie Hai
>> Cc: Song Jiale
>> Cc: Yuan Peng
>> Cc: Raslan Darawsheh
>> Cc: Qiming Yang
>> ---
>> app/test/meson.build | 2 +
>> app/test/test_ethdev_api.c | 169 +
>> lib/ethdev/rte_ethdev.h | 5 ++
>> 3 files changed, 176 insertions(+)
>> create mode 100644 app/test/test_ethdev_api.c
>>
>> diff --git a/app/test/meson.build b/app/test/meson.build
>> index b89cf0368fb5..8e409cf59c35 100644
>> --- a/app/test/meson.build
>> +++ b/app/test/meson.build
>> @@ -49,6 +49,7 @@ test_sources = files(
>> 'test_efd_perf.c',
>> 'test_errno.c',
>> 'test_ethdev_link.c',
>> + 'test_ethdev_api.c',
>> 'test_event_crypto_adapter.c',
>> 'test_event_eth_rx_adapter.c',
>> 'test_event_ring.c',
>> @@ -187,6 +188,7 @@ fast_tests = [
>> ['eal_fs_autotest', true, true],
>> ['errno_autotest', true, true],
>> ['ethdev_link_status', true, true],
>> + ['ethdev_api', true, true],
>> ['event_ring_autotest', true, true],
>> ['fib_autotest', true, true],
>> ['fib6_autotest', true, true],
>> diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c
>> new file mode 100644
>> index ..1b4569396dda
>> --- /dev/null
>> +++ b/app/test/test_ethdev_api.c
>> @@ -0,0 +1,169 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright (C) 2023, Advanced Micro Devices, Inc.
>> + */
>> +
>> +#include
>> +#include
>> +
>> +#include
>> +#include "test.h"
>> +
>> +#define NUM_RXQ 2
>> +#define NUM_TXQ 2
>> +#define NUM_RXD 512
>> +#define NUM_TXD 512
>> +#define NUM_MBUF 1024
>> +#define MBUF_CACHE_SIZE 256
>> +
>> +static int32_t
>> +ethdev_api_queue_status(void)
>> +{
>> + struct rte_eth_dev_info dev_info;
>> + struct rte_eth_rxq_info rx_qinfo;
>> + struct rte_eth_txq_info tx_qinfo;
>> + struct rte_mempool *mbuf_pool;
>> + /*struct rte_eth_rxconf rx_conf;*/
>> + /*struct rte_eth_txconf tx_conf;*/
>> + struct rte_eth_conf eth_conf;
>> + uint16_t port_id;
>> + int ret;
>> +
>> + mbuf_pool = rte_pktmbuf_pool_create("MBUF_POOL", NUM_MBUF,
>> MBUF_CACHE_SIZE, 0,
>> + RTE_MBUF_DEFAULT_BUF_SIZE, rte_socket_id());
>> +
>> + RTE_ETH_FOREACH_DEV(port_id) {
>> + memset(ð_conf, 0, sizeof(dev_info));
> +1 need to modify.
>
ack
>> + ret = rte_eth_dev_configure(port_id, NUM_RXQ, NUM_TXQ,
>> ð_conf);
>> + TEST_ASSERT(ret == 0,
>> + "Port(%u) failed to configure.\n", port_id);
>> +
>> + /* RxQ setup */
>> + /*memset(&rx_conf, 0, sizeof(rx_conf));*/
> What's here for?
>
will remove
>> + for (uint16_t queue_id = 0; queue_id < NUM_RXQ; queue_id++) {
>> + ret = rte_eth_rx_queue_setup(port_id, queue_id, NUM_RXD,
>> + rte_socket_id(), NULL, mbuf_pool);
>> + TEST_ASSERT(ret == 0,
>> + "Port(%u), queue(%u) failed to setup RxQ.\n",
>> + port_id, queue_id);
>> + }
>> +
>> + /* TxQ setup */
>> + /*memset(&tx_conf, 0, sizeof(tx_conf));*/
>> + for (uint16_t q
[PATCH] ethdev: clarify device queue state after start and stop
Drivers start/stop device queues on port start/stop, but not all drivers
update queue state accordingly.
This becomes more visible if a specific queue stopped explicitly and
port stopped/started later, in this case although all queues are
started, the state of that specific queue is stopped and it is
misleading.
Misrepresentation of queue state became a defect with commit [1] that
does forwarding decision based on queue state and commit [2] that gets
up to date queue state from ethdev/device before forwarding.
[1]
commit 3c4426db54fc ("app/testpmd: do not poll stopped queues")
[2]
commit 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
This patch documents that status of all queues of a device should be
`RTE_ETH_QUEUE_STATE_STOPPED` after port stop and their status should
be`RTE_ETH_QUEUE_STATE_STARTED` after port start.
Also an unit test added to verify drivers.
Signed-off-by: Ferruh Yigit
---
Cc: Jie Hai
Cc: Song Jiale
Cc: Yuan Peng
Cc: Raslan Darawsheh
Cc: Qiming Yang
Cc: Ivan Malov
Cc: Huisong Li
v1:
* fix memset
* remove commented out code
* update unit test to skip queue state if
rte_eth_[rt]x_queue_info_get() is not supported
---
app/test/meson.build | 1 +
app/test/test_ethdev_api.c | 184 +
lib/ethdev/rte_ethdev.h| 5 +
3 files changed, 190 insertions(+)
create mode 100644 app/test/test_ethdev_api.c
diff --git a/app/test/meson.build b/app/test/meson.build
index 05bae9216dbc..05bbe84868f6 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -65,6 +65,7 @@ source_file_deps = {
'test_efd_perf.c': ['efd', 'hash'],
'test_errno.c': [],
'test_ethdev_link.c': ['ethdev'],
+'test_ethdev_api.c': ['ethdev'],
'test_event_crypto_adapter.c': ['cryptodev', 'eventdev', 'bus_vdev'],
'test_event_eth_rx_adapter.c': ['ethdev', 'eventdev', 'bus_vdev'],
'test_event_eth_tx_adapter.c': ['bus_vdev', 'ethdev', 'net_ring',
'eventdev'],
diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c
new file mode 100644
index ..68239f82ff33
--- /dev/null
+++ b/app/test/test_ethdev_api.c
@@ -0,0 +1,184 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2023, Advanced Micro Devices, Inc.
+ */
+
+#include
+#include
+
+#include
+#include "test.h"
+
+#define NUM_RXQ2
+#define NUM_TXQ2
+#define NUM_RXD 512
+#define NUM_TXD 512
+#define NUM_MBUF 1024
+#define MBUF_CACHE_SIZE 256
+
+static int32_t
+ethdev_api_queue_status(void)
+{
+ struct rte_eth_dev_info dev_info;
+ struct rte_eth_rxq_info rx_qinfo;
+ struct rte_eth_txq_info tx_qinfo;
+ struct rte_mempool *mbuf_pool;
+ struct rte_eth_conf eth_conf;
+ uint16_t port_id;
+ int ret;
+
+ mbuf_pool = rte_pktmbuf_pool_create("MBUF_POOL", NUM_MBUF,
MBUF_CACHE_SIZE, 0,
+ RTE_MBUF_DEFAULT_BUF_SIZE, rte_socket_id());
+
+ RTE_ETH_FOREACH_DEV(port_id) {
+ memset(ð_conf, 0, sizeof(eth_conf));
+ ret = rte_eth_dev_configure(port_id, NUM_RXQ, NUM_TXQ,
ð_conf);
+ TEST_ASSERT(ret == 0,
+ "Port(%u) failed to configure.\n", port_id);
+
+ /* RxQ setup */
+ for (uint16_t queue_id = 0; queue_id < NUM_RXQ; queue_id++) {
+ ret = rte_eth_rx_queue_setup(port_id, queue_id, NUM_RXD,
+ rte_socket_id(), NULL, mbuf_pool);
+ TEST_ASSERT(ret == 0,
+ "Port(%u), queue(%u) failed to setup RxQ.\n",
+ port_id, queue_id);
+ }
+
+ /* TxQ setup */
+ for (uint16_t queue_id = 0; queue_id < NUM_TXQ; queue_id++) {
+ ret = rte_eth_tx_queue_setup(port_id, queue_id, NUM_TXD,
+ rte_socket_id(), NULL);
+ TEST_ASSERT(ret == 0,
+ "Port(%u), queue(%u) failed to setup TxQ.\n",
+ port_id, queue_id);
+ }
+
+ ret = rte_eth_dev_info_get(port_id, &dev_info);
+ TEST_ASSERT(ret == 0,
+ "Port(%u) failed to get dev info.\n", port_id);
+
+ /* Initial RxQ */
+ for (uint16_t queue_id = 0; queue_id < dev_info.nb_rx_queues;
queue_id++) {
+ ret = rte_eth_rx_queue_info_get(port_id, queue_id,
&rx_qinfo);
+ if (ret == -ENOTSUP)
+ continue;
+
+ TEST_ASSERT(ret == 0,
+
Re: [PATCH v16 1/8] net/ntnic: initial commit which adds register defines
On 9/29/2023 10:21 AM, Christian Koue Muf wrote: > On 9/21/2023 4:05 PM, Ferruh Yigit wrote: >> On 9/20/2023 2:17 PM, Thomas Monjalon wrote: >>> Hello, >>> >>> 19/09/2023 11:06, Christian Koue Muf: >>>> On 9/18/23 10:34 AM, Ferruh Yigit wrote: >>>>> On 9/15/2023 7:37 PM, Morten Brørup wrote: >>>>>>> From: Ferruh Yigit [mailto:ferruh.yi...@amd.com] >>>>>>> Sent: Friday, 15 September 2023 17.55 >>>>>>> >>>>>>> On 9/8/2023 5:07 PM, Mykola Kostenok wrote: >>>>>>>> From: Christian Koue Muf >>>>>>>> >>>>>>>> The NTNIC PMD does not rely on a kernel space Napatech driver, >>>>>>>> thus all defines related to the register layout is part of the >>>>>>>> PMD code, which will be added in later commits. >>>>>>>> >>>>>>>> Signed-off-by: Christian Koue Muf >>>>>>>> Reviewed-by: Mykola Kostenok >>>>>>>> >>>>>>> >>>>>>> Hi Mykola, Christiam, >>>>>>> >>>>>>> This PMD scares me, overall it is a big drop: >>>>>>> "249 files changed, 87128 insertions(+)" >>>>>>> >>>>>>> I think it is not possible to review all in one release cycle, and >>>>>>> it is not even possible to say if all code used or not. >>>>>>> >>>>>>> I can see code is already developed, and it is difficult to >>>>>>> restructure developed code, but restructure it into small pieces >>>>>>> really helps for reviews. >>>>>>> >>>>>>> >>>>>>> Driver supports good list of features, can it be possible to >>>>>>> distribute upstream effort into multiple release. >>>>>>> Starting from basic functionality and add features gradually. >>>>>>> Target for this release can be providing datapath, and add more if >>>>>>> we have time in the release, what do you think? >>> >>> I was expecting to get only Rx/Tx in this release, not really more. >>> >>> I agree it may be interesting to discuss some design and check whether >>> we need more features in ethdev as part of the driver upstreaming >>> process. >>> >>> >>>>>>> Also there are large amount of base code (HAL / FPGA code), >>>>>>> instead of adding them as a bulk, relevant ones with a feature can >>>>>>> be added with the feature patch, this eliminates dead code in the >>>>>>> base code layer, also helps user/review to understand the link >>>>>>> between driver code and base code. >>> >>> Yes it would be interesting to see what is really needed for the basic >>> initialization and what is linked to a specific offload or configuration >>> feature. >>> >>> As a maintainer, I have to do some changes across all drivers >>> sometimes, and I use git blame a lot to understand why something was added. >>> >>> >>>>>> Jumping in here with an opinion about welcoming new NIC vendors to the >>>>>> community: >>>>>> >>>>>> Generally, if a NIC vendor supplies a PMD for their NIC, I expect the >>>>>> vendor to take responsibility for the quality of the PMD, including >>>>>> providing a maintainer and support backporting of fixes to the PMD in >>>>>> LTS releases. This should align with the vendor's business case for >>>>>> upstreaming their driver. >>>>>> >>>>>> If the vendor provides one big patch series, which may be difficult to >>>>>> understand/review, the fallout mainly hits the vendor's customers (and >>>>>> thus the vendor's support organization), not the community as a whole. >>>>>> >>>>> >>>>> Hi Morten, >>>>> >>>>> I was thinking same before making my above comment, what happens if >>>>> vendors submit as one big patch and when a problem occurs we can ask >>>>> owner to fix. Probably this makes vendor happy and makes my life (or any >>>>> other maintainer's life) easier, it is always easier to say yes. >>>>> >>>>> >
Re: [PATCH v3 3/9] net/nfp: initialize IPsec related content
On 9/29/2023 3:08 AM, Chaoyong He wrote: > From: Chang Miao > > If enable IPsec capability bit, driver need to Initialize IPsec. > Set security context and security offload capabilities in datapath. > Define private session and add SA array for each PF to save all > SA data in driver. Add internal mbuf dynamic flag and field to save > IPsec related data to dynamic mbuf field. > > Also update the relase note, add the inline IPsec offload section for NFP > PMD. > > Signed-off-by: Chang Miao > Signed-off-by: Shihong Wang > Reviewed-by: Chaoyong He > <...> > @@ -38,6 +38,11 @@ DPDK Release 23.11 > which also added support for standard atomics > (Ref: https://releases.llvm.org/3.6.0/tools/clang/docs/ReleaseNotes.html) > > + * **Added inline IPsec offload feature for NFP PMD.** > + > + Added the inline IPsec offload feature based on the security framework for > + NFP PMD. > + > New Features > > Moving release notes update to "New Features" section below while merging.
Re: [PATCH v3 0/9] add the support of ipsec offload
On 9/29/2023 3:08 AM, Chaoyong He wrote: > This patch series add the support of ipsec offload feature, includes: > * Implement the communication channel between PMD and firmware through > mailbox. > * Implement the ipsec offload related APIs based the security framework. > * Implement the ipsec packets process logics in the data path. > > --- > v3: > * Squash the update of mailmap file. > * Add a entry in the release note. > * Remove some unnecessary logic in the TLVs capability parsing function. > v2: > * Fix one spell error cause check warning. > * Try to fix one compile error in Alpine environment of CI. > --- > > Chang Miao (2): > net/nfp: initialize IPsec related content > net/nfp: create security session > > Shihong Wang (7): > net/nfp: add TLVs capability parsing > net/nfp: add mailbox to support IPsec offload > net/nfp: get security capabilities and session size > net/nfp: get IPsec Rx/Tx packet statistics > net/nfp: update security session > net/nfp: support IPsec Rx and Tx offload > net/nfp: destroy security session > Series applied to dpdk-next-net/main, thanks.
Re: [PATCH 0/2] ethdev: add group set miss actions API
On 9/20/2023 1:52 PM, Tomer Shmilovich wrote: > Introduce new group set miss actions API: > rte_flow_group_set_miss_actions(). > > A group's miss actions are a set of actions to be performed > in case of a miss on a group, i.e. when a packet didn't hit any flow > rules in the group. > > Currently, the expected behavior in this case is undefined. > In order to achieve such functionality, a user can add a flow rule > that matches on all traffic with the lowest priority in the group - > this is not explicit however, and can be overridden by another flow rule > with a lower priority. > > This new API function allows a user to set a group's miss actions in an > explicit way. > > RFC discussion: > http://patches.dpdk.org/project/dpdk/patch/20230807133601.164018-1-tshmilov...@nvidia.com/ > > Tomer Shmilovich (2): > ethdev: add group set miss actions API > app/testpmd: add group set miss actions CLI commands > Series applied to dpdk-next-net/main, thanks.
Re: [PATCH v3] net/af_xdp: fix missing UMEM feature
On 9/28/2023 10:32 AM, Bruce Richardson wrote:
> On Thu, Sep 28, 2023 at 09:25:53AM +, Shibin Koikkara Reeny wrote:
>> Shared UMEM feature is missing in the af_xdp driver build
>> after the commit 33d66940e9ba ("build: use C11 standard").
>>
>> Runtime Error log while using Shared UMEM feature:
>> rte_pmd_af_xdp_probe(): Initializing pmd_af_xdp for net_af_xdp0
>> init_internals(): Shared UMEM feature not available. Check kernel
>> and libbpf version
>> rte_pmd_af_xdp_probe(): Failed to init internals
>> vdev_probe(): failed to initialize net_af_xdp0 device
>> EAL: Bus (vdev) probe failed.
>>
>> Reason for the missing UMEM feature is because the C11 standard
>> doesn't include the GNU compiler extensions typeof and asm, used
>> by the libbpf and libxdp header files.
>>
>> Meson error log:
>> In file included from
>> dpdk/build/meson-private/tmpf74nkhqd/testfile.c:5:
>> /usr/local/include/bpf/xsk.h: In function 'xsk_prod_nb_free':
>> /usr/local/include/bpf/xsk.h:165:26: error: expected ';' before '___p1'
>> 165 | r->cached_cons = libbpf_smp_load_acquire(r->consumer);
>> | ^~~
>> /usr/local/include/bpf/xsk.h:165:26: error: 'asm' undeclared (first use
>> in this function)
>> ...
>> /usr/local/include/bpf/xsk.h:199:9: error: unknown type name 'typeof'
>> 199 | libbpf_smp_store_release(prod->producer, *prod->producer
>> + nb);
>> | ^~~~
>>
>> Fix is to provide alternative keywords by using Option Controlling C
>> Dialect [1].
>>
>
> Minor nit, this patch provides the alternative keywords using macros rather
> than any
> C dialect options.
>
Fixed while merging.
>> Fixes: 33d66940e9ba ("build: use C11 standard")
>>
>> [1] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html
>>
>> v3: Used alternative keywords fix.
>> v2: Added original commit causing the issue.
>> Signed-off-by: Shibin Koikkara Reeny
>> ---
>
> Acked-by: Bruce Richardson
>
Applied to dpdk-next-net/main, thanks.
Re: [PATCH 11/11] net/ngbe: add YT PHY fiber mode autoneg 100M support
On 9/28/2023 10:47 AM, Jiawen Wu wrote: > Support auto-neg 100M for YT PHY fiber mode. > > Signed-off-by: Jiawen Wu > --- > doc/guides/rel_notes/release_23_11.rst | 4 ++ > drivers/net/ngbe/base/ngbe_phy_yt.c| 66 +++--- > 2 files changed, 52 insertions(+), 18 deletions(-) > > diff --git a/doc/guides/rel_notes/release_23_11.rst > b/doc/guides/rel_notes/release_23_11.rst > index 9746809a66..578900f504 100644 > --- a/doc/guides/rel_notes/release_23_11.rst > +++ b/doc/guides/rel_notes/release_23_11.rst > @@ -41,6 +41,10 @@ DPDK Release 23.11 > New Features > > > +* **Updated Wangxun ngbe driver.** > + > + * Added 100M and auto-neg support in YT PHY fiber mode. > + > This is added into section comment, moved to proper location while merging.
Re: [PATCH 00/11] Wanguxn NICs fixes and supports
On 9/28/2023 10:47 AM, Jiawen Wu wrote: > Fix some bugs in txgbe/ngbe driver, and support new feature in > ngbe driver. > > Jiawen Wu (11): > net/txgbe: add Tx queue maximum limit > net/txgbe: fix GRE tunnel packet checksum > net/ngbe: fix to set flow control > net/ngbe: prevent the NIC from slowing down link speed > net/txgbe: reconfigure MAC Rx when link update > net/ngbe: reconfigure MAC Rx when link update > net/txgbe: fix to keep link down after device close > net/ngbe: fix to keep link down after device close > net/txgbe: check process type in close operation > net/ngbe: check process type in close operation > net/ngbe: add YT PHY fiber mode autoneg 100M support > Series applied to dpdk-next-net/main, thanks.
Re: [PATCH v2] net/tap: resolve stringop-overflow with gcc 12 on ppc64le
On 6/7/2023 7:47 PM, Ferruh Yigit wrote:
> On 5/16/2023 10:55 AM, Ferruh Yigit wrote:
>> On 5/16/2023 2:28 AM, Stephen Hemminger wrote:
>>> On Tue, 16 May 2023 00:35:56 +0100
>>> Ferruh Yigit wrote:
>>>
>>>> Yes only some scripts and possible applications that hotplug tap
>>>> interface with hardcoded parameters may impacted, don't know how big is
>>>> this amount but this ends up breaking something that was working before
>>>> upgrading DPDK for them.
>>>>
>>>> And I believe the motivation is weak to break the behavior.
>>>>
>>>> Won't it be better to update 'rte_ether_unformat_addr()' to accept more
>>>> flexible syntax, and use it? Is there any disadvantage of this approach?
>>>
>>> It is already more flexible than the standard ether_aton().
>>
>> I mean to accept single chars, as 'tap' currently does, like "a:a:a:a:a:a".
>>
>> Agree that impact of tap change is small, but if we can eliminate it
>> completely without any side affect, why not?
>>
>>
>> As accepting single char will be expanding 'rte_ether_unformat_addr()'
>> capability, it will be backward compatible, am I missing anything?
>>
>
> Hi David,
>
> If API update is not planned, what do you think to just solve the build
> error without changing functionality with a change something like below:
>
> ```
> - (strlen(mac_byte) == strspn(mac_byte,
> - ETH_TAP_CMP_MAC_FMT))) {
> + (strlen(mac_byte) == strspn(mac_byte, ETH_TAP_CMP_MAC_FMT)) &&
> + index < RTE_ETHER_ADDR_LEN) {
>
> ```
Hi David,
If you can confirm above fixes the issue, I can send a patch for it.
Re: [PATCH] net/dpaa2: change threshold value
On 6/9/2023 3:20 PM, Ferruh Yigit wrote:
> On 5/15/2023 9:16 AM, Sachin Saxena (OSS) wrote:
>> On 5/8/2023 4:11 PM, Tianli Lai wrote:
>>> Caution: This is an external email. Please take care when clicking
>>> links or opening attachments. When in doubt, report the message using
>>> the 'Report this email' button
>>>
>>>
>>> this threshold value can be changed with function argument nb_rx_desc.
>>>
>>> Signed-off-by: Tianli Lai
>>> ---
>>> drivers/net/dpaa2/dpaa2_ethdev.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c
>>> b/drivers/net/dpaa2/dpaa2_ethdev.c
>>> index 679f33ae1a..ad892ded4a 100644
>>> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
>>> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
>>> @@ -829,7 +829,7 @@ dpaa2_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>> dpaa2_q->cgid,
>>> &taildrop);
>>> } else {
>>> /*enabling per rx queue congestion control */
>>> - taildrop.threshold = CONG_THRESHOLD_RX_BYTES_Q;
>>> + taildrop.threshold = nb_rx_desc * 1024;
>>> taildrop.units = DPNI_CONGESTION_UNIT_BYTES;
>>> taildrop.oal = CONG_RX_OAL;
>>> DPAA2_PMD_DEBUG("Enabling Byte based Drop on
>>> queue= %d",
>>> --
>>> 2.27.0
>>>
>> Hi Tianli,
>>
>> The number of bytes based tail drop threshold value
>> "CONG_THRESHOLD_RX_BYTES_Q" is an optimized value for dpaa2 platform. we
>> concluded this value after multiple benchmark experiments in past.
>> Although, number of frame based threshold value is "nb_rx_desc" based only.
>> We will further review this suggestion and get back.
>>
>
> Hi Sachin, What is the status of this patch?
>
Ping
Re: [PATCH] net/sfc: support packet replay in transfer flows
On 9/27/2023 11:36 AM, Ivan Malov wrote: > Packet replay enables users to leverage multiple counters in > one flow and allows to request delivery to multiple ports. > > A given flow rule may use either one inline count action > and multiple indirect counters or just multiple indirect > counters. The inline count action (if any) must come > before the first delivery action or before the first > indirect count action, whichever comes earlier. > > These are some testpmd examples of supported > multi-count and mirroring use cases: > > flow create 0 transfer pattern represented_port ethdev_port_id is 0 / end \ > actions port_representor port_id 0 / port_representor port_id 1 / end > > or > > flow indirect_action 0 create action_id 239 transfer action count / end > > flow create 0 transfer pattern represented_port ethdev_port_id is 0 / end \ > actions count / port_representor port_id 0 / indirect 239 / \ > port_representor port_id 1 / end > > or > > flow indirect_action 0 create action_id 239 transfer action count / end > > flow create 0 transfer pattern represented_port ethdev_port_id is 0 / end \ > actions indirect 239 / port_representor port_id 0 / indirect 239 / \ > port_representor port_id 1 / end > > and the likes. > > Signed-off-by: Ivan Malov > Reviewed-by: Andy Moreton > Hi Andrew, Reminder of this patch waiting for review.
Re: [PATCH v1] net/memif: fix segfault with large burst size
On 9/4/2023 8:10 AM, Joyce Kong wrote:
> There will be a segfault when Rx burst size is greater than
> MAX_PKT_BURST of memif. Fix the issue by correcting the
> wrong mbuf index in eth_memif_rx, which results in accessing
> invalid memory address.
>
> Bugzilla ID: 1273
> Fixes: aa17df860891 ("net/memif: add a Rx fast path")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Joyce Kong
> Reviewed-by: Feifei Wang
> Reviewed-by: Ruifeng Wang
>
Hi Joyce, good catch.
Reviewed-by: Ferruh Yigit
Applied to dpdk-next-net/main, thanks.
For record, if nb_pkts > MAX_PKT_BURST, memif buffer consumed in chunks
of MAX_PKT_BURST mbufs, next chunk consumption starts with 'goto
next_bulk' call.
For each chunk, MAX_PKT_BURST mbufs allocated and filled, they are
accessed by 'n_rx_pkts' index, but 'n_rx_pkts' is overall received mbuf
number, so it shouldn't be used as index for that chunk, but 'rx_pkts'
should be used which is reset at the beginning of the chunk processing.
For the first chunk using 'n_rx_pkts' or 'rx_pkts' are same, that
explains how issue lived till now, as commit log mentions issue can be
observed when nb_pkts > MAX_PKT_BURST.
Re: [PATCH] net/sfc: support packet replay in transfer flows
On 9/30/2023 7:31 AM, Andrew Rybchenko wrote: > On 9/27/23 13:36, Ivan Malov wrote: >> Packet replay enables users to leverage multiple counters in >> one flow and allows to request delivery to multiple ports. >> >> A given flow rule may use either one inline count action >> and multiple indirect counters or just multiple indirect >> counters. The inline count action (if any) must come >> before the first delivery action or before the first >> indirect count action, whichever comes earlier. >> >> These are some testpmd examples of supported >> multi-count and mirroring use cases: >> >> flow create 0 transfer pattern represented_port ethdev_port_id is 0 / >> end \ >> actions port_representor port_id 0 / port_representor port_id 1 / end >> >> or >> >> flow indirect_action 0 create action_id 239 transfer action count / end >> >> flow create 0 transfer pattern represented_port ethdev_port_id is 0 / >> end \ >> actions count / port_representor port_id 0 / indirect 239 / \ >> port_representor port_id 1 / end >> >> or >> >> flow indirect_action 0 create action_id 239 transfer action count / end >> >> flow create 0 transfer pattern represented_port ethdev_port_id is 0 / >> end \ >> actions indirect 239 / port_representor port_id 0 / indirect 239 / \ >> port_representor port_id 1 / end >> >> and the likes. >> >> Signed-off-by: Ivan Malov >> Reviewed-by: Andy Moreton > > Acked-by: Andrew Rybchenko > > Applied to dpdk-next-net/main, thanks.
Re: [PATCH v4] net/af_xdp: fix missing UMEM feature
On 10/2/2023 2:01 PM, Bruce Richardson wrote:
> On Mon, Oct 02, 2023 at 12:48:52PM +, Shibin Koikkara Reeny wrote:
>> Shared UMEM feature is missing in the af_xdp driver build
>> after the commit 33d66940e9ba ("build: use C11 standard").
>>
>> Runtime Error log while using Shared UMEM feature:
>> rte_pmd_af_xdp_probe(): Initializing pmd_af_xdp for net_af_xdp0
>> init_internals(): Shared UMEM feature not available. Check kernel
>> and libbpf version
>> rte_pmd_af_xdp_probe(): Failed to init internals
>> vdev_probe(): failed to initialize net_af_xdp0 device
>> EAL: Bus (vdev) probe failed.
>>
>> Reason for the missing UMEM feature is because the C11 standard
>> doesn't include the GNU compiler extensions typeof and asm, used
>> by the libbpf and libxdp header files.
>>
>> Meson error log:
>> In file included from
>> dpdk/build/meson-private/tmpf74nkhqd/testfile.c:5:
>> /usr/local/include/bpf/xsk.h: In function 'xsk_prod_nb_free':
>> /usr/local/include/bpf/xsk.h:165:26: error: expected ';' before '___p1'
>> 165 | r->cached_cons = libbpf_smp_load_acquire(r->consumer);
>> | ^~~
>> /usr/local/include/bpf/xsk.h:165:26: error: 'asm' undeclared (first use
>> in this function)
>> ...
>> /usr/local/include/bpf/xsk.h:199:9: error: unknown type name 'typeof'
>> 199 | libbpf_smp_store_release(prod->producer, *prod->producer
>> + nb);
>> | ^~~~
>>
>> Fix is to provide alternative keywords by using macros [1].
>>
>> Fixes: 33d66940e9ba ("build: use C11 standard")
>>
>> [1] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html
>>
>> v4: Updated the commit message.
>> v3: Used alternative keywords fix.
>> v2: Added original commit causing the issue.
>>
>> Signed-off-by: Shibin Koikkara Reeny
>
> Reviewed-by: Bruce Richardson
>
I already merged the v3, and updated the commit log while merging,
can you please check the next-net [1] if the commit log is good as it is?
[1]
https://git.dpdk.org/next/dpdk-next-net/commit/?id=a499de2602df505e0313ae468817863b29f2311e
Re: [PATCH v4] net/af_xdp: fix missing UMEM feature
On 10/2/2023 2:23 PM, Bruce Richardson wrote:
> On Mon, Oct 02, 2023 at 02:15:51PM +0100, Ferruh Yigit wrote:
>> On 10/2/2023 2:01 PM, Bruce Richardson wrote:
>>> On Mon, Oct 02, 2023 at 12:48:52PM +, Shibin Koikkara Reeny wrote:
>>>> Shared UMEM feature is missing in the af_xdp driver build
>>>> after the commit 33d66940e9ba ("build: use C11 standard").
>>>>
>>>> Runtime Error log while using Shared UMEM feature:
>>>> rte_pmd_af_xdp_probe(): Initializing pmd_af_xdp for net_af_xdp0
>>>> init_internals(): Shared UMEM feature not available. Check kernel
>>>> and libbpf version
>>>> rte_pmd_af_xdp_probe(): Failed to init internals
>>>> vdev_probe(): failed to initialize net_af_xdp0 device
>>>> EAL: Bus (vdev) probe failed.
>>>>
>>>> Reason for the missing UMEM feature is because the C11 standard
>>>> doesn't include the GNU compiler extensions typeof and asm, used
>>>> by the libbpf and libxdp header files.
>>>>
>>>> Meson error log:
>>>> In file included from
>>>> dpdk/build/meson-private/tmpf74nkhqd/testfile.c:5:
>>>> /usr/local/include/bpf/xsk.h: In function 'xsk_prod_nb_free':
>>>> /usr/local/include/bpf/xsk.h:165:26: error: expected ';' before '___p1'
>>>> 165 | r->cached_cons = libbpf_smp_load_acquire(r->consumer);
>>>> | ^~~
>>>> /usr/local/include/bpf/xsk.h:165:26: error: 'asm' undeclared (first use
>>>> in this function)
>>>> ...
>>>> /usr/local/include/bpf/xsk.h:199:9: error: unknown type name 'typeof'
>>>> 199 | libbpf_smp_store_release(prod->producer, *prod->producer
>>>> + nb);
>>>> | ^~~~
>>>>
>>>> Fix is to provide alternative keywords by using macros [1].
>>>>
>>>> Fixes: 33d66940e9ba ("build: use C11 standard")
>>>>
>>>> [1] https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html
>>>>
>>>> v4: Updated the commit message.
>>>> v3: Used alternative keywords fix.
>>>> v2: Added original commit causing the issue.
>>>>
>>>> Signed-off-by: Shibin Koikkara Reeny
>>>
>>> Reviewed-by: Bruce Richardson
>>>
>>
>> I already merged the v3, and updated the commit log while merging,
>> can you please check the next-net [1] if the commit log is good as it is?
>>
>> [1]
>> https://git.dpdk.org/next/dpdk-next-net/commit/?id=a499de2602df505e0313ae468817863b29f2311e
>>
>
> LGTM anyway.
>
Shibin, if you also don't have any objection, I will keep the merged
version as it is.
Re: [PATCH v2 0/3] rte_ether_unformat_addr changes
On 10/2/2023 7:37 PM, Stephen Hemminger wrote: > This patchset makes rte_ether_unformat_addr allow other formats > for MAC address. Need to remove some inputs from existing > cmdline_etheraddr test, and add a new test in test suite > to cover this. There is some overlap between the two tests > but that is fine. > > Stephen Hemminger (3): > test: remove some strings from cmdline_etheraddr tests > rte_ether_unformat: accept more inputs > test: add tests for rte_ether routines > Thanks Stephen, This enables using the API as replacement to the tap PMD's local parse implementation: https://patchwork.dpdk.org/project/dpdk/patch/20230323170145.129901-1-...@linux.vnet.ibm.com/
Re: [PATCH v2 1/3] test: remove some strings from cmdline_etheraddr tests
On 10/2/2023 7:37 PM, Stephen Hemminger wrote:
> Some of the ethernet address formats which were invalid will
> now become valid inputs when rte_ether_unformat_addr is modified
> to allow leading zeros.
>
> Also, make local variables static.
>
> Signed-off-by: Stephen Hemminger
>
<...>
> @@ -61,10 +60,8 @@ const char * ether_addr_invalid_strs[] = {
> "INVA:LIDC:HARS",
> /* misc */
> "01 23 45 67 89 AB",
> - "01.23.45.67.89.AB",
> "01,23,45,67,89,AB",
> - "01:23:45\0:67:89:AB",
> - "01:23:45#:67:89:AB",
>
Why these two are valid now?
And I guess second one is still not valid, but first one is parsed as
[1], is this expected?
[1] 00:01:00:23:00:45
Re: [PATCH v2 1/3] test: remove some strings from cmdline_etheraddr tests
On 10/3/2023 11:47 AM, Ferruh Yigit wrote:
> On 10/2/2023 7:37 PM, Stephen Hemminger wrote:
>> Some of the ethernet address formats which were invalid will
>> now become valid inputs when rte_ether_unformat_addr is modified
>> to allow leading zeros.
>>
>> Also, make local variables static.
>>
>> Signed-off-by: Stephen Hemminger
>>
>
> <...>
>
>> @@ -61,10 +60,8 @@ const char * ether_addr_invalid_strs[] = {
>> "INVA:LIDC:HARS",
>> /* misc */
>> "01 23 45 67 89 AB",
>> -"01.23.45.67.89.AB",
>> "01,23,45,67,89,AB",
>> -"01:23:45\0:67:89:AB",
>> -"01:23:45#:67:89:AB",
>>
>
> Why these two are valid now?
>
> And I guess second one is still not valid, but first one is parsed as
> [1], is this expected?
>
> [1] 00:01:00:23:00:45
>
>
Ah, I guess it is taken as "::" format, but number of digit
is not enforced, so "1:2:3" is a valid format, should we add this to API
documentation as example format? Or is this unintended side effect?
Re: [PATCH 0/4] support offload of simple conntrack flow rules
On 9/30/2023 11:00 AM, Chaoyong He wrote: > This patch series add the support of simple conntrack flow rules offload > through flower firmware by import the needed data structure and logic of > flow merge. > > Chaoyong He (4): > net/nfp: prepare for the flow merge > net/nfp: add infrastructure for ct flow merge > 'ct' is conntrack, right? Can you prefer long version if possible? > net/nfp: add call to add and delete the flows to firmware > net/nfp: add support for merged flows and conntrack stats > > './devtools/check-doc-vs-code.sh' reports some errors, can you please check?
Re: [PATCH v2 1/3] test: remove some strings from cmdline_etheraddr tests
On 10/3/2023 5:36 PM, Stephen Hemminger wrote:
> On Tue, 3 Oct 2023 11:47:51 +0100
> Ferruh Yigit wrote:
>
>> On 10/2/2023 7:37 PM, Stephen Hemminger wrote:
>>> Some of the ethernet address formats which were invalid will
>>> now become valid inputs when rte_ether_unformat_addr is modified
>>> to allow leading zeros.
>>>
>>> Also, make local variables static.
>>>
>>> Signed-off-by: Stephen Hemminger
>>>
>>
>> <...>
>>
>>> @@ -61,10 +60,8 @@ const char * ether_addr_invalid_strs[] = {
>>> "INVA:LIDC:HARS",
>>> /* misc */
>>> "01 23 45 67 89 AB",
>>> - "01.23.45.67.89.AB",
>>> "01,23,45,67,89,AB",
>>> - "01:23:45\0:67:89:AB",
>>> - "01:23:45#:67:89:AB",
>>>
>>
>> Why these two are valid now?
>>
>> And I guess second one is still not valid, but first one is parsed as
>> [1], is this expected?
>>
>> [1] 00:01:00:23:00:45
>
> The code in cmdline converts the comment character # to a null byte.
> So both test are the same.
>
> With new unformat, it allows a 3 part mac address with leading
> zeros.
> 01:23:45 is equivalent to 0001:0023:0045
>
With 3 part MAC, omitting leading zeros looks confusing to me, because
that omitted part becomes an octet in MAC. Like:
01:23:45 being equivalent to 00:01:00:23:00:45
As 3 part MAC, and 6 part MAC has separate functions, does it makes
sense to require leading zeros in the 3 part MAC, what do you think?
Re: [PATCH v3 0/4] rte_ether_unformat_addr related changes
On 10/3/2023 9:29 PM, Stephen Hemminger wrote: > This patchset makes rte_ether_unformat_addr allow other formats > for MAC address. Need to remove some inputs from existing > cmdline_etheraddr test, and add a new test in test suite > to cover this. There is some overlap between the two tests > but that is fine. > > v4 - fix spelling errors > don't allow leading zeros in Cisco 3 part format > incorporate tap patch to use unformat addr > > David Christensen (1): > net/tap: use rte_ether_unformat_address > > Stephen Hemminger (3): > test: remove some strings from cmdline_etheraddr tests > rte_ether_unformat: accept more inputs > test: add tests for rte_ether routines > > Thanks for the patch, appreciated. For series, Acked-by: Ferruh Yigit Series applied to dpdk-next-net/main, thanks.
Re: [PATCH v2 0/4] support offload of simple conntrack flow rules
On 10/4/2023 10:35 AM, Chaoyong He wrote: > This patch series add the support of simple conntrack flow rules offload > through flower firmware by import the needed data structure and logic of > flow merge. > > --- > v2: > * Fix one mis-spell in comment. > * Revise logic and document to solve the 'devtools/check-doc-vs-code.sh' > warning. > * Adjust the commit message as the advice of reviewer. > --- > > Chaoyong He (4): > net/nfp: prepare for the flow merge > net/nfp: add infrastructure for conntrack flow merge > net/nfp: add call to add and delete the flows to firmware > net/nfp: add support for merged flows and conntrack stats > > Series applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/gve: allow GVE MTU greater than RTE_ETHER_MTU
On 9/29/2023 9:38 PM, Joshua Washington wrote:
> This patch corrects the MTU setting behavior in the GVE DPDK driver to
> remove the artificial upper limit of RTE_ETHER_MTU. Instead, the max MTU
> is dictated by the default value of the MTU that the device sends during
> initialization, which will always be the maximum supported MTU.
>
> Signed-off-by: Joshua Washington
>
Fixes: 71dea04cdf9a ("net/gve: support device info and configure")
Cc: sta...@dpdk.org
Applied to dpdk-next-net/main, thanks.
Re: [PATCH v2 2/2] eal: remove NUMFLAGS enumeration
On 10/6/2023 9:27 AM, David Marchand wrote: > On Fri, Aug 11, 2023 at 8:08 AM Sivaprasad Tummala > wrote: >> >> This patch removes RTE_CPUFLAG_NUMFLAGS to allow new CPU >> features without breaking ABI each time. >> >> Signed-off-by: Sivaprasad Tummala > > I relooked at the API and I see one case (that I had missed in my > previous reply) where an ABI issue may be present so I updated the > commitlog in this regard. > > And then I merged this patch, with a few modifications. > - removed the deprecation notice and updated RN, > - removed leftover "Last item" comments in arch cpuflag headers, > > I hope everyone is happy with this. > > Looks good to me, thanks David.
Re: [PATCH] net/nfp: fix one memory access problem
On 10/8/2023 8:08 AM, Chaoyong He wrote:
> The initial logic do not give a value to the out parameter in the
> abnormal logic, which cause an local variable uninitialized problem.
>
> Fixes: 3d6811281392 ("net/nfp: add infrastructure for conntrack flow merge")
> Cc: chaoyong...@corigine.com
>
> Signed-off-by: Chaoyong He
>
Tested-by: Daxue Gao
Acked-by: Ferruh Yigit
Applied to dpdk-next-net/main, thanks.
Re: [PATCH v2] net/tap: resolve stringop-overflow with gcc 12 on ppc64le
On 10/6/2023 7:31 PM, David Christensen wrote:
>
>
> On 9/29/23 6:48 AM, Ferruh Yigit wrote:
>> On 6/7/2023 7:47 PM, Ferruh Yigit wrote:
>>> On 5/16/2023 10:55 AM, Ferruh Yigit wrote:
>>>> On 5/16/2023 2:28 AM, Stephen Hemminger wrote:
>>>>> On Tue, 16 May 2023 00:35:56 +0100
>>>>> Ferruh Yigit wrote:
>>>>>
>>>>>> Yes only some scripts and possible applications that hotplug tap
>>>>>> interface with hardcoded parameters may impacted, don't know how
>>>>>> big is
>>>>>> this amount but this ends up breaking something that was working
>>>>>> before
>>>>>> upgrading DPDK for them.
>>>>>>
>>>>>> And I believe the motivation is weak to break the behavior.
>>>>>>
>>>>>> Won't it be better to update 'rte_ether_unformat_addr()' to accept
>>>>>> more
>>>>>> flexible syntax, and use it? Is there any disadvantage of this
>>>>>> approach?
>>>>>
>>>>> It is already more flexible than the standard ether_aton().
>>>>
>>>> I mean to accept single chars, as 'tap' currently does, like
>>>> "a:a:a:a:a:a".
>>>>
>>>> Agree that impact of tap change is small, but if we can eliminate it
>>>> completely without any side affect, why not?
>>>>
>>>>
>>>> As accepting single char will be expanding 'rte_ether_unformat_addr()'
>>>> capability, it will be backward compatible, am I missing anything?
>>>>
>>>
>>> Hi David,
>>>
>>> If API update is not planned, what do you think to just solve the build
>>> error without changing functionality with a change something like below:
>>>
>>> ```
>>> - (strlen(mac_byte) == strspn(mac_byte,
>>> - ETH_TAP_CMP_MAC_FMT))) {
>>> + (strlen(mac_byte) == strspn(mac_byte, ETH_TAP_CMP_MAC_FMT)) &&
>>> + index < RTE_ETHER_ADDR_LEN) {
>>>
>>> ```
>>
>> Hi David,
>>
>> If you can confirm above fixes the issue, I can send a patch for it.
>
> Confirmed that your proposed change resolves the build issue on ppc64le.
> Appreciate if you can submit the patch.
>
>
Thanks for checking, but Stephen updated the 'rte_ether_unformat_addr()'
API [1] and sent a new version of this patch [2], which is merged in
next-net [3] now.
Build error for PPC should be fixed now.
[1]
https://patchwork.dpdk.org/project/dpdk/patch/20231003202909.391330-3-step...@networkplumber.org/
[2]
https://patchwork.dpdk.org/project/dpdk/patch/20231003202909.391330-5-step...@networkplumber.org/
[3]
https://git.dpdk.org/next/dpdk-next-net/log/
Re: [PATCH v3 0/2] fix help string and add a new command
On 10/8/2023 7:46 AM, Dengdui Huang wrote: > This series fix help string and add a new command. > > v3->v4 > help string add '\n' at last. > > v2->v3 > add 'mcast_addr add|remove' to help string and > update the new command description. > > v1->v2 > update order on help string. > > Dengdui Huang (2): > app/testpmd: fix help string > app/testpmd: add flush all multicast MAC address command > > For series, Acked-by: Ferruh Yigit Series applied to dpdk-next-net/main, thanks.
Re: [PATCH v16 1/8] net/ntnic: initial commit which adds register defines
On 10/9/2023 8:57 AM, Christian Koue Muf wrote: > On 9/29/2023 12:24 PM, Thomas Monjalon wrote: >> 29/09/2023 11:46, Ferruh Yigit: >>> On 9/29/2023 10:21 AM, Christian Koue Muf wrote: >>>> On 9/21/2023 4:05 PM, Ferruh Yigit wrote: >>>>> On 9/20/2023 2:17 PM, Thomas Monjalon wrote: >>>>>> Hello, >>>>>> >>>>>> 19/09/2023 11:06, Christian Koue Muf: >>>>>>> On 9/18/23 10:34 AM, Ferruh Yigit wrote: >>>>>>>> On 9/15/2023 7:37 PM, Morten Brørup wrote: >>>>>>>>>> From: Ferruh Yigit [mailto:ferruh.yi...@amd.com] >>>>>>>>>> Sent: Friday, 15 September 2023 17.55 >>>>>>>>>> >>>>>>>>>> On 9/8/2023 5:07 PM, Mykola Kostenok wrote: >>>>>>>>>>> From: Christian Koue Muf >>>>>>>>>>> >>>>>>>>>>> The NTNIC PMD does not rely on a kernel space Napatech >>>>>>>>>>> driver, thus all defines related to the register layout is >>>>>>>>>>> part of the PMD code, which will be added in later commits. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Christian Koue Muf >>>>>>>>>>> Reviewed-by: Mykola Kostenok >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Hi Mykola, Christiam, >>>>>>>>>> >>>>>>>>>> This PMD scares me, overall it is a big drop: >>>>>>>>>> "249 files changed, 87128 insertions(+)" >>>>>>>>>> >>>>>>>>>> I think it is not possible to review all in one release cycle, >>>>>>>>>> and it is not even possible to say if all code used or not. >>>>>>>>>> >>>>>>>>>> I can see code is already developed, and it is difficult to >>>>>>>>>> restructure developed code, but restructure it into small >>>>>>>>>> pieces really helps for reviews. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Driver supports good list of features, can it be possible to >>>>>>>>>> distribute upstream effort into multiple release. >>>>>>>>>> Starting from basic functionality and add features gradually. >>>>>>>>>> Target for this release can be providing datapath, and add >>>>>>>>>> more if we have time in the release, what do you think? >>>>>> >>>>>> I was expecting to get only Rx/Tx in this release, not really more. >>>>>> >>>>>> I agree it may be interesting to discuss some design and check >>>>>> whether we need more features in ethdev as part of the driver >>>>>> upstreaming process. >>>>>> >>>>>> >>>>>>>>>> Also there are large amount of base code (HAL / FPGA code), >>>>>>>>>> instead of adding them as a bulk, relevant ones with a feature >>>>>>>>>> can be added with the feature patch, this eliminates dead code >>>>>>>>>> in the base code layer, also helps user/review to understand >>>>>>>>>> the link between driver code and base code. >>>>>> >>>>>> Yes it would be interesting to see what is really needed for the >>>>>> basic initialization and what is linked to a specific offload or >>>>>> configuration feature. >>>>>> >>>>>> As a maintainer, I have to do some changes across all drivers >>>>>> sometimes, and I use git blame a lot to understand why something was >>>>>> added. >>>>>> >>>>>> >>>>>>>>> Jumping in here with an opinion about welcoming new NIC vendors to >>>>>>>>> the community: >>>>>>>>> >>>>>>>>> Generally, if a NIC vendor supplies a PMD for their NIC, I expect the >>>>>>>>> vendor to take responsibility for the quality of the PMD, including >>>>>>>>> providing a maintainer and support backporting of fixes to the PMD in
Re: DPDK Release Status Meeting 2023-10-05
On 10/5/2023 10:04 AM, Mcnamara, John wrote: > Release status meeting minutes 2023-10-05 > = > > Agenda: > * Release Dates > * Subtrees > * Roadmaps > * LTS > * Defects > * Opens > > Participants: > * AMD > * Intel > * Marvell > * Nvidia > * Red Hat > > Release Dates > - > > The following are the current working dates for 23.11: > > * V1: 12 August 2023 > * RC1: 11 October 2023 > * RC2: 27 October 2023 > * RC3: 3 November 2023 > * Release: 15 November 2023 > > > Subtrees > > > * next-net > * Initial pull done. > * Working on the rest of the tree. > * There are still a number of rte_flow patches pending merge but not having > reviews > I created two public bundles for rte flow patches waiting for review, 1) rte_flow patches: https://patchwork.dpdk.org/bundle/fyigit/rte_flow_v23.11/ 2) rte_flow related testpmd patches: https://patchwork.dpdk.org/bundle/fyigit/rte_flow_testpmd_v23.11/ Can you please help/support reviewing these patches? Thanks in advance, ferruh
Re: [PATCH] ethdev: remove init_color from METER_MARK action
On 8/8/2023 11:00 AM, Gregory Etelson wrote: > Indirect list API defines 2 types of action update: > • Action mutable context is always shared between all flows > that referenced indirect actions list handle. > Action mutable context can be changed by explicit invocation > of indirect handle update function. > • Flow mutable context is private to a flow. > Flow mutable context can be updated by indirect list handle > flow rule configuration. > > `METER_MARK::init_color` is flow resource. > Current flows implementation placed `init_color` in the > `rte_flow_action_meter_mark` making it action level resource. > > The patch removes `init_color` from the `rte_flow_action_meter_mark` > structure. > > API change: > The patch removed: > • struct rte_flow_action_meter_mark::init_color > > • struct rte_flow_update_meter_mark::init_color_valid > > Signed-off-by: Gregory Etelson > Ori, et al., mechanics of the patches looks good, and mlx is the only user, if there is no objection it will be merged in a few days.
Re: [RFC PATCH] ethdev: introduce NAT64 action
On 9/21/2023 4:45 PM, Ferruh Yigit wrote: > On 9/19/2023 11:05 AM, Ori Kam wrote: >> Hi Bing >> >>> -Original Message- >>> From: Bing Zhao >>> Sent: Friday, August 11, 2023 5:07 PM >>> Subject: [RFC PATCH] ethdev: introduce NAT64 action >>> >>> In order to support the communication between IPv4 and IPv6 nodes in >>> the network, different technologies are used, like dual-stacks, >>> tunneling and NAT64. In some IPv4-only clients, it is hard to deploy >>> new software and hardware to support IPv6. >>> >>> NAT64 is a choice and it will also reduce the unnecessary overhead of >>> the traffic in the network. The NAT64 gateways take the >>> responsibility of the packet headers translation between the IPv6 >>> clouds and IPv4-only clouds. >>> >>> This action should support the offloading of the IP headers' >>> translation. The following fields should be reset correctly in the >>> translation. >>> - Version >>> - Traffic Class / TOS >>> - Flow Label (0 in v4) >>> - Payload Length / Total length >>> - Next Header >>> - Hop Limit / TTL >>> >>> Since there are different mapping and translating modes of the >>> addresses, it will depend on the capabilities of each vendor. >>> >>> The ICMP* and transport layers protocol is out of the scope of NAT64 >>> rte_flow action. >>> >>> Reference links: >>> - https://datatracker.ietf.org/doc/html/rfc6146 >>> - https://datatracker.ietf.org/doc/html/rfc6052 >>> - https://datatracker.ietf.org/doc/html/rfc6145 >>> >>> Signed-off-by: Bing Zhao >>> --- >> >> Acked-by: Ori Kam >> > > Hi Bing, > > This is a RFC, but we are not having more comment & objection, so what > do you think to continue with a patch including testpmd implementation? > > Hi Bing, what is the latest status of the patch?
Re: [PATCH v2] ethdev: add TCP/IP modify field IDs
On 9/8/2023 4:49 AM, Suanming Mou wrote:
> Currently, get TCP/IP header or data length information from traffic
> is missing in the modify field IDs. This commit adds the missing
> TCP data_offset, IPv4 IHL/total_len, IPv6 payload_len to modify filed
> IDs. This allows users be able to manager more TCP/IP fields.
>
> Signed-off-by: Suanming Mou
> ---
>
> v2: fix typo tcp_date_off -> tcp_data_off
>
> ---
> app/test-pmd/cmdline_flow.c | 1 +
> lib/ethdev/rte_flow.h | 4
> 2 files changed, 5 insertions(+)
>
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 94827bcc4a..310068ce88 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -937,6 +937,7 @@ static const char *const modify_field_ids[] = {
> "flex_item",
> "hash_result",
> "geneve_opt_type", "geneve_opt_class", "geneve_opt_data", "mpls",
> + "tcp_data_off", "ipv4_ihl", "ipv4_total_len", "ipv6_payload_len",
> NULL
> };
>
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index 2ebb76dbc0..43ba51da6e 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -3875,6 +3875,10 @@ enum rte_flow_field_id {
> RTE_FLOW_FIELD_GENEVE_OPT_CLASS,/**< GENEVE option class. */
> RTE_FLOW_FIELD_GENEVE_OPT_DATA, /**< GENEVE option data. */
> RTE_FLOW_FIELD_MPLS,/**< MPLS header. */
> + RTE_FLOW_FIELD_TCP_DATA_OFFSET, /**< TCP data offset. */
> + RTE_FLOW_FIELD_IPV4_IHL,/**< IPv4 IHL. */
> + RTE_FLOW_FIELD_IPV4_TOTAL_LEN, /**< IPv4 total length. */
> + RTE_FLOW_FIELD_IPV6_PAYLOAD_LEN /**< IPv6 payload length. */
> };
>
> /**
Hi Suanming,
Patch looks good. But, testpmd modify flow action support seems not
documented at all, can you please first add it [1], later update that
document with this patch?
Also can you please check if `rte_flow.rst` also needs to be updated or not?
[1]: `doc/guides/testpmd_app_ug/testpmd_funcs.rst`, `Flow rules
management` section
Re: [PATCH v4] app/testpmd: enable cli for programmable action
On 10/7/2023 11:47 AM, Qi Zhang wrote:
> Parsing command line for rte_flow_action_prog.
>
> Syntax:
>
> "prog name [arguments \
>... end]"
>
Can you please put full rte flow command in the commit log? Like what is
the 'pattern' for above command?
> Use parse_string0 to parse name string.
> Use parse_hex to parse hex string.
> Use struct action_prog_data to store parsed result.
>
> Example:
>
> Action with 2 arguments:
>
> "prog name action0 arguments field0 03FF field1 55AA end"
>
> Action without argument:
>
> "prog name action1"
>
> Signed-off-by: Qi Zhang
>
Is there an existing driver implementation, checking it helps to
understand feature implementation?
> ---
>
> v4:
> - be more generous on the max size of name and value.
>
> v3:
> - refine struct action_prog_data
> - enlarge the max size
>
> v2:
> - fix title
> - minor coding style refine.
>
> app/test-pmd/cmdline_flow.c | 232
> 1 file changed, 232 insertions(+)
>
Hi Qi,
Can you please update documentation too,
`doc/guides/testpmd_app_ug/testpmd_funcs.rst`, `Flow rules management`
section.
> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
> index 21828c144c..ae5556e704 100644
> --- a/app/test-pmd/cmdline_flow.c
> +++ b/app/test-pmd/cmdline_flow.c
> @@ -719,6 +719,13 @@ enum index {
> ACTION_IPV6_EXT_PUSH,
> ACTION_IPV6_EXT_PUSH_INDEX,
> ACTION_IPV6_EXT_PUSH_INDEX_VALUE,
> + ACTION_PROG,
> + ACTION_PROG_NAME,
> + ACTION_PROG_NAME_STRING,
> + ACTION_PROG_ARGUMENTS,
> + ACTION_PROG_ARG_NAME,
> + ACTION_PROG_ARG_VALUE,
> + ACTION_PROG_ARG_END,
> };
>
> /** Maximum size for pattern in struct rte_flow_item_raw. */
> @@ -749,6 +756,23 @@ struct action_rss_data {
> uint16_t queue[ACTION_RSS_QUEUE_NUM];
> };
>
> +#define ACTION_PROG_NAME_SIZE_MAX 256
> +#define ACTION_PROG_ARG_NUM_MAX 16
> +#define ACTION_PROG_ARG_VALUE_SIZE_MAX 64
> +
> +/** Storage for struct rte_flow_action_prog including external data. */
> +struct action_prog_data {
> + struct rte_flow_action_prog conf;
> + struct {
> + char name[ACTION_PROG_NAME_SIZE_MAX];
> + struct rte_flow_action_prog_argument
> args[ACTION_PROG_ARG_NUM_MAX];
> + struct {
> + char names[ACTION_PROG_NAME_SIZE_MAX];
> + uint8_t value[ACTION_PROG_ARG_VALUE_SIZE_MAX];
> + } arg_data[ACTION_PROG_ARG_NUM_MAX];
> + } data;
> +};
> +
> /** Maximum data size in struct rte_flow_action_raw_encap. */
> #define ACTION_RAW_ENCAP_MAX_DATA 512
> #define RAW_ENCAP_CONFS_MAX_NUM 8
> @@ -2169,6 +2193,7 @@ static const enum index next_action[] = {
> ACTION_QUOTA_QU,
> ACTION_IPV6_EXT_REMOVE,
> ACTION_IPV6_EXT_PUSH,
> + ACTION_PROG,
> ZERO,
> };
>
> @@ -2510,6 +2535,13 @@ static const enum index action_represented_port[] = {
> ZERO,
> };
>
> +static const enum index action_prog[] = {
> + ACTION_PROG_NAME,
> + ACTION_PROG_ARGUMENTS,
> + ACTION_NEXT,
> + ZERO,
> +};
> +
> static int parse_set_raw_encap_decap(struct context *, const struct token *,
>const char *, unsigned int,
>void *, unsigned int);
> @@ -2786,6 +2818,18 @@ static int
> parse_qu_mode_name(struct context *ctx, const struct token *token,
> const char *str, unsigned int len, void *buf,
> unsigned int size);
> +static int
> +parse_vc_action_prog(struct context *, const struct token *,
> + const char *, unsigned int, void *,
> + unsigned int);
> +static int
> +parse_vc_action_prog_arg_name(struct context *, const struct token *,
> + const char *, unsigned int, void *,
> + unsigned int);
> +static int
> +parse_vc_action_prog_arg_value(struct context *, const struct token *,
> +const char *, unsigned int, void *,
> +unsigned int);
> static int comp_none(struct context *, const struct token *,
>unsigned int, char *, unsigned int);
> static int comp_boolean(struct context *, const struct token *,
> @@ -7518,6 +7562,48 @@ static const struct token token_list[] = {
> .args = ARGS(ARGS_ENTRY(struct rte_flow_item_tx_queue,
> tx_queue)),
> },
> + [ACTION_PROG] = {
> + .name = "prog",
> + .help = "match a programmable action",
> + .priv = PRIV_ACTION(PROG, sizeof(struct action_prog_data)),
> + .next = NEXT(action_prog),
> + .call = parse_vc_action_prog,
> + },
> + [ACTION_PROG_NAME] = {
> + .name = "name",
> + .help = "programble action name",
>
Can you please remind me again what was the 'name' filed of "struct
rte_flow_action_prog" was for?
> + .
Re: [PATCH v1] app/testpmd: refine encap content
On 8/22/2023 2:13 AM, Zhang, Yuying wrote:
> From: Yuying Zhang
>
> Refine vxlan encap content of all protocol headers.
>
> Fixes: 1960be7d32f8 ("app/testpmd: add VXLAN encap/decap")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Yuying Zhang
>
Ori, Aman, ping for review.
Re: [PATCH] ethdev: add calculate hash function
On 9/26/2023 12:37 PM, Ori Kam wrote: > rte_flow supports insert by index table[1]. > > Using the above table, the application can create rules > that are based on hash. > For example application can create the following logic in order > to create load balancing: > 1. Create insert by index table with 2 rules, that hashes based on dmac > 2. Insert to index 0 a rule that sends the traffic to port A. > 3. Insert to index 1 a rule that sends the traffic to port B. > > Let's also assume that before this table, there is a 5 tuple > match table that jumps to the above table. > > So each packet that matches one of the 5 tuple rules is RSSed > to port A or B, based on dmac hash. > > The issue arises when there is a miss on the 5 tuple table, > which resulted due to the packet being the first packet of this flow, or > fragmented packet or any other reason. > In this case, the application must calculate what would be the > hash calculated by the HW so it can send the packet to the correct > port. > > This new API allows applications to calculate the hash value of a given > packet for a given table. > > [1] - > http://patches.dpdk.org/project/dpdk/patch/20230208030624.78465-2-akozy...@nvidia.com/ > > Signed-off-by: Ori Kam > --- > app/test-pmd/cmdline_flow.c | 86 +++- > app/test-pmd/config.c| 54 ++ > app/test-pmd/testpmd.h | 2 + > lib/ethdev/rte_flow.c| 21 + > lib/ethdev/rte_flow.h| 32 ++ > lib/ethdev/rte_flow_driver.h | 5 +++ > lib/ethdev/version.map | 1 + > 7 files changed, 200 insertions(+), 1 deletion(-) > This is a new rte_flow API but unfortunately there isn't any review/comment, at least it is experimental API. If there is no objection/discussion in next few days, I will merge the feature. Probably it will be another rte flow feature that only NVIDIA knows and uses. While mentioned from using, is the driver update for the feature planned for this release? Meanwhile, can you please update the documentation, `rte_flow.rst` and `testpmd_funcs.rst`? Also can you please rebase on top of latest next-net, this patch conflicts with merged group set miss action feature.
Re: [PATCH v3] ethdev: add packet type matching item
On 10/9/2023 5:24 PM, Alexander Kozyrev wrote: > Add RTE_FLOW_ITEM_TYPE_PTYPE to allow matching on > L2/L3/L4 and tunnel information as defined in mbuf. > > To match on RTE_PTYPE_L4_TCP and RTE_PTYPE_INNER_L4_UDP: > flow pattern_template 0 create pattern_template_id 1 > ingress template ptype packet_type mask 0x0f000f00 / end > flow queue 0 create 0 template_table 1 > pattern_template 0 actions_template 0 > pattern ptype packet_type is 0x02000100 / end > actions queue index 1 / end > > Signed-off-by: Alexander Kozyrev > Acked-by: Ori Kam > Applied to dpdk-next-net/main, thanks.
Re: [PATCH v3] net/axgbe: use CPUID to identify cpu
On 10/4/2023 11:07 AM, Selwin Sebastian wrote:
> Using root complex to identify cpu will not work for vm passthrough.
> CPUID is used to get family and modelid to identify cpu
>
> Fixes: b0db927b5eba ("net/axgbe: use PCI root complex device to distinguish
> device")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Selwin Sebastian
>
Acked-by: Ferruh Yigit
Applied to dpdk-next-net/main, thanks.
Progressing with this patch for now, to not block the functionality, but
when cpuid abstraction by David [1] is matured, can you please support
porting this patch to new API?
[1]
[Introduce x86 specific identification
API](https://patchwork.dpdk.org/project/dpdk/list/?series=29605)
Re: [PATCH 1/2] ethdev: add IPsec event subtype range for PMD specific code
On 10/4/2023 1:59 PM, Nithin Dabilpuram wrote:
> Add IPsec event subtype range for PMD specific code in order
> to accommodate wide range of errors that PMD supports.
> These IPsec event subtypes are used when an error doesn't
> match the spec defined subtypes between RTE_ETH_EVENT_IPSEC_UNKNOWN
> and RTE_ETH_EVENT_IPSEC_MAX. Adding this as -ve error range
> to avoid ABI breakage.
>
> Signed-off-by: Nithin Dabilpuram
> ---
> lib/ethdev/rte_ethdev.h | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 8542257721..f949dfc83d 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -3905,6 +3905,10 @@ struct rte_eth_event_macsec_desc {
> * eth device.
> */
> enum rte_eth_event_ipsec_subtype {
> + /** PMD specific error start */
> + RTE_ETH_EVENT_IPSEC_PMD_ERROR_START = -256,
> + /** PMD specific error end */
> + RTE_ETH_EVENT_IPSEC_PMD_ERROR_END = -1,
> /** Unknown event type */
> RTE_ETH_EVENT_IPSEC_UNKNOWN = 0,
> /** Sequence number overflow */
>
I don't see any problem to extend event subtype with custom error range,
@Akhil, @Anoob what do you think?
Re: [PATCH 2/3] net/ark: remove RQ pacing firmware from PMD
On 10/5/2023 9:52 PM, Ed Czeck wrote:
> features and function have been removed from FPGA firmware
>
I am always a little confused how you manage the deployment, if a
customer requires RQ pacing, how you manage it, at least should it be
documented in driver documentation that RQ pacing supported before
v23.11, or something like that?
If this doesn't make sense for your deployment model, scratch it, this
is just a reminder if it is useful.
> Signed-off-by: Ed Czeck
> ---
> drivers/net/ark/ark_ethdev.c | 62
> drivers/net/ark/ark_global.h | 3 --
> drivers/net/ark/ark_rqp.c| 70
> drivers/net/ark/ark_rqp.h| 58 --
> drivers/net/ark/meson.build | 1 -
> 5 files changed, 15 insertions(+), 179 deletions(-)
> delete mode 100644 drivers/net/ark/ark_rqp.c
> delete mode 100644 drivers/net/ark/ark_rqp.h
>
> diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
> index 90d3c8abe6..306121ba31 100644
> --- a/drivers/net/ark/ark_ethdev.c
> +++ b/drivers/net/ark/ark_ethdev.c
> @@ -17,7 +17,6 @@
> #include "ark_mpu.h"
> #include "ark_ddm.h"
> #include "ark_udm.h"
> -#include "ark_rqp.h"
> #include "ark_pktdir.h"
> #include "ark_pktgen.h"
> #include "ark_pktchkr.h"
> @@ -107,36 +106,32 @@ static const struct rte_pci_id pci_id_ark_map[] = {
> * This structure is used to statically define the capabilities
> * of supported devices.
> * Capabilities:
> - * rqpacing -
> - * Some HW variants require that PCIe read-requests be correctly throttled.
> - * This is called "rqpacing" and has to do with credit and flow control
> - * on certain Arkville implementations.
> + *isvf -- defined for function id that are virtual
> */
> struct ark_caps {
> - bool rqpacing;
> bool isvf;
> };
> struct ark_dev_caps {
> uint32_t device_id;
> struct ark_caps caps;
> };
> -#define SET_DEV_CAPS(id, rqp, vf)\
> - {id, {.rqpacing = rqp, .isvf = vf} }
> +#define SET_DEV_CAPS(id, vf) \
> + {id, {.isvf = vf} }
>
> static const struct ark_dev_caps
> ark_device_caps[] = {
> - SET_DEV_CAPS(0x100d, true, false),
> - SET_DEV_CAPS(0x100e, true, false),
> - SET_DEV_CAPS(0x100f, true, false),
> - SET_DEV_CAPS(0x1010, false, false),
> - SET_DEV_CAPS(0x1017, true, false),
> - SET_DEV_CAPS(0x1018, true, false),
> - SET_DEV_CAPS(0x1019, true, false),
> - SET_DEV_CAPS(0x101a, true, false),
> - SET_DEV_CAPS(0x101b, true, false),
> - SET_DEV_CAPS(0x101c, true, true),
> - SET_DEV_CAPS(0x101e, false, false),
> - SET_DEV_CAPS(0x101f, false, false),
> + SET_DEV_CAPS(0x100d, false),
> + SET_DEV_CAPS(0x100e, false),
> + SET_DEV_CAPS(0x100f, false),
> + SET_DEV_CAPS(0x1010, false),
> + SET_DEV_CAPS(0x1017, false),
> + SET_DEV_CAPS(0x1018, false),
> + SET_DEV_CAPS(0x1019, false),
> + SET_DEV_CAPS(0x101a, false),
> + SET_DEV_CAPS(0x101b, false),
> + SET_DEV_CAPS(0x101c, true),
> + SET_DEV_CAPS(0x101e, false),
> + SET_DEV_CAPS(0x101f, false),
>{.device_id = 0,}
> };
>
> @@ -301,9 +296,6 @@ eth_ark_dev_init(struct rte_eth_dev *dev)
> int port_count = 1;
> int p;
> uint16_t num_queues;
> - bool rqpacing = false;
> -
> - ark->eth_dev = dev;
>
Above "ark->eth_dev" assignment doesn't look directly related with RQ
pacing, I just want to double check if it is removed intentionally?
Re: [PATCH v2 2/3] net/ark: remove RQ pacing firmware from PMD
On 10/10/2023 5:08 PM, Ed Czeck wrote: > features and function have been removed from FPGA firmware > > Signed-off-by: Ed Czeck > --- > v2: > * restore previously incorrectly deleted line > Can you please send v2 of whole set, this helps on CI checks and back tracing via patchwork etc...
Re: [PATCH] ethdev: add calculate hash function
On 10/11/2023 3:11 AM, fengchengwen wrote: > Hi, > > On 2023/10/10 19:05, Ferruh Yigit wrote: >> On 9/26/2023 12:37 PM, Ori Kam wrote: >>> rte_flow supports insert by index table[1]. >>> >>> Using the above table, the application can create rules >>> that are based on hash. >>> For example application can create the following logic in order >>> to create load balancing: >>> 1. Create insert by index table with 2 rules, that hashes based on dmac >>> 2. Insert to index 0 a rule that sends the traffic to port A. >>> 3. Insert to index 1 a rule that sends the traffic to port B. >>> >>> Let's also assume that before this table, there is a 5 tuple >>> match table that jumps to the above table. >>> >>> So each packet that matches one of the 5 tuple rules is RSSed >>> to port A or B, based on dmac hash. >>> >>> The issue arises when there is a miss on the 5 tuple table, >>> which resulted due to the packet being the first packet of this flow, or >>> fragmented packet or any other reason. >>> In this case, the application must calculate what would be the >>> hash calculated by the HW so it can send the packet to the correct >>> port. >>> >>> This new API allows applications to calculate the hash value of a given >>> packet for a given table. >>> >>> [1] - >>> http://patches.dpdk.org/project/dpdk/patch/20230208030624.78465-2-akozy...@nvidia.com/ >>> >>> Signed-off-by: Ori Kam >>> --- >>> app/test-pmd/cmdline_flow.c | 86 +++- >>> app/test-pmd/config.c| 54 ++ >>> app/test-pmd/testpmd.h | 2 + >>> lib/ethdev/rte_flow.c| 21 + >>> lib/ethdev/rte_flow.h| 32 ++ >>> lib/ethdev/rte_flow_driver.h | 5 +++ >>> lib/ethdev/version.map | 1 + >>> 7 files changed, 200 insertions(+), 1 deletion(-) >>> >> >> This is a new rte_flow API but unfortunately there isn't any >> review/comment, at least it is experimental API. If there is no >> objection/discussion in next few days, I will merge the feature. >> >> Probably it will be another rte flow feature that only NVIDIA knows and >> uses. While mentioned from using, is the driver update for the feature > > The hns3 driver support subset of rte_flow, we found the rte_flow feature is > very flexible. > And its implementation varies according to vendors. > > Can the rte_flow be standardized ? > Hi Chengwen, Yes rte_flow is already implemented by many vendors, each uses some subset of it. It is flexible and useful, no concern about it. My point was, most of the new rte_flow features are coming from single vendor and most of them are not fully reviewed by the wider community. As some of the features merged without much review from wider community, not everyone aware of them, and features are not fully benefited from, although that is somewhat related to HW support as Jerin pointed before. As hns3 is a user of the rte_flow already, it would be great to get more feedback and review from hns3 maintainers, that boosts the confidence to the new proposed features/APIs. Thanks, ferruh >> planned for this release? >> >> >> Meanwhile, can you please update the documentation, `rte_flow.rst` and >> `testpmd_funcs.rst`? >> Also can you please rebase on top of latest next-net, this patch >> conflicts with merged group set miss action feature. >> >> . >>
Re: [PATCH] doc: remove confusing command to send patch
On 10/11/2023 9:30 AM, Bruce Richardson wrote: > On Wed, Oct 11, 2023 at 10:03:07AM +0200, Thomas Monjalon wrote: >> 11/10/2023 09:30, David Marchand: >>> On Tue, Oct 10, 2023 at 6:26 PM Thomas Monjalon wrote: In the contributor guide, it was said that no need to Cc maintainers for new additions, probably for new directories not having a maintainer. There is no harm, and it is a good habit, to always Cc maintainers. Remove this case as it can mislead to not Cc maintainers when needed. Signed-off-by: Thomas Monjalon >>> >>> I agree Cc: maintainers should be the default / recommended way of >>> sending patches. >>> >>> Just to convince myself, adding some meson skeleton for a "plop" >>> library, adding an entry in the release notes and hooking in >>> lib/meson.build: >>> $ git show --stat >>> doc/guides/rel_notes/release_23_11.rst | 4 >>> lib/meson.build| 1 + >>> lib/plop/meson.build | 2 ++ >>> >>> $ ./devtools/get-maintainer.sh 0001-new-awesome-library.patch >>> >>> In this case, it translates to an empty To: list if you follow the >>> example command line: >>>git send-email --to-cmd ./devtools/get-maintainer.sh --cc >>> dev@dpdk.org 000*.patch >>> >>> We could add a default list of recipients if no maintainer is found by >>> the script. >>> And the next question is who should be in that list.. >> >> Or we can send to dev@dpdk.org, Cc maintainers. >> This is what I do: >> git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh >> > +1 for this, mainly on the basis of it being what I do too! :-) > I am for "--to-cmd=./devtools/get-maintainer.sh --cc dev@dpdk.org" To highlight response is expected from the maintainers, and community is informed. Also people may have filters to give higher priority to emails they are in 'to' list, high priority is what we want from maintainers :)
Re: [PATCH v4] app/testpmd: enable cli for programmable action
On 10/11/2023 3:24 AM, Zhang, Qi Z wrote:
>
>> -Original Message-
>> From: Ferruh Yigit
>> Sent: Tuesday, October 10, 2023 6:49 PM
>> To: Zhang, Qi Z ; Singh, Aman Deep
>> ; Zhang, Yuying
>> Cc: dev@dpdk.org; Dumitrescu, Cristian ;
>> or...@nvidia.com
>> Subject: Re: [PATCH v4] app/testpmd: enable cli for programmable action
>>
>> On 10/7/2023 11:47 AM, Qi Zhang wrote:
>>> Parsing command line for rte_flow_action_prog.
>>>
>>> Syntax:
>>>
>>> "prog name [arguments \
>>> ... end]"
>>>
>>
>> Can you please put full rte flow command in the commit log? Like what is the
>> 'pattern' for above command?
>
> The pattern part should be independent of the action part,
>
> though for our P4 device, we will prefer use rte_flow_flex_item, something
> like:
>
> flow create 0 pattern flex item is xxx pattern is xxx / flex item is xxx
> pattern is / actions prog name ..
>
> but it does not limit PMD to support flow like below
>
I think agreement was to use flex pattern, and my understand is "struct
rte_flow_item_flex" will be used to present the table_id.
Without not using flex, how driver will detect which table to update?
> flow create 0 pattern eth / ipv4 src is 1.1.1.1 / actions prog name ..
>
> So I think it may not be necessary to highlight the pattern format here.
>
Complete samples helps a lot to user, can you please include the full
rte flow command, you can have flex pattern sample and if you want add
more samples with other patterns but we need to clarify it first.
>>
>>
>>> Use parse_string0 to parse name string.
>>> Use parse_hex to parse hex string.
>>> Use struct action_prog_data to store parsed result.
>>>
>>> Example:
>>>
>>> Action with 2 arguments:
>>>
>>> "prog name action0 arguments field0 03FF field1 55AA end"
>>>
>>> Action without argument:
>>>
>>> "prog name action1"
>>>
>>> Signed-off-by: Qi Zhang
>>>
>>
>> Is there an existing driver implementation, checking it helps to understand
>> feature implementation?
>
> This work is still ongoing, currently we target to upstream on DPDK 24.03
>
If you won't have driver yet, do you have a way to test these commands?
Or is this implementation just theoretical at this stage?
>>
>>
>>> ---
>>>
>>> v4:
>>> - be more generous on the max size of name and value.
>>>
>>> v3:
>>> - refine struct action_prog_data
>>> - enlarge the max size
>>>
>>> v2:
>>> - fix title
>>> - minor coding style refine.
>>>
>>> app/test-pmd/cmdline_flow.c | 232
>>>
>>> 1 file changed, 232 insertions(+)
>>>
>>
>> Hi Qi,
>>
>> Can you please update documentation too,
>> `doc/guides/testpmd_app_ug/testpmd_funcs.rst`, `Flow rules management`
>> section.
>
> Sure.
>
>>
>>
>>> diff --git a/app/test-pmd/cmdline_flow.c b/app/test-pmd/cmdline_flow.c
>>> index 21828c144c..ae5556e704 100644
>>> --- a/app/test-pmd/cmdline_flow.c
>>> +++ b/app/test-pmd/cmdline_flow.c
>>> @@ -719,6 +719,13 @@ enum index {
>>> ACTION_IPV6_EXT_PUSH,
>>> ACTION_IPV6_EXT_PUSH_INDEX,
>>> ACTION_IPV6_EXT_PUSH_INDEX_VALUE,
>>> + ACTION_PROG,
>>> + ACTION_PROG_NAME,
>>> + ACTION_PROG_NAME_STRING,
>>> + ACTION_PROG_ARGUMENTS,
>>> + ACTION_PROG_ARG_NAME,
>>> + ACTION_PROG_ARG_VALUE,
>>> + ACTION_PROG_ARG_END,
>>> };
>>>
>>> /** Maximum size for pattern in struct rte_flow_item_raw. */ @@
>>> -749,6 +756,23 @@ struct action_rss_data {
>>> uint16_t queue[ACTION_RSS_QUEUE_NUM]; };
>>>
>>> +#define ACTION_PROG_NAME_SIZE_MAX 256 #define
>> ACTION_PROG_ARG_NUM_MAX
>>> +16 #define ACTION_PROG_ARG_VALUE_SIZE_MAX 64
>>> +
>>> +/** Storage for struct rte_flow_action_prog including external data.
>>> +*/ struct action_prog_data {
>>> + struct rte_flow_action_prog conf;
>>> + struct {
>>> + char name[ACTION_PROG_NAME_SIZE_MAX];
>>> + struct rte_flow_action_prog_argument
>> args[ACTION_PROG_ARG_NUM_MAX];
>>> + struct {
>>> + char names[ACTION_PROG_NAME_SIZE_MAX];
>>> + uint8_t
>> value[ACTION_PROG_A
Re: [PATCH] doc: remove confusing command to send patch
On 10/11/2023 11:20 AM, Thomas Monjalon wrote: > 11/10/2023 10:41, Ferruh Yigit: >> On 10/11/2023 9:30 AM, Bruce Richardson wrote: >>> On Wed, Oct 11, 2023 at 10:03:07AM +0200, Thomas Monjalon wrote: >>>> 11/10/2023 09:30, David Marchand: >>>>> On Tue, Oct 10, 2023 at 6:26 PM Thomas Monjalon >>>>> wrote: >>>>>> >>>>>> In the contributor guide, it was said that no need to Cc maintainers >>>>>> for new additions, probably for new directories not having a maintainer. >>>>>> There is no harm, and it is a good habit, to always Cc maintainers. >>>>>> >>>>>> Remove this case as it can mislead to not Cc maintainers when needed. >>>>>> >>>>>> Signed-off-by: Thomas Monjalon >>>>> >>>>> I agree Cc: maintainers should be the default / recommended way of >>>>> sending patches. >>>>> >>>>> Just to convince myself, adding some meson skeleton for a "plop" >>>>> library, adding an entry in the release notes and hooking in >>>>> lib/meson.build: >>>>> $ git show --stat >>>>> doc/guides/rel_notes/release_23_11.rst | 4 >>>>> lib/meson.build| 1 + >>>>> lib/plop/meson.build | 2 ++ >>>>> >>>>> $ ./devtools/get-maintainer.sh 0001-new-awesome-library.patch >>>>> >>>>> In this case, it translates to an empty To: list if you follow the >>>>> example command line: >>>>>git send-email --to-cmd ./devtools/get-maintainer.sh --cc >>>>> dev@dpdk.org 000*.patch >>>>> >>>>> We could add a default list of recipients if no maintainer is found by >>>>> the script. >>>>> And the next question is who should be in that list.. >>>> >>>> Or we can send to dev@dpdk.org, Cc maintainers. >>>> This is what I do: >>>> git send-email --to dev@dpdk.org --cc-cmd devtools/get-maintainer.sh >>>> >>> +1 for this, mainly on the basis of it being what I do too! :-) >>> >> >> I am for "--to-cmd=./devtools/get-maintainer.sh --cc dev@dpdk.org" >> >> To highlight response is expected from the maintainers, and community is >> informed. >> >> Also people may have filters to give higher priority to emails they are >> in 'to' list, high priority is what we want from maintainers :) > > They should give high priority when they are Cc as well. > > The problem is that we may have patches with empty "To", > especially for cover letters and new libs. > There are indeed, for those cases I am putting 'dev' to "To:".
Re: [PATCH v3] net/netvsc: add support for mtu_set
On 10/10/2023 8:08 PM, Long Li wrote: >> Subject: [PATCH v3] net/netvsc: add support for mtu_set >> >> Add support for changing the netvsc MTU. The MTU can only be set at nvs >> initialization, therefore to change the MTU the underlying vmbus >> channel(s) are torn down and the vmbus device unmapped and remapped. The >> existing rx and tx queue(s) are reconnected to the new vmbus channel(s). >> >> Signed-off-by: Sam Andrew >> Acked-by: Stephen Hemminger > > Acked-by: Long Li > Applied to dpdk-next-net/main, thanks.
Re: [PATCH v2 1/3] net/ark: support for single function with multiple port
On 10/10/2023 9:42 PM, Ed Czeck wrote:
> Support the creation of multiple ports from one ark device via
> the use of ark pmd extension. I.e., one device with q queue can
> seen a p ports each with q/p queues.
>
> Add unique dev_private data for each port to manage queue assignment.
>
> This patch repairs a latent issue uncovered during testing.
> Fixes: 6799275eeea6 ("net/ark: support virtual functions")
> Cc: sta...@dpdk.org
> Backporting is not requested.
>
> Signed-off-by: Ed Czeck
>
Series applied to dpdk-next-net/main, thanks.
Re: [PATCH v3 4/4] net/tap: use rte_ether_unformat_address
On 10/3/2023 9:29 PM, Stephen Hemminger wrote: > From: David Christensen > > Building DPDK with gcc 12 on a ppc64le system generates a > stringop-overflow warning. Replace the local MAC address > validation function parse_user_mac() with a call to > rte_ether_unformat_addr() instead. > > Bugzilla ID: 1197 > For record, this should be 1195, caught by David: Bugzilla ID: 1195
Re: [PATCH v2] ethdev: add calculate hash function
On 10/10/2023 3:24 PM, Ori Kam wrote: > rte_flow supports insert by index table[1]. > > Using the above table, the application can create rules > that are based on hash. > For example application can create the following logic in order > to create load balancing: > 1. Create insert by index table with 2 rules, that hashes based on dmac > 2. Insert to index 0 a rule that sends the traffic to port A. > 3. Insert to index 1 a rule that sends the traffic to port B. > > Let's also assume that before this table, there is a 5 tuple > match table that jumps to the above table. > > So each packet that matches one of the 5 tuple rules is RSSed > to port A or B, based on dmac hash. > > The issue arises when there is a miss on the 5 tuple table, > which resulted due to the packet being the first packet of this flow, or > fragmented packet or any other reason. > In this case, the application must calculate what would be the > hash calculated by the HW so it can send the packet to the correct > port. > > This new API allows applications to calculate the hash value of a given > packet for a given table. > > [1] - > http://patches.dpdk.org/project/dpdk/patch/20230208030624.78465-2-akozy...@nvidia.com/ > > Signed-off-by: Ori Kam > Applied to dpdk-next-net/main, thanks.
Re: [PATCH v5 01/40] ethdev: overwrite some comment related to RSS
On 10/11/2023 10:27 AM, Jie Hai wrote:
> 1. overwrite the comments of fields of 'rte_eth_rss_conf'.
> 2. Add comments for RTE_ETH_HASH_FUNCTION_DEFAULT.
>
> Signed-off-by: Jie Hai
> ---
> lib/ethdev/rte_ethdev.h | 29 ++---
> lib/ethdev/rte_flow.h | 3 +++
> 2 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 8542257721c9..b9e4e21189d2 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -448,24 +448,23 @@ struct rte_vlan_filter_conf {
> /**
> * A structure used to configure the Receive Side Scaling (RSS) feature
> * of an Ethernet port.
> - * If not NULL, the *rss_key* pointer of the *rss_conf* structure points
> - * to an array holding the RSS key to use for hashing specific header
> - * fields of received packets. The length of this array should be indicated
> - * by *rss_key_len* below. Otherwise, a default random hash key is used by
> - * the device driver.
> - *
> - * The *rss_key_len* field of the *rss_conf* structure indicates the length
> - * in bytes of the array pointed by *rss_key*. To be compatible, this length
> - * will be checked in i40e only. Others assume 40 bytes to be used as before.
> - *
> - * The *rss_hf* field of the *rss_conf* structure indicates the different
> - * types of IPv4/IPv6 packets to which the RSS hashing must be applied.
> - * Supplying an *rss_hf* equal to zero disables the RSS feature.
> */
> struct rte_eth_rss_conf {
> - uint8_t *rss_key;/**< If not NULL, 40-byte hash key. */
> + /**
> + * If used to query, the'rss_key_len' indicates the size of rss key of
> + * the hardware. And only when rss_key_len is not zero, the 'rss_key'
> + * is valid.
> + * If used to configure, rss_key_len indicates the length of the
> + * 'rss_key' if 'rss_key' is not empty.
>
Ahh, different APIs have different expectations :(
Can you please explicitly name the APIs, instead of "to query", "to
configure"?
And there is a note in original comment that *rss_key_len* is only
checked by i40e, rest assume this value as 40 bytes. New comment doesn't
have it.
> + */
> + uint8_t *rss_key;
> uint8_t rss_key_len; /**< hash key length in bytes. */
> - uint64_t rss_hf; /**< Hash functions to apply - see below. */
> + /**
> + * Indicating which type of packets and which part of the packets
> + * to apply for RSS hash, (see RTE_ETH_RSS_*).
>
There is something doesn't sound right from language perspective,
perhaps someone whose native language is English can help, what about:
"Indicates the type of packets or the specific part of packets to which
RSS hashing is to be applied."
> + * Setting *rss_hf* to zero disables the RSS feature.
> + */
> + uint64_t rss_hf;
> };
>
> /*
> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> index b385741fba6d..5d9e3c68af7b 100644
> --- a/lib/ethdev/rte_flow.h
> +++ b/lib/ethdev/rte_flow.h
> @@ -3227,6 +3227,9 @@ struct rte_flow_query_count {
> * Hash function types.
> */
> enum rte_eth_hash_function {
> + /**
> + * DEFAULT means driver decides which hash algorithm to pick.
> + */
> RTE_ETH_HASH_FUNCTION_DEFAULT = 0,
> RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */
> RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */
Re: [PATCH v5 05/40] net/bnx2x: check RSS hash algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote:
> A new field 'algorithm' has been added to rss_conf, check it
> in case of ignoring unsupported values.
>
> Signed-off-by: Jie Hai
> ---
> drivers/net/bnx2x/bnx2x_ethdev.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c
> b/drivers/net/bnx2x/bnx2x_ethdev.c
> index 4448cf2de2d7..078d6db75d1b 100644
> --- a/drivers/net/bnx2x/bnx2x_ethdev.c
> +++ b/drivers/net/bnx2x/bnx2x_ethdev.c
> @@ -196,6 +196,10 @@ bnx2x_dev_configure(struct rte_eth_dev *dev)
> PMD_DRV_LOG(DEBUG, sc, "num_queues=%d, mtu=%d",
> sc->num_queues, sc->mtu);
>
> + if (dev->data->dev_conf.rx_adv_conf.rss_conf.algorithm !=
> + RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> /* allocate ilt */
> if (bnx2x_alloc_ilt_mem(sc) != 0) {
> PMD_DRV_LOG(ERR, sc, "bnx2x_alloc_ilt_mem was failed");
bnx2x doesn't claim RSS support (doc/guides/nics/features/bnx2x.ini), so
I think it will be reasonable to drop this patch.
Re: [PATCH v5 08/40] net/cnxk: check RSS hash algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote:
> A new field 'algorithm' has been added to rss_conf, check it
> in case of ignoring unsupported values.
>
> Signed-off-by: Jie Hai
> ---
> drivers/net/cnxk/cnxk_ethdev.c | 5 +
> drivers/net/cnxk/cnxk_ethdev_ops.c | 3 +++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/net/cnxk/cnxk_ethdev.c b/drivers/net/cnxk/cnxk_ethdev.c
> index 01b707b6c4ac..dc150de745df 100644
> --- a/drivers/net/cnxk/cnxk_ethdev.c
> +++ b/drivers/net/cnxk/cnxk_ethdev.c
> @@ -976,6 +976,10 @@ nix_rss_default_setup(struct cnxk_eth_dev *dev)
> if (rss_hash_level)
> rss_hash_level -= 1;
>
> + if (eth_dev->data->dev_conf.rx_adv_conf.rss_conf.algorithm !=
> + RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> flowkey_cfg = cnxk_rss_ethdev_to_nix(dev, rss_hf, rss_hash_level);
> return roc_nix_rss_default_setup(&dev->nix, flowkey_cfg);
> }
> @@ -1373,6 +1377,7 @@ cnxk_nix_configure(struct rte_eth_dev *eth_dev)
> }
>
> /* Configure RSS */
> +
>
Looks like unintended change.
> rc = nix_rss_default_setup(dev);
> if (rc) {
> plt_err("Failed to configure rss rc=%d", rc);
> diff --git a/drivers/net/cnxk/cnxk_ethdev_ops.c
> b/drivers/net/cnxk/cnxk_ethdev_ops.c
> index 3ade8eed3626..b6cba99cbb7f 100644
> --- a/drivers/net/cnxk/cnxk_ethdev_ops.c
> +++ b/drivers/net/cnxk/cnxk_ethdev_ops.c
> @@ -1054,6 +1054,9 @@ cnxk_nix_rss_hash_update(struct rte_eth_dev *eth_dev,
> int rc = -EINVAL;
> uint8_t alg_idx;
>
> + if (rss_conf->algorithm != RTE_ETH_HASH_FUNCTION_DEFAULT)
> + goto fail;
> +
> if (rss_conf->rss_key && rss_conf->rss_key_len != ROC_NIX_RSS_KEY_LEN) {
> plt_err("Hash key size mismatch %d vs %d",
> rss_conf->rss_key_len, ROC_NIX_RSS_KEY_LEN);
Re: [PATCH v5 09/40] net/cpfl: check RSS hash algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote:
> A new field 'algorithm' has been added to rss_conf, check it
> in case of ignoring unsupported values.
>
> Signed-off-by: Jie Hai
> ---
> drivers/net/cpfl/cpfl_ethdev.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
> index c4ca9343c3e0..6acb6ce9fd22 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/cpfl/cpfl_ethdev.c
> @@ -450,6 +450,9 @@ cpfl_init_rss(struct idpf_vport *vport)
> rss_conf = &dev_data->dev_conf.rx_adv_conf.rss_conf;
> nb_q = dev_data->nb_rx_queues;
>
> + if (rss_conf->algorithm != RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> if (rss_conf->rss_key == NULL) {
> for (i = 0; i < vport->rss_key_size; i++)
> vport->rss_key[i] = (uint8_t)rte_rand();
> @@ -568,6 +571,9 @@ cpfl_rss_hash_update(struct rte_eth_dev *dev,
> return -ENOTSUP;
> }
>
> + if (rss_conf->algorithm != RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> if (!rss_conf->rss_key || rss_conf->rss_key_len == 0) {
> PMD_DRV_LOG(DEBUG, "No key to be configured");
> goto skip_rss_key;
cpfl also doesn't report RSS capability
(doc/guides/nics/features/cpfl.ini), but it is clear that driver
supports RSS.
@Yuying, @Beilei, can you please update .ini file in a separate file?
Re: [PATCH v5 13/40] net/ena: check RSS hash algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote:
> A new field 'algorithm' has been added to rss_conf, check it
> in case of ignoring unsupported values.
>
> Signed-off-by: Jie Hai
> ---
> drivers/net/ena/ena_rss.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ena/ena_rss.c b/drivers/net/ena/ena_rss.c
> index d0ba9d5c0a14..06aff9f3bd49 100644
> --- a/drivers/net/ena/ena_rss.c
> +++ b/drivers/net/ena/ena_rss.c
> @@ -398,6 +398,9 @@ static int ena_rss_hash_set(struct ena_com_dev *ena_dev,
> uint8_t *rss_key;
> int rc;
>
> + if (rss_conf->algorithm != RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> if (rss_conf->rss_key != NULL) {
> /* Reorder the RSS key bytes for the hardware requirements. */
> ena_reorder_rss_hash_key(hw_rss_key, rss_conf->rss_key,
I can see in some drivers configure() ops is not updated, I assume these
are the ones don't have any RSS related config in it, it is not clear
still to add check, but I guess what you are doing is reasonable, I am
OK with this approach.
Re: [PATCH v5 14/40] net/enic: check RSS hash algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote:
> A new field 'algorithm' has been added to rss_conf, check it
> in case of ignoring unsupported values.
>
> Signed-off-by: Jie Hai
> ---
> drivers/net/enic/enic_ethdev.c | 1 +
> drivers/net/enic/enic_main.c | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> index cdf091559196..164f423a85c8 100644
> --- a/drivers/net/enic/enic_ethdev.c
> +++ b/drivers/net/enic/enic_ethdev.c
> @@ -834,6 +834,7 @@ static int enicpmd_dev_rss_hash_conf_get(struct
> rte_eth_dev *dev,
> ENICPMD_FUNC_TRACE();
> if (rss_conf == NULL)
> return -EINVAL;
> +
>
unintended change.
also 'enicpmd_dev_configure()' looks like can be updated.
> if (rss_conf->rss_key != NULL &&
> rss_conf->rss_key_len < ENIC_RSS_HASH_KEY_SIZE) {
> dev_err(enic, "rss_hash_conf_get: wrong rss_key_len. given=%u"
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 19a99a82c501..2eafe7637b3a 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -1428,6 +1428,9 @@ int enic_set_rss_conf(struct enic *enic, struct
> rte_eth_rss_conf *rss_conf)
> }
> }
>
> + if (rss_enable && rss_conf->algorithm != RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> ret = enic_set_niccfg(enic, ENIC_RSS_DEFAULT_CPU, rss_hash_type,
> ENIC_RSS_HASH_BITS, ENIC_RSS_BASE_CPU,
> rss_enable);
Re: [PATCH v5 21/40] net/igc: check RSS hash algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote:
> A new field 'algorithm' has been added to rss_conf, check it
> in case of ignoring unsupported values.
>
> Signed-off-by: Jie Hai
> ---
> drivers/net/igc/igc_ethdev.c | 4
> drivers/net/igc/igc_txrx.c | 5 +
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c
> index 58c4f8092772..11c0f5ff231b 100644
> --- a/drivers/net/igc/igc_ethdev.c
> +++ b/drivers/net/igc/igc_ethdev.c
> @@ -2442,6 +2442,10 @@ eth_igc_rss_hash_update(struct rte_eth_dev *dev,
> struct rte_eth_rss_conf *rss_conf)
> {
> struct igc_hw *hw = IGC_DEV_PRIVATE_HW(dev);
> +
> + if (rss_conf->algorithm != RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> igc_hw_rss_hash_set(hw, rss_conf);
> return 0;
> }
> diff --git a/drivers/net/igc/igc_txrx.c b/drivers/net/igc/igc_txrx.c
> index 5c60e3e99709..5e62e00d2ad9 100644
> --- a/drivers/net/igc/igc_txrx.c
> +++ b/drivers/net/igc/igc_txrx.c
> @@ -818,6 +818,7 @@ igc_rss_configure(struct rte_eth_dev *dev)
> rss_conf = dev->data->dev_conf.rx_adv_conf.rss_conf;
> if (rss_conf.rss_key == NULL)
> rss_conf.rss_key = default_rss_key;
> +
>
unintended change
> igc_hw_rss_hash_set(hw, &rss_conf);
> }
>
> @@ -958,6 +959,10 @@ igc_dev_mq_rx_configure(struct rte_eth_dev *dev)
> return -EINVAL;
> }
>
> + if (dev->data->dev_conf.rx_adv_conf.rss_conf.algorithm !=
> + RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> switch (dev->data->dev_conf.rxmode.mq_mode) {
> case RTE_ETH_MQ_RX_RSS:
> igc_rss_configure(dev);
Re: [PATCH v5 26/40] net/mvpp2: check RSS hash algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote:
> A new field 'algorithm' has been added to rss_conf, check it
> in case of ignoring unsupported values.
>
> Signed-off-by: Jie Hai
> ---
> drivers/net/mvpp2/mrvl_ethdev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/mvpp2/mrvl_ethdev.c b/drivers/net/mvpp2/mrvl_ethdev.c
> index 359a5d1df7ab..817153af2ef1 100644
> --- a/drivers/net/mvpp2/mrvl_ethdev.c
> +++ b/drivers/net/mvpp2/mrvl_ethdev.c
> @@ -440,6 +440,9 @@ mrvl_configure_rss(struct mrvl_priv *priv, struct
> rte_eth_rss_conf *rss_conf)
> if (rss_conf->rss_key)
> MRVL_LOG(WARNING, "Changing hash key is not supported");
>
> + if (rss_conf->algorithm != RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> if (rss_conf->rss_hf == 0) {
> priv->ppio_params.inqs_params.hash_type = PP2_PPIO_HASH_T_NONE;
> } else if (rss_conf->rss_hf & RTE_ETH_RSS_IPV4) {
what about updating 'mrvl_dev_configure()' ?
Re: [PATCH v5 30/40] net/null: check RSS hash algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote:
> A new field 'algorithm' has been added to rss_conf, check it
> in case of ignoring unsupported values.
>
> Signed-off-by: Jie Hai
> ---
> drivers/net/null/rte_eth_null.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/null/rte_eth_null.c b/drivers/net/null/rte_eth_null.c
> index 31081af79752..8427d7484178 100644
> --- a/drivers/net/null/rte_eth_null.c
> +++ b/drivers/net/null/rte_eth_null.c
> @@ -186,6 +186,11 @@ eth_null_copy_tx(void *q, struct rte_mbuf **bufs,
> uint16_t nb_bufs)
> static int
> eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
> {
> + if ((dev->data->dev_conf.rxmode.mq_mode & RTE_ETH_MQ_RX_RSS_FLAG) &&
> + dev->data->dev_conf.rx_adv_conf.rss_conf.algorithm !=
> + RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> return 0;
> }
>
> @@ -444,6 +449,9 @@ eth_rss_hash_update(struct rte_eth_dev *dev, struct
> rte_eth_rss_conf *rss_conf)
>
> rte_spinlock_lock(&internal->rss_lock);
>
> + if (rss_conf->algorithm != RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> if ((rss_conf->rss_hf & internal->flow_type_rss_offloads) != 0)
> dev->data->dev_conf.rx_adv_conf.rss_conf.rss_hf =
> rss_conf->rss_hf &
> internal->flow_type_rss_offloads;
I think 'eth_dev_configure()' can be dropped, as it is not checking any
RSS configuration at all, no need to add this specific one.
Re: [PATCH v5 33/40] net/tap: check RSS hash algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote:
> A new field 'algorithm' has been added to rss_conf, check it
> in case of ignoring unsupported values.
>
> Signed-off-by: Jie Hai
> ---
> drivers/net/tap/rte_eth_tap.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index b25a52655fa2..5e4813637f0b 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -1038,6 +1038,10 @@ tap_dev_configure(struct rte_eth_dev *dev)
> TAP_LOG(INFO, "%s: %s: RX configured queues number: %u",
> dev->device->name, pmd->name, dev->data->nb_rx_queues);
>
> + if (dev->data->dev_conf.rx_adv_conf.rss_conf.algorithm !=
> + RTE_ETH_HASH_FUNCTION_DEFAULT)
> + return -EINVAL;
> +
> return 0;
> }
>
> @@ -1894,6 +1898,10 @@ tap_rss_hash_update(struct rte_eth_dev *dev,
> rte_errno = EINVAL;
> return -rte_errno;
> }
> + if (rss_conf->algorithm != RTE_ETH_HASH_FUNCTION_DEFAULT) {
> + rte_errno = EINVAL;
> + return -rte_errno;
> + }
> if (rss_conf->rss_key && rss_conf->rss_key_len) {
> /*
>* Currently TAP RSS key is hard coded
Similar to null PMD, I think 'tap_dev_configure()' update can be dropped.
Re: [PATCH v5 00/40] support setting and querying RSS algorithms
On 10/11/2023 10:27 AM, Jie Hai wrote: > This patchset is to support setting and querying RSS algorithms. > > -- > v5: > 1. rewrite some comments. > 2. check RSS algorithm for drivers supporting RSS. > 3. change field "func" of rss_conf to "algorithm". > 4. fix commit log for [PATCH v4 4/7]. > 5. add Acked-by Reshma Pattan. > 6. add symmetric_toeplitz_sort for showing. > 7. change "hf" to "hash function" for showing. > > v4: > 1. recomment some definitions related to RSS. > 2. allocate static memory for rss_key instead of dynamic. > 3. use array of strings to get the name of rss algorithm. > 4. add display of rss algorithm with testpmd. > > v3: > 1. fix commit log for PATCH [1/5]. > 2. make RSS ABI changes description to start the actual text at the margin. > 3. move defnition of enum rte_eth_hash_function to rte_ethdev.h. > 4. fix some comment codes. > > v2: > 1. return error if "func" is invalid. > 2. modify the comments of the "func" field. > 3. modify commit log of patch [3/5]. > 4. use malloc instead of rte_malloc. > 5. adjust display format of RSS info. > 6. remove the string display of rss_hf. > > Huisong Li (1): > net/hns3: support setting and querying RSS hash function > > Jie Hai (39): > ethdev: overwrite some comment related to RSS > ethdev: support setting and querying RSS algorithm > net/atlantic: check RSS hash algorithms > net/axgbe: check RSS hash algorithms > net/bnx2x: check RSS hash algorithms > net/bnxt: check RSS hash algorithms > net/bonding: check RSS hash algorithms > net/cnxk: check RSS hash algorithms > net/cpfl: check RSS hash algorithms > net/cxgbe: check RSS hash algorithms > net/dpaa: check RSS hash algorithms > net/dpaa2: check RSS hash algorithms > net/ena: check RSS hash algorithms > net/enic: check RSS hash algorithms > net/fm10k: check RSS hash algorithms > net/hinic: check RSS hash algorithms > net/i40e: check RSS hash algorithms > net/iavf: check RSS hash algorithms > net/ice: check RSS hash algorithms > net/idpf: check RSS hash algorithms > net/igc: check RSS hash algorithms > net/ionic: check RSS hash algorithms > net/ixgbe: check RSS hash algorithms > net/mana: check RSS hash algorithms > net/mlx5: check RSS hash algorithms > net/mvpp2: check RSS hash algorithms > net/netvsc: check RSS hash algorithms > net/ngbe: : check RSS hash algorithms > net/nfp: check RSS hash algorithms > net/null: check RSS hash algorithms > net/qede: check RSS hash algorithms > net/sfc: check RSS hash algorithms > net/tap: check RSS hash algorithms > net/thunderx: check RSS hash algorithms > net/txgbe: check RSS hash algorithms > app/proc-info: fix never show RSS info > app/proc-info: adjust the display format of RSS info > app/proc-info: support querying RSS hash algorithm > app/testpmd: add RSS hash algorithms display > > app/proc-info/main.c | 32 ++- > app/test-pmd/cmdline.c | 29 ++--- > app/test-pmd/config.c | 38 - > app/test-pmd/testpmd.h | 2 +- > doc/guides/rel_notes/release_23_11.rst | 2 + > drivers/net/atlantic/atl_ethdev.c | 2 + > drivers/net/axgbe/axgbe_ethdev.c | 9 + > drivers/net/bnx2x/bnx2x_ethdev.c | 4 ++ > drivers/net/bnxt/bnxt_ethdev.c | 6 +++ > drivers/net/bonding/rte_eth_bond_pmd.c | 6 +++ > drivers/net/cnxk/cnxk_ethdev.c | 5 +++ > drivers/net/cnxk/cnxk_ethdev_ops.c | 3 ++ > drivers/net/cpfl/cpfl_ethdev.c | 6 +++ > drivers/net/cxgbe/cxgbe_ethdev.c | 9 - > drivers/net/dpaa/dpaa_ethdev.c | 7 > drivers/net/dpaa2/dpaa2_ethdev.c | 7 > drivers/net/ena/ena_rss.c | 3 ++ > drivers/net/enic/enic_ethdev.c | 1 + > drivers/net/enic/enic_main.c | 3 ++ > drivers/net/fm10k/fm10k_ethdev.c | 9 - > drivers/net/hinic/hinic_pmd_ethdev.c | 3 ++ > drivers/net/hinic/hinic_pmd_rx.c | 3 ++ > drivers/net/hns3/hns3_rss.c| 47 - > drivers/net/i40e/i40e_ethdev.c | 7 > drivers/net/iavf/iavf_ethdev.c | 6 +++ > drivers/net/ice/ice_dcf.c | 3 ++ > drivers/net/ice/ice_dcf_ethdev.c | 3 ++ > drivers/net/ice/ice_ethdev.c | 7 > drivers/net/idpf/idpf_ethdev.c | 6 +++ > drivers/net/igc/igc_ethdev.c | 4 ++ > drivers/net/igc/igc_txrx.c | 5 +++ > drivers/net/ionic/ionic_ethdev.c | 6 +++ > drivers/net/ixgbe/ixgbe_ethdev.c | 12 +- > drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++ > drivers/net/mana/mana.c| 11 - > drivers/net/mlx5/mlx5_ethdev.c | 4 ++ > drivers/net/mlx5/mlx5_rss.c| 3 +- > drivers/net/mvpp2/mrvl_ethdev.c| 3 ++ > drivers/net/netvsc/hn_ethdev.c | 6 +++ > drivers/net/nfp/nfp_common.c | 9 - > drivers/net/ngbe/ngbe_ethdev.c
Re: [EXT] Re: [PATCH 1/2] ethdev: add IPsec event subtype range for PMD specific code
On 10/10/2023 3:48 PM, Akhil Goyal wrote:
>> On 10/4/2023 1:59 PM, Nithin Dabilpuram wrote:
>>> Add IPsec event subtype range for PMD specific code in order
>>> to accommodate wide range of errors that PMD supports.
>>> These IPsec event subtypes are used when an error doesn't
>>> match the spec defined subtypes between
>> RTE_ETH_EVENT_IPSEC_UNKNOWN
>>> and RTE_ETH_EVENT_IPSEC_MAX. Adding this as -ve error range
>>> to avoid ABI breakage.
>>>
>>> Signed-off-by: Nithin Dabilpuram
>>> ---
>>> lib/ethdev/rte_ethdev.h | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>>> index 8542257721..f949dfc83d 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -3905,6 +3905,10 @@ struct rte_eth_event_macsec_desc {
>>> * eth device.
>>> */
>>> enum rte_eth_event_ipsec_subtype {
>>> + /** PMD specific error start */
>>> + RTE_ETH_EVENT_IPSEC_PMD_ERROR_START = -256,
>>> + /** PMD specific error end */
>>> + RTE_ETH_EVENT_IPSEC_PMD_ERROR_END = -1,
>>> /** Unknown event type */
>>> RTE_ETH_EVENT_IPSEC_UNKNOWN = 0,
>>> /** Sequence number overflow */
>>>
>>
>> I don't see any problem to extend event subtype with custom error range,
>> @Akhil, @Anoob what do you think?
>
> I believe it is ok to add the custom error range.
> It is just that the user will need to check the negative values too, which I
> believe is ok.
>
> Acked-by: Akhil Goyal
>
>
> Acked-by: Anoob Joseph
>
Series applied to dpdk-next-net/main, thanks.
Re: [PATCH v3 1/2] doc: add modify_field action description
On 10/11/2023 12:30 PM, Suanming Mou wrote: > This commit adds the missing modify_field action description to > `testpmd_funcs.rst`. > > Signed-off-by: Suanming Mou > For series, Acked-by: Ferruh Yigit Series applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/nfp: fix the initialization of physical representors
On 10/10/2023 7:03 AM, Chaoyong He wrote:
> From: Zerun Fu
>
> The former logic of initializing the array of physical port
> representors is according to 'nfp_idx', but referencing based
> on 'port_id'. This is a potential bug and will cause segment
> fault when these two values are not equal. Fix it by using the
> 'port_id' as index at all time.
>
> Fixes: e1124c4f8a45 ("net/nfp: add flower representor framework")
> Cc: chaoyong...@corigine.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Zerun Fu
> Reviewed-by: Long Wu
> Reviewed-by: Peng Zhang
> Reviewed-by: Chaoyong He
>
Applied to dpdk-next-net/main, thanks.
Re: [PATCH] ethdev: clarify device queue state after start and stop
On 10/12/2023 10:33 AM, David Marchand wrote:
> Hello Ferruh,
>
> On Thu, Sep 28, 2023 at 10:59 PM Ferruh Yigit wrote:
>>
>> Drivers start/stop device queues on port start/stop, but not all drivers
>> update queue state accordingly.
>>
>> This becomes more visible if a specific queue stopped explicitly and
>> port stopped/started later, in this case although all queues are
>> started, the state of that specific queue is stopped and it is
>> misleading.
>>
>> Misrepresentation of queue state became a defect with commit [1] that
>> does forwarding decision based on queue state and commit [2] that gets
>> up to date queue state from ethdev/device before forwarding.
>>
>> [1]
>> commit 3c4426db54fc ("app/testpmd: do not poll stopped queues")
>>
>> [2]
>> commit 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
>>
>> This patch documents that status of all queues of a device should be
>> `RTE_ETH_QUEUE_STATE_STOPPED` after port stop and their status should
>> be`RTE_ETH_QUEUE_STATE_STARTED` after port start.
>>
>> Also an unit test added to verify drivers.
>>
>> Signed-off-by: Ferruh Yigit
>> ---
>> Cc: Jie Hai
>> Cc: Song Jiale
>> Cc: Yuan Peng
>> Cc: Raslan Darawsheh
>> Cc: Qiming Yang
>> Cc: Ivan Malov
>> Cc: Huisong Li
>>
>> v1:
>> * fix memset
>> * remove commented out code
>> * update unit test to skip queue state if
>> rte_eth_[rt]x_queue_info_get() is not supported
>> ---
>> app/test/meson.build | 1 +
>> app/test/test_ethdev_api.c | 184 +
>> lib/ethdev/rte_ethdev.h| 5 +
>> 3 files changed, 190 insertions(+)
>> create mode 100644 app/test/test_ethdev_api.c
>>
>> diff --git a/app/test/meson.build b/app/test/meson.build
>> index 05bae9216dbc..05bbe84868f6 100644
>> --- a/app/test/meson.build
>> +++ b/app/test/meson.build
>> @@ -65,6 +65,7 @@ source_file_deps = {
>> 'test_efd_perf.c': ['efd', 'hash'],
>> 'test_errno.c': [],
>> 'test_ethdev_link.c': ['ethdev'],
>> +'test_ethdev_api.c': ['ethdev'],
>
> Nit: aphabetical sort
>
ack
>> 'test_event_crypto_adapter.c': ['cryptodev', 'eventdev', 'bus_vdev'],
>> 'test_event_eth_rx_adapter.c': ['ethdev', 'eventdev', 'bus_vdev'],
>> 'test_event_eth_tx_adapter.c': ['bus_vdev', 'ethdev', 'net_ring',
>> 'eventdev'],
>> diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c
>> new file mode 100644
>> index ..68239f82ff33
>> --- /dev/null
>> +++ b/app/test/test_ethdev_api.c
>> @@ -0,0 +1,184 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause
>> + * Copyright (C) 2023, Advanced Micro Devices, Inc.
>> + */
>> +
>> +#include
>> +#include
>> +
>> +#include
>> +#include "test.h"
>> +
>> +#define NUM_RXQ2
>> +#define NUM_TXQ2
>> +#define NUM_RXD 512
>> +#define NUM_TXD 512
>> +#define NUM_MBUF 1024
>> +#define MBUF_CACHE_SIZE 256
>> +
>> +static int32_t
>> +ethdev_api_queue_status(void)
>> +{
>> + struct rte_eth_dev_info dev_info;
>> + struct rte_eth_rxq_info rx_qinfo;
>> + struct rte_eth_txq_info tx_qinfo;
>> + struct rte_mempool *mbuf_pool;
>> + struct rte_eth_conf eth_conf;
>> + uint16_t port_id;
>> + int ret;
>> +
>
> Should we return TEST_SKIPPED if no ethdev port is present?
> It seems more valid to me than returning TEST_SUCCESS.
>
+1 to TEST_SKIPPED
>
>> + mbuf_pool = rte_pktmbuf_pool_create("MBUF_POOL", NUM_MBUF,
>> MBUF_CACHE_SIZE, 0,
>> + RTE_MBUF_DEFAULT_BUF_SIZE, rte_socket_id());
>> +
>> + RTE_ETH_FOREACH_DEV(port_id) {
>> + memset(ð_conf, 0, sizeof(eth_conf));
>> + ret = rte_eth_dev_configure(port_id, NUM_RXQ, NUM_TXQ,
>> ð_conf);
>> + TEST_ASSERT(ret == 0,
>> + "Port(%u) failed to configure.\n", port_id);
>> +
>> + /* RxQ setup */
>> + for (uint16_t queue_id = 0; queue_id < NUM_RXQ; queue_id++) {
>> + ret = rte_eth_rx_queue_setup(port_id, queue_id,
>
Re: [PATCH] doc: update feature list for idpf and cpfl
On 10/12/2023 4:15 PM, beilei.x...@intel.com wrote: > From: Beilei Xing > > Add RSS support in idpf.ini and cpfl.ini. > > Signed-off-by: Beilei Xing > --- > doc/guides/nics/features/cpfl.ini | 3 +++ > doc/guides/nics/features/idpf.ini | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/doc/guides/nics/features/cpfl.ini > b/doc/guides/nics/features/cpfl.ini > index f4e45c7c68..4eb1c829cc 100644 > --- a/doc/guides/nics/features/cpfl.ini > +++ b/doc/guides/nics/features/cpfl.ini > @@ -9,6 +9,9 @@ > [Features] > MTU update = Y > TSO = P > +RSS hash = Y > +RSS key update = Y > +RSS reta update = Y > L3 checksum offload = P > L4 checksum offload = P > Linux= Y > diff --git a/doc/guides/nics/features/idpf.ini > b/doc/guides/nics/features/idpf.ini > index 099fd7f216..d004681c3a 100644 > --- a/doc/guides/nics/features/idpf.ini > +++ b/doc/guides/nics/features/idpf.ini > @@ -9,6 +9,9 @@ > [Features] > MTU update = Y > TSO = P > +RSS hash = Y > +RSS key update = Y > +RSS reta update = Y > L3 checksum offload = P > L4 checksum offload = P > Timestamp offload= P Thanks Beilei, for RSS hash feature update, but both driver has still very small set of feature list, I didn't check the details but is this the feature list complete? Can you please double check? Thanks, ferruh
Re: [PATCH v3 0/8] Enhance the bond framework to support offload
On 10/13/2023 3:22 AM, Chaoyong He wrote: > A gentle ping ~ > > As this patch series add new APIs, hoping it will catch up deadline of > 23.11-RC1 (API freeze). > Hi Chaoyong, I did a quick scan of the patch, followings are red flag to me - adding an ethdev dev_ops for a specific driver (bonding), - adding more PMD specific API for bondig I will try to spend more time to understand the logic behind it but meanwhile I cc'ed more people for comment/review. > Thanks. > >> -Original Message- >> From: Chaoyong He >> Sent: Sunday, October 8, 2023 9:51 AM >> To: dev@dpdk.org >> Cc: oss-drivers ; Chaoyong He >> >> Subject: [PATCH v3 0/8] Enhance the bond framework to support offload >> >> This patch series try to enhance the bond framework to support the offload >> feature better: >> * Add new API to make the member port can access some information of the >> bond port which belongs. >> * Add new API to get the result of whether bond port is created by the >> member port. >> * Add two command line argument to control if enable member port >> notification and dedicated queue features. >> * Add logic to support add ports which share the same PCI address into >> bond port. >> * Also modify the testpmd application to test the new APIs and logics >> added by this patch series. >> >> --- >> v2: >> * Fix compile error on github-robot by removing the redundancy function >> declaration in the header file. >> v3: >> * Use the hole in the structure for the new added flag data field. >> --- >> >> >> Long Wu (8): >> ethdev: add member notification for bonding port >> ethdev: add API to get hardware creation of bonding port >> net/bonding: modify interface comment format >> net/bonding: add bonding port arguments >> net/bonding: support add port by data name >> net/bonding: create new rte flow header file >> net/bonding: support checking valid bonding port ID >> net/bonding: add commands for bonding port notification >> >> .../link_bonding_poll_mode_drv_lib.rst| 19 ++ >> drivers/net/bonding/bonding_testpmd.c | 128 ++ >> drivers/net/bonding/eth_bond_8023ad_private.h | 52 ++-- >> drivers/net/bonding/eth_bond_private.h| 24 +- >> drivers/net/bonding/rte_eth_bond.h| 238 +- >> drivers/net/bonding/rte_eth_bond_8023ad.h | 76 -- >> drivers/net/bonding/rte_eth_bond_alb.h| 34 ++- >> drivers/net/bonding/rte_eth_bond_api.c| 123 + >> drivers/net/bonding/rte_eth_bond_args.c | 47 >> drivers/net/bonding/rte_eth_bond_flow.c | 1 + >> drivers/net/bonding/rte_eth_bond_flow.h | 22 ++ >> drivers/net/bonding/rte_eth_bond_pmd.c| 89 ++- >> drivers/net/bonding/version.map | 5 + >> lib/ethdev/ethdev_driver.h| 38 +++ >> 14 files changed, 766 insertions(+), 130 deletions(-) create mode 100644 >> drivers/net/bonding/rte_eth_bond_flow.h >> >> -- >> 2.39.1 >
[PATCH v2] ethdev: clarify device queue state after start and stop
Drivers start/stop device queues on port start/stop, but not all drivers
update queue state accordingly.
This becomes more visible if a specific queue stopped explicitly and
port stopped/started later, in this case although all queues are
started, the state of that specific queue is stopped and it is
misleading.
Misrepresentation of queue state became a defect with commit [1] that
does forwarding decision based on queue state and commit [2] that gets
up to date queue state from ethdev/device before forwarding.
[1]
commit 3c4426db54fc ("app/testpmd: do not poll stopped queues")
[2]
commit 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
This patch documents that status of all queues of a device should be
`RTE_ETH_QUEUE_STATE_STOPPED` after port stop and their status should
be`RTE_ETH_QUEUE_STATE_STARTED` after port start.
Also an unit test added to verify drivers.
Signed-off-by: Ferruh Yigit
---
Cc: Jie Hai
Cc: Song Jiale
Cc: Yuan Peng
Cc: Raslan Darawsheh
Cc: Qiming Yang
Cc: Ivan Malov
Cc: Huisong Li
v1:
* fix memset
* remove commented out code
* update unit test to skip queue state if
rte_eth_[rt]x_queue_info_get() is not supported
v2:
* return test_skipped when there is no port available
* Syntax fixes
---
app/test/meson.build | 1 +
app/test/test_ethdev_api.c | 185 +
lib/ethdev/rte_ethdev.h| 5 +
3 files changed, 191 insertions(+)
create mode 100644 app/test/test_ethdev_api.c
diff --git a/app/test/meson.build b/app/test/meson.build
index 20a9333c726d..dd0675638b5e 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -68,6 +68,7 @@ source_file_deps = {
'test_efd.c': ['efd', 'net'],
'test_efd_perf.c': ['efd', 'hash'],
'test_errno.c': [],
+'test_ethdev_api.c': ['ethdev'],
'test_ethdev_link.c': ['ethdev'],
'test_event_crypto_adapter.c': ['cryptodev', 'eventdev', 'bus_vdev'],
'test_event_eth_rx_adapter.c': ['ethdev', 'eventdev', 'bus_vdev'],
diff --git a/app/test/test_ethdev_api.c b/app/test/test_ethdev_api.c
new file mode 100644
index ..dc72603d000f
--- /dev/null
+++ b/app/test/test_ethdev_api.c
@@ -0,0 +1,185 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (C) 2023, Advanced Micro Devices, Inc.
+ */
+
+#include
+#include
+
+#include
+#include "test.h"
+
+#define NUM_RXQ2
+#define NUM_TXQ2
+#define NUM_RXD 512
+#define NUM_TXD 512
+#define NUM_MBUF 1024
+#define MBUF_CACHE_SIZE 256
+
+static int32_t
+ethdev_api_queue_status(void)
+{
+ struct rte_eth_dev_info dev_info;
+ struct rte_eth_rxq_info rx_qinfo;
+ struct rte_eth_txq_info tx_qinfo;
+ struct rte_mempool *mbuf_pool;
+ struct rte_eth_conf eth_conf;
+ uint16_t port_id;
+ int ret;
+
+ if (rte_eth_dev_count_avail() == 0)
+ return TEST_SKIPPED;
+
+ mbuf_pool = rte_pktmbuf_pool_create("MBUF_POOL", NUM_MBUF,
MBUF_CACHE_SIZE, 0,
+ RTE_MBUF_DEFAULT_BUF_SIZE, rte_socket_id());
+
+ RTE_ETH_FOREACH_DEV(port_id) {
+ memset(ð_conf, 0, sizeof(eth_conf));
+ ret = rte_eth_dev_configure(port_id, NUM_RXQ, NUM_TXQ,
ð_conf);
+ TEST_ASSERT(ret == 0,
+ "Port(%u) failed to configure.\n", port_id);
+
+ /* RxQ setup */
+ for (uint16_t queue_id = 0; queue_id < NUM_RXQ; queue_id++) {
+ ret = rte_eth_rx_queue_setup(port_id, queue_id, NUM_RXD,
+ rte_socket_id(), NULL, mbuf_pool);
+ TEST_ASSERT(ret == 0,
+ "Port(%u), queue(%u) failed to setup RxQ.\n",
+ port_id, queue_id);
+ }
+
+ /* TxQ setup */
+ for (uint16_t queue_id = 0; queue_id < NUM_TXQ; queue_id++) {
+ ret = rte_eth_tx_queue_setup(port_id, queue_id, NUM_TXD,
+ rte_socket_id(), NULL);
+ TEST_ASSERT(ret == 0,
+ "Port(%u), queue(%u) failed to setup TxQ.\n",
+ port_id, queue_id);
+ }
+
+ ret = rte_eth_dev_info_get(port_id, &dev_info);
+ TEST_ASSERT(ret == 0,
+ "Port(%u) failed to get dev info.\n", port_id);
+
+ /* Initial RxQ */
+ for (uint16_t queue_id = 0; queue_id < dev_info.nb_rx_queues;
queue_id++) {
+ ret = rte_eth_rx_queue_info_get(port_id, queue_id,
&rx_qinfo);
+ if (ret == -ENOTSUP)
+
Re: [PATCH] ethdev: remove init_color from METER_MARK action
On 10/12/2023 10:16 AM, Ori Kam wrote: > Hi Gregory, > >> -Original Message- >> From: Gregory Etelson >> Sent: Tuesday, August 8, 2023 1:01 PM >> >> Indirect list API defines 2 types of action update: >> • Action mutable context is always shared between all flows >> that referenced indirect actions list handle. >> Action mutable context can be changed by explicit invocation >> of indirect handle update function. >> • Flow mutable context is private to a flow. >> Flow mutable context can be updated by indirect list handle >> flow rule configuration. >> >> `METER_MARK::init_color` is flow resource. >> Current flows implementation placed `init_color` in the >> `rte_flow_action_meter_mark` making it action level resource. >> >> The patch removes `init_color` from the `rte_flow_action_meter_mark` >> structure. >> >> API change: >> The patch removed: >> • struct rte_flow_action_meter_mark::init_color >> >> • struct rte_flow_update_meter_mark::init_color_valid >> >> Signed-off-by: Gregory Etelson >> > > Acked-by: Ori Kam > > Applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/mana: add missing \n to DP logs
On 10/13/2023 9:37 PM, lon...@linuxonhyperv.com wrote:
> From: Long Li
>
> Add a missing "\n" to DP logs. This makes the logs correctly formatted.
>
> Signed-off-by: Long Li
>
Fixes: e2d3a3c060c4 ("net/mana: use datapath logging")
Cc: sta...@dpdk.org
Applied to dpdk-next-net/main, thanks.
Re: [PATCH v2 1/8] lib/ethdev: update Rx and Tx queue status
On 10/7/2023 9:36 AM, Jie Hai wrote:
> On 2023/9/28 21:15, Ferruh Yigit wrote:
>> On 9/28/2023 8:42 AM, Jie Hai wrote:
>>> The DPDK framework reports the queue status, which is stored in
>>> 'dev->data->tx_queue_state' and 'dev->data->rx_queue_state'.The
>>> state is currently maintained by the drivers. Users may determine
>>> whether a queue participates in packet forwarding based on the
>>> state. However, not all drivers correctly report the queue status.
>>> This may cause forwarding problems.
>>>
>>> Since it is difficult to modify the queue status of all drivers,
>>> we consider updating the queue status at the framework level.
>>> Some drivers support queues for hairpin, leaving status updating
>>> for such queues to the drivers. Some drivers support
>>> 'deferred_start'. Assume that all drivers that support
>>> 'deferred_start' can obtain the configuration through
>>> 'rte_eth_tx_queue_info_get' and 'rte_eth_rx_queue_info_get'. So
>>> we can directly update the queue status in 'rte_eth_dev_start'
>>> and 'rte_eth_dev_stop'.
>>>
>>> Signed-off-by: Jie Hai
>>> ---
>>> lib/ethdev/rte_ethdev.c | 40
>>> 1 file changed, 40 insertions(+)
>>>
>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>> index 0840d2b5942a..e3aaa71eba9e 100644
>>> --- a/lib/ethdev/rte_ethdev.c
>>> +++ b/lib/ethdev/rte_ethdev.c
>>> @@ -1641,6 +1641,9 @@ rte_eth_dev_start(uint16_t port_id)
>>> struct rte_eth_dev_info dev_info;
>>> int diag;
>>> int ret, ret_stop;
>>> + uint16_t i;
>>> + struct rte_eth_rxq_info rxq_info;
>>> + struct rte_eth_txq_info txq_info;
>>> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>>> dev = &rte_eth_devices[port_id];
>>> @@ -1697,6 +1700,30 @@ rte_eth_dev_start(uint16_t port_id)
>>> (*dev->dev_ops->link_update)(dev, 0);
>>> }
>>> + for (i = 0; i < dev->data->nb_rx_queues; i++) {
>>> + if (rte_eth_dev_is_rx_hairpin_queue(dev, i))
>>> + continue;
>>> +
>>> + memset(&rxq_info, 0, sizeof(rxq_info));
>>> + ret = rte_eth_rx_queue_info_get(port_id, i, &rxq_info);
>>> + if (ret == 0 && rxq_info.conf.rx_deferred_start != 0)
>>> + dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
>>> + else
>>> + dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
>>> + }
>>> +
>>> + for (i = 0; i < dev->data->nb_tx_queues; i++) {
>>> + if (rte_eth_dev_is_tx_hairpin_queue(dev, i))
>>> + continue;
>>> +
>>> + memset(&txq_info, 0, sizeof(txq_info));
>>> + ret = rte_eth_tx_queue_info_get(port_id, i, &txq_info);
>>> + if (ret == 0 && txq_info.conf.tx_deferred_start != 0)
>>> + dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
>>> + else
>>> + dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
>>> + }
>>> +
>>
>> Hi Jie,
>>
>> I am not sure about adding queue_info_get() calls a dependency to
>> start(), since start() is a mandatory API, I am concerned about possible
>> side affects.
>> Like if rte_eth_rx_queue_info_get() fails,Yes, unless we let
>> rte_eth_rx|tx_queue_info_get a mandatory API.
>> Or event though deferred_start is set, can application first call
>> rx_queue_start(), later start(), like:
>> start()
>> rx_queue_start()
>> stop()
>> rx_queue_start()
>> start()
> start() --> deferred_start is confugured, the queue is stopped
> rx_queue_start() --> the queue is started
> stop() --> all queues are stopped
> rx_queue_start() --> not supported, port should starts first
> start()
>
> If we change the order as:
> start()
> rx_queue_start()
> stop()
> start()
>
> The last status of the queue for different drivers may be different.
> Most drivers starts all queues except the queue setting deferred_start.
> For sfc driver, all queues are started and the status of the queue
> setting deferred_start will be reported as stopped, which is not correct.
> That's the point, drivers have their own private
Re: [PATCH 00/36] fix Rx and Tx queue state
On 9/8/2023 12:28 PM, Jie Hai wrote:
> The DPDK framework reports the queue state, which is stored in
> dev->data->tx_queue_state and dev->data->rx_queue_state. The
> state is maintained by the driver. Users may determine whether
> a queue participates in packet forwarding based on the state,
> for example,
>
> [1] 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding"
> [2] 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
>
> Therefore, the drivers need to modify the queue state in time
> according to the actual situation, especially when dev_start
> and dev_stop are called. see [3] for more information.
>
> [3] https://inbox.dpdk.org/dev/20230721160422.3848154-1-ferruh.yi...@amd.com/
>
> This patchset also resubmit the patch [2] and makes some fixes on the patch.
>
> Jie Hai (36):
> net/axgbe: fix Rx and Tx queue state
> net/af_packet: fix Rx and Tx queue state
> net/af_xdp: fix Rx and Tx queue state
> net/avp: fix Rx and Tx queue state
> net/bnx2x: fix Rx and Tx queue state
> net/bnxt: fix Rx and Tx queue state
> net/bonding: fix Rx and Tx queue state
> net/cxgbe: fix Rx and Tx queue state
> net/dpaa: fix Rx and Tx queue state
> net/dpaa2: fix Rx and Tx queue state
> net/e1000: fix Rx and Tx queue state
> net/ena: fix Rx and Tx queue state
> net/enetc: fix Rx and Tx queue state
> net/enic: fix Rx and Tx queue state
> net/hinic: fix Rx and Tx queue state
> net/ipn3ke: fix Rx and Tx queue state
> net/memif: fix Rx and Tx queue state
> net/mana: fix Rx and Tx queue state
> net/mlx4: fix Rx and Tx queue state
> net/mvneta: fix Rx and Tx queue state
> net/mvpp2: fix Rx and Tx queue state
> net/netvsc: fix Rx and Tx queue state
> net/nfp: fix Rx and Tx queue state
> net/ngbe: fix Rx and Tx queue state
> net/null: fix Rx and Tx queue state
> net/octeon_ep: fix Rx and Tx queue state
> net/octeontx: fix Rx and Tx queue state
> net/pfe: fix Rx and Tx queue state
> net/ring: fix Rx and Tx queue state
> net/sfc: fix Rx and Tx queue state
> net/softnic: fix Rx and Tx queue state
> net/txgbe: fix Rx and Tx queue state
> net/vhost: fix Rx and Tx queue state
> net/virtio: fix Rx and Tx queue state
> net/vmxnet3: fix Rx and Tx queue state
> app/testpmd: fix primary process not polling all queues
>
For series,
Acked-by: Ferruh Yigit
Except net/mana (it has separate patch by its maintainer),
Series applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/mana: set the correct queue state
On 7/7/2023 7:53 PM, lon...@linuxonhyperv.com wrote: > From: Long Li > > Set the queue state when queue is started/stopped > > Signed-off-by: Long Li > Applied to dpdk-next-net/main, thanks.
Re: [PATCH 00/36] fix Rx and Tx queue state
On 10/16/2023 12:51 PM, Ferruh Yigit wrote:
> On 9/8/2023 12:28 PM, Jie Hai wrote:
>> The DPDK framework reports the queue state, which is stored in
>> dev->data->tx_queue_state and dev->data->rx_queue_state. The
>> state is maintained by the driver. Users may determine whether
>> a queue participates in packet forwarding based on the state,
>> for example,
>>
>> [1] 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding"
>> [2] 141a520b35f7 ("app/testpmd: fix primary process not polling all queues")
>>
>> Therefore, the drivers need to modify the queue state in time
>> according to the actual situation, especially when dev_start
>> and dev_stop are called. see [3] for more information.
>>
>> [3] https://inbox.dpdk.org/dev/20230721160422.3848154-1-ferruh.yi...@amd.com/
>>
>> This patchset also resubmit the patch [2] and makes some fixes on the patch.
>>
>> Jie Hai (36):
>> net/axgbe: fix Rx and Tx queue state
>> net/af_packet: fix Rx and Tx queue state
>> net/af_xdp: fix Rx and Tx queue state
>> net/avp: fix Rx and Tx queue state
>> net/bnx2x: fix Rx and Tx queue state
>> net/bnxt: fix Rx and Tx queue state
>> net/bonding: fix Rx and Tx queue state
>> net/cxgbe: fix Rx and Tx queue state
>> net/dpaa: fix Rx and Tx queue state
>> net/dpaa2: fix Rx and Tx queue state
>> net/e1000: fix Rx and Tx queue state
>> net/ena: fix Rx and Tx queue state
>> net/enetc: fix Rx and Tx queue state
>> net/enic: fix Rx and Tx queue state
>> net/hinic: fix Rx and Tx queue state
>> net/ipn3ke: fix Rx and Tx queue state
>> net/memif: fix Rx and Tx queue state
>> net/mana: fix Rx and Tx queue state
>> net/mlx4: fix Rx and Tx queue state
>> net/mvneta: fix Rx and Tx queue state
>> net/mvpp2: fix Rx and Tx queue state
>> net/netvsc: fix Rx and Tx queue state
>> net/nfp: fix Rx and Tx queue state
>> net/ngbe: fix Rx and Tx queue state
>> net/null: fix Rx and Tx queue state
>> net/octeon_ep: fix Rx and Tx queue state
>> net/octeontx: fix Rx and Tx queue state
>> net/pfe: fix Rx and Tx queue state
>> net/ring: fix Rx and Tx queue state
>> net/sfc: fix Rx and Tx queue state
>> net/softnic: fix Rx and Tx queue state
>> net/txgbe: fix Rx and Tx queue state
>> net/vhost: fix Rx and Tx queue state
>> net/virtio: fix Rx and Tx queue state
>> net/vmxnet3: fix Rx and Tx queue state
>> app/testpmd: fix primary process not polling all queues
>>
>
> For series,
> Acked-by: Ferruh Yigit
>
>
> Except net/mana (it has separate patch by its maintainer),
> Series applied to dpdk-next-net/main, thanks.
>
net/netvsc also dropped from this set and maintainer version [1] merged.
[1]
https://patchwork.dpdk.org/project/dpdk/patch/169769-10296-1-git-send-email-lon...@linuxonhyperv.com/
Re: [Patch v2] net/netvsc: set the correct queue state
On 8/29/2023 7:29 PM, lon...@linuxonhyperv.com wrote: > From: Long Li > > Set the queue state when queue is started/stopped. > > Signed-off-by: Long Li > Acked-by: Stephen Hemminger > Applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/bonding: fix link status callback stop
On 10/16/2023 11:33 AM, fengchengwen wrote:
> On 2023/10/16 16:47, David Marchand wrote:
>> If a bonding port gets released, a link status alarm callback still
>> referenced the ethdev port that may be reused later.
>> Cancel this callback when stopping the port.
>>
>> Bugzilla ID: 1301
>> Fixes: a45b288ef21a ("bond: support link status polling")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: David Marchand
>
> Acked-by: Chengwen Feng
>
Acked-by: Ferruh Yigit
Applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/nfp: fix the Tx performance issue
On 10/13/2023 8:46 AM, Chaoyong He wrote:
> From: Zerun Fu
>
> The former commit imports a register read operation into the data path
> logic, which will severely degrade performance.
> Fix this bug by moving the register read logic out of the data path.
> Read 'cap_extend' only once in the initialisation logic and store it
> in the data structure. And all other logics that need this value should
> get it from the data structure.
>
> Fixes: 310a1780581e ("net/nfp: support IPsec Rx and Tx offload")
> Cc: shihong.w...@corigine.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Zerun Fu
> Reviewed-by: Long Wu
> Reviewed-by: Peng Zhang
> Reviewed-by: Chaoyong He
>
Applied to dpdk-next-net/main, thanks.
Re: [PATCH 0/2] Fix two converity issue of NFP PMD
On 10/10/2023 7:17 AM, Chaoyong He wrote: > This patch series fix two converity issue 403098 and 403100. > > Long Wu (2): > net/nfp: fix checking return value > net/nfp: fix illegal memory accesses > > Series applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/nfp: update incorrect MAC stats offset
On 10/10/2023 7:09 AM, Chaoyong He wrote:
> From: James Hershaw
>
> The pointer to the beginning of the MAC stats counters for port 1 are
> incorrectly set as the pointer to the beginning of the port 0 MAC stats
> counters, plus the size of the MAC stats counters multiplied by the port
> number.
>
> This patch corrects this by setting the multiplier as the eth_table
> index of the port.
>
> Fixes: f26e82397f6d ("net/nfp: implement xstats")
> Cc: sta...@dpdk.org
>
> Signed-off-by: James Hershaw
> Reviewed-by: Peng Zhang
> Reviewed-by: Chaoyong He
>
Applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/nfp: fix jumbo packet descriptors for NFDk
On 10/10/2023 7:06 AM, Chaoyong He wrote:
> From: Long Wu
>
> When sending a jumbo packet on NFDk the packet must be split between
> multiple descriptors. The first descriptor contains the packet header
> and is limited to NFDK_DESC_TX_DMA_LEN_HEAD bytes. If the packet is
> large, one or more payload descriptors, without a packet header, and
> a size limit of NFDK_DESC_TX_DMA_LEN bytes are appended.
>
> When adjusting the coding style for the for NFDk datapath an error was
> made and the total packet size was modified when creating the first
> descriptor. This resulted in no payload descriptors being created and
> the jumbo packets where truncated.
>
> Fix this by not modifying the total packet length when constructing the
> first descriptor, allowing one or more payload descriptors to be
> created.
>
> Fixes: d7f6d9b21ffa ("net/nfp: adjust coding style for NFDk")
> Cc: chaoyong...@corigine.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Long Wu
> Reviewed-by: Chaoyong He
>
Applied to dpdk-next-net/main, thanks.
Re: [PATCH v2] ethdev: clarify device queue state after start and stop
On 10/16/2023 3:59 PM, Thomas Monjalon wrote:
> 13/10/2023 17:57, Ferruh Yigit:
>> Drivers start/stop device queues on port start/stop, but not all drivers
>> update queue state accordingly.
>>
>> This becomes more visible if a specific queue stopped explicitly and
>> port stopped/started later, in this case although all queues are
>> started, the state of that specific queue is stopped and it is
>> misleading.
>>
>> Misrepresentation of queue state became a defect with commit [1] that
>> does forwarding decision based on queue state and commit [2] that gets
>> up to date queue state from ethdev/device before forwarding.
>>
>> [1]
>> commit 3c4426db54fc ("app/testpmd: do not poll stopped queues")
>>
>> [2]
>> commit 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
>>
>> This patch documents that status of all queues of a device should be
>> `RTE_ETH_QUEUE_STATE_STOPPED` after port stop and their status should
>> be`RTE_ETH_QUEUE_STATE_STARTED` after port start.
>
> It is so basic that it may look stupid :)
> Yes we still have to enforce such basic things, thank you for this work.
>
>> +REGISTER_TEST_COMMAND(ethdev_api, test_ethdev_api);
>
> Maybe add a comment here to explain it is not part of basic suites,
> waiting for all drivers being compliant.
>
ack, I will add while merging
>> + * All device queues (except form deferred start queues) status should be
>> + * `RTE_ETH_QUEUE_STATE_STARTED` after start.
>> + *
>> * On success, all basic functions exported by the Ethernet API (link
>> status,
>> * receive/transmit, and so on) can be invoked.
>> *
>> @@ -2828,6 +2831,8 @@ int rte_eth_dev_start(uint16_t port_id);
>> * Stop an Ethernet device. The device can be restarted with a call to
>> * rte_eth_dev_start()
>> *
>> + * All device queues status should be `RTE_ETH_QUEUE_STATE_STOPPED` after
>> stop.
>> + *
>
> Acked-by: Thomas Monjalon
>
>
Re: [PATCH v2] ethdev: clarify device queue state after start and stop
On 10/16/2023 3:59 PM, Thomas Monjalon wrote:
> 13/10/2023 17:57, Ferruh Yigit:
>> Drivers start/stop device queues on port start/stop, but not all drivers
>> update queue state accordingly.
>>
>> This becomes more visible if a specific queue stopped explicitly and
>> port stopped/started later, in this case although all queues are
>> started, the state of that specific queue is stopped and it is
>> misleading.
>>
>> Misrepresentation of queue state became a defect with commit [1] that
>> does forwarding decision based on queue state and commit [2] that gets
>> up to date queue state from ethdev/device before forwarding.
>>
>> [1]
>> commit 3c4426db54fc ("app/testpmd: do not poll stopped queues")
>>
>> [2]
>> commit 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding")
>>
>> This patch documents that status of all queues of a device should be
>> `RTE_ETH_QUEUE_STATE_STOPPED` after port stop and their status should
>> be`RTE_ETH_QUEUE_STATE_STARTED` after port start.
>
> It is so basic that it may look stupid :)
> Yes we still have to enforce such basic things, thank you for this work.
>
>> +REGISTER_TEST_COMMAND(ethdev_api, test_ethdev_api);
>
> Maybe add a comment here to explain it is not part of basic suites,
> waiting for all drivers being compliant.
>
Following comment added while merging:
/* TODO: Make part of the fast test suite, `REGISTER_FAST_TEST()`,
* when all drivers complies to the queue state requirement
*/
>> + * All device queues (except form deferred start queues) status should be
>> + * `RTE_ETH_QUEUE_STATE_STARTED` after start.
>> + *
>> * On success, all basic functions exported by the Ethernet API (link
>> status,
>> * receive/transmit, and so on) can be invoked.
>> *
>> @@ -2828,6 +2831,8 @@ int rte_eth_dev_start(uint16_t port_id);
>> * Stop an Ethernet device. The device can be restarted with a call to
>> * rte_eth_dev_start()
>> *
>> + * All device queues status should be `RTE_ETH_QUEUE_STATE_STOPPED` after
>> stop.
>> + *
>
> Acked-by: Thomas Monjalon
>
>
Applied to dpdk-next-net/main, thanks.
Re: [PATCH v3 11/11] net/nfp: refact the meson build file
On 10/13/2023 7:06 AM, Chaoyong He wrote: > Make the source files follow the alphabeta sequence. > Also update the copyright header line. > > Signed-off-by: Chaoyong He > Reviewed-by: Long Wu > Reviewed-by: Peng Zhang > --- > drivers/net/nfp/meson.build | 23 --- > 1 file changed, 12 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/nfp/meson.build b/drivers/net/nfp/meson.build > index 7627c3e3f1..40e9ef8524 100644 > --- a/drivers/net/nfp/meson.build > +++ b/drivers/net/nfp/meson.build > @@ -1,10 +1,11 @@ > # SPDX-License-Identifier: BSD-3-Clause > -# Copyright(c) 2018 Intel Corporation > +# Copyright(c) 2018 Corigine, Inc. > ack
Re: [PATCH v3 00/11] Unify the PMD coding style
On 10/13/2023 7:06 AM, Chaoyong He wrote: > This patch series aims to unify the coding style of NFP PMD, make the > logics following the same rules, to make it easier to understand and > extend. > Also prepare for the upcoming vDPA PMD patch series. > > --- > v2: > * Add some missing modification. > v3: > * Remove the '\t' character in the log statement as the advice of > reviewer. > --- > > Chaoyong He (11): > net/nfp: explicitly compare to null and 0 > net/nfp: unify the indent coding style > net/nfp: unify the type of integer variable > net/nfp: standard the local variable coding style > net/nfp: adjust the log statement > net/nfp: standard the comment style > net/nfp: standard the blank character > net/nfp: unify the guide line of header file > net/nfp: rename some parameter and variable > net/nfp: adjust logic to make it more readable > net/nfp: refact the meson build file > It is good to take care of the code and update syntax, coding convention etc, but it also creates noise in the git history and makes backporting fixes/patches harder. For a while the driver got lots of refactoring changes, I hope they are completed with the patches in this release. Series applied to dpdk-next-net/main, thanks.
Re: [PATCH] config/x86: config support for AMD EPYC processors
On 10/17/2023 10:45 AM, Kevin Traynor wrote: > On 16/10/2023 06:20, Tummala, Sivaprasad wrote: >> [AMD Official Use Only - General] >> >> Hi DPDK Techboard, >> >> Request to add this topic for discussion in the next techboard meeting. >> > > Hi Sivaprasad, > > Are you or a representative (Ferruh?) able to attend the techboard > meeting (3pm UTC Wednesday) to discuss this item? > Hi Kevin, I confirmed with Siva that both me and Siva will join the meeting. Thanks, ferruh > thanks, > Kevin. > >> Thanks & Regards, >> Sivaprasad >> >>> -Original Message- >>> From: Tummala, Sivaprasad >>> Sent: Monday, October 16, 2023 10:44 AM >>> To: David Marchand >>> Cc: bruce.richard...@intel.com; konstantin.v.anan...@yandex.ru; >>> dev@dpdk.org; >>> dpdk-techboard ; Thomas Monjalon >>> >>> Subject: RE: [PATCH] config/x86: config support for AMD EPYC processors >>> >>> [AMD Official Use Only - General] >>> >>> Caution: This message originated from an External Source. Use proper >>> caution >>> when opening attachments, clicking links, or responding. >>> >>> >>> [AMD Official Use Only - General] >>> -Original Message- From: David Marchand Sent: Friday, October 6, 2023 1:21 PM To: Tummala, Sivaprasad Cc: bruce.richard...@intel.com; konstantin.v.anan...@yandex.ru; dev@dpdk.org; dpdk-techboard ; Thomas Monjalon Subject: Re: [PATCH] config/x86: config support for AMD EPYC processors Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. On Mon, Sep 25, 2023 at 5:11 PM Sivaprasad Tummala wrote: > > From: Sivaprasad Tummala > > By default, max lcores are limited to 128 for x86 platforms. > On AMD EPYC processors, this limit needs to be increased to leverage > all the cores. > > The patch adjusts the limit specifically for native compilation on > AMD EPYC CPUs. > > Signed-off-by: Sivaprasad Tummala This patch is a revamp of >>> http://inbox.dpdk.org/dev/BY5PR12MB3681C3FC6676BC03F0B42CCC96789@BY5PR 12MB3681.namprd12.prod.outlook.com/ for which a discussion at techboard is supposed to have taken place. But I didn't find a trace of it. One option that had been discussed in the previous thread was to increase the max number of cores for x86. I am unclear if this option has been properly evaluated/debatted. Can the topic be brought again at techboard? >>> >>> Hi David, >>> >>> The patch is intended to detect AMD platforms and enable all CPU >>> cores by default >>> on native builds. >>> >>> As an optimization for memory footprint, users can override this by >>> specifying "- >>> Dmax_lcores" option based on DPDK lcores required for their usecases. >>> >>> Sure, will request to add this topic for discussion at techboard. Thanks. -- David Marchand >> >
Re: [PATCH] ethdev: fix flow action async query coverity
On 10/17/2023 1:21 PM, Ori Kam wrote:
> Hi Suanming,
>
>> -Original Message-
>> From: Suanming Mou
>> Sent: Tuesday, October 17, 2023 11:23 AM
>>
>> This commit adds the ops chcek to fix the coverity issue.
>>
>> Coverity issue: 403258
>> Fixes: c9dc03840873 ("ethdev: add indirect action async query")
>>
>> Signed-off-by: Suanming Mou
>
> Acked-by: Ori Kam
>
Applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/nfp: fix coredump problem when testpmd exit
On 10/17/2023 3:37 AM, Chaoyong He wrote:
> The ".dev_close" should not call rte_eth_dev_release_port() API
> directly, the rte_eth_dev_close() API will do it.
>
> Fixes: 831c44ab7869 ("net/nfp: add flower PF related routines")
> Cc: chaoyong...@corigine.com
> Cc: sta...@dpdk.org
>
> Signed-off-by: Chaoyong He
> Reviewed-by: Long Wu
> Reviewed-by: Peng Zhang
>
Applied to dpdk-next-net/main, thanks.
Re: [PATCH v5 00/40] support setting and querying RSS algorithms
On 10/17/2023 3:06 PM, Thomas Monjalon wrote: > Hello, > > 11/10/2023 11:27, Jie Hai: >> app/proc-info/main.c | 32 ++- >> app/test-pmd/cmdline.c | 29 ++--- >> app/test-pmd/config.c | 38 - >> app/test-pmd/testpmd.h | 2 +- >> doc/guides/rel_notes/release_23_11.rst | 2 + >> drivers/net/atlantic/atl_ethdev.c | 2 + >> drivers/net/axgbe/axgbe_ethdev.c | 9 + >> drivers/net/bnx2x/bnx2x_ethdev.c | 4 ++ >> drivers/net/bnxt/bnxt_ethdev.c | 6 +++ >> drivers/net/bonding/rte_eth_bond_pmd.c | 6 +++ >> drivers/net/cnxk/cnxk_ethdev.c | 5 +++ >> drivers/net/cnxk/cnxk_ethdev_ops.c | 3 ++ >> drivers/net/cpfl/cpfl_ethdev.c | 6 +++ >> drivers/net/cxgbe/cxgbe_ethdev.c | 9 - >> drivers/net/dpaa/dpaa_ethdev.c | 7 >> drivers/net/dpaa2/dpaa2_ethdev.c | 7 >> drivers/net/ena/ena_rss.c | 3 ++ >> drivers/net/enic/enic_ethdev.c | 1 + >> drivers/net/enic/enic_main.c | 3 ++ >> drivers/net/fm10k/fm10k_ethdev.c | 9 - >> drivers/net/hinic/hinic_pmd_ethdev.c | 3 ++ >> drivers/net/hinic/hinic_pmd_rx.c | 3 ++ >> drivers/net/hns3/hns3_rss.c| 47 - >> drivers/net/i40e/i40e_ethdev.c | 7 >> drivers/net/iavf/iavf_ethdev.c | 6 +++ >> drivers/net/ice/ice_dcf.c | 3 ++ >> drivers/net/ice/ice_dcf_ethdev.c | 3 ++ >> drivers/net/ice/ice_ethdev.c | 7 >> drivers/net/idpf/idpf_ethdev.c | 6 +++ >> drivers/net/igc/igc_ethdev.c | 4 ++ >> drivers/net/igc/igc_txrx.c | 5 +++ >> drivers/net/ionic/ionic_ethdev.c | 6 +++ >> drivers/net/ixgbe/ixgbe_ethdev.c | 12 +- >> drivers/net/ixgbe/ixgbe_rxtx.c | 4 ++ >> drivers/net/mana/mana.c| 11 - >> drivers/net/mlx5/mlx5_ethdev.c | 4 ++ >> drivers/net/mlx5/mlx5_rss.c| 3 +- >> drivers/net/mvpp2/mrvl_ethdev.c| 3 ++ >> drivers/net/netvsc/hn_ethdev.c | 6 +++ >> drivers/net/nfp/nfp_common.c | 9 - >> drivers/net/ngbe/ngbe_ethdev.c | 6 ++- >> drivers/net/ngbe/ngbe_rxtx.c | 3 ++ >> drivers/net/null/rte_eth_null.c| 8 >> drivers/net/qede/qede_ethdev.c | 9 - >> drivers/net/sfc/sfc_ethdev.c | 3 ++ >> drivers/net/sfc/sfc_rx.c | 3 ++ >> drivers/net/tap/rte_eth_tap.c | 8 >> drivers/net/thunderx/nicvf_ethdev.c| 10 - >> drivers/net/txgbe/txgbe_ethdev.c | 7 +++- >> drivers/net/txgbe/txgbe_ethdev_vf.c| 7 +++- >> drivers/net/txgbe/txgbe_rxtx.c | 3 ++ >> lib/ethdev/rte_ethdev.c| 17 >> lib/ethdev/rte_ethdev.h| 56 +++--- >> lib/ethdev/rte_flow.c | 1 - >> lib/ethdev/rte_flow.h | 25 +--- >> 55 files changed, 395 insertions(+), 106 deletions(-) > > Changing all drivers is suspicious. > It shows that something is missing in ethdev. > Please can you add the checks in ethdev only? > That is kind of request from me, let me try to summarize what is going on, there is a new config item added to "struct rte_eth_rss_conf" introduced in this set, which is RSS hashing algorithm to use. Problem is none of the existing drivers are taking account this new config item, so application will request it but drivers silently ignore it. This is a generic problem when adding a new config item to existing config struct. So my request was if drivers not supporting it, and it is requested by the application, driver should return an error to let application know that it is not supported, that is why bunch of drivers updated. One option can be adding a new, specific API and dev_ops for this, for this case new config item is related to the existing RSS API, so I think it can be part of the existing API. Other can be to have some kind of capability reporting by drivers, and application will detect it and won't request this new config item, I think Stephen already suggested something like this. This capability flag is again a generic requirement, and `rte_eth_dev_info_get()` partially used for this purpose. I think it will be odd from application perspective to have a capability for just one config item of a feature set. Anyway, I think updating drivers to report they are not supporting new config item is best option to me, but also I think we should discuss this capability reporting in ethdev in a wider context.
[PATCH] mempool: fix internal function documentation
static function `rte_mempool_do_generic_get()` returns zero on success,
not >=0 as its function comment documents.
Since this function called by public API, the comment causes confusion
on the public API return value.
Fixing the internal function documentation for return value.
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Reported-by: Mahesh Adulla
Signed-off-by: Ferruh Yigit
---
.mailmap | 1 +
lib/mempool/rte_mempool.h | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/.mailmap b/.mailmap
index 3f5bab26a81f..bfe451980f1c 100644
--- a/.mailmap
+++ b/.mailmap
@@ -836,6 +836,7 @@ Maciej Rabeda
Maciej Szwed
Madhu Chittim
Madhuker Mythri
+Mahesh Adulla
Mahipal Challa
Mah Yock Gen
Mairtin o Loingsigh
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index f70bf36080fb..86598bc639e6 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -1484,7 +1484,7 @@ rte_mempool_put(struct rte_mempool *mp, void *obj)
* @param cache
* A pointer to a mempool cache structure. May be NULL if not needed.
* @return
- * - >=0: Success; number of objects supplied.
+ * - 0: Success; number of objects supplied.
* - <0: Error; code of driver dequeue function.
*/
static __rte_always_inline int
--
2.34.1
Re: [PATCH] ethdev: fix incorrect function name in comment
On 10/20/2023 2:05 PM, Bruce Richardson wrote:
> For those using the function comments as a guide, provide the name of
> the correct callback function to use when wanting to count dropped
> packets from the ethdev tx buffering system.
>
> Fixes: d6c99e62c852 ("ethdev: add buffered Tx")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Bruce Richardson
>
Acked-by: Ferruh Yigit
Re: [PATCH] docs: add note about experimental API in LTS
On 10/20/2023 4:01 PM, Bruce Richardson wrote: > On Fri, Oct 20, 2023 at 03:19:30PM +0100, Kevin Traynor wrote: >> The justification and impact for changing experimental API on LTS >> branches is different from the main branch. So the policy that is >> being used for allowing experimental APIs to change is stricter on >> the LTS branches. >> >> This was not documented anywhere, so add some documentation. >> >> Suggested-by: David Marchand >> Signed-off-by: Kevin Traynor >> --- > Acked-by: Bruce Richardson > Acked-by: Ferruh Yigit
Re: [PATCH] mempool: fix internal function documentation
On 10/20/2023 5:08 PM, Morten Brørup wrote:
>> From: Ferruh Yigit [mailto:ferruh.yi...@amd.com]
>> Sent: Friday, 20 October 2023 16.47
>>
>> static function `rte_mempool_do_generic_get()` returns zero on success,
>> not >=0 as its function comment documents.
>>
>> Since this function called by public API, the comment causes confusion
>> on the public API return value.
>>
>> Fixing the internal function documentation for return value.
>>
>> Fixes: af75078fece3 ("first public release")
>> Cc: sta...@dpdk.org
>>
>> Reported-by: Mahesh Adulla
>> Signed-off-by: Ferruh Yigit
>> ---
>
> I agree that this is the current situation, and is relied upon elsewhere in
> DPDK.
>
> Reviewed-by: Morten Brørup
>
> However, the documentation for the mempool driver dequeue function type,
> rte_mempool_dequeue_t [1], does not specify allowed return values, so some
> future mempool driver might return a positive value. Please consider updating
> this too. (Also, the mempool driver enqueue/dequeue functions work on a bulk
> of objects, not "an object", as their documentation says.)
>
Hi Morten,
Yes, mempool_ops are missing API parameter documentation, although this
is not directly related with this patch, I can update
'rte_mempool_dequeue_t' while I am around.
> [1]:
> https://elixir.bootlin.com/dpdk/latest/source/lib/mempool/rte_mempool.h#L476
>
[PATCH v2 1/2] mempool: fix internal function documentation
static function `rte_mempool_do_generic_get()` returns zero on success,
not >=0 as its function comment documents.
Since this function called by public API, the comment causes confusion
on the public API return value.
Fixing the internal function documentation for return value.
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Reported-by: Mahesh Adulla
Signed-off-by: Ferruh Yigit
Reviewed-by: Morten Brørup
Acked-by: Huisong Li
---
.mailmap | 1 +
lib/mempool/rte_mempool.h | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/.mailmap b/.mailmap
index 3f5bab26a81f..bfe451980f1c 100644
--- a/.mailmap
+++ b/.mailmap
@@ -836,6 +836,7 @@ Maciej Rabeda
Maciej Szwed
Madhu Chittim
Madhuker Mythri
+Mahesh Adulla
Mahipal Challa
Mah Yock Gen
Mairtin o Loingsigh
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index f70bf36080fb..86598bc639e6 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -1484,7 +1484,7 @@ rte_mempool_put(struct rte_mempool *mp, void *obj)
* @param cache
* A pointer to a mempool cache structure. May be NULL if not needed.
* @return
- * - >=0: Success; number of objects supplied.
+ * - 0: Success; number of objects supplied.
* - <0: Error; code of driver dequeue function.
*/
static __rte_always_inline int
--
2.34.1
[PATCH v2 2/2] mempool: clarify enqueue and dequeue ops return type
API documentations doesn't clarify expected return types for enqueue and
dequeue mempool_ops, clarifying it.
Fixes: 449c49b93a6b ("mempool: support handler operations")
Cc: sta...@dpdk.org
Reported-by: Morten Brørup
Signed-off-by: Ferruh Yigit
---
lib/mempool/rte_mempool.h | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 86598bc639e6..8ed0386ba3f1 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -465,13 +465,19 @@ typedef int (*rte_mempool_alloc_t)(struct rte_mempool
*mp);
typedef void (*rte_mempool_free_t)(struct rte_mempool *mp);
/**
- * Enqueue an object into the external pool.
+ * Enqueue 'n' objects into the external pool.
+ * @return
+ * - 0: Success
+ * - <0: Error
*/
typedef int (*rte_mempool_enqueue_t)(struct rte_mempool *mp,
void * const *obj_table, unsigned int n);
/**
- * Dequeue an object from the external pool.
+ * Dequeue 'n' objects from the external pool.
+ * @return
+ * - 0: Success
+ * - <0: Error
*/
typedef int (*rte_mempool_dequeue_t)(struct rte_mempool *mp,
void **obj_table, unsigned int n);
--
2.34.1
Re: [PATCH] maintainers: volunteer to maintain power library
On 10/23/2023 10:48 AM, Hunt, David wrote: > > On 23/10/2023 05:27, Sivaprasad Tummala wrote: >> Add co-maintainer for power library. >> >> Signed-off-by: Sivaprasad Tummala >> --- >> MAINTAINERS | 1 + >> 1 file changed, 1 insertion(+) >> > Thanks for the help, Sivaprasad. > > Acked-by: David Hunt > Acked-by: Ferruh Yigit
