diff mbox series

[v12,1/7] ethdev: added UNKNOWN speed value

Message ID 20200416124258.15549-2-i.dyukov@samsung.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series [v12,1/7] ethdev: added UNKNOWN speed value |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/Intel-compilation fail apply issues

Commit Message

Ivan Dyukov April 16, 2020, 12:42 p.m. UTC 
  UNKNOWN speed equals to 0xffffffff

Signed-off-by: Ivan Dyukov <i.dyukov@samsung.com>
---
 lib/librte_ethdev/rte_ethdev.h | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

Comments

Thomas Monjalon April 16, 2020, 10:14 p.m. UTC | #1 
Hi,

Please look at an update below from ethdev co-maintainers.

16/04/2020 14:42, Ivan Dyukov:
> UNKNOWN speed equals to 0xffffffff
[...]
> +#define ETH_SPEED_NUM_UNKNOWN 0xffffffff /**< Unknown */

This approach is being rejected in another thread:
http://inbox.dpdk.org/dev/42de4bd1-0a6c-6591-cd27-67ce692fabc9@intel.com/
Ivan Dyukov April 17, 2020, 6:40 a.m. UTC | #2 
Hello Everyone,

Ook. I can take care about examples updates. link_speed could be printed 
in following way:
("link speed %u%s", link_speed, link_speed 
==ETH_SPEED_NUM_UNKNOWN?"(UNKNOWN)":"")

Please let me know if you have any objections.

There are about 47 cases.

$ grep -rn link_speed examples/ app/ doc/ | wc -l
47

Thanks,
Ivan
17.04.2020 01:14, Thomas Monjalon пишет:
> Hi,
>
> Please look at an update below from ethdev co-maintainers.
>
> 16/04/2020 14:42, Ivan Dyukov:
>> UNKNOWN speed equals to 0xffffffff
> [...]
>> +#define ETH_SPEED_NUM_UNKNOWN 0xffffffff /**< Unknown */
> This approach is being rejected in another thread:
> https://protect2.fireeye.com/url?k=ed2d2a70-b0fe73ce-ed2ca13f-0cc47a31ba82-80584d32127c24cd&q=1&u=http%3A%2F%2Finbox.dpdk.org%2Fdev%2F42de4bd1-0a6c-6591-cd27-67ce692fabc9%40intel.com%2F
>
>
>
>
Maxime Coquelin April 17, 2020, 3:14 p.m. UTC | #3 
Hi Ferruh & Andrew,

On 4/17/20 8:40 AM, Ivan Dyukov wrote:
> Hello Everyone,
> 
> Ook. I can take care about examples updates. link_speed could be printed 
> in following way:
> ("link speed %u%s", link_speed, link_speed 
> ==ETH_SPEED_NUM_UNKNOWN?"(UNKNOWN)":"")
> 
> Please let me know if you have any objections.
> 
> There are about 47 cases.
> 
> $ grep -rn link_speed examples/ app/ doc/ | wc -l
> 47
> 
> Thanks,
> Ivan
> 17.04.2020 01:14, Thomas Monjalon пишет:
>> Hi,
>>
>> Please look at an update below from ethdev co-maintainers.
>>
>> 16/04/2020 14:42, Ivan Dyukov:
>>> UNKNOWN speed equals to 0xffffffff
>> [...]
>>> +#define ETH_SPEED_NUM_UNKNOWN 0xffffffff /**< Unknown */
>> This approach is being rejected in another thread:
>> https://protect2.fireeye.com/url?k=ed2d2a70-b0fe73ce-ed2ca13f-0cc47a31ba82-80584d32127c24cd&q=1&u=http%3A%2F%2Finbox.dpdk.org%2Fdev%2F42de4bd1-0a6c-6591-cd27-67ce692fabc9%40intel.com%2F

Would that work for you?
I would need your ACK before applying the series (which I planned to do
for -rc1).

Thanks in advance,
Maxime
Ferruh Yigit April 17, 2020, 3:44 p.m. UTC | #4 
On 4/17/2020 4:14 PM, Maxime Coquelin wrote:
> Hi Ferruh & Andrew,
> 
> On 4/17/20 8:40 AM, Ivan Dyukov wrote:
>> Hello Everyone,
>>
>> Ook. I can take care about examples updates. link_speed could be printed 
>> in following way:
>> ("link speed %u%s", link_speed, link_speed 
>> ==ETH_SPEED_NUM_UNKNOWN?"(UNKNOWN)":"")
>>
>> Please let me know if you have any objections.
>>
>> There are about 47 cases.
>>
>> $ grep -rn link_speed examples/ app/ doc/ | wc -l
>> 47
>>
>> Thanks,
>> Ivan
>> 17.04.2020 01:14, Thomas Monjalon пишет:
>>> Hi,
>>>
>>> Please look at an update below from ethdev co-maintainers.
>>>
>>> 16/04/2020 14:42, Ivan Dyukov:
>>>> UNKNOWN speed equals to 0xffffffff
>>> [...]
>>>> +#define ETH_SPEED_NUM_UNKNOWN 0xffffffff /**< Unknown */
>>> This approach is being rejected in another thread:
>>> https://protect2.fireeye.com/url?k=ed2d2a70-b0fe73ce-ed2ca13f-0cc47a31ba82-80584d32127c24cd&q=1&u=http%3A%2F%2Finbox.dpdk.org%2Fdev%2F42de4bd1-0a6c-6591-cd27-67ce692fabc9%40intel.com%2F
> 
> Would that work for you?
> I would need your ACK before applying the series (which I planned to do
> for -rc1).

Hi Maxime,

There is another patch from Thomas that targets this change only [1], and it is
waiting for change request, because the scope of the change is larger than just
defining a new macro, documentation & sample/test applications should be aware
of this new speed definition.

Instead of this patch, this patchset can wait [1] as dependency.

Or if this patchset is urgent, perhaps this patch can go in as it as and Thomas'
patch can replace it later with full implementation, if Thomas agrees.

And not sure if it is good idea, but perhaps this "unknown speed' can be used
local to virtio until [1] becomes ready, though I am for having this as last option.

[1]
https://patches.dpdk.org/patch/67915/
Maxime Coquelin April 17, 2020, 3:54 p.m. UTC | #5 
Hi Ferruh,

On 4/17/20 5:44 PM, Ferruh Yigit wrote:
> On 4/17/2020 4:14 PM, Maxime Coquelin wrote:
>> Hi Ferruh & Andrew,
>>
>> On 4/17/20 8:40 AM, Ivan Dyukov wrote:
>>> Hello Everyone,
>>>
>>> Ook. I can take care about examples updates. link_speed could be printed 
>>> in following way:
>>> ("link speed %u%s", link_speed, link_speed 
>>> ==ETH_SPEED_NUM_UNKNOWN?"(UNKNOWN)":"")
>>>
>>> Please let me know if you have any objections.
>>>
>>> There are about 47 cases.
>>>
>>> $ grep -rn link_speed examples/ app/ doc/ | wc -l
>>> 47
>>>
>>> Thanks,
>>> Ivan
>>> 17.04.2020 01:14, Thomas Monjalon пишет:
>>>> Hi,
>>>>
>>>> Please look at an update below from ethdev co-maintainers.
>>>>
>>>> 16/04/2020 14:42, Ivan Dyukov:
>>>>> UNKNOWN speed equals to 0xffffffff
>>>> [...]
>>>>> +#define ETH_SPEED_NUM_UNKNOWN 0xffffffff /**< Unknown */
>>>> This approach is being rejected in another thread:
>>>> https://protect2.fireeye.com/url?k=ed2d2a70-b0fe73ce-ed2ca13f-0cc47a31ba82-80584d32127c24cd&q=1&u=http%3A%2F%2Finbox.dpdk.org%2Fdev%2F42de4bd1-0a6c-6591-cd27-67ce692fabc9%40intel.com%2F
>>
>> Would that work for you?
>> I would need your ACK before applying the series (which I planned to do
>> for -rc1).
> 
> Hi Maxime,
> 
> There is another patch from Thomas that targets this change only [1], and it is
> waiting for change request, because the scope of the change is larger than just
> defining a new macro, documentation & sample/test applications should be aware
> of this new speed definition.
> 
> Instead of this patch, this patchset can wait [1] as dependency.
> 
> Or if this patchset is urgent, perhaps this patch can go in as it as and Thomas'
> patch can replace it later with full implementation, if Thomas agrees.
> 
> And not sure if it is good idea, but perhaps this "unknown speed' can be used
> local to virtio until [1] becomes ready, though I am for having this as last option.

I was replying to my mail after discussing with Thomas.

I agree Thomas's series is better, and now understand this is an API
change that was not announced.

What I propose it basically to apply Ivan's v8 with a few fixes on top
that he did. It means that if no value is set in the NIC or not value
defined as devargs, then 10G will be picked as default.

Note that the 10G value is the one currently displayyed, without Ivan's
series, so it seems a reasonable temporary solution.

Thanks,
Maxime

> [1]
> https://patches.dpdk.org/patch/67915/
>
Thomas Monjalon April 17, 2020, 5:23 p.m. UTC | #6 
17/04/2020 17:54, Maxime Coquelin:
> Hi Ferruh,
> 
> On 4/17/20 5:44 PM, Ferruh Yigit wrote:
> > On 4/17/2020 4:14 PM, Maxime Coquelin wrote:
> >> Hi Ferruh & Andrew,
> >>
> >> On 4/17/20 8:40 AM, Ivan Dyukov wrote:
> >>> Hello Everyone,
> >>>
> >>> Ook. I can take care about examples updates. link_speed could be printed 
> >>> in following way:
> >>> ("link speed %u%s", link_speed, link_speed 
> >>> ==ETH_SPEED_NUM_UNKNOWN?"(UNKNOWN)":"")
> >>>
> >>> Please let me know if you have any objections.
> >>>
> >>> There are about 47 cases.
> >>>
> >>> $ grep -rn link_speed examples/ app/ doc/ | wc -l
> >>> 47
> >>>
> >>> Thanks,
> >>> Ivan
> >>> 17.04.2020 01:14, Thomas Monjalon пишет:
> >>>> Hi,
> >>>>
> >>>> Please look at an update below from ethdev co-maintainers.
> >>>>
> >>>> 16/04/2020 14:42, Ivan Dyukov:
> >>>>> UNKNOWN speed equals to 0xffffffff
> >>>> [...]
> >>>>> +#define ETH_SPEED_NUM_UNKNOWN 0xffffffff /**< Unknown */
> >>>> This approach is being rejected in another thread:
> >>>> https://protect2.fireeye.com/url?k=ed2d2a70-b0fe73ce-ed2ca13f-0cc47a31ba82-80584d32127c24cd&q=1&u=http%3A%2F%2Finbox.dpdk.org%2Fdev%2F42de4bd1-0a6c-6591-cd27-67ce692fabc9%40intel.com%2F
> >>
> >> Would that work for you?
> >> I would need your ACK before applying the series (which I planned to do
> >> for -rc1).
> > 
> > Hi Maxime,
> > 
> > There is another patch from Thomas that targets this change only [1], and it is
> > waiting for change request, because the scope of the change is larger than just
> > defining a new macro, documentation & sample/test applications should be aware
> > of this new speed definition.
> > 
> > Instead of this patch, this patchset can wait [1] as dependency.
> > 
> > Or if this patchset is urgent, perhaps this patch can go in as it as and Thomas'
> > patch can replace it later with full implementation, if Thomas agrees.
> > 
> > And not sure if it is good idea, but perhaps this "unknown speed' can be used
> > local to virtio until [1] becomes ready, though I am for having this as last option.
> 
> I was replying to my mail after discussing with Thomas.
> 
> I agree Thomas's series is better, and now understand this is an API
> change that was not announced.
> 
> What I propose it basically to apply Ivan's v8 with a few fixes on top
> that he did. It means that if no value is set in the NIC or not value
> defined as devargs, then 10G will be picked as default.
> 
> Note that the 10G value is the one currently displayyed, without Ivan's
> series, so it seems a reasonable temporary solution.

I am OK with the suggested solution for virtio in 20.05.

Any help to implement and document unknown speed is welcome.
I won't work on it during the next 2 weeks, so feel free to take over
my patch and make it complete for 20.08.
diff mbox series

Patch

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index d1a593ad1..a15ea572e 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -287,19 +287,20 @@  struct rte_eth_stats {
 /**
  * Ethernet numeric link speeds in Mbps
  */
-#define ETH_SPEED_NUM_NONE         0 /**< Not defined */
-#define ETH_SPEED_NUM_10M         10 /**<  10 Mbps */
-#define ETH_SPEED_NUM_100M       100 /**< 100 Mbps */
-#define ETH_SPEED_NUM_1G        1000 /**<   1 Gbps */
-#define ETH_SPEED_NUM_2_5G      2500 /**< 2.5 Gbps */
-#define ETH_SPEED_NUM_5G        5000 /**<   5 Gbps */
-#define ETH_SPEED_NUM_10G      10000 /**<  10 Gbps */
-#define ETH_SPEED_NUM_20G      20000 /**<  20 Gbps */
-#define ETH_SPEED_NUM_25G      25000 /**<  25 Gbps */
-#define ETH_SPEED_NUM_40G      40000 /**<  40 Gbps */
-#define ETH_SPEED_NUM_50G      50000 /**<  50 Gbps */
-#define ETH_SPEED_NUM_56G      56000 /**<  56 Gbps */
-#define ETH_SPEED_NUM_100G    100000 /**< 100 Gbps */
+#define ETH_SPEED_NUM_NONE             0 /**< Not defined */
+#define ETH_SPEED_NUM_10M             10 /**<  10 Mbps */
+#define ETH_SPEED_NUM_100M           100 /**< 100 Mbps */
+#define ETH_SPEED_NUM_1G            1000 /**<   1 Gbps */
+#define ETH_SPEED_NUM_2_5G          2500 /**< 2.5 Gbps */
+#define ETH_SPEED_NUM_5G            5000 /**<   5 Gbps */
+#define ETH_SPEED_NUM_10G          10000 /**<  10 Gbps */
+#define ETH_SPEED_NUM_20G          20000 /**<  20 Gbps */
+#define ETH_SPEED_NUM_25G          25000 /**<  25 Gbps */
+#define ETH_SPEED_NUM_40G          40000 /**<  40 Gbps */
+#define ETH_SPEED_NUM_50G          50000 /**<  50 Gbps */
+#define ETH_SPEED_NUM_56G          56000 /**<  56 Gbps */
+#define ETH_SPEED_NUM_100G        100000 /**< 100 Gbps */
+#define ETH_SPEED_NUM_UNKNOWN 0xffffffff /**< Unknown */
 
 /**
  * A structure used to retrieve link-level information of an Ethernet port.