From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id D036D5584 for ; Thu, 28 Apr 2016 16:43:42 +0200 (CEST) Received: from [10.16.0.195] (unknown [10.16.0.195]) by proxy.6wind.com (Postfix) with ESMTP id C417E25124; Thu, 28 Apr 2016 16:42:05 +0200 (CEST) To: "John Daley (johndale)" , "Ananyev, Konstantin" , "dev@dpdk.org" , helin.zhang@intel.com, adrien.mazarguil@6wind.com, rahul.lakkireddy@chelsio.com, alejandro.lucero@netronome.com, sony.chacko@qlogic.com References: <55ece514189d46d59cca2217214d89be@XCH-RCD-007.cisco.com> <2601191342CEEE43887BDE71AB97725836B415A3@irsmsx105.ger.corp.intel.com> <571E20C0.2080509@6wind.com> From: Olivier MATZ Message-ID: <57222193.2040907@6wind.com> Date: Thu, 28 Apr 2016 16:43:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] PKT_RX_VLAN_PKT when VLAN stripping is disabled X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Apr 2016 14:43:43 -0000 Hi John, (adding some PMD maintainers in the loop) > From: Olivier Matz [mailto:olivier.matz@6wind.com] >> It seems the meaning of the PKT_RX_VLAN_PKT bit depends on the port >> configuration: >> - if vlan stripping is configured, it means VLAN is present in vlan_tci >> mbuf field >> - if not configured, it means a VLAN is present in the packet >> >> I don't think this is a good behavior since the application has to know the port >> configuration to properly interpret the meaning of the flag. >> >> I suggest to change the meaning of this flag to: "vlan was stripped by >> hardware, and vlan tag is now located in m->vlan_tci". >> >> The same could apply to PKT_RX_QINQ_PKT and m->outer_vlan_tci. >> >> We could add a new packet_type to tell if the mbuf is a VLAN/QinQ is >> detected in the packet but not stripped. >> >> Example: >> >> - vlan stripping is disabled >> >> - vlan packet recvd: flags=0, ptype=RTE_PTYPE_L2_ETHER_VLAN >> - qinq packet recvd: flags=0, ptype=RTE_PTYPE_L2_ETHER_QINQ >> >> - vlan stripping is enabled >> >> - vlan packet recvd: flags=PKT_RX_VLAN_PKT, >> ptype=RTE_PTYPE_L2_ETHER, >> m->vlan_tci=id >> - qinq packet recvd: flags=PKT_RX_VLAN_PKT|PKT_RX_QINQ_PKT, >> ptype=RTE_PTYPE_L2_ETHER, m->vlan_tci=id, m->vlan_tci_outer=id >> >> >> Thoughts? On 04/26/2016 02:16 AM, John Daley (johndale) wrote: > I like the new packet types and how they work the same for VLAN and QINQ. Just > so I understand your suggestion, X710 (as it seems to work today) would not set > RTE_PTYPE_L2_ETHER_VLAN in dev_supported_ptypes_get() because it does not know > how to determine that packet type in the receive path if stripping is disabled? > But if stripping was enabled, the application could still trust m->vlan_tci if > the flag was set? Well, this depends on the exact meaning of the supported_ptype flags: does it mean that the hw/driver *must* recognize and set the ptype or does it mean that the hw/driver *can* recognize and set the ptype? In case a packet with 2 vlans is received, if the hw/driver is able to strip the first one and recognize the second one, I would say that RTE_PTYPE_L2_ETHER_VLAN should be set in the mbuf flags. > Re changing the meaning of the PKT_RX_VLAN_PKT flag- I think it could cause hard > to find errors and confusion. I would rather see the flag deprecated and a new > one defined. Perhaps the flag could be called PKT_RX_VLAN_STRIPPED*. Yes, good idea. > Maybe another less elegant but more compatible solution would be just keep the > Niantic behavior and fix other pmd's to match its behavior. For X710, with vlan > stripping disabled this might mean looking at each packet's Ethernet type and > set the flag accordingly. It might not be too expensive since Ethernet type is > in the 1st cacheline and hopefully prefetched. Thoughts? I don't think the ixgbe behavior is correct. Today, depending on the configuration (vlan stripped or not), the meaning of the flag is different. This is not useable in an application that may use several ports with different configurations. > *In the future perhaps another flag could be added called > PKT_RX_VLAN_TCI_VALID. This may not be the same as PKT_RX_VLAN_STRIPPED- enic > and maybe some other nics are able to set vlan_tci even when not stripping vlan > tags and this feature could be exposed with this separate flag. At first read, I would say it's a bit overkill because it will complexify the combinatorial dimension of the offload flags. But if you agree, let's say it's another topic and focus on the fix of PKT_RX_VLAN_PKT first. Thanks, Olivier PS: please avoid top-posting, it's harder to read in a thread