Hi Ferruh, Morten,
07.09.2020 18:29, Morten Brørup пишет:
>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Monday, September 7, 2020 3:25 PM
>>
>> On 8/11/2020 9:52 AM, Ivan Dyukov wrote:
>>
>> <...>
>>
>>> v9 changes:
>>> * remove rte_eth_link_printf function
>>> * add ETH_LINK_MAX_STR_LEN definition
>>> * add usage of ETH_LINK_MAX_STR_LEN in examples
>>>
>>> v1 changes:
>>> This is initial patchset which introduces UNKNOWN speed to dpdk
>>> applications. Also it contains changes related to printf formating.
>>> Patchset contains changes for app/ and doc/ folders.
>>> examples/ folder will be provided later.
>>>
>>>
>> Hi Ivan,
>>
>> Logging discussion is going on, this is preventing 'unknown' link speed
>> merged
>> and used by drivers. This has potential conflict in more drivers.
>>
>> So I will get those patches (1,4,5,6/24) although logging link speed
>> won't be
>> correct, and logging discussions can continue separately.
>>
>>
>>
>> Related to the link speed logging, the problem we are trying to solve
>> is
>> 'unknown' speed value representation (UINT_MAX) won't be correct and
>> can be
>> confusing, which requires additional check/parsing before log.
>>
>> The proposed solution is a link to string function. Because of the
>> logging
>> difference/needs in the applications the function provides fine grade
>> logging
>> capability, which works fine but I think it is too complex for the
>> problem it is
>> solving.
>> I wonder if the logging link information differences in the
>> applications is an
>> actual need, or it happened by time since those samples/applications
>> developed
>> by different people and common logging function was missing. So perhaps
>> we can
>> switch all to a generic logging.
> Good thinking, Ferruh. I agree that examples probably use different
> formatting by accident, not by deliberate choice.
>> What do you think following:
>>
>> For the immediate need for 'unknown' link speed parsing, can have a
>> 'rte_eth_link_speed_to_str()' function which returns only link speed
>> and
>> applications need custom message can use it for logging.
> That function is unlikely to serve application specific needs for formatting
> speed.
>
> But it might serve the needs of DPDK libraries and examples.
>
>> And can have a simple/brief link to string function for generic usage,
>> and if
>> application want custom message it can parse link struct itself.
>> The link string can be something like: "Link Up 10Gbps full-duplex
>> autoneg"
>>
> For DPDK libraries and examples, I agree that a generic one-format-fits-all
> function should suffice.
>
> And as a DPDK application developer, parsing the link struct ourselves
> suffices for our application. In this context, an strf-style function is
> "nice to have", not "must have".
Almost all examples have same link status formating, except two cases:
testpmd has multiline link status format and another application has
extremely brief status info which is mixed with other text. so I would
prefer to keep such statuses without changes but we should give some
helpers for such cases.I would prefer MACROS:
#define RTE_ETH_LINK_STATUS_TO_STR(link_status) (link_status ==
ETH_LINK_DOWN ? "Down" : "Up")
#define RTE_ETH_LINK_SPEED_TO_STR(link_speed) (link_speed ==
ETH_SPEED_NUM_UNKNOWN ? "Unknown" : \
link_speed ==
ETH_SPEED_NUM_NONE ? "0" : \
link_speed ==
ETH_SPEED_NUM_10M ? "10 Mbps" : \
link_speed ==
ETH_SPEED_NUM_100M ? "100 Mbps" : \
link_speed ==
ETH_SPEED_NUM_1G ? "1 Gbps" : \
link_speed
==ETH_SPEED_NUM_2_5G ? "2.5 Gbps" : \
link_speed
==ETH_SPEED_NUM_5G ? "5 Gbps" : \
link_speed
==ETH_SPEED_NUM_10G ? "10 Gbps" : \
link_speed ==
ETH_SPEED_NUM_20G ? "20 Gbps" : \
link_speed ==
ETH_SPEED_NUM_25G ? "25 Gbps" : \
link_speed ==
ETH_SPEED_NUM_40G ? "40 Gbps" : \
link_speed ==
ETH_SPEED_NUM_50G ? "50 Gbps" : \
link_speed ==
ETH_SPEED_NUM_56G ? "56 Gbps" : \
link_speed == ETH_SPEED_NUM_100G ? "100 Gbps" : \
link_speed == ETH_SPEED_NUM_200G ? "200 Gbps" : \
"Invalid speed")
#define RTE_ETH_LINK_DUPLEX_TO_STR(link_duplex) (link_duplex ==
ETH_LINK_FULL_DUPLEX? "FDX" : "HDX")
#define RTE_ETH_LINK_AUTONEG_TO_STR(link_autoneg) (link_autoneg==
ETH_LINK_FULL_DUPLEX? "Autoneg" : "Fixed")
and one function which construct a default status string:
int rte_eth_link_to_str(char *str, size_t len, const struct rte_eth_link
*eth_link) {
static const char link_down_str[] = "Link down";
static const char link_up_str[] = "Link up at";
if (eth_link->link_status == ETH_LINK_DOWN)
return snprintf(str, len, "%s", link_down_str);
else
return snprintf(str, len, "%s %s %s %s", link_up_str,
RTE_ETH_LINK_SPEED_TO_STR(eth_link->link_speed),
RTE_ETH_LINK_DUPLEX_TO_STR(eth_link->link_duplex),
RTE_ETH_LINK_AUTONEG_TO_STR(eth_link->link_autoneg));
}
> Med venlig hilsen / kind regards
> - Morten Brørup
>
>
>
