From: zhoumin <zhoumin@loongson.cn>
To: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Cc: dev@dpdk.org, maobibo@loongson.cn, qiming.yang@intel.com,
wenjun1.wu@intel.com, ruifeng.wang@arm.com,
drc@linux.vnet.ibm.com
Subject: Re: [PATCH v2] net/ixgbe: add proper memory barriers for some Rx functions
Date: Thu, 4 May 2023 21:16:39 +0800 [thread overview]
Message-ID: <43fb4e62-63b5-7076-327e-dc30cd561fff@loongson.cn> (raw)
In-Reply-To: <20671e5e-8e86-4bc3-2d95-4cec014b0539@yandex.ru>
Hi Konstantin,
Thanks for your comments.
On 2023/5/1 下午9:29, Konstantin Ananyev wrote:
>> Segmentation fault has been observed while running the
>> ixgbe_recv_pkts_lro() function to receive packets on the Loongson 3C5000
>> processor which has 64 cores and 4 NUMA nodes.
>>
>> From the ixgbe_recv_pkts_lro() function, we found that as long as the
>> first
>> packet has the EOP bit set, and the length of this packet is less
>> than or
>> equal to rxq->crc_len, the segmentation fault will definitely happen
>> even
>> though on the other platforms, such as X86.
>>
>> Because when processd the first packet the first_seg->next will be
>> NULL, if
>> at the same time this packet has the EOP bit set and its length is less
>> than or equal to rxq->crc_len, the following loop will be excecuted:
>>
>> for (lp = first_seg; lp->next != rxm; lp = lp->next)
>> ;
>>
>> We know that the first_seg->next will be NULL under this condition.
>> So the
>> expression of lp->next->next will cause the segmentation fault.
>>
>> Normally, the length of the first packet with EOP bit set will be
>> greater
>> than rxq->crc_len. However, the out-of-order execution of CPU may
>> make the
>> read ordering of the status and the rest of the descriptor fields in
>> this
>> function not be correct. The related codes are as following:
>>
>> rxdp = &rx_ring[rx_id];
>> #1 staterr = rte_le_to_cpu_32(rxdp->wb.upper.status_error);
>>
>> if (!(staterr & IXGBE_RXDADV_STAT_DD))
>> break;
>>
>> #2 rxd = *rxdp;
>>
>> The sentence #2 may be executed before sentence #1. This action is
>> likely
>> to make the ready packet zero length. If the packet is the first
>> packet and
>> has the EOP bit set, the above segmentation fault will happen.
>>
>> So, we should add rte_rmb() to ensure the read ordering be correct.
>> We also
>> did the same thing in the ixgbe_recv_pkts() function to make the rxd
>> data
>> be valid even thougth we did not find segmentation fault in this
>> function.
>>
>> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
>> ---
>> v2:
>> - Make the calling of rte_rmb() for all platforms
>> ---
>> drivers/net/ixgbe/ixgbe_rxtx.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c
>> b/drivers/net/ixgbe/ixgbe_rxtx.c
>> index c9d6ca9efe..302a5ab7ff 100644
>> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
>> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
>> @@ -1823,6 +1823,8 @@ ixgbe_recv_pkts(void *rx_queue, struct rte_mbuf
>> **rx_pkts,
>> staterr = rxdp->wb.upper.status_error;
>> if (!(staterr & rte_cpu_to_le_32(IXGBE_RXDADV_STAT_DD)))
>> break;
>> +
>> + rte_rmb();
>> rxd = *rxdp;
>
>
>
> Indeed, looks like a problem to me on systems with relaxed MO.
> Strange that it was never hit on arm or ppc - cc-ing ARM/PPC maintainers.
The LoongArch architecture uses the Weak Consistency model which can
cause the problem, especially in scenario with many cores, such as
Loongson 3C5000 with four NUMA node, which has 64 cores. I cannot
reproduce it on Loongson 3C5000 with one NUMA node, which just has 16 cores.
> About a fix - looks right, but a bit excessive to me -
> as I understand all we need here is to prevent re-ordering by CPU itself.
Yes, thanks for cc-ing.
> So rte_smp_rmb() seems enough here.
> Or might be just:
> staterr = __atomic_load_n(&rxdp->wb.upper.status_error,
> __ATOMIC_ACQUIRE);
>
Does __atomic_load_n() work on Windows if we use it to solve this problem ?
>
>> /*
>> @@ -2122,6 +2124,7 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct
>> rte_mbuf **rx_pkts, uint16_t nb_pkts,
>> if (!(staterr & IXGBE_RXDADV_STAT_DD))
>> break;
>>
>> + rte_rmb();
>> rxd = *rxdp;
>>
>> PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u "
>> --
>> 2.31.1
Best regards,
Min
next prev parent reply other threads:[~2023-05-04 13:16 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-24 9:05 Min Zhou
2023-04-28 3:43 ` Zhang, Qi Z
2023-04-28 6:27 ` Morten Brørup
2023-05-04 12:58 ` zhoumin
2023-05-04 12:42 ` zhoumin
2023-05-01 13:29 ` Konstantin Ananyev
2023-05-04 6:13 ` Ruifeng Wang
2023-05-05 1:45 ` zhoumin
2023-05-04 13:16 ` zhoumin [this message]
2023-05-04 13:21 ` Morten Brørup
2023-05-04 13:33 ` Zhang, Qi Z
2023-05-05 2:42 ` zhoumin
2023-05-06 1:30 ` Zhang, Qi Z
2023-05-05 1:54 ` zhoumin
2023-05-06 10:23 ` [PATCH v3] " Min Zhou
2023-05-08 6:03 ` Ruifeng Wang
2023-05-15 2:10 ` Zhang, Qi Z
2023-06-12 10:26 ` Thomas Monjalon
2023-06-12 11:58 ` zhoumin
2023-06-12 12:44 ` Thomas Monjalon
2023-06-13 1:42 ` zhoumin
2023-06-13 3:30 ` Jiawen Wu
2023-06-13 10:12 ` zhoumin
2023-06-14 10:58 ` Konstantin Ananyev
2023-06-13 9:25 ` Ruifeng Wang
2023-06-20 15:52 ` Thomas Monjalon
2023-06-21 6:50 ` Ruifeng Wang
2023-06-13 9:44 ` [PATCH v4] " Min Zhou
2023-06-13 10:20 ` Ruifeng Wang
2023-06-13 12:11 ` Zhang, Qi Z
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=43fb4e62-63b5-7076-327e-dc30cd561fff@loongson.cn \
--to=zhoumin@loongson.cn \
--cc=dev@dpdk.org \
--cc=drc@linux.vnet.ibm.com \
--cc=konstantin.v.ananyev@yandex.ru \
--cc=maobibo@loongson.cn \
--cc=qiming.yang@intel.com \
--cc=ruifeng.wang@arm.com \
--cc=wenjun1.wu@intel.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).