From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 3791F23A for ; Sat, 14 Jul 2018 10:02:46 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1-us3.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id B6F589C005B; Sat, 14 Jul 2018 08:02:44 +0000 (UTC) Received: from [192.168.1.16] (85.187.13.33) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Sat, 14 Jul 2018 09:02:37 +0100 To: rkudurumalla , Ferruh Yigit , Pavan Nikhilesh , , CC: , "Kudurumalla, Rakesh" , Shahaf Shuler , Thomas Monjalon , Olivier MATZ References: <20180701164637.978-1-pbhagavatula@caviumnetworks.com> <63fa281f-e777-dc35-fa39-33e5dbcb0d42@intel.com> From: Andrew Rybchenko Message-ID: <5b4889f3-2b72-509d-216c-63d435d332f9@solarflare.com> Date: Sat, 14 Jul 2018 11:02:26 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [85.187.13.33] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23968.000 X-TM-AS-Result: No--11.066300-0.000000-31 X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-MDID: 1531555365-wODc24l10AoL Subject: Re: [dpdk-dev] [PATCH] net/thunderx: add support for Rx VLAN offload 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: Sat, 14 Jul 2018 08:02:46 -0000 On 13.07.2018 17:16, rkudurumalla wrote: > On 07/04/2018 11:06 PM, Ferruh Yigit wrote: >> External Email >> >> On 7/1/2018 5:46 PM, Pavan Nikhilesh wrote: >>> From: "Kudurumalla, Rakesh" >>> >>> This feature is used to offload stripping of vlan header from recevied >>> packets and update vlan_tci field in mbuf when >>> DEV_RX_OFFLOAD_VLAN_STRIP & ETH_VLAN_STRIP_MASK flag is set. >>> >>> Signed-off-by: Rakesh Kudurumalla >>> Signed-off-by: Pavan Nikhilesh >>> --- >>> drivers/net/thunderx/base/nicvf_hw.c | 1 + >>> drivers/net/thunderx/nicvf_ethdev.c | 59 +++++++++++++++++------ >>> drivers/net/thunderx/nicvf_rxtx.c | 70 ++++++++++++++++++++++++---- >>> drivers/net/thunderx/nicvf_rxtx.h | 15 ++++-- >>> drivers/net/thunderx/nicvf_struct.h | 1 + >> In thunderx.ini, "VLAN offload" already marked as P(Partially) is it still >> partially? Why? >> >> It is still partial because Tx VLAN offload(insertion of vlan header > > for tx packets) is yet to be Implemented >> <...> >> >>> @@ -1590,9 +1595,9 @@ nicvf_vf_start(struct rte_eth_dev *dev, struct nicvf *nic, uint32_t rbdrsz) >>> nic->rbdr->tail, nb_rbdr_desc, nic->vf_id); >>> >>> /* Configure VLAN Strip */ >>> - vlan_strip = !!(dev->data->dev_conf.rxmode.offloads & >>> - DEV_RX_OFFLOAD_VLAN_STRIP); >>> - nicvf_vlan_hw_strip(nic, vlan_strip); >>> + mask = ETH_VLAN_STRIP_MASK | ETH_VLAN_FILTER_MASK | >>> + ETH_VLAN_EXTEND_MASK; >> You don't need anything more than ETH_VLAN_STRIP_MASK but agreed no issue add >> more if you prefer. >> >>> + ret = nicvf_vlan_offload_config(dev, mask); >>> >>> /* Based on the packet type(IPv4 or IPv6), the nicvf HW aligns L3 data >>> * to the 64bit memory address. >>> @@ -1983,6 +1988,7 @@ static const struct eth_dev_ops nicvf_eth_dev_ops = { >>> .dev_infos_get = nicvf_dev_info_get, >>> .dev_supported_ptypes_get = nicvf_dev_supported_ptypes_get, >>> .mtu_set = nicvf_dev_set_mtu, >>> + .vlan_offload_set = nicvf_vlan_offload_set, >> Not related to this patch but I believe this name 'vlan_offload_set' is >> confusing, it enable/disable VLAN related config: >> - vlan strip offload >> - vlan filtering package (drop/accept specific vlans) >> - double vlan feature (not offload if I am not missing anything) >> We can think about a more proper name later... >> >> Also rte_eth_dev_set_vlan_offload() API may have a defect, it seems not taking >> capability flags into account, cc'ed Shahaf and Andrew for information. Yes, the function could check if corresponding offloads are supported. Right now we have unified interface to control offloads on device configure and queues setup. This API to change VLAN offloads looks really legacy now. If we really need API to control offloads at run time (I'm not sure), it should be generic API for all offloads and corresponding information in dev_info which specifies offloads are controllable at run time. >> And I have a question about DEV_TX_OFFLOAD_VLAN_INSERT, perhaps goes to Olivier, >> if DEV_TX_OFFLOAD_VLAN_INSERT enabled what is the correct way to provide >> vlan_tci to insert? >> And do we need something like PKT_RX_VLAN_INSERT and use mbuf->vlan_tci value to >> have the ability to insert VLAN to some packets? As I understand ol_flags should have PKT_TX_VLAN. Right now the description does not mentione it, however the description for double-tagged insertion PKT_TX_QINQ does.