From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B0264A04AF; Tue, 11 Aug 2020 14:48:59 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 07F771C022; Tue, 11 Aug 2020 14:48:59 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id E26A21C021 for ; Tue, 11 Aug 2020 14:48:57 +0200 (CEST) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20200811124857euoutp021cbbcb41390d1985a832e69d49c055b0~qN5Z6zInT0446304463euoutp02k for ; Tue, 11 Aug 2020 12:48:57 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20200811124857euoutp021cbbcb41390d1985a832e69d49c055b0~qN5Z6zInT0446304463euoutp02k DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1597150137; bh=Lxs8VXSRaUwiqCr2yp0JyHse3L/G/XxkzmldxZErVMo=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=fhitdiEQQIepJOUhAvNSkVXNo9rI8CUXAGgVvQS7TrSvo5dNtsVif7srqXpYo+Ll8 2zGQwWvP9gH1n2uM72+h+jz+g/PYhSW23aEj8w925VBA3rZWYnuttdFnSKoQJssM7t GXzSFzychWOO37lPumcLUbIEkdZJbY8GvfrcHAko= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20200811124857eucas1p19e3d45100c0f97442919ac080bba9481~qN5Zuvhc12045320453eucas1p1Y; Tue, 11 Aug 2020 12:48:57 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id EC.21.05997.9B3923F5; Tue, 11 Aug 2020 13:48:57 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p2.samsung.com (KnoxPortal) with ESMTPA id 20200811124856eucas1p233d0a7b0aca6c906ad2967e8f7d8c2e1~qN5ZZdF3s1269912699eucas1p28; Tue, 11 Aug 2020 12:48:56 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20200811124856eusmtrp236a643bf6afbbd3a9015a06c16108138~qN5ZY12Kf0687306873eusmtrp2K; Tue, 11 Aug 2020 12:48:56 +0000 (GMT) X-AuditID: cbfec7f4-677ff7000000176d-60-5f3293b984f7 Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id 1A.BC.06314.8B3923F5; Tue, 11 Aug 2020 13:48:56 +0100 (BST) Received: from [106.109.129.29] (unknown [106.109.129.29]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20200811124856eusmtip1a49deaa7e1e323426a143d21ca8357a8~qN5YyDMap0552705527eusmtip1c; Tue, 11 Aug 2020 12:48:56 +0000 (GMT) To: =?UTF-8?Q?Ga=c3=abtan_Rivet?= Cc: dev@dpdk.org, thomas@monjalon.net, ferruh.yigit@intel.com, stephen@networkplumber.org, =?UTF-8?Q?Morten_Br=c3=b8rup?= From: Ivan Dyukov Message-ID: <60cb10d2-a8e9-d4ac-b0d6-ab9c26970990@samsung.com> Date: Tue, 11 Aug 2020 15:48:55 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200811110241.tfucvkcgn7yhlvoo@u256.net> Content-Transfer-Encoding: 8bit Content-Language: ru-RU X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrNKsWRmVeSWpSXmKPExsWy7djPc7o7JxvFGzx4zmzx7tN2Jos7e0+z W+y/dJzNou/XVUaLxXfkLD49OMHiwObxa8FSVo/Fe14yeRy7OY3do+fkPCaPU4+6mT12f5rE EsAWxWWTkpqTWZZapG+XwJXR8nMTe0G/bcWfeY2sDYznNboYOTkkBEwk1s2ZzNzFyMUhJLCC UaJ9y0I2COcLo8SRObMZIZzPjBL3zj1lhmk5MmkiVNVyRokzEzYwQTjvGSXuNJ9l6WLk4BAW iJA4s80VpEFEwEji9rdfYJOYBWYwSqxfdI4VJMEmoCFxumMeE4jNK2AnsWvnI0YQm0VAVeJt UydYjSjQnP+THrNA1AhKnJz5BMzmFDCXOLNzKthFzALyEs1bZ0PZ4hIfth8Eu05CYBO7xKst x1kgznaR2L/rKNQLwhKvjm9hh7BlJP7vnM8E0dDOKDFx22Z2CGcCo0Tz7LdQVfYSW16fYwd5 jVlAU2L9Ln2IsKPEmuermEDCEgJ8EjfeCkIcwScxadt0Zogwr0RHmxBEtZLEgZOXoMISEn9/ 80xgVJqF5LNZSL6ZheSbWQhrFzCyrGIUTy0tzk1PLTbKSy3XK07MLS7NS9dLzs/dxAhMQqf/ Hf+yg3HXn6RDjAIcjEo8vAsmGsULsSaWFVfmHmKU4GBWEuF1Ons6Tog3JbGyKrUoP76oNCe1 +BCjNAeLkjiv8aKXsUIC6YklqdmpqQWpRTBZJg5OqQZGsYc1mbNbn7HsvfPyyAXvTjF9RwOj usbNUaan5kz1ca9feXyprab20x1T5Z0Y99QHK5jwfvXzsNOLfvZ/9sEFb877ZWptjHg+fXb5 g+jnm3inVe3MlPrG+D/sl6mhl8Wkq4E5qV8ZOK8tSUg96GeVctnrZ22Szr/+EO1/p1WmS8j/ WBYa9UpLiaU4I9FQi7moOBEAYAwgSz4DAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrLIsWRmVeSWpSXmKPExsVy+t/xu7o7JhvFGxyaombx7tN2Jos7e0+z W+y/dJzNou/XVUaLxXfkLD49OMHiwObxa8FSVo/Fe14yeRy7OY3do+fkPCaPU4+6mT12f5rE EsAWpWdTlF9akqqQkV9cYqsUbWhhpGdoaaFnZGKpZ2hsHmtlZKqkb2eTkpqTWZZapG+XoJfR 8nMTe0G/bcWfeY2sDYznNboYOTkkBEwkjkyayNbFyMUhJLCUUaLj1hzmLkYOoISExOsnzBA1 whJ/rnVB1bxllPg78w8LSI2wQITEmW2uIDUiAkYSt7/9YgSpYRaYwShx8Nt0doiGWUwSK06v YwKpYhPQkDjdMQ/M5hWwk9i18xEjiM0ioCrxtqmTFcQWBRr6+eARNogaQYmTM5+wgNicAuYS Z3ZOBbuIWcBMYt7mh1C2vETz1tlQtrjEh+0H2SYwCs1C0j4LScssJC2zkLQsYGRZxSiSWlqc m55bbKhXnJhbXJqXrpecn7uJERh124793LyD8dLG4EOMAhyMSjy8CyYaxQuxJpYVV+YeYpTg YFYS4XU6ezpOiDclsbIqtSg/vqg0J7X4EKMp0HMTmaVEk/OBCSGvJN7Q1NDcwtLQ3Njc2MxC SZy3Q+BgjJBAemJJanZqakFqEUwfEwenVAMjh9KqyQZtJ9Oz+3XSDMyf6+ewBywoPiu0cKtG segLCS0+Rh65pM7TV1YZ+u2705K7NNtP4VrbFOFg6TJvyWrDikdhfL3n4u5zl0hNVDbNrud1 FnTn2elfxbt80QS+qXqRwuwvxLiqjbm2y6dL8TMnsmzMPq2p1N4wnWXm6V9Krx+5l98+rMRS nJFoqMVcVJwIAMRBmHjQAgAA X-CMS-MailID: 20200811124856eucas1p233d0a7b0aca6c906ad2967e8f7d8c2e1 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200811085304eucas1p150bf23b6f183a28fbceca06b0bced5af X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200811085304eucas1p150bf23b6f183a28fbceca06b0bced5af References: <20200427095737.11082-1-i.dyukov@samsung.com> <20200811085246.28735-1-i.dyukov@samsung.com> <20200811085246.28735-3-i.dyukov@samsung.com> <20200811110241.tfucvkcgn7yhlvoo@u256.net> Subject: Re: [dpdk-dev] [PATCH v9 02/24] ethdev: add a link status text representation X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > 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 >