From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 372C92E81 for ; Mon, 15 Sep 2014 02:16:20 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 14 Sep 2014 17:21:48 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,523,1406617200"; d="scan'208";a="599472331" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga002.fm.intel.com with ESMTP; 14 Sep 2014 17:21:47 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.195.1; Sun, 14 Sep 2014 17:21:47 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.195.1; Sun, 14 Sep 2014 17:21:47 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.230]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.204]) with mapi id 14.03.0195.001; Mon, 15 Sep 2014 08:21:45 +0800 From: "Zhang, Helin" To: Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH 1/3] i40evf: support I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX in DPDK PF host Thread-Index: AQHPxGEpdMCLhjzuJkGGgIrdmZE0RZwBbRpA Date: Mon, 15 Sep 2014 00:21:45 +0000 Message-ID: References: <1408505611-6959-1-git-send-email-helin.zhang@intel.com> <1408505611-6959-2-git-send-email-helin.zhang@intel.com> <1897473.c67KeMpIux@xps13> In-Reply-To: <1897473.c67KeMpIux@xps13> Accept-Language: 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 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: Mon, 15 Sep 2014 00:16:20 -0000 Hi Thomas > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Saturday, August 30, 2014 10:43 PM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/3] i40evf: support > I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX in DPDK PF host >=20 > Hi Helin, >=20 > The title mention i40evf but the patch is related to PF (for VF communica= tion). > So I wonder wether it would be clearer to prefix it with i40e? Not sure. >=20 > 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. >=20 > 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 >=20 > > - int ret =3D I40E_SUCCESS; > > - struct i40e_virtchnl_vsi_queue_config_info *qconfig =3D > > - (struct i40e_virtchnl_vsi_queue_config_info *)msg; > > - int i; > > - struct i40e_virtchnl_queue_pair_info *qpair; > > - > > - if (msg =3D=3D NULL || msglen <=3D sizeof(*qconfig) || > > - qconfig->num_queue_pairs > vsi->nb_qps) { > > + struct i40e_virtchnl_vsi_queue_config_info *vc_vqci =3D > > + (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 =3D NULL; > > + int i, ret =3D I40E_SUCCESS; > > + > > + if (msg =3D=3D NULL || msglen <=3D sizeof(*vc_vqci) || > > + vc_vqci->num_queue_pairs > vsi->nb_qps) { > > PMD_DRV_LOG(ERR, "vsi_queue_config_info argument wrong\n"); > > ret =3D I40E_ERR_PARAM; > > goto send_msg; > > } > > > > - qpair =3D qconfig->qpair; > > - for (i =3D 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 =3D vc_vqci->qpair; > > + if (opcode =3D=3D I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EX) > > + vc_qpei =3D (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 =3D 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 =3D I40E_ERR_PARAM; > > goto send_msg; > > } >=20 > Mixing renaming and new feature makes it difficult to read. >=20 > > 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; > > } >=20 > Line wrapping should go in a cleanup patch (like renaming). >=20 > Thanks > -- > Thomas Thank you very much! I have reworked the patches and sent out again. Regards, Helin