On 01/05/15 04:59, Ouyang, Changchun wrote: > >> -----Original Message----- >> From: Vlad Zolotarov [mailto:vladz at cloudius-systems.com] >> Sent: Sunday, January 4, 2015 4:39 PM >> To: Ouyang, Changchun; dev at 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 at 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 = >>> + *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; >>> }