From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f49.google.com (mail-wg0-f49.google.com [74.125.82.49]) by dpdk.org (Postfix) with ESMTP id 50ACD1F5 for ; Tue, 6 Jan 2015 12:26:55 +0100 (CET) Received: by mail-wg0-f49.google.com with SMTP id n12so29203998wgh.36 for ; Tue, 06 Jan 2015 03:26:55 -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=0MiNsv2TvuTpA9uZZCdXrkuLD7uDR9BBuZRHzF49ZZ0=; b=D1JfCbvuxAmZ5J04pSCFTHNQG2utR9P6gG46ESwrsz6yk5CCKASVPv6rKThylveJv4 jWwG0I9U6C5SAc53c5QxmgBIDM8xx1JEhShfGbSDvLUF/gfqKzZ+DHGgxUgtIjUvugBO DOq8Dl3cMXhJ0Z+WWEA07aCpZI4o4SYyxtS1yJS5vMLu5y/N0zG50WeWBWpQh5k8Skzx +i4mNHPq38oRomlG7GH86My7koSHJOTrwoYqw42Bdy/dyf40+d7/H6ZdosAuHsFEeAyv vchbedK1KD/UOk23bcHVuJabX8gI5dLnTG4sJxP4uRSJo79Kcb/90BJmnp0bMRdmRse0 ictg== X-Gm-Message-State: ALoCoQmuwVQIoyLLgbMJrHwBlZ6rWG+FrXq7g9THQrxFK2hpCfEHIIY+iv+YInfD/VlHPSq2Em+I X-Received: by 10.180.100.98 with SMTP id ex2mr36078975wib.58.1420543615184; Tue, 06 Jan 2015 03:26:55 -0800 (PST) Received: from [10.0.0.165] (system.cloudius-systems.com. [84.94.198.183]) by mx.google.com with ESMTPSA id l6sm38896421wjx.33.2015.01.06.03.26.52 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 06 Jan 2015 03:26:54 -0800 (PST) Message-ID: <54ABC679.9070202@cloudius-systems.com> Date: Tue, 06 Jan 2015 13:26:49 +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> <54AA625E.9060607@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: Tue, 06 Jan 2015 11:26:55 -0000 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 = >>>>> + *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. > If I don't miss your point, you also agree it is not related to vf rss itself, right? Right. > As for Vlan stripping, it need another patch to support it. Well, at least put some fat comment in bold there that some the fields in the command is not filled and why. ;) > >>>>> + >>>>> + 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. > 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 they are right. Ooops. I missed the default value msg_size is 1 - I've confused its initialization with mbx_size initialization. So, u are right - your code is perfectly fine. My apologies! ;) > >>>>> + 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; >>>>> }