* [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used
@ 2023-07-07 0:17 longli
2023-07-07 9:01 ` Ferruh Yigit
2023-07-10 11:00 ` Ferruh Yigit
0 siblings, 2 replies; 5+ messages in thread
From: longli @ 2023-07-07 0:17 UTC (permalink / raw)
To: Ferruh Yigit, Andrew Rybchenko; +Cc: dev, Ajay Sharma, Long Li
From: Long Li <longli@microsoft.com>
On a fatal CQE error when coalescing is used, update the correct index
and allow proceeding to the next CQE.
Fixes: 3409e0f172f6 ("net/mana: implement Rx CQE coalescing")
Signed-off-by: Long Li <longli@microsoft.com>
---
drivers/net/mana/rx.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/net/mana/rx.c b/drivers/net/mana/rx.c
index cacfd9ae1b..220b372b15 100644
--- a/drivers/net/mana/rx.c
+++ b/drivers/net/mana/rx.c
@@ -416,23 +416,21 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
switch (oob->cqe_hdr.cqe_type) {
case CQE_RX_OKAY:
+ case CQE_RX_COALESCED_4:
/* Proceed to process mbuf */
break;
case CQE_RX_TRUNCATED:
- DP_LOG(DEBUG, "Drop a truncated packet");
+ default:
+ DP_LOG(ERR, "RX CQE type %d client %d vendor %d",
+ oob->cqe_hdr.cqe_type, oob->cqe_hdr.client_type,
+ oob->cqe_hdr.vendor_err);
+
rxq->stats.errors++;
rte_pktmbuf_free(mbuf);
- goto drop;
-
- case CQE_RX_COALESCED_4:
- /* Proceed to process mbuf */
- break;
- default:
- DP_LOG(ERR, "Unknown RX CQE type %d",
- oob->cqe_hdr.cqe_type);
- continue;
+ i++;
+ goto drop;
}
DP_LOG(DEBUG, "mana_rx_comp_oob type %d rxq %p",
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used
2023-07-07 0:17 [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used longli
@ 2023-07-07 9:01 ` Ferruh Yigit
2023-07-07 18:01 ` Long Li
2023-07-10 11:00 ` Ferruh Yigit
1 sibling, 1 reply; 5+ messages in thread
From: Ferruh Yigit @ 2023-07-07 9:01 UTC (permalink / raw)
To: longli, Andrew Rybchenko; +Cc: dev, Ajay Sharma, Long Li
On 7/7/2023 1:17 AM, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> On a fatal CQE error when coalescing is used, update the correct index
> and allow proceeding to the next CQE.
>
> Fixes: 3409e0f172f6 ("net/mana: implement Rx CQE coalescing")
>
Is above fixes commit correct?
Logic for 'CQE_RX_COALESCED_4' is not changed with this commit, in above
commit and in this commit both does breaks the loop.
This commit changes logic for 'CQE_RX_TRUNCATED' and 'default' cases,
which are added with different commits, not the one in fixes line.
"fatal CQE error when coalescing" mentioned in the commit log, to which
switch case does this corresponds to?
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
> drivers/net/mana/rx.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/mana/rx.c b/drivers/net/mana/rx.c
> index cacfd9ae1b..220b372b15 100644
> --- a/drivers/net/mana/rx.c
> +++ b/drivers/net/mana/rx.c
> @@ -416,23 +416,21 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>
> switch (oob->cqe_hdr.cqe_type) {
> case CQE_RX_OKAY:
> + case CQE_RX_COALESCED_4:
> /* Proceed to process mbuf */
> break;
>
> case CQE_RX_TRUNCATED:
> - DP_LOG(DEBUG, "Drop a truncated packet");
> + default:
> + DP_LOG(ERR, "RX CQE type %d client %d vendor %d",
> + oob->cqe_hdr.cqe_type, oob->cqe_hdr.client_type,
> + oob->cqe_hdr.vendor_err);
> +
> rxq->stats.errors++;
> rte_pktmbuf_free(mbuf);
> - goto drop;
> -
> - case CQE_RX_COALESCED_4:
> - /* Proceed to process mbuf */
> - break;
>
> - default:
> - DP_LOG(ERR, "Unknown RX CQE type %d",
> - oob->cqe_hdr.cqe_type);
> - continue;
> + i++;
> + goto drop;
> }
>
> DP_LOG(DEBUG, "mana_rx_comp_oob type %d rxq %p",
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used
2023-07-07 9:01 ` Ferruh Yigit
@ 2023-07-07 18:01 ` Long Li
2023-07-10 10:42 ` Ferruh Yigit
0 siblings, 1 reply; 5+ messages in thread
From: Long Li @ 2023-07-07 18:01 UTC (permalink / raw)
To: Ferruh Yigit, longli, Andrew Rybchenko; +Cc: dev, Ajay Sharma
> Subject: Re: [PATCH] net/mana: fix wrong indexing on CQE error when coalescing
> is used
>
> On 7/7/2023 1:17 AM, longli@linuxonhyperv.com wrote:
> > From: Long Li <longli@microsoft.com>
> >
> > On a fatal CQE error when coalescing is used, update the correct index
> > and allow proceeding to the next CQE.
> >
> > Fixes: 3409e0f172f6 ("net/mana: implement Rx CQE coalescing")
> >
>
> Is above fixes commit correct?
> Logic for 'CQE_RX_COALESCED_4' is not changed with this commit, in above
> commit and in this commit both does breaks the loop.
Yes, the "Fixes" tag is correct. Here CQE_RX_COALESCED_4 is rearranged to make it easier to read, but it doesn't change the behavior.
>
> This commit changes logic for 'CQE_RX_TRUNCATED' and 'default' cases, which
> are added with different commits, not the one in fixes line.
>
> "fatal CQE error when coalescing" mentioned in the commit log, to which switch
> case does this corresponds to?
The previous patch (3409e0f172f6 ) introduced variable "i", an index to completion CQEs. But both 'default' and 'CQE_RX_TRUNCATED' cases don't advance "i", hence not advance to next CQE on error.
The fatal CQE error means the "default" case. On 'CQE_RX_TRUNCATED' the code can recover when all CQEs are read. But on "default", it's a dead loop.
>
>
> > Signed-off-by: Long Li <longli@microsoft.com>
> > ---
> > drivers/net/mana/rx.c | 18 ++++++++----------
> > 1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/mana/rx.c b/drivers/net/mana/rx.c index
> > cacfd9ae1b..220b372b15 100644
> > --- a/drivers/net/mana/rx.c
> > +++ b/drivers/net/mana/rx.c
> > @@ -416,23 +416,21 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf
> > **pkts, uint16_t pkts_n)
> >
> > switch (oob->cqe_hdr.cqe_type) {
> > case CQE_RX_OKAY:
> > + case CQE_RX_COALESCED_4:
> > /* Proceed to process mbuf */
> > break;
> >
> > case CQE_RX_TRUNCATED:
> > - DP_LOG(DEBUG, "Drop a truncated packet");
> > + default:
> > + DP_LOG(ERR, "RX CQE type %d client %d vendor %d",
> > + oob->cqe_hdr.cqe_type, oob->cqe_hdr.client_type,
> > + oob->cqe_hdr.vendor_err);
> > +
> > rxq->stats.errors++;
> > rte_pktmbuf_free(mbuf);
> > - goto drop;
> > -
> > - case CQE_RX_COALESCED_4:
> > - /* Proceed to process mbuf */
> > - break;
> >
> > - default:
> > - DP_LOG(ERR, "Unknown RX CQE type %d",
> > - oob->cqe_hdr.cqe_type);
> > - continue;
> > + i++;
> > + goto drop;
> > }
> >
> > DP_LOG(DEBUG, "mana_rx_comp_oob type %d rxq %p",
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used
2023-07-07 18:01 ` Long Li
@ 2023-07-10 10:42 ` Ferruh Yigit
0 siblings, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2023-07-10 10:42 UTC (permalink / raw)
To: Long Li, longli, Andrew Rybchenko; +Cc: dev, Ajay Sharma
On 7/7/2023 7:01 PM, Long Li wrote:
>> Subject: Re: [PATCH] net/mana: fix wrong indexing on CQE error when coalescing
>> is used
>>
>> On 7/7/2023 1:17 AM, longli@linuxonhyperv.com wrote:
>>> From: Long Li <longli@microsoft.com>
>>>
>>> On a fatal CQE error when coalescing is used, update the correct index
>>> and allow proceeding to the next CQE.
>>>
>>> Fixes: 3409e0f172f6 ("net/mana: implement Rx CQE coalescing")
>>>
>>
>> Is above fixes commit correct?
>> Logic for 'CQE_RX_COALESCED_4' is not changed with this commit, in above
>> commit and in this commit both does breaks the loop.
>
> Yes, the "Fixes" tag is correct. Here CQE_RX_COALESCED_4 is rearranged to make it easier to read, but it doesn't change the behavior.
>
>>
>> This commit changes logic for 'CQE_RX_TRUNCATED' and 'default' cases, which
>> are added with different commits, not the one in fixes line.
>>
>> "fatal CQE error when coalescing" mentioned in the commit log, to which switch
>> case does this corresponds to?
>
> The previous patch (3409e0f172f6 ) introduced variable "i", an index to completion CQEs. But both 'default' and 'CQE_RX_TRUNCATED' cases don't advance "i", hence not advance to next CQE on error.
>
Right, thanks for clarification, I was checking from CQE_RX_COALESCED_4
perspective.
> The fatal CQE error means the "default" case. On 'CQE_RX_TRUNCATED' the code can recover when all CQEs are read. But on "default", it's a dead loop.
>
ack.
>>
>>
>>> Signed-off-by: Long Li <longli@microsoft.com>
>>> ---
>>> drivers/net/mana/rx.c | 18 ++++++++----------
>>> 1 file changed, 8 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/net/mana/rx.c b/drivers/net/mana/rx.c index
>>> cacfd9ae1b..220b372b15 100644
>>> --- a/drivers/net/mana/rx.c
>>> +++ b/drivers/net/mana/rx.c
>>> @@ -416,23 +416,21 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf
>>> **pkts, uint16_t pkts_n)
>>>
>>> switch (oob->cqe_hdr.cqe_type) {
>>> case CQE_RX_OKAY:
>>> + case CQE_RX_COALESCED_4:
>>> /* Proceed to process mbuf */
>>> break;
>>>
>>> case CQE_RX_TRUNCATED:
>>> - DP_LOG(DEBUG, "Drop a truncated packet");
>>> + default:
>>> + DP_LOG(ERR, "RX CQE type %d client %d vendor %d",
>>> + oob->cqe_hdr.cqe_type, oob->cqe_hdr.client_type,
>>> + oob->cqe_hdr.vendor_err);
>>> +
>>> rxq->stats.errors++;
>>> rte_pktmbuf_free(mbuf);
>>> - goto drop;
>>> -
>>> - case CQE_RX_COALESCED_4:
>>> - /* Proceed to process mbuf */
>>> - break;
>>>
>>> - default:
>>> - DP_LOG(ERR, "Unknown RX CQE type %d",
>>> - oob->cqe_hdr.cqe_type);
>>> - continue;
>>> + i++;
>>> + goto drop;
>>> }
>>>
>>> DP_LOG(DEBUG, "mana_rx_comp_oob type %d rxq %p",
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used
2023-07-07 0:17 [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used longli
2023-07-07 9:01 ` Ferruh Yigit
@ 2023-07-10 11:00 ` Ferruh Yigit
1 sibling, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2023-07-10 11:00 UTC (permalink / raw)
To: longli, Andrew Rybchenko; +Cc: dev, Ajay Sharma, Long Li
On 7/7/2023 1:17 AM, longli@linuxonhyperv.com wrote:
> From: Long Li <longli@microsoft.com>
>
> On a fatal CQE error when coalescing is used, update the correct index
> and allow proceeding to the next CQE.
>
> Fixes: 3409e0f172f6 ("net/mana: implement Rx CQE coalescing">
> Signed-off-by: Long Li <longli@microsoft.com>
>
Applied to dpdk-next-net/main, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-10 11:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-07 0:17 [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used longli
2023-07-07 9:01 ` Ferruh Yigit
2023-07-07 18:01 ` Long Li
2023-07-10 10:42 ` Ferruh Yigit
2023-07-10 11:00 ` Ferruh Yigit
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).