10/07/2020 09:02, Ivan Dyukov: > This commit add function which treat link status structure > and format it to text representation.
It is missing an explanation about why it is required. Which problem is it solving? > Signed-off-by: Ivan Dyukov <i.dyu...@samsung.com> I'm surprised there is not so much review on this patch. [...] > --- a/lib/librte_ethdev/rte_ethdev.h > +++ b/lib/librte_ethdev/rte_ethdev.h > +/** > + * print formatted link status to stdout. This function threats all > + * special values like ETH_SPEED_NUM_UNKNOWN, ETH_LINK_DOWN etc. and convert > + * them to textual representation. I don't understand the need for this function. If needed, the application can send the output of rte_eth_link_strf() to fprintf or a log function. > + * > + * @param fmt > + * Format string which allow to format link status. If NULL is provided > + * , default formatting will be applied. > + * Following specifiers are available: > + * - '%M' link speed in Mbits/s > + * - '%G' link speed in Gbits/s > + * - '%S' link status. e.g. Up or Down > + * - '%A' link autonegotiation state > + * - '%D' link duplex state > + * @param link > + * Link status provided by rte_eth_link_get function > + * @return > + * - Number of bytes written to stdout. In case of error, -1 is returned. > + * > + */ > +__rte_experimental > +int rte_eth_link_printf(const char *const fmt, > + const struct rte_eth_link *link); > + > +/** > + * Format link status to textual representation. This function threats all not "threats" > + * special values like ETH_SPEED_NUM_UNKNOWN, ETH_LINK_DOWN etc. and convert > + * them to textual representation. > + * > + * @param str > + * A pointer to a string to be filled with textual representation of > + * device status. > + * @param len > + * Length of available memory at 'str' string. > + * @param fmt > + * Format string which allow to format link status. If NULL is provided > + * , default formatting will be applied. Please do not start a line with a comma, it should be ending the previous line, like here ;) Even better: start the new sentence on a new line. > + * Following specifiers are available: > + * - '%M' link speed in Mbits/s > + * - '%G' link speed in Gbits/s > + * - '%S' link status. e.g. Up or Down > + * - '%A' link autonegotiation state > + * - '%D' link duplex state These specifiers look OK. > + * @param link > + * Link status provided by rte_eth_link_get function > + * @return > + * - Number of bytes written to str array. In case of error, -1 is > returned. Better to have error case on a separate line. > + * > + */ > +__rte_experimental > +int rte_eth_link_strf(char *str, size_t len, const char *const fmt, > + const struct rte_eth_link *eth_link);
