From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <changchun.ouyang@intel.com>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
 by dpdk.org (Postfix) with ESMTP id 82706590C
 for <dev@dpdk.org>; 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" <changchun.ouyang@intel.com>
To: Vlad Zolotarov <vladz@cloudius-systems.com>, "dev@dpdk.org" <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: <F52918179C57134FAEC9EA62FA2F96251194E93B@shsmsx102.ccr.corp.intel.com>
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>
 <F52918179C57134FAEC9EA62FA2F96251194E2CB@shsmsx102.ccr.corp.intel.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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=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 <changchun.ouyang@intel.com>
> >>> ---
> >>>    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;
> >>>    }