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 05153A0350; Mon, 22 Jun 2020 09:07:23 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id EB2321C2B7; Mon, 22 Jun 2020 09:05:36 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id CF68B1C28F for ; Mon, 22 Jun 2020 09:05:34 +0200 (CEST) IronPort-SDR: Tx08KXKcS6zCLzp2jNHWqPldL9L2a9K6BsCohse/khhUyrMMDdPt7ictEqH8UWh2rN+roRSUIA xbG3uGYGgs6A== X-IronPort-AV: E=McAfee;i="6000,8403,9659"; a="208888040" X-IronPort-AV: E=Sophos;i="5.75,266,1589266800"; d="scan'208";a="208888040" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 22 Jun 2020 00:05:33 -0700 IronPort-SDR: tZRfh/zpjI7y1qgM4gUQvpNYPKPV+XjEHvYDO5u2NkEgrO5M92rSRe6BEpa8WkdsrEMfDXgWVx dRdNnk8fbbgg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,266,1589266800"; d="scan'208";a="274922242" Received: from anishrag-mobl.ir.intel.com (HELO [10.252.29.113]) ([10.252.29.113]) by orsmga003.jf.intel.com with ESMTP; 22 Jun 2020 00:05:29 -0700 To: =?UTF-8?Q?Morten_Br=c3=b8rup?= , Ivan Dyukov , dev@dpdk.org, v.kuramshin@samsung.com, thomas@monjalon.net, david.marchand@redhat.com, arybchenko@solarflare.com, wei.zhao1@intel.com, jia.guo@intel.com, beilei.xing@intel.com, qiming.yang@intel.com, wenzhuo.lu@intel.com, Stephen Hemminger References: <20200427095737.11082-1-i.dyukov@samsung.com> <20200615090158.18912-1-i.dyukov@samsung.com> <20200615090158.18912-3-i.dyukov@samsung.com> <8d238174-0419-1524-95ad-a1434f848e0d@intel.com> <5426ad17-a026-7d74-12b8-09516dbd5c98@samsung.com> <501790b8-d1d3-08e6-4263-b7d8e95fbde9@intel.com> <98CBD80474FA8B44BF855DF32C47DC35C6109F@smartserver.smartshare.dk> From: Ferruh Yigit Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJsBBMBCgBWAhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEABQkKqZZ8FiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl6ha3sXGHZrczovL2tl eXMub3BlbnBncC5vcmcACgkQ+TPrQ98TYR8uLA//QwltuFliUWe60xwmu9sY38c1DXvX67wk UryQ1WijVdIoj4H8cf/s2KtyIBjc89R254KMEfJDao/LrXqJ69KyGKXFhFPlF3VmFLsN4XiT PSfxkx8s6kHVaB3O183p4xAqnnl/ql8nJ5ph9HuwdL8CyO5/7dC/MjZ/mc4NGq5O9zk3YRGO lvdZAp5HW9VKW4iynvy7rl3tKyEqaAE62MbGyfJDH3C/nV/4+mPc8Av5rRH2hV+DBQourwuC ci6noiDP6GCNQqTh1FHYvXaN4GPMHD9DX6LtT8Fc5mL/V9i9kEVikPohlI0WJqhE+vQHFzR2 1q5nznE+pweYsBi3LXIMYpmha9oJh03dJOdKAEhkfBr6n8BWkWQMMiwfdzg20JX0o7a/iF8H 4dshBs+dXdIKzPfJhMjHxLDFNPNH8zRQkB02JceY9ESEah3wAbzTwz+e/9qQ5OyDTQjKkVOo cxC2U7CqeNt0JZi0tmuzIWrfxjAUulVhBmnceqyMOzGpSCQIkvalb6+eXsC9V1DZ4zsHZ2Mx Hi+7pCksdraXUhKdg5bOVCt8XFmx1MX4AoV3GWy6mZ4eMMvJN2hjXcrreQgG25BdCdcxKgqp e9cMbCtF+RZax8U6LkAWueJJ1QXrav1Jk5SnG8/5xANQoBQKGz+yFiWcgEs9Tpxth15o2v59 gXK5Ag0EV9ZMvgEQAKc0Db17xNqtSwEvmfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ES YpV8QWj0xK4YM0dLxnDU2IYxjEshSB1TqAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4Ai bPtrHuIXWQOBECcVZTTOdZYGAzaYzxiAONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxD UQljeNvKYt1lZE/gAUUxNLWsYyTT+22/vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/ 3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35piVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVj sM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQI3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdc q9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYHfVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH7 1PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZqw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFB VOQOxCvwRG2QCgcJ/UTn5vlivul+cThi6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI 8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJlRr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYC GwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNhHwUCXqFrngUJCKxSYAAKCRD5M+tD3xNhH3YWD/9b cUiWaHJasX+OpiuZ1Li5GG3m9aw4lR/k2lET0UPRer2Jy1JsL+uqzdkxGvPqzFTBXgx/6Byz EMa2mt6R9BCyR286s3lxVS5Bgr5JGB3EkpPcoJT3A7QOYMV95jBiiJTy78Qdzi5LrIu4tW6H o0MWUjpjdbR01cnj6EagKrDx9kAsqQTfvz4ff5JIFyKSKEHQMaz1YGHyCWhsTwqONhs0G7V2 0taQS1bGiaWND0dIBJ/u0pU998XZhmMzn765H+/MqXsyDXwoHv1rcaX/kcZIcN3sLUVcbdxA WHXOktGTQemQfEpCNuf2jeeJlp8sHmAQmV3dLS1R49h0q7hH4qOPEIvXjQebJGs5W7s2vxbA 5u5nLujmMkkfg1XHsds0u7Zdp2n200VC4GQf8vsUp6CSMgjedHeF9zKv1W4lYXpHp576ZV7T GgsEsvveAE1xvHnpV9d7ZehPuZfYlP4qgo2iutA1c0AXZLn5LPcDBgZ+KQZTzm05RU1gkx7n gL9CdTzVrYFy7Y5R+TrE9HFUnsaXaGsJwOB/emByGPQEKrupz8CZFi9pkqPuAPwjN6Wonokv ChAewHXPUadcJmCTj78Oeg9uXR6yjpxyFjx3vdijQIYgi5TEGpeTQBymLANOYxYWYOjXk+ae dYuOYKR9nbPv+2zK9pwwQ2NXbUBystaGyQ== Message-ID: <86ad46a3-507a-0d9e-5926-cc84c69f3bac@intel.com> Date: Mon, 22 Jun 2020 08:05:28 +0100 MIME-Version: 1.0 In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35C6109F@smartserver.smartshare.dk> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 2/7] ethdev: add a link status textrepresentation 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" On 6/18/2020 1:32 PM, Morten Brørup wrote: >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit >> Sent: Thursday, June 18, 2020 2:03 PM >> >> On 6/18/2020 11:08 AM, Ivan Dyukov wrote: >>> Hi Ferruh, >>> >>> Thank you for the comments. >>> >>> My answers are inlined. >>> >>> Best regards, >>> Ivan >>> 17.06.2020 19:45, Ferruh Yigit пишет: >>>> On 6/15/2020 10:01 AM, Ivan Dyukov wrote: >>>>> This commit add function which treat link status structure >>>>> and format it to text representation. >>>> If I am following correctly, the initial need was to escape from >> speed checks >>>> everytime loging link information caused by this new 'unknown' >> speed. >>>> >>>> And later suggestion was to have a pre-formatted text for link >> logging. >>> Correct. >>>> >>>> This patch brings additional link status printing/formatting >> capability with >>>> custom format string support and with new format specifiers for link >> (like, '%D' >>>> link duplex state), >>>> although this is nice work and thanks for it, I am not sure this >> complexity and >>>> two new APIs are really needed. >>> Yep. >>>> For me only 'rte_eth_link_format()' without custom format support >> looks good >>>> enough but I won't object if the concensus is to have them. >>>> I am aware there are multiple applications you are updating logging >> slightly >>>> different which requires this flexibility but what happens if they >> use same >>>> pre-formatted text, is that difference really required or happened >> by time based >>>> on developers taste? >>> I have changed only few applications but I have plan to change all >> dpdk >>> examples. Even in those few apps, we have various link status logging >>> text. e.g  app/test-pmd/config.c >>> @@ -600,10 +600,9 @@ port_infos_display(portid_t port_id) >>>         } else >>>                 printf("\nmemory allocation on the socket: >>> %u",port->socket_id); >>> >>> -       printf("\nLink status: %s\n", (link.link_status) ? ("up") : >>> ("down")); >>> -       printf("Link speed: %u Mbps\n", (unsigned) link.link_speed); >>> -       printf("Link duplex: %s\n", (link.link_duplex == >>> ETH_LINK_FULL_DUPLEX) ? >>> -              ("full-duplex") : ("half-duplex")); >>> +       rte_eth_link_printf("\nLink status: %S\n" >>> +                           "Link speed: %M Mbps\n" >>> +                           "Link duplex: %D\n", &link); >>> the status is logged in 3 lines. this is special text layoting and I >>> don't know how it will look in one line. Myabe it will require >> reformat >>> all screen. I don't want to change screen layoting in this patchset. >> >> cc'ed Morten & Stephen. >> >> To keep existing layout in the applications yes you need more >> flexibility, a >> pre-formatted text won't cut it, but I guess I prefer your approach in >> v2 for >> simplicity. >> >> And I would prefer extending the one in v2 with a larger string for the >> pre-formatted text, so both flexibility and the standard output can be >> provided, >> Morten didn't like it but if I understand correctly his comment was to >> keep more >> simple solution in ethdev and applications can do it themselves if they >> want >> more custom log formatting, but this patch adds more complex and >> flexible >> logging support to the ethdev. >> > > If you are going to add a flexible link status formatting function, like strftime(), it should not print to stdout, but to a string buffer, like strftime(). It will also allow for new format specifiers in the future, e.g. %1D for "FDX"/"HDX", %2D for "Full"/"Half" or %3D for "full-duplex"/"half-duplex". This would provide more flexible support for a larger number of application specific formats. These will make it even more complex, do we really need this? > > Then the function for printing to stdout in a DPDK preferred format (if you add such a function to the library) can use the above eth_link_strf() function. > >