From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 82706590C for ; Tue, 6 Jan 2015 02:54:09 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP; 05 Jan 2015 17:54:08 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,704,1413270000"; d="scan'208";a="646759453" Received: from kmsmsx153.gar.corp.intel.com ([172.21.73.88]) by fmsmga001.fm.intel.com with ESMTP; 05 Jan 2015 17:54:07 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by KMSMSX153.gar.corp.intel.com (172.21.73.88) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 6 Jan 2015 09:54:04 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.216]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.67]) with mapi id 14.03.0195.001; Tue, 6 Jan 2015 09:54:04 +0800 From: "Ouyang, Changchun" To: Vlad Zolotarov , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v4 3/6] ixgbe: Get VF queue number Thread-Index: AQHQJ+7EUQ6pHqTQkUSsC2t7Bk4jWJyvHbsAgAG1gdD///WZAIABh0OA Date: Tue, 6 Jan 2015 01:54:02 +0000 Message-ID: References: <1419398584-19520-1-git-send-email-changchun.ouyang@intel.com> <1420355937-18484-1-git-send-email-changchun.ouyang@intel.com> <1420355937-18484-4-git-send-email-changchun.ouyang@intel.com> <54A8FC16.40402@cloudius-systems.com> <54AA625E.9060607@cloudius-systems.com> In-Reply-To: <54AA625E.9060607@cloudius-systems.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 3/6] ixgbe: Get VF queue number X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Jan 2015 01:54:10 -0000 > -----Original Message----- > From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com] > Sent: Monday, January 5, 2015 6:07 PM > To: Ouyang, Changchun; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v4 3/6] ixgbe: Get VF queue number >=20 >=20 > On 01/05/15 04:59, Ouyang, Changchun wrote: > > > >> -----Original Message----- > >> From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com] > >> Sent: Sunday, January 4, 2015 4:39 PM > >> To: Ouyang, Changchun; dev@dpdk.org > >> Subject: Re: [dpdk-dev] [PATCH v4 3/6] ixgbe: Get VF queue number > >> > >> > >> On 01/04/15 09:18, Ouyang Changchun wrote: > >>> Get the available Rx and Tx queue number when receiving > >> IXGBE_VF_GET_QUEUES message from VF. > >>> Signed-off-by: Changchun Ouyang > >>> --- > >>> lib/librte_pmd_ixgbe/ixgbe_pf.c | 35 > >> ++++++++++++++++++++++++++++++++++- > >>> 1 file changed, 34 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/lib/librte_pmd_ixgbe/ixgbe_pf.c > >>> b/lib/librte_pmd_ixgbe/ixgbe_pf.c index 495aff5..cbb0145 100644 > >>> --- a/lib/librte_pmd_ixgbe/ixgbe_pf.c > >>> +++ b/lib/librte_pmd_ixgbe/ixgbe_pf.c > >>> @@ -53,6 +53,8 @@ > >>> #include "ixgbe_ethdev.h" > >>> > >>> #define IXGBE_MAX_VFTA (128) > >>> +#define IXGBE_VF_MSG_SIZE_DEFAULT 1 #define > >>> +IXGBE_VF_GET_QUEUE_MSG_SIZE 5 > >>> > >>> static inline uint16_t > >>> dev_num_vf(struct rte_eth_dev *eth_dev) @@ -491,9 +493,36 @@ > >>> ixgbe_negotiate_vf_api(struct rte_eth_dev *dev, uint32_t vf, > >>> uint32_t > >> *msgbuf) > >>> } > >>> > >>> static int > >>> +ixgbe_get_vf_queues(struct rte_eth_dev *dev, uint32_t vf, uint32_t > >>> +*msgbuf) { > >>> + struct ixgbe_vf_info *vfinfo =3D > >>> + *IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data- > >>> dev_private); > >>> + uint32_t default_q =3D vf * RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool; > >>> + > >>> + /* Verify if the PF supports the mbox APIs version or not */ > >>> + switch (vfinfo[vf].api_version) { > >>> + case ixgbe_mbox_api_20: > >>> + case ixgbe_mbox_api_11: > >>> + break; > >>> + default: > >>> + return -1; > >>> + } > >>> + > >>> + /* Notify VF of Rx and Tx queue number */ > >>> + msgbuf[IXGBE_VF_RX_QUEUES] =3D > >> RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool; > >>> + msgbuf[IXGBE_VF_TX_QUEUES] =3D > >> RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool; > >>> + > >>> + /* Notify VF of default queue */ > >>> + msgbuf[IXGBE_VF_DEF_QUEUE] =3D default_q; > >> What about IXGBE_VF_TRANS_VLAN field? > > This field is used for vlan strip or dcb case, which the vf rss don't n= eed it. >=20 > But VFs do support VLAN stripping and u don't add it to just RSS. If VFs = do not > support VLAN stripping in the DPDK yet they should and then we will need > this field. If I don't miss your point, you also agree it is not related to vf rss itse= lf, right? As for Vlan stripping, it need another patch to support it. =20 > > > >>> + > >>> + return 0; > >>> +} > >>> + > >>> +static int > >>> ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf) > >>> { > >>> uint16_t mbx_size =3D IXGBE_VFMAILBOX_SIZE; > >>> + uint16_t msg_size =3D IXGBE_VF_MSG_SIZE_DEFAULT; > >>> uint32_t msgbuf[IXGBE_VFMAILBOX_SIZE]; > >>> int32_t retval; > >>> struct ixgbe_hw *hw =3D > >>> IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private); > >>> @@ -537,6 +566,10 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev > *dev, > >> uint16_t vf) > >>> case IXGBE_VF_API_NEGOTIATE: > >>> retval =3D ixgbe_negotiate_vf_api(dev, vf, msgbuf); > >>> break; > >>> + case IXGBE_VF_GET_QUEUES: > >>> + retval =3D ixgbe_get_vf_queues(dev, vf, msgbuf); > >>> + msg_size =3D IXGBE_VF_GET_QUEUE_MSG_SIZE; > >> Although the msg_size semantics and motivation is clear, if u want to = do > then > >> do it all the way - add it to all other cases too not just to > >> IXGBE_VF_GET_QUEUES. > >> For instance, why do u write all 16 DWORDS for API negotiation (only 2= are > >> required) and only here u decided to get "greedy"? ;) > >> > >> My point is: either drop it completely or fix all other places as well= . > > This is because the actual message size required by 2 different > message(api-negotiation and vf-get-queue) > > are different, the first one require only 4 bytes, the second one need = 20 > bytes. > > If both use 4 bytes, then the second one will have incomplete message. > > If both use 20 bytes, then the first one will contain garbage info whic= h is not > necessary at all. > > So the code logic looks as above. >=20 > I understood the motivation at the first place but as I've explained > above we already bring the garbage for some opcodes like API > negotiation. So, u should either fix it for all opcodes like u did for > GET_QUEUES or just drop it in GET_QUEUES and fix it for all opcodes in a > different patch. Here maybe I miss your point, my understanding is that 4 bytes are enough = for all other opcode except for get_queue opcode, get_queues is the only one that need 20 bytes currently. So I don't quite understand why I need fix any codes which we both think th= ey are right. =20 > > > >>> + break; > >>> default: > >>> PMD_DRV_LOG(DEBUG, "Unhandled Msg %8.8x", > >> (unsigned)msgbuf[0]); > >>> retval =3D IXGBE_ERR_MBX; > >>> @@ -551,7 +584,7 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev > *dev, > >>> uint16_t vf) > >>> > >>> msgbuf[0] |=3D IXGBE_VT_MSGTYPE_CTS; > >>> > >>> - ixgbe_write_mbx(hw, msgbuf, 1, vf); > >>> + ixgbe_write_mbx(hw, msgbuf, msg_size, vf); > >>> > >>> return retval; > >>> }