[dpdk-dev,v2] net/ixgbe: clean up rte_eth_dev_info_get

Message ID 1486346972-30710-1-git-send-email-wenzhuo.lu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Wenzhuo Lu Feb. 6, 2017, 2:09 a.m. UTC
  It's not appropriate to call rte_eth_dev_info_get in PMD,
as rte_eth_dev_info_get need to get info from PMD.
Remove rte_eth_dev_info_get from PMD code and get the
info directly.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
v2:
- change is_ixgbe_pmd to is_device_supported to make it more generic.

 drivers/net/ixgbe/ixgbe_ethdev.c | 160 ++++++++++++++++++---------------------
 1 file changed, 75 insertions(+), 85 deletions(-)
  

Comments

Tiwei Bie Feb. 6, 2017, 2:30 a.m. UTC | #1
On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote:
[...]
>  static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct ixgbe_dcb_config *dcb_config);
> -static int is_ixgbe_pmd(const char *driver_name);
> +static int is_device_supported(struct rte_eth_dev *dev, struct eth_driver *drv);
>  

Should be:
static bool is_device_supported(struct rte_eth_dev *dev, struct eth_driver *drv);

>  /* For Virtual Function support */
>  static int eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev);
> @@ -4380,16 +4380,14 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>  	ixgbe_add_rar(dev, addr, 0, 0);
>  }
>  
> -static int
> -is_ixgbe_pmd(const char *driver_name)
> +static bool
> +is_device_supported(struct rte_eth_dev *dev, struct eth_driver *drv)
>  {
> -	if (!strstr(driver_name, "ixgbe"))
> -		return -ENOTSUP;
> +	if (strcmp(dev->driver->pci_drv.driver.name,
> +		   drv->pci_drv.driver.name))
> +		return FALSE;
>  

It would be better to use `false' instead of `FALSE'.

Best regards,
Tiwei Bie
  
Wenzhuo Lu Feb. 6, 2017, 2:41 a.m. UTC | #2
Hi Tiwei,

> -----Original Message-----

> From: Bie, Tiwei

> Sent: Monday, February 6, 2017 10:31 AM

> To: Lu, Wenzhuo

> Cc: dev@dpdk.org

> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get

> 

> On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote:

> [...]

> >  static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct

> > ixgbe_dcb_config *dcb_config); -static int is_ixgbe_pmd(const char

> > *driver_name);

> > +static int is_device_supported(struct rte_eth_dev *dev, struct

> > +eth_driver *drv);

> >

> 

> Should be:

> static bool is_device_supported(struct rte_eth_dev *dev, struct eth_driver

> *drv);

O, forget to change it. Thanks.

> 

> >  /* For Virtual Function support */

> >  static int eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev); @@

> > -4380,16 +4380,14 @@ static int

> ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,

> >  	ixgbe_add_rar(dev, addr, 0, 0);

> >  }

> >

> > -static int

> > -is_ixgbe_pmd(const char *driver_name)

> > +static bool

> > +is_device_supported(struct rte_eth_dev *dev, struct eth_driver *drv)

> >  {

> > -	if (!strstr(driver_name, "ixgbe"))

> > -		return -ENOTSUP;

> > +	if (strcmp(dev->driver->pci_drv.driver.name,

> > +		   drv->pci_drv.driver.name))

> > +		return FALSE;

> >

> 

> It would be better to use `false' instead of `FALSE'.

I see both 'false' and 'FALSE' are defined and used. Is there any reason that 'false' is better?

> 

> Best regards,

> Tiwei Bie
  
Tiwei Bie Feb. 6, 2017, 2:51 a.m. UTC | #3
On Mon, Feb 06, 2017 at 10:41:28AM +0800, Lu, Wenzhuo wrote:
> Hi Tiwei,
> 
> > -----Original Message-----
> > From: Bie, Tiwei
> > Sent: Monday, February 6, 2017 10:31 AM
> > To: Lu, Wenzhuo
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get
> > 
> > On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote:
> > [...]
> > >  static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct
> > > ixgbe_dcb_config *dcb_config); -static int is_ixgbe_pmd(const char
> > > *driver_name);
> > > +static int is_device_supported(struct rte_eth_dev *dev, struct
> > > +eth_driver *drv);
> > >
> > 
> > Should be:
> > static bool is_device_supported(struct rte_eth_dev *dev, struct eth_driver
> > *drv);
> O, forget to change it. Thanks.
> 
> > 
> > >  /* For Virtual Function support */
> > >  static int eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev); @@
> > > -4380,16 +4380,14 @@ static int
> > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
> > >  	ixgbe_add_rar(dev, addr, 0, 0);
> > >  }
> > >
> > > -static int
> > > -is_ixgbe_pmd(const char *driver_name)
> > > +static bool
> > > +is_device_supported(struct rte_eth_dev *dev, struct eth_driver *drv)
> > >  {
> > > -	if (!strstr(driver_name, "ixgbe"))
> > > -		return -ENOTSUP;
> > > +	if (strcmp(dev->driver->pci_drv.driver.name,
> > > +		   drv->pci_drv.driver.name))
> > > +		return FALSE;
> > >
> > 
> > It would be better to use `false' instead of `FALSE'.
> I see both 'false' and 'FALSE' are defined and used. Is there any reason that 'false' is better?
> 

I think `true' and `false' are standard keywords defined and
reserved by C. So I think it would be better to use them if
the return type is `bool'.

Best regards,
Tiwei Bie
  
Wenzhuo Lu Feb. 6, 2017, 2:59 a.m. UTC | #4
Hi Tiwei,


> -----Original Message-----

> From: Bie, Tiwei

> Sent: Monday, February 6, 2017 10:51 AM

> To: Lu, Wenzhuo

> Cc: dev@dpdk.org

> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get

> 

> On Mon, Feb 06, 2017 at 10:41:28AM +0800, Lu, Wenzhuo wrote:

> > Hi Tiwei,

> >

> > > -----Original Message-----

> > > From: Bie, Tiwei

> > > Sent: Monday, February 6, 2017 10:31 AM

> > > To: Lu, Wenzhuo

> > > Cc: dev@dpdk.org

> > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up

> > > rte_eth_dev_info_get

> > >

> > > On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote:

> > > [...]

> > > >  static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct

> > > > ixgbe_dcb_config *dcb_config); -static int is_ixgbe_pmd(const char

> > > > *driver_name);

> > > > +static int is_device_supported(struct rte_eth_dev *dev, struct

> > > > +eth_driver *drv);

> > > >

> > >

> > > Should be:

> > > static bool is_device_supported(struct rte_eth_dev *dev, struct

> > > eth_driver *drv);

> > O, forget to change it. Thanks.

> >

> > >

> > > >  /* For Virtual Function support */  static int

> > > > eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev); @@

> > > > -4380,16 +4380,14 @@ static int

> > > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,

> > > >  	ixgbe_add_rar(dev, addr, 0, 0);

> > > >  }

> > > >

> > > > -static int

> > > > -is_ixgbe_pmd(const char *driver_name)

> > > > +static bool

> > > > +is_device_supported(struct rte_eth_dev *dev, struct eth_driver

> > > > +*drv)

> > > >  {

> > > > -	if (!strstr(driver_name, "ixgbe"))

> > > > -		return -ENOTSUP;

> > > > +	if (strcmp(dev->driver->pci_drv.driver.name,

> > > > +		   drv->pci_drv.driver.name))

> > > > +		return FALSE;

> > > >

> > >

> > > It would be better to use `false' instead of `FALSE'.

> > I see both 'false' and 'FALSE' are defined and used. Is there any reason that

> 'false' is better?

> >

> 

> I think `true' and `false' are standard keywords defined and reserved by C. So I

> think it would be better to use them if the return type is `bool'.

O, there's no 'bool' in C. You have to define it. The same for 'false' and 'true'.

> 

> Best regards,

> Tiwei Bie
  
Tiwei Bie Feb. 6, 2017, 3:08 a.m. UTC | #5
On Mon, Feb 06, 2017 at 10:59:42AM +0800, Lu, Wenzhuo wrote:
> Hi Tiwei,
> 
> 
> > -----Original Message-----
> > From: Bie, Tiwei
> > Sent: Monday, February 6, 2017 10:51 AM
> > To: Lu, Wenzhuo
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get
> > 
> > On Mon, Feb 06, 2017 at 10:41:28AM +0800, Lu, Wenzhuo wrote:
> > > Hi Tiwei,
> > >
> > > > -----Original Message-----
> > > > From: Bie, Tiwei
> > > > Sent: Monday, February 6, 2017 10:31 AM
> > > > To: Lu, Wenzhuo
> > > > Cc: dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up
> > > > rte_eth_dev_info_get
> > > >
> > > > On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote:
> > > > [...]
> > > > >  static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct
> > > > > ixgbe_dcb_config *dcb_config); -static int is_ixgbe_pmd(const char
> > > > > *driver_name);
> > > > > +static int is_device_supported(struct rte_eth_dev *dev, struct
> > > > > +eth_driver *drv);
> > > > >
> > > >
> > > > Should be:
> > > > static bool is_device_supported(struct rte_eth_dev *dev, struct
> > > > eth_driver *drv);
> > > O, forget to change it. Thanks.
> > >
> > > >
> > > > >  /* For Virtual Function support */  static int
> > > > > eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev); @@
> > > > > -4380,16 +4380,14 @@ static int
> > > > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
> > > > >  	ixgbe_add_rar(dev, addr, 0, 0);
> > > > >  }
> > > > >
> > > > > -static int
> > > > > -is_ixgbe_pmd(const char *driver_name)
> > > > > +static bool
> > > > > +is_device_supported(struct rte_eth_dev *dev, struct eth_driver
> > > > > +*drv)
> > > > >  {
> > > > > -	if (!strstr(driver_name, "ixgbe"))
> > > > > -		return -ENOTSUP;
> > > > > +	if (strcmp(dev->driver->pci_drv.driver.name,
> > > > > +		   drv->pci_drv.driver.name))
> > > > > +		return FALSE;
> > > > >
> > > >
> > > > It would be better to use `false' instead of `FALSE'.
> > > I see both 'false' and 'FALSE' are defined and used. Is there any reason that
> > 'false' is better?
> > >
> > 
> > I think `true' and `false' are standard keywords defined and reserved by C. So I
> > think it would be better to use them if the return type is `bool'.
> O, there's no 'bool' in C. You have to define it. The same for 'false' and 'true'.
> 

The `bool', `true' and `false' are all standard keywords defined and
reserved by C, although the stdbool.h is not used in ixgbe.

C adds this support by introducing a new header stdbool.h:

#ifndef __bool_true_false_are_defined
#define __bool_true_false_are_defined   1

#ifndef __cplusplus

#define false   0
#define true    1

#define bool    _Bool
#if __STDC_VERSION__ < 199901L && __GNUC__ < 3 && !defined(__INTEL_COMPILER)
typedef int     _Bool;
#endif

#endif /* !__cplusplus */
#endif /* __bool_true_false_are_defined */

Best regards,
Tiwei Bie
  
Wenzhuo Lu Feb. 6, 2017, 3:45 a.m. UTC | #6
Hi Tiwei,

> -----Original Message-----

> From: Bie, Tiwei

> Sent: Monday, February 6, 2017 11:08 AM

> To: Lu, Wenzhuo

> Cc: dev@dpdk.org

> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get

> 

> On Mon, Feb 06, 2017 at 10:59:42AM +0800, Lu, Wenzhuo wrote:

> > Hi Tiwei,

> >

> >

> > > -----Original Message-----

> > > From: Bie, Tiwei

> > > Sent: Monday, February 6, 2017 10:51 AM

> > > To: Lu, Wenzhuo

> > > Cc: dev@dpdk.org

> > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up

> > > rte_eth_dev_info_get

> > >

> > > On Mon, Feb 06, 2017 at 10:41:28AM +0800, Lu, Wenzhuo wrote:

> > > > Hi Tiwei,

> > > >

> > > > > -----Original Message-----

> > > > > From: Bie, Tiwei

> > > > > Sent: Monday, February 6, 2017 10:31 AM

> > > > > To: Lu, Wenzhuo

> > > > > Cc: dev@dpdk.org

> > > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up

> > > > > rte_eth_dev_info_get

> > > > >

> > > > > On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote:

> > > > > [...]

> > > > > >  static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct

> > > > > > ixgbe_dcb_config *dcb_config); -static int is_ixgbe_pmd(const

> > > > > > char *driver_name);

> > > > > > +static int is_device_supported(struct rte_eth_dev *dev,

> > > > > > +struct eth_driver *drv);

> > > > > >

> > > > >

> > > > > Should be:

> > > > > static bool is_device_supported(struct rte_eth_dev *dev, struct

> > > > > eth_driver *drv);

> > > > O, forget to change it. Thanks.

> > > >

> > > > >

> > > > > >  /* For Virtual Function support */  static int

> > > > > > eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev); @@

> > > > > > -4380,16 +4380,14 @@ static int

> > > > > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev

> > > > > *dev,

> > > > > >  	ixgbe_add_rar(dev, addr, 0, 0);  }

> > > > > >

> > > > > > -static int

> > > > > > -is_ixgbe_pmd(const char *driver_name)

> > > > > > +static bool

> > > > > > +is_device_supported(struct rte_eth_dev *dev, struct

> > > > > > +eth_driver

> > > > > > +*drv)

> > > > > >  {

> > > > > > -	if (!strstr(driver_name, "ixgbe"))

> > > > > > -		return -ENOTSUP;

> > > > > > +	if (strcmp(dev->driver->pci_drv.driver.name,

> > > > > > +		   drv->pci_drv.driver.name))

> > > > > > +		return FALSE;

> > > > > >

> > > > >

> > > > > It would be better to use `false' instead of `FALSE'.

> > > > I see both 'false' and 'FALSE' are defined and used. Is there any

> > > > reason that

> > > 'false' is better?

> > > >

> > >

> > > I think `true' and `false' are standard keywords defined and

> > > reserved by C. So I think it would be better to use them if the return type is

> `bool'.

> > O, there's no 'bool' in C. You have to define it. The same for 'false' and 'true'.

> >

> 

> The `bool', `true' and `false' are all standard keywords defined and reserved by

> C, although the stdbool.h is not used in ixgbe.

> 

> C adds this support by introducing a new header stdbool.h:

> 

> #ifndef __bool_true_false_are_defined

> #define __bool_true_false_are_defined   1

> 

> #ifndef __cplusplus

> 

> #define false   0

> #define true    1

> 

> #define bool    _Bool

> #if __STDC_VERSION__ < 199901L && __GNUC__ < 3

> && !defined(__INTEL_COMPILER)

> typedef int     _Bool;

> #endif

O, you're talking about C99. _Bool is a  keyword added by it. 'bool', 'true', 'false' are  not. That's why this header file have to define them.

> 

> #endif /* !__cplusplus */

> #endif /* __bool_true_false_are_defined */

> 

> Best regards,

> Tiwei Bie
  
Tiwei Bie Feb. 6, 2017, 4:57 a.m. UTC | #7
On Mon, Feb 06, 2017 at 11:45:41AM +0800, Lu, Wenzhuo wrote:
> Hi Tiwei,
> 
> > -----Original Message-----
> > From: Bie, Tiwei
> > Sent: Monday, February 6, 2017 11:08 AM
> > To: Lu, Wenzhuo
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get
> > 
> > On Mon, Feb 06, 2017 at 10:59:42AM +0800, Lu, Wenzhuo wrote:
> > > Hi Tiwei,
> > >
> > >
> > > > -----Original Message-----
> > > > From: Bie, Tiwei
> > > > Sent: Monday, February 6, 2017 10:51 AM
> > > > To: Lu, Wenzhuo
> > > > Cc: dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up
> > > > rte_eth_dev_info_get
> > > >
> > > > On Mon, Feb 06, 2017 at 10:41:28AM +0800, Lu, Wenzhuo wrote:
> > > > > Hi Tiwei,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Bie, Tiwei
> > > > > > Sent: Monday, February 6, 2017 10:31 AM
> > > > > > To: Lu, Wenzhuo
> > > > > > Cc: dev@dpdk.org
> > > > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up
> > > > > > rte_eth_dev_info_get
> > > > > >
> > > > > > On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote:
> > > > > > [...]
> > > > > > >  static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct
> > > > > > > ixgbe_dcb_config *dcb_config); -static int is_ixgbe_pmd(const
> > > > > > > char *driver_name);
> > > > > > > +static int is_device_supported(struct rte_eth_dev *dev,
> > > > > > > +struct eth_driver *drv);
> > > > > > >
> > > > > >
> > > > > > Should be:
> > > > > > static bool is_device_supported(struct rte_eth_dev *dev, struct
> > > > > > eth_driver *drv);
> > > > > O, forget to change it. Thanks.
> > > > >
> > > > > >
> > > > > > >  /* For Virtual Function support */  static int
> > > > > > > eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev); @@
> > > > > > > -4380,16 +4380,14 @@ static int
> > > > > > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev
> > > > > > *dev,
> > > > > > >  	ixgbe_add_rar(dev, addr, 0, 0);  }
> > > > > > >
> > > > > > > -static int
> > > > > > > -is_ixgbe_pmd(const char *driver_name)
> > > > > > > +static bool
> > > > > > > +is_device_supported(struct rte_eth_dev *dev, struct
> > > > > > > +eth_driver
> > > > > > > +*drv)
> > > > > > >  {
> > > > > > > -	if (!strstr(driver_name, "ixgbe"))
> > > > > > > -		return -ENOTSUP;
> > > > > > > +	if (strcmp(dev->driver->pci_drv.driver.name,
> > > > > > > +		   drv->pci_drv.driver.name))
> > > > > > > +		return FALSE;
> > > > > > >
> > > > > >
> > > > > > It would be better to use `false' instead of `FALSE'.
> > > > > I see both 'false' and 'FALSE' are defined and used. Is there any
> > > > > reason that
> > > > 'false' is better?
> > > > >
> > > >
> > > > I think `true' and `false' are standard keywords defined and
> > > > reserved by C. So I think it would be better to use them if the return type is
> > `bool'.
> > > O, there's no 'bool' in C. You have to define it. The same for 'false' and 'true'.
> > >
> > 
> > The `bool', `true' and `false' are all standard keywords defined and reserved by
> > C, although the stdbool.h is not used in ixgbe.
> > 
> > C adds this support by introducing a new header stdbool.h:
> > 
> > #ifndef __bool_true_false_are_defined
> > #define __bool_true_false_are_defined   1
> > 
> > #ifndef __cplusplus
> > 
> > #define false   0
> > #define true    1
> > 
> > #define bool    _Bool
> > #if __STDC_VERSION__ < 199901L && __GNUC__ < 3
> > && !defined(__INTEL_COMPILER)
> > typedef int     _Bool;
> > #endif
> O, you're talking about C99. _Bool is a  keyword added by it. 'bool', 'true', 'false' are  not. That's why this header file have to define them.
> 

C99 added all those as keyword, although doesn't implement
all of them as the builtin type (e.g. int). All of them are
standard keywords defined by C99. The `bool', `true' and
`false' are defined in section 7.16 of the C99 spec [1] and
implemented as macros:

7.16 Boolean type and values <stdbool.h>

1 The header <stdbool.h> defines four macros.

2 The macro
          bool
expands to _Bool.

3 The remaining three macros are suitable for use in #if preprocessing directives. They are
          true
which expands to the integer constant 1,
          false
which expands to the integer constant 0, and
          __bool_true_false_are_defined
which expands to the integer constant 1.

4 Notwithstanding the provisions of 7.1.3, a program may undefine and perhaps then redefine the macros bool, true, and false.222)

Footnotes

222) See ''future library directions'' (7.26.7).

[1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf

Best regards,
Tiwei Bie
  
Wenzhuo Lu Feb. 6, 2017, 5:17 a.m. UTC | #8
> -----Original Message-----

> From: Bie, Tiwei

> Sent: Monday, February 6, 2017 12:57 PM

> To: Lu, Wenzhuo

> Cc: dev@dpdk.org

> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get

> 

> On Mon, Feb 06, 2017 at 11:45:41AM +0800, Lu, Wenzhuo wrote:

> > Hi Tiwei,

> >

> > > -----Original Message-----

> > > From: Bie, Tiwei

> > > Sent: Monday, February 6, 2017 11:08 AM

> > > To: Lu, Wenzhuo

> > > Cc: dev@dpdk.org

> > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up

> > > rte_eth_dev_info_get

> > >

> > > On Mon, Feb 06, 2017 at 10:59:42AM +0800, Lu, Wenzhuo wrote:

> > > > Hi Tiwei,

> > > >

> > > >

> > > > > -----Original Message-----

> > > > > From: Bie, Tiwei

> > > > > Sent: Monday, February 6, 2017 10:51 AM

> > > > > To: Lu, Wenzhuo

> > > > > Cc: dev@dpdk.org

> > > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up

> > > > > rte_eth_dev_info_get

> > > > >

> > > > > On Mon, Feb 06, 2017 at 10:41:28AM +0800, Lu, Wenzhuo wrote:

> > > > > > Hi Tiwei,

> > > > > >

> > > > > > > -----Original Message-----

> > > > > > > From: Bie, Tiwei

> > > > > > > Sent: Monday, February 6, 2017 10:31 AM

> > > > > > > To: Lu, Wenzhuo

> > > > > > > Cc: dev@dpdk.org

> > > > > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up

> > > > > > > rte_eth_dev_info_get

> > > > > > >

> > > > > > > On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote:

> > > > > > > [...]

> > > > > > > >  static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct

> > > > > > > > ixgbe_dcb_config *dcb_config); -static int

> > > > > > > > is_ixgbe_pmd(const char *driver_name);

> > > > > > > > +static int is_device_supported(struct rte_eth_dev *dev,

> > > > > > > > +struct eth_driver *drv);

> > > > > > > >

> > > > > > >

> > > > > > > Should be:

> > > > > > > static bool is_device_supported(struct rte_eth_dev *dev,

> > > > > > > struct eth_driver *drv);

> > > > > > O, forget to change it. Thanks.

> > > > > >

> > > > > > >

> > > > > > > >  /* For Virtual Function support */  static int

> > > > > > > > eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev); @@

> > > > > > > > -4380,16 +4380,14 @@ static int

> > > > > > > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev

> > > > > > > *dev,

> > > > > > > >  	ixgbe_add_rar(dev, addr, 0, 0);  }

> > > > > > > >

> > > > > > > > -static int

> > > > > > > > -is_ixgbe_pmd(const char *driver_name)

> > > > > > > > +static bool

> > > > > > > > +is_device_supported(struct rte_eth_dev *dev, struct

> > > > > > > > +eth_driver

> > > > > > > > +*drv)

> > > > > > > >  {

> > > > > > > > -	if (!strstr(driver_name, "ixgbe"))

> > > > > > > > -		return -ENOTSUP;

> > > > > > > > +	if (strcmp(dev->driver->pci_drv.driver.name,

> > > > > > > > +		   drv->pci_drv.driver.name))

> > > > > > > > +		return FALSE;

> > > > > > > >

> > > > > > >

> > > > > > > It would be better to use `false' instead of `FALSE'.

> > > > > > I see both 'false' and 'FALSE' are defined and used. Is there

> > > > > > any reason that

> > > > > 'false' is better?

> > > > > >

> > > > >

> > > > > I think `true' and `false' are standard keywords defined and

> > > > > reserved by C. So I think it would be better to use them if the

> > > > > return type is

> > > `bool'.

> > > > O, there's no 'bool' in C. You have to define it. The same for 'false' and

> 'true'.

> > > >

> > >

> > > The `bool', `true' and `false' are all standard keywords defined and

> > > reserved by C, although the stdbool.h is not used in ixgbe.

> > >

> > > C adds this support by introducing a new header stdbool.h:

> > >

> > > #ifndef __bool_true_false_are_defined

> > > #define __bool_true_false_are_defined   1

> > >

> > > #ifndef __cplusplus

> > >

> > > #define false   0

> > > #define true    1

> > >

> > > #define bool    _Bool

> > > #if __STDC_VERSION__ < 199901L && __GNUC__ < 3 &&

> > > !defined(__INTEL_COMPILER)

> > > typedef int     _Bool;

> > > #endif

> > O, you're talking about C99. _Bool is a  keyword added by it. 'bool', 'true',

> 'false' are  not. That's why this header file have to define them.

> >

> 

> C99 added all those as keyword, although doesn't implement all of them as

> the builtin type (e.g. int). All of them are standard keywords defined by C99.

> The `bool', `true' and `false' are defined in section 7.16 of the C99 spec [1] and

> implemented as macros:

> 

> 7.16 Boolean type and values <stdbool.h>

> 

> 1 The header <stdbool.h> defines four macros.

> 

> 2 The macro

>           bool

> expands to _Bool.

> 

> 3 The remaining three macros are suitable for use in #if preprocessing

> directives. They are

>           true

> which expands to the integer constant 1,

>           false

> which expands to the integer constant 0, and

>           __bool_true_false_are_defined

> which expands to the integer constant 1.

> 

> 4 Notwithstanding the provisions of 7.1.3, a program may undefine and

> perhaps then redefine the macros bool, true, and false.222)

> 

> Footnotes

> 

> 222) See ''future library directions'' (7.26.7).

> 

> [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf

O, I see the divergence. It's about the term 'keyword'. I only count '6.4.1 Keywords'. 
Anyway, as both 'false'/'true' and 'FALSE'/'TRUE' are defined. I don’t know why we cannot use any of them. If 'FALSE'/'TRUE' is not preferred, better create a new patch to clean them up.

> 

> Best regards,

> Tiwei Bie
  
Tiwei Bie Feb. 6, 2017, 5:26 a.m. UTC | #9
On Mon, Feb 06, 2017 at 01:17:30PM +0800, Lu, Wenzhuo wrote:
> > -----Original Message-----
> > From: Bie, Tiwei
> > Sent: Monday, February 6, 2017 12:57 PM
> > To: Lu, Wenzhuo
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up rte_eth_dev_info_get
> > 
> > On Mon, Feb 06, 2017 at 11:45:41AM +0800, Lu, Wenzhuo wrote:
> > > Hi Tiwei,
> > >
> > > > -----Original Message-----
> > > > From: Bie, Tiwei
> > > > Sent: Monday, February 6, 2017 11:08 AM
> > > > To: Lu, Wenzhuo
> > > > Cc: dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up
> > > > rte_eth_dev_info_get
> > > >
> > > > On Mon, Feb 06, 2017 at 10:59:42AM +0800, Lu, Wenzhuo wrote:
> > > > > Hi Tiwei,
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Bie, Tiwei
> > > > > > Sent: Monday, February 6, 2017 10:51 AM
> > > > > > To: Lu, Wenzhuo
> > > > > > Cc: dev@dpdk.org
> > > > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up
> > > > > > rte_eth_dev_info_get
> > > > > >
> > > > > > On Mon, Feb 06, 2017 at 10:41:28AM +0800, Lu, Wenzhuo wrote:
> > > > > > > Hi Tiwei,
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: Bie, Tiwei
> > > > > > > > Sent: Monday, February 6, 2017 10:31 AM
> > > > > > > > To: Lu, Wenzhuo
> > > > > > > > Cc: dev@dpdk.org
> > > > > > > > Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: clean up
> > > > > > > > rte_eth_dev_info_get
> > > > > > > >
> > > > > > > > On Mon, Feb 06, 2017 at 10:09:32AM +0800, Wenzhuo Lu wrote:
> > > > > > > > [...]
> > > > > > > > >  static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct
> > > > > > > > > ixgbe_dcb_config *dcb_config); -static int
> > > > > > > > > is_ixgbe_pmd(const char *driver_name);
> > > > > > > > > +static int is_device_supported(struct rte_eth_dev *dev,
> > > > > > > > > +struct eth_driver *drv);
> > > > > > > > >
> > > > > > > >
> > > > > > > > Should be:
> > > > > > > > static bool is_device_supported(struct rte_eth_dev *dev,
> > > > > > > > struct eth_driver *drv);
> > > > > > > O, forget to change it. Thanks.
> > > > > > >
> > > > > > > >
> > > > > > > > >  /* For Virtual Function support */  static int
> > > > > > > > > eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev); @@
> > > > > > > > > -4380,16 +4380,14 @@ static int
> > > > > > > > ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev
> > > > > > > > *dev,
> > > > > > > > >  	ixgbe_add_rar(dev, addr, 0, 0);  }
> > > > > > > > >
> > > > > > > > > -static int
> > > > > > > > > -is_ixgbe_pmd(const char *driver_name)
> > > > > > > > > +static bool
> > > > > > > > > +is_device_supported(struct rte_eth_dev *dev, struct
> > > > > > > > > +eth_driver
> > > > > > > > > +*drv)
> > > > > > > > >  {
> > > > > > > > > -	if (!strstr(driver_name, "ixgbe"))
> > > > > > > > > -		return -ENOTSUP;
> > > > > > > > > +	if (strcmp(dev->driver->pci_drv.driver.name,
> > > > > > > > > +		   drv->pci_drv.driver.name))
> > > > > > > > > +		return FALSE;
> > > > > > > > >
> > > > > > > >
> > > > > > > > It would be better to use `false' instead of `FALSE'.
> > > > > > > I see both 'false' and 'FALSE' are defined and used. Is there
> > > > > > > any reason that
> > > > > > 'false' is better?
> > > > > > >
> > > > > >
> > > > > > I think `true' and `false' are standard keywords defined and
> > > > > > reserved by C. So I think it would be better to use them if the
> > > > > > return type is
> > > > `bool'.
> > > > > O, there's no 'bool' in C. You have to define it. The same for 'false' and
> > 'true'.
> > > > >
> > > >
> > > > The `bool', `true' and `false' are all standard keywords defined and
> > > > reserved by C, although the stdbool.h is not used in ixgbe.
> > > >
> > > > C adds this support by introducing a new header stdbool.h:
> > > >
> > > > #ifndef __bool_true_false_are_defined
> > > > #define __bool_true_false_are_defined   1
> > > >
> > > > #ifndef __cplusplus
> > > >
> > > > #define false   0
> > > > #define true    1
> > > >
> > > > #define bool    _Bool
> > > > #if __STDC_VERSION__ < 199901L && __GNUC__ < 3 &&
> > > > !defined(__INTEL_COMPILER)
> > > > typedef int     _Bool;
> > > > #endif
> > > O, you're talking about C99. _Bool is a  keyword added by it. 'bool', 'true',
> > 'false' are  not. That's why this header file have to define them.
> > >
> > 
> > C99 added all those as keyword, although doesn't implement all of them as
> > the builtin type (e.g. int). All of them are standard keywords defined by C99.
> > The `bool', `true' and `false' are defined in section 7.16 of the C99 spec [1] and
> > implemented as macros:
> > 
> > 7.16 Boolean type and values <stdbool.h>
> > 
> > 1 The header <stdbool.h> defines four macros.
> > 
> > 2 The macro
> >           bool
> > expands to _Bool.
> > 
> > 3 The remaining three macros are suitable for use in #if preprocessing
> > directives. They are
> >           true
> > which expands to the integer constant 1,
> >           false
> > which expands to the integer constant 0, and
> >           __bool_true_false_are_defined
> > which expands to the integer constant 1.
> > 
> > 4 Notwithstanding the provisions of 7.1.3, a program may undefine and
> > perhaps then redefine the macros bool, true, and false.222)
> > 
> > Footnotes
> > 
> > 222) See ''future library directions'' (7.26.7).
> > 
> > [1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
> O, I see the divergence. It's about the term 'keyword'. I only count '6.4.1 Keywords'. 
> Anyway, as both 'false'/'true' and 'FALSE'/'TRUE' are defined. I don’t know why we cannot use any of them. If 'FALSE'/'TRUE' is not preferred, better create a new patch to clean them up.
> 

I didn't say we cannot use FALSE/TRUE, I just suggested that false/true
would be better. :-)

I think the reason why introduce _Bool as builtin keyword and implement
others as macros is to provide applications the ability to redefine them
for compatibility issues. I think new code should follow the spec if
possible. :-)

Best regards,
Tiwei Bie
  

Patch

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 5b625a3..2f8d43a 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -253,7 +253,7 @@  static void ixgbe_add_rar(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 static void ixgbe_set_default_mac_addr(struct rte_eth_dev *dev,
 					   struct ether_addr *mac_addr);
 static void ixgbe_dcb_init(struct ixgbe_hw *hw, struct ixgbe_dcb_config *dcb_config);
-static int is_ixgbe_pmd(const char *driver_name);
+static int is_device_supported(struct rte_eth_dev *dev, struct eth_driver *drv);
 
 /* For Virtual Function support */
 static int eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev);
@@ -4380,16 +4380,14 @@  static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	ixgbe_add_rar(dev, addr, 0, 0);
 }
 
-static int
-is_ixgbe_pmd(const char *driver_name)
+static bool
+is_device_supported(struct rte_eth_dev *dev, struct eth_driver *drv)
 {
-	if (!strstr(driver_name, "ixgbe"))
-		return -ENOTSUP;
+	if (strcmp(dev->driver->pci_drv.driver.name,
+		   drv->pci_drv.driver.name))
+		return FALSE;
 
-	if (strstr(driver_name, "ixgbe_vf"))
-		return -ENOTSUP;
-
-	return 0;
+	return TRUE;
 }
 
 int
@@ -4401,17 +4399,17 @@  static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	int rar_entry;
 	uint8_t *new_mac = (uint8_t *)(mac_addr);
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	if (vf >= dev_info.max_vfs)
+	if (vf >= pci_dev->max_vfs)
 		return -EINVAL;
 
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -4902,17 +4900,17 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	struct ixgbe_hw *hw;
 	struct ixgbe_mac_info *mac;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	if (vf >= dev_info.max_vfs)
+	if (vf >= pci_dev->max_vfs)
 		return -EINVAL;
 
 	if (on > 1)
@@ -4932,17 +4930,17 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	struct ixgbe_hw *hw;
 	struct ixgbe_mac_info *mac;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	if (vf >= dev_info.max_vfs)
+	if (vf >= pci_dev->max_vfs)
 		return -EINVAL;
 
 	if (on > 1)
@@ -4961,17 +4959,17 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	struct ixgbe_hw *hw;
 	uint32_t ctrl;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	if (vf >= dev_info.max_vfs)
+	if (vf >= pci_dev->max_vfs)
 		return -EINVAL;
 
 	if (vlan_id > ETHER_MAX_VLAN_ID)
@@ -4997,14 +4995,12 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	struct ixgbe_hw *hw;
 	uint32_t ctrl;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
 	if (on > 1)
@@ -5031,14 +5027,12 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	int i;
 	int num_queues = (int)(IXGBE_QDE_IDX_MASK >> IXGBE_QDE_IDX_SHIFT);
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
 	if (on > 1)
@@ -5061,18 +5055,18 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	struct ixgbe_hw *hw;
 	uint32_t reg_value;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
 	/* only support VF's 0 to 63 */
-	if ((vf >= dev_info.max_vfs) || (vf > 63))
+	if ((vf >= pci_dev->max_vfs) || (vf > 63))
 		return -EINVAL;
 
 	if (on > 1)
@@ -5094,19 +5088,21 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 rte_pmd_ixgbe_set_vf_vlan_stripq(uint8_t port, uint16_t vf, uint8_t on)
 {
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;
+	struct ixgbe_hw *hw;
 	uint16_t queues_per_pool;
 	uint32_t q;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
+	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	if (vf >= dev_info.max_vfs)
+	if (vf >= pci_dev->max_vfs)
 		return -EINVAL;
 
 	if (on > 1)
@@ -5122,8 +5118,12 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	 * first 124 queues 0-123 will be allocated to VF's and only
 	 * the last 4 queues 123-127 will be assigned to the PF.
 	 */
-
-	queues_per_pool = dev_info.vmdq_queue_num / dev_info.max_vmdq_pools;
+	if (hw->mac.type == ixgbe_mac_82598EB)
+		queues_per_pool = (uint16_t)hw->mac.max_rx_queues /
+				  ETH_16_POOLS;
+	else
+		queues_per_pool = (uint16_t)hw->mac.max_rx_queues /
+				  ETH_64_POOLS;
 
 	for (q = 0; q < queues_per_pool; q++)
 		(*dev->dev_ops->vlan_strip_queue_set)(dev,
@@ -5136,19 +5136,19 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 {
 	int val = 0;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;
 	struct ixgbe_hw *hw;
 	uint32_t vmolr;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	if (vf >= dev_info.max_vfs)
+	if (vf >= pci_dev->max_vfs)
 		return -EINVAL;
 
 	if (on > 1)
@@ -5181,7 +5181,7 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 rte_pmd_ixgbe_set_vf_rx(uint8_t port, uint16_t vf, uint8_t on)
 {
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;
 	uint32_t reg, addr;
 	uint32_t val;
 	const uint8_t bit1 = 0x1;
@@ -5190,12 +5190,12 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	if (vf >= dev_info.max_vfs)
+	if (vf >= pci_dev->max_vfs)
 		return -EINVAL;
 
 	if (on > 1)
@@ -5231,7 +5231,7 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 rte_pmd_ixgbe_set_vf_tx(uint8_t port, uint16_t vf, uint8_t on)
 {
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;
 	uint32_t reg, addr;
 	uint32_t val;
 	const uint8_t bit1 = 0x1;
@@ -5241,12 +5241,12 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	if (vf >= dev_info.max_vfs)
+	if (vf >= pci_dev->max_vfs)
 		return -EINVAL;
 
 	if (on > 1)
@@ -5282,7 +5282,6 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 			uint64_t vf_mask, uint8_t vlan_on)
 {
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 	int ret = 0;
 	uint16_t vf_idx;
 	struct ixgbe_hw *hw;
@@ -5290,9 +5289,8 @@  static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
 	if ((vlan > ETHER_MAX_VLAN_ID) || (vf_mask == 0))
@@ -5318,7 +5316,6 @@  int rte_pmd_ixgbe_set_vf_rate_limit(uint8_t port, uint16_t vf,
 	uint16_t tx_rate, uint64_t q_msk)
 {
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 	struct ixgbe_hw *hw;
 	struct ixgbe_vf_info *vfinfo;
 	struct rte_eth_link link;
@@ -5332,13 +5329,13 @@  int rte_pmd_ixgbe_set_vf_rate_limit(uint8_t port, uint16_t vf,
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);
 	rte_eth_link_get_nowait(port, &link);
 
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	if (vf >= dev_info.max_vfs)
+	if (vf >= pci_dev->max_vfs)
 		return -EINVAL;
 
 	if (tx_rate > link.link_speed)
@@ -5347,7 +5344,6 @@  int rte_pmd_ixgbe_set_vf_rate_limit(uint8_t port, uint16_t vf,
 	if (q_msk == 0)
 		return 0;
 
-	pci_dev = IXGBE_DEV_TO_PCI(dev);
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	vfinfo = *(IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private));
 	nb_q_per_pool = RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool;
@@ -8227,16 +8223,15 @@  int ixgbe_enable_sec_tx_path_generic(struct ixgbe_hw *hw)
 {
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 	uint32_t ctrl;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
-	rte_eth_dev_info_get(port, &dev_info);
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	dev = &rte_eth_devices[port];
+
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	dev = &rte_eth_devices[port];
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	/* Stop the data paths */
@@ -8311,16 +8306,15 @@  int ixgbe_enable_sec_tx_path_generic(struct ixgbe_hw *hw)
 {
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 	uint32_t ctrl;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
-	rte_eth_dev_info_get(port, &dev_info);
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	dev = &rte_eth_devices[port];
+
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	dev = &rte_eth_devices[port];
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	/* Stop the data paths */
@@ -8376,16 +8370,15 @@  int ixgbe_enable_sec_tx_path_generic(struct ixgbe_hw *hw)
 {
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 	uint32_t ctrl;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
-	rte_eth_dev_info_get(port, &dev_info);
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	dev = &rte_eth_devices[port];
+
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	dev = &rte_eth_devices[port];
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	ctrl = mac[0] | (mac[1] << 8) | (mac[2] << 16) | (mac[3] << 24);
@@ -8402,16 +8395,15 @@  int ixgbe_enable_sec_tx_path_generic(struct ixgbe_hw *hw)
 {
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 	uint32_t ctrl;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
-	rte_eth_dev_info_get(port, &dev_info);
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	dev = &rte_eth_devices[port];
+
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	dev = &rte_eth_devices[port];
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	ctrl = mac[0] | (mac[1] << 8) | (mac[2] << 16) | (mac[3] << 24);
@@ -8430,16 +8422,15 @@  int ixgbe_enable_sec_tx_path_generic(struct ixgbe_hw *hw)
 {
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 	uint32_t ctrl, i;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
-	rte_eth_dev_info_get(port, &dev_info);
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	dev = &rte_eth_devices[port];
+
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	dev = &rte_eth_devices[port];
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	if (idx != 0 && idx != 1)
@@ -8487,16 +8478,15 @@  int ixgbe_enable_sec_tx_path_generic(struct ixgbe_hw *hw)
 {
 	struct ixgbe_hw *hw;
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
 	uint32_t ctrl, i;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
 
-	rte_eth_dev_info_get(port, &dev_info);
-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	dev = &rte_eth_devices[port];
+
+	if (!is_device_supported(dev, &rte_ixgbe_pmd))
 		return -ENOTSUP;
 
-	dev = &rte_eth_devices[port];
 	hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	if (idx != 0 && idx != 1)