[dpdk-dev] [PATCH v1 5/6] doc: update sample app with unknown speed

[Varghese, Vipin]

Hi,

Sharing an observation especially with Fortville (X710), if the port is not started the advertised speed is not max speed (`10000` or `40000`). Hence would cross check on `link_state` be useful before display?



> -----Original Message-----
> From: dev <dev-bounces at dpdk.org> On Behalf Of Andrew Rybchenko
> Sent: Sunday, May 3, 2020 7:27 PM
> To: Ivan Dyukov <i.dyukov at samsung.com>; Thomas Monjalon
> <thomas at monjalon.net>; dev at dpdk.org; v.kuramshin at samsung.com;
> david.marchand at redhat.com; Yigit, Ferruh <ferruh.yigit at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v1 5/6] doc: update sample app with unknown
> speed
> 
> On 5/2/20 10:35 PM, Ivan Dyukov wrote:
> > 01.05.2020 16:28, Andrew Rybchenko пишет:
> >> On 4/27/20 12:57 PM, Ivan Dyukov wrote:
> >>> Signed-off-by: Ivan Dyukov <i.dyukov at samsung.com>
> >>> ---
> >>>   doc/guides/sample_app_ug/link_status_intr.rst | 3 ++-
> >>>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/doc/guides/sample_app_ug/link_status_intr.rst
> >>> b/doc/guides/sample_app_ug/link_status_intr.rst
> >>> index 5283be8b7..6ebc707b7 100644
> >>> --- a/doc/guides/sample_app_ug/link_status_intr.rst
> >>> +++ b/doc/guides/sample_app_ug/link_status_intr.rst
> >>> @@ -177,7 +177,8 @@ An example callback function that has been
> >>> written as indicated below.
> >>>               printf("Failed to get port %d link status: %s\n\n",
> >>>                      port_id, rte_strerror(-ret));
> >>>           } else if (link.link_status) {
> >>> -            printf("Port %d Link Up - speed %u Mbps - %s\n\n",
> >>> port_id, (unsigned)link.link_speed,
> >>> +            printf("Port %d Link Up - speed %u%s - %s\n\n",
> >>> +port_id,
> >>> (unsigned)link.link_speed,
> >>> +                  (link.link_speed == UINT32_MAX) ? ("(UNKNOWN)") :
> >>> (" Mbps"),
> >>>                     (link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
> >>> ("full-duplex") : ("half-duplex"));
> >>>           } else
> >>>               printf("Port %d Link Down\n\n", port_id);
> >>>
> >> I think that 0 looks nicer than UINT32_MAX when printed as integer
> >> keeping in mind that it is unknown.
> >>
> > zero will mislead developers about real value of the link_speed.
> > therefore we should print real value of the speed or print nothing. e.g.
> >
> >     if (link.link_speed == UINT32_MAX)
> >
> >          printf("Port %d Link Up - speed UNKNOWN - %s\n\n", port_id,
> >                     (link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
> > ("full-duplex") : ("half-duplex"));
> >     else
> >
> >          printf("Port %d Link Up - speed %u Mbps - %s\n\n", port_id,
> > link.link_speed,
> >
> >                      (link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
> > ("full-duplex") : ("half-duplex"));
> 
> I'm not sure about 0 to be misleading, but it could be.
> Above definitely will look better in stdout.
> The only problem is code duplication in many-many places.
> May be add simple function in ethdev to do it and use it in all examples?
> 
> Please, wait more feedback before doing it.