DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: "Sardar, Shamsher singh" <Shamshersingh.Sardar@amd.com>
Cc: "Kumar, Ravi1" <Ravi1.Kumar@amd.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2] net/axgbe: vlan support enabling in axgbe
Date: Mon, 6 Apr 2020 10:03:52 +0100	[thread overview]
Message-ID: <39b86649-17c8-5473-dd9f-b9ee862e2a38@intel.com> (raw)
In-Reply-To: <DM6PR12MB2890EEA50DA801AD9B25D25184C50@DM6PR12MB2890.namprd12.prod.outlook.com>

On 4/5/2020 6:00 PM, Sardar, Shamsher singh wrote:
> +dev@dpdk.org
> 
> -----Original Message-----
> From: Sardar, Shamsher singh 
> Sent: Friday, April 3, 2020 10:12 PM
> To: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: Kumar, Ravi1 <Ravi1.Kumar@amd.com>
> Subject: RE: [dpdk-dev] [PATCH v2] net/axgbe: vlan support enabling in axgbe
> 
> Hi Yigit,
> 
> 1. The Rx vlan filter set calls vlan_offload_set as shown below:
> rx_vlan_filter_set() <-- app/test-pmd/config.c  ...
>   rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) <--lib/librte_ethdev/rte_ethdev.c  {  ...
>  ret = (*dev->dev_ops->vlan_offload_set) <-- this calls the respective driver's func  ...
>  }
>  2. The tx vlan check for flags and based on same it add/remove as shown below the code flow:
> tx_vlan_set(portid_t port_id, uint16_t vlan_id)  <-- app/test-pmd/config.c  ...
>   rte_eth_dev_info_get(port_id, &dev_info);  <-- collect flags from driver  check capability DEV_TX_OFFLOAD_VLAN_INSERT  ...
>  ports[port_id].tx_vlan_id = vlan_id; <--insert vlan to respective port id

Hi Shamsher singh,

VLAN filter looks good, but in the VLAN insert/strip proper 'mbuf.ol_flags' and
'mbuf.vlan_tci' should be set.

In the Rx path, driver should set 'PKT_RX_VLAN' and/or 'PKT_RX_VLAN_STRIPPED'
flags (please check documentation) and 'mbuf.vlan_tci', otherwise how
application can know about the vlan tag and use it.
Like if the 'PKT_RX_VLAN' is set you will see additional log in the testpmd
packet view, "VLAN tci=0x%x", showing the received VLAN tag.

Similar in Tx, driver should check 'PKT_TX_VLAN' flag to insert 'mbuf.vlan_tci'
VLAN tag for the packet it has this flag.

Regards,
ferruh

> 
> Note: Also attached logs to check call for filter flow and strip on/off. 
> In strip log by default the strip is off and hence header shown when sent vlan pkt from pktgen running in other device and in strip on the header is getting removed.
> Let me know if you need more logs.
> 
> Note: In pkt-gen log, the log captured for filtering with different vlan settings to check data flow post addition/removal of vlan in both side (pkt-gen and DUT, device under test).
> 
> Thanks & regards
> S Shamsher Singh
> Mob:7259016141
> 
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Friday, April 3, 2020 6:26 PM
> To: Sardar, Shamsher singh <Shamshersingh.Sardar@amd.com>; dev@dpdk.org
> Cc: Kumar, Ravi1 <Ravi1.Kumar@amd.com>
> Subject: Re: [dpdk-dev] [PATCH v2] net/axgbe: vlan support enabling in axgbe
> 
> [CAUTION: External Email]
> 
> On 4/3/2020 7:20 AM, ssardar@amd.com wrote:
>> From: Sardar Shamsher Singh <Shamshersingh.Sardar@amd.com>
>>
>> adding below APIs for axgbe
>> - axgbe_enable_rx_vlan_stripping: to enable vlan header stipping
>> - axgbe_disable_rx_vlan_stripping: to disable vlan header stipping
>> - axgbe_enable_rx_vlan_filtering: to enable vlan filter mode
>> - axgbe_disable_rx_vlan_filtering: to disable vlan filter mode
>> - axgbe_update_vlan_hash_table: crc calculation and hash table update 
>> based on vlan values post filter enable
>> - axgbe_vlan_filter_set: setting of active vlan out of max 4K values 
>> before doing hash update of same
>> - axgbe_vlan_tpid_set: setting of default tpid values
>> - axgbe_vlan_offload_set: a top layer function to call stip/filter etc 
>> based on mask values
>>
>> Signed-off-by: Sardar Shamsher Singh <Shamshersingh.Sardar@amd.com>
>> ---
>>  doc/guides/nics/features/axgbe.ini |   1 +
>>  drivers/net/axgbe/axgbe_common.h   |  29 +++++
>>  drivers/net/axgbe/axgbe_dev.c      | 171 +++++++++++++++++++++++++++--
>>  drivers/net/axgbe/axgbe_ethdev.c   | 169 +++++++++++++++++++++++++++-
>>  drivers/net/axgbe/axgbe_ethdev.h   |  16 +++
>>  drivers/net/axgbe/axgbe_rxtx.c     |   6 +
>>  6 files changed, 381 insertions(+), 11 deletions(-)
>>
>> diff --git a/doc/guides/nics/features/axgbe.ini
>> b/doc/guides/nics/features/axgbe.ini
>> index 0becaa097..b7b4dd992 100644
>> --- a/doc/guides/nics/features/axgbe.ini
>> +++ b/doc/guides/nics/features/axgbe.ini
>> @@ -11,6 +11,7 @@ Scattered Rx         = Y
>>  Promiscuous mode     = Y
>>  Allmulticast mode    = Y
>>  RSS hash             = Y
>> +VLAN                 = Y
> 
> There is no "VLAN" feautre defined [1], but "VLAN filter" [2] & "VLAN offload"
> [3], and in this patch I guess you mean both.
> 
> But for "VLAN offload", I don't see the driver sets "PKT_RX_VLAN" or "PKT_RX_VLAN_STRIPPED" mbuf flags at all. And it doesn't use 'mbuf.vlan_tci' at all, in both Rx & Tx. Are you sure VLAN offload supported by driver?
> 
> And for Tx, driver still uses 'PKT_TX_VLAN_PKT' which is deprecated, can you please replace it with 'PKT_TX_VLAN'?
> 
> 
> 
> [1]
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Flxr.dpdk.org%2Fdpdk%2Fv20.02%2Fsource%2Fdoc%2Fguides%2Fnics%2Ffeatures%2Fdefault.ini&amp;data=02%7C01%7CShamshersingh.Sardar%40amd.com%7C1c359b4b479d4ac7090608d7d7ce5e33%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637215153650573793&amp;sdata=Ujp55KpjRyP%2FFFa9Y8v1RJQdCtS2qGjRj7sdE3y7uAk%3D&amp;reserved=0
> 
> [2]
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Flxr.dpdk.org%2Fdpdk%2Fv20.02%2Fsource%2Fdoc%2Fguides%2Fnics%2Ffeatures.rst%23L363&amp;data=02%7C01%7CShamshersingh.Sardar%40amd.com%7C1c359b4b479d4ac7090608d7d7ce5e33%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637215153650583782&amp;sdata=QUZF1xle9plBK1HeGEf4FMJaXrcMAr0%2BFpvyv7BB7ic%3D&amp;reserved=0
> 
> [3]
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Flxr.dpdk.org%2Fdpdk%2Fv20.02%2Fsource%2Fdoc%2Fguides%2Fnics%2Ffeatures.rst%23L474&amp;data=02%7C01%7CShamshersingh.Sardar%40amd.com%7C1c359b4b479d4ac7090608d7d7ce5e33%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637215153650583782&amp;sdata=cU%2FdVldaEzozUdoOdi3jkjIhx9K95NGPuZjdxM4YFl8%3D&amp;reserved=0
> 


      reply	other threads:[~2020-04-06  9:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-03  6:20 ssardar
2020-04-03  6:22 ` Kumar, Ravi1
2020-04-03 12:56 ` Ferruh Yigit
     [not found]   ` <DM6PR12MB28906AF812E27286360BE59884C70@DM6PR12MB2890.namprd12.prod.outlook.com>
2020-04-05 17:00     ` Sardar, Shamsher singh
2020-04-06  9:03       ` Ferruh Yigit [this message]

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=39b86649-17c8-5473-dd9f-b9ee862e2a38@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=Ravi1.Kumar@amd.com \
    --cc=Shamshersingh.Sardar@amd.com \
    --cc=dev@dpdk.org \
    /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).