From: Jie Hai <haijie1@huawei.com>
To: Ferruh Yigit <ferruh.yigit@amd.com>, <dev@dpdk.org>
Cc: <lihuisong@huawei.com>, <fengchengwen@huawei.com>,
<liuyonglong@huawei.com>, <huangdengdui@huawei.com>
Subject: Re: [PATCH] net/hns3: fix Rx packet truncation when KEEP CRC enabled
Date: Mon, 26 Feb 2024 11:16:02 +0800 [thread overview]
Message-ID: <6c831a66-f916-48d4-68d9-4c3bcfcb4979@huawei.com> (raw)
In-Reply-To: <6246e1f8-dcd4-468d-a05d-2e292f6e1714@amd.com>
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 <huangdengdui@huawei.com>
>>>>
>>>> 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.
> .
next prev parent reply other threads:[~2024-02-26 3:16 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-06 1:10 Jie Hai
2024-02-07 14:15 ` Ferruh Yigit
2024-02-20 3:58 ` Jie Hai
2024-02-23 13:53 ` Ferruh Yigit
2024-02-26 3:16 ` Jie Hai [this message]
2024-02-26 16:43 ` Ferruh Yigit
2024-02-28 2:27 ` huangdengdui
2024-02-28 13:07 ` Ferruh Yigit
2024-02-29 3:58 ` huangdengdui
2024-02-29 9:25 ` Ferruh Yigit
2024-03-01 6:55 ` huangdengdui
2024-03-01 11:10 ` Ferruh Yigit
2024-03-08 11:36 ` Jie Hai
2024-03-22 6:28 ` Jie Hai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6c831a66-f916-48d4-68d9-4c3bcfcb4979@huawei.com \
--to=haijie1@huawei.com \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=ferruh.yigit@amd.com \
--cc=huangdengdui@huawei.com \
--cc=lihuisong@huawei.com \
--cc=liuyonglong@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).