Re: [PATCH 1/1] net/ixgbe: add a proper memory barrier for LoongArch

["bibo, mao" <maobibo@loongson.cn>]

在 2023/4/21 9:12, zhoumin 写道:
> On Fri, Apr 7, 2023 at 4:50PM, Min Zhou 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.
>>
>> Reason is the read ordering of the status and the rest of the
>> descriptor fields in this function may not be correct on the
>> LoongArch processor. We should add rte_rmb() to ensure the read
>> ordering be correct.
>>
>> We also did the same thing in the ixgbe_recv_pkts() function.
>>
>> Signed-off-by: Min Zhou <zhoumin@loongson.cn>
>> ---
>>   drivers/net/ixgbe/ixgbe_rxtx.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c 
>> b/drivers/net/ixgbe/ixgbe_rxtx.c
>> index c9d6ca9efe..16391a42f9 100644
>> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
>> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
>> @@ -1823,6 +1823,9 @@ 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;
>> +#if defined(RTE_ARCH_LOONGARCH)
>> +        rte_rmb();
>> +#endif
>>           rxd = *rxdp;
Hi Min,

Could you add more detailed analysis aboout the issu? Althrough rxdp is 
declared as volatile, which is only in order for compiler. However some 
architectures like LoongArch are weak-ordered. For this piece of code:

  1: staterr = rxdp->wb.upper.status_error;
Sentence 1 can be execute after sentence 1, dd indicated that packet is 
ready with new value.

  2:  rxd = *rxdp;
Sentence 2 can be execute first and get old value.

.......Balabala


Regards
Bibo, Mao

>>           /*
>> @@ -2122,6 +2125,9 @@ ixgbe_recv_pkts_lro(void *rx_queue, struct 
>> rte_mbuf **rx_pkts, uint16_t nb_pkts,
>>           if (!(staterr & IXGBE_RXDADV_STAT_DD))
>>               break;
>> +#if defined(RTE_ARCH_LOONGARCH)
>> +        rte_rmb();
>> +#endif
>>           rxd = *rxdp;
>>           PMD_RX_LOG(DEBUG, "port_id=%u queue_id=%u rx_id=%u "
> 
> Kindly ping.
> 
> Any comments or suggestions will be appreciated.
> 
> 
> Min
>