From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 02D7BA0A02; Wed, 24 Mar 2021 17:22:29 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 72AA0140F2F; Wed, 24 Mar 2021 17:22:29 +0100 (CET) Received: from m12-11.163.com (m12-11.163.com [220.181.12.11]) by mails.dpdk.org (Postfix) with ESMTP id 87069140F2E for ; Wed, 24 Mar 2021 17:22:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=Subject:From:Message-ID:Date:MIME-Version; bh=QQQhb /s3fgHV/PREEsAxWa/tE4GHVgBpcK2k8nWZRl8=; b=jf3qlEcdXbkz9JQ8tNyuU u3gEZ8odu6PYxi7SZaSC55eDWXreZ588nvcwXo46JJkVTi9tms/a2eZC2cIGVT8s tljnusuBXptL6D1XlkI/Gyi8sXKG3C+OwIK6rzi4DqSXKHNnwMoDv44CpKm/ct3M NDE08BdPiG9D8jmcs/uki8= Received: from appledeMacBook-Pro.local (unknown [223.104.65.244]) by smtp7 (Coremail) with SMTP id C8CowAC3dbU+Z1tg4oiWTw--.27664S3; Thu, 25 Mar 2021 00:22:23 +0800 (CST) To: Slava Ovsiienko , "dev@dpdk.org" Cc: "zhujiawei12@huawei.com" , Matan Azrad , Shahaf Shuler References: <1616427966-3481-1-git-send-email-17826875952@163.com> From: Jiawei Zhu <17826875952@163.com> Message-ID: Date: Thu, 25 Mar 2021 00:22:22 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0) Gecko/20100101 Thunderbird/68.12.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-CM-TRANSID: C8CowAC3dbU+Z1tg4oiWTw--.27664S3 X-Coremail-Antispam: 1Uf129KBjvJXoWxZw18WryrWw48GF43XF45GFg_yoWrXr4fpr 4SkFy3XFyUAFW5uw10qa1ru3y5Wwsayr4j9r1UGwn8Wr9ruryvqrWrKayrua4DCFs2kw1f tF4UZwnxG3W8Za7anT9S1TB71UUUUjUqnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07b5_MfUUUUU= X-Originating-IP: [223.104.65.244] X-CM-SenderInfo: bprxmjywyxkmivs6il2tof0z/1tbiER9f9l7+3TSvxwAAsG Subject: Re: [dpdk-dev] [PATCH] net/mlx5: add Rx checksum offload flag return bad X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi，Slava Thanks for your explain，the multiplications and divisions are in the TRANSPOSE，not in the rte_be_to_cpu_16. So I think use if-else directly could improves the performance. And the second point about the bad sum，I agree with you. With best regards, Jiawei On 2021/3/24 12:33 上午, Slava Ovsiienko wrote: > Hi, Jiawei > >> -----Original Message----- >> From: Jiawei Zhu <17826875952@163.com> >> Sent: Monday, March 22, 2021 17:46 >> To: dev@dpdk.org >> Cc: zhujiawei12@huawei.com; Matan Azrad ; Shahaf >> Shuler ; Slava Ovsiienko >> Subject: [PATCH] net/mlx5: add Rx checksum offload flag return bad >> >> From: Jiawei Zhu >> >> When open the rx checksum offload and receive the wrong checksum, add >> the ol_flags return bad. And it's not best to use multiplication and division >> here. > > I'm sorry, there should be no any multiplications and divisions - the arguments > are constants (can be determined at compilation time) and ara power of 2, > hence compiler engages simple shifts. For example (I did rxq_cq_to_ol_flags not inline to > get the listing for x86-64): > > 29 rxq_cq_to_ol_flags: > 21 /* > 22 * An architecture-optimized byte swap for a 16-bit value. > 23 * > 24 * Do not use this function directly. The preferred function is rte_bswap16(). > 25 */ > 26 static inline uint16_t rte_arch_bswap16(uint16_t _x) > 27 { > 28 uint16_t x = _x; > 29 0034 86D6 asm volatile ("xchgb %b[x1],%h[x2]" > 30 : [x1] "=Q" (x) > 37 > 38 0036 89D0 movl %edx,%eax > 39 0038 6681E200 andw $512,%dx > 39 02 > 40 003d 66250004 andw $1024,%ax > 41 0041 0FB7D2 movzwl %dx,%edx > 42 0044 0FB7C0 movzwl %ax,%eax > 43 0047 48C1EA02 shrq $2,%rdx > 44 004b 48C1E802 shrq $2,%rax > 45 004f 09D0 orl %edx,%eax > 46 0051 C3 ret > > As we can see - there is no any mul/div and no any branches, that > improves the performance. > >> >> Signed-off-by: Jiawei Zhu >> --- >> drivers/net/mlx5/mlx5_rxtx.c | 17 ++++++++++------- >> drivers/net/mlx5/mlx5_utils.h | 6 ------ >> 2 files changed, 10 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c >> index e3ce9fd..9233af8 100644 >> --- a/drivers/net/mlx5/mlx5_rxtx.c >> +++ b/drivers/net/mlx5/mlx5_rxtx.c >> @@ -1325,13 +1325,16 @@ enum mlx5_txcmp_code { >> uint32_t ol_flags = 0; >> uint16_t flags = rte_be_to_cpu_16(cqe->hdr_type_etc); >> >> - ol_flags = >> - TRANSPOSE(flags, >> - MLX5_CQE_RX_L3_HDR_VALID, >> - PKT_RX_IP_CKSUM_GOOD) | >> - TRANSPOSE(flags, >> - MLX5_CQE_RX_L4_HDR_VALID, >> - PKT_RX_L4_CKSUM_GOOD); >> + if (flags & MLX5_CQE_RX_L3_HDR_VALID) >> + ol_flags |= PKT_RX_IP_CKSUM_GOOD; >> + else >> + ol_flags |= PKT_RX_IP_CKSUM_BAD; >> + > > If MLX5_CQE_RX_L3_HDR_VALID is not set - it does not always mean the sum is bad. > If might happen if HW just did not recognize the packet format (for example, for > some tunnels) > > With best regards, > Slava > > >> + if (flags & MLX5_CQE_RX_L4_HDR_VALID) >> + ol_flags |= PKT_RX_IP_CKSUM_GOOD; >> + else >> + ol_flags |= PKT_RX_IP_CKSUM_BAD; >> + >> return ol_flags; >> } >> >> diff --git a/drivers/net/mlx5/mlx5_utils.h b/drivers/net/mlx5/mlx5_utils.h >> index 7a62187..2f71a23 100644 >> --- a/drivers/net/mlx5/mlx5_utils.h >> +++ b/drivers/net/mlx5/mlx5_utils.h >> @@ -44,12 +44,6 @@ >> #define NB_SEGS(m) ((m)->nb_segs) >> #define PORT(m) ((m)->port) >> >> -/* Transpose flags. Useful to convert IBV to DPDK flags. */ -#define >> TRANSPOSE(val, from, to) \ >> - (((from) >= (to)) ? \ >> - (((val) & (from)) / ((from) / (to))) : \ >> - (((val) & (from)) * ((to) / (from)))) >> - >> /* >> * For the case which data is linked with sequence increased index, the >> * array table will be more efficiect than hash table once need to serarch >> -- >> 1.8.3.1 >>