memif: memif driver does not crashes when there's different N of TX and RX queues

Message ID 20220726101628.2118564-1-huzaifa.rahman@emumba.com (mailing list archive)
State Accepted, archived
Delegated to: Andrew Rybchenko
Headers
Series memif: memif driver does not crashes when there's different N of TX and RX queues |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

Huzaifa Rahman July 26, 2022, 10:16 a.m. UTC
  Bugzilla ID: 734

there's a bug in memif_stats_get() function due to confusion
between C2S (client->server) and S2C (server->client) rings,
causing a crash if there's a different number of RX and TX queues.

this is fixed by selectiing the correct rings for RX and TX i.e
for RX, S2C rings are selected and for TX, C2S rings are selected.

Signed-off-by: huzaifa.rahman <huzaifa.rahman@emumba.com>
---
 drivers/net/memif/rte_eth_memif.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
  

Comments

Joyce Kong Aug. 8, 2022, 10:39 a.m. UTC | #1
Hi Huzaifa,

This patch looks good to me.
And would you please help review my memif patches?
https://patches.dpdk.org/project/dpdk/cover/20220701102815.1444223-1-joyce.kong@arm.com/

Thanks,
Joyce

> -----Original Message-----
> From: huzaifa.rahman <huzaifa.rahman@emumba.com>
> Sent: Tuesday, July 26, 2022 6:16 PM
> To: jgrajcia@cisco.com
> Cc: dev@dpdk.org; huzaifa.rahman <huzaifa.rahman@emumba.com>
> Subject: [PATCH] memif: memif driver does not crashes when there's
> different N of TX and RX queues
net/memif: fix memif crash with different Tx Rx queues

>
> Bugzilla ID: 734
>
> there's a bug in memif_stats_get() function due to confusion between C2S
> (client->server) and S2C (server->client) rings, causing a crash if there's a
> different number of RX and TX queues.
>
> this is fixed by selectiing the correct rings for RX and TX i.e for RX, S2C rings
> are selected and for TX, C2S rings are selected.
>
Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
Cc: stable@dpdk.org

> Signed-off-by: huzaifa.rahman <huzaifa.rahman@emumba.com>
Reviewed-by: Joyce Kong <joyce.kong@arm.com>

> ---
>  drivers/net/memif/rte_eth_memif.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/memif/rte_eth_memif.c
> b/drivers/net/memif/rte_eth_memif.c
> index dd951b8296..e56df84e10 100644
> --- a/drivers/net/memif/rte_eth_memif.c
> +++ b/drivers/net/memif/rte_eth_memif.c
> @@ -1444,8 +1444,8 @@ memif_stats_get(struct rte_eth_dev *dev, struct
> rte_eth_stats *stats)
>       stats->opackets = 0;
>       stats->obytes = 0;
>
> -     tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd-
> >run.num_c2s_rings :
> -         pmd->run.num_s2c_rings;
> +     tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd-
> >run.num_s2c_rings :
> +         pmd->run.num_c2s_rings;
>       nq = (tmp < RTE_ETHDEV_QUEUE_STAT_CNTRS) ? tmp :
>           RTE_ETHDEV_QUEUE_STAT_CNTRS;
>
> @@ -1458,8 +1458,8 @@ memif_stats_get(struct rte_eth_dev *dev, struct
> rte_eth_stats *stats)
>               stats->ibytes += mq->n_bytes;
>       }
>
> -     tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd-
> >run.num_s2c_rings :
> -         pmd->run.num_c2s_rings;
> +     tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd-
> >run.num_c2s_rings :
> +         pmd->run.num_s2c_rings;
>       nq = (tmp < RTE_ETHDEV_QUEUE_STAT_CNTRS) ? tmp :
>           RTE_ETHDEV_QUEUE_STAT_CNTRS;
>
> --
> 2.25.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  
Andrew Rybchenko Oct. 4, 2022, 2:53 p.m. UTC | #2
On 8/8/22 13:39, Joyce Kong wrote:
> Hi Huzaifa,
> 
> This patch looks good to me.
> And would you please help review my memif patches?
> https://patches.dpdk.org/project/dpdk/cover/20220701102815.1444223-1-joyce.kong@arm.com/
> 
> Thanks,
> Joyce
> 
>> -----Original Message-----
>> From: huzaifa.rahman <huzaifa.rahman@emumba.com>
>> Sent: Tuesday, July 26, 2022 6:16 PM
>> To: jgrajcia@cisco.com
>> Cc: dev@dpdk.org; huzaifa.rahman <huzaifa.rahman@emumba.com>
>> Subject: [PATCH] memif: memif driver does not crashes when there's
>> different N of TX and RX queues
> net/memif: fix memif crash with different Tx Rx queues
> 
>>
>> Bugzilla ID: 734
>>
>> there's a bug in memif_stats_get() function due to confusion between C2S
>> (client->server) and S2C (server->client) rings, causing a crash if there's a
>> different number of RX and TX queues.
>>
>> this is fixed by selectiing the correct rings for RX and TX i.e for RX, S2C rings
>> are selected and for TX, C2S rings are selected.
>>
> Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
> Cc: stable@dpdk.org
> 
>> Signed-off-by: huzaifa.rahman <huzaifa.rahman@emumba.com>
> Reviewed-by: Joyce Kong <joyce.kong@arm.com>

Fixed above on applying.

Applied to dpdk-next-net/main, thanks.
  
Huzaifa Rahman Nov. 10, 2022, 10:02 a.m. UTC | #3
Hi,

Is there any other work/changes required for this patch to be submitted?

Thanks


On Tue, Oct 4, 2022 at 7:53 PM Andrew Rybchenko <
andrew.rybchenko@oktetlabs.ru> wrote:

> On 8/8/22 13:39, Joyce Kong wrote:
> > Hi Huzaifa,
> >
> > This patch looks good to me.
> > And would you please help review my memif patches?
> >
> https://patches.dpdk.org/project/dpdk/cover/20220701102815.1444223-1-joyce.kong@arm.com/
> >
> > Thanks,
> > Joyce
> >
> >> -----Original Message-----
> >> From: huzaifa.rahman <huzaifa.rahman@emumba.com>
> >> Sent: Tuesday, July 26, 2022 6:16 PM
> >> To: jgrajcia@cisco.com
> >> Cc: dev@dpdk.org; huzaifa.rahman <huzaifa.rahman@emumba.com>
> >> Subject: [PATCH] memif: memif driver does not crashes when there's
> >> different N of TX and RX queues
> > net/memif: fix memif crash with different Tx Rx queues
> >
> >>
> >> Bugzilla ID: 734
> >>
> >> there's a bug in memif_stats_get() function due to confusion between C2S
> >> (client->server) and S2C (server->client) rings, causing a crash if
> there's a
> >> different number of RX and TX queues.
> >>
> >> this is fixed by selectiing the correct rings for RX and TX i.e for RX,
> S2C rings
> >> are selected and for TX, C2S rings are selected.
> >>
> > Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
> > Cc: stable@dpdk.org
> >
> >> Signed-off-by: huzaifa.rahman <huzaifa.rahman@emumba.com>
> > Reviewed-by: Joyce Kong <joyce.kong@arm.com>
>
> Fixed above on applying.
>
> Applied to dpdk-next-net/main, thanks.
>
>
>
  
Ferruh Yigit Nov. 10, 2022, 2:20 p.m. UTC | #4
On 11/10/2022 10:02 AM, Huzaifa Rahman wrote:
> Hi,
> 
> Is there any other work/changes required for this patch to be submitted?
> 

Hi Huzaifa,

Patch seems already merged by Andrew and pulled to main repo:
https://git.dpdk.org/dpdk/commit/?id=231435a5e6c7fa915697d8f84a91b44176

So it will in oncoming 22.11 release.

> Thanks
> 
> 
> On Tue, Oct 4, 2022 at 7:53 PM Andrew Rybchenko 
> <andrew.rybchenko@oktetlabs.ru <mailto:andrew.rybchenko@oktetlabs.ru>> 
> wrote:
> 
>     On 8/8/22 13:39, Joyce Kong wrote:
>      > Hi Huzaifa,
>      >
>      > This patch looks good to me.
>      > And would you please help review my memif patches?
>      >
>     https://patches.dpdk.org/project/dpdk/cover/20220701102815.1444223-1-joyce.kong@arm.com/ <https://patches.dpdk.org/project/dpdk/cover/20220701102815.1444223-1-joyce.kong@arm.com/>
>      >
>      > Thanks,
>      > Joyce
>      >
>      >> -----Original Message-----
>      >> From: huzaifa.rahman <huzaifa.rahman@emumba.com
>     <mailto:huzaifa.rahman@emumba.com>>
>      >> Sent: Tuesday, July 26, 2022 6:16 PM
>      >> To: jgrajcia@cisco.com <mailto:jgrajcia@cisco.com>
>      >> Cc: dev@dpdk.org <mailto:dev@dpdk.org>; huzaifa.rahman
>     <huzaifa.rahman@emumba.com <mailto:huzaifa.rahman@emumba.com>>
>      >> Subject: [PATCH] memif: memif driver does not crashes when there's
>      >> different N of TX and RX queues
>      > net/memif: fix memif crash with different Tx Rx queues
>      >
>      >>
>      >> Bugzilla ID: 734
>      >>
>      >> there's a bug in memif_stats_get() function due to confusion
>     between C2S
>      >> (client->server) and S2C (server->client) rings, causing a crash
>     if there's a
>      >> different number of RX and TX queues.
>      >>
>      >> this is fixed by selectiing the correct rings for RX and TX i.e
>     for RX, S2C rings
>      >> are selected and for TX, C2S rings are selected.
>      >>
>      > Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
>      > Cc: stable@dpdk.org <mailto:stable@dpdk.org>
>      >
>      >> Signed-off-by: huzaifa.rahman <huzaifa.rahman@emumba.com
>     <mailto:huzaifa.rahman@emumba.com>>
>      > Reviewed-by: Joyce Kong <joyce.kong@arm.com
>     <mailto:joyce.kong@arm.com>>
> 
>     Fixed above on applying.
> 
>     Applied to dpdk-next-net/main, thanks.
> 
>
  

Patch

diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c
index dd951b8296..e56df84e10 100644
--- a/drivers/net/memif/rte_eth_memif.c
+++ b/drivers/net/memif/rte_eth_memif.c
@@ -1444,8 +1444,8 @@  memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	stats->opackets = 0;
 	stats->obytes = 0;
 
-	tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd->run.num_c2s_rings :
-	    pmd->run.num_s2c_rings;
+	tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd->run.num_s2c_rings :
+	    pmd->run.num_c2s_rings;
 	nq = (tmp < RTE_ETHDEV_QUEUE_STAT_CNTRS) ? tmp :
 	    RTE_ETHDEV_QUEUE_STAT_CNTRS;
 
@@ -1458,8 +1458,8 @@  memif_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		stats->ibytes += mq->n_bytes;
 	}
 
-	tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd->run.num_s2c_rings :
-	    pmd->run.num_c2s_rings;
+	tmp = (pmd->role == MEMIF_ROLE_CLIENT) ? pmd->run.num_c2s_rings :
+	    pmd->run.num_s2c_rings;
 	nq = (tmp < RTE_ETHDEV_QUEUE_STAT_CNTRS) ? tmp :
 	    RTE_ETHDEV_QUEUE_STAT_CNTRS;