Message ID | 20200811085246.28735-1-i.dyukov@samsung.com (mailing list archive) |
---|---|
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id CB59DA04AF; Tue, 11 Aug 2020 10:53:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B0DCD1C022; Tue, 11 Aug 2020 10:53:04 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id 156BD1C01F for <dev@dpdk.org>; Tue, 11 Aug 2020 10:53:02 +0200 (CEST) Received: from eucas1p1.samsung.com (unknown [182.198.249.206]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20200811085301euoutp02b3f60a2fa5509fa3f1aef56373fdbeb3~qKrajZVnP0711807118euoutp02o; Tue, 11 Aug 2020 08:53:01 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20200811085301euoutp02b3f60a2fa5509fa3f1aef56373fdbeb3~qKrajZVnP0711807118euoutp02o DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1597135981; bh=N9qXjqbpFkWsbCVgUFEk7MObbiHhRmVjVRIrrFPwnrY=; h=From:To:Subject:Date:In-Reply-To:Reply-To:References:From; b=nOFu6Ct/OAZfYW2/7Lf80yihUONUiM3I/NwivN/XjseL4smralEsh/xR2JWT6FVMw +HF5gAiadH14LifdLM60nBooFlyttVUoBVDmMnatLeM5HyoJWJ1sHHXnLdAt8b8CFI KAg5kfHmq9s72yg7GpvwvJ/gxe+i6nfjqrA35QBQ= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p2.samsung.com (KnoxPortal) with ESMTP id 20200811085301eucas1p25396a5f27a548c84dea5ace51e3e1072~qKraMLF1u3145931459eucas1p2p; Tue, 11 Aug 2020 08:53:01 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 53.DD.06318.D6C523F5; Tue, 11 Aug 2020 09:53:01 +0100 (BST) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20200811085300eucas1p17b6087e1ba9a84f7cbefd6f042a6ddaa~qKrZweaVH1829518295eucas1p11; Tue, 11 Aug 2020 08:53:00 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20200811085300eusmtrp1f9d7a39883e3d7f8a281e08a64571453~qKrZvFn8B1603916039eusmtrp1N; Tue, 11 Aug 2020 08:53:00 +0000 (GMT) X-AuditID: cbfec7f5-38bff700000018ae-bd-5f325c6debbb Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id 88.3D.06017.C6C523F5; Tue, 11 Aug 2020 09:53:00 +0100 (BST) Received: from idyukov.rnd.samsung.ru (unknown [106.109.129.29]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20200811085258eusmtip1cf3e997328a479f380df641e3f4a065e~qKrX1YPQ11725317253eusmtip1d; Tue, 11 Aug 2020 08:52:58 +0000 (GMT) From: Ivan Dyukov <i.dyukov@samsung.com> To: dev@dpdk.org, i.dyukov@samsung.com, v.kuramshin@samsung.com, thomas@monjalon.net, david.marchand@redhat.com, ferruh.yigit@intel.com, arybchenko@solarflare.com, wei.zhao1@intel.com, jia.guo@intel.com, beilei.xing@intel.com, qiming.yang@intel.com, wenzhuo.lu@intel.com, mb@smartsharesystems.com, stephen@networkplumber.org, nicolas.chautru@intel.com, bruce.richardson@intel.com, konstantin.ananyev@intel.com, cristian.dumitrescu@intel.com, radu.nicolau@intel.com, akhil.goyal@nxp.com, declan.doherty@intel.com, skori@marvell.com, pbhagavatula@marvell.com, jerinj@marvell.com, kirankumark@marvell.com, david.hunt@intel.com, anatoly.burakov@intel.com, xiaoyun.li@intel.com, jingjing.wu@intel.com, john.mcnamara@intel.com, jasvinder.singh@intel.com, byron.marohn@intel.com, yipeng1.wang@intel.com Date: Tue, 11 Aug 2020 11:52:19 +0300 Message-Id: <20200811085246.28735-1-i.dyukov@samsung.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200427095737.11082-1-i.dyukov@samsung.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA01SeUwTaRTPNzOdGRprhmL0BY1KjYqbgAdqHqvruZtMjPFITFwNihUnYKC1 acVd/cNUcdEURA6PUBQqREVYgiy0Ra0EaxCwUIIneFbDLquiWAQjAiplMPrf+x3v/d778vG0 2s6G8jv1uyWjXpukYZWM42a/N0IXMy92Tt2/IVjelE/wxdMiGn3Hn7CY9u4QhW0lS9HlPK3A 7GdmCjOy/RQ6iy0sdvkOMvi2x0nh42seDq23D3LY4+7k8GhLHsHOwVc05treE2zJtyuw//oV FrsHCxnM+HSP4LGqBoL16X4Gj1+uJphqEfDdxQscFj2ejD2+BgY97ZUc/t3XSqO9aYhgVn05 webUL4plYeIn2zmFWOR6SYk5Z1tp8Wb7SU5Mb8ynxIq31ZTYXXOPFTOqSoh460UaLZZ2fGTX KTcrF++QknbukYyzl2xTJhzI+8XwKPzPsrsrzKQs1EJ4HoT50GqeYiFKXi0UE6g7kamQQS+B o+l+IoP3BGo7XcMgaKRjoOEQIwsXCOQ6s0ZBH4EDxxwjLlYIB8+RfCogjBMqWHAOPqACQojw M9jKGplAzQjT4aGjdYRXCdHQ1nGDlSOmQOmlWjqwYNCw/78vcQFaLUyGBwW9o/ZgaMztGBlD D9tT7Hl0IAuEjzz4TjYp5Dm/grnZTsl1CLyqr+LkehJ4ctIZueEwgSxHJSeDTAIpeW9GXUuh 6rWXC2xBC7Og/MpsmV4OaWc+sPLrjYW2N8HyEmMh23GKlmkVHElVy24N1DbeHqUBhgbGyLQI /1/+wGSSMOsP11h/uMb6PdZG6BIyQUo26eIlU5Re+iPSpNWZkvXxkXG7dP+Q4c/s+VzfV01q Bre7icATzRiVYdHcWLVCu8e0V+cmwNOacaoVzZ6tatUO7d59knFXrDE5STK5yUSe0UxQRRW+ 3KIW4rW7pURJMkjGbyrFB4WaiSt3Y2H259fWVY8SVRGbZlztuJXR/Xvw+DtJ3uK7q9TrqiOj up72SPFnQuvObxp4GH19fVf0NMEdsyC64pLX9jzBG2boxYWF7QOW5S2J2+5PXbO/ICxzraCP WV2w8vCpOFdNSsTMReLK9ZL/2cImx2+V8xpyug2R/X9F+bM2+IYW4AwNY0rQzv2JNpq0XwFc zcf4yAMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA02Sf0yMcRzHfZ/vc89dzdnjZL67TXSY/LrcVe5zEdaMx2bG8hdxbvWsTl13 u+dC/HEhtAfJj/lx2Tma0ZGUUhHjbCqHVpKoaDRGLbowUrgT47/3Pu/X+/3ZZ/vIsKJGopSZ Mu28LdOYoWJCad+Pus45GUlaw1xvfxSUPnAhePWiCEPX0U4G9n3MpaDNswhqq05J4PDLHAry D/dTUHVBZKC3aycNff4qCjpu+qTgbN4pBb/3rRQONBYieDv0HsNJ9wCCRlelBL7duc7Ah6Gz NOQPPkFwsKIeQd3+fhqO1lQj2COy8LH4vBSKOsLB31VPg+/ZVSlc+tyEofLBMIJDdaUIHu75 KVkcwQ26z0m4otp3FHfkTBPm7j07JuX2N7gorqyvmuI+3HrCcPkVHsTdf7UPcxe7vzKrQteq F9gsWXZ+cppFsMer1mlAq9boQa2N0as10br1cdpYVdTCBSl8hmkzb4tauFGdtqMw3toeubWk JSEHlShFFCIjbAz5Xp9LiyhUpmDPIdJy1SkVkSxgENLTjUeYcWSoVWRGmAFEyo8XUUGDYSOJ L89FBY0wtp0hj/fuoIPGODaOuEsafmuanUaeX2v6HZCzetLWfZcZaZ1ELl65jYPLQgL8m5/J wbEigPhf5NEjOpw8Pf3pT3QsaTjZTQdxzE4npS5FcIwDLbsqC3EBGuv8j3L+o5z/UW6EPSiM zxLMqWZBqxaMZiErM1WdbDGXo8DbXLv3raIaiX2JXsTKkGq03DpfY1BIjJuFbLMXERlWhckT Hvo2KOQpxuxtvM1isGVl8IIXxQauPISV45MtgSfMtBs0sRod6DW6aF30PFBNkOexd5IUbKrR zqfzvJW3/c1RshBlDlrj3rQxJm7UjNEJs1pHFfgn26xlva+3JfYMyP3mFV+urH7kwMOG3ulb dim2z5x994S1bHznynm7Ha2etROXrb+xvKbdxCZtrxEdHcqlbfGnixfppmBlc4843LuuOPGU dUlx8mAHqc9WbknXO46n+8f8yA2/PDWm3BPf2hAZ0WyyLlXRQppRMxPbBOMvNCajX0wDAAA= X-CMS-MailID: 20200811085300eucas1p17b6087e1ba9a84f7cbefd6f042a6ddaa X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200811085300eucas1p17b6087e1ba9a84f7cbefd6f042a6ddaa X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200811085300eucas1p17b6087e1ba9a84f7cbefd6f042a6ddaa References: <20200427095737.11082-1-i.dyukov@samsung.com> <CGME20200811085300eucas1p17b6087e1ba9a84f7cbefd6f042a6ddaa@eucas1p1.samsung.com> Subject: [dpdk-dev] [PATCH v9 00/24] ethdev: allow unknown link speed X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: i.dyukov@samsung.com List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Series |
ethdev: allow unknown link speed
|
|
Message
Ivan Dyukov
Aug. 11, 2020, 8:52 a.m. UTC
MAINTAINERS | 1 + app/proc-info/main.c | 9 +- app/test-pipeline/init.c | 11 ++- app/test-pmd/config.c | 20 +++-- app/test-pmd/testpmd.c | 9 +- app/test/Makefile | 3 + app/test/meson.build | 2 + app/test/test_ethdev_link.c | 299 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ app/test/test_pmd_perf.c | 17 ++-- doc/guides/sample_app_ug/link_status_intr.rst | 10 +-- drivers/net/i40e/i40e_ethdev.c | 5 +- drivers/net/i40e/i40e_ethdev_vf.c | 10 +-- drivers/net/ice/ice_ethdev.c | 5 +- drivers/net/ixgbe/ixgbe_ethdev.c | 6 +- examples/bbdev_app/main.c | 8 +- examples/ioat/ioatfwd.c | 13 ++- examples/ip_fragmentation/main.c | 13 ++- examples/ip_pipeline/cli.c | 12 +-- examples/ip_reassembly/main.c | 12 +-- examples/ipsec-secgw/ipsec-secgw.c | 12 +-- examples/ipv4_multicast/main.c | 12 +-- examples/kni/main.c | 26 ++---- examples/l2fwd-crypto/main.c | 12 +-- examples/l2fwd-event/main.c | 12 +-- examples/l2fwd-jobstats/main.c | 12 +-- examples/l2fwd-keepalive/main.c | 12 +-- examples/l2fwd/main.c | 12 +-- examples/l3fwd-acl/main.c | 12 +-- examples/l3fwd-graph/main.c | 14 +-- examples/l3fwd-power/main.c | 13 +-- examples/l3fwd/main.c | 12 +-- examples/link_status_interrupt/main.c | 30 +++---- examples/multi_process/client_server_mp/mp_server/init.c | 14 ++- examples/multi_process/symmetric_mp/main.c | 12 +-- examples/ntb/ntb_fwd.c | 10 +-- examples/performance-thread/l3fwd-thread/main.c | 12 +-- examples/qos_sched/init.c | 10 +-- examples/server_node_efd/server/init.c | 15 ++-- examples/vm_power_manager/main.c | 14 ++- lib/librte_ethdev/rte_ethdev.c | 174 ++++++++++++++++++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev.h | 76 ++++++++++++++--- lib/librte_ethdev/rte_ethdev_version.map | 4 + 42 files changed, 715 insertions(+), 282 deletions(-) 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 v8 changes: * rename rte_eth_link_strf to rte_eth_link_to_str * refactor rte_eth_link_to_str according to review comments * fix codestyle * fix commit message in 02 patch * fix compile error in ntb application * merge "app" and "doc" commits v7 changes: * fix meson build * change _strf function. now it does not fails in case of unknown specifiers like %d. it just copy it to target string. * remove invalid_fmt unit test. * add unknown specifier test. * fix codestyle v6 changes: * fix spelling in comments according to checkpatch warning v5 changes: * rename rte_eth_link_format to rte_eth_link_strf * add '\n' to default strings * update remaining examples. patch with subj 'examples: new link status print format' contains examples which have no maintainers. TBD: update remaining nic drivers with 'unknown' speed. It should be provided in separate patchset. v4 changes: * refactor rte_eth_link_format using strlcat func instead of snprintf * added new checks to unit tests * few minor fixes according review comments TBD: update examples in 'example' folder with new status printing mechanism update remaining nic drivers with 'unknown' speed v3 changes: * remove rte_eth_link_prepare_text function * add rte_eth_link_format and rte_eth_link_printf functions * added unit tests for rte_eth_link_format function TBD: update examples in 'example' folder with new status printing mechanism update remaining nic drivers with 'unknown' speed v2 changes: * add function which format link status to textual representation * update drivers for Intel nics with 'unknown' speed TBD: update examples in 'example' folder with new status printing mechanism update remaining nic drivers with 'unknown' speed 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.
Comments
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 > > v8 changes: > * rename rte_eth_link_strf to rte_eth_link_to_str > * refactor rte_eth_link_to_str according to review comments > * fix codestyle > * fix commit message in 02 patch > * fix compile error in ntb application > * merge "app" and "doc" commits > > v7 changes: > * fix meson build > * change _strf function. now it does not fails in case of unknown specifiers like %d. it just copy it to target string. > * remove invalid_fmt unit test. > * add unknown specifier test. > * fix codestyle > > v6 changes: > * fix spelling in comments according to checkpatch warning > > v5 changes: > * rename rte_eth_link_format to rte_eth_link_strf > * add '\n' to default strings > * update remaining examples. patch with subj 'examples: new link status print format' contains examples which have no maintainers. > TBD: > update remaining nic drivers with 'unknown' speed. It should be provided in separate patchset. > > v4 changes: > * refactor rte_eth_link_format using strlcat func instead of snprintf > * added new checks to unit tests > * few minor fixes according review comments > TBD: > update examples in 'example' folder with new status printing mechanism > update remaining nic drivers with 'unknown' speed > > v3 changes: > * remove rte_eth_link_prepare_text function > * add rte_eth_link_format and rte_eth_link_printf functions > * added unit tests for rte_eth_link_format function > TBD: > update examples in 'example' folder with new status printing mechanism > update remaining nic drivers with 'unknown' speed > > v2 changes: > * add function which format link status to textual representation > * update drivers for Intel nics with 'unknown' speed > TBD: > update examples in 'example' folder with new status printing mechanism > update remaining nic drivers with 'unknown' speed > > 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. 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. 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"
> From: dev [mailto:dev-bounces@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". Med venlig hilsen / kind regards - Morten Brørup
Hi Ferruh, Morten, 07.09.2020 18:29, Morten Brørup пишет: >> From: dev [mailto:dev-bounces@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 > > >
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ivan Dyukov > Sent: Tuesday, September 8, 2020 8:16 AM > > Hi Ferruh, Morten, > > 07.09.2020 18:29, Morten Brørup пишет: > >> From: dev [mailto:dev-bounces@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)); > > } I think that the DPDK developer community has a preference for functions over macros. This is not in the fast path, so there is no need for macros. Make the macros functions instead, and consider it... Acked-by: Morten Brørup <mb@smartsharesystems.com>