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

["Morten Brørup" <mb@smartsharesystems.com>]

> 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.

-Morten