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 2494243BDE; Mon, 26 Feb 2024 04:16:09 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AA31A402B2; Mon, 26 Feb 2024 04:16:08 +0100 (CET) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id A2C0A40271 for ; Mon, 26 Feb 2024 04:16:06 +0100 (CET) Received: from mail.maildlp.com (unknown [172.19.163.174]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4Tjm1l6KFszNlpM; Mon, 26 Feb 2024 11:14:35 +0800 (CST) Received: from kwepemd100004.china.huawei.com (unknown [7.221.188.31]) by mail.maildlp.com (Postfix) with ESMTPS id F27A81404F2; Mon, 26 Feb 2024 11:16:03 +0800 (CST) Received: from [10.67.121.175] (10.67.121.175) by kwepemd100004.china.huawei.com (7.221.188.31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.2.1258.28; Mon, 26 Feb 2024 11:16:03 +0800 Message-ID: <6c831a66-f916-48d4-68d9-4c3bcfcb4979@huawei.com> Date: Mon, 26 Feb 2024 11:16:02 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH] net/hns3: fix Rx packet truncation when KEEP CRC enabled To: Ferruh Yigit , CC: , , , References: <20240206011030.2007689-1-haijie1@huawei.com> <11b8feac-4a9e-4d2c-8995-ed492d684750@amd.com> <7438563e-c7b4-6e13-68bf-74ff423546af@huawei.com> <6246e1f8-dcd4-468d-a05d-2e292f6e1714@amd.com> From: Jie Hai In-Reply-To: <6246e1f8-dcd4-468d-a05d-2e292f6e1714@amd.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.121.175] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemd100004.china.huawei.com (7.221.188.31) 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 On 2024/2/23 21:53, Ferruh Yigit wrote: > On 2/20/2024 3:58 AM, Jie Hai wrote: >> Hi, Ferruh, >> >> Thanks for your review. >> >> On 2024/2/7 22:15, Ferruh Yigit wrote: >>> On 2/6/2024 1:10 AM, Jie Hai wrote: >>>> From: Dengdui Huang >>>> >>>> When KEEP_CRC offload is enabled, some packets will be truncated and >>>> the CRC is still be stripped in following cases: >>>> 1. For HIP08 hardware, the packet type is TCP and the length >>>>     is less than or equal to 60B. >>>> 2. For other hardwares, the packet type is IP and the length >>>>     is less than or equal to 60B. >>>> >>> >>> If a device doesn't support the offload by some packets, it can be >>> option to disable offload for that device, instead of calculating it in >>> software and append it. >> >> The KEEP CRC feature of hns3 is faulty only in the specific packet >> type and small packet(<60B) case. >> What's more, the small ethernet packet is not common. >> >>> Unless you have a specific usecase, or requirement to support the >>> offload. >> >> Yes, some users of hns3 are already using this feature. >> So we cannot drop this offload >> >>> <...> >>> >>>> @@ -2492,10 +2544,16 @@ hns3_recv_pkts_simple(void *rx_queue, >>>>               goto pkt_err; >>>>             rxm->packet_type = hns3_rx_calc_ptype(rxq, l234_info, >>>> ol_info); >>>> - >>>>           if (rxm->packet_type == RTE_PTYPE_L2_ETHER_TIMESYNC) >>>>               rxm->ol_flags |= RTE_MBUF_F_RX_IEEE1588_PTP; >>>>   +        if (unlikely(rxq->crc_len > 0)) { >>>> +            if (hns3_need_recalculate_crc(rxq, rxm)) >>>> +                hns3_recalculate_crc(rxq, rxm); >>>> +            rxm->pkt_len -= rxq->crc_len; >>>> +            rxm->data_len -= rxq->crc_len; >>>> >>> >>> Removing 'crc_len' from 'mbuf->pkt_len' & 'mbuf->data_len' is >>> practically same as stripping CRC. >>> >>> We don't count CRC length in the statistics, but it should be accessible >>> in the payload by the user. >> Our drivers are behaving exactly as you say. >> > > If so I missed why mbuf 'pkt_len' and 'data_len' reduced by > 'rxq->crc_len', can you please explain what above lines does? > > @@ -2470,8 +2523,7 @@ hns3_recv_pkts_simple(void *rx_queue, rxdp->rx.bd_base_info = 0; rxm->data_off = RTE_PKTMBUF_HEADROOM; - rxm->pkt_len = (uint16_t)(rte_le_to_cpu_16(rxd.rx.pkt_len)) - - rxq->crc_len; + rxm->pkt_len = rte_le_to_cpu_16(rxd.rx.pkt_len); In the previous code above, the 'pkt_len' is set to the length obtained from the BD. the length obtained from the BD already contains CRC length. But as you said above, the DPDK requires that the length of the mbuf does not contain CRC length . So we subtract 'rxq->crc_len' from mbuf'pkt_len' and 'data_len'. This patch doesn't change the logic, it just moves the code around. > .