* [PATCH] net/ixgbe: using dpdk-dumpcap capture packet coredump
@ 2024-03-19 16:18 Jun Wang
2024-03-20 17:33 ` [PATCH] net/ixgbe: do not update link status in secondary process Stephen Hemminger
0 siblings, 1 reply; 4+ messages in thread
From: Jun Wang @ 2024-03-19 16:18 UTC (permalink / raw)
To: dev
Signed-off-by: Jun Wang <junwang01@cestc.cn>
---
drivers/net/ixgbe/ixgbe_ethdev.c | 73 ++++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index c61c52b..0e624f5 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4313,49 +4313,50 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
#ifdef RTE_EXEC_ENV_FREEBSD
wait = 1;
#endif
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ if (vf)
+ diag = ixgbevf_check_link(hw, &link_speed, &link_up, wait);
+ else
+ diag = ixgbe_check_link(hw, &link_speed, &link_up, wait);
- if (vf)
- diag = ixgbevf_check_link(hw, &link_speed, &link_up, wait);
- else
- diag = ixgbe_check_link(hw, &link_speed, &link_up, wait);
+ if (diag != 0) {
+ link.link_speed = RTE_ETH_SPEED_NUM_100M;
+ link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
+ return rte_eth_linkstatus_set(dev, &link);
+ }
- if (diag != 0) {
- link.link_speed = RTE_ETH_SPEED_NUM_100M;
- link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
- return rte_eth_linkstatus_set(dev, &link);
- }
+ if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber &&
+ !ad->sdp3_no_tx_disable) {
+ esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
+ if ((esdp_reg & IXGBE_ESDP_SDP3))
+ link_up = 0;
+ }
- if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber &&
- !ad->sdp3_no_tx_disable) {
- esdp_reg = IXGBE_READ_REG(hw, IXGBE_ESDP);
- if ((esdp_reg & IXGBE_ESDP_SDP3))
- link_up = 0;
- }
-
- if (link_up == 0) {
- if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
- ixgbe_dev_wait_setup_link_complete(dev, 0);
- /* NOTE: review for potential ordering optimization */
- if (!__atomic_test_and_set(&ad->link_thread_running, __ATOMIC_SEQ_CST)) {
- /* To avoid race condition between threads, set
- * the IXGBE_FLAG_NEED_LINK_CONFIG flag only
- * when there is no link thread running.
- */
- intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
- if (rte_thread_create_internal_control(&ad->link_thread_tid,
- "ixgbe-link",
- ixgbe_dev_setup_link_thread_handler, dev) < 0) {
+ if (link_up == 0) {
+ if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
+ ixgbe_dev_wait_setup_link_complete(dev, 0);
+ /* NOTE: review for potential ordering optimization */
+ if (!__atomic_test_and_set(&ad->link_thread_running, __ATOMIC_SEQ_CST)) {
+ /* To avoid race condition between threads, set
+ * the IXGBE_FLAG_NEED_LINK_CONFIG flag only
+ * when there is no link thread running.
+ */
+ intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
+ if (rte_thread_create_internal_control(&ad->link_thread_tid,
+ "ixgbe-link",
+ ixgbe_dev_setup_link_thread_handler, dev) < 0) {
+ PMD_DRV_LOG(ERR,
+ "Create link thread failed!");
+ /* NOTE: review for potential ordering optimization */
+ __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
+ }
+ } else {
PMD_DRV_LOG(ERR,
- "Create link thread failed!");
- /* NOTE: review for potential ordering optimization */
- __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
+ "Other link thread is running now!");
}
- } else {
- PMD_DRV_LOG(ERR,
- "Other link thread is running now!");
}
+ return rte_eth_linkstatus_set(dev, &link);
}
- return rte_eth_linkstatus_set(dev, &link);
}
link.link_status = RTE_ETH_LINK_UP;
--
1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] net/ixgbe: do not update link status in secondary process
2024-03-19 16:18 [PATCH] net/ixgbe: using dpdk-dumpcap capture packet coredump Jun Wang
@ 2024-03-20 17:33 ` Stephen Hemminger
2024-03-21 4:39 ` junwang01
2024-04-04 14:56 ` Bruce Richardson
0 siblings, 2 replies; 4+ messages in thread
From: Stephen Hemminger @ 2024-03-20 17:33 UTC (permalink / raw)
To: dev; +Cc: junwang01, Stephen Hemminger
The code to update link status is not safe in secondary process.
If called from secondary it will crash, example from dumpcap:
ixgbe_dev_link_update_share()
ixgbe_dev_link_update()
rte_eth_link_get()
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reported-by: Jun Wang <junwang01@cestc.cn>
---
Simpler version of earlier patch, and add explanation.
drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index c61c52b2966b..86ccbdd78292 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4293,6 +4293,9 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
int wait = 1;
u32 esdp_reg;
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ return -1;
+
memset(&link, 0, sizeof(link));
link.link_status = RTE_ETH_LINK_DOWN;
link.link_speed = RTE_ETH_SPEED_NUM_NONE;
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/ixgbe: do not update link status in secondary process
2024-03-20 17:33 ` [PATCH] net/ixgbe: do not update link status in secondary process Stephen Hemminger
@ 2024-03-21 4:39 ` junwang01
2024-04-04 14:56 ` Bruce Richardson
1 sibling, 0 replies; 4+ messages in thread
From: junwang01 @ 2024-03-21 4:39 UTC (permalink / raw)
To: Stephen Hemminger, dev; +Cc: Stephen Hemminger
[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]
I tried this modification and it works as well.
[root@compute3 /]# /dpdk/app/dpdk-dumpcap -i 0000:18:00.0
File: /tmp/dpdk-dumpcap_0_0000:18:00.0_20240321043451.pcapng
Capturing on '0000:18:00.0'
Packets captured: 499 ^C
Packets received/dropped on interface '0000:18:00.0': 499/0 (100.0)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index c61c52b2966b..86ccbdd78292 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4293,6 +4293,9 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
int wait = 1;
u32 esdp_reg;
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ return -1;
+
memset(&link, 0, sizeof(link));
link.link_status = RTE_ETH_LINK_DOWN;
link.link_speed = RTE_ETH_SPEED_NUM_NONE;
junwang01@cestc.cn
From: Stephen Hemminger
Date: 2024-03-21 01:33
To: dev
CC: junwang01; Stephen Hemminger
Subject: [PATCH] net/ixgbe: do not update link status in secondary process
The code to update link status is not safe in secondary process.
If called from secondary it will crash, example from dumpcap:
ixgbe_dev_link_update_share()
ixgbe_dev_link_update()
rte_eth_link_get()
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reported-by: Jun Wang <junwang01@cestc.cn>
---
Simpler version of earlier patch, and add explanation.
drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index c61c52b2966b..86ccbdd78292 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4293,6 +4293,9 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
int wait = 1;
u32 esdp_reg;
+ if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+ return -1;
+
memset(&link, 0, sizeof(link));
link.link_status = RTE_ETH_LINK_DOWN;
link.link_speed = RTE_ETH_SPEED_NUM_NONE;
--
2.43.0
[-- Attachment #2: Type: text/html, Size: 9736 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net/ixgbe: do not update link status in secondary process
2024-03-20 17:33 ` [PATCH] net/ixgbe: do not update link status in secondary process Stephen Hemminger
2024-03-21 4:39 ` junwang01
@ 2024-04-04 14:56 ` Bruce Richardson
1 sibling, 0 replies; 4+ messages in thread
From: Bruce Richardson @ 2024-04-04 14:56 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, junwang01, stable
On Wed, Mar 20, 2024 at 10:33:04AM -0700, Stephen Hemminger wrote:
> The code to update link status is not safe in secondary process.
> If called from secondary it will crash, example from dumpcap:
> ixgbe_dev_link_update_share()
> ixgbe_dev_link_update()
> rte_eth_link_get()
>
Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Reported-by: Jun Wang <junwang01@cestc.cn>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Applied to dpdk-next-net-intel.
Thanks,
/Bruce
> ---
> Simpler version of earlier patch, and add explanation.
>
> drivers/net/ixgbe/ixgbe_ethdev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index c61c52b2966b..86ccbdd78292 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -4293,6 +4293,9 @@ ixgbe_dev_link_update_share(struct rte_eth_dev *dev,
> int wait = 1;
> u32 esdp_reg;
>
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> + return -1;
> +
> memset(&link, 0, sizeof(link));
> link.link_status = RTE_ETH_LINK_DOWN;
> link.link_speed = RTE_ETH_SPEED_NUM_NONE;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-04 14:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-19 16:18 [PATCH] net/ixgbe: using dpdk-dumpcap capture packet coredump Jun Wang
2024-03-20 17:33 ` [PATCH] net/ixgbe: do not update link status in secondary process Stephen Hemminger
2024-03-21 4:39 ` junwang01
2024-04-04 14:56 ` Bruce Richardson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).