On Thu, Oct 10, 2019 at 3:54 PM Ferruh Yigit <ferruh.yi...@intel.com> wrote:
> On 10/10/2019 8:20 AM, Vivek Sharma wrote: > > On Wed, Oct 9, 2019 at 6:26 PM Ferruh Yigit <ferruh.yi...@intel.com > > <mailto:ferruh.yi...@intel.com>> wrote: > > > > On 9/13/2019 1:15 PM, viveksha...@marvell.com > > <mailto:viveksha...@marvell.com> wrote: > > > From: Vivek Sharma <viveksha...@marvell.com <mailto: > viveksha...@marvell.com>> > > > > > > Segregate QinQ from Extend Offload and support QinQ offload > > > in vlan set command. Merge all port wise rx vlan offloads in > > > command line help and documentation for a cleaner structure. > > > > > > Signed-off-by: Vivek Sharma <viveksha...@marvell.com > > <mailto:viveksha...@marvell.com>> > > > > <...> > > > > > @@ -3902,6 +3895,8 @@ cmd_vlan_offload_parsed(void *parsed_result, > > > } > > > else if (!strcmp(res->what, "filter")) > > > rx_vlan_filter_set(port_id, on); > > > + else if (!strcmp(res->what, "qinq")) > > > > Indeed this is enabling QinQ strip, what do you think calling it > 'qinq_strip'? > > > > > > Agreed! > > > > > > > > > + rx_vlan_qinq_strip_set(port_id, on); > > > else > > > vlan_extend_set(port_id, on); > > > > > > @@ -3916,7 +3911,7 @@ cmdline_parse_token_string_t > cmd_vlan_offload_set = > > > set, "set"); > > > cmdline_parse_token_string_t cmd_vlan_offload_what = > > > TOKEN_STRING_INITIALIZER(struct cmd_vlan_offload_result, > > > - what, "strip#filter#qinq#stripq"); > > > + what, > "strip#filter#qinq#extend#stripq"); > > > cmdline_parse_token_string_t cmd_vlan_offload_on = > > > TOKEN_STRING_INITIALIZER(struct cmd_vlan_offload_result, > > > on, "on#off"); > > > @@ -3927,9 +3922,9 @@ cmdline_parse_token_string_t > cmd_vlan_offload_portid = > > > cmdline_parse_inst_t cmd_vlan_offload = { > > > .f = cmd_vlan_offload_parsed, > > > .data = NULL, > > > - .help_str = "vlan set strip|filter|qinq|stripq on|off " > > > + .help_str = "vlan set strip|filter|qinq|extend|stripq on|off > " > > > > "show port info ..." command ('port_infos_display()') displays the > vlan offloads > > too, can you please add displaying 'qinq' support too. > > > > > > This is already implemented in another patch which introduces 'qinq > offload'. > > Right, I found it [1], but I believe that change suits better here. Since > you > are the author of both patch, would you mind I include that bit into this > patch > while merging? > > Sure, please go ahead! Anyways, I was planning to send v2 of both the patches, which would take care of all the valid points raised by you. > [1] > https://patches.dpdk.org/patch/60847/ > > > > > > > > > btw, it displays line by line: > > VLAN offload: > > strip off > > filter on > > qinq(extend) off > > > > perhaps we can flatten it, to reduce the info length, what do you > think? > > > > > > Absolutely. In fact, I thought about doing so in the original patch. > > I was hoping the change can be part of your patch, but since it is already > out > this can be done as separate patch later. > > > > > > > VLAN offload: > > strip off, filter on, qinq(extend) off, qinq off > > > >
