On 11/08/20 11:52 +0300, Ivan Dyukov wrote:
> Link status structure keeps complicated values which are hard to
> represent to end user. e.g. link_speed has INT_MAX value which
> means that speed is unknown, link_duplex equal to 0 means
> 'half-duplex' etc. To simplify processing of the values
> in application, new dpdk function is introduced.
>
> This commit adds function which treat link status structure
> and format it to text representation. User may create custom
> link status string using format string. If format string is NULL,
> the function construct standard link status string.
>
> Signed-off-by: Ivan Dyukov <i.dyu...@samsung.com>
Hello Ivan,
I don't see much difference for this patch, from what I read in previous
thread on the principle it does not seem motivated enough?
I'd have a few nits on the implementation, but on the principle: I think
this can be an incentive to get properly formatted port status strings.
The API is a little awkward however, and it is definitely complex to
maintain a format-based function. I think we could do without.
I've tried a smaller alternative.
+ simpler to use.
+ simpler to maintain.
+ safer in general.
+ no need to declare local string to store intermediate output.
- one ugly macro.
@Thomas, Stephen: would something like this be easier to accept
in librte_ethdev?
index d79699e2ed..9d72a0b70e 100644
--- a/examples/ip_pipeline/cli.c
+++ b/examples/ip_pipeline/cli.c
@@ -273,13 +273,13 @@ print_link_info(struct link *link, char *out, size_t
out_size)
"\n"
"%s: flags=<%s> mtu %u\n"
"\tether %02X:%02X:%02X:%02X:%02X:%02X rxqueues %u txqueues
%u\n"
- "\tport# %u speed %u Mbps\n"
+ "\tport# %u speed %s\n"
"\tRX packets %" PRIu64" bytes %" PRIu64"\n"
"\tRX errors %" PRIu64" missed %" PRIu64" no-mbuf %"
PRIu64"\n"
"\tTX packets %" PRIu64" bytes %" PRIu64"\n"
"\tTX errors %" PRIu64"\n",
link->name,
- eth_link.link_status == 0 ? "DOWN" : "UP",
+ rte_eth_link_status_str(eth_link.link_status),
mtu,
mac_addr.addr_bytes[0], mac_addr.addr_bytes[1],
mac_addr.addr_bytes[2], mac_addr.addr_bytes[3],
@@ -287,7 +287,7 @@ print_link_info(struct link *link, char *out, size_t
out_size)
link->n_rxq,
link->n_txq,
link->port_id,
- eth_link.link_speed,
+ rte_eth_link_speed_str(eth_link.link_speed),
stats.ipackets,
stats.ibytes,
stats.ierrors,
diff --git a/examples/ipv4_multicast/main.c b/examples/ipv4_multicast/main.c
index 7e255c35a3..1350313ee9 100644
--- a/examples/ipv4_multicast/main.c
+++ b/examples/ipv4_multicast/main.c
@@ -591,14 +591,8 @@ check_all_ports_link_status(uint32_t port_mask)
}
/* print link status if flag set */
if (print_flag == 1) {
- if (link.link_status)
- printf(
- "Port%d Link Up. Speed %u Mbps - %s\n",
- portid, link.link_speed,
- (link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
- ("full-duplex") : ("half-duplex"));
- else
- printf("Port %d Link Down\n", portid);
+ printf("Port %s " RTE_ETH_LINK_FMT "\n",
+ RTE_ETH_LINK_STR(link));
continue;
}
/* clear all_ports_up flag if any link down */
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 57e4a6ca58..f81e876d49 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4993,6 +4993,102 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
}
+/**
+ * Missing: doc.
+ */
+static inline const char *
+rte_eth_link_speed_str(uint32_t speed)
+{
+ struct {
+ const char *str;
+ uint32_t speed;
+ } speed_str_map[] = {
+ { "Unknown", ETH_SPEED_NUM_NONE },
+ { "10 Mbps", ETH_SPEED_NUM_10M },
+ { "100 Mbps", ETH_SPEED_NUM_100M },
+ { "1 Gbps", ETH_SPEED_NUM_1G },
+ { "2.5 Gbps", ETH_SPEED_NUM_2_5G },
+ { "5 Gbps", ETH_SPEED_NUM_5G },
+ { "10 Gbps", ETH_SPEED_NUM_10G },
+ { "20 Gbps", ETH_SPEED_NUM_20G },
+ { "25 Gbps", ETH_SPEED_NUM_25G },
+ { "40 Gbps", ETH_SPEED_NUM_40G },
+ { "50 Gbps", ETH_SPEED_NUM_50G },
+ { "56 Gbps", ETH_SPEED_NUM_56G },
+ { "100 Gbps", ETH_SPEED_NUM_100G },
+ { "200 Gbps", ETH_SPEED_NUM_200G },
+ };
+ size_t i;
+
+ for (i = 0; i < RTE_DIM(speed_str_map); i++) {
+ if (speed == speed_str_map[i].speed)
+ return speed_str_map[i].str;
+ }
+
+ return speed_str_map[0].str;
+}
+
+/**
+ * Missing: doc.
+ */
+static inline const char *
+rte_eth_link_duplex_str(uint16_t duplex)
+{
+ const char *str[] = {
+ [0] = "HDX",
+ [1] = "FDX",
+ };
+
+ return str[!!duplex];
+}
+
+/**
+ * Missing: doc.
+ */
+static inline const char *
+rte_eth_link_autoneg_str(uint16_t autoneg)
+{
+ const char *str[] = {
+ [0] = "Fixed",
+ [1] = "Autoneg",
+ };
+
+ return str[!!autoneg];
+}
+
+/**
+ * Missing: doc.
+ */
+static inline const char *
+rte_eth_link_status_str(uint16_t status)
+{
+ const char *str[] = {
+ [0] = "Down",
+ [1] = "Up",
+ };
+
+ return str[!!status];
+}
+
+/* internal. */
+#define RTE_ETH_LINK_DOWN_OR_WHAT_(link, what, sep) \
+ ((link).link_status == ETH_LINK_DOWN ? "" : (what)), \
+ ((link).link_status == ETH_LINK_DOWN ? "" : (sep)) \
+
+/**
+ * Missing: doc.
+ */
+#define RTE_ETH_LINK_FMT "%s%s%s%s%s%s"
+
+/**
+ * Missing: doc.
+ */
+#define RTE_ETH_LINK_STR(link) \
+ ((link).link_status == ETH_LINK_DOWN ? "Link down" : "Link up at "), \
+ RTE_ETH_LINK_DOWN_OR_WHAT_(link,
rte_eth_link_speed_str((link).link_speed), " "), \
+ RTE_ETH_LINK_DOWN_OR_WHAT_(link,
rte_eth_link_duplex_str((link).link_duplex), " "), \
+ RTE_ETH_LINK_DOWN_OR_WHAT_(link,
rte_eth_link_autoneg_str((link).link_autoneg), "")
+
#ifdef __cplusplus
}
#endif
--
Gaëtan
