From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Iremonger, Bernard" Subject: Re: [PATCH v5 29/29] net/i40e: set/clear VF stats from PF Date: Thu, 22 Dec 2016 16:38:45 +0000 Message-ID: <8CEF83825BEC744B83065625E567D7C224D1894F@IRSMSX108.ger.corp.intel.com> References: <20161216143919.4909-1-ferruh.yigit@intel.com> <20161216190257.6921-1-ferruh.yigit@intel.com> <20161216190257.6921-30-ferruh.yigit@intel.com> <587a2f08-0541-3f99-d16f-1c14206f11e9@intel.com> <8CEF83825BEC744B83065625E567D7C224D17A76@IRSMSX108.ger.corp.intel.com> <6A0DE07E22DDAD4C9103DF62FEBC09093B54C602@shsmsx102.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "Wu, Jingjing" , "Zhang, Helin" , "Zhang, Qi Z" , "Chen, Jing D" , "Iremonger, Bernard" To: "Lu, Wenzhuo" , "Yigit, Ferruh" , "dev@dpdk.org" Return-path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 65F4310DA7 for ; Thu, 22 Dec 2016 17:38:49 +0100 (CET) In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093B54C602@shsmsx102.ccr.corp.intel.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > -----Original Message----- > From: Lu, Wenzhuo > Sent: Wednesday, December 21, 2016 12:56 AM > To: Iremonger, Bernard ; Yigit, Ferruh > ; dev@dpdk.org > Cc: Wu, Jingjing ; Zhang, Helin > ; Zhang, Qi Z ; Chen, Jing D > > Subject: RE: [dpdk-dev] [PATCH v5 29/29] net/i40e: set/clear VF stats fro= m PF >=20 > Hi all, >=20 >=20 > > -----Original Message----- > > From: Iremonger, Bernard > > Sent: Tuesday, December 20, 2016 9:40 PM > > To: Yigit, Ferruh; dev@dpdk.org > > Cc: Wu, Jingjing; Zhang, Helin; Zhang, Qi Z; Lu, Wenzhuo; Chen, Jing D > > Subject: RE: [dpdk-dev] [PATCH v5 29/29] net/i40e: set/clear VF stats > > from PF > > > > Hi Ferruh, > > > > > -----Original Message----- > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit > > > Sent: Tuesday, December 20, 2016 1:25 PM > > > To: dev@dpdk.org > > > Cc: Wu, Jingjing ; Zhang, Helin > > > ; Zhang, Qi Z ; Lu, > > > Wenzhuo ; Chen, Jing D > > > > Subject: Re: [dpdk-dev] [PATCH v5 29/29] net/i40e: set/clear VF > > > stats from PF > > > > > > On 12/16/2016 7:02 PM, Ferruh Yigit wrote: > > > > From: Qi Zhang > > > > > > > > This patch add support to get/clear VF statistics from PF side. > > > > Two APIs are added: > > > > rte_pmd_i40e_get_vf_stats. > > > > rte_pmd_i40e_reset_vf_stats. > > > > > > > > Signed-off-by: Qi Zhang > > > > --- > > > > > > <...> > > > > > > > diff --git a/drivers/net/i40e/rte_pmd_i40e_version.map > > > > b/drivers/net/i40e/rte_pmd_i40e_version.map > > > > index 8ac1bc8..7a5d211 100644 > > > > --- a/drivers/net/i40e/rte_pmd_i40e_version.map > > > > +++ b/drivers/net/i40e/rte_pmd_i40e_version.map > > > > @@ -6,7 +6,9 @@ DPDK_2.0 { > > > > DPDK_17.02 { > > > > global: > > > > > > > > + rte_pmd_i40e_get_vf_stats; > > > > rte_pmd_i40e_ping_vfs; > > > > + rte_pmd_i40e_reset_vf_stats; > > > > rte_pmd_i40e_set_tx_loopback; > > > > rte_pmd_i40e_set_vf_broadcast; > > > > rte_pmd_i40e_set_vf_mac_addr; > > > > > > Hi Wenzhuo, Mark, > > > > > > I think this is the list of all APIs added with this patchset. > > > > > > Just a question, what do you think following a logic in API naming as= : > > > __ ? > > > > > > So API names become: > > > rte_pmd_i40e_tx_loopback_set; > > > rte_pmd_i40e_vf_broadcast_set; > > > rte_pmd_i40e_vf_mac_addr_set; > > > rte_pmd_i40e_vfs_ping; > > > rte_pmd_i40e_vf_stats_get; > > > rte_pmd_i40e_vf_stats_reset; > > > > > > > > > After above rename, rte_pmd_i40e_tx_loopback_set() is not giving a > > > hint that this is something related to the PF controlling VF, > > > perhaps we can rename the API ? > > > > > > Also rte_pmd_i40e_vfs_ping() can become rte_pmd_i40e_vf_ping_all() > > > to be more consistent about _vf_ usage. > > > > > > Overall, they can be something like: > > > rte_pmd_i40e_vf_broadcast_set; > > > rte_pmd_i40e_vf_mac_addr_set; > > > rte_pmd_i40e_vf_ping_all; > > > rte_pmd_i40e_vf_stats_get; > > > rte_pmd_i40e_vf_stats_reset; > > > rte_pmd_i40e_vf_tx_loopback_set; > > > > > > What do you think? > > > > > > > I think the naming should be consistent with what has already been > > implemented for the ixgbe PMD. > > rte_pmd_ixgbe_set_all_queues_drop_en; > > rte_pmd_ixgbe_set_tx_loopback; > > rte_pmd_ixgbe_set_vf_mac_addr; > > rte_pmd_ixgbe_set_vf_mac_anti_spoof; > > rte_pmd_ixgbe_set_vf_split_drop_en; > > rte_pmd_ixgbe_set_vf_vlan_anti_spoof; > > rte_pmd_ixgbe_set_vf_vlan_insert; > > rte_pmd_ixgbe_set_vf_vlan_stripq; > > > > rte_pmd_ixgbe_set_vf_rate_limit; > > rte_pmd_ixgbe_set_vf_rx; > > rte_pmd_ixgbe_set_vf_rxmode; > > rte_pmd_ixgbe_set_vf_tx; > > rte_pmd_ixgbe_set_vf_vlan_filter; > So, seems better to use the current names. Rework both ixgbe and i40e's > later. Not sure if it'll be counted as the ABI change if we change the ix= gbe's > name. >=20 A similar naming convention was used originally in the ethdev: rte_eth_dev_set_vf_rxmode rte_eth_dev_set_vf_rx rte_eth_dev_set_vf_tx rte_eth_dev_set_vf_vlan_filter rte_eth_dev_set_vf_rate_limit rte_eth_dev has just been replaced with rte_pmd_ Regards, Bernard.