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

[zhoumin]

Hi Bibo,

On Sat, Apr 22, 2023 at 8:29AM, bibo, mao wrote:

>
>
> 在 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 at 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
>

Thanks for your kind comments.


I have sent the v2 patch and give the more detailed analysis for the 
segmentation fault.


Regards

Min
>>>           /*
>>> @@ -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
>>