rte_ethdev fix/improvement

PATCH Applied Jia Yu 3 participants 14 messages Nov 7, 2014 – Feb 18, 2015 8 messages not in the archive

lore

Share & tools

lore

Jia Yu

author

2014-11-07 17:31 UTC #0 lore raw

cover rte_ethdev fix/improvement

This patch series include a fix and an improvement to rte_ethdev lib.

Jia Yu (2):
rte_ethdev: update link status (speed, duplex, link_up) after
rte_eth_dev_start
rte_ethdev: add return status for rte_eth_stats_get

 lib/librte_ether/rte_ethdev.c | 11 ++++++++---
 lib/librte_ether/rte_ethdev.h |  4 +++-
 2 files changed, 11 insertions(+), 4 deletions(-)

Signature · 1 line

1.9.1

A message that is not in the archive (sent off-list or not yet received): 1415381511-43364-1-git-send-email-jyu-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org

Jia Yu

author

2014-11-07 17:31 UTC #2 lore raw

PATCH 1/2 rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start

Since LSR interrupt is disabled by pmd drivers, link status in rte_eth_device is always down. Bond slave_configure() enables LSR interrupt on devices to get notification if link status changes. However, the LSC interrupt at device start time is still lost.

In this fix, call link_update to read link status from hardware register at device start time.

Issue:

Change-Id Ib57a1c9114f922485c7b0f4338bfe7b3d3f87d65

Signed-off-by Jia Yu <[email protected]>

---
 lib/librte_ether/rte_ethdev.c | 4 ++++
 1 file changed, 4 insertions(+)

lib/librte_ether/rte_ethdev.c +4 -0

index ff1c769..6c01b02 100644
@@ -869,6 +869,10 @@ rte_eth_dev_start(uint8_t port_id)
 
 	rte_eth_dev_config_restore(port_id);
 
+	if (dev->data->dev_conf.intr_conf.lsc != 0) {
+		FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update, -ENOTSUP);
+		(*dev->dev_ops->link_update)(dev, 0);
+	}
 	return 0;
 }

Signature · 1 line

1.9.1

A message that is not in the archive (sent off-list or not yet received): 1415381511-43364-2-git-send-email-jyu-pghWNbHTmq7QT0dZR+AlfA@public.gmane.org

Zhang, Helin

2014-11-12 03:57 UTC #4 lore raw

Re: [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start

Hi Jia

Quoted 9 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jia Yu
Sent: Saturday, November 8, 2014 1:32 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start
Since LSR interrupt is disabled by pmd drivers, link status in rte_eth_device is always down.

If LSC interrupt is disabled by default, it will poll the link status during the initialization or in dev_start, and then the link status should he correct. If I am not wrong.

Quoted 2 lines

Bond slave_configure() enables LSR interrupt on devices to get notification if link status changes. However, the LSC interrupt at device start time is still lost.

Before enabling interrupt for LSC, the link status should be polled. So after the port startup, the link status should be there.

Quoted 3 lines

In this fix, call link_update to read link status from hardware register at device start time.

Could you help to explain this code changes a bit more? Why we need it?

Quoted 24 lines

Issue:
Change-Id Ib57a1c9114f922485c7b0f4338bfe7b3d3f87d65
Signed-off-by Jia Yu <[email protected]>
---
 lib/librte_ether/rte_ethdev.c | 4 ++++
 1 file changed, 4 insertions(+)
lib/librte_ether/rte_ethdev.c index renamed from lib/librte_ether/rte_ethdev.c +0 -0
ff1c769..6c01b02 100644
lib/librte_ether/rte_ethdev.c +4 -1
@@ -869,6 +869,10 @@ rte_eth_dev_start(uint8_t port_id)
 
 	rte_eth_dev_config_restore(port_id);
 
+	if (dev->data->dev_conf.intr_conf.lsc != 0) {
+		FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update, -ENOTSUP);
+		(*dev->dev_ops->link_update)(dev, 0);
+	}
 	return 0;
 }
 
--

Quoted 1 line

1.9.1

Regards,
Helin

A message that is not in the archive (sent off-list or not yet received): F35DEAC7BCE34641BA9FAC6BCA4A12E70A7BBF2A-0J0gbvR4kTg/[email protected]

Thomas Monjalon

2015-01-30 10:28 UTC #6 lore raw

Re: [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start

Jia, any news on this patchset?

2014-11-12 03:57, Zhang, Helin:

Quoted 52 lines

Hi Jia
Quoted 9 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jia Yu
Sent: Saturday, November 8, 2014 1:32 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start
Since LSR interrupt is disabled by pmd drivers, link status in rte_eth_device is always down.
If LSC interrupt is disabled by default, it will poll the link status during the initialization or in dev_start, and then the link status should he correct. If I am not wrong.
Quoted 2 lines
Bond slave_configure() enables LSR interrupt on devices to get notification if link status changes. However, the LSC interrupt at device start time is still lost.
Before enabling interrupt for LSC, the link status should be polled. So after the port startup, the link status should be there.
Quoted 3 lines
In this fix, call link_update to read link status from hardware register at device start time.
Could you help to explain this code changes a bit more? Why we need it?
Quoted 24 lines
Issue:
Change-Id Ib57a1c9114f922485c7b0f4338bfe7b3d3f87d65
Signed-off-by Jia Yu <[email protected]>
---
 lib/librte_ether/rte_ethdev.c | 4 ++++
 1 file changed, 4 insertions(+)
lib/librte_ether/rte_ethdev.c index renamed from lib/librte_ether/rte_ethdev.c +0 -0
ff1c769..6c01b02 100644
lib/librte_ether/rte_ethdev.c +4 -1
@@ -869,6 +869,10 @@ rte_eth_dev_start(uint8_t port_id)
 
 	rte_eth_dev_config_restore(port_id);
 
+	if (dev->data->dev_conf.intr_conf.lsc != 0) {
+		FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update, -ENOTSUP);
+		(*dev->dev_ops->link_update)(dev, 0);
+	}
 	return 0;
 }
 
--
Quoted 1 line
1.9.1
Regards,
Helin

Jia Yu

author

2015-02-03 08:00 UTC #7 lore raw

Re: [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start

My answer to Helin¹s comments:

This patch is needed for bond slave devices or other devices, when LSC interrupt is enabled.
1. slave_configure() -> slave_eth_dev->Š.lsc = 1

2. rte_eth_link_get() reads dev_link from eth_dev, when lsc interrupt is
enabled. However, the dev_link on eth_dev has not be initialized and showed link down state. This patch initializes the device¹s dev_link at rte_eth_dev_start time.

Please let me know if you have further questions/comments.

Thanks,
Jia

Quoted 65 lines · Thomas Monjalon

Jia, any news on this patchset?
2014-11-12 03:57, Zhang, Helin:
Quoted 61 lines
Hi Jia
Quoted 5 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jia Yu
Sent: Saturday, November 8, 2014 1:32 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status
(speed, duplex,
Quoted 3 lines
link_up) after rte_eth_dev_start
Since LSR interrupt is disabled by pmd drivers, link status in
rte_eth_device is
Quoted 1 line
always down.
If LSC interrupt is disabled by default, it will poll the link status during the initialization
or in dev_start, and then the link status should he correct. If I am not wrong.
Quoted 1 line
Bond slave_configure() enables LSR interrupt on devices to get
notification if link
Quoted 1 line
status changes. However, the LSC interrupt at device start time is
still lost.
Before enabling interrupt for LSC, the link status should be polled. So after the port
startup, the link status should be there.
Quoted 2 lines
In this fix, call link_update to read link status from hardware
register at device
Quoted 1 line
start time.
Could you help to explain this code changes a bit more? Why we need it?
Review
Quoted 9 lines
Issue:
Change-Id Ib57a1c9114f922485c7b0f4338bfe7b3d3f87d65
Signed-off-by Jia Yu <[email protected]>
---
 lib/librte_ether/rte_ethdev.c | 4 ++++
 1 file changed, 4 insertions(+)
(quoted diff) +0 -0
b/lib/librte_ether/rte_ethdev.c index
Quoted 15 lines
ff1c769..6c01b02 100644
lib/librte_ether/rte_ethdev.c +4 -1
@@ -869,6 +869,10 @@ rte_eth_dev_start(uint8_t port_id)
 
 	rte_eth_dev_config_restore(port_id);
 
+	if (dev->data->dev_conf.intr_conf.lsc != 0) {
+		FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update, -ENOTSUP);
+		(*dev->dev_ops->link_update)(dev, 0);
+	}
 	return 0;
 }
 
--
Quoted 1 line
1.9.1
Regards,
Helin

A message that is not in the archive (sent off-list or not yet received): D0F5BD49.32466%[email protected]

Zhang, Helin

2015-02-03 08:35 UTC #9 lore raw

Re: [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start

Quoted 12 lines

-----Original Message-----
From: Jia Yu [mailto:[email protected]]
Sent: Tuesday, February 3, 2015 4:00 PM
To: Zhang, Helin
Cc: [email protected]; Thomas Monjalon
Subject: Re: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start
My answer to Helin¹s comments:

Quoted 8 lines

This patch is needed for bond slave devices or other devices, when LSC interrupt is enabled.
1. slave_configure() -> slave_eth_dev->Š.lsc = 1
2. rte_eth_link_get() reads dev_link from eth_dev, when lsc interrupt is
enabled. However, the dev_link on eth_dev has not be initialized and showed link down state. This patch initializes the device¹s dev_link at rte_eth_dev_start time.

So the link update is for bond only? But your code changes is in rte_ethdev, it will be used for all PMDs. For hardware NIC (e.g. 82599), this link update is not needed at all. It just need to wait the link status change event. So can those code changes be put in librte_pmd_bond, but not in librte_ether?

Regards,
Helin

Quoted 73 lines

Please let me know if you have further questions/comments.
Thanks,
Jia
Quoted 48 lines · Thomas Monjalon
Jia, any news on this patchset?
2014-11-12 03:57, Zhang, Helin:
Quoted 45 lines
Hi Jia
Quoted 5 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jia Yu
Sent: Saturday, November 8, 2014 1:32 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status
(speed, duplex,
Quoted 3 lines
link_up) after rte_eth_dev_start
Since LSR interrupt is disabled by pmd drivers, link status in
rte_eth_device is
Quoted 1 line
always down.
If LSC interrupt is disabled by default, it will poll the link status during the initialization or in dev_start, and then the link status should he correct. If I am not wrong.
Quoted 1 line
Bond slave_configure() enables LSR interrupt on devices to get
notification if link
Quoted 1 line
status changes. However, the LSC interrupt at device start time is
still lost.
Before enabling interrupt for LSC, the link status should be polled. So after the port startup, the link status should be there.
Quoted 2 lines
In this fix, call link_update to read link status from hardware
register at device
Quoted 1 line
start time.
Could you help to explain this code changes a bit more? Why we need it?
Review
Quoted 9 lines
Issue:
Change-Id Ib57a1c9114f922485c7b0f4338bfe7b3d3f87d65
Signed-off-by Jia Yu <[email protected]>
---
 lib/librte_ether/rte_ethdev.c | 4 ++++
 1 file changed, 4 insertions(+)
(quoted diff) +0 -0
b/lib/librte_ether/rte_ethdev.c index
Quoted 5 lines
ff1c769..6c01b02 100644
lib/librte_ether/rte_ethdev.c +0 -0
@@ -869,6 +869,10 @@ rte_eth_dev_start(uint8_t port_id)
 
Quoted 4 lines
Quoted 4 lines
lib/librte_ether/rte_ethdev.c rte_eth_dev_start(uint8_t port_id) · line 873
Quoted 4 lines
lib/librte_ether/rte_ethdev.c +2 -0
 	rte_eth_dev_config_restore(port_id);
 
+	if (dev->data->dev_conf.intr_conf.lsc != 0) {
+		FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update,
-ENOTSUP);
Quoted 11 lines
Quoted 10 lines
Quoted 7 lines
lib/librte_ether/rte_ethdev.c +2 -1
+		(*dev->dev_ops->link_update)(dev, 0);
+	}
 	return 0;
 }
 
--
1.9.1
Regards,
Helin

A message that is not in the archive (sent off-list or not yet received): F35DEAC7BCE34641BA9FAC6BCA4A12E70A803FED-0J0gbvR4kTg/[email protected]

Jia Yu

author

2015-02-03 18:53 UTC #11 lore raw

Re: [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start

Helin,

Thanks for comment. Any device that enabled LSC needs this fix, otherwise they all need to call link_update separately. We cannot assume that only Bond enables LSC interrupt.

The fix will not be used for all PMDs, as it explicitly checks if (dev->data->dev_conf.intr_conf.lsc != 0). Therefore, for hardware NIC (e.g. 82599) that disabled lsc by default, the link_update callback will not be executed. Please let me know if you have other concerns.

Thanks,
Jia

Quoted 109 lines · Helin

Quoted 17 lines
-----Original Message-----
From: Jia Yu [mailto:[email protected]]
Sent: Tuesday, February 3, 2015 4:00 PM
To: Zhang, Helin
Cc: [email protected]; Thomas Monjalon
Subject: Re: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status (speed,
duplex, link_up) after rte_eth_dev_start
My answer to Helin¹s comments:
This patch is needed for bond slave devices or other devices, when LSC interrupt is enabled.
1. slave_configure() -> slave_eth_dev->Š.lsc = 1
Quoted 6 lines
2. rte_eth_link_get() reads dev_link from eth_dev, when lsc interrupt
is
enabled. However, the dev_link on eth_dev has not be initialized and showed
link down state. This patch initializes the device¹s dev_link at rte_eth_dev_start time.
So the link update is for bond only? But your code changes is in rte_ethdev, it will
be used for all PMDs. For hardware NIC (e.g. 82599), this link update is not needed
at all. It just need to wait the link status change event. So can those code changes
be put in librte_pmd_bond, but not in librte_ether?
Regards,
Helin
Quoted 74 lines
Please let me know if you have further questions/comments.
Thanks,
Jia
Quoted 32 lines · Thomas Monjalon
Jia, any news on this patchset?
2014-11-12 03:57, Zhang, Helin:
Quoted 29 lines
Hi Jia
Quoted 5 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jia Yu
Sent: Saturday, November 8, 2014 1:32 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status
(speed, duplex,
Quoted 3 lines
link_up) after rte_eth_dev_start
Since LSR interrupt is disabled by pmd drivers, link status in
rte_eth_device is
Quoted 1 line
always down.
If LSC interrupt is disabled by default, it will poll the link status during the initialization or in dev_start, and then the link status should he correct. If I am not wrong.
Quoted 1 line
Bond slave_configure() enables LSR interrupt on devices to get
notification if link
Quoted 1 line
status changes. However, the LSC interrupt at device start time is
still lost.
Before enabling interrupt for LSC, the link status should be polled. So after the port startup, the link status should be there.
Quoted 2 lines
In this fix, call link_update to read link status from hardware
register at device
Quoted 1 line
start time.
Could you help to explain this code changes a bit more? Why we need
it?
Quoted 16 lines
Quoted 16 lines
Review
Quoted 10 lines
Issue:
Change-Id Ib57a1c9114f922485c7b0f4338bfe7b3d3f87d65
Signed-off-by Jia Yu <[email protected]>
---
 lib/librte_ether/rte_ethdev.c | 4 ++++
 1 file changed, 4 insertions(+)
(quoted diff) +0 -0
b/lib/librte_ether/rte_ethdev.c index
Quoted 5 lines
ff1c769..6c01b02 100644
lib/librte_ether/rte_ethdev.c +0 -0
@@ -869,6 +869,10 @@ rte_eth_dev_start(uint8_t port_id)
 
Quoted 4 lines
Quoted 4 lines
lib/librte_ether/rte_ethdev.c rte_eth_dev_start(uint8_t port_id) · line 873
Quoted 4 lines
lib/librte_ether/rte_ethdev.c +2 -0
 	rte_eth_dev_config_restore(port_id);
 
+	if (dev->data->dev_conf.intr_conf.lsc != 0) {
+		FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update,
-ENOTSUP);
Quoted 11 lines
Quoted 10 lines
Quoted 7 lines
lib/librte_ether/rte_ethdev.c +2 -1
+		(*dev->dev_ops->link_update)(dev, 0);
+	}
 	return 0;
 }
 
--
1.9.1
Regards,
Helin

A message that is not in the archive (sent off-list or not yet received): D0F656A6.32525%[email protected]

Zhang, Helin

2015-02-04 00:37 UTC #13 lore raw

Re: [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start

Hi Jia

Quoted 10 lines

-----Original Message-----
From: Jia Yu [mailto:[email protected]]
Sent: Wednesday, February 4, 2015 2:53 AM
To: Zhang, Helin
Cc: [email protected]; Thomas Monjalon
Subject: Re: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start
Helin,

Quoted 8 lines

Thanks for comment. Any device that enabled LSC needs this fix, otherwise they all need to call link_update separately. We cannot assume that only Bond enables LSC interrupt.
The fix will not be used for all PMDs, as it explicitly checks if (dev->data->dev_conf.intr_conf.lsc != 0). Therefore, for hardware NIC (e.g. 82599) that disabled lsc by default, the link_update callback will not be executed. Please let me know if you have other concerns.

I understood. I mean if (lsc == 1), for 82599, it can report lsc event correctly after startup. But if a manually link update is executed, no link change can be found when a lsc event is received in application.
Your fix is to solve the problem in bond only, am I correct? My comment is that can we avoid doing that for 82599, Fortville, etc? As some of DPDK customers may not want it if lsc is enabled.

Regards,
Helin

Quoted 112 lines

Thanks,
Jia
Quoted 106 lines · Helin
Quoted 12 lines
-----Original Message-----
From: Jia Yu [mailto:[email protected]]
Sent: Tuesday, February 3, 2015 4:00 PM
To: Zhang, Helin
Cc: [email protected]; Thomas Monjalon
Subject: Re: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status (speed, duplex, link_up) after rte_eth_dev_start
My answer to Helin¹s comments:
Quoted 8 lines
This patch is needed for bond slave devices or other devices, when LSC interrupt is enabled.
1. slave_configure() -> slave_eth_dev->Š.lsc = 1
2. rte_eth_link_get() reads dev_link from eth_dev, when lsc
interrupt is enabled. However, the dev_link on eth_dev has not be initialized and showed link down state. This patch initializes the device¹s dev_link at rte_eth_dev_start time.
So the link update is for bond only? But your code changes is in rte_ethdev, it will be used for all PMDs. For hardware NIC (e.g. 82599), this link update is not needed at all. It just need to wait the link status change event. So can those code changes be put in librte_pmd_bond, but not in librte_ether?
Regards,
Helin
Quoted 76 lines
Please let me know if you have further questions/comments.
Thanks,
Jia
Quoted 34 lines · Thomas Monjalon
Jia, any news on this patchset?
2014-11-12 03:57, Zhang, Helin:
Quoted 31 lines
Hi Jia
Quoted 5 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jia Yu
Sent: Saturday, November 8, 2014 1:32 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH 1/2] rte_ethdev: update link status
(speed, duplex,
Quoted 3 lines
link_up) after rte_eth_dev_start
Since LSR interrupt is disabled by pmd drivers, link status in
rte_eth_device is
Quoted 1 line
always down.
If LSC interrupt is disabled by default, it will poll the link status during the initialization or in dev_start, and then the link status should he correct. If I am not wrong.
Quoted 1 line
Bond slave_configure() enables LSR interrupt on devices to get
notification if link
Quoted 2 lines
status changes. However, the LSC interrupt at device start time is
still lost.
Before enabling interrupt for LSC, the link status should be polled. So after the port startup, the link status should be there.
Quoted 2 lines
In this fix, call link_update to read link status from hardware
register at device
Quoted 1 line
start time.
Could you help to explain this code changes a bit more? Why we need
it?
Quoted 16 lines
Quoted 16 lines
Review
Quoted 10 lines
Issue:
Change-Id Ib57a1c9114f922485c7b0f4338bfe7b3d3f87d65
Signed-off-by Jia Yu <[email protected]>
---
 lib/librte_ether/rte_ethdev.c | 4 ++++
 1 file changed, 4 insertions(+)
(quoted diff) +0 -0
b/lib/librte_ether/rte_ethdev.c index
Quoted 5 lines
ff1c769..6c01b02 100644
lib/librte_ether/rte_ethdev.c +0 -0
@@ -869,6 +869,10 @@ rte_eth_dev_start(uint8_t port_id)
 
Quoted 4 lines
Quoted 4 lines
lib/librte_ether/rte_ethdev.c rte_eth_dev_start(uint8_t port_id) · line 873
Quoted 4 lines
lib/librte_ether/rte_ethdev.c +2 -0
 	rte_eth_dev_config_restore(port_id);
 
+	if (dev->data->dev_conf.intr_conf.lsc != 0) {
+		FUNC_PTR_OR_ERR_RET(*dev->dev_ops->link_update,
-ENOTSUP);
Quoted 11 lines
Quoted 10 lines
Quoted 7 lines
lib/librte_ether/rte_ethdev.c +2 -1
+		(*dev->dev_ops->link_update)(dev, 0);
+	}
 	return 0;
 }
 
--
1.9.1
Regards,
Helin

Jia Yu

author

2014-11-07 17:31 UTC #14 lore raw

PATCH 2/2 rte_ethdev: add return status for rte_eth_stats_get

In reply to a message not in the archive

rte_eth_stats_get is to get physical device statistics. Without return status, caller does not know whether function fails or not (i.e. invalid port_id, driver has no stats_get callback). Caller cannot differiente normal 0 stats or error case. This fix adds a return status to the function.

Signed-off-by Jia Yu <[email protected]>

---
 lib/librte_ether/rte_ethdev.c | 7 ++++---
 lib/librte_ether/rte_ethdev.h | 4 +++-
 2 files changed, 7 insertions(+), 4 deletions(-)

lib/librte_ether/rte_ethdev.c +4 -3

index 6c01b02..4ec9ca6 100644
@@ -1226,21 +1226,22 @@ rte_eth_link_get_nowait(uint8_t port_id, struct rte_eth_link *eth_link)
 	}
 }
 
-void
+int
 rte_eth_stats_get(uint8_t port_id, struct rte_eth_stats *stats)
 {
 	struct rte_eth_dev *dev;
 
 	if (port_id >= nb_ports) {
 		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
-		return;
+		return (-ENODEV);
 	}
 	dev = &rte_eth_devices[port_id];
 	memset(stats, 0, sizeof(*stats));
 
-	FUNC_PTR_OR_RET(*dev->dev_ops->stats_get);
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->stats_get, -ENOTSUP);
 	(*dev->dev_ops->stats_get)(dev, stats);
 	stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed;
+        return 0;
 }
 
 void

lib/librte_ether/rte_ethdev.h +3 -1

index 8bf274d..e29e9be 100644
@@ -2054,8 +2054,10 @@ extern void rte_eth_link_get_nowait(uint8_t port_id,
  *   - *obytes*   with the total of successfully transmitted bytes.
  *   - *ierrors*  with the total of erroneous received packets.
  *   - *oerrors*  with the total of failed transmitted packets.
+ * @return
+ *   Zero if successful. Non-zero otherwise.
  */
-extern void rte_eth_stats_get(uint8_t port_id, struct rte_eth_stats *stats);
+extern int rte_eth_stats_get(uint8_t port_id, struct rte_eth_stats *stats);
 
 /**
  * Reset the general I/O statistics of an Ethernet device.

Signature · 1 line

1.9.1

Thomas Monjalon

2014-11-10 09:40 UTC #15 lore raw

In reply to a message not in the archive

Hi Jia,

2014-11-07 09:31, Jia Yu:

Quoted 1 line

This patch series include a fix and an improvement to rte_ethdev lib.

New enhancements won't be integrated in release 1.8.
But fixes are welcome.
The problem is that it's not easy to track partially applies patchset. So it would be simpler if you send your fix separately.

Thanks

Signature · 1 line

Thomas

Jia Yu

author

2014-11-11 23:48 UTC #16 lore raw

Thanks, Thomas.

The two patches are minor fixes. No new API is introduced. I will revise the description.

Thanks,
Jia

Quoted 13 lines · Thomas Monjalon

Hi Jia,
2014-11-07 09:31, Jia Yu:
Quoted 1 line
This patch series include a fix and an improvement to rte_ethdev lib.
New enhancements won't be integrated in release 1.8.
But fixes are welcome.
The problem is that it's not easy to track partially applies patchset. So it would be simpler if you send your fix separately.
Thanks
Signature · 1 line
Thomas

A message that is not in the archive (sent off-list or not yet received): D087DFF2.2BC9E%[email protected]

Thomas Monjalon

2014-11-12 09:35 UTC #18 lore raw

2014-11-11 23:48, Jia Yu:

Quoted 1 line

The two patches are minor fixes. No new API is introduced.

You are changing the API of rte_eth_stats_get().
It's a tricky corner case because it's a light change which should have no impact. Anyone against changing the void return into int? Impact in mind? I can integrate it if someone else vote for.

Signature · 1 line

Thomas

Zhang, Helin

Acked-by

2015-02-11 03:20 UTC #19 lore raw

In reply to a message not in the archive

Quoted 15 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Jia Yu
Sent: Saturday, November 8, 2014 1:32 AM
To: [email protected]
Subject: [dpdk-dev] [PATCH 0/2] rte_ethdev fix/improvement
This patch series include a fix and an improvement to rte_ethdev lib.
Jia Yu (2):
rte_ethdev: update link status (speed, duplex, link_up) after
rte_eth_dev_start
rte_ethdev: add return status for rte_eth_stats_get

Quoted 6 lines

lib/librte_ether/rte_ethdev.c | 11 ++++++++--- lib/librte_ether/rte_ethdev.h
| 4 +++-
 2 files changed, 11 insertions(+), 4 deletions(-)
--
1.9.1

Acked-by Helin Zhang <[email protected]>

A message that is not in the archive (sent off-list or not yet received): F35DEAC7BCE34641BA9FAC6BCA4A12E70A806C0B-0J0gbvR4kTg/[email protected]

Thomas Monjalon

Applied

2015-02-18 16:51 UTC #21 lore raw

Quoted 8 lines

Quoted 6 lines
This patch series include a fix and an improvement to rte_ethdev lib.
Jia Yu (2):
rte_ethdev: update link status (speed, duplex, link_up) after
rte_eth_dev_start
rte_ethdev: add return status for rte_eth_stats_get
Acked-by Helin Zhang <[email protected]>

Applied, thanks