[v9,02/24] ethdev: add a link status text representation
Checks
Commit Message
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.dyukov@samsung.com>
---
MAINTAINERS | 1 +
app/test/Makefile | 3 +
app/test/meson.build | 2 +
app/test/test_ethdev_link.c | 315 +++++++++++++++++++++++
lib/librte_ethdev/rte_ethdev.c | 160 ++++++++++++
lib/librte_ethdev/rte_ethdev.h | 32 +++
lib/librte_ethdev/rte_ethdev_version.map | 3 +
7 files changed, 516 insertions(+)
create mode 100644 app/test/test_ethdev_link.c
Comments
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.dyukov@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
11.08.2020 14:02, Gaëtan Rivet пишет:
> 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.dyukov@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.
It would be good for all values except link_speed because link speed
should be formated using sprintf e.g.
char str[15];
...
char *rte_eth_link_speed_str(uint32_t link_speed) {
if (link_speed == UINT32_MAX)
return "Unknown";
else
snprintf(str,sizeof(str),"%d",link_speed);
return str;
}
so rte_eth_link_speed_str will require some global string, not local.
>
> @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
>
On 11/08/20 15:48 +0300, Ivan Dyukov wrote:
> 11.08.2020 14:02, Gaëtan Rivet пишет:
> > 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.dyukov@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.
>
> It would be good for all values except link_speed because link speed
> should be formated using sprintf e.g.
>
> char str[15];
>
> ...
>
> char *rte_eth_link_speed_str(uint32_t link_speed) {
>
> if (link_speed == UINT32_MAX)
>
> return "Unknown";
>
> else
>
> snprintf(str,sizeof(str),"%d",link_speed);
>
> return str;
>
> }
>
> so rte_eth_link_speed_str will require some global string, not local.
Sorry I don't understand your point, the implementation below works?
> > +/**
> > + * 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;
> > +}
11.08.2020 15:53, Gaëtan Rivet пишет:
> On 11/08/20 15:48 +0300, Ivan Dyukov wrote:
>> 11.08.2020 14:02, Gaëtan Rivet пишет:
>>> 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.dyukov@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.
>> It would be good for all values except link_speed because link speed
>> should be formated using sprintf e.g.
>>
>> char str[15];
>>
>> ...
>>
>> char *rte_eth_link_speed_str(uint32_t link_speed) {
>>
>> if (link_speed == UINT32_MAX)
>>
>> return "Unknown";
>>
>> else
>>
>> snprintf(str,sizeof(str),"%d",link_speed);
>>
>> return str;
>>
>> }
>>
>> so rte_eth_link_speed_str will require some global string, not local.
> Sorry I don't understand your point, the implementation below works?
Oh. Sorry I missed it. Thanks for clarification. It should work. Let's
wait Thomas and Stephen comments.
>>> +/**
>>> + * 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;
>>> +}
On Tue, 11 Aug 2020 11:52:21 +0300
Ivan Dyukov <i.dyukov@samsung.com> 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.dyukov@samsung.com>
Why pander to driver specific link messages?
This should come from core code in one format for all drivers.
11.08.2020 18:47, Stephen Hemminger пишет:
> On Tue, 11 Aug 2020 11:52:21 +0300
> Ivan Dyukov <i.dyukov@samsung.com> 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.dyukov@samsung.com>
> Why pander to driver specific link messages?
> This should come from core code in one format for all drivers.
>
I'm not sure that I understand your question correctly, but there is no
driver specific messages.The function is intended for application usage
only, not for drivers. The function has default format of link status,
but application can customize it for own needs. We have disscussed it
before. some dpdk applications like testpmd has custom status formating
which lays on few lines of screen. so the function supports custom link
messages just to not break screen layout of applications.
@@ -393,6 +393,7 @@ T: git://dpdk.org/next/dpdk-next-net
F: lib/librte_ethdev/
F: devtools/test-null.sh
F: doc/guides/prog_guide/switch_representation.rst
+F: app/test/test_ethdev*
Flow API
M: Ori Kam <orika@mellanox.com>
@@ -251,6 +251,9 @@ SRCS-$(CONFIG_RTE_LIBRTE_SECURITY) += test_security.c
SRCS-$(CONFIG_RTE_LIBRTE_IPSEC) += test_ipsec.c test_ipsec_perf.c
SRCS-$(CONFIG_RTE_LIBRTE_IPSEC) += test_ipsec_sad.c
+
+SRCS-$(CONFIG_RTE_LIBRTE_ETHER) += test_ethdev_link.c
+
ifeq ($(CONFIG_RTE_LIBRTE_IPSEC),y)
LDLIBS += -lrte_ipsec
endif
@@ -39,6 +39,7 @@ test_sources = files('commands.c',
'test_efd.c',
'test_efd_perf.c',
'test_errno.c',
+ 'test_ethdev_link.c',
'test_event_crypto_adapter.c',
'test_event_eth_rx_adapter.c',
'test_event_ring.c',
@@ -199,6 +200,7 @@ fast_tests = [
['eal_flags_misc_autotest', false],
['eal_fs_autotest', true],
['errno_autotest', true],
+ ['ethdev_link_status', true],
['event_ring_autotest', true],
['fib_autotest', true],
['fib6_autotest', true],
new file mode 100644
@@ -0,0 +1,315 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2020 Samsung Electronics Co., Ltd All Rights Reserved
+ */
+
+#include <rte_log.h>
+#include <rte_ethdev.h>
+
+#include <rte_test.h>
+#include "test.h"
+
+
+static int32_t
+test_link_status_up_default(void)
+{
+ int ret = 0;
+ struct rte_eth_link link_status = {
+ .link_speed = ETH_SPEED_NUM_2_5G,
+ .link_status = ETH_LINK_UP,
+ .link_autoneg = ETH_LINK_AUTONEG,
+ .link_duplex = ETH_LINK_FULL_DUPLEX
+ };
+ char text[RTE_ETH_LINK_MAX_STR_LEN + 1];
+
+ ret = rte_eth_link_to_str(text, sizeof(text), NULL, &link_status);
+ RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
+ printf("Default link up #1: %s\n", text);
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 2.5 Gbit/s FDX Autoneg",
+ text, strlen(text), "Invalid default link status string");
+
+ link_status.link_duplex = ETH_LINK_HALF_DUPLEX;
+ link_status.link_autoneg = ETH_LINK_FIXED;
+ link_status.link_speed = ETH_SPEED_NUM_10M,
+ ret = rte_eth_link_to_str(text, sizeof(text), NULL, &link_status);
+ printf("Default link up #2: %s\n", text);
+ RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at 10 Mbit/s HDX Fixed",
+ text, strlen(text), "Invalid default link status "
+ "string with HDX");
+
+ link_status.link_speed = ETH_SPEED_NUM_UNKNOWN;
+ ret = rte_eth_link_to_str(text, sizeof(text), NULL, &link_status);
+ printf("Default link up #3: %s\n", text);
+ RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("Link up at Unknown speed HDX Fixed",
+ text, strlen(text), "Invalid default link status "
+ "string with HDX");
+
+ /* test max str len */
+ link_status.link_speed = ETH_SPEED_NUM_UNKNOWN - 1;
+ link_status.link_duplex = ETH_LINK_HALF_DUPLEX;
+ link_status.link_autoneg = ETH_LINK_AUTONEG;
+ ret = rte_eth_link_to_str(text, sizeof(text), NULL, &link_status);
+ printf("Default link up #4:len = %d, %s\n", ret, text);
+ RTE_TEST_ASSERT(ret > RTE_ETH_LINK_MAX_STR_LEN,
+ "String length exceeds max allowed value\n");
+ return TEST_SUCCESS;
+}
+
+static int32_t
+test_link_status_down_default(void)
+{
+ int ret = 0;
+ struct rte_eth_link link_status = {
+ .link_speed = ETH_SPEED_NUM_2_5G,
+ .link_status = ETH_LINK_DOWN,
+ .link_autoneg = ETH_LINK_AUTONEG,
+ .link_duplex = ETH_LINK_FULL_DUPLEX
+ };
+ char text[RTE_ETH_LINK_MAX_STR_LEN];
+
+ ret = rte_eth_link_to_str(text, sizeof(text), NULL, &link_status);
+ RTE_TEST_ASSERT(ret > 0, "Failed to format default string\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("Link down",
+ text, strlen(text), "Invalid default link status string");
+
+ return TEST_SUCCESS;
+}
+
+static int32_t
+test_link_status_string_overflow(void)
+{
+ int ret = 0;
+ struct rte_eth_link link_status = {
+ .link_speed = ETH_SPEED_NUM_2_5G,
+ .link_status = ETH_LINK_UP,
+ .link_autoneg = ETH_LINK_AUTONEG,
+ .link_duplex = ETH_LINK_FULL_DUPLEX
+ };
+ char text[128];
+ int i = 0;
+
+ for (i = 0; i < 128; i++)
+ text[i] = 'Y';
+ text[127] = '\0';
+
+ ret = rte_eth_link_to_str(NULL, 2, "status %S, %G Gbits/s",
+ &link_status);
+ RTE_TEST_ASSERT(ret < 0, "Format string should fail, but it's ok\n");
+
+ ret = rte_eth_link_to_str(text, 2, "status %S, %G Gbits/s",
+ &link_status);
+ RTE_TEST_ASSERT(ret < 0, "Format string should fail, but it's ok\n");
+ RTE_TEST_ASSERT(text[2] == 'Y', "String1 overflow\n");
+
+ ret = rte_eth_link_to_str(text, 8, NULL,
+ &link_status);
+ RTE_TEST_ASSERT(ret < 0, "Default format string should fail,"
+ " but it's ok\n");
+ RTE_TEST_ASSERT(text[8] == 'Y', "String1 overflow\n");
+
+ ret = rte_eth_link_to_str(text, 10, NULL,
+ &link_status);
+ RTE_TEST_ASSERT(ret < 0, "Default format string should fail,"
+ " but it's ok\n");
+ RTE_TEST_ASSERT(text[10] == 'Y', "String1 overflow\n");
+
+ text[1] = 'Y';
+ ret = rte_eth_link_to_str(text, 1, "%S",
+ &link_status);
+ RTE_TEST_ASSERT(ret < 0, "Status string should fail, but it's ok\n");
+ RTE_TEST_ASSERT(text[1] == 'Y', "String1 overflow\n");
+
+ text[1] = 'Y';
+ ret = rte_eth_link_to_str(text, 1, "%s",
+ &link_status);
+ RTE_TEST_ASSERT(ret < 0, "Status string should fail, but it's ok\n");
+ RTE_TEST_ASSERT(text[1] == 'Y', "String1 overflow\n");
+
+
+ return TEST_SUCCESS;
+}
+
+static int32_t
+test_link_status_format(void)
+{
+ int ret = 0;
+ struct rte_eth_link link_status = {
+ .link_speed = ETH_SPEED_NUM_40G,
+ .link_status = ETH_LINK_UP,
+ .link_autoneg = ETH_LINK_AUTONEG,
+ .link_duplex = ETH_LINK_FULL_DUPLEX
+ };
+ char text[128];
+ int i = 0;
+
+ for (i = 0; i < 128; i++)
+ text[i] = 'Y';
+ text[127] = '\0';
+ printf("status format #1: %s\n", text);
+ ret = rte_eth_link_to_str(text, 128, "status = %S, duplex = %D\n",
+ &link_status);
+ printf("status format #2: %s\n", text);
+ RTE_TEST_ASSERT(ret > 0, "Failed to format string\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("status = Up, duplex = FDX\n",
+ text, strlen(text), "Invalid status string1.");
+
+ ret = rte_eth_link_to_str(text, 128, "%A", &link_status);
+ printf("status format #3: %s\n", text);
+ RTE_TEST_ASSERT(ret > 0, "Failed to format string\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("Autoneg",
+ text, strlen(text), "Invalid status string2.");
+
+ ret = rte_eth_link_to_str(text, 128,
+ "%G",
+ &link_status);
+ printf("status format #4: %s\n", text);
+ RTE_TEST_ASSERT(ret > 0, "Failed to format string\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("40.0",
+ text, strlen(text), "Invalid status string3.");
+
+ ret = rte_eth_link_to_str(text, 128,
+ "%d %M %",
+ &link_status);
+ printf("status format #5: %s\n", text);
+ RTE_TEST_ASSERT(ret > 0, "Failed to format string\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("%d 40000 %",
+ text, strlen(text), "Invalid status string4.");
+ return TEST_SUCCESS;
+}
+
+static int32_t
+test_link_status_return_value(void)
+{
+ int ret = 0;
+ struct rte_eth_link link_status = {
+ .link_speed = ETH_SPEED_NUM_40G,
+ .link_status = ETH_LINK_UP,
+ .link_autoneg = ETH_LINK_AUTONEG,
+ .link_duplex = ETH_LINK_FULL_DUPLEX
+ };
+ char text[128];
+ int i = 0;
+
+ for (i = 0; i < 128; i++)
+ text[i] = 'Y';
+ text[127] = '\0';
+ ret = rte_eth_link_to_str(text, 128, "status = %S, ",
+ &link_status);
+ printf("return value #1:ret=%u, text=%s\n", ret, text);
+ ret += rte_eth_link_to_str(text + ret, 128 - ret,
+ "%A",
+ &link_status);
+ printf("return value #2:ret=%u, text=%s\n", ret, text);
+ ret += rte_eth_link_to_str(text + ret, 128 - ret,
+ ", duplex = %D\n",
+ &link_status);
+ printf("return value #3:ret=%u, text=%s\n", ret, text);
+ ret += rte_eth_link_to_str(text + ret, 128 - ret,
+ "%M Mbits/s\n",
+ &link_status);
+ printf("return value #4:ret=%u, text=%s\n", ret, text);
+ RTE_TEST_ASSERT(ret > 0, "Failed to format string\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("status = Up, Autoneg, duplex = FDX\n"
+ "40000 Mbits/s\n",
+ text, strlen(text), "Invalid status string");
+
+ return TEST_SUCCESS;
+}
+
+static int32_t
+test_link_status_unknown_specifier(void)
+{
+ int ret = 0;
+ struct rte_eth_link link_status = {
+ .link_speed = ETH_SPEED_NUM_40G,
+ .link_status = ETH_LINK_UP,
+ .link_autoneg = ETH_LINK_AUTONEG,
+ .link_duplex = ETH_LINK_FULL_DUPLEX
+ };
+ char text[128];
+
+ ret = rte_eth_link_to_str(text, 128, "status = %",
+ &link_status);
+ RTE_TEST_ASSERT(ret > 0, "Status string1 is failed\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("status = %",
+ text, strlen(text), "Invalid status string1");
+
+ ret = rte_eth_link_to_str(text, 128,
+ ", duplex = %d\n",
+ &link_status);
+ RTE_TEST_ASSERT(ret > 0, "Status string2 is failed\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL(", duplex = %d\n",
+ text, strlen(text), "Invalid status string2");
+
+ ret = rte_eth_link_to_str(text, 128,
+ "% Mbits/s\n",
+ &link_status);
+ RTE_TEST_ASSERT(ret > 0, "Status string3 is failed\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("% Mbits/s\n",
+ text, strlen(text), "Invalid status string3");
+
+ ret = rte_eth_link_to_str(text, 128,
+ "%w Mbits/s\n",
+ &link_status);
+ RTE_TEST_ASSERT(ret > 0, "Status string4 should be ok, but it's fail\n");
+ TEST_ASSERT_BUFFERS_ARE_EQUAL("%w Mbits/s\n",
+ text, strlen(text), "Invalid status string4");
+ return TEST_SUCCESS;
+}
+
+static int32_t
+test_link_status_format_edges(void)
+{
+ int ret = 0;
+ struct rte_eth_link link_status = {
+ .link_speed = ETH_SPEED_NUM_UNKNOWN,
+ .link_status = ETH_LINK_DOWN,
+ .link_autoneg = ETH_LINK_AUTONEG,
+ .link_duplex = ETH_LINK_HALF_DUPLEX
+ };
+ char text[128];
+
+ ret = rte_eth_link_to_str(text, 4, "%S", &link_status);
+ printf("format edges #1: %s\n", text);
+ RTE_TEST_ASSERT(ret < 0, "It should fail. No space for "
+ "zero terminator\n");
+ ret = rte_eth_link_to_str(text, 6, "123%D", &link_status);
+ printf("format edges #2: %s\n", text);
+ RTE_TEST_ASSERT(ret < 0, "It should fail. No space for "
+ "zero terminator\n");
+ ret = rte_eth_link_to_str(text, 7, "%A", &link_status);
+ printf("format edges #3: %s\n", text);
+ RTE_TEST_ASSERT(ret < 0, "It should fail. No space for "
+ "zero terminator\n");
+ ret = rte_eth_link_to_str(text, 8, "%A", &link_status);
+ printf("format edges #4: %s\n", text);
+ RTE_TEST_ASSERT(ret > 0, "It should ok, but it fails\n");
+ return TEST_SUCCESS;
+}
+static struct unit_test_suite link_status_testsuite = {
+ .suite_name = "link status formatting",
+ .setup = NULL,
+ .teardown = NULL,
+ .unit_test_cases = {
+ TEST_CASE(test_link_status_up_default),
+ TEST_CASE(test_link_status_down_default),
+ TEST_CASE(test_link_status_string_overflow),
+ TEST_CASE(test_link_status_format),
+ TEST_CASE(test_link_status_format_edges),
+ TEST_CASE(test_link_status_unknown_specifier),
+ TEST_CASE(test_link_status_return_value),
+ TEST_CASES_END() /**< NULL terminate unit test array */
+ }
+};
+
+static int
+test_link_status(void)
+{
+ rte_log_set_global_level(RTE_LOG_DEBUG);
+ rte_log_set_level(RTE_LOGTYPE_EAL, RTE_LOG_DEBUG);
+
+ return unit_test_suite_runner(&link_status_testsuite);
+}
+
+REGISTER_TEST_COMMAND(ethdev_link_status, test_link_status);
@@ -2383,6 +2383,166 @@ rte_eth_link_get_nowait(uint16_t port_id, struct rte_eth_link *eth_link)
return 0;
}
+static int
+rte_eth_link_to_str_parser(char *str, size_t len, const char *const fmt,
+ const struct rte_eth_link *eth_link)
+{
+ size_t offset = 0;
+ const char *fmt_cur = fmt;
+ double gbits = (double)eth_link->link_speed / 1000.;
+ static const char autoneg_str[] = "Autoneg";
+ static const char fixed_str[] = "Fixed";
+ static const char fdx_str[] = "FDX";
+ static const char hdx_str[] = "HDX";
+ static const char unknown_str[] = "Unknown";
+ static const char up_str[] = "Up";
+ static const char down_str[] = "Down";
+ char gbits_str[20];
+ char mbits_str[20];
+
+ /* preformat complex formatting to easily concatinate it further */
+ snprintf(mbits_str, sizeof(mbits_str), "%u", eth_link->link_speed);
+ snprintf(gbits_str, sizeof(gbits_str), "%.1f", gbits);
+ /* init str before formatting */
+ str[0] = 0;
+ while (*fmt_cur) {
+ /* check str bounds */
+ if (offset >= len) {
+ str[len - 1] = '\0';
+ return -EINVAL;
+ }
+ if (*fmt_cur == '%') {
+ /* set null terminator to current position,
+ * it's required for strlcat
+ */
+ str[offset] = '\0';
+ switch (*++fmt_cur) {
+ /* Speed in Mbits/s */
+ case 'M':
+ if (eth_link->link_speed ==
+ ETH_SPEED_NUM_UNKNOWN)
+ offset = strlcat(str, unknown_str,
+ len);
+ else
+ offset = strlcat(str, mbits_str, len);
+ break;
+ /* Speed in Gbits/s */
+ case 'G':
+ if (eth_link->link_speed ==
+ ETH_SPEED_NUM_UNKNOWN)
+ offset = strlcat(str, unknown_str,
+ len);
+ else
+ offset = strlcat(str, gbits_str, len);
+ break;
+ /* Link status */
+ case 'S':
+ offset = strlcat(str, eth_link->link_status ?
+ up_str : down_str, len);
+ break;
+ /* Link autoneg */
+ case 'A':
+ offset = strlcat(str, eth_link->link_autoneg ?
+ autoneg_str : fixed_str, len);
+ break;
+ /* Link duplex */
+ case 'D':
+ offset = strlcat(str, eth_link->link_duplex ?
+ fdx_str : hdx_str, len);
+ break;
+ /* ignore unknown specifier
+ * just copy it to target str
+ */
+ default:
+ str[offset++] = '%';
+ fmt_cur--;
+ break;
+
+ }
+ if (offset >= len)
+ return -EINVAL;
+
+ } else {
+ str[offset++] = *fmt_cur;
+ }
+ fmt_cur++;
+ }
+ str[offset] = '\0';
+ return offset;
+}
+
+int
+rte_eth_link_to_str(char *str, size_t len, const char *const fmt,
+ const struct rte_eth_link *eth_link)
+{
+ size_t offset = 0;
+ double gbits = (double)eth_link->link_speed / 1000.;
+ char speed_gbits_str[20];
+ char speed_mbits_str[20];
+ /* TBD: make it international? */
+ static const char link_down_str[] = "Link down";
+ static const char link_up_str[] = "Link up at ";
+ static const char unknown_speed_str[] = "Unknown speed ";
+ static const char mbits_str[] = "Mbit/s";
+ static const char gbits_str[] = "Gbit/s";
+ static const char fdx_str[] = "FDX ";
+ static const char hdx_str[] = "HDX ";
+ static const char autoneg_str[] = "Autoneg";
+ static const char fixed_str[] = "Fixed";
+
+ if (str == NULL || len == 0)
+ return -EINVAL;
+ /* default format string, if no fmt is specified */
+ if (fmt == NULL) {
+ if (eth_link->link_status == ETH_LINK_DOWN) {
+ if (sizeof(link_down_str) > len)
+ return -EINVAL;
+ return strlcpy(str, link_down_str, len);
+ }
+
+ /* preformat complex strings to easily concatinate it further */
+ snprintf(speed_mbits_str, sizeof(speed_mbits_str), "%u %s ",
+ eth_link->link_speed, mbits_str);
+ snprintf(speed_gbits_str, sizeof(speed_gbits_str), "%.1f %s ",
+ gbits, gbits_str);
+
+ offset = strlcpy(str, link_up_str, len);
+ /* reserve one byte to null terminator */
+ if (offset >= len)
+ return -EINVAL;
+ /* link speed */
+ if (eth_link->link_speed == ETH_SPEED_NUM_UNKNOWN) {
+ offset = strlcat(str, unknown_speed_str, len);
+ if (offset >= len)
+ return -EINVAL;
+ } else {
+ if (eth_link->link_speed < ETH_SPEED_NUM_1G) {
+ offset = strlcat(str, speed_mbits_str, len);
+ if (offset >= len)
+ return -EINVAL;
+ } else {
+ offset = strlcat(str, speed_gbits_str, len);
+ if (offset >= len)
+ return -EINVAL;
+ }
+ }
+ /* link duplex */
+ offset = strlcat(str, eth_link->link_duplex ?
+ fdx_str : hdx_str, len);
+ if (offset >= len)
+ return -EINVAL;
+ /* link autonegotiation */
+ offset = strlcat(str, eth_link->link_autoneg ?
+ autoneg_str : fixed_str, len);
+ if (offset >= len)
+ return -EINVAL;
+ /* Formatted status */
+ } else
+ offset = rte_eth_link_to_str_parser(str, len, fmt, eth_link);
+
+ return offset;
+}
+
int
rte_eth_stats_get(uint16_t port_id, struct rte_eth_stats *stats)
{
@@ -323,6 +323,7 @@ struct rte_eth_link {
#define ETH_LINK_UP 1 /**< Link is up (see link_status). */
#define ETH_LINK_FIXED 0 /**< No autonegotiation (see link_autoneg). */
#define ETH_LINK_AUTONEG 1 /**< Autonegotiated (see link_autoneg). */
+#define ETH_LINK_MAX_STR_LEN 60 /**< Max length of default link string. */
/**
* A structure used to configure the ring threshold registers of an RX/TX
@@ -2295,6 +2296,37 @@ 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. This function treats all
+ * 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. At least ETH_LINK_MAX_STR_LEN bytes should be allocated to
+ * store default link status text.
+ * @param len
+ * Length of available memory at 'str' string.
+ * @param fmt
+ * Format string which allows 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 eth_link
+ * Link status provided by rte_eth_link_get function
+ * @return
+ * - Number of bytes written to str array.
+ * - (-EINVAL) if size of target buffer is too small for the string
+ *
+ */
+__rte_experimental
+int rte_eth_link_to_str(char *str, size_t len, const char *const fmt,
+ const struct rte_eth_link *eth_link);
+
/**
* Retrieve the general I/O statistics of an Ethernet device.
*
@@ -241,4 +241,7 @@ EXPERIMENTAL {
__rte_ethdev_trace_rx_burst;
__rte_ethdev_trace_tx_burst;
rte_flow_get_aged_flows;
+
+ # added in 20.11
+ rte_eth_link_to_str;
};