From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rcdn-iport-1.cisco.com (rcdn-iport-1.cisco.com [173.37.86.72]) by dpdk.org (Postfix) with ESMTP id 7FE561B6AD for ; Mon, 23 Oct 2017 19:42:35 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=2301; q=dns/txt; s=iport; t=1508780555; x=1509990155; h=subject:to:cc:references:from:message-id:date: mime-version:in-reply-to:content-transfer-encoding; bh=fRA5bRPNgIbQRZ6bZn8h0P9wmuVCF+n1cKqaQs8fReg=; b=YSYOxY3W/B3wC1sjlbSqMzHDaYRrHr7m/I//kCtBCwYelq4FPHL9cwBG ewX8Sidwvh9hCOiBVXZSUVCevvSrsqc1NFozLwlsXVIFW1VE7/FMQcw92 kEqXv/4PjI6Gl63SK7hdM3XKCQYy7FDeBVecpPdu+mNiTPTgS5MAWw1bk Q=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0ChAACTKe5Z/5JdJa1bGQEBAQEBAQEBA?= =?us-ascii?q?QEBBwEBAQEBgzEugVIng3qKH49CgXqWOYIRCoU7AoRUPxgBAgEBAQEBAQFrKIU?= =?us-ascii?q?eAQUjBBFBEAsYAgImAgJXBgEMBgIBAYocpzKBbTqLIQEBAQEBAQEBAQEBAQEBA?= =?us-ascii?q?QEBAQEfgQ+CH4IHgVCCEoMBiBmCYQWhZgKLRYkti2yHNYRDkTmBOR84gVt6FYM?= =?us-ascii?q?tglsBHBmBaiQ2jC8BAQE?= X-IronPort-AV: E=Sophos;i="5.43,424,1503360000"; d="scan'208";a="314899596" Received: from rcdn-core-10.cisco.com ([173.37.93.146]) by rcdn-iport-1.cisco.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Oct 2017 17:42:34 +0000 Received: from [10.150.214.125] ([10.150.214.125]) by rcdn-core-10.cisco.com (8.14.5/8.14.5) with ESMTP id v9NHgY7S027815; Mon, 23 Oct 2017 17:42:34 GMT To: Ferruh Yigit , wenzhuo.lu@intel.com Cc: dev@dpdk.org, Bruce Richardson , "Ananyev, Konstantin" References: <20171012172435.34700-1-rmelton@cisco.com> <1e8e5766-6c52-d09d-d7ac-11492b946c90@intel.com> From: "Roger B. Melton" Message-ID: <3b4d34fa-309e-10d4-e992-3d82034e4c41@cisco.com> Date: Mon, 23 Oct 2017 13:42:18 -0400 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <1e8e5766-6c52-d09d-d7ac-11492b946c90@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [dpdk-dev] [PATCH v2] net/e1000: correct VLAN tag byte order for i35x LB packets 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: Mon, 23 Oct 2017 17:42:35 -0000 On 10/20/17 3:04 PM, Ferruh Yigit wrote: > On 10/12/2017 10:24 AM, Roger B Melton wrote: >> When copying VLAN tags from the RX descriptor to the vlan_tci field >> in the mbuf header, igb_rxtx.c:eth_igb_recv_pkts() and >> eth_igb_recv_scattered_pkts() both assume that the VLAN tag is always >> little endian. While i350, i354 and /i350vf VLAN non-loopback >> packets are stored little endian, VLAN tags in loopback packets for >> those devices are big endian. >> >> For i350, i354 and i350vf VLAN loopback packets, swap the tag when >> copying from the RX descriptor to the mbuf header. This will ensure >> that the mbuf vlan_tci is always little endian. >> >> Signed-off-by: Roger B Melton > <...> > >> @@ -946,9 +954,16 @@ eth_igb_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, >> >> rxm->hash.rss = rxd.wb.lower.hi_dword.rss; >> hlen_type_rss = rte_le_to_cpu_32(rxd.wb.lower.lo_dword.data); >> - /* Only valid if PKT_RX_VLAN_PKT set in pkt_flags */ >> - rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan); >> - >> + /* >> + * The vlan_tci field is only valid when PKT_RX_VLAN_PKT is >> + * set in the pkt_flags field and must be in CPU byte order. >> + */ >> + if ((staterr & rte_cpu_to_le_32(E1000_RXDEXT_STATERR_LB)) && >> + (rxq->flags & IGB_RXQ_FLAG_LB_BSWAP_VLAN)) { > This is adding more condition checks into Rx path. > What is the performance cost of this addition? I have not measured the performance cost, but I can collect data. What specifically are you looking for? To be clear the current implementation incorrect as it does not normalize the vlan tag to CPU byte order before copying it into mbuf and applications have no visibility to determine if the tag in the mbuf is big or little endian. Do you have any suggestions for an alternative approach to avoid rx patch checks? Thanks, Roger > >> + rxm->vlan_tci = rte_be_to_cpu_16(rxd.wb.upper.vlan); >> + } else { >> + rxm->vlan_tci = rte_le_to_cpu_16(rxd.wb.upper.vlan); >> + } >> pkt_flags = rx_desc_hlen_type_rss_to_pkt_flags(rxq, hlen_type_rss); >> pkt_flags = pkt_flags | rx_desc_status_to_pkt_flags(staterr); >> pkt_flags = pkt_flags | rx_desc_error_to_pkt_flags(staterr); > <...> > . >