From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f181.google.com (mail-wi0-f181.google.com [209.85.212.181]) by dpdk.org (Postfix) with ESMTP id D35805A15 for ; Mon, 5 Jan 2015 11:07:29 +0100 (CET) Received: by mail-wi0-f181.google.com with SMTP id r20so2902574wiv.8 for ; Mon, 05 Jan 2015 02:07:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=/R7rMQOG9MFi8fHfsTb1sKGVSlPmk1hRHa5jJzU2qlw=; b=TNem/cvx7ZLT2QEHd/4P0l+9DhWEFWkhDixZ6aED1KeSOMY2xRqGC/DSg5ykH1jQPl UqqTU4AVjyfY5IhQj9yYaFADftuHEVqxSgeTcoqYDkk/6MqsyoGstXqRwyYpAL70Xx7H ylKErMI/O5LTfQAetglSBst7eq3p7d7QaRbj4aec7Sa4CyN0pRXCAO3nMH3NROOdDX8w 1tnw0YYztMWO1fRKjQrs2/BalvYswPv7DF4WY7w1Dz3PFI9tn8B+2Zv0CBNd8eqnVVoq zgBJzbkfBIsCa4oZXUXZvgJweUsAWc/4UkdN6Nb/sd0BBlTBhMAVRqueSjkI7WKO39MH EOYg== X-Gm-Message-State: ALoCoQmczcCVFP4jWUAo4Wi+/FopGZh5aZSYJ59po6/MQr68SqHVQbMniYrlmD2WBGv39Ghz3mmb X-Received: by 10.194.187.235 with SMTP id fv11mr177217806wjc.16.1420452449669; Mon, 05 Jan 2015 02:07:29 -0800 (PST) Received: from [10.0.0.165] (system.cloudius-systems.com. [84.94.198.183]) by mx.google.com with ESMTPSA id fx6sm74743351wjc.39.2015.01.05.02.07.28 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 05 Jan 2015 02:07:29 -0800 (PST) Message-ID: <54AA625E.9060607@cloudius-systems.com> Date: Mon, 05 Jan 2015 12:07:26 +0200 From: Vlad Zolotarov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: "Ouyang, Changchun" , "dev@dpdk.org" 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> In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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: Mon, 05 Jan 2015 10:07:30 -0000 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 = >>> + *IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data- >>> dev_private); >>> + uint32_t default_q = 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] = >> RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool; >>> + msgbuf[IXGBE_VF_TX_QUEUES] = >> RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool; >>> + >>> + /* Notify VF of default queue */ >>> + msgbuf[IXGBE_VF_DEF_QUEUE] = 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. > >>> + >>> + return 0; >>> +} >>> + >>> +static int >>> ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf) >>> { >>> uint16_t mbx_size = IXGBE_VFMAILBOX_SIZE; >>> + uint16_t msg_size = IXGBE_VF_MSG_SIZE_DEFAULT; >>> uint32_t msgbuf[IXGBE_VFMAILBOX_SIZE]; >>> int32_t retval; >>> struct ixgbe_hw *hw = >>> 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 = ixgbe_negotiate_vf_api(dev, vf, msgbuf); >>> break; >>> + case IXGBE_VF_GET_QUEUES: >>> + retval = ixgbe_get_vf_queues(dev, vf, msgbuf); >>> + msg_size = 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 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. > >>> + break; >>> default: >>> PMD_DRV_LOG(DEBUG, "Unhandled Msg %8.8x", >> (unsigned)msgbuf[0]); >>> retval = IXGBE_ERR_MBX; >>> @@ -551,7 +584,7 @@ ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, >>> uint16_t vf) >>> >>> msgbuf[0] |= IXGBE_VT_MSGTYPE_CTS; >>> >>> - ixgbe_write_mbx(hw, msgbuf, 1, vf); >>> + ixgbe_write_mbx(hw, msgbuf, msg_size, vf); >>> >>> return retval; >>> }