Re: [dpdk-dev] [RFC] ethdev: improve link speed to string

["Min Hu (Connor)" <humin29@huawei.com>]

在 2021/10/30 17:59, Morten Brørup 写道:
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Min Hu (Connor)
>> Sent: Friday, 17 September 2021 02.44
>>
>> Agree with you. Thanks Andrew
>>
>> 在 2021/9/16 16:21, Andrew Rybchenko 写道:
>>> On 9/16/21 11:16 AM, Min Hu (Connor) wrote:
>>>> Hi, Andrew,
>>>>
>>>> 在 2021/9/16 14:22, Andrew Rybchenko 写道:
>>>>> On 9/16/21 5:56 AM, Min Hu (Connor) wrote:
>>>>>> Currently, link speed to string only supports specific speeds,
>> like 10M,
>>>>>> 100M, 1G etc.
>>>>>>
>>>>>> This patch expands support for any link speed which is over 1M and
>> one
>>>>>> decimal place will kept for display at most.
>>>>>>
>>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>>> ---
>>>>>>     lib/ethdev/rte_ethdev.c | 34 +++++++++++++++++-----------------
>>>>>>     1 file changed, 17 insertions(+), 17 deletions(-)
>>>>>>
>>>>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>>>>> index daf5ca9242..1d3b960305 100644
>>>>>> --- a/lib/ethdev/rte_ethdev.c
>>>>>> +++ b/lib/ethdev/rte_ethdev.c
>>>>>> @@ -2750,24 +2750,24 @@ rte_eth_link_get_nowait(uint16_t port_id,
>>>>>> struct rte_eth_link *eth_link)
>>>>>>     const char *
>>>>>>     rte_eth_link_speed_to_str(uint32_t link_speed)
>>>>>>     {
>>>>>> -    switch (link_speed) {
>>>>>> -    case ETH_SPEED_NUM_NONE: return "None";
>>>>>> -    case ETH_SPEED_NUM_10M:  return "10 Mbps";
>>>>>> -    case ETH_SPEED_NUM_100M: return "100 Mbps";
>>>>>> -    case ETH_SPEED_NUM_1G:   return "1 Gbps";
>>>>>> -    case ETH_SPEED_NUM_2_5G: return "2.5 Gbps";
>>>>>> -    case ETH_SPEED_NUM_5G:   return "5 Gbps";
>>>>>> -    case ETH_SPEED_NUM_10G:  return "10 Gbps";
>>>>>> -    case ETH_SPEED_NUM_20G:  return "20 Gbps";
>>>>>> -    case ETH_SPEED_NUM_25G:  return "25 Gbps";
>>>>>> -    case ETH_SPEED_NUM_40G:  return "40 Gbps";
>>>>>> -    case ETH_SPEED_NUM_50G:  return "50 Gbps";
>>>>>> -    case ETH_SPEED_NUM_56G:  return "56 Gbps";
>>>>>> -    case ETH_SPEED_NUM_100G: return "100 Gbps";
>>>>>> -    case ETH_SPEED_NUM_200G: return "200 Gbps";
>>>>>> -    case ETH_SPEED_NUM_UNKNOWN: return "Unknown";
>>>>>> -    default: return "Invalid";
>>>>>> +#define SPEED_STRING_LEN 16
>>>>>> +    static char name[SPEED_STRING_LEN];
>>>>>
>>>>> NACK
>>>>>
>>>>> Nothing good will happen if you try to use the function to
>>>>> print two different link speeds in one log message.
>>>> You are right.
>>>> And use malloc for "name" will result in memory leakage, which is
>> also
>>>> not a good option.
>>>>
>>>> BTW, do you think if we need to modify the function
>>>> "rte_eth_link_speed_to_str"?
>>>
>>> IMHO it would be more pain than gain in this case.
> 
> If ETH_SPEED_NUM_xyz values was an enum instead of #define, the default case could be removed from this switch, and the compiler would emit a warning if a new ETH_SPEED_NUM_xyz was introduced without adding a case for it in this function.
But, according to some coding standard, default case should be added in
"switch case" syntax.
> 
> -Morten
>