DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"Wu, Jingjing" <jingjing.wu@intel.com>,
	"Zhang, Helin" <helin.zhang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 2/2] app/testpmd: enable VF untag drop in testpmd
Date: Tue, 14 Mar 2017 13:32:15 +0000	[thread overview]
Message-ID: <2f857fde-0149-9ee5-1d5a-3a664d205c08@intel.com> (raw)
In-Reply-To: <039ED4275CED7440929022BC67E7061153064A3C@SHSMSX103.ccr.corp.intel.com>

On 3/9/2017 2:59 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Tuesday, March 7, 2017 7:14 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
>> Zhang, Helin <helin.zhang@intel.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 2/2] app/testpmd: enable VF untag drop in
>> testpmd
>>
>> On 3/3/2017 1:59 AM, Qi Zhang wrote:
>>> Add command line to support untag drop to specific VF in testpmd.
>>>
>>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>>> ---
>>>  app/test-pmd/cmdline.c | 104
>>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 104 insertions(+)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
>>> 43fc636..4ddc2c9 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -311,6 +311,10 @@ static void cmd_help_long_parsed(void
>>> *parsed_result,
>>>
>>>  			"set vf vlan antispoof (port_id) (vf_id) (on|off)\n"
>>>  			"    Set VLAN antispoof for a VF from the PF.\n\n"
>>> +#ifdef RTE_LIBRTE_I40E_PMD
>>> +			"set vf vlan untagdrop (port_id) (vf_id) (on|off)\n"
>>> +			"    Set VLAN untag drop for a VF from the PF.\n\n"
>>> +#endif
>>
>> We should decide how to implement PMD specific APIs in testpmd, and be
>> consistent about it.
>>
>> Currently there are two approaches:
>>
>> 1- Wrap PMD specific feature and API with and PMD #ifdef, as done here.
>>
>> 2- Enable feature by default, return -ENOTSUP for port_id that does not support
>> it. Ex: cmd_vf_rxvlan_filter.
>>
>> I am for second option. And explicitly not disabling I40E driver does not mean
>> you should have those NICs in your runtime environment, so the effect will be
>> same as always enabling it.
>>
> Yes, I notice this problem, during implementation, I saw both patterns exist, so I have to choose one of them
> We'd better align this.
> Both option ok for me, but a little bit prefer option 1 , since it's not necessary to explore a command if no backend device, that make the hint more clean.

I agree it's not necessary to explore a command if no backend device,
but first option does not guaranties it. What it checks is compile time
option, not runtime device detection.
I40E driver is enabled by default, and unless user explicitly disables
it, testpmd command will be there independent from device is there or not.

>>
>> And since number of PMD specific APIs are increasing, perhaps we should find a
>> better approach for testpmd to prevent them corrupting testpmd.
> Will think about this, also like to know if you or anyone have any good suggestion.
>>
>> Also it may worth to discuss why number of PMD specific APIs are increasing.
>>
>>>
>>>  			"set vf vlan tag (port_id) (vf_id) (on|off)\n"
>>>  			"    Set VLAN tag for a VF from the PF.\n\n"
>>> @@ -10995,6 +10999,103 @@ cmdline_parse_inst_t
>> cmd_set_vf_vlan_anti_spoof = {
>>>  	},
>>>  };
>>>
>> <...>

  reply	other threads:[~2017-03-14 13:32 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-03  1:59 [dpdk-dev] [PATCH 0/2] enable VF untag drop Qi Zhang
2017-03-03  1:59 ` [dpdk-dev] [PATCH 1/2] net/i40e: " Qi Zhang
2017-03-07 10:51   ` Ferruh Yigit
2017-03-09  3:24     ` Zhang, Qi Z
2017-03-14 13:29       ` Ferruh Yigit
2017-03-14 14:32         ` Zhang, Qi Z
2017-03-14 18:16           ` Iremonger, Bernard
2017-03-23 17:00             ` Ferruh Yigit
2017-03-23 17:22               ` Iremonger, Bernard
2017-03-03  1:59 ` [dpdk-dev] [PATCH 2/2] app/testpmd: enable VF untag drop in testpmd Qi Zhang
2017-03-07 11:13   ` Ferruh Yigit
2017-03-09  2:59     ` Zhang, Qi Z
2017-03-14 13:32       ` Ferruh Yigit [this message]
2017-03-14 14:43         ` Zhang, Qi Z
2017-03-13  4:55 ` [dpdk-dev] [PATCH v2 0/2] net/i40e: enable VF untag drop Qi Zhang
2017-03-13  4:55   ` [dpdk-dev] [PATCH v2 1/2] " Qi Zhang
2017-03-13  4:55   ` [dpdk-dev] [PATCH v2 2/2] app/testpmd: enable VF untag drop in testpmd Qi Zhang
2017-03-23 17:01     ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2f857fde-0149-9ee5-1d5a-3a664d205c08@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=dev@dpdk.org \
    --cc=helin.zhang@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=qi.z.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).