On 5/27/2020 8:45 AM, Morten Brørup wrote: >> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ivan Dyukov >> Sent: Tuesday, May 26, 2020 9:10 PM >> >> This commit add function which treat link status structure >> and format it to text representation. >> >> Signed-off-by: Ivan Dyukov <i.dyu...@samsung.com> >> --- >> lib/librte_ethdev/rte_ethdev.c | 39 ++++++++++++++++++++++++++++++++++ >> lib/librte_ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++ >> 2 files changed, 70 insertions(+) >> >> diff --git a/lib/librte_ethdev/rte_ethdev.c >> b/lib/librte_ethdev/rte_ethdev.c >> index 8e10a6fc3..8d75c2440 100644 >> --- a/lib/librte_ethdev/rte_ethdev.c >> +++ b/lib/librte_ethdev/rte_ethdev.c >> @@ -2385,6 +2385,45 @@ rte_eth_link_get_nowait(uint16_t port_id, struct >> rte_eth_link *eth_link) >> return 0; >> } >> >> +void >> +rte_eth_link_prepare_text(struct rte_eth_link *eth_link, uint32_t >> speed_unit, >> + struct rte_eth_link_text *link_text) >> +{ >> + uint32_t link_speed = 0; >> + /* prepare link speed */ >> + if (eth_link->link_speed == ETH_SPEED_NUM_UNKNOWN) >> + memcpy(link_text->link_speed, "unknown", >> sizeof("unknown")); >> + else { >> + if (speed_unit == ETH_SPEED_UNIT_GBPS) >> + link_speed = eth_link->link_speed / 1000; >> + else >> + link_speed = eth_link->link_speed; >> + snprintf(link_text->link_speed, sizeof(link_text- >>> link_speed), >> + "%u", link_speed); >> + } >> + /* prepare link duplex */ >> + if (eth_link->link_duplex == ETH_LINK_FULL_DUPLEX) >> + memcpy(link_text->link_duplex, "full-duplex", >> + sizeof("full-duplex")); >> + else >> + memcpy(link_text->link_duplex, "half-duplex", >> + sizeof("half-duplex")); >> + /* prepare autoneg */ >> + if (eth_link->link_autoneg == ETH_LINK_AUTONEG) >> + memcpy(link_text->link_autoneg, "autoneg", >> + sizeof("autoneg")); >> + else >> + memcpy(link_text->link_autoneg, "fixed", >> + sizeof("fixed")); >> + /* prepare status */ >> + if (eth_link->link_status == ETH_LINK_DOWN) >> + memcpy(link_text->link_status, "down", >> + sizeof("down")); >> + else >> + memcpy(link_text->link_status, "up", >> + sizeof("up")); >> +} >> + >> int >> rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats) >> { >> diff --git a/lib/librte_ethdev/rte_ethdev.h >> b/lib/librte_ethdev/rte_ethdev.h >> index 2090af501..53d2f0c78 100644 >> --- a/lib/librte_ethdev/rte_ethdev.h >> +++ b/lib/librte_ethdev/rte_ethdev.h >> @@ -316,6 +316,19 @@ struct rte_eth_link { >> uint16_t link_status : 1; /**< ETH_LINK_[DOWN/UP] */ >> } __rte_aligned(8); /**< aligned for atomic64 read/write */ >> >> +/** >> + * Link speed units >> + */ >> +#define ETH_SPEED_UNIT_GBPS 0 >> +#define ETH_SPEED_UNIT_MBPS 1 >> + >> + >> +struct rte_eth_link_text { >> + char link_speed[14]; /** link speed */ >> + char link_duplex[12]; /** full-duplex or half-duplex */ >> + char link_autoneg[8]; /** autoneg or fixed */ >> + char link_status[5]; /** down or up */ >> +}; >> /* Utility constants */ >> #define ETH_LINK_HALF_DUPLEX 0 /**< Half-duplex connection (see >> link_duplex). */ >> #define ETH_LINK_FULL_DUPLEX 1 /**< Full-duplex connection (see >> link_duplex). */ >> @@ -2295,6 +2308,24 @@ int rte_eth_link_get(uint16_t port_id, struct >> rte_eth_link *link); >> */ >> int rte_eth_link_get_nowait(uint16_t port_id, struct rte_eth_link >> *link); >> >> +/** >> + * Format link status to textual representation. speed_unit is used to >> convert >> + * link_speed to specified unit. Also this function threats a special >> + * ETH_SPEED_NUM_UNKNOWN value of link_speed and return 'UNKNOWN' >> speed >> + * in this case. >> + * >> + * @param link >> + * Link status provided by rte_eth_link_get function >> + * @param speed_unit >> + * Target units for the speed. Following values are available: >> + * - ETH_SPEED_UNIT_GBPS >> + * - ETH_SPEED_UNIT_MBPS >> + * @param link_text >> + * A pointer to an *rte_eth_link_text* structure to be filled with >> + * textual representation of device status >> + */ >> +void rte_eth_link_prepare_text(struct rte_eth_link *link, uint32_t >> speed_unit, >> + struct rte_eth_link_text *link_text); >> /** >> * Retrieve the general I/O statistics of an Ethernet device. >> * >> -- >> 2.17.1 >> > > Considering that this function will only be used by applications that don't > need to follow a vendor-specific textual format for their link status, you > can choose your own text format, and don't need the struct for the text > strings. The function can simply write the entire information into a single > string. It also makes speed_unit superfluous. E.g. like this: > > void rte_eth_link_prepare_text(char *str, const struct rte_eth_link *const > link) > { > if (link.link_status == ETH_LINK_DOWN) { > str += sprintf(str, "Link down"); > } else { > str += sprintf(str, "Link up at "); > if (link.link_speed == ETH_SPEED_NUM_UNKNOWN) { > str += sprintf("Unknown speed"); > } else { > if (link.link_speed < ETH_SPEED_NUM_1G) > str += sprintf(str, "%u Mbit/s", link.link_speed); > else if (link.link_speed == ETH_SPEED_NUM_2_5G) > str += sprintf(str, "2.5 Gbit/s"); > else > str += sprintf(str, "%u Gbit/s", link.link_speed / 1000); > str += sprintf(str, " %cDX", link.link_duplex ? 'F' : 'H'); > str += sprintf(str, " %s", link.link_autoneg ? "Autoneg" : > "Fixed"); > } > } > } > > > And if DPDK already has an established style for "convert information to > human readable text" functions regarding function name and parameter order, > the function should follow such style. >
What about having them both, a per-formatted string that can make life easy for applications, and struct for text strings for the apps need some granularity.
