From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 308D25398 for ; Tue, 10 Jan 2017 21:08:38 +0100 (CET) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga105.jf.intel.com with ESMTP; 10 Jan 2017 12:08:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,344,1477983600"; d="scan'208";a="47563485" Received: from lallumx-mobl2.ger.corp.intel.com (HELO [10.252.24.132]) ([10.252.24.132]) by orsmga004.jf.intel.com with ESMTP; 10 Jan 2017 12:08:36 -0800 To: Wenzhuo Lu , dev@dpdk.org References: <1480637533-37425-1-git-send-email-wenzhuo.lu@intel.com> <1484032580-60134-1-git-send-email-wenzhuo.lu@intel.com> <1484032580-60134-19-git-send-email-wenzhuo.lu@intel.com> <8dfea289-72da-8a84-4d30-368d6d4ea4d6@intel.com> Cc: "Chen Jing D(Mark)" , Bernard Iremonger From: Ferruh Yigit Message-ID: <598bd9db-073a-b40c-b60e-1d755b8f95ce@intel.com> Date: Tue, 10 Jan 2017 20:08:34 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <8dfea289-72da-8a84-4d30-368d6d4ea4d6@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v8 18/25] app/testpmd: use VFD APIs on i40e X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Jan 2017 20:08:39 -0000 On 1/10/2017 11:29 AM, Ferruh Yigit wrote: > On 1/10/2017 7:16 AM, Wenzhuo Lu wrote: >> The new VF Daemon (VFD) APIs is implemented on i40e. Change >> testpmd code to use them, including VF MAC anti-spoofing, >> VF VLAN anti-spoofing, TX loopback, VF VLAN strip, VF VLAN >> insert. >> >> Signed-off-by: Wenzhuo Lu >> Signed-off-by: Chen Jing D(Mark) >> Signed-off-by: Bernard Iremonger >> --- > > <...> > >> +#ifdef RTE_LIBRTE_IXGBE_PMD > > Within this ifdef, there is following call: > > ret = rte_pmd_ixgbe_set_all_queues_drop_en(res->port_id, is_on); > > It is not safe to make that call directly, port should be verified first > if it is ixgbe port, as done in other functions. With a second thought, although this part is not correct, it is out of this patches scope, because it concerns ixgbe related part. So below comment also become invalid. Wenzhuo, What do you think not changing this patch, but fixing this in a separate patch? Thanks, ferruh > >> /* all queues drop enable configuration */ >> >> /* Common result structure for all queues drop enable */ >> @@ -11277,6 +11351,9 @@ struct cmd_all_queues_drop_en_result { >> case -ENODEV: >> printf("invalid port_id %d\n", res->port_id); >> break; >> + case -ENOTSUP: >> + printf("function not implemented\n"); >> + break; >> default: >> printf("programming error: (%s)\n", strerror(-ret)); >> } >> @@ -11381,6 +11458,7 @@ struct cmd_vf_split_drop_en_result { >> NULL, >> }, >> }; >> +#endif >> > > <...> > >> @@ -11619,20 +11711,20 @@ struct cmd_set_vf_mac_addr_result { >> (cmdline_parse_inst_t *)&cmd_config_e_tag_forwarding_en_dis, >> (cmdline_parse_inst_t *)&cmd_config_e_tag_filter_add, >> (cmdline_parse_inst_t *)&cmd_config_e_tag_filter_del, >> -#ifdef RTE_LIBRTE_IXGBE_PMD >> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_anti_spoof, >> (cmdline_parse_inst_t *)&cmd_set_vf_mac_anti_spoof, >> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_stripq, >> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_insert, >> (cmdline_parse_inst_t *)&cmd_set_tx_loopback, >> +#ifdef RTE_LIBRTE_IXGBE_PMD > > Overall, since port will be verified within the function, why not remove > this ifdef completely? > > So all functions will be visible to everyone, but only the PMDs support > it will be called, others will return "not supported", as done in some > functions supports bot i40e and ixgbe pmd specific APIs. > >> (cmdline_parse_inst_t *)&cmd_set_all_queues_drop_en, >> (cmdline_parse_inst_t *)&cmd_set_vf_split_drop_en, >> - (cmdline_parse_inst_t *)&cmd_set_vf_mac_addr, >> (cmdline_parse_inst_t *)&cmd_set_vf_rxmode, >> (cmdline_parse_inst_t *)&cmd_set_vf_traffic, >> (cmdline_parse_inst_t *)&cmd_vf_rxvlan_filter, >> (cmdline_parse_inst_t *)&cmd_vf_rate_limit, >> #endif >> + (cmdline_parse_inst_t *)&cmd_set_vf_mac_addr, >> NULL, >> }; >> >> >