[dpdk-dev] [PATCH v4] ethdev: add sanity checks in control APIs
This patch adds more sanity checks in control path APIs.
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
v4:
* add error logging.
* delete check in control path API.
v3:
* set port_id checked first.
* add error logging.
v2:
* Removed unnecessary checks.
* Deleted checks in internal API.
* Added documentation in the header file.
---
lib/librte_ethdev/rte_ethdev.c | 288 ++---
lib/librte_ethdev/rte_ethdev.h | 20 ++-
2 files changed, 287 insertions(+), 21 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 6b5cfd6..e734a30 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -199,6 +199,16 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
char *cls_str = NULL;
int str_size;
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterator init for NULL\n");
+ return -EINVAL;
+ }
+
+ if (devargs_str == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterate matching NULL\n");
+ return -EINVAL;
+ }
+
memset(iter, 0, sizeof(*iter));
/*
@@ -293,6 +303,11 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
uint16_t
rte_eth_iterator_next(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterate next for NULL\n");
+ return RTE_MAX_ETHPORTS;
+ }
+
if (iter->cls == NULL) /* invalid ethdev iterator */
return RTE_MAX_ETHPORTS;
@@ -322,6 +337,11 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
void
rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterator clear up for NULL\n");
+ return;
+ }
+
if (iter->bus_str == NULL)
return; /* nothing to free in pure class filter */
free(RTE_CAST_FIELD(iter, bus_str, char *)); /* workaround const */
@@ -622,6 +642,11 @@ rte_eth_find_next_owned_by(uint16_t port_id, const
uint64_t owner_id)
int
rte_eth_dev_owner_new(uint64_t *owner_id)
{
+ if (owner_id == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to get owner id by NULL\n");
+ return -EINVAL;
+ }
+
eth_dev_shared_data_prepare();
rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
@@ -645,6 +670,12 @@ eth_dev_owner_set(const uint16_t port_id, const uint64_t
old_owner_id,
return -ENODEV;
}
+ if (new_owner == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to set ethdev port %u owner to
NULL\n",
+ port_id);
+ return -EINVAL;
+ }
+
if (!eth_is_valid_owner_id(new_owner->id) &&
!eth_is_valid_owner_id(old_owner_id)) {
RTE_ETHDEV_LOG(ERR,
@@ -738,23 +769,30 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
int
rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
{
- int ret = 0;
- struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
-
- eth_dev_shared_data_prepare();
+ struct rte_eth_dev *ethdev;
- rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
- if (port_id >= RTE_MAX_ETHPORTS || !eth_dev_is_allocated(ethdev)) {
+ ethdev = &rte_eth_devices[port_id];
+ if (!eth_dev_is_allocated(ethdev)) {
RTE_ETHDEV_LOG(ERR, "Port id %"PRIu16" is not allocated\n",
port_id);
- ret = -ENODEV;
- } else {
- rte_memcpy(owner, ðdev->data->owner, sizeof(*owner));
+ return -ENODEV;
}
+ if (owner == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u owner by
NULL\n",
+ port_id);
+ return -EINVAL;
+ }
+
+ eth_dev_shared_data_prepare();
+
+ rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
+ rte_memcpy(owner, ðdev->data->owner, sizeof(*owner));
rte_spinlock_unlock(ð_dev_shared_data->ownership_lock);
- return ret;
+
+ return 0;
}
int
@@ -820,7 +858,7 @@ rte_eth_dev_get_port_by_name(const char *name, uint16_t
Re: [dpdk-dev] [PATCH v3] ethdev: add sanity checks in control APIs
Hi, Andrew,
All has been fixed in v4, check it out, thanks.
在 2021/4/14 20:00, Andrew Rybchenko 写道:
On 4/14/21 2:11 PM, Min Hu (Connor) wrote:
This patch adds more sanity checks in control path APIs.
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Please, see below. Error logging is missing in few cases and
I'd like to understand why.
Signed-off-by: Min Hu (Connor)
---
v3:
* set port_id checked first.
* add error logging.
v2:
* Removed unnecessary checks.
* Deleted checks in internal API.
* Added documentation in the header file.
---
lib/librte_ethdev/rte_ethdev.c | 274 ++---
lib/librte_ethdev/rte_ethdev.h | 20 ++-
2 files changed, 271 insertions(+), 23 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 6b5cfd6..dfebcc9 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -199,6 +199,9 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
char *cls_str = NULL;
int str_size;
+ if (iter == NULL || devargs_str == NULL)
+ return -EINVAL;
+
Is error logging skipped here intentially? Why?
memset(iter, 0, sizeof(*iter));
/*
@@ -293,7 +296,7 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
uint16_t
rte_eth_iterator_next(struct rte_dev_iterator *iter)
{
- if (iter->cls == NULL) /* invalid ethdev iterator */
+ if (iter == NULL || iter->cls == NULL) /* invalid ethdev iterator */
return RTE_MAX_ETHPORTS;
Is error logging skipped here intentially? Why?
do { /* loop to try all matching rte_device */
@@ -322,7 +325,7 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
void
rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
{
- if (iter->bus_str == NULL)
+ if (iter == NULL || iter->bus_str == NULL)
return; /* nothing to free in pure class filter */
Is error logging skipped here intentially? Why?
free(RTE_CAST_FIELD(iter, bus_str, char *)); /* workaround const */
free(RTE_CAST_FIELD(iter, cls_str, char *)); /* workaround const */
[snip]
@@ -2491,6 +2536,12 @@ rte_eth_tx_done_cleanup(uint16_t port_id, uint16_t
queue_id, uint32_t free_cnt)
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_done_cleanup, -ENOTSUP);
+ if (queue_id >= dev->data->nb_tx_queues) {
+ RTE_ETHDEV_LOG(ERR, "Queue id should be < %u.",
+ dev->data->nb_tx_queues);
+ return -EINVAL;
+ }
+
Again, it is not always a control path. So, I'm not sure that we should
add the check there.
/* Call driver to free pending mbufs. */
ret = (*dev->dev_ops->tx_done_cleanup)(dev->data->tx_queues[queue_id],
free_cnt);
[snip]
@@ -2667,6 +2732,9 @@ rte_eth_link_speed_to_str(uint32_t link_speed)
int
rte_eth_link_to_str(char *str, size_t len, const struct rte_eth_link
*eth_link)
{
+ if (str == NULL || eth_link == NULL)
+ return -EINVAL;
+
Is error logging skipped here intentionally? Why?
if (eth_link->link_status == ETH_LINK_DOWN)
return snprintf(str, len, "Link down");
else
[snip]
@@ -4602,6 +4784,9 @@ rte_eth_dma_zone_free(const struct rte_eth_dev *dev,
const char *ring_name,
const struct rte_memzone *mz;
int rc = 0;
+ if (dev == NULL || ring_name == NULL)
+ return -EINVAL;
+
Same question about logging here.
rc = eth_dev_dma_mzone_name(z_name, sizeof(z_name), dev->data->port_id,
queue_id, ring_name);
if (rc >= RTE_MEMZONE_NAMESIZE) {
[snip]
@@ -5629,6 +5861,8 @@ rte_eth_representor_id_get(const struct rte_eth_dev
*ethdev,
struct rte_eth_representor_info *info = NULL;
size_t size;
+ if (ethdev == NULL)
+ return -EINVAL;
Question about logging here as well.
if (type == RTE_ETH_REPRESENTOR_NONE)
return 0;
if (repr_id == NULL)
[snip]
.
Re: [dpdk-dev] [PATCH 6/9] net/hns3: report the speed capability for PF
在 2021/4/15 8:57, Ferruh Yigit 写道: On 4/15/2021 1:39 AM, Ferruh Yigit wrote: On 4/13/2021 2:47 PM, Min Hu (Connor) wrote: From: Huisong Li The speed capability of the device can be reported to the upper-layer app in rte_eth_dev_info_get API. In this API, the speed capability is derived from the 'supported_speed', which is the speed capability actually supported by the NIC. The value of the 'supported_speed' is obtained once in the probe stage and may be updated in the scheduled task to deal with the change of the transmission interface. Signed-off-by: Huisong Li Signed-off-by: Min Hu (Connor) <...> @@ -2688,6 +2749,7 @@ hns3_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *info) .nb_mtu_seg_max = hw->max_non_tso_bd_num, }; + info->speed_capa = hns3_get_speed_capa(hw); Can you please update 'hns3.ini', to advertise 'Speed capabilities' support? 'hns3.ini' added while merging, please verify the final commit. Thanks Ferruh. .
[dpdk-dev] [PATCH v2] net/hns3: rename Rx burst API
From: Chengwen Feng
Currently, user could use runtime config "rx_func_hint=simple" to
select the hns3_recv_pkts API, but the API's name get from
rte_eth_rx_burst_mode_get is "Scalar" which has not reflected "simple".
So this patch renames hns3_recv_pkts to hns3_recv_pkts_simple, and
also change it's name which gets from rte_eth_rx_burst_mode_get to
"Scalar Simple" to maintain conceptual consistency.
Also changes the hns3_recv_scattered_pkts API's name which gets from
rte_eth_rx_burst_mode_get to "Scalar".
Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
v2:
* revert "Scalar Scattered".
---
drivers/net/hns3/hns3_rxtx.c | 18 ++
drivers/net/hns3/hns3_rxtx.h | 4 ++--
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 19adf00..bc087fc 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -1992,7 +1992,7 @@ hns3_dev_supported_ptypes_get(struct rte_eth_dev *dev)
RTE_PTYPE_UNKNOWN
};
- if (dev->rx_pkt_burst == hns3_recv_pkts ||
+ if (dev->rx_pkt_burst == hns3_recv_pkts_simple ||
dev->rx_pkt_burst == hns3_recv_scattered_pkts ||
dev->rx_pkt_burst == hns3_recv_pkts_vec ||
dev->rx_pkt_burst == hns3_recv_pkts_vec_sve)
@@ -2360,7 +2360,9 @@ hns3_rx_ptp_timestamp_handle(struct hns3_rx_queue *rxq,
struct rte_mbuf *mbuf,
}
uint16_t
-hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
+hns3_recv_pkts_simple(void *rx_queue,
+ struct rte_mbuf **rx_pkts,
+ uint16_t nb_pkts)
{
volatile struct hns3_desc *rx_ring; /* RX ring (desc) */
volatile struct hns3_desc *rxdp; /* pointer of the current desc */
@@ -2743,10 +2745,10 @@ hns3_rx_burst_mode_get(struct rte_eth_dev *dev,
__rte_unused uint16_t queue_id,
eth_rx_burst_t pkt_burst;
const char *info;
} burst_infos[] = {
- { hns3_recv_pkts, "Scalar" },
+ { hns3_recv_pkts_simple,"Scalar Simple" },
{ hns3_recv_scattered_pkts, "Scalar Scattered" },
- { hns3_recv_pkts_vec, "Vector Neon" },
- { hns3_recv_pkts_vec_sve, "Vector Sve" },
+ { hns3_recv_pkts_vec, "Vector Neon" },
+ { hns3_recv_pkts_vec_sve, "Vector Sve"},
};
eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
@@ -2792,14 +2794,14 @@ hns3_get_rx_function(struct rte_eth_dev *dev)
if (hns->rx_func_hint == HNS3_IO_FUNC_HINT_SVE && sve_allowed)
return hns3_recv_pkts_vec_sve;
if (hns->rx_func_hint == HNS3_IO_FUNC_HINT_SIMPLE && simple_allowed)
- return hns3_recv_pkts;
+ return hns3_recv_pkts_simple;
if (hns->rx_func_hint == HNS3_IO_FUNC_HINT_COMMON)
return hns3_recv_scattered_pkts;
if (vec_allowed)
return hns3_recv_pkts_vec;
if (simple_allowed)
- return hns3_recv_pkts;
+ return hns3_recv_pkts_simple;
return hns3_recv_scattered_pkts;
}
@@ -4474,7 +4476,7 @@ hns3_dev_rx_descriptor_status(void *rx_queue, uint16_t
offset)
rxdp = &rxq->rx_ring[desc_id];
bd_base_info = rte_le_to_cpu_32(rxdp->rx.bd_base_info);
dev = &rte_eth_devices[rxq->port_id];
- if (dev->rx_pkt_burst == hns3_recv_pkts ||
+ if (dev->rx_pkt_burst == hns3_recv_pkts_simple ||
dev->rx_pkt_burst == hns3_recv_scattered_pkts) {
if (offset >= rxq->nb_rx_desc - rxq->rx_free_hold)
return RTE_ETH_RX_DESC_UNAVAIL;
diff --git a/drivers/net/hns3/hns3_rxtx.h b/drivers/net/hns3/hns3_rxtx.h
index 10a6c64..ad85d0d 100644
--- a/drivers/net/hns3/hns3_rxtx.h
+++ b/drivers/net/hns3/hns3_rxtx.h
@@ -683,8 +683,8 @@ int hns3_dev_rx_queue_start(struct rte_eth_dev *dev,
uint16_t rx_queue_id);
int hns3_dev_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id);
int hns3_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id);
int hns3_dev_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id);
-uint16_t hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
- uint16_t nb_pkts);
+uint16_t hns3_recv_pkts_simple(void *rx_queue, struct rte_mbuf **rx_pkts,
+ uint16_t nb_pkts);
uint16_t hns3_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
uint16_t nb_pkts);
uint16_t hns3_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
--
2.7.4
Re: [dpdk-dev] [PATCH 10/12] net/hns3: rename Rx burst API
在 2021/4/15 1:41, Ferruh Yigit 写道:
On 4/13/2021 12:50 PM, Min Hu (Connor) wrote:
From: Chengwen Feng
Currently, user could use runtime config "rx_func_hint=simple" to
select the hns3_recv_pkts API, but the API's name get from
rte_eth_rx_burst_mode_get is "Scalar" which has not reflected "simple".
So this patch renames hns3_recv_pkts to hns3_recv_pkts_simple, and
also change it's name which gets from rte_eth_rx_burst_mode_get to
"Scalar Simple" to maintain conceptual consistency.
Also changes the hns3_recv_scattered_pkts API's name which gets from
rte_eth_rx_burst_mode_get to "Scalar".
Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
<...>
@@ -2743,10 +2745,10 @@ hns3_rx_burst_mode_get(struct rte_eth_dev
*dev, __rte_unused uint16_t queue_id,
eth_rx_burst_t pkt_burst;
const char *info;
} burst_infos[] = {
- { hns3_recv_pkts, "Scalar" },
- { hns3_recv_scattered_pkts, "Scalar Scattered" },
- { hns3_recv_pkts_vec, "Vector Neon" },
- { hns3_recv_pkts_vec_sve, "Vector Sve" },
+ { hns3_recv_pkts_simple, "Scalar Simple" },
+ { hns3_recv_scattered_pkts, "Scalar" },
+ { hns3_recv_pkts_vec, "Vector Neon" },
+ { hns3_recv_pkts_vec_sve, "Vector Sve" },
No concern on the burst function rename, that is driver internal, but
related to the above change, I think new value "Scalar Simple" is not
clear, what does 'Simple' mean?
At least previously "Scalar Scattered" vs "Scalar" was more clear, one
can easily say difference is scattered Rx support, but with "Scalar" vs
"Scalar Simple" the difference is not clear.
Agreed to retain the hns3_recv_scattered_pkts name "Scalar Scattered",
but suggests changing the hns3_recv_pkts_simple name to "Scalar Simple"
for the following reasons:
1. Currently, the transmit and receive algorithms implemented in C language
only process single-BD algorithms. The Rx direction is Scalar, while the
Tx direction is Scalar Simple. The two do not correspond with each other.
2. The algorithm name selected by using rx_func_hint=simple does not contain
simple. The DPDK user may be confused.
BTW, v2 has been sent, please check it out, thanks.
.
[dpdk-dev] [PATCH v2] examples: add eal cleanup to examples
From: Chengchang Tang
According to the programming guide, the rte_eal_init should be used pairs
with rte_eal_cleanup.
This patch add rte_eal_cleanup to examples to encourage new users of
DPDK to use it.
Fixes: aec9c13c5257 ("eal: add function to release internal resources")
Fixes: 3d0fad56b74a ("examples/fips_validation: add crypto FIPS application")
Fixes: c8e6ceecebc1 ("examples/ioat: add new sample app for ioat driver")
Fixes: 4ff457986f76 ("examples/l2fwd-event: add default poll mode routines")
Fixes: 08bd1a174461 ("examples/l3fwd-graph: add graph-based l3fwd skeleton")
Fixes: c5eebf85badc ("examples/ntb: add example for NTB")
Fixes: b77f66002812 ("examples/pipeline: add new example application")
Fixes: edbed86d1cc3 ("examples/vdpa: introduce a new sample for vDPA")
Fixes: c19beb3f38cd ("examples/vhost_blk: introduce vhost storage sample")
Fixes: f5188211c721 ("examples/vhost_crypto: add sample application")
Cc: sta...@dpdk.org
Signed-off-by: Chengchang Tang
---
examples/bbdev_app/main.c | 3 +++
examples/bond/main.c | 4
examples/cmdline/main.c| 3 +++
examples/distributor/main.c| 3 +++
examples/ethtool/ethtool-app/main.c| 3 +++
examples/fips_validation/main.c| 3 +++
examples/flow_classify/flow_classify.c | 3 +++
examples/flow_filtering/main.c | 7 ++-
examples/helloworld/main.c | 4
examples/ioat/ioatfwd.c| 3 +++
examples/ip_fragmentation/main.c | 3 +++
examples/ip_reassembly/main.c | 3 +++
examples/ipsec-secgw/ipsec-secgw.c | 3 +++
examples/ipv4_multicast/main.c | 3 +++
examples/kni/main.c| 3 +++
examples/l2fwd-cat/l2fwd-cat.c | 3 +++
examples/l2fwd-crypto/main.c | 3 +++
examples/l2fwd-event/main.c| 3 +++
examples/l2fwd-jobstats/main.c | 3 +++
examples/l2fwd-keepalive/main.c| 4
examples/l2fwd/main.c | 3 +++
examples/l3fwd-acl/main.c | 3 +++
examples/l3fwd-graph/main.c| 3 +++
examples/l3fwd/main.c | 4
examples/link_status_interrupt/main.c | 3 +++
examples/multi_process/client_server_mp/mp_client/client.c | 3 +++
examples/multi_process/client_server_mp/mp_server/main.c | 4
examples/multi_process/simple_mp/main.c| 4
examples/multi_process/symmetric_mp/main.c | 3 +++
examples/ntb/ntb_fwd.c | 3 +++
examples/packet_ordering/main.c| 4
examples/performance-thread/l3fwd-thread/main.c| 3 +++
examples/performance-thread/pthread_shim/main.c| 4
examples/pipeline/main.c | 3 +++
examples/ptpclient/ptpclient.c | 3 +++
examples/qos_meter/main.c | 3 +++
examples/qos_sched/main.c | 3 +++
examples/rxtx_callbacks/main.c | 4
examples/server_node_efd/node/node.c | 3 +++
examples/server_node_efd/server/main.c | 4
examples/service_cores/main.c | 3 +++
examples/skeleton/basicfwd.c | 3 +++
examples/timer/main.c | 3 +++
examples/vdpa/main.c | 3 +++
examples/vhost/main.c | 4 +++-
examples/vhost_blk/vhost_blk.c | 3 +++
examples/vhost_crypto/main.c | 3 +++
examples/vm_power_manager/guest_cli/main.c | 3 +++
examples/vm_power_manager/main.c | 3 +++
examples/vmdq/main.c | 3 +++
examples/vmdq_dcb/main.c | 3 +++
51 files changed, 166 insertions(+), 2 deletions(-)
diff --git a/examples/bbdev_app/main.c b/examples/bbdev_app/main.c
index 20cfd32..5251db0 100644
--- a/examples/bbdev_app/main.c
+++ b/examples/bbdev_app/main.c
@@ -1195,5 +1195,8 @@ main(int argc, char **argv)
ret |= rte_eal_wait_lcore(lcore_id);
}
+ /* clean up the EAL */
+ rte_eal_cleanup();
+
return ret;
}
diff --git a/examples/bond/main.c b/examples/bond/main.c
index 81a6fa9..f48400e 100644
--- a/examples/bond/main.c
++
Re: [dpdk-dev] [PATCH 00/45] add eal clean up to the example
在 2021/4/14 16:23, Thomas Monjalon 写道: 14/04/2021 04:10, Min Hu (Connor): According to the programming guide, the rte_eal_init should be used pairs with rte_eal_cleanup. This set of patches add eal clean up to the examples. Thank you Chengchang Tang (45): It can be a single patch for all examples. Well, merge to a single patch in v2, please check it, thanks. .
[dpdk-dev] [PATCH 2/3] net/hns3: support Rx checksum simple process
From: Chengwen Feng Currently, the L3L4P/L3E/L4E/OL3E/OL4E fields in Rx descriptor used to indicate hardware checksum result: 1. L3L4P: indicates hardware has processed L3L4 checksum for this packet, if this bit is 1 then L3E/L4E/OL3E/OL4E is trustable. 2. L3E: L3 checksum error indication, 1 means with error. 3. L4E: L4 checksum error indication, 1 means with error. 4. OL3E: outer L3 checksum error indication, 1 means with error. 5. OL4E: outer L4 checksum error indication, 1 means with error. Driver will set the good checksum flag through packet type and L3E/L4E/OL3E/OL4E when L3L4P is 1, it runs as follows: 1. If packet type indicates it's tunnel packet: 1.1. If packet type indicates it has inner L3 and L3E is zero, then mark the IP checksum good. 1.2. If packet type indicates it has inner L4 and L4E is zero, then mark the L4 checksum good. 1.3. If packet type indicates it has outer L4 and OL4E is zero, then mark the outer L4 checksum good. 2. If packet type indicates it's not tunnel packet: 2.1. If packet type indicates it has L3 and L3E is zero, then mark the IP checksum good. 2.2. If packet type indicates it has L4 and L4E is zero, then mark the L4 checksum good. As described above, the good checksum calculation is time consuming, it impacts the Rx performance. By balancing performance and functionality, driver uses the following scheme to set good checksum flag when L3L4P is 1: 1. If L3E is zero, then mark the IP checksum good. 2. If L4E is zero, then mark the L4 checksum good. The performance gains are 3% in small packet iofwd scenarios. Signed-off-by: Chengwen Feng Signed-off-by: Min Hu (Connor) --- drivers/net/hns3/hns3_rxtx.c | 14 + drivers/net/hns3/hns3_rxtx.h | 103 ++ drivers/net/hns3/hns3_rxtx_vec_neon.h | 7 +-- drivers/net/hns3/hns3_rxtx_vec_sve.c | 6 +- 4 files changed, 45 insertions(+), 85 deletions(-) diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c index bec7fae..c29c0cf 100644 --- a/drivers/net/hns3/hns3_rxtx.c +++ b/drivers/net/hns3/hns3_rxtx.c @@ -2400,7 +2400,6 @@ hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) struct rte_mbuf *nmb; /* pointer of the new mbuf */ struct rte_mbuf *rxm; uint32_t bd_base_info; - uint32_t cksum_err; uint32_t l234_info; uint32_t ol_info; uint64_t dma_addr; @@ -2475,8 +2474,7 @@ hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) /* Load remained descriptor data and extract necessary fields */ l234_info = rte_le_to_cpu_32(rxd.rx.l234_info); ol_info = rte_le_to_cpu_32(rxd.rx.ol_info); - ret = hns3_handle_bdinfo(rxq, rxm, bd_base_info, -l234_info, &cksum_err); + ret = hns3_handle_bdinfo(rxq, rxm, bd_base_info, l234_info); if (unlikely(ret)) goto pkt_err; @@ -2485,9 +2483,6 @@ hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) if (rxm->packet_type == RTE_PTYPE_L2_ETHER_TIMESYNC) rxm->ol_flags |= PKT_RX_IEEE1588_PTP; - if (likely(bd_base_info & BIT(HNS3_RXD_L3L4P_B))) - hns3_rx_set_cksum_flag(rxm, rxm->packet_type, - cksum_err); hns3_rxd_to_vlan_tci(rxq, rxm, l234_info, &rxd); /* Increment bytes counter */ @@ -2526,7 +2521,6 @@ hns3_recv_scattered_pkts(void *rx_queue, struct rte_mbuf *rxm; struct rte_eth_dev *dev; uint32_t bd_base_info; - uint32_t cksum_err; uint32_t l234_info; uint32_t gro_size; uint32_t ol_info; @@ -2700,17 +2694,13 @@ hns3_recv_scattered_pkts(void *rx_queue, l234_info = rte_le_to_cpu_32(rxd.rx.l234_info); ol_info = rte_le_to_cpu_32(rxd.rx.ol_info); ret = hns3_handle_bdinfo(rxq, first_seg, bd_base_info, -l234_info, &cksum_err); +l234_info); if (unlikely(ret)) goto pkt_err; first_seg->packet_type = hns3_rx_calc_ptype(rxq, l234_info, ol_info); - if (bd_base_info & BIT(HNS3_RXD_L3L4P_B)) - hns3_rx_set_cksum_flag(first_seg, - first_seg->packet_type, - cksum_err); hns3_rxd_to_vlan_tci(rxq, first_seg, l234_info, &rxd); /* Increment bytes counter */ diff --git a/drivers/net/hns3/hns3_rxtx.h b/drivers/net/hns3/hns3_rxtx.h index 10a6c64..9dfae61 100644 --- a/drivers/net/hns3/hns3_rxtx.h +++ b/driver
[dpdk-dev] [PATCH 0/3] some features about hns3 PMD
This patch set includes 3 features: check max SIMD bitwidth. support Rx checksum simple process. support runtime config of mask device capability. Chengwen Feng (3): net/hns3: support runtime config of mask device capability net/hns3: support Rx checksum simple process net/hns3: check max SIMD bitwidth doc/guides/nics/hns3.rst | 9 +++ drivers/net/hns3/hns3_cmd.c | 67 ++ drivers/net/hns3/hns3_ethdev.c| 24 +++- drivers/net/hns3/hns3_ethdev.h| 4 ++ drivers/net/hns3/hns3_ethdev_vf.c | 3 +- drivers/net/hns3/hns3_rxtx.c | 19 +++ drivers/net/hns3/hns3_rxtx.h | 103 ++ drivers/net/hns3/hns3_rxtx_vec_neon.h | 7 +-- drivers/net/hns3/hns3_rxtx_vec_sve.c | 6 +- 9 files changed, 155 insertions(+), 87 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH 1/3] net/hns3: support runtime config of mask device capability
From: Chengwen Feng
This patch supports runtime config of mask device capability, it was
used to mask the capability which queried from firmware.
The device args key is "dev_caps_mask" which takes hexadecimal bitmask
where each bit represents whether mask corresponding capability.
Its main purpose is to debug and avoid problems.
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
doc/guides/nics/hns3.rst | 9 ++
drivers/net/hns3/hns3_cmd.c | 67 +++
drivers/net/hns3/hns3_ethdev.c| 24 +-
drivers/net/hns3/hns3_ethdev.h| 4 +++
drivers/net/hns3/hns3_ethdev_vf.c | 3 +-
5 files changed, 105 insertions(+), 2 deletions(-)
diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst
index 3366562..9592551 100644
--- a/doc/guides/nics/hns3.rst
+++ b/doc/guides/nics/hns3.rst
@@ -84,6 +84,15 @@ Runtime Config Options
be first checked, if meets, use the ``vec``. Then, ``simple``, at last
``common``.
+- ``dev_caps_mask`` (default ``0``)
+
+ Used to mask the capability which queried from firmware.
+ This args take hexadecimal bitmask where each bit represents whether mask
+ corresponding capability. eg. If the capability is 0x queried from
+ firmware, and the args value is 0xF which means the bit0~bit3 should be
+ masked off, then the capability will be 0xFFF0.
+ Its main purpose is to debug and avoid problems.
+
Driver compilation and testing
--
diff --git a/drivers/net/hns3/hns3_cmd.c b/drivers/net/hns3/hns3_cmd.c
index 75d5299..04e5786 100644
--- a/drivers/net/hns3/hns3_cmd.c
+++ b/drivers/net/hns3/hns3_cmd.c
@@ -411,6 +411,68 @@ hns3_cmd_send(struct hns3_hw *hw, struct hns3_cmd_desc
*desc, int num)
return retval;
}
+static const char *
+hns3_get_caps_name(uint32_t caps_id)
+{
+ const struct {
+ enum HNS3_CAPS_BITS caps;
+ const char *name;
+ } dev_caps[] = {
+ { HNS3_CAPS_UDP_GSO_B, "udp_gso" },
+ { HNS3_CAPS_ATR_B, "atr" },
+ { HNS3_CAPS_FD_QUEUE_REGION_B, "fd_queue_region" },
+ { HNS3_CAPS_PTP_B, "ptp" },
+ { HNS3_CAPS_INT_QL_B, "int_ql" },
+ { HNS3_CAPS_SIMPLE_BD_B, "simple_bd" },
+ { HNS3_CAPS_TX_PUSH_B, "tx_push" },
+ { HNS3_CAPS_PHY_IMP_B, "phy_imp" },
+ { HNS3_CAPS_TQP_TXRX_INDEP_B, "tqp_txrx_indep" },
+ { HNS3_CAPS_HW_PAD_B, "hw_pad" },
+ { HNS3_CAPS_STASH_B, "stash" },
+ { HNS3_CAPS_UDP_TUNNEL_CSUM_B, "udp_tunnel_csum" },
+ { HNS3_CAPS_RAS_IMP_B, "ras_imp" },
+ { HNS3_CAPS_FEC_B, "fec" },
+ { HNS3_CAPS_PAUSE_B, "pause" },
+ { HNS3_CAPS_RXD_ADV_LAYOUT_B, "rxd_adv_layout" }
+ };
+ uint32_t i;
+
+ for (i = 0; i < RTE_DIM(dev_caps); i++) {
+ if (dev_caps[i].caps == caps_id)
+ return dev_caps[i].name;
+ }
+
+ return "unknown";
+}
+
+static void
+hns3_mask_capability(struct hns3_hw *hw,
+struct hns3_query_version_cmd *cmd)
+{
+#define MAX_CAPS_BIT 64
+
+ struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
+ uint64_t caps_org, caps_new, caps_masked;
+ uint32_t i;
+
+ if (hns->dev_caps_mask == 0)
+ return;
+
+ memcpy(&caps_org, &cmd->caps[0], sizeof(caps_org));
+ caps_org = rte_le_to_cpu_64(caps_org);
+ caps_new = caps_org ^ (caps_org & hns->dev_caps_mask);
+ caps_masked = caps_org ^ caps_new;
+ caps_new = rte_cpu_to_le_64(caps_new);
+ memcpy(&cmd->caps[0], &caps_new, sizeof(caps_new));
+
+ for (i = 0; i < MAX_CAPS_BIT; i++) {
+ if (!(caps_masked & BIT_ULL(i)))
+ continue;
+ hns3_info(hw, "mask capabiliy: id-%u, name-%s.",
+ i, hns3_get_caps_name(i));
+ }
+}
+
static void
hns3_parse_capability(struct hns3_hw *hw,
struct hns3_query_version_cmd *cmd)
@@ -478,6 +540,11 @@ hns3_cmd_query_firmware_version_and_capability(struct
hns3_hw *hw)
return ret;
hw->fw_version = rte_le_to_cpu_32(resp->firmware);
+ /*
+* Make sure mask the capability before parse capability because it
+* may overwrite resp's data.
+*/
+ hns3_mask_capability(hw, resp);
hns3_parse_capability(hw, resp);
return 0;
d
[dpdk-dev] [PATCH 3/3] net/hns3: check max SIMD bitwidth
From: Chengwen Feng
This patch supports check max SIMD bitwidth when choosing NEON and SVE
vector path.
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_rxtx.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index c29c0cf..85ee67a 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -13,6 +13,7 @@
#include
#if defined(RTE_ARCH_ARM64)
#include
+#include
#endif
#include "hns3_ethdev.h"
@@ -2788,6 +2789,8 @@ static bool
hns3_get_default_vec_support(void)
{
#if defined(RTE_ARCH_ARM64)
+ if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_128)
+ return false;
if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON))
return true;
#endif
@@ -2798,6 +2801,8 @@ static bool
hns3_get_sve_support(void)
{
#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
+ if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
+ return false;
if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
return true;
#endif
--
2.7.4
[dpdk-dev] [PATCH] app/testpmd: add link autoneg status display
From: Huisong Li
This patch adds link autoneg status display in port_infos_display().
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
app/test-pmd/config.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 40b2b29..0dd6a6f 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -619,6 +619,8 @@ port_infos_display(portid_t port_id)
printf("Link speed: %s\n", rte_eth_link_speed_to_str(link.link_speed));
printf("Link duplex: %s\n", (link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
("full-duplex") : ("half-duplex"));
+ printf("Autoneg status: %s\n", (link.link_autoneg == ETH_LINK_AUTONEG) ?
+ ("On") : ("Off"));
if (!rte_eth_dev_get_mtu(port_id, &mtu))
printf("MTU: %u\n", mtu);
--
2.7.4
[dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info
From: Huisong Li
This patch supports the query of the link flow control parameter
on a port.
The command format is as follows:
show port flow_ctrl
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
app/test-pmd/cmdline.c | 83 +
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 +++
2 files changed, 90 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5bf1497..9fa85c4 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"show port (port_id) fec_mode"
" Show fec mode of a port.\n\n"
+
+ "show port flow_ctrl"
+ " Show flow control info of a port.\n\n"
);
}
@@ -6863,6 +6866,85 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
},
};
+/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
+struct cmd_link_flow_ctrl_show {
+ cmdline_fixed_string_t show;
+ cmdline_fixed_string_t port;
+ portid_t port_id;
+ cmdline_fixed_string_t flow_ctrl;
+};
+
+cmdline_parse_token_string_t cmd_lfc_show_show =
+ TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+ show, "show");
+cmdline_parse_token_string_t cmd_lfc_show_port =
+ TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+ port, "port");
+cmdline_parse_token_num_t cmd_lfc_show_portid =
+ TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
+ port_id, RTE_UINT16);
+cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
+ TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+ flow_ctrl, "flow_ctrl");
+
+static void
+cmd_link_flow_ctrl_show_parsed(void *parsed_result,
+ __rte_unused struct cmdline *cl,
+ __rte_unused void *data)
+{
+ struct cmd_link_flow_ctrl_show *res = parsed_result;
+ static const char *info_border = "*";
+ struct rte_eth_fc_conf fc_conf;
+ bool rx_fc_en = false;
+ bool tx_fc_en = false;
+ int ret;
+
+ if (!rte_eth_dev_is_valid_port(res->port_id)) {
+ printf("Invalid port id %u\n", res->port_id);
+ return;
+ }
+
+ ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
+ if (ret != 0) {
+ printf("Failed to get current flow ctrl information: err =
%d\n",
+ ret);
+ return;
+ }
+
+ if ((fc_conf.mode == RTE_FC_RX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
+ rx_fc_en = true;
+ if ((fc_conf.mode == RTE_FC_TX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
+ tx_fc_en = true;
+
+ printf("\n%s Flow control infos for port %-2d %s\n",
+ info_border, res->port_id, info_border);
+ printf("FC mode:\n");
+ printf(" Rx: %s\n", rx_fc_en ? "On" : "Off");
+ printf(" Tx: %s\n", tx_fc_en ? "On" : "Off");
+ printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
+ printf("pause_time: 0x%x\n", fc_conf.pause_time);
+ printf("high_water: 0x%x\n", fc_conf.high_water);
+ printf("low_water: 0x%x\n", fc_conf.low_water);
+ printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+ printf("mac ctrl frame fwd: %s\n",
+ fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
+ printf("\n%s** End ***%s\n",
+ info_border, info_border);
+}
+
+cmdline_parse_inst_t cmd_link_flow_control_show = {
+ .f = cmd_link_flow_ctrl_show_parsed,
+ .data = NULL,
+ .help_str = "show port flow_ctrl",
+ .tokens = {
+ (void *)&cmd_lfc_show_show,
+ (void *)&cmd_lfc_show_port,
+ (void *)&cmd_lfc_show_portid,
+ (void *)&cmd_lfc_show_flow_ctrl,
+ NULL,
+ },
+};
+
/* *** SETUP ETHERNET LINK FLOW CONTROL *** */
struct cmd_link_flow_ctrl_set_result {
cmdline_fixed_string_t set;
@@ -17001,6 +17083,7 @@ cmdline_parse_ctx_t main_ctx[] = {
(cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
(cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
(cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
+ (cmdline_parse_inst_t *)&cmd_link_flow_control_show,
(cmdline_parse_inst_t *)&cmd_priority_flow
[dpdk-dev] [PATCH] net/bonding: fix add bonded device itself as its slave
From: Chengchang Tang
Adding the bond device as its own slave should be forbidden. This
will cause a recursive endless loop in many subsequent operations,
and eventually lead to coredump.
This problem was found in testpmd, the related logs are as follows:
testpmd> create bonded device 1 0
Created new bonded device net_bonding_testpmd_0 on (port 4).
testpmd> add bonding slave 4 4
Segmentation fault (core dumped)
The call stack is as follows:
0x0064eb90 in rte_eth_dev_info_get ()
0x006df4b4 in bond_ethdev_info ()
0x0064eb90 in rte_eth_dev_info_get ()
0x006df4b4 in bond_ethdev_info ()
0x0064eb90 in rte_eth_dev_info_get ()
0x00564e58 in eth_dev_info_get_print_err ()
0x0055e8a4 in init_port_config ()
0x0052730c in cmd_add_bonding_slave_parsed ()
0x00646f60 in cmdline_parse ()
0x00645e08 in cmdline_valid_buffer ()
0x0064956c in rdline_char_in ()
0x00645ee0 in cmdline_in ()
0x006460a4 in cmdline_interact ()
0x00531904 in prompt ()
0x0051cca8 in main ()
Fixes: 2efb58cbab6e ("bond: new link bonding library")
Cc: sta...@dpdk.org
Signed-off-by: Chengchang Tang
Signed-off-by: Min Hu (Connor)
---
drivers/net/bonding/eth_bond_private.h | 2 +-
drivers/net/bonding/rte_eth_bond_api.c | 26 +-
2 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/net/bonding/eth_bond_private.h
b/drivers/net/bonding/eth_bond_private.h
index 75fb8dc..fc179a2 100644
--- a/drivers/net/bonding/eth_bond_private.h
+++ b/drivers/net/bonding/eth_bond_private.h
@@ -212,7 +212,7 @@ int
valid_bonded_port_id(uint16_t port_id);
int
-valid_slave_port_id(uint16_t port_id, uint8_t mode);
+valid_slave_port_id(struct bond_dev_private *internals, uint16_t port_id);
void
deactivate_slave(struct rte_eth_dev *eth_dev, uint16_t port_id);
diff --git a/drivers/net/bonding/rte_eth_bond_api.c
b/drivers/net/bonding/rte_eth_bond_api.c
index 17e6ff8..eb8d15d 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -56,19 +56,25 @@ check_for_master_bonded_ethdev(const struct rte_eth_dev
*eth_dev)
}
int
-valid_slave_port_id(uint16_t port_id, uint8_t mode)
+valid_slave_port_id(struct bond_dev_private *internals, uint16_t slave_port_id)
{
- RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(slave_port_id, -1);
- /* Verify that port_id refers to a non bonded port */
- if (check_for_bonded_ethdev(&rte_eth_devices[port_id]) == 0 &&
- mode == BONDING_MODE_8023AD) {
+ /* Verify that slave_port_id refers to a non bonded port */
+ if (check_for_bonded_ethdev(&rte_eth_devices[slave_port_id]) == 0 &&
+ internals->mode == BONDING_MODE_8023AD) {
RTE_BOND_LOG(ERR, "Cannot add slave to bonded device in 802.3ad"
" mode as slave is also a bonded device, only "
"physical devices can be support in this
mode.");
return -1;
}
+ if (internals->port_id == slave_port_id) {
+ RTE_BOND_LOG(ERR,
+ "Cannot add the bonded device itself as its slave.");
+ return -1;
+ }
+
return 0;
}
@@ -456,7 +462,7 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id,
uint16_t slave_port_id)
bonded_eth_dev = &rte_eth_devices[bonded_port_id];
internals = bonded_eth_dev->data->dev_private;
- if (valid_slave_port_id(slave_port_id, internals->mode) != 0)
+ if (valid_slave_port_id(internals, slave_port_id) != 0)
return -1;
slave_eth_dev = &rte_eth_devices[slave_port_id];
@@ -605,13 +611,15 @@ rte_eth_bond_slave_add(uint16_t bonded_port_id, uint16_t
slave_port_id)
int retval;
- /* Verify that port id's are valid bonded and slave ports */
if (valid_bonded_port_id(bonded_port_id) != 0)
return -1;
bonded_eth_dev = &rte_eth_devices[bonded_port_id];
internals = bonded_eth_dev->data->dev_private;
+ if (valid_slave_port_id(internals, slave_port_id) != 0)
+ return -1;
+
rte_spinlock_lock(&internals->lock);
retval = __eth_bond_slave_add_lock_free(bonded_port_id, slave_port_id);
@@ -635,7 +643,7 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
bonded_eth_dev = &rte_eth_devices[bonded_port_id];
internals = bonded_eth_dev->data->dev_private;
- if (valid_slave_port_id(slave_port_id, internals->mode) < 0)
+ if (valid_slave_port_id(internals, slave_port_id) < 0)
return -1;
/* first remove from active slave list */
@@ -783,7 +791,7 @@ rte_eth_bond_pri
[dpdk-dev] [PATCH] examples/timer: fix incorrect time interval
From: Chengchang Tang
Timer sample example assumes that the frequency of the timer is about
2Ghz to control the period of calling rte_timer_manage(). But this
assumption is easy to fail. For example. the frequency of tsc on ARM64
is much less than 2Ghz.
This patch uses the frequency of the current timer to calculate the
correct time interval to ensure consistent result on all platforms.
In addition, the rte_rdtsc() is replaced with the more recommended
rte_get_timer_cycles function in this patch.
Fixes: af75078fece3 ("first public release")
Cc: sta...@dpdk.org
Signed-off-by: Chengchang Tang
Signed-off-by: Min Hu (Connor)
---
examples/timer/main.c | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/examples/timer/main.c b/examples/timer/main.c
index 5a57e48..05f4a9f 100644
--- a/examples/timer/main.c
+++ b/examples/timer/main.c
@@ -18,8 +18,7 @@
#include
#include
-#define TIMER_RESOLUTION_CYCLES 2000ULL /* around 10ms at 2 Ghz */
-
+static uint64_t timer_resolution_cycles;
static struct rte_timer timer0;
static struct rte_timer timer1;
@@ -66,15 +65,16 @@ lcore_mainloop(__rte_unused void *arg)
while (1) {
/*
-* Call the timer handler on each core: as we don't
-* need a very precise timer, so only call
-* rte_timer_manage() every ~10ms (at 2Ghz). In a real
-* application, this will enhance performances as
-* reading the HPET timer is not efficient.
+* Call the timer handler on each core: as we don't need a
+* very precise timer, so only call rte_timer_manage()
+* every ~10ms. since rte_eal_hpet_init() has not been
+* called, the rte_rdtsc() will be used at runtime.
+* In a real application, this will enhance performances
+* as reading the HPET timer is not efficient.
*/
- cur_tsc = rte_rdtsc();
+ cur_tsc = rte_get_timer_cycles();
diff_tsc = cur_tsc - prev_tsc;
- if (diff_tsc > TIMER_RESOLUTION_CYCLES) {
+ if (diff_tsc > timer_resolution_cycles) {
rte_timer_manage();
prev_tsc = cur_tsc;
}
@@ -100,8 +100,10 @@ main(int argc, char **argv)
rte_timer_init(&timer0);
rte_timer_init(&timer1);
- /* load timer0, every second, on main lcore, reloaded automatically */
hz = rte_get_timer_hz();
+ timer_resolution_cycles = hz * 10 / 1000; /* around 10ms */
+
+ /* load timer0, every second, on main lcore, reloaded automatically */
lcore_id = rte_lcore_id();
rte_timer_reset(&timer0, hz, PERIODICAL, lcore_id, timer0_cb, NULL);
--
2.7.4
[dpdk-dev] [PATCH v3] net/hns3: rename Rx burst API
From: Chengwen Feng
Currently, user could use runtime config "rx_func_hint=simple" to
select the hns3_recv_pkts API, but the API's name get from
rte_eth_rx_burst_mode_get is "Scalar" which has not reflected "simple".
So this patch renames hns3_recv_pkts to hns3_recv_pkts_simple, and
also change it's name which gets from rte_eth_rx_burst_mode_get to
"Scalar Simple" to maintain conceptual consistency.
Fixes: 521ab3e93361 ("net/hns3: add simple Rx path")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
Acked-by: Ferruh Yigit
---
v3:
* Modify commit info.
v2:
* Revert "Scalar Scattered".
---
drivers/net/hns3/hns3_rxtx.c | 18 ++
drivers/net/hns3/hns3_rxtx.h | 4 ++--
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index ce5d852..bc107d0 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -1991,7 +1991,7 @@ hns3_dev_supported_ptypes_get(struct rte_eth_dev *dev)
RTE_PTYPE_UNKNOWN
};
- if (dev->rx_pkt_burst == hns3_recv_pkts ||
+ if (dev->rx_pkt_burst == hns3_recv_pkts_simple ||
dev->rx_pkt_burst == hns3_recv_scattered_pkts ||
dev->rx_pkt_burst == hns3_recv_pkts_vec ||
dev->rx_pkt_burst == hns3_recv_pkts_vec_sve)
@@ -2366,7 +2366,9 @@ hns3_rx_alloc_buffer(struct hns3_rx_queue *rxq)
}
uint16_t
-hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
+hns3_recv_pkts_simple(void *rx_queue,
+ struct rte_mbuf **rx_pkts,
+ uint16_t nb_pkts)
{
volatile struct hns3_desc *rx_ring; /* RX ring (desc) */
volatile struct hns3_desc *rxdp; /* pointer of the current desc */
@@ -2742,10 +2744,10 @@ hns3_rx_burst_mode_get(struct rte_eth_dev *dev,
__rte_unused uint16_t queue_id,
eth_rx_burst_t pkt_burst;
const char *info;
} burst_infos[] = {
- { hns3_recv_pkts, "Scalar" },
+ { hns3_recv_pkts_simple,"Scalar Simple" },
{ hns3_recv_scattered_pkts, "Scalar Scattered" },
- { hns3_recv_pkts_vec, "Vector Neon" },
- { hns3_recv_pkts_vec_sve, "Vector Sve" },
+ { hns3_recv_pkts_vec, "Vector Neon" },
+ { hns3_recv_pkts_vec_sve, "Vector Sve"},
};
eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
@@ -2792,14 +2794,14 @@ hns3_get_rx_function(struct rte_eth_dev *dev)
if (hns->rx_func_hint == HNS3_IO_FUNC_HINT_SVE && sve_allowed)
return hns3_recv_pkts_vec_sve;
if (hns->rx_func_hint == HNS3_IO_FUNC_HINT_SIMPLE && simple_allowed)
- return hns3_recv_pkts;
+ return hns3_recv_pkts_simple;
if (hns->rx_func_hint == HNS3_IO_FUNC_HINT_COMMON)
return hns3_recv_scattered_pkts;
if (vec_allowed)
return hns3_recv_pkts_vec;
if (simple_allowed)
- return hns3_recv_pkts;
+ return hns3_recv_pkts_simple;
return hns3_recv_scattered_pkts;
}
@@ -4433,7 +4435,7 @@ hns3_dev_rx_descriptor_status(void *rx_queue, uint16_t
offset)
rxdp = &rxq->rx_ring[desc_id];
bd_base_info = rte_le_to_cpu_32(rxdp->rx.bd_base_info);
dev = &rte_eth_devices[rxq->port_id];
- if (dev->rx_pkt_burst == hns3_recv_pkts ||
+ if (dev->rx_pkt_burst == hns3_recv_pkts_simple ||
dev->rx_pkt_burst == hns3_recv_scattered_pkts) {
if (offset >= rxq->nb_rx_desc - rxq->rx_free_hold)
return RTE_ETH_RX_DESC_UNAVAIL;
diff --git a/drivers/net/hns3/hns3_rxtx.h b/drivers/net/hns3/hns3_rxtx.h
index 6689397..d4ac12e 100644
--- a/drivers/net/hns3/hns3_rxtx.h
+++ b/drivers/net/hns3/hns3_rxtx.h
@@ -680,8 +680,8 @@ int hns3_dev_rx_queue_start(struct rte_eth_dev *dev,
uint16_t rx_queue_id);
int hns3_dev_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id);
int hns3_dev_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id);
int hns3_dev_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id);
-uint16_t hns3_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
- uint16_t nb_pkts);
+uint16_t hns3_recv_pkts_simple(void *rx_queue, struct rte_mbuf **rx_pkts,
+ uint16_t nb_pkts);
uint16_t hns3_recv_scattered_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
uint16_t nb_pkts);
uint16_t hns3_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
--
2.7.4
[dpdk-dev] [PATCH v5] ethdev: add sanity checks in control APIs
This patch adds more sanity checks in control path APIs.
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
Reviewed-by: Andrew Rybchenko
---
v5:
* Keep "RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV)"
and "dev = &rte_eth_devices[port_id]" together.
v4:
* add error logging.
* delete check in control path API.
v3:
* set port_id checked first.
* add error logging.
v2:
* Removed unnecessary checks.
* Deleted checks in internal API.
* Added documentation in the header file.
---
lib/librte_ethdev/rte_ethdev.c | 313 -
lib/librte_ethdev/rte_ethdev.h | 20 ++-
2 files changed, 293 insertions(+), 40 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 3059aa5..95115a5 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -199,6 +199,16 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
char *cls_str = NULL;
int str_size;
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterator init for NULL\n");
+ return -EINVAL;
+ }
+
+ if (devargs_str == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterate matching NULL\n");
+ return -EINVAL;
+ }
+
memset(iter, 0, sizeof(*iter));
/*
@@ -293,6 +303,11 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
uint16_t
rte_eth_iterator_next(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterate next for NULL\n");
+ return RTE_MAX_ETHPORTS;
+ }
+
if (iter->cls == NULL) /* invalid ethdev iterator */
return RTE_MAX_ETHPORTS;
@@ -322,6 +337,11 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
void
rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterator clear up for NULL\n");
+ return;
+ }
+
if (iter->bus_str == NULL)
return; /* nothing to free in pure class filter */
free(RTE_CAST_FIELD(iter, bus_str, char *)); /* workaround const */
@@ -622,6 +642,11 @@ rte_eth_find_next_owned_by(uint16_t port_id, const
uint64_t owner_id)
int
rte_eth_dev_owner_new(uint64_t *owner_id)
{
+ if (owner_id == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to get owner id by NULL\n");
+ return -EINVAL;
+ }
+
eth_dev_shared_data_prepare();
rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
@@ -645,6 +670,12 @@ eth_dev_owner_set(const uint16_t port_id, const uint64_t
old_owner_id,
return -ENODEV;
}
+ if (new_owner == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to set ethdev port %u owner to
NULL\n",
+ port_id);
+ return -EINVAL;
+ }
+
if (!eth_is_valid_owner_id(new_owner->id) &&
!eth_is_valid_owner_id(old_owner_id)) {
RTE_ETHDEV_LOG(ERR,
@@ -738,23 +769,30 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
int
rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
{
- int ret = 0;
- struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
-
- eth_dev_shared_data_prepare();
+ struct rte_eth_dev *ethdev;
- rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
- if (port_id >= RTE_MAX_ETHPORTS || !eth_dev_is_allocated(ethdev)) {
+ ethdev = &rte_eth_devices[port_id];
+ if (!eth_dev_is_allocated(ethdev)) {
RTE_ETHDEV_LOG(ERR, "Port id %"PRIu16" is not allocated\n",
port_id);
- ret = -ENODEV;
- } else {
- rte_memcpy(owner, ðdev->data->owner, sizeof(*owner));
+ return -ENODEV;
}
+ if (owner == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u owner by
NULL\n",
+ port_id);
+ return -EINVAL;
+ }
+
+ eth_dev_shared_data_prepare();
+
+ rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
+ rte_memcpy(owner, ðdev->data->owner, sizeof(*owner));
rte_spinlock_
Re: [dpdk-dev] [PATCH v4] ethdev: add sanity checks in control APIs
在 2021/4/15 16:15, Andrew Rybchenko 写道:
On 4/15/21 3:52 AM, Min Hu (Connor) wrote:
This patch adds more sanity checks in control path APIs.
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Many thanks. I'll keep log messages gramma review to native
speekers. Content looks good to me.
One minor issue below lost on previous review.
Other than that,
Reviewed-by: Andrew Rybchenko
Signed-off-by: Min Hu (Connor)
[snip]
@@ -1299,6 +1337,12 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t
nb_rx_q, uint16_t nb_tx_q,
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ if (dev_conf == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to configure ethdev port %u to
NULL\n",
+ port_id);
+ return -EINVAL;
+ }
+
dev = &rte_eth_devices[port_id];
I think it is better to keep:
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
dev = &rte_eth_devices[port_id];
together since they are tightly related. I.e. I'd even remove
empty line between them when it is present and add args
sanity check after the dev assignment.
>
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -ENOTSUP);
Thanks Andrew, this has been fixed in v5.
In theory, the first argument is sufficient to make the ops
check, but I think it is the right solution to keep it as is
since current tendency is to check operation support when
driver callback is really required and we're going to use it.
However, if we do it just after port_id check, we'll have a
way to check for callback support without any side effects
if we provide invalid argument value. I.e. if -ENOTSUP is
returned, callback is not supported, if -EINVAL, callback is
supported (but argument is invalid and nothing done).
However, it looks a bit fragile and not always possible.
Thoughts on it are welcome.
.
[dpdk-dev] [PATCH v3 0/2] fix missing check for thread creation
There exist some thread creation function without result check. This set of patches add result check and message print out after failure. Chengwen Feng (2): telemetry: fix missing check for thread creation test: fix missing check for thread creation --- v3: * modify return value check and error logging. v2: * fix compiling bugs of missig value. app/test/process.h | 8 ++-- lib/librte_telemetry/telemetry.c| 30 +++--- lib/librte_telemetry/telemetry_legacy.c | 10 -- 3 files changed, 41 insertions(+), 7 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH v3 2/2] test: fix missing check for thread creation
From: Chengwen Feng
There was a call for thread create function without result check.
Add result check and message print out after failure.
Fixes: 086eb64db39e ("test/pdump: add unit test for pdump library")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
app/test/process.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/app/test/process.h b/app/test/process.h
index 27f1b1c..2a0a8da 100644
--- a/app/test/process.h
+++ b/app/test/process.h
@@ -48,6 +48,7 @@ process_dup(const char *const argv[], int numargs, const char
*env_value)
#ifdef RTE_LIB_PDUMP
#ifdef RTE_NET_RING
pthread_t thread;
+ int rc;
#endif
#endif
@@ -126,8 +127,11 @@ process_dup(const char *const argv[], int numargs, const
char *env_value)
/* parent process does a wait */
#ifdef RTE_LIB_PDUMP
#ifdef RTE_NET_RING
- if ((strcmp(env_value, "run_pdump_server_tests") == 0))
- pthread_create(&thread, NULL, &send_pkts, NULL);
+ if ((strcmp(env_value, "run_pdump_server_tests") == 0)) {
+ rc = pthread_create(&thread, NULL, &send_pkts, NULL);
+ if (rc != 0)
+ rte_panic("Cannot start send pkts thread\n");
+ }
#endif
#endif
--
2.7.4
[dpdk-dev] [PATCH v3 1/2] telemetry: fix missing check for thread creation
From: Chengwen Feng
Add result check and message print out for thread creation after
failure.
Fixes: b80fe1805eee ("telemetry: introduce backward compatibility")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
lib/librte_telemetry/telemetry.c| 30 +++---
lib/librte_telemetry/telemetry_legacy.c | 10 --
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index 7e08afd..e6a99f3 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -350,6 +350,7 @@ socket_listener(void *socket)
{
while (1) {
pthread_t th;
+ int rc;
struct socket *s = (struct socket *)socket;
int s_accepted = accept(s->sock, NULL, NULL);
if (s_accepted < 0) {
@@ -366,7 +367,12 @@ socket_listener(void *socket)
__atomic_add_fetch(s->num_clients, 1,
__ATOMIC_RELAXED);
}
- pthread_create(&th, NULL, s->fn, (void *)(uintptr_t)s_accepted);
+ rc = pthread_create(&th, NULL, s->fn, (void
*)(uintptr_t)s_accepted);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create client thread\n");
+ close(s_accepted);
+ return NULL;
+ }
pthread_detach(th);
}
return NULL;
@@ -425,6 +431,7 @@ static int
telemetry_legacy_init(void)
{
pthread_t t_old;
+ int rc;
if (num_legacy_callbacks == 1) {
TMTY_LOG(WARNING, "No legacy callbacks, legacy socket not
created\n");
@@ -440,7 +447,15 @@ telemetry_legacy_init(void)
v1_socket.sock = create_socket(v1_socket.path);
if (v1_socket.sock < 0)
return -1;
- pthread_create(&t_old, NULL, socket_listener, &v1_socket);
+ rc = pthread_create(&t_old, NULL, socket_listener, &v1_socket);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create thread for legacy socket\n");
+ close(v1_socket.sock);
+ v1_socket.sock = -1;
+ unlink(v1_socket.path);
+ v1_socket.path[0] = '\0';
+ return -1;
+ }
pthread_setaffinity_np(t_old, sizeof(*thread_cpuset), thread_cpuset);
TMTY_LOG(DEBUG, "Legacy telemetry socket initialized ok\n");
@@ -451,6 +466,7 @@ static int
telemetry_v2_init(void)
{
pthread_t t_new;
+ int rc;
v2_socket.num_clients = &v2_clients;
rte_telemetry_register_cmd("/", list_commands,
@@ -469,7 +485,15 @@ telemetry_v2_init(void)
v2_socket.sock = create_socket(v2_socket.path);
if (v2_socket.sock < 0)
return -1;
- pthread_create(&t_new, NULL, socket_listener, &v2_socket);
+ rc = pthread_create(&t_new, NULL, socket_listener, &v2_socket);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create thread for socket");
+ close(v2_socket.sock);
+ v2_socket.sock = -1;
+ unlink(v2_socket.path);
+ v2_socket.path[0] = '\0';
+ return -1;
+ }
pthread_setaffinity_np(t_new, sizeof(*thread_cpuset), thread_cpuset);
atexit(unlink_sockets);
diff --git a/lib/librte_telemetry/telemetry_legacy.c
b/lib/librte_telemetry/telemetry_legacy.c
index 5e9af37..fd242bf 100644
--- a/lib/librte_telemetry/telemetry_legacy.c
+++ b/lib/librte_telemetry/telemetry_legacy.c
@@ -83,6 +83,7 @@ register_client(const char *cmd __rte_unused, const char
*params,
pthread_t th;
char data[BUF_SIZE];
int fd;
+ int rc;
struct sockaddr_un addrs;
#endif /* !RTE_EXEC_ENV_WINDOWS */
@@ -112,8 +113,13 @@ register_client(const char *cmd __rte_unused, const char
*params,
close(fd);
return -1;
}
- pthread_create(&th, NULL, &legacy_client_handler,
- (void *)(uintptr_t)fd);
+ rc = pthread_create(&th, NULL, &legacy_client_handler,
+ (void *)(uintptr_t)fd);
+ if (rc != 0) {
+ perror("Failed to create legacy client thread\n");
+ close(fd);
+ return -1;
+ }
#endif /* !RTE_EXEC_ENV_WINDOWS */
return 0;
}
--
2.7.4
Re: [dpdk-dev] [PATCH v2 1/2] telemetry: fix missing check for thread creation
Hi, David,
ALl has been fixed in v3, please check it out, thanks.
在 2021/4/12 15:48, David Marchand 写道:
On Mon, Apr 12, 2021 at 2:32 AM Min Hu (Connor) wrote:
From: Chengwen Feng
Add result check and message print out for thread creation after
failure.
Ah, I was looking at this code too, while looking at your other series :-).
Fixes: b80fe1805eee ("telemetry: introduce backward compatibility")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
lib/librte_telemetry/telemetry.c| 28 +---
lib/librte_telemetry/telemetry_legacy.c | 10 --
2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index 7e08afd..3f1a4c4 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -350,6 +350,7 @@ socket_listener(void *socket)
{
while (1) {
pthread_t th;
+ int rc;
struct socket *s = (struct socket *)socket;
int s_accepted = accept(s->sock, NULL, NULL);
if (s_accepted < 0) {
@@ -366,7 +367,14 @@ socket_listener(void *socket)
__atomic_add_fetch(s->num_clients, 1,
__ATOMIC_RELAXED);
}
- pthread_create(&th, NULL, s->fn, (void *)(uintptr_t)s_accepted);
+ rc = pthread_create(&th, NULL, s->fn,
+ (void *)(uintptr_t)s_accepted);
+ if (rc) {
Either you use this rc variable (adding strerror(rc) in the log
message below) or you can remove it.
This comment applies to others updates below.
+ TMTY_LOG(ERR, "Error with create thread, telemetry thread
quitting\n");
+ close(s_accepted);
+ return NULL;
+ }
+
pthread_detach(th);
}
return NULL;
@@ -425,6 +433,7 @@ static int
telemetry_legacy_init(void)
{
pthread_t t_old;
+ int rc;
if (num_legacy_callbacks == 1) {
TMTY_LOG(WARNING, "No legacy callbacks, legacy socket not
created\n");
@@ -440,7 +449,13 @@ Fixes: 3b3757bda3c3 ("net/ice: get VF hardware index in
DCF")
(void)
v1_socket.sock = create_socket(v1_socket.path);
if (v1_socket.sock < 0)
return -1;
- pthread_create(&t_old, NULL, socket_listener, &v1_socket);
+ rc = pthread_create(&t_old, NULL, socket_listener, &v1_socket);
+ if (rc) {
+ TMTY_LOG(ERR, "Error with create thread\n");
+ unlink_sockets();
Before this patch, v2 mode could be working fine with v1 ko.
I think it was intended since telemetry_legacy_init return value is
not checked and unlink_sockets() checks whether the v1 and v2 sockets
have been created.
Plus, in this error handling, the v1 socket fd is left open.
How about:
+ ret = pthread_create(&t_old, NULL, socket_listener, &v1_socket);
+ if (ret != 0) {
+ TMTY_LOG(DEBUG, "Failed to create handler for legacy
socket: %s\n",
+ strerror(ret));
+ close(v1_socket.sock);
+ v1_socket.sock = -1;
+ if (v1_socket.path[0]) {
+ unlink(v1_socket.path);
+ v1_socket.path[0] = '\0';
+ }
+ return -1;
+ }
ok, but there is no need to judge v1_socket.path[0] because previous
setup will ensure it's validation.
+ return -1;
+ }
+
pthread_setaffinity_np(t_old, sizeof(*thread_cpuset), thread_cpuset);
TMTY_LOG(DEBUG, "Legacy telemetry socket initialized ok\n");
@@ -451,6 +466,7 @@ static int
telemetry_v2_init(void)
{
pthread_t t_new;
+ int rc;
v2_socket.num_clients = &v2_clients;
rte_telemetry_register_cmd("/", list_commands,
@@ -469,7 +485,13 @@ telemetry_v2_init(void)
v2_socket.sock = create_socket(v2_socket.path);
if (v2_socket.sock < 0)
return -1;
- pthread_create(&t_new, NULL, socket_listener, &v2_socket);
+ rc = pthread_create(&t_new, NULL, socket_listener, &v2_socket);
+ if (rc) {
+ TMTY_LOG(ERR, "Error with create thread\n");
+ unlink_sockets();
+ return -1;
+ }
+
Missing a close on v2 socket,
pthread_setaffinity_np(t_new, sizeof(*thread_cpuset), thread_cpuset);
atexit(unlink_sockets);
diff --git a/lib/librte_telemetry/telemetry_legacy.c
b/lib/librte_telemetry/telemetry_legacy.c
index 5e9af37..9587bfe 100644
--- a/lib/librte_telemetry/telemetry_legacy.c
+++ b/lib/librte_telemetry/telemetry_
[dpdk-dev] [PATCH v9] app/testpmd: support multi-process
This patch adds multi-process support for testpmd.
The test cmd example as follows:
the primary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=0
the secondary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=1
Signed-off-by: Min Hu (Connor)
Signed-off-by: Lijun Ou
Acked-by: Xiaoyun Li
Acked-by: Ajit Khaparde
---
v9:
* Updated release notes and rst doc.
* Deleted deprecated codes.
* move macro and variable.
v8:
* Added warning info about queue numbers and process numbers.
v7:
* Fixed compiling error for unexpected unindent.
v6:
* Add rte flow description for multiple process.
v5:
* Fixed run_app.rst for multiple process description.
* Fix compiling error.
v4:
* Fixed minimum vlaue of Rxq or Txq in doc.
v3:
* Fixed compiling error using gcc10.0.
v2:
* Added document for this patch.
---
app/test-pmd/cmdline.c | 6 ++
app/test-pmd/config.c | 14 -
app/test-pmd/parameters.c | 12
app/test-pmd/testpmd.c | 108 +++--
app/test-pmd/testpmd.h | 3 +
doc/guides/rel_notes/release_21_05.rst | 1 +
doc/guides/testpmd_app_ug/run_app.rst | 86 ++
7 files changed, 196 insertions(+), 34 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5bf1497..e465824 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -5354,6 +5354,12 @@ cmd_set_flush_rx_parsed(void *parsed_result,
__rte_unused void *data)
{
struct cmd_set_flush_rx *res = parsed_result;
+
+ if (num_procs > 1 && (strcmp(res->mode, "on") == 0)) {
+ printf("multi-process doesn't support to flush rx queues.\n");
+ return;
+ }
+
no_flush_rx = (uint8_t)((strcmp(res->mode, "on") == 0) ? 0 : 1);
}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 40b2b29..c982c87 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2860,6 +2860,8 @@ rss_fwd_config_setup(void)
queueid_t rxq;
queueid_t nb_q;
streamid_t sm_id;
+ int start;
+ int end;
nb_q = nb_rxq;
if (nb_q > nb_txq)
@@ -2877,7 +2879,15 @@ rss_fwd_config_setup(void)
init_fwd_streams();
setup_fwd_config_of_each_lcore(&cur_fwd_config);
- rxp = 0; rxq = 0;
+
+ if (proc_id > 0 && nb_q % num_procs)
+ printf("Warning! queue numbers should be multiple of "
+ "processes, or packet loss will happen.\n");
+
+ start = proc_id * nb_q / num_procs;
+ end = start + nb_q / num_procs;
+ rxp = 0;
+ rxq = start;
for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) {
struct fwd_stream *fs;
@@ -2894,6 +2904,8 @@ rss_fwd_config_setup(void)
continue;
rxp = 0;
rxq++;
+ if (rxq >= end)
+ rxq = start;
}
}
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index f3954c1..d86cc21 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -508,6 +508,9 @@ parse_link_speed(int n)
void
launch_args_parse(int argc, char** argv)
{
+#define PARAM_PROC_ID "proc-id"
+#define PARAM_NUM_PROCS "num-procs"
+
int n, opt;
char **argvopt;
int opt_idx;
@@ -625,6 +628,8 @@ launch_args_parse(int argc, char** argv)
{ "rx-mq-mode", 1, 0, 0 },
{ "record-core-cycles", 0, 0, 0 },
{ "record-burst-stats", 0, 0, 0 },
+ { PARAM_NUM_PROCS, 1, 0, 0 },
+ { PARAM_PROC_ID,1, 0, 0 },
{ 0, 0, 0, 0 },
};
@@ -1391,6 +1396,13 @@ launch_args_parse(int argc, char** argv)
record_core_cycles = 1;
if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
record_burst_stats = 1;
+
+ if (strncmp(lgopts[opt_idx].name,
+ PARAM_NUM_PROCS, 9) == 0)
+ num_procs = atoi(optarg);
+ if (strncmp(lgopts[opt_idx].name,
+ PARAM_PROC_ID, 7) == 0)
+ proc_id = atoi(optarg);
break;
case 'h':
usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 96d2e0f..01d0d82 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -518,6 +518,16 @@ enum rte_eth_rx_mq_mode rx_mq_mode =
ETH_MQ_R
Re: [dpdk-dev] [PATCH v8] app/testpmd: support multi-process
在 2021/4/15 15:54, Ferruh Yigit 写道: On 4/12/2021 5:37 PM, Ferruh Yigit wrote: On 3/30/2021 2:48 AM, Min Hu (Connor) wrote: From: Lijun Ou This patch adds multi-process support for testpmd. The test cmd example as follows: the primary cmd: ./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \ --rxq=4 --txq=4 --num-procs=2 --proc-id=0 the secondary cmd: ./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \ --rxq=4 --txq=4 --num-procs=2 --proc-id=1 Hi Connor, Please find a few minor comments below, since they are minor issues please feel free to keep the existing acks in next version. Reminder of this patch, waiting for minor updates, it would be good to have it before -rc1, so that the change can be tested thoroughly. Hi, Ferruh, v9 has been sent, please check it out, thanks. Meanwhile I have concern/question on long term testing of the feature. It is possible to break the multi-process support unintentionally and not recognize it. How can we continually verify the feature? Are you using DTS? Does it make sense to add a testcase into dts to test testpmd multi-process support so that it is verified on each release? . Well, we have our own DTS system which contains testcase of multiple process in testpmd: 1. When we upstream patches to DPDK，the patch should pass all the testcase. 2. Before DPDK version release(in RC1) in every three months, we rebase the new DPDK version, and check all the testcase in our DTS system. If some testcase fails, we will fix it and upstream the patch to DPDK, of course, including multi-process support in testpmd.
[dpdk-dev] [PATCH v6] ethdev: add sanity checks in control APIs
This patch adds more sanity checks in control path APIs.
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
Reviewed-by: Andrew Rybchenko
---
v6:
* Change logging grammar.
* "Failed to" change to "Cannot".
* Break the message part to next line.
* Delete param check for internal function.
v5:
* Keep "RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV)"
and "dev = &rte_eth_devices[port_id]" together.
v4:
* add error logging.
* delete check in control path API.
v3:
* set port_id checked first.
* add error logging.
v2:
* Removed unnecessary checks.
* Deleted checks in internal API.
* Added documentation in the header file.
---
lib/librte_ethdev/rte_ethdev.c | 365 +
lib/librte_ethdev/rte_ethdev.h | 20 ++-
2 files changed, 349 insertions(+), 36 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 6b5cfd6..9ddd5d7 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -199,6 +199,16 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
char *cls_str = NULL;
int str_size;
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot init iterator for NULL iterator\n");
+ return -EINVAL;
+ }
+
+ if (devargs_str == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot init iterator for NULL devargs\n");
+ return -EINVAL;
+ }
+
memset(iter, 0, sizeof(*iter));
/*
@@ -293,6 +303,11 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
uint16_t
rte_eth_iterator_next(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot iterate next for NULL\n");
+ return RTE_MAX_ETHPORTS;
+ }
+
if (iter->cls == NULL) /* invalid ethdev iterator */
return RTE_MAX_ETHPORTS;
@@ -322,6 +337,11 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
void
rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot iterator clear up for NULL iter\n");
+ return;
+ }
+
if (iter->bus_str == NULL)
return; /* nothing to free in pure class filter */
free(RTE_CAST_FIELD(iter, bus_str, char *)); /* workaround const */
@@ -622,6 +642,11 @@ rte_eth_find_next_owned_by(uint16_t port_id, const
uint64_t owner_id)
int
rte_eth_dev_owner_new(uint64_t *owner_id)
{
+ if (owner_id == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot get owner id for NULL param\n");
+ return -EINVAL;
+ }
+
eth_dev_shared_data_prepare();
rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
@@ -645,6 +670,13 @@ eth_dev_owner_set(const uint16_t port_id, const uint64_t
old_owner_id,
return -ENODEV;
}
+ if (new_owner == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot set ethdev port %u owner to NULL new_owner\n",
+ port_id);
+ return -EINVAL;
+ }
+
if (!eth_is_valid_owner_id(new_owner->id) &&
!eth_is_valid_owner_id(old_owner_id)) {
RTE_ETHDEV_LOG(ERR,
@@ -738,23 +770,30 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
int
rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
{
- int ret = 0;
- struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
-
- eth_dev_shared_data_prepare();
-
- rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
+ struct rte_eth_dev *ethdev;
- if (port_id >= RTE_MAX_ETHPORTS || !eth_dev_is_allocated(ethdev)) {
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ ethdev = &rte_eth_devices[port_id];
+ if (!eth_dev_is_allocated(ethdev)) {
RTE_ETHDEV_LOG(ERR, "Port id %"PRIu16" is not allocated\n",
port_id);
- ret = -ENODEV;
- } else {
- rte_memcpy(owner, ðdev->data->owner, sizeof(*owner));
+ return -ENODEV;
}
+ if (owner == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot get ethdev port %"PRIu16" owner for NULL
param\n&qu
Re: [dpdk-dev] [PATCH v4] ethdev: add sanity checks in control APIs
Thanks Kevin,
all is fixed in v6, please review it, thanks.
Some comments are below.
在 2021/4/15 20:04, Kevin Traynor 写道:
On 15/04/2021 01:52, Min Hu (Connor) wrote:
This patch adds more sanity checks in control path APIs.
Hi Connor,
A few general comments,
--
Some of the functions have unit tests, you could consider adding unit
tests for the new checks. Considering the checks are not subtle and
unlikely to be messed up in future, not adding unit tests is not a
blocker imho.
--
It took me a while to get what you meant with "by NULL". It's usage
seems like in "Death by taxes". Perhaps "because NULL ptr" would be a
better way to phrase this generically, but I think it is more useful to
say what is NULL.
e.g. "Failed to convert NULL to string\n" is very generic and would be
better as "Failed to convert NULL link to string\n" . ok, still a bit
generic but more of a clue.
I won't comment on each log message individually but I've added a few
suggestions here and there.
--
Did you check the usage of these functions in DPDK, and if the return
value is handled ok? e.g. RTE_ETH_FOREACH_MATCHING_DEV will keep calling
iterator functions. I'm not sure that having a return check is needed in
that case, but there could be other cases where you want to take some
different action now.
As iterator functions are all APIs, they may be used by APP directly.
I think param check is necessary.
some other comments inlined,
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
v4:
* add error logging.
* delete check in control path API.
v3:
* set port_id checked first.
* add error logging.
v2:
* Removed unnecessary checks.
* Deleted checks in internal API.
* Added documentation in the header file.
---
lib/librte_ethdev/rte_ethdev.c | 288 ++---
lib/librte_ethdev/rte_ethdev.h | 20 ++-
2 files changed, 287 insertions(+), 21 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 6b5cfd6..e734a30 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -199,6 +199,16 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
char *cls_str = NULL;
int str_size;
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterator init for NULL\n");
"Failed to init iterator for NULL iterator\n"
+ return -EINVAL;
+ }
+
+ if (devargs_str == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterate matching NULL\n");
"Failed to init iterator for NULL devargs\n"
+ return -EINVAL;
+ }
+
memset(iter, 0, sizeof(*iter));
/*
@@ -293,6 +303,11 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
uint16_t
rte_eth_iterator_next(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterate next for NULL\n");
+ return RTE_MAX_ETHPORTS;
+ }
+
if (iter->cls == NULL) /* invalid ethdev iterator */
return RTE_MAX_ETHPORTS;
@@ -322,6 +337,11 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
void
rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterator clear up for NULL\n");
+ return;
+ }
+
if (iter->bus_str == NULL)
return; /* nothing to free in pure class filter */
free(RTE_CAST_FIELD(iter, bus_str, char *)); /* workaround const */
@@ -622,6 +642,11 @@ rte_eth_find_next_owned_by(uint16_t port_id, const
uint64_t owner_id)
int
rte_eth_dev_owner_new(uint64_t *owner_id)
{
+ if (owner_id == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to get owner id by NULL\n");
+ return -EINVAL;
+ }
+
eth_dev_shared_data_prepare();
rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
@@ -645,6 +670,12 @@ eth_dev_owner_set(const uint16_t port_id, const uint64_t
old_owner_id,
return -ENODEV;
}
+ if (new_owner == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to set ethdev port %u owner to
NULL\n",
+ port_id);
+ return -EINVAL;
+ }
+
if (!eth_is_valid_owner_id(new_owner->id) &
Re: [dpdk-dev] [PATCH v4] ethdev: add sanity checks in control APIs
Hi, Thomas,
v6 has been sent to fix it, please check it out, thanks.
在 2021/4/15 20:15, Thomas Monjalon 写道:
15/04/2021 14:04, Kevin Traynor:
On 15/04/2021 01:52, Min Hu (Connor) wrote:
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterator init for NULL\n");
"Failed to init iterator for NULL iterator\n"
The word "Failed" looks weird in these checks.
What about "Cannot"?
Example: "Cannot init NULL iterator"
+ if (devargs_str == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to iterate matching NULL\n");
"Failed to init iterator for NULL devargs\n"
"Cannot init iterator for NULL devargs"
+ if (owner == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to get ethdev port %u owner by
NULL\n",
+ port_id);
+ return -EINVAL;
+ }
This fn uses both %u and %"PRIu16" for port_id
I don't see the benefit of PRIu16.
+ if (str == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to convert link to NULL\n");
"Failed to convert link to NULL string\n"
"Cannot convert link in NULL string"
+ if (eth_link == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to convert NULL to string\n");
"Failed to convert NULL link to string\n"
"Cannot convert NULL link"
.
Re: [dpdk-dev] [PATCH v5] ethdev: add sanity checks in control APIs
Hi, Ferruh,
v6 has been sent to fix it, please check it out, thanks.
在 2021/4/15 23:38, Ferruh Yigit 写道:
On 4/15/2021 12:09 PM, Min Hu (Connor) wrote:
This patch adds more sanity checks in control path APIs.
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and
variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple
process model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
Reviewed-by: Andrew Rybchenko
Hi Connor,
Please find a few comments below, with they are addressed:
Reviewed-by: Ferruh Yigit
@@ -738,23 +769,30 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
int
rte_eth_dev_owner_get(const uint16_t port_id, struct
rte_eth_dev_owner *owner)
{
- int ret = 0;
- struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
-
- eth_dev_shared_data_prepare();
+ struct rte_eth_dev *ethdev;
- rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -1);
Not sure '-1' is a good return value, what do you think sending
'-ENODEV' as it is mostly done in the file?
Also emptly line can be removed as suggested by Andrew.
<...>
@@ -2138,8 +2181,14 @@ rte_eth_rx_hairpin_queue_setup(uint16_t
port_id, uint16_t rx_queue_id,
int count;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
dev = &rte_eth_devices[port_id];
+
+ if (conf == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to setup ethdev port %u Rx
hairpin queue to NULL\n",
+ port_id);
When the line is long, can you break the message part to next line, to
reduce the length at least a few columns, like:
RTE_ETHDEV_LOG(ERR,
"Failed to setup ethdev port %u Rx hairpin queue to NULL\n",
port_id);
Same for all long messages.
<...>
@@ -5619,6 +5853,11 @@ rte_eth_representor_id_get(const struct
rte_eth_dev *ethdev,
struct rte_eth_representor_info *info = NULL;
size_t size;
+ if (ethdev == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Failed to get device representor info
from NULL\n");
+ return -EINVAL;
+ }
+
This is also an internal function, not sure if we should verify the
'ehtdev' here.
.
Re: [dpdk-dev] [PATCH v5] ethdev: add sanity checks in control APIs
在 2021/4/16 0:21, Thomas Monjalon 写道: 15/04/2021 17:45, Ferruh Yigit: Not exactly related to this patch, but related to the API input, some input that is filled by API is memset first before passing it to the PMD, which makes sense. And for this we have two different ordering with dev_ops check: 1. memset(input) check dev_ops, return error if not supported call dev_ops 2. check dev_ops, return error if not supported memset(input) call dev_ops Connor, Thomas, Andrew, Do you think does it have any benifit to unify it, and is one better than other? Yes good catch. It deserves a separate patch to not memset if not needed. We must check if supported before memset. Agreed, I will send next patch to fix it. .
[dpdk-dev] [PATCH v4 2/2] test: fix missing check for thread creation
From: Chengwen Feng
There was a call for thread create function without result check.
Add result check and message print out after failure.
Fixes: 086eb64db39e ("test/pdump: add unit test for pdump library")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
app/test/process.h | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/app/test/process.h b/app/test/process.h
index 27f1b1c..a09a088 100644
--- a/app/test/process.h
+++ b/app/test/process.h
@@ -48,6 +48,7 @@ process_dup(const char *const argv[], int numargs, const char
*env_value)
#ifdef RTE_LIB_PDUMP
#ifdef RTE_NET_RING
pthread_t thread;
+ int rc;
#endif
#endif
@@ -126,8 +127,13 @@ process_dup(const char *const argv[], int numargs, const
char *env_value)
/* parent process does a wait */
#ifdef RTE_LIB_PDUMP
#ifdef RTE_NET_RING
- if ((strcmp(env_value, "run_pdump_server_tests") == 0))
- pthread_create(&thread, NULL, &send_pkts, NULL);
+ if ((strcmp(env_value, "run_pdump_server_tests") == 0)) {
+ rc = pthread_create(&thread, NULL, &send_pkts, NULL);
+ if (rc != 0) {
+ rte_panic("Cannot start send pkts thread: %s\n",
+ strerror(rc));
+ }
+ }
#endif
#endif
--
2.7.4
[dpdk-dev] [PATCH v4 0/2] fix missing check for thread creation
There exist some thread creation function without result check. This set of patches add result check and message print out after failure. Chengwen Feng (2): telemetry: fix missing check for thread creation test: fix missing check for thread creation --- v4: * change error logging. v3: * modify return value check and error logging. v2: * fix compiling bugs of missig value. app/test/process.h | 10 +++-- lib/librte_telemetry/telemetry.c| 37 ++--- lib/librte_telemetry/telemetry_legacy.c | 11 -- 3 files changed, 51 insertions(+), 7 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH v4 1/2] telemetry: fix missing check for thread creation
From: Chengwen Feng
Add result check and message print out for thread creation after
failure.
Fixes: b80fe1805eee ("telemetry: introduce backward compatibility")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
lib/librte_telemetry/telemetry.c| 37 ++---
lib/librte_telemetry/telemetry_legacy.c | 11 --
2 files changed, 43 insertions(+), 5 deletions(-)
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index 7e08afd..08ce189 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -350,6 +350,7 @@ socket_listener(void *socket)
{
while (1) {
pthread_t th;
+ int rc;
struct socket *s = (struct socket *)socket;
int s_accepted = accept(s->sock, NULL, NULL);
if (s_accepted < 0) {
@@ -366,7 +367,17 @@ socket_listener(void *socket)
__atomic_add_fetch(s->num_clients, 1,
__ATOMIC_RELAXED);
}
- pthread_create(&th, NULL, s->fn, (void *)(uintptr_t)s_accepted);
+ rc = pthread_create(&th, NULL, s->fn,
+ (void *)(uintptr_t)s_accepted);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create client thread: %s\n",
+strerror(rc));
+ close(s_accepted);
+ if (s->num_clients != NULL)
+ __atomic_sub_fetch(s->num_clients, 1,
+ __ATOMIC_RELAXED);
+ continue;
+ }
pthread_detach(th);
}
return NULL;
@@ -425,6 +436,7 @@ static int
telemetry_legacy_init(void)
{
pthread_t t_old;
+ int rc;
if (num_legacy_callbacks == 1) {
TMTY_LOG(WARNING, "No legacy callbacks, legacy socket not
created\n");
@@ -440,7 +452,16 @@ telemetry_legacy_init(void)
v1_socket.sock = create_socket(v1_socket.path);
if (v1_socket.sock < 0)
return -1;
- pthread_create(&t_old, NULL, socket_listener, &v1_socket);
+ rc = pthread_create(&t_old, NULL, socket_listener, &v1_socket);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create legcay socket thread: %s\n",
+strerror(rc));
+ close(v1_socket.sock);
+ v1_socket.sock = -1;
+ unlink(v1_socket.path);
+ v1_socket.path[0] = '\0';
+ return -1;
+ }
pthread_setaffinity_np(t_old, sizeof(*thread_cpuset), thread_cpuset);
TMTY_LOG(DEBUG, "Legacy telemetry socket initialized ok\n");
@@ -451,6 +472,7 @@ static int
telemetry_v2_init(void)
{
pthread_t t_new;
+ int rc;
v2_socket.num_clients = &v2_clients;
rte_telemetry_register_cmd("/", list_commands,
@@ -469,7 +491,16 @@ telemetry_v2_init(void)
v2_socket.sock = create_socket(v2_socket.path);
if (v2_socket.sock < 0)
return -1;
- pthread_create(&t_new, NULL, socket_listener, &v2_socket);
+ rc = pthread_create(&t_new, NULL, socket_listener, &v2_socket);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create socket thread: %s\n",
+strerror(rc));
+ close(v2_socket.sock);
+ v2_socket.sock = -1;
+ unlink(v2_socket.path);
+ v2_socket.path[0] = '\0';
+ return -1;
+ }
pthread_setaffinity_np(t_new, sizeof(*thread_cpuset), thread_cpuset);
atexit(unlink_sockets);
diff --git a/lib/librte_telemetry/telemetry_legacy.c
b/lib/librte_telemetry/telemetry_legacy.c
index 5e9af37..b7cd1bd 100644
--- a/lib/librte_telemetry/telemetry_legacy.c
+++ b/lib/librte_telemetry/telemetry_legacy.c
@@ -83,6 +83,7 @@ register_client(const char *cmd __rte_unused, const char
*params,
pthread_t th;
char data[BUF_SIZE];
int fd;
+ int rc;
struct sockaddr_un addrs;
#endif /* !RTE_EXEC_ENV_WINDOWS */
@@ -112,8 +113,14 @@ register_client(const char *cmd __rte_unused, const char
*params,
close(fd);
return -1;
}
- pthread_create(&th, NULL, &legacy_client_handler,
- (void *)(uintptr_t)fd);
+ rc = pthread_create(&th, NULL, &legacy_client_handler,
+ (void *)(uintptr_t)fd);
+ if (rc != 0) {
+ fprintf(stderr, "Failed to create legacy client thread: %s\n",
+ strerror(rc));
+ close(fd);
+ return -1;
+ }
#endif /* !RTE_EXEC_ENV_WINDOWS */
return 0;
}
--
2.7.4
Re: [dpdk-dev] [PATCH v3 1/2] telemetry: fix missing check for thread creation
Thanks Power, v4 has been sent, please check it out.
在 2021/4/16 0:11, Power, Ciara 写道:
Hi Connor,
Thanks for looking at this. Left some comments inline.
Ciara
-Original Message-
From: Min Hu (Connor)
Sent: Thursday 15 April 2021 12:50
To: dev@dpdk.org
Cc: Yigit, Ferruh ; Power, Ciara
; Pattan, Reshma ;
david.march...@redhat.com
Subject: [PATCH v3 1/2] telemetry: fix missing check for thread creation
From: Chengwen Feng
Add result check and message print out for thread creation after failure.
Fixes: b80fe1805eee ("telemetry: introduce backward compatibility")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
lib/librte_telemetry/telemetry.c| 30 +++-
--
lib/librte_telemetry/telemetry_legacy.c | 10 --
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/lib/librte_telemetry/telemetry.c
b/lib/librte_telemetry/telemetry.c
index 7e08afd..e6a99f3 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -350,6 +350,7 @@ socket_listener(void *socket) {
while (1) {
pthread_t th;
+ int rc;
struct socket *s = (struct socket *)socket;
int s_accepted = accept(s->sock, NULL, NULL);
if (s_accepted < 0) {
@@ -366,7 +367,12 @@ socket_listener(void *socket)
__atomic_add_fetch(s->num_clients, 1,
__ATOMIC_RELAXED);
}
- pthread_create(&th, NULL, s->fn, (void
*)(uintptr_t)s_accepted);
+ rc = pthread_create(&th, NULL, s->fn, (void
*)(uintptr_t)s_accepted);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create client thread\n");
+ close(s_accepted);
+ return NULL;
I think this should be "continue" instead of "return NULL" - if one client
connection thread fails,
we probably want to keep listening for more clients rather than stop completely.
The "s->num_clients" counter should be decremented here too,
it gets incremented above when the connection is accepted, but if we are
closing the connection,
the counter should reflect that.
+ }
pthread_detach(th);
}
return NULL;
@@ -425,6 +431,7 @@ static int
telemetry_legacy_init(void)
{
pthread_t t_old;
+ int rc;
if (num_legacy_callbacks == 1) {
TMTY_LOG(WARNING, "No legacy callbacks, legacy socket not
created\n"); @@ -440,7 +447,15 @@ telemetry_legacy_init(void)
v1_socket.sock = create_socket(v1_socket.path);
if (v1_socket.sock < 0)
return -1;
- pthread_create(&t_old, NULL, socket_listener, &v1_socket);
+ rc = pthread_create(&t_old, NULL, socket_listener, &v1_socket);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create thread for legacy
socket\n");
+ close(v1_socket.sock);
+ v1_socket.sock = -1;
+ unlink(v1_socket.path);
+ v1_socket.path[0] = '\0';
+ return -1;
+ }
pthread_setaffinity_np(t_old, sizeof(*thread_cpuset),
thread_cpuset);
TMTY_LOG(DEBUG, "Legacy telemetry socket initialized ok\n"); @@ -
451,6 +466,7 @@ static int
telemetry_v2_init(void)
{
pthread_t t_new;
+ int rc;
v2_socket.num_clients = &v2_clients;
rte_telemetry_register_cmd("/", list_commands, @@ -469,7 +485,15
@@ telemetry_v2_init(void)
v2_socket.sock = create_socket(v2_socket.path);
if (v2_socket.sock < 0)
return -1;
- pthread_create(&t_new, NULL, socket_listener, &v2_socket);
+ rc = pthread_create(&t_new, NULL, socket_listener, &v2_socket);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create thread for socket");
+ close(v2_socket.sock);
+ v2_socket.sock = -1;
+ unlink(v2_socket.path);
+ v2_socket.path[0] = '\0';
+ return -1;
+ }
pthread_setaffinity_np(t_new, sizeof(*thread_cpuset),
thread_cpuset);
atexit(unlink_sockets);
diff --git a/lib/librte_telemetry/telemetry_legacy.c
b/lib/librte_telemetry/telemetry_legacy.c
index 5e9af37..fd242bf 100644
--- a/lib/librte_telemetry/telemetry_legacy.c
+++ b/lib/librte_telemetry/telemetry_legacy.c
@@ -83,6 +83,7 @@ register_client(const char *cmd __rte_unused, const
char *params,
pthread_t th;
char data[BUF_SIZE];
int fd;
+ int rc;
struct sockaddr_un addrs;
#endif /* !RTE_EXEC_ENV_WINDOWS */
@@ -112,8 +113,13 @@ register_client(const char *cmd __rte_unused, const
char *params,
close(fd);
Re: [dpdk-dev] [PATCH v3 2/2] test: fix missing check for thread creation
在 2021/4/16 1:05, Pattan, Reshma 写道:
-Original Message-
From: Min Hu (Connor)
+ if ((strcmp(env_value, "run_pdump_server_tests") == 0)) {
+ rc = pthread_create(&thread, NULL, &send_pkts, NULL);
+ if (rc != 0)
+ rte_panic("Cannot start send pkts thread\n");
+ }
I think you still have not addressed the David comment on previous version of
the patch.
So , can you change this to something like below
Option1)
rc = pthread_create(&thread, NULL, &send_pkts, NULL);
If ( rc ! = 0 )
{
rte_panic("Cannot start send pkts thread : %s\n", strerror(rc));
}
Or
Option2)
If ( pthread_create(&thread, NULL, &send_pkts, NULL) !=0 )
rte_panic("Cannot start send pkts thread\n");
Also,
test: fix missing check for thread creation
Change this subject line to "test/pdump: fix missing check for thread creation"
Hi, Pattan, fixed in v4, except for "test/pdump", if v4 is OK, please
modify it for me, @Ferruh, thanks.
Thanks,
Reshma
.
Re: [dpdk-dev] [PATCH v3 1/2] telemetry: fix missing check for thread creation
Hi, David, fixed in v4. Thanks.
在 2021/4/15 22:55, David Marchand 写道:
On Thu, Apr 15, 2021 at 1:50 PM Min Hu (Connor) wrote:
diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c
index 7e08afd..e6a99f3 100644
--- a/lib/librte_telemetry/telemetry.c
+++ b/lib/librte_telemetry/telemetry.c
@@ -350,6 +350,7 @@ socket_listener(void *socket)
{
while (1) {
pthread_t th;
+ int rc;
struct socket *s = (struct socket *)socket;
int s_accepted = accept(s->sock, NULL, NULL);
if (s_accepted < 0) {
@@ -366,7 +367,12 @@ socket_listener(void *socket)
__atomic_add_fetch(s->num_clients, 1,
__ATOMIC_RELAXED);
}
- pthread_create(&th, NULL, s->fn, (void *)(uintptr_t)s_accepted);
+ rc = pthread_create(&th, NULL, s->fn, (void
*)(uintptr_t)s_accepted);
+ if (rc != 0) {
+ TMTY_LOG(ERR, "Error with create client thread\n");
+ close(s_accepted);
+ return NULL;
+ }
Repeating my comment:
Either you use this rc variable (adding strerror(rc) in the log
message in TMTY_LOG()) or you remove the variable and simply test if
(pthread_create(...) != 0).
This comment applies to other changes in this patch.
And this applies to patch 2 too.
pthread_detach(th);
}
return NULL;
Thanks.
--
David Marchand
.
Re: [dpdk-dev] [PATCH v3 2/2] test: fix missing check for thread creation
在 2021/4/16 16:34, Ferruh Yigit 写道:
On 4/16/2021 9:21 AM, Min Hu (Connor) wrote:
在 2021/4/16 1:05, Pattan, Reshma 写道:
-Original Message-
From: Min Hu (Connor)
+ if ((strcmp(env_value, "run_pdump_server_tests") == 0)) {
+ rc = pthread_create(&thread, NULL, &send_pkts, NULL);
+ if (rc != 0)
+ rte_panic("Cannot start send pkts thread\n");
+ }
I think you still have not addressed the David comment on previous
version of the patch.
So , can you change this to something like below
Option1)
rc = pthread_create(&thread, NULL, &send_pkts, NULL);
If ( rc ! = 0 )
{
rte_panic("Cannot start send pkts thread : %s\n", strerror(rc));
}
Or
Option2)
If ( pthread_create(&thread, NULL, &send_pkts, NULL) !=0 )
rte_panic("Cannot start send pkts thread\n");
Also,
test: fix missing check for thread creation
Change this subject line to "test/pdump: fix missing check for
thread creation"
Hi, Pattan, fixed in v4, except for "test/pdump", if v4 is OK, please
modify it for me, @Ferruh, thanks.
Hi Connor, this patch is on David's domain, he will commit the patch,
not me.
Sorry for that, Ferruh, @David, thanks.
.
[dpdk-dev] [PATCH 0/2] bugfix for doc
This patchset contains 2 patches, to fix testpmd doc and hns3 doc. Huisong Li (1): doc/testpmd: delete queue stats mapping description Min Hu (Connor) (1): doc: fix HiSilicon copyright syntax doc/guides/nics/hns3.rst| 6 +++--- doc/guides/testpmd_app_ug/run_app.rst | 8 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 14 ++ 3 files changed, 9 insertions(+), 19 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH 2/2] doc: fix HiSilicon copyright syntax
This patch fixes HiSilicon copyright syntax.
According to the suggestion of our legal department,
to standardize the copyright license of our code to
avoid potential copyright risks, we make a unified
modification to the "Hisilicon", which was nonstandard,
in the main modules we maintain.
We change it to "HiSilicon", which is consistent with
the terms used on the following official website:
https://www.hisilicon.com/en/terms-of-use.
Fixes: 565829db8b8f ("net/hns3: add build and doc infrastructure")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
doc/guides/nics/hns3.rst | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst
index 3366562..20c7387 100644
--- a/doc/guides/nics/hns3.rst
+++ b/doc/guides/nics/hns3.rst
@@ -1,12 +1,12 @@
.. SPDX-License-Identifier: BSD-3-Clause
-Copyright(c) 2018-2019 Hisilicon Limited.
+Copyright(c) 2018-2019 HiSilicon Limited.
HNS3 Poll Mode Driver
===
The hns3 PMD (**librte_net_hns3**) provides poll mode driver support
-for the inbuilt Hisilicon Network Subsystem(HNS) network engine
-found in the Hisilicon Kunpeng 920 SoC.
+for the inbuilt HiSilicon Network Subsystem(HNS) network engine
+found in the HiSilicon Kunpeng 920 SoC.
Features
--
2.7.4
[dpdk-dev] [PATCH 1/2] doc/testpmd: delete queue stats mapping description
From: Huisong Li
The "--tx-queue-stats-mapping" and "--rx-queue-stats-mapping"
, and display and clear of "stats_map" have been removed from
testpmd.
This patch deletes some descriptions about queue stats mapping
in testpmd doc.
Fixes: 08dcd1870686 ("app/testpmd: fix queue stats mapping configuration")
Cc: sta...@dpdk.org
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
doc/guides/testpmd_app_ug/run_app.rst | 8
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 14 ++
2 files changed, 6 insertions(+), 16 deletions(-)
diff --git a/doc/guides/testpmd_app_ug/run_app.rst
b/doc/guides/testpmd_app_ug/run_app.rst
index d714951..6642efc 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -356,14 +356,6 @@ The command line options are:
Set the transmit RS bit threshold of TX rings to N, where 0 <= N <= value
of ``--txd``.
The default value is 0.
-* ``--rx-queue-stats-mapping=(port,queue,mapping)[,(port,queue,mapping)]``
-
-Set the RX queues statistics counters mapping 0 <= mapping <= 15.
-
-* ``--tx-queue-stats-mapping=(port,queue,mapping)[,(port,queue,mapping)]``
-
-Set the TX queues statistics counters mapping 0 <= mapping <= 15.
-
* ``--no-flush-rx``
Don't flush the RX streams before starting forwarding. Used mainly with
the PCAP PMD.
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index e3bfed5..9854b70 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -24,10 +24,10 @@ If you type a partial command and hit you get a
list of the available
testpmd> show port
- info [Mul-choice STRING]: show|clear port
info|stats|xstats|fdir|stat_qmap|dcb_tc X
- info [Mul-choice STRING]: show|clear port
info|stats|xstats|fdir|stat_qmap|dcb_tc all
- stats [Mul-choice STRING]: show|clear port
info|stats|xstats|fdir|stat_qmap|dcb_tc X
- stats [Mul-choice STRING]: show|clear port
info|stats|xstats|fdir|stat_qmap|dcb_tc all
+ info [Mul-choice STRING]: show|clear port
info|stats|xstats|fdir|dcb_tc|cap X
+ info [Mul-choice STRING]: show|clear port
info|stats|xstats|fdir|dcb_tc|cap all
+ stats [Mul-choice STRING]: show|clear port
info|stats|xstats|fdir|dcb_tc|cap X
+ stats [Mul-choice STRING]: show|clear port
info|stats|xstats|fdir|dcb_tc|cap all
...
@@ -159,7 +159,7 @@ show port
Display information for a given port or all ports::
- testpmd> show port (info|summary|stats|xstats|fdir|stat_qmap|dcb_tc)
(port_id|all)
+ testpmd> show port (info|summary|stats|xstats|fdir|dcb_tc|cap) (port_id|all)
The available information categories are:
@@ -173,8 +173,6 @@ The available information categories are:
* ``fdir``: Flow Director information and statistics.
-* ``stat_qmap``: Queue statistics mapping.
-
* ``dcb_tc``: DCB information such as TC mapping.
For example:
@@ -244,7 +242,7 @@ clear port
Clear the port statistics and forward engine statistics for a given port or
for all ports::
- testpmd> clear port (info|stats|xstats|fdir|stat_qmap) (port_id|all)
+ testpmd> clear port (info|stats|xstats|fdir) (port_id|all)
For example::
--
2.7.4
Re: [dpdk-dev] [PATCH v4] ethdev: add sanity checks in control APIs
在 2021/4/16 18:09, Kevin Traynor 写道:
On 16/04/2021 08:00, Min Hu (Connor) wrote:
Thanks Kevin,
all is fixed in v6, please review it, thanks.
Some comments are below.
在 2021/4/15 20:04, Kevin Traynor 写道:
On 15/04/2021 01:52, Min Hu (Connor) wrote:
This patch adds more sanity checks in control path APIs.
Hi Connor,
A few general comments,
--
Some of the functions have unit tests, you could consider adding unit
tests for the new checks. Considering the checks are not subtle and
unlikely to be messed up in future, not adding unit tests is not a
blocker imho.
--
It took me a while to get what you meant with "by NULL". It's usage
seems like in "Death by taxes". Perhaps "because NULL ptr" would be a
better way to phrase this generically, but I think it is more useful to
say what is NULL.
e.g. "Failed to convert NULL to string\n" is very generic and would be
better as "Failed to convert NULL link to string\n" . ok, still a bit
generic but more of a clue.
I won't comment on each log message individually but I've added a few
suggestions here and there.
Thanks, I think it looks a lot nicer to read in v6 my completely
subjective biased opinion :-)
--
Did you check the usage of these functions in DPDK, and if the return
value is handled ok? e.g. RTE_ETH_FOREACH_MATCHING_DEV will keep calling
iterator functions. I'm not sure that having a return check is needed in
that case, but there could be other cases where you want to take some
different action now.
As iterator functions are all APIs, they may be used by APP directly.
I think param check is necessary.
The point is that it would continue to call the functions even after it
caught this error, so would continue to print error messages. Yes, that
is much better than a seg fault and maybe in this case that is ok. I
will leave it to maintainers to decided.
I was just wondering if there was additional things similar to this in
DPDK where handling these new errors could now be improved too. I don't
think it has to be a prerequisite for this patch, as this patch is still
an improvement.
Thanks Kevin.
Well, for what your metioned, I will try to look for it.
If found, I will send another patch or initiate discussions.
Thanks.
some other comments inlined,
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
.
[dpdk-dev] [PATCH v7] ethdev: add sanity checks in control APIs
This patch adds more sanity checks in control path APIs.
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
Reviewed-by: Andrew Rybchenko
Acked-by: Kevin Traynor
---
v7:
* use a natural sounding names instead of argument name.
v6:
* Change logging grammar.
* "Failed to" change to "Cannot".
* Break the message part to next line.
* Delete param check for internal function.
v5:
* Keep "RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV)"
and "dev = &rte_eth_devices[port_id]" together.
v4:
* add error logging.
* delete check in control path API.
v3:
* set port_id checked first.
* add error logging.
v2:
* Removed unnecessary checks.
* Deleted checks in internal API.
* Added documentation in the header file.
---
lib/librte_ethdev/rte_ethdev.c | 365 +
lib/librte_ethdev/rte_ethdev.h | 20 ++-
2 files changed, 349 insertions(+), 36 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 6b5cfd6..0c23128 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -199,6 +199,16 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
char *cls_str = NULL;
int str_size;
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot init iterator for NULL iterator\n");
+ return -EINVAL;
+ }
+
+ if (devargs_str == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot init iterator for NULL devargs\n");
+ return -EINVAL;
+ }
+
memset(iter, 0, sizeof(*iter));
/*
@@ -293,6 +303,11 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
uint16_t
rte_eth_iterator_next(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot iterate next for NULL\n");
+ return RTE_MAX_ETHPORTS;
+ }
+
if (iter->cls == NULL) /* invalid ethdev iterator */
return RTE_MAX_ETHPORTS;
@@ -322,6 +337,11 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
void
rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot iterator clear up for NULL iter\n");
+ return;
+ }
+
if (iter->bus_str == NULL)
return; /* nothing to free in pure class filter */
free(RTE_CAST_FIELD(iter, bus_str, char *)); /* workaround const */
@@ -622,6 +642,11 @@ rte_eth_find_next_owned_by(uint16_t port_id, const
uint64_t owner_id)
int
rte_eth_dev_owner_new(uint64_t *owner_id)
{
+ if (owner_id == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot get owner id for NULL param\n");
+ return -EINVAL;
+ }
+
eth_dev_shared_data_prepare();
rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
@@ -645,6 +670,13 @@ eth_dev_owner_set(const uint16_t port_id, const uint64_t
old_owner_id,
return -ENODEV;
}
+ if (new_owner == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot set ethdev port %u owner to NULL new_owner\n",
+ port_id);
+ return -EINVAL;
+ }
+
if (!eth_is_valid_owner_id(new_owner->id) &&
!eth_is_valid_owner_id(old_owner_id)) {
RTE_ETHDEV_LOG(ERR,
@@ -738,23 +770,30 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
int
rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
{
- int ret = 0;
- struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
-
- eth_dev_shared_data_prepare();
-
- rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
+ struct rte_eth_dev *ethdev;
- if (port_id >= RTE_MAX_ETHPORTS || !eth_dev_is_allocated(ethdev)) {
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ ethdev = &rte_eth_devices[port_id];
+ if (!eth_dev_is_allocated(ethdev)) {
RTE_ETHDEV_LOG(ERR, "Port id %"PRIu16" is not allocated\n",
port_id);
- ret = -ENODEV;
- } else {
- rte_memcpy(owner, ðdev->data->owner, sizeof(*owner));
+ return -ENODEV;
}
+ if (owner == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+
Re: [dpdk-dev] [PATCH v6] ethdev: add sanity checks in control APIs
Hi, Kevin and all
fixed in v7, thanks.
在 2021/4/16 18:22, Kevin Traynor 写道:
On 16/04/2021 07:52, Min Hu (Connor) wrote:
This patch adds more sanity checks in control path APIs.
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
Reviewed-by: Andrew Rybchenko
@@ -1298,9 +1342,15 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t
nb_rx_q, uint16_t nb_tx_q,
uint16_t old_mtu;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-
dev = &rte_eth_devices[port_id];
+ if (dev_conf == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot configure ethdev port %u to NULL dev_conf\n",
The others use a natural sounding names instead of argument name. If you
wanted to match that it could be "..to NULL conf"
+ port_id);
+ return -EINVAL;
+ }
+
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -ENOTSUP);
if (dev->data->dev_started) {
@@ -2609,6 +2680,13 @@ rte_eth_link_get(uint16_t port_id, struct rte_eth_link
*eth_link)
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
dev = &rte_eth_devices[port_id];
+ if (eth_link == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot get ethdev port %u link for NULL eth_link\n",
^^^
+ port_id);
+ return -EINVAL;
+ }
+
if (dev->data->dev_conf.intr_conf.lsc &&
dev->data->dev_started)
rte_eth_linkstatus_get(dev, eth_link);
@@ -2629,6 +2707,13 @@ rte_eth_link_get_nowait(uint16_t port_id, struct
rte_eth_link *eth_link)
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
dev = &rte_eth_devices[port_id];
+ if (eth_link == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot get nowait ethdev port %u for NULL link\n",
^^^
I would probably stick with "link" for both these functions, rather than
argument name "eth_link" in one "link" in other.
+ port_id);
+ return -EINVAL;
+ }
+
if (dev->data->dev_conf.intr_conf.lsc &&
dev->data->dev_started)
rte_eth_linkstatus_get(dev, eth_link);
Thanks Connor. There are only minor nits, so
Acked-by: Kevin Traynor
.
Re: [dpdk-dev] [RFC 0/2] add Tx prepare support for bonding device
Looks good to me. 在 2021/4/16 19:04, Chengchang Tang 写道: This patch add Tx prepare for bonding device. Currently, the bonding driver has not implemented the callback of rte_eth_tx_prepare function. Therefore, the TX prepare function of the slave devices will never be invoked. When hardware offloading such as CKSUM and TSO are enabled for some drivers, tx_prepare needs to be used to adjust packets (for example, set correct pseudo packet headers). Otherwise, related offloading fails and even packets are sent incorrectly. Due to this limitation, the bonded device cannot use these HW offloading in the Tx direction. Because packet sending algorithms are numerous and complex in bond PMD, it is hard to design the callback for rte_eth_tx_prepare. In this patch, the tx_prepare callback of bonding PMD is not implemented. Instead, rte_eth_tx_prepare has been called in tx_burst callback. And a global variable is introduced to control whether the bonded device need call the rte_eth_tx_prepare. If upper-layer users need to use some TX offloading that depend on tx_prepare , they should enable the preparation function. In this way, the bonded device will call the rte_eth_tx_prepare for the fast path packets in the tx_burst callback. Chengchang Tang (2): net/bonding: add Tx prepare for bonding app/testpmd: add cmd for bonding Tx prepare app/test-pmd/cmdline.c | 66 + doc/guides/testpmd_app_ug/testpmd_funcs.rst | 9 drivers/net/bonding/eth_bond_private.h | 1 + drivers/net/bonding/rte_eth_bond.h | 29 + drivers/net/bonding/rte_eth_bond_api.c | 28 drivers/net/bonding/rte_eth_bond_pmd.c | 33 +-- drivers/net/bonding/version.map | 5 +++ 7 files changed, 167 insertions(+), 4 deletions(-) -- 2.7.4 .
Re: [dpdk-dev] [PATCH v6] ethdev: add sanity checks in control APIs
在 2021/4/17 0:28, Stephen Hemminger 写道:
On Fri, 16 Apr 2021 11:22:02 +0100
Kevin Traynor wrote:
+ if (dev_conf == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot configure ethdev port %u to NULL dev_conf\n",
The others use a natural sounding names instead of argument name. If you
wanted to match that it could be "..to NULL conf"
I would prefer that error messages don't try to be English sentences.
The wording ends up awkward. and overly wordy.
If function name is automatically included by RTE_ETHDEV_LOG() then
Just:
RTE_ETHDEV_LOG(ERR, "NULL ethdev")
should be enough for programmer to find/fix the problem
.
Hi, Stephen,
Your opinion is quit different from that of Andrew Rybchenko.
Andrew does not support show function name in the log:
"- log messages should be human readable (i.e. I'd avoid
usage of function name)"
@Andrew ,@Thoms, @Ferruh, @Kevin, so, what's your opinion ?
[dpdk-dev] [PATCH v10] app/testpmd: support multi-process
This patch adds multi-process support for testpmd.
The test cmd example as follows:
the primary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=0
the secondary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=1
Signed-off-by: Min Hu (Connor)
Signed-off-by: Lijun Ou
Acked-by: Xiaoyun Li
Acked-by: Ajit Khaparde
---
v10:
* Hid process type checks behind new functions.
* Added comments.
v9:
* Updated release notes and rst doc.
* Deleted deprecated codes.
* move macro and variable.
v8:
* Added warning info about queue numbers and process numbers.
v7:
* Fixed compiling error for unexpected unindent.
v6:
* Add rte flow description for multiple process.
v5:
* Fixed run_app.rst for multiple process description.
* Fix compiling error.
v4:
* Fixed minimum vlaue of Rxq or Txq in doc.
v3:
* Fixed compiling error using gcc10.0.
v2:
* Added document for this patch.
---
app/test-pmd/cmdline.c | 6 ++
app/test-pmd/config.c | 21 +-
app/test-pmd/parameters.c | 11 +++
app/test-pmd/testpmd.c | 127 ++---
app/test-pmd/testpmd.h | 9 +++
doc/guides/rel_notes/release_21_05.rst | 1 +
doc/guides/testpmd_app_ug/run_app.rst | 86 ++
7 files changed, 236 insertions(+), 25 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5bf1497..e465824 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -5354,6 +5354,12 @@ cmd_set_flush_rx_parsed(void *parsed_result,
__rte_unused void *data)
{
struct cmd_set_flush_rx *res = parsed_result;
+
+ if (num_procs > 1 && (strcmp(res->mode, "on") == 0)) {
+ printf("multi-process doesn't support to flush rx queues.\n");
+ return;
+ }
+
no_flush_rx = (uint8_t)((strcmp(res->mode, "on") == 0) ? 0 : 1);
}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 40b2b29..d0e5b29 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2860,6 +2860,8 @@ rss_fwd_config_setup(void)
queueid_t rxq;
queueid_t nb_q;
streamid_t sm_id;
+ int start;
+ int end;
nb_q = nb_rxq;
if (nb_q > nb_txq)
@@ -2877,7 +2879,22 @@ rss_fwd_config_setup(void)
init_fwd_streams();
setup_fwd_config_of_each_lcore(&cur_fwd_config);
- rxp = 0; rxq = 0;
+
+ if (proc_id > 0 && nb_q % num_procs)
+ printf("Warning! queue numbers should be multiple of "
+ "processes, or packet loss will happen.\n");
+
+ /**
+* In multi-process, All queues are allocated to different
+* processes based on num_procs and proc_id. For example:
+* if supports 4 queues(nb_q), 2 processes(num_procs),
+* the 0~1 queue for primary process.
+* the 2~3 queue for secondary process.
+*/
+ start = proc_id * nb_q / num_procs;
+ end = start + nb_q / num_procs;
+ rxp = 0;
+ rxq = start;
for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) {
struct fwd_stream *fs;
@@ -2894,6 +2911,8 @@ rss_fwd_config_setup(void)
continue;
rxp = 0;
rxq++;
+ if (rxq >= end)
+ rxq = start;
}
}
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index f3954c1..ece05c1 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -508,6 +508,9 @@ parse_link_speed(int n)
void
launch_args_parse(int argc, char** argv)
{
+#define PARAM_PROC_ID "proc-id"
+#define PARAM_NUM_PROCS "num-procs"
+
int n, opt;
char **argvopt;
int opt_idx;
@@ -625,6 +628,8 @@ launch_args_parse(int argc, char** argv)
{ "rx-mq-mode", 1, 0, 0 },
{ "record-core-cycles", 0, 0, 0 },
{ "record-burst-stats", 0, 0, 0 },
+ { PARAM_NUM_PROCS, 1, 0, 0 },
+ { PARAM_PROC_ID,1, 0, 0 },
{ 0, 0, 0, 0 },
};
@@ -1391,6 +1396,12 @@ launch_args_parse(int argc, char** argv)
record_core_cycles = 1;
if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
record_burst_stats = 1;
+ if (strncmp(lgopts[opt_idx].name,
+ PARAM_NUM_PROCS, 9) == 0)
+ num_procs = atoi(optarg);
+ if (strncmp(lgopts[opt_idx].name,
+ PARAM_PROC_ID, 7) == 0)
+
Re: [dpdk-dev] [PATCH v9] app/testpmd: support multi-process
Hi, Ferrruh,
All is fixed in v10, please check it out, thanks.
在 2021/4/17 0:30, Ferruh Yigit 写道:
On 4/16/2021 2:52 AM, Min Hu (Connor) wrote:
This patch adds multi-process support for testpmd.
The test cmd example as follows:
the primary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=0
the secondary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=1
Hi Connor,
Thanks for the update. +Anatoly as multi-process maintainer, and ethdev
maintainers.
Signed-off-by: Min Hu (Connor)
Signed-off-by: Lijun Ou
Acked-by: Xiaoyun Li
Acked-by: Ajit Khaparde
---
v9:
* Updated release notes and rst doc.
* Deleted deprecated codes.
* move macro and variable.
v8:
* Added warning info about queue numbers and process numbers.
v7:
* Fixed compiling error for unexpected unindent.
v6:
* Add rte flow description for multiple process.
v5:
* Fixed run_app.rst for multiple process description.
* Fix compiling error.
v4:
* Fixed minimum vlaue of Rxq or Txq in doc.
v3:
* Fixed compiling error using gcc10.0.
v2:
* Added document for this patch.
---
app/test-pmd/cmdline.c | 6 ++
app/test-pmd/config.c | 14 -
app/test-pmd/parameters.c | 12
app/test-pmd/testpmd.c | 108
+++--
app/test-pmd/testpmd.h | 3 +
doc/guides/rel_notes/release_21_05.rst | 1 +
doc/guides/testpmd_app_ug/run_app.rst | 86 ++
7 files changed, 196 insertions(+), 34 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5bf1497..e465824 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -5354,6 +5354,12 @@ cmd_set_flush_rx_parsed(void *parsed_result,
__rte_unused void *data)
{
struct cmd_set_flush_rx *res = parsed_result;
+
+ if (num_procs > 1 && (strcmp(res->mode, "on") == 0)) {
+ printf("multi-process doesn't support to flush rx queues.\n");
+ return;
+ }
+
no_flush_rx = (uint8_t)((strcmp(res->mode, "on") == 0) ? 0 : 1);
}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 40b2b29..c982c87 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2860,6 +2860,8 @@ rss_fwd_config_setup(void)
queueid_t rxq;
queueid_t nb_q;
streamid_t sm_id;
+ int start;
+ int end;
nb_q = nb_rxq;
if (nb_q > nb_txq)
@@ -2877,7 +2879,15 @@ rss_fwd_config_setup(void)
init_fwd_streams();
setup_fwd_config_of_each_lcore(&cur_fwd_config);
- rxp = 0; rxq = 0;
+
+ if (proc_id > 0 && nb_q % num_procs)
+ printf("Warning! queue numbers should be multiple of "
+ "processes, or packet loss will happen.\n");
+
+ start = proc_id * nb_q / num_procs;
+ end = start + nb_q / num_procs;
+ rxp = 0;
+ rxq = start;
Can you put some comment above, on what/why is done, something similar
to you already put into the documentation?
for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) {
struct fwd_stream *fs;
@@ -2894,6 +2904,8 @@ rss_fwd_config_setup(void)
continue;
rxp = 0;
rxq++;
+ if (rxq >= end)
+ rxq = start;
}
}
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index f3954c1..d86cc21 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -508,6 +508,9 @@ parse_link_speed(int n)
void
launch_args_parse(int argc, char** argv)
{
+#define PARAM_PROC_ID "proc-id"
+#define PARAM_NUM_PROCS "num-procs"
+
int n, opt;
char **argvopt;
int opt_idx;
@@ -625,6 +628,8 @@ launch_args_parse(int argc, char** argv)
{ "rx-mq-mode", 1, 0, 0 },
{ "record-core-cycles", 0, 0, 0 },
{ "record-burst-stats", 0, 0, 0 },
+ { PARAM_NUM_PROCS, 1, 0, 0 },
+ { PARAM_PROC_ID, 1, 0, 0 },
{ 0, 0, 0, 0 },
};
@@ -1391,6 +1396,13 @@ launch_args_parse(int argc, char** argv)
record_core_cycles = 1;
if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
record_burst_stats = 1;
+
To be consistent for rest, this empty line can be removed.
+ if (strncmp(lgopts[opt_idx].name,
+ PARAM_NUM_PROCS, 9) == 0)
+ num_procs = atoi(optarg);
+ if (strncmp(lgopts[opt_idx].name,
+ PARAM_PROC_ID, 7) == 0)
+ proc_id = atoi(optarg);
break;
case 'h':
usage(argv[0]);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 96d2e0f..01d0d82 100644
--- a/app/test-pmd/tes
Re: [dpdk-dev] [PATCH 0/7] support set thread name
在 2021/4/17 2:40, Thomas Monjalon 写道: 13/04/2021 03:12, Min Hu (Connor): 在 2021/4/12 15:19, David Marchand 写道: On Sat, Apr 10, 2021 at 12:40 PM Min Hu (Connor) wrote: This set of patches support set thread name for debugging. Chengwen Feng (7): net/ark: support set thread name net/ice: support set VSI reset thread name vdpa/ifc: support set notify and vring relay thread name raw/ifpga: support set monitor thread name examples/performance-thread: support set thread name telemetry: support set init threads name examples/vhost_blk: support set ctrl worker thread name Rather than add those calls, can maintainers check if their component can use ctrl threads instead? rte_ctrl_thread_create ensures both that the name is set, and that the ctrl thread won't run on the same cpu as "datapath" threads. I also saw some issues with components creating threads. I'll post a series addressing those later (net/ark, net/ice not detaching/joining created threads + telemetry not checking pthread_create failures). Totally agree with David. By the way, for "telemetry not checking pthread_create failures", I have sent patches to fix it. Please check it out. Thanks. Do I understand correctly that we prefer switching to rte_ctrl_thread_create() instead of this patch series adding rte_thread_setname()? Any volunteer to add the use of rte_ctrl_thread_create() in these drivers and examples? Hi, Thomos, we will send next patch to fix it, thanks. .
[dpdk-dev] [PATCH v8] ethdev: add sanity checks in control APIs
This patch adds more sanity checks in control path APIs.
Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input")
Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables")
Fixes: 0366137722a0 ("ethdev: check for invalid device name")
Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process
model")
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Fixes: f8244c6399d9 ("ethdev: increase port id range")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
Reviewed-by: Andrew Rybchenko
Acked-by: Kevin Traynor
---
v8:
* Change logging style.
v7:
* Use a natural sounding names instead of argument name.
v6:
* Change logging grammar.
* "Failed to" change to "Cannot".
* Break the message part to next line.
* Delete param check for internal function.
v5:
* Keep "RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV)"
and "dev = &rte_eth_devices[port_id]" together.
v4:
* add error logging.
* delete check in control path API.
v3:
* set port_id checked first.
* add error logging.
v2:
* Removed unnecessary checks.
* Deleted checks in internal API.
* Added documentation in the header file.
---
lib/librte_ethdev/rte_ethdev.c | 365 +
lib/librte_ethdev/rte_ethdev.h | 20 ++-
2 files changed, 349 insertions(+), 36 deletions(-)
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 6b5cfd6..60f9ab3 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -199,6 +199,16 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
char *cls_str = NULL;
int str_size;
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot init NULL iterator\n");
+ return -EINVAL;
+ }
+
+ if (devargs_str == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot init iterator for NULL devargs\n");
+ return -EINVAL;
+ }
+
memset(iter, 0, sizeof(*iter));
/*
@@ -293,6 +303,11 @@ rte_eth_iterator_init(struct rte_dev_iterator *iter, const
char *devargs_str)
uint16_t
rte_eth_iterator_next(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot iterate next NULL iter\n");
+ return RTE_MAX_ETHPORTS;
+ }
+
if (iter->cls == NULL) /* invalid ethdev iterator */
return RTE_MAX_ETHPORTS;
@@ -322,6 +337,11 @@ rte_eth_iterator_next(struct rte_dev_iterator *iter)
void
rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot clean up NULL iterator\n");
+ return;
+ }
+
if (iter->bus_str == NULL)
return; /* nothing to free in pure class filter */
free(RTE_CAST_FIELD(iter, bus_str, char *)); /* workaround const */
@@ -622,6 +642,11 @@ rte_eth_find_next_owned_by(uint16_t port_id, const
uint64_t owner_id)
int
rte_eth_dev_owner_new(uint64_t *owner_id)
{
+ if (owner_id == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot return owner id\n");
+ return -EINVAL;
+ }
+
eth_dev_shared_data_prepare();
rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
@@ -645,6 +670,13 @@ eth_dev_owner_set(const uint16_t port_id, const uint64_t
old_owner_id,
return -ENODEV;
}
+ if (new_owner == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot set ethdev port %u owner to NULL new_owner\n",
+ port_id);
+ return -EINVAL;
+ }
+
if (!eth_is_valid_owner_id(new_owner->id) &&
!eth_is_valid_owner_id(old_owner_id)) {
RTE_ETHDEV_LOG(ERR,
@@ -738,23 +770,30 @@ rte_eth_dev_owner_delete(const uint64_t owner_id)
int
rte_eth_dev_owner_get(const uint16_t port_id, struct rte_eth_dev_owner *owner)
{
- int ret = 0;
- struct rte_eth_dev *ethdev = &rte_eth_devices[port_id];
-
- eth_dev_shared_data_prepare();
-
- rte_spinlock_lock(ð_dev_shared_data->ownership_lock);
+ struct rte_eth_dev *ethdev;
- if (port_id >= RTE_MAX_ETHPORTS || !eth_dev_is_allocated(ethdev)) {
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ ethdev = &rte_eth_devices[port_id];
+ if (!eth_dev_is_allocated(ethdev)) {
RTE_ETHDEV_LOG(ERR, "Port id %"PRIu16" is not allocated\n",
port_id);
- ret = -ENODEV;
- } else {
- rte_memcpy(owner, ðdev->data->owner, sizeof(*owner));
+ return -ENODEV;
}
+ if (owner == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot g
Re: [dpdk-dev] [PATCH v7] ethdev: add sanity checks in control APIs
在 2021/4/16 20:02, Thomas Monjalon 写道:
16/04/2021 13:00, Min Hu (Connor):
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot init iterator for NULL iterator\n");
Don't you think it would be better as
"Cannot init NULL iterator"?
rte_eth_iterator_cleanup(struct rte_dev_iterator *iter)
{
+ if (iter == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot iterator clear up for NULL iter\n");
Cannot clean up NULL iterator.
rte_eth_dev_owner_new(uint64_t *owner_id)
{
+ if (owner_id == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot get owner id for NULL param\n");
Cannot return owner id.
@@ -825,6 +864,11 @@ rte_eth_dev_get_port_by_name(const char *name, uint16_t
*port_id)
+ if (port_id == NULL) {
+ RTE_ETHDEV_LOG(ERR, "Cannot get port id for NULL param\n");
Cannot return port ID
etc for all other messages
Do you agree with the idea?
Hi， Thomos, fixed in v10, thanks.
.
Re: [dpdk-dev] [PATCH v6] ethdev: add sanity checks in control APIs
在 2021/4/17 0:28, Stephen Hemminger 写道:
On Fri, 16 Apr 2021 11:22:02 +0100
Kevin Traynor wrote:
+ if (dev_conf == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot configure ethdev port %u to NULL dev_conf\n",
The others use a natural sounding names instead of argument name. If you
wanted to match that it could be "..to NULL conf"
I would prefer that error messages don't try to be English sentences.
The wording ends up awkward. and overly wordy.
If function name is automatically included by RTE_ETHDEV_LOG() then
Just:
RTE_ETHDEV_LOG(ERR, "NULL ethdev")
should be enough for programmer to find/fix the problem
.
Hi, Stephen, thanks your for comment.
But I am on side with Andrew Rybchenko.
"- log messages should be human readable (i.e. I'd avoid
usage of function name)"
[dpdk-dev] [PATCH v2 3/7] vdpa/ifc: support set notify and vring relay thread name
From: Chengwen Feng
This patch supports set notify and vring relay thread name which is
helpful for debugging.
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
drivers/vdpa/ifc/ifcvf_vdpa.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
index 7a06f97..1dc813d 100644
--- a/drivers/vdpa/ifc/ifcvf_vdpa.c
+++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
@@ -37,6 +37,8 @@ RTE_LOG_REGISTER(ifcvf_vdpa_logtype, pmd.vdpa.ifcvf, NOTICE);
#define IFCVF_VDPA_MODE"vdpa"
#define IFCVF_SW_FALLBACK_LM "sw-live-migration"
+#define THREAD_NAME_LEN16
+
static const char * const ifcvf_valid_arguments[] = {
IFCVF_VDPA_MODE,
IFCVF_SW_FALLBACK_LM,
@@ -494,14 +496,17 @@ notify_relay(void *arg)
static int
setup_notify_relay(struct ifcvf_internal *internal)
{
+ char name[THREAD_NAME_LEN];
int ret;
- ret = pthread_create(&internal->tid, NULL, notify_relay,
- (void *)internal);
- if (ret) {
+ snprintf(name, sizeof(name), "ifc-notify-%d", internal->vid);
+ ret = rte_ctrl_thread_create(&internal->tid, name, NULL, notify_relay,
+(void *)internal);
+ if (ret != 0) {
DRV_LOG(ERR, "failed to create notify relay pthread.");
return -1;
}
+
return 0;
}
@@ -797,14 +802,17 @@ vring_relay(void *arg)
static int
setup_vring_relay(struct ifcvf_internal *internal)
{
+ char name[THREAD_NAME_LEN];
int ret;
- ret = pthread_create(&internal->tid, NULL, vring_relay,
- (void *)internal);
- if (ret) {
+ snprintf(name, sizeof(name), "ifc-vring-%d", internal->vid);
+ ret = rte_ctrl_thread_create(&internal->tid, name, NULL, vring_relay,
+(void *)internal);
+ if (ret != 0) {
DRV_LOG(ERR, "failed to create ring relay pthread.");
return -1;
}
+
return 0;
}
--
2.7.4
[dpdk-dev] [PATCH v2 5/7] examples/performance-thread: support set thread name
From: Chengwen Feng This patch supports set helloworld thread name which is helpful for debugging. Signed-off-by: Chengwen Feng Signed-off-by: Min Hu (Connor) --- examples/performance-thread/pthread_shim/main.c | 5 + 1 file changed, 5 insertions(+) diff --git a/examples/performance-thread/pthread_shim/main.c b/examples/performance-thread/pthread_shim/main.c index 23e3b5e..4ce3622 100644 --- a/examples/performance-thread/pthread_shim/main.c +++ b/examples/performance-thread/pthread_shim/main.c @@ -27,6 +27,7 @@ #define DEBUG_APP 0 #define HELLOW_WORLD_MAX_LTHREADS 10 +#define THREAD_NAME_LEN16 #ifndef __GLIBC__ /* sched_getcpu() is glibc-specific */ #define sched_getcpu() rte_lcore_id() @@ -149,6 +150,7 @@ static void *initial_lthread(void *args __rte_unused) */ pthread_attr_t attr; rte_cpuset_t cpuset; + char name[THREAD_NAME_LEN]; CPU_ZERO(&cpuset); CPU_SET(lcore, &cpuset); @@ -160,6 +162,9 @@ static void *initial_lthread(void *args __rte_unused) helloworld_pthread, (void *) i); if (ret != 0) rte_exit(EXIT_FAILURE, "Cannot create helloworld thread\n"); + + snprintf(name, sizeof(name), "helloworld-%u", (uint32_t)i); + rte_thread_setname(tid[i], name); } /* wait for 1s to allow threads -- 2.7.4
[dpdk-dev] [PATCH v2 0/7] support set thread name
This set of patches support set thread name for debugging. Chengwen Feng (7): net/ark: support set thread name net/ice: support set VSI reset thread name vdpa/ifc: support set notify and vring relay thread name raw/ifpga: support set monitor thread name examples/performance-thread: support set thread name telemetry: support set init threads name examples/vhost_blk: support set ctrl worker thread name --- v2: * change 'pthread_create' to 'rte_ctrl_thread_create' except two: 'examples/performance-thread', because special CPU affinity needs to be set, the ctrl interface cannot be used to set the affinity. 'telemetry', telemetry is an independent lib and does not depend on EAL. drivers/net/ark/ark_ethdev.c| 4 ++-- drivers/net/ice/ice_dcf_parent.c| 9 ++--- drivers/raw/ifpga/ifpga_rawdev.c| 8 drivers/vdpa/ifc/ifcvf_vdpa.c | 20 ++-- examples/performance-thread/pthread_shim/main.c | 5 + examples/vhost_blk/vhost_blk.c | 3 ++- lib/librte_telemetry/telemetry.c| 3 ++- 7 files changed, 35 insertions(+), 17 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH v2 7/7] examples/vhost_blk: support set ctrl worker thread name
From: Chengwen Feng
This patch supports set ctrl worker thread name which is helpful for
debugging.
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
examples/vhost_blk/vhost_blk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/examples/vhost_blk/vhost_blk.c b/examples/vhost_blk/vhost_blk.c
index 5c64071..54f81b3 100644
--- a/examples/vhost_blk/vhost_blk.c
+++ b/examples/vhost_blk/vhost_blk.c
@@ -685,7 +685,8 @@ new_device(int vid)
/* start polling vring */
worker_thread_status = WORKER_STATE_START;
fprintf(stdout, "New Device %s, Device ID %d\n", path, vid);
- if (pthread_create(&tid, NULL, &ctrlr_worker, ctrlr) < 0) {
+ if (rte_ctrl_thread_create(&tid, "vhostblk-ctrlr", NULL,
+ &ctrlr_worker, ctrlr) != 0) {
fprintf(stderr, "Worker Thread Started Failed\n");
return -1;
}
--
2.7.4
[dpdk-dev] [PATCH v2 6/7] telemetry: support set init threads name
From: Chengwen Feng This patch supports set init threads name which is helpful for debugging. Signed-off-by: Chengwen Feng Signed-off-by: Min Hu (Connor) --- lib/librte_telemetry/telemetry.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/librte_telemetry/telemetry.c b/lib/librte_telemetry/telemetry.c index 7e08afd..8f48337 100644 --- a/lib/librte_telemetry/telemetry.c +++ b/lib/librte_telemetry/telemetry.c @@ -442,7 +442,7 @@ telemetry_legacy_init(void) return -1; pthread_create(&t_old, NULL, socket_listener, &v1_socket); pthread_setaffinity_np(t_old, sizeof(*thread_cpuset), thread_cpuset); - + pthread_setname_np(t_old, "telemetry-v1"); TMTY_LOG(DEBUG, "Legacy telemetry socket initialized ok\n"); return 0; } @@ -471,6 +471,7 @@ telemetry_v2_init(void) return -1; pthread_create(&t_new, NULL, socket_listener, &v2_socket); pthread_setaffinity_np(t_new, sizeof(*thread_cpuset), thread_cpuset); + pthread_setname_np(t_new, "telemetry-v2"); atexit(unlink_sockets); return 0; -- 2.7.4
[dpdk-dev] [PATCH v2 1/7] net/ark: support set thread name
From: Chengwen Feng
This patch supports set delay packet generator start thread name which
is helpful for debugging.
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
drivers/net/ark/ark_ethdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c
index 9dea5fa..6372cf7 100644
--- a/drivers/net/ark/ark_ethdev.c
+++ b/drivers/net/ark/ark_ethdev.c
@@ -568,8 +568,8 @@ eth_ark_dev_start(struct rte_eth_dev *dev)
/* Delay packet generatpr start allow the hardware to be ready
* This is only used for sanity checking with internal generator
*/
- if (pthread_create(&thread, NULL,
- ark_pktgen_delay_start, ark->pg)) {
+ if (rte_ctrl_thread_create(&thread, "ark-delay-pg", NULL,
+ ark_pktgen_delay_start, ark) != 0) {
ARK_PMD_LOG(ERR, "Could not create pktgen "
"starter thread\n");
return -1;
--
2.7.4
[dpdk-dev] [PATCH v2 4/7] raw/ifpga: support set monitor thread name
From: Chengwen Feng
This patch supports set monitor thread name which is helpful for
debugging.
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
drivers/raw/ifpga/ifpga_rawdev.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/raw/ifpga/ifpga_rawdev.c b/drivers/raw/ifpga/ifpga_rawdev.c
index d9a46ef..f2551be 100644
--- a/drivers/raw/ifpga/ifpga_rawdev.c
+++ b/drivers/raw/ifpga/ifpga_rawdev.c
@@ -526,10 +526,10 @@ ifpga_monitor_start_func(void)
int ret;
if (ifpga_monitor_start == 0) {
- ret = pthread_create(&ifpga_monitor_start_thread,
- NULL,
- ifpga_rawdev_gsd_handle, NULL);
- if (ret) {
+ ret = rte_ctrl_thread_create(&ifpga_monitor_start_thread,
+"ifpga-monitor", NULL,
+ifpga_rawdev_gsd_handle, NULL);
+ if (ret != 0) {
IFPGA_RAWDEV_PMD_ERR(
"Fail to create ifpga nonitor thread");
return -1;
--
2.7.4
[dpdk-dev] [PATCH v2 2/7] net/ice: support set VSI reset thread name
From: Chengwen Feng
This patch supports set VSI reset thread name which is helpful for
debugging.
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
drivers/net/ice/ice_dcf_parent.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ice/ice_dcf_parent.c b/drivers/net/ice/ice_dcf_parent.c
index a8571b3..c8e4332 100644
--- a/drivers/net/ice/ice_dcf_parent.c
+++ b/drivers/net/ice/ice_dcf_parent.c
@@ -151,7 +151,9 @@ ice_dcf_vsi_update_service_handler(void *param)
static void
start_vsi_reset_thread(struct ice_dcf_hw *dcf_hw, bool vfr, uint16_t vf_id)
{
+#define THREAD_NAME_LEN16
struct ice_dcf_reset_event_param *param;
+ char name[THREAD_NAME_LEN];
pthread_t thread;
int ret;
@@ -165,9 +167,10 @@ start_vsi_reset_thread(struct ice_dcf_hw *dcf_hw, bool
vfr, uint16_t vf_id)
param->vfr = vfr;
param->vf_id = vf_id;
- ret = pthread_create(&thread, NULL,
-ice_dcf_vsi_update_service_handler, param);
- if (ret) {
+ snprintf(name, sizeof(name), "ice-reset-%u", vf_id);
+ ret = rte_ctrl_thread_create(&thread, name, NULL,
+ice_dcf_vsi_update_service_handler, param);
+ if (ret != 0) {
PMD_DRV_LOG(ERR, "Failed to start the thread for reset
handling");
free(param);
}
--
2.7.4
[dpdk-dev] [PATCH 0/7] features and bugfix for hns3 PMD
This patchset contains 2 features and 5 bugfixes. Chengwen Feng (3): net/hns3: check max SIMD bitwidth net/hns3: Rx vector add compile-time verify net/hns3: fix FDIR lock bug Huisong Li (4): net/hns3: remove redundant code about mbx net/hns3: fix the check for DCB mode net/hns3: fix the check for VMDq mode net/hns3: move the link speeds check to dev configure drivers/net/hns3/hns3_ethdev.c| 89 +--- drivers/net/hns3/hns3_ethdev.h| 4 ++ drivers/net/hns3/hns3_ethdev_vf.c | 9 ++-- drivers/net/hns3/hns3_fdir.c | 28 +- drivers/net/hns3/hns3_fdir.h | 3 +- drivers/net/hns3/hns3_flow.c | 96 --- drivers/net/hns3/hns3_rxtx.c | 5 ++ drivers/net/hns3/hns3_rxtx_vec.c | 22 drivers/net/hns3/hns3_rxtx_vec_neon.h | 8 +++ drivers/net/hns3/hns3_rxtx_vec_sve.c | 6 +++ 10 files changed, 211 insertions(+), 59 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH 3/7] net/hns3: remove redundant code about mbx
From: Huisong Li
Some mbx messages do not need to reply with data. In this case,
it is no need to set the response data address and the response
length.
This patch removes these redundant codes from mbx messages that do
not need be replied.
Fixes: a5475d61fa34 ("net/hns3: support VF")
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_ethdev_vf.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c
b/drivers/net/hns3/hns3_ethdev_vf.c
index 5770c47..58809fb 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -1511,7 +1511,6 @@ static void
hns3vf_request_link_info(struct hns3_hw *hw)
{
struct hns3_vf *vf = HNS3_DEV_HW_TO_VF(hw);
- uint8_t resp_msg;
bool send_req;
int ret;
@@ -1524,7 +1523,7 @@ hns3vf_request_link_info(struct hns3_hw *hw)
return;
ret = hns3_send_mbx_msg(hw, HNS3_MBX_GET_LINK_STATUS, 0, NULL, 0, false,
- &resp_msg, sizeof(resp_msg));
+ NULL, 0);
if (ret) {
hns3_err(hw, "failed to fetch link status, ret = %d", ret);
return;
@@ -1756,11 +1755,10 @@ hns3vf_keep_alive_handler(void *param)
struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)param;
struct hns3_adapter *hns = eth_dev->data->dev_private;
struct hns3_hw *hw = &hns->hw;
- uint8_t respmsg;
int ret;
ret = hns3_send_mbx_msg(hw, HNS3_MBX_KEEP_ALIVE, 0, NULL, 0,
- false, &respmsg, sizeof(uint8_t));
+ false, NULL, 0);
if (ret)
hns3_err(hw, "VF sends keeping alive cmd failed(=%d)",
ret);
--
2.7.4
[dpdk-dev] [PATCH 7/7] net/hns3: move the link speeds check to dev configure
From: Huisong Li
This patch moves the check for "link_speeds" in dev_conf to dev_configure,
so that users know whether "link_speeds" is valid in advance.
Fixes: 0d90a6b8e59f ("net/hns3: support link speed autoneg for PF")
Fixes: 30b08275cb87 ("net/hns3: support fixed link speed")
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_ethdev.c | 57 +-
1 file changed, 45 insertions(+), 12 deletions(-)
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index bcbebd0..a3ea513 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -105,6 +105,7 @@ static int hns3_remove_mc_addr(struct hns3_hw *hw,
static int hns3_restore_fec(struct hns3_hw *hw);
static int hns3_query_dev_fec_info(struct hns3_hw *hw);
static int hns3_do_stop(struct hns3_adapter *hns);
+static int hns3_check_port_speed(struct hns3_hw *hw, uint32_t link_speeds);
void hns3_ether_format_addr(char *buf, uint16_t size,
const struct rte_ether_addr *ether_addr)
@@ -2431,6 +2432,46 @@ hns3_refresh_mtu(struct rte_eth_dev *dev, struct
rte_eth_conf *conf)
}
static int
+hns3_check_link_speed(struct hns3_hw *hw, uint32_t link_speeds)
+{
+ int ret;
+
+ /*
+* Some hardware doesn't support auto-negotiation, but users may not
+* configure link_speeds (default 0), which means auto-negotiation.
+* In this case, a warning message need to be printed, instead of
+* an error.
+*/
+ if (link_speeds == ETH_LINK_SPEED_AUTONEG &&
+ hw->mac.support_autoneg == 0) {
+ hns3_warn(hw, "auto-negotiation is not supported, use default
fixed speed!");
+ return 0;
+ }
+
+ if (link_speeds != ETH_LINK_SPEED_AUTONEG) {
+ ret = hns3_check_port_speed(hw, link_speeds);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int
+hns3_check_dev_conf(struct rte_eth_dev *dev)
+{
+ struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ struct rte_eth_conf *conf = &dev->data->dev_conf;
+ int ret;
+
+ ret = hns3_check_mq_mode(dev);
+ if (ret)
+ return ret;
+
+ return hns3_check_link_speed(hw, conf->link_speeds);
+}
+
+static int
hns3_dev_configure(struct rte_eth_dev *dev)
{
struct hns3_adapter *hns = dev->data->dev_private;
@@ -2465,7 +2506,7 @@ hns3_dev_configure(struct rte_eth_dev *dev)
}
hw->adapter_state = HNS3_NIC_CONFIGURING;
- ret = hns3_check_mq_mode(dev);
+ ret = hns3_check_dev_conf(dev);
if (ret)
goto cfg_err;
@@ -5466,14 +5507,11 @@ hns3_set_fiber_port_link_speed(struct hns3_hw *hw,
/*
* Some hardware doesn't support auto-negotiation, but users may not
-* configure link_speeds (default 0), which means auto-negotiation
-* In this case, a warning message need to be printed, instead of
-* an error.
+* configure link_speeds (default 0), which means auto-negotiation.
+* In this case, it should return success.
*/
- if (cfg->autoneg) {
- hns3_warn(hw, "auto-negotiation is not supported.");
+ if (cfg->autoneg)
return 0;
- }
return hns3_cfg_mac_speed_dup(hw, cfg->speed, cfg->duplex);
}
@@ -5514,16 +5552,11 @@ hns3_apply_link_speed(struct hns3_hw *hw)
{
struct rte_eth_conf *conf = &hw->data->dev_conf;
struct hns3_set_link_speed_cfg cfg;
- int ret;
memset(&cfg, 0, sizeof(struct hns3_set_link_speed_cfg));
cfg.autoneg = (conf->link_speeds == ETH_LINK_SPEED_AUTONEG) ?
ETH_LINK_AUTONEG : ETH_LINK_FIXED;
if (cfg.autoneg != ETH_LINK_AUTONEG) {
- ret = hns3_check_port_speed(hw, conf->link_speeds);
- if (ret)
- return ret;
-
cfg.speed = hns3_get_link_speed(conf->link_speeds);
cfg.duplex = hns3_get_link_duplex(conf->link_speeds);
}
--
2.7.4
[dpdk-dev] [PATCH 2/7] net/hns3: Rx vector add compile-time verify
From: Chengwen Feng
According the Rx vector implement, it depends on the MBUF's fields
(such as rearm_data/rx_descriptor_fields1) layout, this patch adds
compile-time verifies with this.
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_rxtx_vec.c | 22 ++
drivers/net/hns3/hns3_rxtx_vec_neon.h | 8
drivers/net/hns3/hns3_rxtx_vec_sve.c | 6 ++
3 files changed, 36 insertions(+)
diff --git a/drivers/net/hns3/hns3_rxtx_vec.c b/drivers/net/hns3/hns3_rxtx_vec.c
index 08a86e0..dc1e1ae 100644
--- a/drivers/net/hns3/hns3_rxtx_vec.c
+++ b/drivers/net/hns3/hns3_rxtx_vec.c
@@ -147,6 +147,28 @@ hns3_rxq_vec_setup_rearm_data(struct hns3_rx_queue *rxq)
mb_def.port = rxq->port_id;
rte_mbuf_refcnt_set(&mb_def, 1);
+ /* compile-time verifies the rearm_data first 8bytes */
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, data_off) <
+offsetof(struct rte_mbuf, rearm_data));
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, refcnt) <
+offsetof(struct rte_mbuf, rearm_data));
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, refcnt) <
+offsetof(struct rte_mbuf, rearm_data));
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, nb_segs) <
+offsetof(struct rte_mbuf, rearm_data));
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, port) <
+offsetof(struct rte_mbuf, rearm_data));
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, data_off) -
+offsetof(struct rte_mbuf, rearm_data) > 6);
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, refcnt) -
+offsetof(struct rte_mbuf, rearm_data) > 6);
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, refcnt) -
+offsetof(struct rte_mbuf, rearm_data) > 6);
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, nb_segs) -
+offsetof(struct rte_mbuf, rearm_data) > 6);
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, port) -
+offsetof(struct rte_mbuf, rearm_data) > 6);
+
/* prevent compiler reordering: rearm_data covers previous fields */
rte_compiler_barrier();
p = (uintptr_t)&mb_def.rearm_data;
diff --git a/drivers/net/hns3/hns3_rxtx_vec_neon.h
b/drivers/net/hns3/hns3_rxtx_vec_neon.h
index 14d6fb0..69af7b3 100644
--- a/drivers/net/hns3/hns3_rxtx_vec_neon.h
+++ b/drivers/net/hns3/hns3_rxtx_vec_neon.h
@@ -161,6 +161,14 @@ hns3_recv_burst_vec(struct hns3_rx_queue *__restrict rxq,
0, 0, 0, /* ignore non-length fields */
};
+ /* compile-time verifies the shuffle mask */
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, pkt_len) !=
+offsetof(struct rte_mbuf, rx_descriptor_fields1) + 4);
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, data_len) !=
+offsetof(struct rte_mbuf, rx_descriptor_fields1) + 8);
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, hash.rss) !=
+offsetof(struct rte_mbuf, rx_descriptor_fields1) + 12);
+
for (pos = 0; pos < nb_pkts; pos += HNS3_DEFAULT_DESCS_PER_LOOP,
rxdp += HNS3_DEFAULT_DESCS_PER_LOOP) {
uint64x2x2_t descs[HNS3_DEFAULT_DESCS_PER_LOOP];
diff --git a/drivers/net/hns3/hns3_rxtx_vec_sve.c
b/drivers/net/hns3/hns3_rxtx_vec_sve.c
index 2eaf692..1fd87ca 100644
--- a/drivers/net/hns3/hns3_rxtx_vec_sve.c
+++ b/drivers/net/hns3/hns3_rxtx_vec_sve.c
@@ -122,6 +122,12 @@ hns3_recv_burst_vec_sve(struct hns3_rx_queue *__restrict
rxq,
svuint32_t rss_tbl1 = svld1_u32(PG32_256BIT, rss_adjust);
svuint32_t rss_tbl2 = svld1_u32(PG32_256BIT, &rss_adjust[8]);
+ /* compile-time verifies the xlen_adjust mask */
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, data_len) !=
+offsetof(struct rte_mbuf, pkt_len) + 4);
+ RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, vlan_tci) !=
+offsetof(struct rte_mbuf, data_len) + 2);
+
for (pos = 0; pos < nb_pkts; pos += HNS3_SVE_DEFAULT_DESCS_PER_LOOP,
rxdp += HNS3_SVE_DEFAULT_DESCS_PER_LOOP) {
svuint64_t vld_clz, mbp1st, mbp2st, mbuf_init;
--
2.7.4
[dpdk-dev] [PATCH 1/7] net/hns3: check max SIMD bitwidth
From: Chengwen Feng
This patch supports check max SIMD bitwidth when choosing NEON and SVE
vector path.
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_rxtx.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 9bb30fc..f2022a5 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -13,6 +13,7 @@
#include
#if defined(RTE_ARCH_ARM64)
#include
+#include
#endif
#include "hns3_ethdev.h"
@@ -2800,6 +2801,8 @@ static bool
hns3_get_default_vec_support(void)
{
#if defined(RTE_ARCH_ARM64)
+ if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_128)
+ return false;
if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON))
return true;
#endif
@@ -2810,6 +2813,8 @@ static bool
hns3_get_sve_support(void)
{
#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
+ if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
+ return false;
if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
return true;
#endif
--
2.7.4
[dpdk-dev] [PATCH 5/7] net/hns3: fix the check for VMDq mode
From: Huisong Li
HNS3 PF driver only supports RSS, DCB or NONE multiple queues mode.
Currently, driver doesn't verify the VMDq multi-queue mode completely.
This patch fixes the verification for VMDq mode.
Fixes: 62e3ccc2b94c ("net/hns3: support flow control")
Cc: sta...@dpdk.org
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_ethdev.c | 28
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index b705a72..c276c6b 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2224,23 +2224,16 @@ hns3_check_mq_mode(struct rte_eth_dev *dev)
int max_tc = 0;
int i;
- dcb_rx_conf = &dev->data->dev_conf.rx_adv_conf.dcb_rx_conf;
- dcb_tx_conf = &dev->data->dev_conf.tx_adv_conf.dcb_tx_conf;
-
- if (rx_mq_mode == ETH_MQ_RX_VMDQ_DCB_RSS) {
- hns3_err(hw, "ETH_MQ_RX_VMDQ_DCB_RSS is not supported. "
-"rx_mq_mode = %d", rx_mq_mode);
- return -EINVAL;
- }
-
- if (rx_mq_mode == ETH_MQ_RX_VMDQ_DCB ||
- tx_mq_mode == ETH_MQ_TX_VMDQ_DCB) {
- hns3_err(hw, "ETH_MQ_RX_VMDQ_DCB and ETH_MQ_TX_VMDQ_DCB "
-"is not supported. rx_mq_mode = %d, tx_mq_mode = %d",
+ if ((rx_mq_mode & ETH_MQ_RX_VMDQ_FLAG) ||
+ (tx_mq_mode == ETH_MQ_TX_VMDQ_DCB ||
+tx_mq_mode == ETH_MQ_TX_VMDQ_ONLY)) {
+ hns3_err(hw, "VMDQ is not supported, rx_mq_mode = %d,
tx_mq_mode = %d.",
rx_mq_mode, tx_mq_mode);
- return -EINVAL;
+ return -EOPNOTSUPP;
}
+ dcb_rx_conf = &dev->data->dev_conf.rx_adv_conf.dcb_rx_conf;
+ dcb_tx_conf = &dev->data->dev_conf.tx_adv_conf.dcb_tx_conf;
if (rx_mq_mode & ETH_MQ_RX_DCB_FLAG) {
if (dcb_rx_conf->nb_tcs > pf->tc_max) {
hns3_err(hw, "nb_tcs(%u) > max_tc(%u) driver
supported.",
@@ -2299,8 +2292,7 @@ hns3_check_dcb_cfg(struct rte_eth_dev *dev)
return -EOPNOTSUPP;
}
- /* Check multiple queue mode */
- return hns3_check_mq_mode(dev);
+ return 0;
}
static int
@@ -2473,6 +2465,10 @@ hns3_dev_configure(struct rte_eth_dev *dev)
}
hw->adapter_state = HNS3_NIC_CONFIGURING;
+ ret = hns3_check_mq_mode(dev);
+ if (ret)
+ goto cfg_err;
+
if ((uint32_t)mq_mode & ETH_MQ_RX_DCB_FLAG) {
ret = hns3_check_dcb_cfg(dev);
if (ret)
--
2.7.4
[dpdk-dev] [PATCH 6/7] net/hns3: fix FDIR lock bug
From: Chengwen Feng
Currently, the fdir lock was used to protect concurrent access in
multiple processes, it has the following problems:
1) Lack of protection for fdir reset recover.
2) Only part data is protected, eg. the filterlist is not protected.
We use the following scheme:
1) Del the fdir lock.
2) Add a flow lock and provides rte flow driver ops API-level
protection.
3) Declare support RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE.
Fixes: fcba820d9b9e ("net/hns3: support flow director")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_ethdev.c| 4 +-
drivers/net/hns3/hns3_ethdev.h| 4 ++
drivers/net/hns3/hns3_ethdev_vf.c | 3 +-
drivers/net/hns3/hns3_fdir.c | 28 ++--
drivers/net/hns3/hns3_fdir.h | 3 +-
drivers/net/hns3/hns3_flow.c | 96 ---
6 files changed, 111 insertions(+), 27 deletions(-)
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index c276c6b..bcbebd0 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -7334,8 +7334,8 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
PMD_INIT_LOG(ERR, "Failed to alloc memory for process private");
return -ENOMEM;
}
- /* initialize flow filter lists */
- hns3_filterlist_init(eth_dev);
+
+ hns3_flow_init(eth_dev);
hns3_set_rxtx_function(eth_dev);
eth_dev->dev_ops = &hns3_eth_dev_ops;
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 7b7d359..b1360cb 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -5,6 +5,7 @@
#ifndef _HNS3_ETHDEV_H_
#define _HNS3_ETHDEV_H_
+#include
#include
#include
#include
@@ -613,6 +614,9 @@ struct hns3_hw {
uint8_t udp_cksum_mode;
struct hns3_port_base_vlan_config port_base_vlan_cfg;
+
+ pthread_mutex_t flows_lock; /* rte_flow ops lock */
+
/*
* PMD setup and configuration is not thread safe. Since it is not
* performance sensitive, it is better to guarantee thread-safety
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c
b/drivers/net/hns3/hns3_ethdev_vf.c
index 58809fb..cf2b74e 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2910,8 +2910,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
return -ENOMEM;
}
- /* initialize flow filter lists */
- hns3_filterlist_init(eth_dev);
+ hns3_flow_init(eth_dev);
hns3_set_rxtx_function(eth_dev);
eth_dev->dev_ops = &hns3vf_eth_dev_ops;
diff --git a/drivers/net/hns3/hns3_fdir.c b/drivers/net/hns3/hns3_fdir.c
index 603cc82..87c1aef 100644
--- a/drivers/net/hns3/hns3_fdir.c
+++ b/drivers/net/hns3/hns3_fdir.c
@@ -830,7 +830,6 @@ int hns3_fdir_filter_init(struct hns3_adapter *hns)
fdir_hash_params.socket_id = rte_socket_id();
TAILQ_INIT(&fdir_info->fdir_list);
- rte_spinlock_init(&fdir_info->flows_lock);
snprintf(fdir_hash_name, RTE_HASH_NAMESIZE, "%s", hns->hw.data->name);
fdir_info->hash_handle = rte_hash_create(&fdir_hash_params);
if (fdir_info->hash_handle == NULL) {
@@ -856,7 +855,6 @@ void hns3_fdir_filter_uninit(struct hns3_adapter *hns)
struct hns3_fdir_info *fdir_info = &pf->fdir;
struct hns3_fdir_rule_ele *fdir_filter;
- rte_spinlock_lock(&fdir_info->flows_lock);
if (fdir_info->hash_map) {
rte_free(fdir_info->hash_map);
fdir_info->hash_map = NULL;
@@ -865,7 +863,6 @@ void hns3_fdir_filter_uninit(struct hns3_adapter *hns)
rte_hash_free(fdir_info->hash_handle);
fdir_info->hash_handle = NULL;
}
- rte_spinlock_unlock(&fdir_info->flows_lock);
fdir_filter = TAILQ_FIRST(&fdir_info->fdir_list);
while (fdir_filter) {
@@ -891,10 +888,8 @@ static int hns3_fdir_filter_lookup(struct hns3_fdir_info
*fdir_info,
hash_sig_t sig;
int ret;
- rte_spinlock_lock(&fdir_info->flows_lock);
sig = rte_hash_crc(key, sizeof(*key), 0);
ret = rte_hash_lookup_with_hash(fdir_info->hash_handle, key, sig);
- rte_spinlock_unlock(&fdir_info->flows_lock);
return ret;
}
@@ -908,11 +903,9 @@ static int hns3_insert_fdir_filter(struct hns3_hw *hw,
int ret;
key = &fdir_filter->fdir_conf.key_conf;
- rte_spinlock_lock(&fdir_info->flows_lock);
sig = rte_hash_crc(key, sizeof(*key), 0);
ret = rte_hash_add_key_with_hash(fdir_info->hash_handle, key, sig);
if (ret < 0) {
- rte_spinlock_unlock(&fdir_info->flows_lock);
hns3_err(hw, "Hash table full? err:%d(%s)!", ret,
[dpdk-dev] [PATCH 4/7] net/hns3: fix the check for DCB mode
From: Huisong Li
Currently, "ONLY DCB" and "DCB+RSS" mode are both supported by HNS3
PF driver. But the driver verifies only the "DCB+RSS" multiple queues
mode.
Fixes: 62e3ccc2b94c ("net/hns3: support flow control")
Cc: sta...@dpdk.org
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 1832d26..b705a72 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2241,7 +2241,7 @@ hns3_check_mq_mode(struct rte_eth_dev *dev)
return -EINVAL;
}
- if (rx_mq_mode == ETH_MQ_RX_DCB_RSS) {
+ if (rx_mq_mode & ETH_MQ_RX_DCB_FLAG) {
if (dcb_rx_conf->nb_tcs > pf->tc_max) {
hns3_err(hw, "nb_tcs(%u) > max_tc(%u) driver
supported.",
dcb_rx_conf->nb_tcs, pf->tc_max);
--
2.7.4
Re: [dpdk-dev] [PATCH v6] ethdev: add sanity checks in control APIs
在 2021/4/18 5:37, Thomas Monjalon 写道:
17/04/2021 02:28, Min Hu (Connor):
在 2021/4/17 0:28, Stephen Hemminger 写道:
On Fri, 16 Apr 2021 11:22:02 +0100
Kevin Traynor wrote:
+ if (dev_conf == NULL) {
+ RTE_ETHDEV_LOG(ERR,
+ "Cannot configure ethdev port %u to NULL dev_conf\n",
The others use a natural sounding names instead of argument name. If you
wanted to match that it could be "..to NULL conf"
I would prefer that error messages don't try to be English sentences.
The wording ends up awkward. and overly wordy.
If function name is automatically included by RTE_ETHDEV_LOG() then
Just:
RTE_ETHDEV_LOG(ERR, "NULL ethdev")
should be enough for programmer to find/fix the problem
.
Hi, Stephen,
Your opinion is quit different from that of Andrew Rybchenko.
Andrew does not support show function name in the log:
"- log messages should be human readable (i.e. I'd avoid
usage of function name)"
@Andrew ,@Thoms, @Ferruh, @Kevin, so, what's your opinion ?
I prefer human readable messages which are unique enough to be "grepped".
Agree with Thomas, thanks.
.
[dpdk-dev] [PATCH v11] app/testpmd: support multi-process
This patch adds multi-process support for testpmd.
The test cmd example as follows:
the primary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=0
the secondary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=1
Signed-off-by: Min Hu (Connor)
Signed-off-by: Lijun Ou
Acked-by: Xiaoyun Li
Acked-by: Ajit Khaparde
Reviewed-by: Ferruh Yigit
---
v11:
* Fixed some minor syntax.
v10:
* Hid process type checks behind new functions.
* Added comments.
v9:
* Updated release notes and rst doc.
* Deleted deprecated codes.
* move macro and variable.
v8:
* Added warning info about queue numbers and process numbers.
v7:
* Fixed compiling error for unexpected unindent.
v6:
* Add rte flow description for multiple process.
v5:
* Fixed run_app.rst for multiple process description.
* Fix compiling error.
v4:
* Fixed minimum vlaue of Rxq or Txq in doc.
v3:
* Fixed compiling error using gcc10.0.
v2:
* Added document for this patch.
---
app/test-pmd/cmdline.c | 6 ++
app/test-pmd/config.c | 21 +-
app/test-pmd/parameters.c | 11 +++
app/test-pmd/testpmd.c | 129 ++---
app/test-pmd/testpmd.h | 9 +++
doc/guides/rel_notes/release_21_05.rst | 1 +
doc/guides/testpmd_app_ug/run_app.rst | 86 ++
7 files changed, 236 insertions(+), 27 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5bf1497..e465824 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -5354,6 +5354,12 @@ cmd_set_flush_rx_parsed(void *parsed_result,
__rte_unused void *data)
{
struct cmd_set_flush_rx *res = parsed_result;
+
+ if (num_procs > 1 && (strcmp(res->mode, "on") == 0)) {
+ printf("multi-process doesn't support to flush rx queues.\n");
+ return;
+ }
+
no_flush_rx = (uint8_t)((strcmp(res->mode, "on") == 0) ? 0 : 1);
}
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index d4b0e85..5d91335 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2859,6 +2859,8 @@ rss_fwd_config_setup(void)
queueid_t rxq;
queueid_t nb_q;
streamid_t sm_id;
+ int start;
+ int end;
nb_q = nb_rxq;
if (nb_q > nb_txq)
@@ -2876,7 +2878,22 @@ rss_fwd_config_setup(void)
init_fwd_streams();
setup_fwd_config_of_each_lcore(&cur_fwd_config);
- rxp = 0; rxq = 0;
+
+ if (proc_id > 0 && nb_q % num_procs)
+ printf("Warning! queue numbers should be multiple of "
+ "processes, or packet loss will happen.\n");
+
+ /**
+* In multi-process, All queues are allocated to different
+* processes based on num_procs and proc_id. For example:
+* if supports 4 queues(nb_q), 2 processes(num_procs),
+* the 0~1 queue for primary process.
+* the 2~3 queue for secondary process.
+*/
+ start = proc_id * nb_q / num_procs;
+ end = start + nb_q / num_procs;
+ rxp = 0;
+ rxq = start;
for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) {
struct fwd_stream *fs;
@@ -2893,6 +2910,8 @@ rss_fwd_config_setup(void)
continue;
rxp = 0;
rxq++;
+ if (rxq >= end)
+ rxq = start;
}
}
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index f3954c1..ece05c1 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -508,6 +508,9 @@ parse_link_speed(int n)
void
launch_args_parse(int argc, char** argv)
{
+#define PARAM_PROC_ID "proc-id"
+#define PARAM_NUM_PROCS "num-procs"
+
int n, opt;
char **argvopt;
int opt_idx;
@@ -625,6 +628,8 @@ launch_args_parse(int argc, char** argv)
{ "rx-mq-mode", 1, 0, 0 },
{ "record-core-cycles", 0, 0, 0 },
{ "record-burst-stats", 0, 0, 0 },
+ { PARAM_NUM_PROCS, 1, 0, 0 },
+ { PARAM_PROC_ID,1, 0, 0 },
{ 0, 0, 0, 0 },
};
@@ -1391,6 +1396,12 @@ launch_args_parse(int argc, char** argv)
record_core_cycles = 1;
if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
record_burst_stats = 1;
+ if (strncmp(lgopts[opt_idx].name,
+ PARAM_NUM_PROCS, 9) == 0)
+ num_procs = atoi(optarg);
+ if (strncmp(lgopts[opt_idx].name,
+
Re: [dpdk-dev] [PATCH v10] app/testpmd: support multi-process
Thanks Ferruh, fixed in v11, thanks.
在 2021/4/18 6:21, Ferruh Yigit 写道:
On 4/17/2021 7:12 AM, Min Hu (Connor) wrote:
This patch adds multi-process support for testpmd.
The test cmd example as follows:
the primary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 0-1 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=0
the secondary cmd:
./dpdk-testpmd -a xxx --proc-type=auto -l 2-3 -- -i \
--rxq=4 --txq=4 --num-procs=2 --proc-id=1
Signed-off-by: Min Hu (Connor)
Signed-off-by: Lijun Ou
Acked-by: Xiaoyun Li
Acked-by: Ajit Khaparde
Hi Connor,
I put some minor syntax comments, when they are fixed,
Reviewed-by: Ferruh Yigit
<...>
if (diag != 0) {
- if (rte_atomic16_cmpset(&(port->port_status),
- RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
- printf("Port %d can not be set back "
- "to stopped\n", pi);
- printf("Fail to configure port %d\n", pi);
+ if (rte_atomic16_cmpset(
+ &(port->port_status),
+ RTE_PORT_HANDLING,
+ RTE_PORT_STOPPED) == 0)
+ printf("Port %d can not be set "
+ "back to stopped\n", pi);
It is OK to have long line for the strings, so it can be like
printf("Port %d can not be set back to stopped\n",
pi);
+ printf("Fail to configure port %d\n",
+ pi);
No need to update this line.
<...>
/* Fail to setup rx queue, return */
if (rte_atomic16_cmpset(&(port->port_status),
- RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
+ RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
printf("Port %d can not be set back to "
- "stopped\n", pi);
+ "stopped\n",
+ pi);
These are syntax changes, I think can keep these lines as it is.
.
[dpdk-dev] [PATCH 1/2] app/testpmd: add link speed check before port start
From: Huisong Li
Currently, to check whether the configured link_speeds is valid, we
have to run "port start". In addition, if the configuration fails,
"port->dev_conf.link_speeds" maintained in testpmd cannot be
restored.
This patch adds the link_speeds check before port start by calling
dev_configure, and resolves these problems.
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
app/test-pmd/cmdline.c | 40 ++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5bf1497..09e30cf 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1537,8 +1537,12 @@ cmd_config_speed_all_parsed(void *parsed_result,
__rte_unused void *data)
{
struct cmd_config_speed_all *res = parsed_result;
+ uint32_t old_link_speeds[RTE_MAX_ETHPORTS];
+ struct rte_port *port;
uint32_t link_speed;
portid_t pid;
+ portid_t i;
+ int ret;
if (!all_ports_stopped()) {
printf("Please stop all ports first\n");
@@ -1550,7 +1554,26 @@ cmd_config_speed_all_parsed(void *parsed_result,
return;
RTE_ETH_FOREACH_DEV(pid) {
- ports[pid].dev_conf.link_speeds = link_speed;
+ port = &ports[pid];
+ old_link_speeds[pid] = port->dev_conf.link_speeds;
+ port->dev_conf.link_speeds = link_speed;
+ ret = rte_eth_dev_configure(pid, nb_rxq, nb_txq,
+ &port->dev_conf);
+ if (ret < 0) {
+ printf("Failed to check link speeds for port %d, ret =
%d.\n",
+ pid, ret);
+ goto roolback;
+ }
+ }
+
+ cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
+
+ return;
+
+roolback:
+ for (i = 0; i <= pid; i++) {
+ port = &ports[i];
+ port->dev_conf.link_speeds = old_link_speeds[i];
}
cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
@@ -1609,7 +1632,10 @@ cmd_config_speed_specific_parsed(void *parsed_result,
__rte_unused void *data)
{
struct cmd_config_speed_specific *res = parsed_result;
+ uint32_t old_link_speeds;
+ struct rte_port *port;
uint32_t link_speed;
+ int ret;
if (!all_ports_stopped()) {
printf("Please stop all ports first\n");
@@ -1623,7 +1649,17 @@ cmd_config_speed_specific_parsed(void *parsed_result,
&link_speed) < 0)
return;
- ports[res->id].dev_conf.link_speeds = link_speed;
+ port = &ports[res->id];
+ old_link_speeds = port->dev_conf.link_speeds;
+ port->dev_conf.link_speeds = link_speed;
+ ret = rte_eth_dev_configure(res->id, nb_rxq, nb_txq,
+ &port->dev_conf);
+ if (ret < 0) {
+ printf("Failed to check link speeds for port %d, ret = %d.\n",
+ res->id, ret);
+ port->dev_conf.link_speeds = old_link_speeds;
+ return;
+ }
cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1);
}
--
2.7.4
[dpdk-dev] [PATCH 0/2] fixes for link speed
This patchset contains fixes for link speed in testpmd. Huisong Li (2): app/testpmd: add link speed check before port start app/testpmd: fix link speed for a specified port app/test-pmd/cmdline.c | 48 +++-- 1 file changed, 42 insertions(+), 6 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH 2/2] app/testpmd: fix link speed for a specified port
From: Huisong Li
When we use the following cmd to modify the link speed of specified
port: "port config speed xxx duplex xxx", we have to stop
all ports. It's not necessary.
Fixes: 82113036e4e5 ("ethdev: redesign link speed config")
Cc: sta...@dpdk.org
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
app/test-pmd/cmdline.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 09e30cf..31884c5 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1637,13 +1637,13 @@ cmd_config_speed_specific_parsed(void *parsed_result,
uint32_t link_speed;
int ret;
- if (!all_ports_stopped()) {
- printf("Please stop all ports first\n");
+ if (port_id_is_invalid(res->id, ENABLED_WARN))
return;
- }
- if (port_id_is_invalid(res->id, ENABLED_WARN))
+ if (!port_is_stopped(res->id)) {
+ printf("Please stop port %d first\n", res->id);
return;
+ }
if (parse_and_check_speed_duplex(res->value1, res->value2,
&link_speed) < 0)
--
2.7.4
Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info
Hi,
在 2021/4/19 10:53, Li, Xiaoyun 写道:
Hi
-Original Message-
From: Min Hu (Connor)
Sent: Thursday, April 15, 2021 14:47
To: dev@dpdk.org
Cc: Yigit, Ferruh ; Li, Xiaoyun
Subject: [PATCH] app/testpmd: support the query of link flow ctrl info
From: Huisong Li
This patch supports the query of the link flow control parameter on a port.
The command format is as follows:
show port flow_ctrl
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
app/test-pmd/cmdline.c | 83 +
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 +++
2 files changed, 90 insertions(+)
+ printf("\n%s Flow control infos for port %-2d %s\n",
+ info_border, res->port_id, info_border);
+ printf("FC mode:\n");
+ printf(" Rx: %s\n", rx_fc_en ? "On" : "Off");
+ printf(" Tx: %s\n", tx_fc_en ? "On" : "Off");
+ printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
"fc_conf.autoneg ? "On" : "Off"" is enough like the others in this patch.Got it.
+ printf("pause_time: 0x%x\n", fc_conf.pause_time);
+ printf("high_water: 0x%x\n", fc_conf.high_water);
+ printf("low_water: 0x%x\n", fc_conf.low_water);
+ printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+ printf("mac ctrl frame fwd: %s\n",
Follow others' format will be better like "Send Xon".
"Forward MAC control frames:"
I don not catch your meaning. Is that right?:
change the statement
"
+ printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+ printf("mac ctrl frame fwd: %s\n",fc_conf.mac_ctrl_frame_fwd ? "On" :
"Off")
"
to
"
+ printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+ printf("Forward MAC control frames: %s\n",fc_conf.mac_ctrl_frame_fwd ?
"On" : "Off")
"
+ fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
+ printf("\n%s** End ***%s\n",
+ info_border, info_border);
+}
+
--
2.7.4
.
[dpdk-dev] [PATCH] net/hns3: support RAS process in Kunpeng 930
From: Hongbo Zheng
Kunpeng 930 uses a new RAS exception reporting solution.
The reset type and exception status are reported through
firmware. The driver modifies the corresponding code to
adapt to the new solution.
Signed-off-by: Hongbo Zheng
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_cmd.c | 9 +-
drivers/net/hns3/hns3_cmd.h | 2 +
drivers/net/hns3/hns3_ethdev.c| 11 +-
drivers/net/hns3/hns3_ethdev.h| 23 ++-
drivers/net/hns3/hns3_ethdev_vf.c | 3 +-
drivers/net/hns3/hns3_intr.c | 296 +-
drivers/net/hns3/hns3_intr.h | 70 +
7 files changed, 399 insertions(+), 15 deletions(-)
diff --git a/drivers/net/hns3/hns3_cmd.c b/drivers/net/hns3/hns3_cmd.c
index 75d5299..448e657 100644
--- a/drivers/net/hns3/hns3_cmd.c
+++ b/drivers/net/hns3/hns3_cmd.c
@@ -237,7 +237,12 @@ hns3_is_special_opcode(uint16_t opcode)
HNS3_OPC_STATS_MAC,
HNS3_OPC_STATS_MAC_ALL,
HNS3_OPC_QUERY_32_BIT_REG,
- HNS3_OPC_QUERY_64_BIT_REG};
+ HNS3_OPC_QUERY_64_BIT_REG,
+ HNS3_OPC_QUERY_CLEAR_MPF_RAS_INT,
+ HNS3_OPC_QUERY_CLEAR_PF_RAS_INT,
+ HNS3_OPC_QUERY_CLEAR_ALL_MPF_MSIX_INT,
+ HNS3_OPC_QUERY_CLEAR_ALL_PF_MSIX_INT,
+ HNS3_OPC_QUERY_ALL_ERR_INFO,};
uint32_t i;
for (i = 0; i < ARRAY_SIZE(spec_opcode); i++)
@@ -449,6 +454,8 @@ hns3_parse_capability(struct hns3_hw *hw,
if (hns3_get_bit(caps, HNS3_CAPS_UDP_TUNNEL_CSUM_B))
hns3_set_bit(hw->capability,
HNS3_DEV_SUPPORT_OUTER_UDP_CKSUM_B, 1);
+ if (hns3_get_bit(caps, HNS3_CAPS_RAS_IMP_B))
+ hns3_set_bit(hw->capability, HNS3_DEV_SUPPORT_RAS_IMP_B, 1);
}
static uint32_t
diff --git a/drivers/net/hns3/hns3_cmd.h b/drivers/net/hns3/hns3_cmd.h
index e4f5aa6..bf1772d 100644
--- a/drivers/net/hns3/hns3_cmd.h
+++ b/drivers/net/hns3/hns3_cmd.h
@@ -252,6 +252,8 @@ enum hns3_opcode_type {
HNS3_OPC_QUERY_MSIX_INT_STS_BD_NUM = 0x1513,
HNS3_OPC_QUERY_CLEAR_ALL_MPF_MSIX_INT = 0x1514,
HNS3_OPC_QUERY_CLEAR_ALL_PF_MSIX_INT= 0x1515,
+ HNS3_OPC_QUERY_ALL_ERR_BD_NUM = 0x1516,
+ HNS3_OPC_QUERY_ALL_ERR_INFO = 0x1517,
HNS3_OPC_IGU_EGU_TNL_INT_EN = 0x1803,
HNS3_OPC_IGU_COMMON_INT_EN = 0x1806,
HNS3_OPC_TM_QCN_MEM_INT_CFG = 0x1A14,
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 1832d26..cbcc4f4 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -324,10 +324,8 @@ hns3_interrupt_handler(void *param)
hns3_warn(hw, "received interrupt: vector0_int_stat:0x%x "
"ras_int_stat:0x%x cmdq_int_stat:0x%x",
vector0_int, ras_int, cmdq_int);
- hns3_handle_msix_error(hns, &hw->reset.request);
- hns3_handle_ras_error(hns, &hw->reset.request);
hns3_handle_mac_tnl(hw);
- hns3_schedule_reset(hns);
+ hns3_handle_error(hns);
} else if (event_cause == HNS3_VECTOR0_EVENT_RST) {
hns3_warn(hw, "received reset interrupt");
hns3_schedule_reset(hns);
@@ -6284,12 +6282,15 @@ hns3_is_reset_pending(struct hns3_adapter *hns)
hns3_check_event_cause(hns, NULL);
reset = hns3_get_reset_level(hns, &hw->reset.pending);
- if (hw->reset.level != HNS3_NONE_RESET && hw->reset.level < reset) {
+
+ if (reset != HNS3_NONE_RESET && hw->reset.level != HNS3_NONE_RESET &&
+ hw->reset.level < reset) {
hns3_warn(hw, "High level reset %d is pending", reset);
return true;
}
reset = hns3_get_reset_level(hns, &hw->reset.request);
- if (hw->reset.level != HNS3_NONE_RESET && hw->reset.level < reset) {
+ if (reset != HNS3_NONE_RESET && hw->reset.level != HNS3_NONE_RESET &&
+ hw->reset.level < reset) {
hns3_warn(hw, "High level reset %d is request", reset);
return true;
}
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 7b7d359..2ee2951 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -306,8 +306,9 @@ enum hns3_reset_stage {
};
enum hns3_reset_level {
- HNS3_NONE_RESET,
+ HNS3_FLR_RESET, /* A VF perform FLR reset */
HNS3_VF_FUNC_RESET, /* A VF function reset */
+
/*
Re: [dpdk-dev] [PATCH] examples/ptpclient: delete wrong comments
Hi, Ferruh and all,
If no other comments, could your please merge it into the
branch？
Thanks.
在 2021/4/12 16:04, Ferruh Yigit 写道:
On 4/10/2021 10:11 AM, Min Hu (Connor) wrote:
Hi, Ferruh, kirill and all,
Any comments about this patch?
在 2021/3/27 15:36, Min Hu (Connor) 写道:
This patch deletes the comments which are wrong and unnecessary.
Fixes: ab129e9065a5 ("examples/ptpclient: add minimal PTP client")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
examples/ptpclient/ptpclient.c | 4
1 file changed, 4 deletions(-)
diff --git a/examples/ptpclient/ptpclient.c
b/examples/ptpclient/ptpclient.c
index 33b297e..878d1a0 100644
--- a/examples/ptpclient/ptpclient.c
+++ b/examples/ptpclient/ptpclient.c
@@ -606,10 +606,6 @@ lcore_main(void)
unsigned nb_rx;
struct rte_mbuf *m;
- /*
- * Check that the port is on the same NUMA node as the polling
thread
- * for best performance.
- */
printf("\nCore %u Waiting for SYNC packets. [Ctrl+C to quit]\n",
rte_lcore_id());
Acked-by: Ferruh Yigit
The comment seems left over from the example this example cloned from..
.
Re: [dpdk-dev] [PATCH 0/3] fix check of port and core
Hi,Ferruh, bernard, tomasz, bruce and all, Any comments about this set of patches? 在 2021/4/10 17:14, Min Hu (Connor) 写道: Hi,Ferruh, bernard, tomasz, bruce and all, Any comments about this set of patches? 在 2021/3/27 15:40, Min Hu (Connor) 写道: Currently, some examples check that the port is on the same NUMA node as the polling thread for best performance. The method is to compare the socket id of port and that of current core. If the result is different, warning info will be given. But it ignores the port which is from numa node 0, that is, no warning info will be given if the port is from numa node 0, and this set of patches will fix it. Min Hu (Connor) (3): examples/flow_classify: fix check of port and core examples/l2fwd-cat: fix check of port and core examples/skeleton: fix check of port and core examples/flow_classify/flow_classify.c | 2 +- examples/l2fwd-cat/l2fwd-cat.c | 2 +- examples/skeleton/basicfwd.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) .
[dpdk-dev] [PATCH] net/hns3: delete redundant macro
'HNS3_RXD_TSIND_S' and 'HNS3_RXD_TSIND_M' is unused, which should
be deleted.
This patch fixed it.
Fixes: bba636698316 ("net/hns3: support Rx/Tx and related operations")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_rxtx.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/hns3/hns3_rxtx.h b/drivers/net/hns3/hns3_rxtx.h
index ad85d0d..696f347 100644
--- a/drivers/net/hns3/hns3_rxtx.h
+++ b/drivers/net/hns3/hns3_rxtx.h
@@ -104,8 +104,6 @@
#define HNS3_RXD_LUM_B 9
#define HNS3_RXD_CRCP_B10
#define HNS3_RXD_L3L4P_B 11
-#define HNS3_RXD_TSIND_S 12
-#define HNS3_RXD_TSIND_M (0x7 << HNS3_RXD_TSIND_S)
#define HNS3_RXD_TS_VLD_B 14
#define HNS3_RXD_LKBK_B15
--
2.7.4
[dpdk-dev] [PATCH v2] app/testpmd: support the query of link flow ctrl info
From: Huisong Li
This patch supports the query of the link flow control parameter
on a port.
The command format is as follows:
show port flow_ctrl
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
app/test-pmd/cmdline.c | 83 +
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 +++
2 files changed, 90 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 31884c5..06a6503 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -258,6 +258,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"show port (port_id) fec_mode"
" Show fec mode of a port.\n\n"
+
+ "show port flow_ctrl"
+ " Show flow control info of a port.\n\n"
);
}
@@ -6899,6 +6902,85 @@ cmdline_parse_inst_t cmd_set_allmulti_mode_one = {
},
};
+/* *** GET CURRENT ETHERNET LINK FLOW CONTROL *** */
+struct cmd_link_flow_ctrl_show {
+ cmdline_fixed_string_t show;
+ cmdline_fixed_string_t port;
+ portid_t port_id;
+ cmdline_fixed_string_t flow_ctrl;
+};
+
+cmdline_parse_token_string_t cmd_lfc_show_show =
+ TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+ show, "show");
+cmdline_parse_token_string_t cmd_lfc_show_port =
+ TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+ port, "port");
+cmdline_parse_token_num_t cmd_lfc_show_portid =
+ TOKEN_NUM_INITIALIZER(struct cmd_link_flow_ctrl_show,
+ port_id, RTE_UINT16);
+cmdline_parse_token_string_t cmd_lfc_show_flow_ctrl =
+ TOKEN_STRING_INITIALIZER(struct cmd_link_flow_ctrl_show,
+ flow_ctrl, "flow_ctrl");
+
+static void
+cmd_link_flow_ctrl_show_parsed(void *parsed_result,
+ __rte_unused struct cmdline *cl,
+ __rte_unused void *data)
+{
+ struct cmd_link_flow_ctrl_show *res = parsed_result;
+ static const char *info_border = "*";
+ struct rte_eth_fc_conf fc_conf;
+ bool rx_fc_en = false;
+ bool tx_fc_en = false;
+ int ret;
+
+ if (!rte_eth_dev_is_valid_port(res->port_id)) {
+ printf("Invalid port id %u\n", res->port_id);
+ return;
+ }
+
+ ret = rte_eth_dev_flow_ctrl_get(res->port_id, &fc_conf);
+ if (ret != 0) {
+ printf("Failed to get current flow ctrl information: err =
%d\n",
+ ret);
+ return;
+ }
+
+ if ((fc_conf.mode == RTE_FC_RX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
+ rx_fc_en = true;
+ if ((fc_conf.mode == RTE_FC_TX_PAUSE) || (fc_conf.mode == RTE_FC_FULL))
+ tx_fc_en = true;
+
+ printf("\n%s Flow control infos for port %-2d %s\n",
+ info_border, res->port_id, info_border);
+ printf("FC mode:\n");
+ printf(" Rx: %s\n", rx_fc_en ? "On" : "Off");
+ printf(" Tx: %s\n", tx_fc_en ? "On" : "Off");
+ printf("FC autoneg status: %s\n", fc_conf.autoneg ? "On" : "Off");
+ printf("pause_time: 0x%x\n", fc_conf.pause_time);
+ printf("high_water: 0x%x\n", fc_conf.high_water);
+ printf("low_water: 0x%x\n", fc_conf.low_water);
+ printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+ printf("Forward MAC control frames: %s\n",
+ fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
+ printf("\n%s** End ***%s\n",
+ info_border, info_border);
+}
+
+cmdline_parse_inst_t cmd_link_flow_control_show = {
+ .f = cmd_link_flow_ctrl_show_parsed,
+ .data = NULL,
+ .help_str = "show port flow_ctrl",
+ .tokens = {
+ (void *)&cmd_lfc_show_show,
+ (void *)&cmd_lfc_show_port,
+ (void *)&cmd_lfc_show_portid,
+ (void *)&cmd_lfc_show_flow_ctrl,
+ NULL,
+ },
+};
+
/* *** SETUP ETHERNET LINK FLOW CONTROL *** */
struct cmd_link_flow_ctrl_set_result {
cmdline_fixed_string_t set;
@@ -17037,6 +17119,7 @@ cmdline_parse_ctx_t main_ctx[] = {
(cmdline_parse_inst_t *)&cmd_link_flow_control_set_xon,
(cmdline_parse_inst_t *)&cmd_link_flow_control_set_macfwd,
(cmdline_parse_inst_t *)&cmd_link_flow_control_set_autoneg,
+ (cmdline_parse_inst_t *)&cmd_link_flow_control_show,
(cmdline_parse_inst_t *)&cmd_priority_flow
Re: [dpdk-dev] [PATCH] app/testpmd: support the query of link flow ctrl info
Hi, fixed in v2, thanks
在 2021/4/19 16:04, Li, Xiaoyun 写道:
Hi
-Original Message-
From: Min Hu (Connor)
Sent: Monday, April 19, 2021 14:41
To: Li, Xiaoyun ; dev@dpdk.org
Cc: Yigit, Ferruh
Subject: Re: [PATCH] app/testpmd: support the query of link flow ctrl info
Hi,
在 2021/4/19 10:53, Li, Xiaoyun 写道:
Hi
-Original Message-
From: Min Hu (Connor)
Sent: Thursday, April 15, 2021 14:47
To: dev@dpdk.org
Cc: Yigit, Ferruh ; Li, Xiaoyun
Subject: [PATCH] app/testpmd: support the query of link flow ctrl info
From: Huisong Li
This patch supports the query of the link flow control parameter on a port.
The command format is as follows:
show port flow_ctrl
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
app/test-pmd/cmdline.c | 83
+
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 7 +++
2 files changed, 90 insertions(+)
+ printf("\n%s Flow control infos for port %-2d %s\n",
+ info_border, res->port_id, info_border);
+ printf("FC mode:\n");
+ printf(" Rx: %s\n", rx_fc_en ? "On" : "Off");
+ printf(" Tx: %s\n", tx_fc_en ? "On" : "Off");
+ printf("FC autoneg status: %s\n", fc_conf.autoneg != 0 ? "On" : "Off");
"fc_conf.autoneg ? "On" : "Off"" is enough like the others in this patch.Got it.
+ printf("pause_time: 0x%x\n", fc_conf.pause_time);
+ printf("high_water: 0x%x\n", fc_conf.high_water);
+ printf("low_water: 0x%x\n", fc_conf.low_water);
+ printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+ printf("mac ctrl frame fwd: %s\n",
Follow others' format will be better like "Send Xon".
"Forward MAC control frames:"
I don not catch your meaning. Is that right?:
change the statement
I mean change the statement as:
printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
printf("Forward MAC control frames: %s\n",fc_conf.mac_ctrl_frame_fwd ? "On" :
"Off");
Keep the same format.
"
+ printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+ printf("mac ctrl frame fwd: %s\n",fc_conf.mac_ctrl_frame_fwd ? "On" :
"Off")
"
to
"
+ printf("Send Xon: %s\n", fc_conf.send_xon ? "On" : "Off");
+ printf("Forward MAC control
frames: %s\n",fc_conf.mac_ctrl_frame_fwd ?
"On" : "Off")
"
+ fc_conf.mac_ctrl_frame_fwd ? "On" : "Off");
+ printf("\n%s** End ***%s\n",
+ info_border, info_border);
+}
+
--
2.7.4
.
.
[dpdk-dev] [PATCH] doc: add Kunpeng 930 support description
Hns3 PMD has already supported Kunpeng 930 SoC. This patch added description for it. Signed-off-by: Min Hu (Connor) --- doc/guides/nics/hns3.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst index 20c7387..34e1c43 100644 --- a/doc/guides/nics/hns3.rst +++ b/doc/guides/nics/hns3.rst @@ -6,7 +6,7 @@ HNS3 Poll Mode Driver The hns3 PMD (**librte_net_hns3**) provides poll mode driver support for the inbuilt HiSilicon Network Subsystem(HNS) network engine -found in the HiSilicon Kunpeng 920 SoC. +found in the HiSilicon Kunpeng 920 SoC and Kunpeng 930 SoC . Features -- 2.7.4
[dpdk-dev] [PATCH] app: fix redundant comparison
The return value of 'rte_eal_cleanup' is always zero, so comparison
with zero is redundant.
This patch fixed it by deleting the redundant comparison.
Fixes: 67684d1e87b6 ("app/procinfo: call EAL cleanup before exit")
Fixes: b68a82425da4 ("app/compress-perf: add performance measurement")
Fixes: 5e516c89830a ("app/testpmd: call cleanup on exit")
Fixes: 613ce6691c0d ("examples/l3fwd-power: implement proper shutdown")
Fixes: 48be180de631 ("eal: move OS common debug functions to single file")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
app/pdump/main.c | 4 +---
app/proc-info/main.c | 4 +---
app/test-compress-perf/main.c| 7 +--
app/test-pmd/testpmd.c | 5 +
examples/l3fwd-power/main.c | 3 +--
lib/librte_eal/common/eal_common_debug.c | 4 +---
6 files changed, 6 insertions(+), 21 deletions(-)
diff --git a/app/pdump/main.c b/app/pdump/main.c
index 63bbe65..33523c5 100644
--- a/app/pdump/main.c
+++ b/app/pdump/main.c
@@ -1014,9 +1014,7 @@ main(int argc, char **argv)
/* dump debug stats */
print_pdump_stats();
- ret = rte_eal_cleanup();
- if (ret)
- printf("Error from rte_eal_cleanup(), %d\n", ret);
+ rte_eal_cleanup();
return 0;
}
diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index b9587f7..9498490 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -1458,9 +1458,7 @@ main(int argc, char **argv)
RTE_ETH_FOREACH_DEV(i)
rte_eth_dev_close(i);
- ret = rte_eal_cleanup();
- if (ret)
- printf("Error from rte_eal_cleanup(), %d\n", ret);
+ rte_eal_cleanup();
return 0;
}
diff --git a/app/test-compress-perf/main.c b/app/test-compress-perf/main.c
index cc9951a..03ee385 100644
--- a/app/test-compress-perf/main.c
+++ b/app/test-compress-perf/main.c
@@ -474,12 +474,7 @@ main(int argc, char **argv)
/* fallthrough */
case ST_CLEAR:
default:
- i = rte_eal_cleanup();
- if (i) {
- RTE_LOG(ERR, USER1,
- "Error from rte_eal_cleanup(), %d\n", i);
- ret = i;
- }
+ rte_eal_cleanup();
break;
}
return ret;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index d4be23f..1f8a7b8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3972,10 +3972,7 @@ main(int argc, char** argv)
return 1;
}
- ret = rte_eal_cleanup();
- if (ret != 0)
- rte_exit(EXIT_FAILURE,
-"EAL cleanup failed: %s\n", strerror(-ret));
+ rte_eal_cleanup();
return EXIT_SUCCESS;
}
diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 0af8810..84464d5 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -2914,8 +2914,7 @@ main(int argc, char **argv)
deinit_power_library())
rte_exit(EXIT_FAILURE, "deinit_power_library failed\n");
- if (rte_eal_cleanup() < 0)
- RTE_LOG(ERR, L3FWD_POWER, "EAL cleanup failed\n");
+ rte_eal_cleanup();
return 0;
}
diff --git a/lib/librte_eal/common/eal_common_debug.c
b/lib/librte_eal/common/eal_common_debug.c
index 15418e9..04b1fed 100644
--- a/lib/librte_eal/common/eal_common_debug.c
+++ b/lib/librte_eal/common/eal_common_debug.c
@@ -37,8 +37,6 @@ rte_exit(int exit_code, const char *format, ...)
rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap);
va_end(ap);
- if (rte_eal_cleanup() != 0)
- RTE_LOG(CRIT, EAL,
- "EAL could not release all resources\n");
+ rte_eal_cleanup();
exit(exit_code);
}
--
2.7.4
[dpdk-dev] [PATCH 00/10] fixes for clean code
This patchset fix some bugs, to make the code more clean. Chengchang Tang (1): net/bonding: fix configuration assignment overflow HongBo Zheng (5): app/test: add NULL pointer check of memory allocation lib/librte_pipeline: fix the use of unsafe strcpy examples/l3fwd: add function return value check crypto/virtio: fix return values check error net/e1000: add function return value check Min Hu (Connor) (4): net/pfe: check return value common/sfc_efx/base: delete redundant handling bus/dpaa: fix management command init calling app/regex: fix division by zero app/test-regex/main.c | 8 +--- app/test/test_crc.c | 2 ++ drivers/bus/dpaa/base/qbman/bman.c| 6 ++ drivers/common/sfc_efx/base/rhead_nic.c | 3 +-- drivers/crypto/virtio/virtio_rxtx.c | 4 ++-- drivers/net/bonding/rte_eth_bond_8023ad.c | 2 +- drivers/net/e1000/base/e1000_ich8lan.c| 2 ++ drivers/net/pfe/pfe_ethdev.c | 6 ++ examples/l3fwd/l3fwd_event.c | 6 +- lib/librte_pipeline/rte_swx_pipeline.c| 4 ++-- 10 files changed, 28 insertions(+), 15 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH 03/10] bus/dpaa: fix management command init calling
'bm_mc_init' only return 0, but the function whicl calls int
check the negative ret, and this is redundant.
This patch fixed it by not checking the return value.
Fixes: f38f61e982f8 ("bus/dpaa: add BMAN hardware interfaces")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
drivers/bus/dpaa/base/qbman/bman.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/bus/dpaa/base/qbman/bman.c
b/drivers/bus/dpaa/base/qbman/bman.c
index 8a62907..e1ba2a8 100644
--- a/drivers/bus/dpaa/base/qbman/bman.c
+++ b/drivers/bus/dpaa/base/qbman/bman.c
@@ -70,10 +70,8 @@ struct bman_portal *bman_create_portal(struct bman_portal
*portal,
pr_err("Bman RCR initialisation failed\n");
return NULL;
}
- if (bm_mc_init(p)) {
- pr_err("Bman MC initialisation failed\n");
- goto fail_mc;
- }
+ (void)bm_mc_init(p);
+
portal->pools = kmalloc(2 * sizeof(*pools), GFP_KERNEL);
if (!portal->pools)
goto fail_pools;
--
2.7.4
[dpdk-dev] [PATCH 04/10] app/regex: fix division by zero
Variable nb_jobs, which may be zero, is used as a denominator.
This patch fixed it.
Fixes: f5cffb7eb7fb ("app/regex: read data file once at startup")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
app/test-regex/main.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/app/test-regex/main.c b/app/test-regex/main.c
index 8e665df..b49fa88 100644
--- a/app/test-regex/main.c
+++ b/app/test-regex/main.c
@@ -725,9 +725,11 @@ main(int argc, char **argv)
if (data_len <= 0)
rte_exit(EXIT_FAILURE, "Error, can't read file, or file is
empty.\n");
- job_len = data_len / nb_jobs;
- if (job_len == 0)
- rte_exit(EXIT_FAILURE, "Error, To many jobs, for the given
input.\n");
+ if (!nb_jobs) {
+ job_len = data_len / nb_jobs;
+ if (job_len == 0)
+ rte_exit(EXIT_FAILURE, "Error, To many jobs, for the
given input.\n");
+ }
if (job_len > nb_max_payload)
rte_exit(EXIT_FAILURE, "Error, not enough jobs to cover
input.\n");
--
2.7.4
[dpdk-dev] [PATCH 10/10] net/bonding: fix configuration assignment overflow
From: Chengchang Tang
The expression may cause an overflow.
This patch fix the codeDEX static check warning "INTEGER_OVERFLOW".
Fixes: 46fb43683679 ("bond: add mode 4")
Cc: sta...@dpdk.org
Signed-off-by: Chengchang Tang
Signed-off-by: Min Hu (Connor)
---
drivers/net/bonding/rte_eth_bond_8023ad.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
b/drivers/net/bonding/rte_eth_bond_8023ad.c
index 128754f..483f082 100644
--- a/drivers/net/bonding/rte_eth_bond_8023ad.c
+++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
@@ -1237,7 +1237,7 @@ bond_mode_8023ad_conf_assign(struct mode8023ad_private
*mode4,
mode4->aggregate_wait_timeout = conf->aggregate_wait_timeout_ms *
ms_ticks;
mode4->tx_period_timeout = conf->tx_period_ms * ms_ticks;
mode4->rx_marker_timeout = conf->rx_marker_period_ms * ms_ticks;
- mode4->update_timeout_us = conf->update_timeout_ms * 1000;
+ mode4->update_timeout_us = (uint64_t)conf->update_timeout_ms * 1000;
mode4->dedicated_queues.enabled = 0;
mode4->dedicated_queues.rx_qid = UINT16_MAX;
--
2.7.4
[dpdk-dev] [PATCH 05/10] app/test: add null pointer check of memory allocation
From: HongBo Zheng
The rte_zmalloc is called in test_crc_calc without null pointer
check. This patch adds null pointer checks on return value of
rte_zmalloc.
Fixes: 9c77b848b1c1 ("test: add CRC computation")
Cc: sta...@dpdk.org
Signed-off-by: HongBo Zheng
Signed-off-by: Min Hu (Connor)
---
app/test/test_crc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/app/test/test_crc.c b/app/test/test_crc.c
index bf1d344..8231f81 100644
--- a/app/test/test_crc.c
+++ b/app/test/test_crc.c
@@ -80,6 +80,8 @@ test_crc_calc(void)
/* 32-bit ethernet CRC: Test 2 */
test_data = rte_zmalloc(NULL, CRC32_VEC_LEN1, 0);
+ if (test_data == NULL)
+ return -7;
for (i = 0; i < CRC32_VEC_LEN1; i += 12)
rte_memcpy(&test_data[i], crc32_vec1, 12);
--
2.7.4
[dpdk-dev] [PATCH 07/10] examples/l3fwd: add function return value check
From: HongBo Zheng
Return value of a function 'rte_eth_macaddr_get' called at
l3fwd_eth_dev_port_setup is not checked, but it is usually
checked for this function.
This patch fix this problem.
Fixes: a65bf3d724df ("examples/l3fwd: add ethdev setup based on eventdev")
Cc: sta...@dpdk.org
Signed-off-by: HongBo Zheng
Signed-off-by: Min Hu (Connor)
---
examples/l3fwd/l3fwd_event.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/examples/l3fwd/l3fwd_event.c b/examples/l3fwd/l3fwd_event.c
index 4d31593..7f704f9 100644
--- a/examples/l3fwd/l3fwd_event.c
+++ b/examples/l3fwd/l3fwd_event.c
@@ -105,7 +105,11 @@ l3fwd_eth_dev_port_setup(struct rte_eth_conf *port_conf)
"Cannot adjust number of descriptors: err=%d, "
"port=%d\n", ret, port_id);
- rte_eth_macaddr_get(port_id, &ports_eth_addr[port_id]);
+ ret = rte_eth_macaddr_get(port_id, &ports_eth_addr[port_id]);
+ if (ret < 0)
+ rte_exit(EXIT_FAILURE,
+"Cannot get MAC address: err=%d, port=%d\n",
+ret, port_id);
print_ethaddr(" Address:", &ports_eth_addr[port_id]);
printf(", ");
print_ethaddr("Destination:",
--
2.7.4
[dpdk-dev] [PATCH 06/10] lib/librte_pipeline: fix the use of unsafe strcpy
From: HongBo Zheng
'strcpy' is called in rte_swx_ctl_table_info_get, this function
is unsafe, use 'strncpy' instead.
Fixes: 393b96e2aa2a ("pipeline: add SWX pipeline query API")
Cc: sta...@dpdk.org
Signed-off-by: HongBo Zheng
Signed-off-by: Min Hu (Connor)
---
lib/librte_pipeline/rte_swx_pipeline.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/librte_pipeline/rte_swx_pipeline.c
b/lib/librte_pipeline/rte_swx_pipeline.c
index 4455d91..d4db4dd 100644
--- a/lib/librte_pipeline/rte_swx_pipeline.c
+++ b/lib/librte_pipeline/rte_swx_pipeline.c
@@ -9447,8 +9447,8 @@ rte_swx_ctl_table_info_get(struct rte_swx_pipeline *p,
if (!t)
return -EINVAL;
- strcpy(table->name, t->name);
- strcpy(table->args, t->args);
+ strncpy(table->name, t->name, RTE_SWX_CTL_NAME_SIZE);
+ strncpy(table->args, t->args, RTE_SWX_CTL_NAME_SIZE);
table->n_match_fields = t->n_fields;
table->n_actions = t->n_actions;
table->default_action_is_const = t->default_action_is_const;
--
2.7.4
[dpdk-dev] [PATCH 09/10] net/e1000: add function return value check
From: HongBo Zheng
Return value of a function 'e1000_phy_has_link_generic'
called at e1000_kmrn_lock_loss_workaround_ich8lan is not
checked, but it is usually checked for this function.
This patch fix this problem.
Fixes: 5a32a257f957 ("e1000: more NICs in base driver")
Cc: sta...@dpdk.org
Signed-off-by: HongBo Zheng
Signed-off-by: Min Hu (Connor)
---
drivers/net/e1000/base/e1000_ich8lan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/e1000/base/e1000_ich8lan.c
b/drivers/net/e1000/base/e1000_ich8lan.c
index 14f86b7..67adc46 100644
--- a/drivers/net/e1000/base/e1000_ich8lan.c
+++ b/drivers/net/e1000/base/e1000_ich8lan.c
@@ -5406,6 +5406,8 @@ STATIC s32 e1000_kmrn_lock_loss_workaround_ich8lan(struct
e1000_hw *hw)
* stability
*/
ret_val = e1000_phy_has_link_generic(hw, 1, 0, &link);
+ if (ret_val)
+ return ret_val;
if (!link)
return E1000_SUCCESS;
--
2.7.4
[dpdk-dev] [PATCH 01/10] net/pfe: check return value
Variable 'fd', which may receive negative value when open
"/dev/mem" file.
This patch added checking return value process.
Fixes: 67fc3ff97c39 ("net/pfe: introduce basic functions")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
drivers/net/pfe/pfe_ethdev.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/pfe/pfe_ethdev.c b/drivers/net/pfe/pfe_ethdev.c
index 8cf59e2..b4f5591 100644
--- a/drivers/net/pfe/pfe_ethdev.c
+++ b/drivers/net/pfe/pfe_ethdev.c
@@ -1049,6 +1049,12 @@ pmd_pfe_probe(struct rte_vdev_device *vdev)
g_pfe->cbus_size = cbus_size;
fd = open("/dev/mem", O_RDWR);
+ if (fd < 0) {
+ PFE_PMD_ERR("Can not open /dev/mem");
+ rc = -EIO;
+ goto err;
+ }
+
g_pfe->cbus_baseaddr = mmap(NULL, cbus_size, PROT_READ | PROT_WRITE,
MAP_SHARED, fd, cbus_addr);
close(fd);
--
2.7.4
[dpdk-dev] [PATCH 02/10] common/sfc_efx/base: delete redundant handling
the default case in 'rhead_nic_get_bar_region' is unreachable.
This patch fixed that.
Fixes: 3c1c5cc4a786 ("common/sfc_efx/base: add Riverhead support to NIC module")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
drivers/common/sfc_efx/base/rhead_nic.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/common/sfc_efx/base/rhead_nic.c
b/drivers/common/sfc_efx/base/rhead_nic.c
index f2c18c1..b9af348 100644
--- a/drivers/common/sfc_efx/base/rhead_nic.c
+++ b/drivers/common/sfc_efx/base/rhead_nic.c
@@ -483,8 +483,7 @@ rhead_nic_get_bar_region(
break;
default:
- rc = EINVAL;
- goto fail1;
+ break;
}
return (0);
--
2.7.4
[dpdk-dev] [PATCH 08/10] crypto/virtio: fix return values check error
From: HongBo Zheng
In virtio_crypto_pkt_tx_burst, we check the return values of
virtqueue_crypto_enqueue_xmit, which may returns -ENOSPC/-EMSGSIZE,
but we only check ENOSPC/EMSGSIZE, and cause the result of checks
is always false.
This patch fix this problem.
Fixes: 82adb12a1fce ("crypto/virtio: support burst enqueue/dequeue")
Cc: sta...@dpdk.org
Signed-off-by: HongBo Zheng
Signed-off-by: Min Hu (Connor)
---
drivers/crypto/virtio/virtio_rxtx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/virtio/virtio_rxtx.c
b/drivers/crypto/virtio/virtio_rxtx.c
index e1cb4ad..a35a5b0 100644
--- a/drivers/crypto/virtio/virtio_rxtx.c
+++ b/drivers/crypto/virtio/virtio_rxtx.c
@@ -500,10 +500,10 @@ virtio_crypto_pkt_tx_burst(void *tx_queue, struct
rte_crypto_op **tx_pkts,
/* Enqueue Packet buffers */
error = virtqueue_crypto_enqueue_xmit(txvq, tx_pkts[nb_tx]);
if (unlikely(error)) {
- if (error == ENOSPC)
+ if (error == -ENOSPC)
VIRTIO_CRYPTO_TX_LOG_ERR(
"virtqueue_enqueue Free count = 0");
- else if (error == EMSGSIZE)
+ else if (error == -EMSGSIZE)
VIRTIO_CRYPTO_TX_LOG_ERR(
"virtqueue_enqueue Free count < 1");
else
--
2.7.4
Re: [dpdk-dev] [PATCH 1/3] examples/flow_classify: fix check of port and core
在 2021/4/20 9:08, Thomas Monjalon 写道:
27/03/2021 08:40, Min Hu (Connor):
fix check of port and core in flow_classify example.
Fixes: bab16ddaf2c1 ("examples/flow_classify: add sample application")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
RTE_ETH_FOREACH_DEV(port)
- if (rte_eth_dev_socket_id(port) > 0 &&
+ if (rte_eth_dev_socket_id(port) >= 0 &&
rte_eth_dev_socket_id(port) != (int)rte_socket_id()) {
printf("\n\n");
printf("WARNING: port %u is on remote NUMA node\n",
Please explain which case is broken and why.
If I understand well, we don't detect remote NUMA if not running on first
socket.
Hi, the code is this:
*
/*
* Check that the port is on the same NUMA node as the polling thread
* for best performance.
*/
RTE_ETH_FOREACH_DEV(port)
if (rte_eth_dev_socket_id(port) > 0 &&
rte_eth_dev_socket_id(port) != (int)rte_socket_id()) {
printf("\n\n");
printf("WARNING: port %u is on remote NUMA node\n",
port);
printf("to polling thread.\n");
printf("Performance will not be optimal.\n");
}
printf("\nCore %u forwarding packets. ", rte_lcore_id());
printf("[Ctrl+C to quit]\n");
*
According to the comments and logging, the author just hope user to use
the core and device which are in the same numa node for optimal
performance. If not, A warning gives out.
For example in flow_classify:
./build/flow_classify -w :7d:00.1 -l 93
Here:
:7d:00.1 is on numa node 0.
core 93 is on numa node 3.
the two are not in same numa node, but no warning gives out in old codes.
Well, using this patch, we can get the waring.
Thanks, Thomas.
.
[dpdk-dev] [PATCH 3/5] net/hns3: fix unchecked function call
In hns3 PMD, as the handler always return 0, the return value
of a function 'rte_kvargs_process' no need to be checked. But
the API definition has return value, so 'void' could be used
to ignore that.
Fixes: a124f9e9591b ("net/hns3: add runtime config to select IO burst function")
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_ethdev.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index b29aab5..60267e1 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -7268,11 +7268,11 @@ hns3_parse_devargs(struct rte_eth_dev *dev)
if (!kvlist)
return;
- rte_kvargs_process(kvlist, HNS3_DEVARG_RX_FUNC_HINT,
+ (void)rte_kvargs_process(kvlist, HNS3_DEVARG_RX_FUNC_HINT,
&hns3_parse_io_hint_func, &rx_func_hint);
- rte_kvargs_process(kvlist, HNS3_DEVARG_TX_FUNC_HINT,
+ (void)rte_kvargs_process(kvlist, HNS3_DEVARG_TX_FUNC_HINT,
&hns3_parse_io_hint_func, &tx_func_hint);
- rte_kvargs_process(kvlist, HNS3_DEVARG_DEV_CAPS_MASK,
+ (void)rte_kvargs_process(kvlist, HNS3_DEVARG_DEV_CAPS_MASK,
&hns3_parse_dev_caps_mask, &dev_caps_mask);
rte_kvargs_free(kvlist);
--
2.7.4
[dpdk-dev] [PATCH 2/5] net/hns3: fix enum variable used as boolean
params->leaf.cman has enum type which is not isomorphic with boolean
type, however it is used as a boolean expression.
This patch fixed it.
Fixes: c09c7847d892 ("net/hns3: support traffic management")
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_tm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hns3/hns3_tm.c b/drivers/net/hns3/hns3_tm.c
index 165d1fb..aae4970 100644
--- a/drivers/net/hns3/hns3_tm.c
+++ b/drivers/net/hns3/hns3_tm.c
@@ -385,7 +385,7 @@ hns3_tm_leaf_node_param_check(struct rte_eth_dev *dev
__rte_unused,
return -EINVAL;
}
- if (params->leaf.cman) {
+ if (params->leaf.cman != RTE_TM_CMAN_TAIL_DROP) {
error->type = RTE_TM_ERROR_TYPE_NODE_PARAMS_CMAN;
error->message = "congestion management not supported";
return -EINVAL;
--
2.7.4
[dpdk-dev] [PATCH 0/5] fixes for hns3 PMD
This patch contains three coding fixes and two doc fixes. Min Hu (Connor) (5): net/hns3: delete unused macro net/hns3: fix enum variable used as boolean net/hns3: fix unchecked function call doc: update hns3 feature list doc: fix Rx burst function doc doc/guides/nics/features/hns3.ini | 1 + doc/guides/nics/hns3.rst | 2 +- drivers/net/hns3/hns3_ethdev.c| 6 +++--- drivers/net/hns3/hns3_rxtx.h | 1 - drivers/net/hns3/hns3_tm.c| 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) -- 2.7.4
[dpdk-dev] [PATCH 1/5] net/hns3: delete unused macro
'HNS3_RXD_LKBK_B' was defined in previous versions but no used.
This patch deleted it.
Fixes: bba636698316 ("net/hns3: support Rx/Tx and related operations")
Cc: sta...@dpdk.org
Signed-off-by: Min Hu (Connor)
---
drivers/net/hns3/hns3_rxtx.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/hns3/hns3_rxtx.h b/drivers/net/hns3/hns3_rxtx.h
index f7b457b..703c4b7 100644
--- a/drivers/net/hns3/hns3_rxtx.h
+++ b/drivers/net/hns3/hns3_rxtx.h
@@ -106,7 +106,6 @@
#define HNS3_RXD_L3L4P_B 11
#define HNS3_RXD_TS_VLD_B 14
-#define HNS3_RXD_LKBK_B15
#define HNS3_RXD_GRO_SIZE_S16
#define HNS3_RXD_GRO_SIZE_M(0x3fff << HNS3_RXD_GRO_SIZE_S)
--
2.7.4
[dpdk-dev] [PATCH 4/5] doc: update hns3 feature list
Hns3 PMD has supported SR-IOV in 19.11 version. This patch added feature description in hns3.ini. Signed-off-by: Min Hu (Connor) --- doc/guides/nics/features/hns3.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/guides/nics/features/hns3.ini b/doc/guides/nics/features/hns3.ini index 0f89c60..2a46dde 100644 --- a/doc/guides/nics/features/hns3.ini +++ b/doc/guides/nics/features/hns3.ini @@ -25,6 +25,7 @@ Multicast MAC filter = Y RSS hash = Y RSS key update = Y RSS reta update = Y +SR-IOV = Y DCB = Y VLAN filter = Y Flow control = Y -- 2.7.4
[dpdk-dev] [PATCH 5/5] doc: fix Rx burst function doc
The patch 'net/hns3: rename Rx burst function' changed `simple'
Rx function name from 'scalar' to 'scalar simple', but doc
ignored that.
This patch fixed it.
Fixes: aa5baf47e1a3 ("net/hns3: rename Rx burst function")
Signed-off-by: Min Hu (Connor)
---
doc/guides/nics/hns3.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst
index fa45721..52d6718 100644
--- a/doc/guides/nics/hns3.rst
+++ b/doc/guides/nics/hns3.rst
@@ -59,7 +59,7 @@ Runtime Config Options
``sve``, if supported use the ``sve`` Rx function which indicates the
sve algorithm.
``simple``, if supported use the ``simple`` Rx function which indicates
- the scalar algorithm.
+ the scalar simple algorithm.
``common``, if supported use the ``common`` Rx function which indicates
the scalar scattered algorithm.
--
2.7.4
Re: [dpdk-dev] [PATCH 1/2] examples/ethtool: fix Rx/Tx queue setup with rte socket id
在 2021/4/20 8:57, Thomas Monjalon 写道:
08/04/2021 12:14, Min Hu (Connor):
From: Chengwen Feng
The ethtool use the socket_id which get from rte_eth_dev_socket_id API
in the init stage, but use the rte_socket_id API to get socket_id when
setting ringparam.
This patch make sure it call rte_eth_dev_socket_id API to get
socket_id when setting ringparam.
Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample
application")
Cc: sta...@dpdk.org
The explanation is missing something.
Please tell why it is wrong.
Hi, Thomas,
It is advised that bind queue resources to the NUMA node where the
device resides, not the NUMA node corresponding to the thread where the
command line resides. And that will be better.
.
