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 E1C0F58DD for ; Wed, 7 Jan 2015 02:19:01 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 06 Jan 2015 17:18:55 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,711,1413270000"; d="scan'208";a="633496203" Received: from pgsmsx104.gar.corp.intel.com ([10.221.44.91]) by orsmga001.jf.intel.com with ESMTP; 06 Jan 2015 17:18:54 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by PGSMSX104.gar.corp.intel.com (10.221.44.91) with Microsoft SMTP Server (TLS) id 14.3.195.1; Wed, 7 Jan 2015 09:18:52 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.216]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.182]) with mapi id 14.03.0195.001; Wed, 7 Jan 2015 09:18:52 +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///WZAIABh0OAgAAhQICAAW1eYA== Date: Wed, 7 Jan 2015 01:18:50 +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> <54ABC679.9070202@cloudius-systems.com> In-Reply-To: <54ABC679.9070202@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: Wed, 07 Jan 2015 01:19:03 -0000 > -----Original Message----- > From: Vlad Zolotarov [mailto:vladz@cloudius-systems.com] > Sent: Tuesday, January 6, 2015 7:27 PM > To: Ouyang, Changchun; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v4 3/6] ixgbe: Get VF queue number >=20 >=20 > On 01/06/15 03:54, Ouyang, Changchun wrote: > > > >> -----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 > >> > >> > >> 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= need it. > >> 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 = itself, right? >=20 > Right. >=20 > > As for Vlan stripping, it need another patch to support it. >=20 > Well, at least put some fat comment in bold there that some the fields in= the > command is not filled and why. ;) OK, I will put more comments to explain it in v5. > > > >>>>> + > >>>>> + 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 we= ll. > >>> 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 > >>> which is not > >> necessary at all. > >>> So the code logic looks as above. > >> 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 eno= ugh > 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 thin= k > they are right. >=20 > Ooops. I missed the default value msg_size is 1 - I've confused its initi= alization > with mbx_size initialization. So, u are right - your code is perfectly fi= ne. My > apologies! ;) Never mind :-) =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; > >>>>> }