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 4177AA00BE; Tue, 29 Oct 2019 06:47:23 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0213C1BEE0; Tue, 29 Oct 2019 06:47:23 +0100 (CET) Received: from mail-il1-f193.google.com (mail-il1-f193.google.com [209.85.166.193]) by dpdk.org (Postfix) with ESMTP id 0B2981BED7 for ; Tue, 29 Oct 2019 06:47:22 +0100 (CET) Received: by mail-il1-f193.google.com with SMTP id s6so6761369iln.0 for ; Mon, 28 Oct 2019 22:47:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ERMVaAvK/PU0xaa5JcP39KU75ECrEq+GvL5IeFaxR6s=; b=Hn4D8UgSjGjjJQ9KuxiNozpOxOHE+on49QsBJmMqjXYQAYVzRFqIiG94JugUMyRTjt qITuItWh0NKiUe9kuNoC+cPJSQrRP+lTsuO24dwuG1WTAaZnnCK3UwGsrcA7hdPCo91t XxfUzqOqMUooTEl11l9GE+mGC6WIi0D5m9WWoy/D1qbvxdEedyutZr+2plHSyZIm0exM P6B49wkvP5ckOZMOCvpTEU7+sjvQOKDe1GyslJYzQmugf6WXLvhrWgX6gxpb8jH3yvFw 09iW/N8wrWmdeyb46TUX+6Y/QiPq1T01lqOCP0TrBEv6jnqrrZdtW7Vf2wYFBmbwWKiO q41w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ERMVaAvK/PU0xaa5JcP39KU75ECrEq+GvL5IeFaxR6s=; b=h9e8Dah8NMWaP13wfJuvKxCo5Bt5CaK4noAefQycSbKvNUZ887KPFgl9CuIeeU2ZaC hVlgGRTRtO2MLaAbEagycSytwW+TELVZfnyuUxwjWwxzAsy75mEdIPOugj4OdOsm5o5W hkOJDUapiGNPN4bqD5TvhkrBDAOzLYfa5dGtdLM8cXv113F3OtJeP/IHCXzTnY2Ipr7S 8vMiv4HS97yMleQ2W9XN6exqFqGLNR9Euho0Sp9jqzmQZPSLtMZMZ8UAOmMOcuefk3e9 0o62XJAKa9u88HZHE82aJu7aQOkPzeQuBa41rlH5rxfF5DhMTA8cYf5nG9NExmioCukK H4iw== X-Gm-Message-State: APjAAAU3FakKFrBbQ/t4JJ7Puhbj2Qp+5138pVIcWfNxo8JkQxB6lqQA fAJuAS+4VNrMDjDBBBn68ZNLZOpEEP/LampwuoE= X-Google-Smtp-Source: APXvYqzv2PwKCtGugOc9O+qE7ysCLMMqAeD+tyTURJ0zlqo+2oNNLtsFdrxHiL+fAbMJsBckSQiBeUFzkzairfnQWfQ= X-Received: by 2002:a92:99c5:: with SMTP id t66mr15101706ilk.271.1572328041127; Mon, 28 Oct 2019 22:47:21 -0700 (PDT) MIME-Version: 1.0 References: <20191015075133.38560-1-haiyue.wang@intel.com> <1811898.7XjjD7ZjLQ@xps> <12001140.UMXFOKstgs@xps> In-Reply-To: From: Jerin Jacob Date: Tue, 29 Oct 2019 11:17:04 +0530 Message-ID: To: "Wang, Haiyue" Cc: Thomas Monjalon , "Yigit, Ferruh" , dpdk-dev , "Ye, Xiaolong" , "Kinsella, Ray" , "Iremonger, Bernard" , "Sun, Chenmin" , Andrew Rybchenko , Slava Ovsiienko , Stephen Hemminger , David Marchand , Jerin Jacob Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information 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 Tue, Oct 29, 2019 at 11:12 AM Wang, Haiyue wrote: > > > -----Original Message----- > > From: Jerin Jacob > > Sent: Tuesday, October 29, 2019 13:20 > > To: Wang, Haiyue > > Cc: Thomas Monjalon ; Yigit, Ferruh ; dpdk-dev > > ; Ye, Xiaolong ; Kinsella, Ray ; > > Iremonger, Bernard ; Sun, Chenmin ; Andrew > > Rybchenko ; Slava Ovsiienko ; Stephen Hemminger > > ; David Marchand ; Jerin Jacob > > > > Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information > > > > On Tue, Oct 29, 2019 at 10:14 AM Wang, Haiyue wrote: > > > > > > > -----Original Message----- > > > > From: Jerin Jacob > > > > Sent: Tuesday, October 29, 2019 11:38 > > > > To: Wang, Haiyue > > > > Cc: Thomas Monjalon ; Yigit, Ferruh ; dpdk-dev > > > > ; Ye, Xiaolong ; Kinsella, Ray ; > > > > Iremonger, Bernard ; Sun, Chenmin ; Andrew > > > > Rybchenko ; Slava Ovsiienko ; Stephen > > Hemminger > > > > ; David Marchand ; Jerin Jacob > > > > > > > > Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add the API for getting burst mode information > > > > > > > > > > > > struct rte_eth_burst_mode { > > > > > > > > uint64_t options; > > > > > > > > char dev_specific[128]; /* PMD has specific burst mode information */ > > > > > > > > }; > > > > > > > > > > > > > > I really don't see how we can have generic flags. > > > > > > > The flags which are proposed are just matching > > > > > > > the functions implemented in Intel PMDs. > > > > > > > And this is a complicate solution. > > > > > > > Why not just returning a name for the selected Rx/Tx mode? > > > > > > > > > > > > +1 only for the name > > > > > > > > > > > > Let me clarify my earlier proposal: > > > > > > > > > > > > 1) The public ethdev API should return only "string" i.e the flags > > > > > > SHOULD NOT be exposed as ethdev API > > > > > > i.e > > > > > > int rte_eth_tx_burst_mode_name(uint16_t port_id, uint16_t queue_id, char *name); > > > > > > > > > > > > 2) The PMD interface to the common code can be following > > > > > > > > > > > > struct eth_pmd_burst_mode { > > > > > > uint64_t options; > > > > > > char name[128]; /* PMD specific burst mode information */ > > > > > > }; > > > > > > > > > > > > typedef int (*eth_burst_mode_get_t)(struct rte_eth_dev *dev, > > > > > > uint16_t queue_id, struct eth_burst_mode *mode) > > > > > > > > > > > > 3) The implementation of rte_eth_tx_burst_mode_name() shall do optons > > > > > > flag to string converion(again internal to common code implemetation) > > > > > > and concatenate with eth_pmd_burst_mode::name > > > > > > > > > > > > This would help to reuse some of the flags to name conversion logic > > > > > > across all PMDs. > > > > > > And PMD are free to return eth_pmd_burst_mode::options as zero in > > > > > > that case final > > > > > > string only be eth_pmd_burst_mode::name. > > > > > > > > > > > > > > > > In fact, 'rte_eth_burst_mode_option_name' for single option, not > > > > > for struct eth_pmd_burst_mode::option[s]. Need loop to display them. > > > > > > > > I see two issues with the flag approach in public API(Internally for > > > > common code it fine to avoid code duplication) > > > > > > > > 1) We can not standardize all flags when it comes to HW specific > > > > details. We should NOT pollute public API with HW specific details. > > > > > > Currently, no detail to HW NIC specific. > > > > Yes. What if I want to add a "String" they represent a specific mode of PMD, > > so that I know what mode PMD really runs. > > It is not worth adding a flag for that in public API for HW specific notion. > > That's the problem. > > > > > > > > > 2) There is a danger if application starts taking any action based on > > > > flags. It should be only for display purpose so in that case public > > > > API should be the string to avoid misuse of the API(eventually the app > > > > will fail on some PMD > > > > if it takes any action based on the flag) > > > > > > These flags are *read only* for information. Can't image how to hack DPDK. ;-) > > > > To clarify: > > If we expose flag say RTE_ETH_BURST_SIMPLE then the application can take > > some action based on > > if (flag == RTE_ETH_BURST_SIMPLE) > > do_some_thing(); > > > > If the purpose is ONLY for "display" as info then exposing as the string will > > enable to NOT standardize i.e application can never check based on > > the string name(as it is not standardized) hence no danger. > > > > So what is the purpose of this API? Just display or are you expecting > > the application can do any action based on this? > > Oh, I see. Mainly for showing which burst rx/tx module running: If so, the public API should be as string to avoid any other interpreation of flags in application. And it makes application life easy too. > > https://docs.fd.io/vpp/18.11/d7/d1d/plugins_2dpdk_2device_2format_8c_source.html > > s = format (s, "%Utx burst function: %s\n", > 579 format_white_space, indent + 2, > 580 ptr2sname (rte_eth_devices[xd->port_id].tx_pkt_burst)); > 581 s = format (s, "%Urx burst function: %s\n", > 582 format_white_space, indent + 2, > 583 ptr2sname (rte_eth_devices[xd->port_id].rx_pkt_burst)); > > https://docs.fd.io/vpp/18.11/d7/d1d/plugins_2dpdk_2device_2format_8c_source.html > > 488 static const char * > 489 ptr2sname (void *p) > 490 { > 491 Dl_info info = { 0 }; > 492 > 493 if (dladdr (p, &info) == 0) > 494 return 0; > 495 > 496 return info.dli_sname; > 497 } > > tx burst function: ixgbe_xmit_pkts > rx burst function: ixgbe_recv_pkts > > If the PMD's rx/tx is *static* function, 'ptr2name' returns 'nil'.