From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 5B0FC255 for ; Thu, 29 Mar 2018 16:53:17 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 29 Mar 2018 07:53:16 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,376,1517904000"; d="scan'208";a="28025714" Received: from dwdohert-mobl.ger.corp.intel.com (HELO [10.252.2.125]) ([10.252.2.125]) by fmsmga007.fm.intel.com with ESMTP; 29 Mar 2018 07:53:11 -0700 To: Shahaf Shuler , "dev@dpdk.org" Cc: Alex Rosenbaum , Ferruh Yigit , Thomas Monjalon , Qi Zhang , Alejandro Lucero , Andrew Rybchenko , Mohammad Abdul Awal , Remy Horton , John McNamara , Rony Efraim , Wu , Jingjing , Lu , Wenzhuo , Vincent JArdin , Yuanhan Liu , Richardson , Bruce , Ananyev , Konstantin , Wang , Zhihong References: <20180328135433.20203-1-declan.doherty@intel.com> <20180328135433.20203-5-declan.doherty@intel.com> From: "Doherty, Declan" Message-ID: <4f668011-5f8d-f109-22d9-3a965e935ac1@intel.com> Date: Thu, 29 Mar 2018 15:53:08 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v6 4/8] ethdev: Add port representor device flag 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: , X-List-Received-Date: Thu, 29 Mar 2018 14:53:17 -0000 On 29/03/2018 7:13 AM, Shahaf Shuler wrote: > Wednesday, March 28, 2018 4:54 PM, Declan Doherty: >> Subject: [dpdk-dev][PATCH v6 4/8] ethdev: Add port representor device flag >> >> Add new device flag to specify that ethdev port is a port representor. >> Extend rte_eth_dev_info structure to expose device flags to user which >> enable applications to discover if a port is a representor port. >> >> Signed-off-by: Declan Doherty >> --- >> lib/librte_ether/rte_ethdev.c | 1 + >> lib/librte_ether/rte_ethdev.h | 9 ++++++--- >> lib/librte_ether/rte_ethdev_representor.h | 3 +++ >> 3 files changed, 10 insertions(+), 3 deletions(-) >> >> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c >> index c719f84a3..163246433 100644 >> --- a/lib/librte_ether/rte_ethdev.c >> +++ b/lib/librte_ether/rte_ethdev.c >> @@ -2399,6 +2399,7 @@ rte_eth_dev_info_get(uint16_t port_id, struct >> rte_eth_dev_info *dev_info) >> dev_info->nb_rx_queues = dev->data->nb_rx_queues; >> dev_info->nb_tx_queues = dev->data->nb_tx_queues; >> dev_info->switch_id = dev->data->switch_id; >> + dev_info->dev_flags = dev->data->dev_flags; >> } >> >> int >> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h >> index dced4fc41..226acc8b1 100644 >> --- a/lib/librte_ether/rte_ethdev.h >> +++ b/lib/librte_ether/rte_ethdev.h >> @@ -996,6 +996,7 @@ struct rte_eth_dev_info { >> const char *driver_name; /**< Device Driver name. */ >> unsigned int if_index; /**< Index to bound host interface, or 0 if >> none. >> Use if_indextoname() to translate into an interface name. */ >> + uint32_t dev_flags; /**< Device flags */ >> uint32_t min_rx_bufsize; /**< Minimum size of RX buffer. */ >> uint32_t max_rx_pktlen; /**< Maximum configurable length of RX >> pkt. */ >> uint16_t max_rx_queues; /**< Maximum number of RX queues. */ >> @@ -1229,11 +1230,13 @@ struct rte_eth_dev_owner { }; >> >> /** Device supports link state interrupt */ >> -#define RTE_ETH_DEV_INTR_LSC 0x0002 >> +#define RTE_ETH_DEV_INTR_LSC 0x0002 >> /** Device is a bonded slave */ >> -#define RTE_ETH_DEV_BONDED_SLAVE 0x0004 >> +#define RTE_ETH_DEV_BONDED_SLAVE 0x0004 >> /** Device supports device removal interrupt */ >> -#define RTE_ETH_DEV_INTR_RMV 0x0008 >> +#define RTE_ETH_DEV_INTR_RMV 0x0008 >> +/** Device is port representor */ >> +#define RTE_ETH_DEV_REPRESENTOR 0x0010 > > Maybe it is a good time to make some order here. > I understand the decision to use flags instead of bit-field. It is better. > > However there is a mix here of device capabilities like : RTE_ETH_DEV_INTR_LSC and RTE_ETH_DEV_INTR_RMV > And device attributes like : RTE_ETH_DEV_BONDED_SLAVE and RTE_ETH_DEV_REPRESENTOR. > I don't think they belong together under the genetic name of dev_flags. > > Moreover, I am not sure the fact device is bonded slave should be exposed to the application. It should be internal to ethdev and its port iterators. That's a good point on the bonded slave flag, I'll look at fixing that for the next release. I don't think changing it should effect ABI but I'll need to have a closer look. Do you think that we should have a separate device attributes field, which the representor flag is contained in. > > Finally I think representor port may need more info now (and in the future), for example the associated vf id. > For that, I think it is better it to be exposed as a dedicated struct on device info. I think a switch port id should suffice for that, for SR-IOV devices it would map to the vf_id. > >> >> /** >> * @warning >> diff --git a/lib/librte_ether/rte_ethdev_representor.h >> b/lib/librte_ether/rte_ethdev_representor.h >> index cbc1f2855..f3726d0ba 100644 >> --- a/lib/librte_ether/rte_ethdev_representor.h >> +++ b/lib/librte_ether/rte_ethdev_representor.h >> @@ -22,6 +22,9 @@ eth_dev_representor_port_init(struct rte_eth_dev >> *ethdev, void *init_params) >> /** representor inherits the switch id of it's base device */ >> ethdev->data->switch_id = base_ethdev->data->switch_id; >> >> + /** Set device flags to specify that device is a representor port */ >> + ethdev->data->dev_flags |= RTE_ETH_DEV_REPRESENTOR; > > Should be set in the PMD, not in ethdev layer As in the previous patch this is just a generic port bus init function which meets the simplest use case of representor port with a single switch domain, a PMD doesn't need to use it but having it here saves duplicating the same code across multiple PMD which are only supporting the basic mode. > >> + >> return 0; >> } >> >> -- >> 2.14.3 >