All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] compressdev: fix end of comp PMD list macro conflict
@ 2023-01-01 13:47 Michael Baum
  2023-01-25  6:53 ` Michael Baum
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Michael Baum @ 2023-01-01 13:47 UTC (permalink / raw)
  To: dev
  Cc: Matan Azrad, Akhil Goyal, Ashish Gupta, Fan Zhang, Kai Ji,
	Thomas Monjalon, fiona.trahe, roy.fan.zhang, stable

The "rte_compressdev_info_get()" function retrieves the contextual
information of a device.
The output structure "dev_info" contains a list of devices supported
capabilities for each supported algorithm.

In this function description, it says the element after the last valid
element has op field set to "RTE_COMP_ALGO_LIST_END".
On the other hand, when this function used by
"rte_compressdev_capability_get()" function, it uses
"RTE_COMP_ALGO_UNSPECIFIED" as end of list as same as the
"RTE_COMP_END_OF_CAPABILITIES_LIST()".

The mlx5 and qat PMDs use "RTE_COMP_ALGO_LIST_END" as the end of
capabilities list. When "rte_compressdev_capability_get()" function is
called with unsupported algorithm, it might read memory out of bound.

This patch change the "rte_compressdev_info_get()" function description
to say using "RTE_COMP_ALGO_UNSPECIFIED" as the end of capabilities
list.
In addition, it moves both mlx5 and qat PMDs to use
"RTE_COMP_ALGO_UNSPECIFIED" through
"RTE_COMP_END_OF_CAPABILITIES_LIST()" macro.

Fixes: 5d432f364078 ("compressdev: add device capabilities")
Fixes: 2d148597ce76 ("compress/qat: add gen-specific implementation")
Fixes: 384bac8d6555 ("compress/mlx5: add supported capabilities")
Cc: fiona.trahe@intel.com
Cc: roy.fan.zhang@intel.com
Cc: matan@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>

---

After this change, I'm not sure about the purpose of
"RTE_COMP_ALGO_LIST_END".
There is no any other use of it in DPDK code, and it isn't represent the
number of algorithms supported by the API since the
"RTE_COMP_ALGO_UNSPECIFIED" is part of the enum.

Due to the compress API is experimental I think the
"RTE_COMP_ALGO_LIST_END" can be removed.



 drivers/compress/mlx5/mlx5_compress.c        | 4 +---
 drivers/compress/qat/dev/qat_comp_pmd_gen1.c | 2 +-
 drivers/compress/qat/dev/qat_comp_pmd_gen4.c | 2 +-
 lib/compressdev/rte_compressdev.h            | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/compress/mlx5/mlx5_compress.c b/drivers/compress/mlx5/mlx5_compress.c
index fb2bda9745..459e4b5e8a 100644
--- a/drivers/compress/mlx5/mlx5_compress.c
+++ b/drivers/compress/mlx5/mlx5_compress.c
@@ -96,9 +96,7 @@ static const struct rte_compressdev_capabilities mlx5_caps[] = {
 				      RTE_COMP_FF_HUFFMAN_DYNAMIC,
 		.window_size = {.min = 10, .max = 15, .increment = 1},
 	},
-	{
-		.algo = RTE_COMP_ALGO_LIST_END,
-	}
+	RTE_COMP_END_OF_CAPABILITIES_LIST()
 };
 
 static void
diff --git a/drivers/compress/qat/dev/qat_comp_pmd_gen1.c b/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
index 12d9d89072..3a8484eef1 100644
--- a/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
+++ b/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
@@ -26,7 +26,7 @@ const struct rte_compressdev_capabilities qat_gen1_comp_capabilities[] = {
 				RTE_COMP_FF_OOP_LB_IN_SGL_OUT |
 				RTE_COMP_FF_STATEFUL_DECOMPRESSION,
 	 .window_size = {.min = 15, .max = 15, .increment = 0} },
-	{RTE_COMP_ALGO_LIST_END, 0, {0, 0, 0} } };
+	 RTE_COMP_END_OF_CAPABILITIES_LIST() };
 
 static int
 qat_comp_dev_config_gen1(struct rte_compressdev *dev,
diff --git a/drivers/compress/qat/dev/qat_comp_pmd_gen4.c b/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
index 79b2ceb414..05906f13e0 100644
--- a/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
+++ b/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
@@ -25,7 +25,7 @@ qat_gen4_comp_capabilities[] = {
 				RTE_COMP_FF_OOP_SGL_IN_LB_OUT |
 				RTE_COMP_FF_OOP_LB_IN_SGL_OUT,
 	 .window_size = {.min = 15, .max = 15, .increment = 0} },
-	{RTE_COMP_ALGO_LIST_END, 0, {0, 0, 0} } };
+	 RTE_COMP_END_OF_CAPABILITIES_LIST() };
 
 static int
 qat_comp_dev_config_gen4(struct rte_compressdev *dev,
diff --git a/lib/compressdev/rte_compressdev.h b/lib/compressdev/rte_compressdev.h
index 42bda9fc79..7eb5c58798 100644
--- a/lib/compressdev/rte_compressdev.h
+++ b/lib/compressdev/rte_compressdev.h
@@ -353,7 +353,7 @@ rte_compressdev_stats_reset(uint8_t dev_id);
  * @note The capabilities field of dev_info is set to point to the first
  * element of an array of struct rte_compressdev_capabilities.
  * The element after the last valid element has it's op field set to
- * RTE_COMP_ALGO_LIST_END.
+ * RTE_COMP_ALGO_UNSPECIFIED.
  */
 __rte_experimental
 void
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH] compressdev: fix end of comp PMD list macro conflict
  2023-01-01 13:47 [PATCH] compressdev: fix end of comp PMD list macro conflict Michael Baum
@ 2023-01-25  6:53 ` Michael Baum
  2023-01-30 19:30 ` [EXT] " Akhil Goyal
  2023-02-01 15:35 ` [PATCH v2 0/2] compressdev: fix end of list enums conflict Michael Baum
  2 siblings, 0 replies; 11+ messages in thread
From: Michael Baum @ 2023-01-25  6:53 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Matan Azrad, Akhil Goyal, Ashish Gupta, Fan Zhang, Kai Ji,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	fiona.trahe, roy.fan.zhang, stable

Hi All, any comments?

From: Michael Baum <michaelba@nvidia.com> 

> The "rte_compressdev_info_get()" function retrieves the contextual information
> of a device.
> The output structure "dev_info" contains a list of devices supported capabilities
> for each supported algorithm.
> 
> In this function description, it says the element after the last valid element has
> op field set to "RTE_COMP_ALGO_LIST_END".
> On the other hand, when this function used by
> "rte_compressdev_capability_get()" function, it uses
> "RTE_COMP_ALGO_UNSPECIFIED" as end of list as same as the
> "RTE_COMP_END_OF_CAPABILITIES_LIST()".
> 
> The mlx5 and qat PMDs use "RTE_COMP_ALGO_LIST_END" as the end of
> capabilities list. When "rte_compressdev_capability_get()" function is called with
> unsupported algorithm, it might read memory out of bound.
> 
> This patch change the "rte_compressdev_info_get()" function description to say
> using "RTE_COMP_ALGO_UNSPECIFIED" as the end of capabilities list.
> In addition, it moves both mlx5 and qat PMDs to use
> "RTE_COMP_ALGO_UNSPECIFIED" through
> "RTE_COMP_END_OF_CAPABILITIES_LIST()" macro.
> 
> Fixes: 5d432f364078 ("compressdev: add device capabilities")
> Fixes: 2d148597ce76 ("compress/qat: add gen-specific implementation")
> Fixes: 384bac8d6555 ("compress/mlx5: add supported capabilities")
> Cc: fiona.trahe@intel.com
> Cc: roy.fan.zhang@intel.com
> Cc: matan@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> 
> ---
> 
> After this change, I'm not sure about the purpose of
> "RTE_COMP_ALGO_LIST_END".
> There is no any other use of it in DPDK code, and it isn't represent the number of
> algorithms supported by the API since the "RTE_COMP_ALGO_UNSPECIFIED" is
> part of the enum.
> 
> Due to the compress API is experimental I think the
> "RTE_COMP_ALGO_LIST_END" can be removed.
> 
> 
> 
>  drivers/compress/mlx5/mlx5_compress.c        | 4 +---
>  drivers/compress/qat/dev/qat_comp_pmd_gen1.c | 2 +-
> drivers/compress/qat/dev/qat_comp_pmd_gen4.c | 2 +-
>  lib/compressdev/rte_compressdev.h            | 2 +-
>  4 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/compress/mlx5/mlx5_compress.c
> b/drivers/compress/mlx5/mlx5_compress.c
> index fb2bda9745..459e4b5e8a 100644
> --- a/drivers/compress/mlx5/mlx5_compress.c
> +++ b/drivers/compress/mlx5/mlx5_compress.c
> @@ -96,9 +96,7 @@ static const struct rte_compressdev_capabilities
> mlx5_caps[] = {
>                                       RTE_COMP_FF_HUFFMAN_DYNAMIC,
>                 .window_size = {.min = 10, .max = 15, .increment = 1},
>         },
> -       {
> -               .algo = RTE_COMP_ALGO_LIST_END,
> -       }
> +       RTE_COMP_END_OF_CAPABILITIES_LIST()
>  };
> 
>  static void
> diff --git a/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
> b/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
> index 12d9d89072..3a8484eef1 100644
> --- a/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
> +++ b/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
> @@ -26,7 +26,7 @@ const struct rte_compressdev_capabilities
> qat_gen1_comp_capabilities[] = {
>                                 RTE_COMP_FF_OOP_LB_IN_SGL_OUT |
>                                 RTE_COMP_FF_STATEFUL_DECOMPRESSION,
>          .window_size = {.min = 15, .max = 15, .increment = 0} },
> -       {RTE_COMP_ALGO_LIST_END, 0, {0, 0, 0} } };
> +        RTE_COMP_END_OF_CAPABILITIES_LIST() };
> 
>  static int
>  qat_comp_dev_config_gen1(struct rte_compressdev *dev, diff --git
> a/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
> b/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
> index 79b2ceb414..05906f13e0 100644
> --- a/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
> +++ b/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
> @@ -25,7 +25,7 @@ qat_gen4_comp_capabilities[] = {
>                                 RTE_COMP_FF_OOP_SGL_IN_LB_OUT |
>                                 RTE_COMP_FF_OOP_LB_IN_SGL_OUT,
>          .window_size = {.min = 15, .max = 15, .increment = 0} },
> -       {RTE_COMP_ALGO_LIST_END, 0, {0, 0, 0} } };
> +        RTE_COMP_END_OF_CAPABILITIES_LIST() };
> 
>  static int
>  qat_comp_dev_config_gen4(struct rte_compressdev *dev, diff --git
> a/lib/compressdev/rte_compressdev.h b/lib/compressdev/rte_compressdev.h
> index 42bda9fc79..7eb5c58798 100644
> --- a/lib/compressdev/rte_compressdev.h
> +++ b/lib/compressdev/rte_compressdev.h
> @@ -353,7 +353,7 @@ rte_compressdev_stats_reset(uint8_t dev_id);
>   * @note The capabilities field of dev_info is set to point to the first
>   * element of an array of struct rte_compressdev_capabilities.
>   * The element after the last valid element has it's op field set to
> - * RTE_COMP_ALGO_LIST_END.
> + * RTE_COMP_ALGO_UNSPECIFIED.
>   */
>  __rte_experimental
>  void
> --
> 2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [EXT] [PATCH] compressdev: fix end of comp PMD list macro conflict
  2023-01-01 13:47 [PATCH] compressdev: fix end of comp PMD list macro conflict Michael Baum
  2023-01-25  6:53 ` Michael Baum
@ 2023-01-30 19:30 ` Akhil Goyal
  2023-01-31  8:23   ` Akhil Goyal
  2023-02-01 15:35 ` [PATCH v2 0/2] compressdev: fix end of list enums conflict Michael Baum
  2 siblings, 1 reply; 11+ messages in thread
From: Akhil Goyal @ 2023-01-30 19:30 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Matan Azrad, Ashish Gupta, Fan Zhang, Kai Ji, Thomas Monjalon,
	fiona.trahe, roy.fan.zhang, stable

> The "rte_compressdev_info_get()" function retrieves the contextual
> information of a device.
> The output structure "dev_info" contains a list of devices supported
> capabilities for each supported algorithm.
> 
> In this function description, it says the element after the last valid
> element has op field set to "RTE_COMP_ALGO_LIST_END".
> On the other hand, when this function used by
> "rte_compressdev_capability_get()" function, it uses
> "RTE_COMP_ALGO_UNSPECIFIED" as end of list as same as the
> "RTE_COMP_END_OF_CAPABILITIES_LIST()".
> 
> The mlx5 and qat PMDs use "RTE_COMP_ALGO_LIST_END" as the end of
> capabilities list. When "rte_compressdev_capability_get()" function is
> called with unsupported algorithm, it might read memory out of bound.
> 
> This patch change the "rte_compressdev_info_get()" function description
> to say using "RTE_COMP_ALGO_UNSPECIFIED" as the end of capabilities
> list.
> In addition, it moves both mlx5 and qat PMDs to use
> "RTE_COMP_ALGO_UNSPECIFIED" through
> "RTE_COMP_END_OF_CAPABILITIES_LIST()" macro.
> 
> Fixes: 5d432f364078 ("compressdev: add device capabilities")
> Fixes: 2d148597ce76 ("compress/qat: add gen-specific implementation")
> Fixes: 384bac8d6555 ("compress/mlx5: add supported capabilities")
> Cc: fiona.trahe@intel.com
> Cc: roy.fan.zhang@intel.com
> Cc: matan@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Michael Baum <michaelba@nvidia.com>
> 
> ---
> 
> After this change, I'm not sure about the purpose of
> "RTE_COMP_ALGO_LIST_END".
> There is no any other use of it in DPDK code, and it isn't represent the
> number of algorithms supported by the API since the
> "RTE_COMP_ALGO_UNSPECIFIED" is part of the enum.
> 
> Due to the compress API is experimental I think the
> "RTE_COMP_ALGO_LIST_END" can be removed.
> 
+1 to remove the list end enums. This will also help in avoiding ABI breakage
When we make this lib as stable.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [EXT] [PATCH] compressdev: fix end of comp PMD list macro conflict
  2023-01-30 19:30 ` [EXT] " Akhil Goyal
@ 2023-01-31  8:23   ` Akhil Goyal
  2023-02-01 13:19     ` Akhil Goyal
  0 siblings, 1 reply; 11+ messages in thread
From: Akhil Goyal @ 2023-01-31  8:23 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Matan Azrad, Ashish Gupta, Fan Zhang, Kai Ji, Thomas Monjalon,
	fiona.trahe, stable

> Subject: RE: [EXT] [PATCH] compressdev: fix end of comp PMD list macro
> conflict
> 
> > The "rte_compressdev_info_get()" function retrieves the contextual
> > information of a device.
> > The output structure "dev_info" contains a list of devices supported
> > capabilities for each supported algorithm.
> >
> > In this function description, it says the element after the last valid
> > element has op field set to "RTE_COMP_ALGO_LIST_END".
> > On the other hand, when this function used by
> > "rte_compressdev_capability_get()" function, it uses
> > "RTE_COMP_ALGO_UNSPECIFIED" as end of list as same as the
> > "RTE_COMP_END_OF_CAPABILITIES_LIST()".
> >
> > The mlx5 and qat PMDs use "RTE_COMP_ALGO_LIST_END" as the end of
> > capabilities list. When "rte_compressdev_capability_get()" function is
> > called with unsupported algorithm, it might read memory out of bound.
> >
> > This patch change the "rte_compressdev_info_get()" function description
> > to say using "RTE_COMP_ALGO_UNSPECIFIED" as the end of capabilities
> > list.
> > In addition, it moves both mlx5 and qat PMDs to use
> > "RTE_COMP_ALGO_UNSPECIFIED" through
> > "RTE_COMP_END_OF_CAPABILITIES_LIST()" macro.
> >
> > Fixes: 5d432f364078 ("compressdev: add device capabilities")
> > Fixes: 2d148597ce76 ("compress/qat: add gen-specific implementation")
> > Fixes: 384bac8d6555 ("compress/mlx5: add supported capabilities")
> > Cc: fiona.trahe@intel.com
> > Cc: roy.fan.zhang@intel.com
> > Cc: matan@nvidia.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Michael Baum <michaelba@nvidia.com>
> >
> > ---
> >
> > After this change, I'm not sure about the purpose of
> > "RTE_COMP_ALGO_LIST_END".
> > There is no any other use of it in DPDK code, and it isn't represent the
> > number of algorithms supported by the API since the
> > "RTE_COMP_ALGO_UNSPECIFIED" is part of the enum.
> >
> > Due to the compress API is experimental I think the
> > "RTE_COMP_ALGO_LIST_END" can be removed.
> >
> +1 to remove the list end enums. This will also help in avoiding ABI breakage
> When we make this lib as stable.

Even RTE_COMP_HASH_ALGO_LIST_END can also be removed.
It is not used anywhere.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [EXT] [PATCH] compressdev: fix end of comp PMD list macro conflict
  2023-01-31  8:23   ` Akhil Goyal
@ 2023-02-01 13:19     ` Akhil Goyal
  2023-02-01 13:29       ` Michael Baum
  0 siblings, 1 reply; 11+ messages in thread
From: Akhil Goyal @ 2023-02-01 13:19 UTC (permalink / raw)
  To: Akhil Goyal, Michael Baum, dev
  Cc: Matan Azrad, Ashish Gupta, Fan Zhang, Kai Ji, Thomas Monjalon,
	fiona.trahe, stable

Hi,
> > >
> > > After this change, I'm not sure about the purpose of
> > > "RTE_COMP_ALGO_LIST_END".
> > > There is no any other use of it in DPDK code, and it isn't represent the
> > > number of algorithms supported by the API since the
> > > "RTE_COMP_ALGO_UNSPECIFIED" is part of the enum.
> > >
> > > Due to the compress API is experimental I think the
> > > "RTE_COMP_ALGO_LIST_END" can be removed.
> > >
> > +1 to remove the list end enums. This will also help in avoiding ABI breakage
> > When we make this lib as stable.
> 
> Even RTE_COMP_HASH_ALGO_LIST_END can also be removed.
> It is not used anywhere.

Can you send a patch to remove these list end enums along with this patch?

-Akhil

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [EXT] [PATCH] compressdev: fix end of comp PMD list macro conflict
  2023-02-01 13:19     ` Akhil Goyal
@ 2023-02-01 13:29       ` Michael Baum
  2023-02-01 14:02         ` Akhil Goyal
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Baum @ 2023-02-01 13:29 UTC (permalink / raw)
  To: Akhil Goyal, dev
  Cc: Matan Azrad, Ashish Gupta, Fan Zhang, Kai Ji,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	fiona.trahe, stable



Hi,
> 
> 
> Hi,
> > > >
> > > > After this change, I'm not sure about the purpose of
> > > > "RTE_COMP_ALGO_LIST_END".
> > > > There is no any other use of it in DPDK code, and it isn't
> > > > represent the number of algorithms supported by the API since the
> > > > "RTE_COMP_ALGO_UNSPECIFIED" is part of the enum.
> > > >
> > > > Due to the compress API is experimental I think the
> > > > "RTE_COMP_ALGO_LIST_END" can be removed.
> > > >
> > > +1 to remove the list end enums. This will also help in avoiding ABI
> > > +breakage
> > > When we make this lib as stable.
> >
> > Even RTE_COMP_HASH_ALGO_LIST_END can also be removed.
> > It is not used anywhere.
> 
> Can you send a patch to remove these list end enums along with this patch?

In the same patch? Or add one?
> 
> -Akhil

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [EXT] [PATCH] compressdev: fix end of comp PMD list macro conflict
  2023-02-01 13:29       ` Michael Baum
@ 2023-02-01 14:02         ` Akhil Goyal
  0 siblings, 0 replies; 11+ messages in thread
From: Akhil Goyal @ 2023-02-01 14:02 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Matan Azrad, Ashish Gupta, Fan Zhang, Kai Ji,
	NBU-Contact-Thomas Monjalon (EXTERNAL),
	fiona.trahe, stable

> > Hi,
> > > > >
> > > > > After this change, I'm not sure about the purpose of
> > > > > "RTE_COMP_ALGO_LIST_END".
> > > > > There is no any other use of it in DPDK code, and it isn't
> > > > > represent the number of algorithms supported by the API since the
> > > > > "RTE_COMP_ALGO_UNSPECIFIED" is part of the enum.
> > > > >
> > > > > Due to the compress API is experimental I think the
> > > > > "RTE_COMP_ALGO_LIST_END" can be removed.
> > > > >
> > > > +1 to remove the list end enums. This will also help in avoiding ABI
> > > > +breakage
> > > > When we make this lib as stable.
> > >
> > > Even RTE_COMP_HASH_ALGO_LIST_END can also be removed.
> > > It is not used anywhere.
> >
> > Can you send a patch to remove these list end enums along with this patch?
> 
> In the same patch? Or add one?
Separate patch would be better as the current patch is talking about a conflict.
Removing the enums need not be backported, but this patch is required to be backported.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 0/2] compressdev: fix end of list enums conflict
  2023-01-01 13:47 [PATCH] compressdev: fix end of comp PMD list macro conflict Michael Baum
  2023-01-25  6:53 ` Michael Baum
  2023-01-30 19:30 ` [EXT] " Akhil Goyal
@ 2023-02-01 15:35 ` Michael Baum
  2023-02-01 15:35   ` [PATCH v2 1/2] compressdev: fix end of comp PMD list macro conflict Michael Baum
                     ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Michael Baum @ 2023-02-01 15:35 UTC (permalink / raw)
  To: dev
  Cc: Matan Azrad, Akhil Goyal, Ashish Gupta, Fan Zhang, Kai Ji,
	Thomas Monjalon

Handle the conflict about which enum value is used as end of list.

v2: add patch to remove useless list end enums.

Michael Baum (2):
  compressdev: fix end of comp PMD list macro conflict
  compressdev: remove useless list end enums

 drivers/compress/mlx5/mlx5_compress.c        | 4 +---
 drivers/compress/qat/dev/qat_comp_pmd_gen1.c | 2 +-
 drivers/compress/qat/dev/qat_comp_pmd_gen4.c | 2 +-
 lib/compressdev/rte_comp.h                   | 2 --
 lib/compressdev/rte_compressdev.h            | 2 +-
 5 files changed, 4 insertions(+), 8 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/2] compressdev: fix end of comp PMD list macro conflict
  2023-02-01 15:35 ` [PATCH v2 0/2] compressdev: fix end of list enums conflict Michael Baum
@ 2023-02-01 15:35   ` Michael Baum
  2023-02-01 15:35   ` [PATCH v2 2/2] compressdev: remove useless list end enums Michael Baum
  2023-02-05 17:24   ` [EXT] [PATCH v2 0/2] compressdev: fix end of list enums conflict Akhil Goyal
  2 siblings, 0 replies; 11+ messages in thread
From: Michael Baum @ 2023-02-01 15:35 UTC (permalink / raw)
  To: dev
  Cc: Matan Azrad, Akhil Goyal, Ashish Gupta, Fan Zhang, Kai Ji,
	Thomas Monjalon, fiona.trahe, roy.fan.zhang, stable

The "rte_compressdev_info_get()" function retrieves the contextual
information of a device.
The output structure "dev_info" contains a list of devices supported
capabilities for each supported algorithm.

In this function description, it says the element after the last valid
element has op field set to "RTE_COMP_ALGO_LIST_END".
On the other hand, when this function used by
"rte_compressdev_capability_get()" function, it uses
"RTE_COMP_ALGO_UNSPECIFIED" as end of list as same as the
"RTE_COMP_END_OF_CAPABILITIES_LIST()".

The mlx5 and qat PMDs use "RTE_COMP_ALGO_LIST_END" as the end of
capabilities list. When "rte_compressdev_capability_get()" function is
called with unsupported algorithm, it might read memory out of bound.

This patch change the "rte_compressdev_info_get()" function description
to say using "RTE_COMP_ALGO_UNSPECIFIED" as the end of capabilities
list.
In addition, it moves both mlx5 and qat PMDs to use
"RTE_COMP_ALGO_UNSPECIFIED" through
"RTE_COMP_END_OF_CAPABILITIES_LIST()" macro.

Fixes: 5d432f364078 ("compressdev: add device capabilities")
Fixes: 2d148597ce76 ("compress/qat: add gen-specific implementation")
Fixes: 384bac8d6555 ("compress/mlx5: add supported capabilities")
Cc: fiona.trahe@intel.com
Cc: roy.fan.zhang@intel.com
Cc: matan@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 drivers/compress/mlx5/mlx5_compress.c        | 4 +---
 drivers/compress/qat/dev/qat_comp_pmd_gen1.c | 2 +-
 drivers/compress/qat/dev/qat_comp_pmd_gen4.c | 2 +-
 lib/compressdev/rte_compressdev.h            | 2 +-
 4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/compress/mlx5/mlx5_compress.c b/drivers/compress/mlx5/mlx5_compress.c
index fb2bda9745..459e4b5e8a 100644
--- a/drivers/compress/mlx5/mlx5_compress.c
+++ b/drivers/compress/mlx5/mlx5_compress.c
@@ -96,9 +96,7 @@ static const struct rte_compressdev_capabilities mlx5_caps[] = {
 				      RTE_COMP_FF_HUFFMAN_DYNAMIC,
 		.window_size = {.min = 10, .max = 15, .increment = 1},
 	},
-	{
-		.algo = RTE_COMP_ALGO_LIST_END,
-	}
+	RTE_COMP_END_OF_CAPABILITIES_LIST()
 };
 
 static void
diff --git a/drivers/compress/qat/dev/qat_comp_pmd_gen1.c b/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
index 12d9d89072..3a8484eef1 100644
--- a/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
+++ b/drivers/compress/qat/dev/qat_comp_pmd_gen1.c
@@ -26,7 +26,7 @@ const struct rte_compressdev_capabilities qat_gen1_comp_capabilities[] = {
 				RTE_COMP_FF_OOP_LB_IN_SGL_OUT |
 				RTE_COMP_FF_STATEFUL_DECOMPRESSION,
 	 .window_size = {.min = 15, .max = 15, .increment = 0} },
-	{RTE_COMP_ALGO_LIST_END, 0, {0, 0, 0} } };
+	 RTE_COMP_END_OF_CAPABILITIES_LIST() };
 
 static int
 qat_comp_dev_config_gen1(struct rte_compressdev *dev,
diff --git a/drivers/compress/qat/dev/qat_comp_pmd_gen4.c b/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
index 79b2ceb414..05906f13e0 100644
--- a/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
+++ b/drivers/compress/qat/dev/qat_comp_pmd_gen4.c
@@ -25,7 +25,7 @@ qat_gen4_comp_capabilities[] = {
 				RTE_COMP_FF_OOP_SGL_IN_LB_OUT |
 				RTE_COMP_FF_OOP_LB_IN_SGL_OUT,
 	 .window_size = {.min = 15, .max = 15, .increment = 0} },
-	{RTE_COMP_ALGO_LIST_END, 0, {0, 0, 0} } };
+	 RTE_COMP_END_OF_CAPABILITIES_LIST() };
 
 static int
 qat_comp_dev_config_gen4(struct rte_compressdev *dev,
diff --git a/lib/compressdev/rte_compressdev.h b/lib/compressdev/rte_compressdev.h
index 42bda9fc79..7eb5c58798 100644
--- a/lib/compressdev/rte_compressdev.h
+++ b/lib/compressdev/rte_compressdev.h
@@ -353,7 +353,7 @@ rte_compressdev_stats_reset(uint8_t dev_id);
  * @note The capabilities field of dev_info is set to point to the first
  * element of an array of struct rte_compressdev_capabilities.
  * The element after the last valid element has it's op field set to
- * RTE_COMP_ALGO_LIST_END.
+ * RTE_COMP_ALGO_UNSPECIFIED.
  */
 __rte_experimental
 void
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/2] compressdev: remove useless list end enums
  2023-02-01 15:35 ` [PATCH v2 0/2] compressdev: fix end of list enums conflict Michael Baum
  2023-02-01 15:35   ` [PATCH v2 1/2] compressdev: fix end of comp PMD list macro conflict Michael Baum
@ 2023-02-01 15:35   ` Michael Baum
  2023-02-05 17:24   ` [EXT] [PATCH v2 0/2] compressdev: fix end of list enums conflict Akhil Goyal
  2 siblings, 0 replies; 11+ messages in thread
From: Michael Baum @ 2023-02-01 15:35 UTC (permalink / raw)
  To: dev
  Cc: Matan Azrad, Akhil Goyal, Ashish Gupta, Fan Zhang, Kai Ji,
	Thomas Monjalon

The both "RTE_COMP_ALGO_LIST_END" and "RTE_COMP_HASH_ALGO_LIST_END" are
useless. This patch removes them from the library.

Signed-off-by: Michael Baum <michaelba@nvidia.com>
---
 lib/compressdev/rte_comp.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/compressdev/rte_comp.h b/lib/compressdev/rte_comp.h
index a8f398b57b..5bd711fda1 100644
--- a/lib/compressdev/rte_comp.h
+++ b/lib/compressdev/rte_comp.h
@@ -109,7 +109,6 @@ enum rte_comp_algorithm {
 	/**< LZS compression algorithm
 	 * https://tools.ietf.org/html/rfc2395
 	 */
-	RTE_COMP_ALGO_LIST_END
 };
 
 /** Compression Hash Algorithms */
@@ -120,7 +119,6 @@ enum rte_comp_hash_algorithm {
 	/**< SHA1 hash algorithm */
 	RTE_COMP_HASH_ALGO_SHA2_256,
 	/**< SHA256 hash algorithm of SHA2 family */
-	RTE_COMP_HASH_ALGO_LIST_END
 };
 
 /**< Compression Level.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [EXT] [PATCH v2 0/2] compressdev: fix end of list enums conflict
  2023-02-01 15:35 ` [PATCH v2 0/2] compressdev: fix end of list enums conflict Michael Baum
  2023-02-01 15:35   ` [PATCH v2 1/2] compressdev: fix end of comp PMD list macro conflict Michael Baum
  2023-02-01 15:35   ` [PATCH v2 2/2] compressdev: remove useless list end enums Michael Baum
@ 2023-02-05 17:24   ` Akhil Goyal
  2 siblings, 0 replies; 11+ messages in thread
From: Akhil Goyal @ 2023-02-05 17:24 UTC (permalink / raw)
  To: Michael Baum, dev
  Cc: Matan Azrad, Ashish Gupta, Fan Zhang, Kai Ji, Thomas Monjalon

> Handle the conflict about which enum value is used as end of list.
> 
> v2: add patch to remove useless list end enums.
> 
> Michael Baum (2):
>   compressdev: fix end of comp PMD list macro conflict
>   compressdev: remove useless list end enums
> 
>  drivers/compress/mlx5/mlx5_compress.c        | 4 +---
>  drivers/compress/qat/dev/qat_comp_pmd_gen1.c | 2 +-
>  drivers/compress/qat/dev/qat_comp_pmd_gen4.c | 2 +-
>  lib/compressdev/rte_comp.h                   | 2 --
>  lib/compressdev/rte_compressdev.h            | 2 +-
>  5 files changed, 4 insertions(+), 8 deletions(-)
> 
Series Acked-by: Akhil Goyal <gakhil@marvell.com>
Applied to dpdk-next-crypto

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-02-05 17:24 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-01 13:47 [PATCH] compressdev: fix end of comp PMD list macro conflict Michael Baum
2023-01-25  6:53 ` Michael Baum
2023-01-30 19:30 ` [EXT] " Akhil Goyal
2023-01-31  8:23   ` Akhil Goyal
2023-02-01 13:19     ` Akhil Goyal
2023-02-01 13:29       ` Michael Baum
2023-02-01 14:02         ` Akhil Goyal
2023-02-01 15:35 ` [PATCH v2 0/2] compressdev: fix end of list enums conflict Michael Baum
2023-02-01 15:35   ` [PATCH v2 1/2] compressdev: fix end of comp PMD list macro conflict Michael Baum
2023-02-01 15:35   ` [PATCH v2 2/2] compressdev: remove useless list end enums Michael Baum
2023-02-05 17:24   ` [EXT] [PATCH v2 0/2] compressdev: fix end of list enums conflict Akhil Goyal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.