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 A69D5A00BE; Tue, 29 Oct 2019 06:20:06 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D261D1BED7; Tue, 29 Oct 2019 06:20:05 +0100 (CET) Received: from mail-il1-f194.google.com (mail-il1-f194.google.com [209.85.166.194]) by dpdk.org (Postfix) with ESMTP id 075A61BECD for ; Tue, 29 Oct 2019 06:20:03 +0100 (CET) Received: by mail-il1-f194.google.com with SMTP id b12so8346572ilf.12 for ; Mon, 28 Oct 2019 22:20:03 -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=mCC0YXV5LS3g2gKAA14v05cMfki3di4g/zt02prwVlo=; b=EJgCYn9TBcfUf5HKyiOgwUbnOxw0bKQSmY7oslakDLpcKvbtPhCllM0vUjvBOCCmNo w3dUTjg5wc/QoOBX9RDBvbkBAy3PQWzXFERFyiWJ8nvIH9FQF84vBc2obr2lNAG+08yE i10odFlr2UlHofHwhi4cYhbDTGpk/ACH5dvDAeL7Qa7OXhBdGHngxgpgeoD8DCkoyHR6 V3zTnPe0nuw7PvJJjFRlLfyjD6CReZb6Gog1PMgEvEsxtFLmPy/iJhEsOAGOjTgepCIM gVSHukB65mn3HGxEBN7VVu/1S9aFuYqgILPK6KRfp3yNgpoGRvdaaA6PRvFDPl8USKX2 sKdA== 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=mCC0YXV5LS3g2gKAA14v05cMfki3di4g/zt02prwVlo=; b=RxWcBijXxcEZn/dZ6PNHuNSCEsAZVKW9/yAJf7DF1rtM2DHTTOjPgEQC4XurwCOQlr aGOAWgXzmsLX6O0qEcjjZAtBQySQDvfDgdQAqrkUwc11RHcIggpki46BdLFlKhj5WJlv IeIr9PBlFqQyNKC211z9RK+VNpgkTaw+na04K9UFRGs6KmHwSTotnE8GE83RSqCzShr7 GKzdixj2gVQ46lGwJ7uIMU0ZIICRzAztKxLmcsMtHkm0W7th4UovIyBeCKY1iUVIr4jX bIQaf9QTkl3n21l88/+W4wy1QP/32ofeJKDh0vUCCV9dsfS2lNVkjcY9indFpG/uIQbH 2Izg== X-Gm-Message-State: APjAAAVbq/8hKz5GuKEy/T+b14aCUdhZX1fJgnveW8pOYx2SLsY3RAxY Ezanf0DV1TdapffFqkgIqSccmECRUEQpCC74kQQ= X-Google-Smtp-Source: APXvYqzNMRjVbKJbcJS9p/KNyliKhKEWyyOuV52ffqaW/rw7r1pa/hwauSwVkX2PwGcTbNpWrLBG7h0CW9tytXNckTw= X-Received: by 2002:a92:aa48:: with SMTP id j69mr24854289ili.162.1572326403082; Mon, 28 Oct 2019 22:20:03 -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 10:49:46 +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 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?