From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-we0-f181.google.com (mail-we0-f181.google.com [74.125.82.181]) by dpdk.org (Postfix) with ESMTP id 96DCCB378 for ; Sat, 30 Aug 2014 16:41:50 +0200 (CEST) Received: by mail-we0-f181.google.com with SMTP id x48so3450589wes.40 for ; Sat, 30 Aug 2014 07:46:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=IVhu8FGiZSQwRw+HBe2fJ2FL5SyLL78jETQJ+TJCG/k=; b=l9wRf/K3T4IBLjtjy9W1z32cMD8gIiJYfxUgTKMiSgrYoEPne8HxGU5IL9OmGu12rH Y3qZWgdprGvxuV5CjwX5T4ZGecuq259V4zJyUn1ayjvQlJoPXhknvvRMNI1KPYLfrrtS 2GnWcaf487YwDYdQWfbe+rexDzjFNUMJbSCBVgfNc8wu5gdhlA0Y1GUQM92ZWt9ghzIP apFmOL50f0kmevKjpDr5Y26NaVLRWdGYjkbgAvJjXDp1K2/ZGFuvCCMoHa/PkHKJ2KRa fNvvP8myl+sgWz4cH8qYoTXik8jVUAmIPYfSBv0f7JKK+RrIE8l6r+Z+YKHuT4IHBONs Hv+g== X-Gm-Message-State: ALoCoQn9RSQUha93CEYKQZvM+f84NfjFOl5g5gY6kFclVRVxDF3wzzGlT3sjTMZIc6zwH9UJZcX3 X-Received: by 10.180.149.197 with SMTP id uc5mr10378410wib.75.1409409969085; Sat, 30 Aug 2014 07:46:09 -0700 (PDT) Received: from xps13.localnet (APoitiers-651-1-232-7.w2-6.abo.wanadoo.fr. [2.6.255.7]) by mx.google.com with ESMTPSA id dh7sm6091373wib.18.2014.08.30.07.46.07 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 30 Aug 2014 07:46:08 -0700 (PDT) From: Thomas Monjalon To: Helin Zhang Date: Sat, 30 Aug 2014 16:43:15 +0200 Message-ID: <1897473.c67KeMpIux@xps13> Organization: 6WIND User-Agent: KMail/4.13.3 (Linux/3.15.8-1-ARCH; KDE/4.13.3; x86_64; ; ) In-Reply-To: <1408505611-6959-2-git-send-email-helin.zhang@intel.com> References: <1408505611-6959-1-git-send-email-helin.zhang@intel.com> <1408505611-6959-2-git-send-email-helin.zhang@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH 1/3] i40evf: support I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX in DPDK PF host 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: Sat, 30 Aug 2014 14:41:50 -0000 Hi Helin, The title mention i40evf but the patch is related to PF (for VF communication). So I wonder wether it would be clearer to prefix it with i40e? Not sure. 2014-08-20 11:33, Helin Zhang: > To configure VSI queues for VF, Linux PF host supports > operation of I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES with > limited configurations. To support more configurations > (e.g configurable CRC stripping in VF), a new operation > of I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX has been > supported in DPDK PF host. This patch would be easier to read if you could split it in 3 patches: 1) renamings and line wrapping rework 2) introduction of new extended message/operation 3) crc config > - int ret = I40E_SUCCESS; > - struct i40e_virtchnl_vsi_queue_config_info *qconfig = > - (struct i40e_virtchnl_vsi_queue_config_info *)msg; > - int i; > - struct i40e_virtchnl_queue_pair_info *qpair; > - > - if (msg == NULL || msglen <= sizeof(*qconfig) || > - qconfig->num_queue_pairs > vsi->nb_qps) { > + struct i40e_virtchnl_vsi_queue_config_info *vc_vqci = > + (struct i40e_virtchnl_vsi_queue_config_info *)msg; > + struct i40e_virtchnl_queue_pair_info *vc_qpi; > + struct i40e_virtchnl_queue_pair_extra_info *vc_qpei = NULL; > + int i, ret = I40E_SUCCESS; > + > + if (msg == NULL || msglen <= sizeof(*vc_vqci) || > + vc_vqci->num_queue_pairs > vsi->nb_qps) { > PMD_DRV_LOG(ERR, "vsi_queue_config_info argument wrong\n"); > ret = I40E_ERR_PARAM; > goto send_msg; > } > > - qpair = qconfig->qpair; > - for (i = 0; i < qconfig->num_queue_pairs; i++) { > - if (qpair[i].rxq.queue_id > vsi->nb_qps - 1 || > - qpair[i].txq.queue_id > vsi->nb_qps - 1) { > + vc_qpi = vc_vqci->qpair; > + if (opcode == I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX) > + vc_qpei = (struct i40e_virtchnl_queue_pair_extra_info *) > + (((uint8_t *)vc_qpi) + > + (sizeof(struct i40e_virtchnl_queue_pair_info) * > + vc_vqci->num_queue_pairs)); > + > + for (i = 0; i < vc_vqci->num_queue_pairs; i++) { > + if (vc_qpi[i].rxq.queue_id > vsi->nb_qps - 1 || > + vc_qpi[i].txq.queue_id > vsi->nb_qps - 1) { > ret = I40E_ERR_PARAM; > goto send_msg; > } Mixing renaming and new feature makes it difficult to read. > case I40E_VIRTCHNL_OP_ENABLE_QUEUES: > PMD_DRV_LOG(INFO, "OP_ENABLE_QUEUES received\n"); > - i40e_pf_host_process_cmd_enable_queues(vf, > - msg, msglen); > + i40e_pf_host_process_cmd_enable_queues(vf, msg, msglen); > break; > case I40E_VIRTCHNL_OP_DISABLE_QUEUES: > PMD_DRV_LOG(INFO, "OP_DISABLE_QUEUE received\n"); > - i40e_pf_host_process_cmd_disable_queues(vf, > - msg, msglen); > + i40e_pf_host_process_cmd_disable_queues(vf, msg, msglen); > break; > case I40E_VIRTCHNL_OP_ADD_ETHER_ADDRESS: > PMD_DRV_LOG(INFO, "OP_ADD_ETHER_ADDRESS received\n"); > - i40e_pf_host_process_cmd_add_ether_address(vf, > - msg, msglen); > + i40e_pf_host_process_cmd_add_ether_address(vf, msg, msglen); > break; > case I40E_VIRTCHNL_OP_DEL_ETHER_ADDRESS: > PMD_DRV_LOG(INFO, "OP_DEL_ETHER_ADDRESS received\n"); > - i40e_pf_host_process_cmd_del_ether_address(vf, > - msg, msglen); > + i40e_pf_host_process_cmd_del_ether_address(vf, msg, msglen); > break; > case I40E_VIRTCHNL_OP_ADD_VLAN: > PMD_DRV_LOG(INFO, "OP_ADD_VLAN received\n"); > @@ -932,10 +951,9 @@ i40e_pf_host_handle_vf_msg(struct rte_eth_dev *dev, > case I40E_VIRTCHNL_OP_FCOE: > PMD_DRV_LOG(ERR, "OP_FCOE received, not supported\n"); > default: > - PMD_DRV_LOG(ERR, "%u received, not supported\n", > - opcode); > - i40e_pf_host_send_msg_to_vf(vf, opcode, > - I40E_ERR_PARAM, NULL, 0); > + PMD_DRV_LOG(ERR, "%u received, not supported\n", opcode); > + i40e_pf_host_send_msg_to_vf(vf, opcode, I40E_ERR_PARAM, > + NULL, 0); > break; > } Line wrapping should go in a cleanup patch (like renaming). Thanks -- Thomas