[dpdk-dev,1/3] librte_ether: remove RTE_PROC_PRIMARY_OR_ERR_RET and RTE_PROC_PRIMARY_OR_RET

Message ID 1450873172-21932-1-git-send-email-reshma.pattan@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Pattan, Reshma Dec. 23, 2015, 12:19 p.m. UTC
  Macros RTE_PROC_PRIMARY_OR_ERR_RET and RTE_PROC_PRIMARY_OR_RET
are blocking the secondary process from using the APIs.
API access should be given to both secondary and primary.

Fix minor checkpath issues in rte_ethdev.h

Reported-by: Sean Harte <sean.harte@intel.com>
Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
 lib/librte_ether/rte_ethdev.c | 50 +------------------------------------------
 lib/librte_ether/rte_ethdev.h | 20 ++++++++---------
 2 files changed, 11 insertions(+), 59 deletions(-)
  

Comments

Michael Qiu Dec. 24, 2015, 8:01 a.m. UTC | #1
On 12/23/2015 8:19 PM, Reshma Pattan wrote:
> Macros RTE_PROC_PRIMARY_OR_ERR_RET and RTE_PROC_PRIMARY_OR_RET
> are blocking the secondary process from using the APIs.
> API access should be given to both secondary and primary.

Just as the log says, is it safe to do so?

Thanks,
Michael
> Fix minor checkpath issues in rte_ethdev.h
>
> Reported-by: Sean Harte <sean.harte@intel.com>
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
>  lib/librte_ether/rte_ethdev.c | 50 +------------------------------------------
>  lib/librte_ether/rte_ethdev.h | 20 ++++++++---------
>  2 files changed, 11 insertions(+), 59 deletions(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index ed971b4..5849102 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -711,10 +711,6 @@ rte_eth_dev_rx_queue_start(uint8_t port_id, uint16_t rx_queue_id)
>  {
>  	struct rte_eth_dev *dev;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> @@ -741,10 +737,6 @@ rte_eth_dev_rx_queue_stop(uint8_t port_id, uint16_t rx_queue_id)
>  {
>  	struct rte_eth_dev *dev;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> @@ -771,10 +763,6 @@ rte_eth_dev_tx_queue_start(uint8_t port_id, uint16_t tx_queue_id)
>  {
>  	struct rte_eth_dev *dev;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> @@ -801,10 +789,6 @@ rte_eth_dev_tx_queue_stop(uint8_t port_id, uint16_t tx_queue_id)
>  {
>  	struct rte_eth_dev *dev;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> @@ -874,10 +858,6 @@ rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>  	struct rte_eth_dev_info dev_info;
>  	int diag;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	if (nb_rx_q > RTE_MAX_QUEUES_PER_PORT) {
> @@ -1059,10 +1039,6 @@ rte_eth_dev_start(uint8_t port_id)
>  	struct rte_eth_dev *dev;
>  	int diag;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> @@ -1096,10 +1072,6 @@ rte_eth_dev_stop(uint8_t port_id)
>  {
>  	struct rte_eth_dev *dev;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_RET();
> -
>  	RTE_ETH_VALID_PORTID_OR_RET(port_id);
>  	dev = &rte_eth_devices[port_id];
>  
> @@ -1121,10 +1093,6 @@ rte_eth_dev_set_link_up(uint8_t port_id)
>  {
>  	struct rte_eth_dev *dev;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> @@ -1138,10 +1106,6 @@ rte_eth_dev_set_link_down(uint8_t port_id)
>  {
>  	struct rte_eth_dev *dev;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> @@ -1155,10 +1119,6 @@ rte_eth_dev_close(uint8_t port_id)
>  {
>  	struct rte_eth_dev *dev;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_RET();
> -
>  	RTE_ETH_VALID_PORTID_OR_RET(port_id);
>  	dev = &rte_eth_devices[port_id];
>  
> @@ -1183,10 +1143,6 @@ rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
>  	struct rte_eth_dev *dev;
>  	struct rte_eth_dev_info dev_info;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> @@ -1266,10 +1222,6 @@ rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
>  	struct rte_eth_dev *dev;
>  	struct rte_eth_dev_info dev_info;
>  
> -	/* This function is only safe when called from the primary process
> -	 * in a multi-process setup*/
> -	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
> -
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
>  	dev = &rte_eth_devices[port_id];
> @@ -2479,7 +2431,7 @@ rte_eth_dev_callback_register(uint8_t port_id,
>  	/* create a new callback. */
>  	if (user_cb == NULL)
>  		user_cb = rte_zmalloc("INTR_USER_CALLBACK",
> -		                      sizeof(struct rte_eth_dev_callback), 0);
> +					sizeof(struct rte_eth_dev_callback), 0);
>  	if (user_cb != NULL) {
>  		user_cb->cb_fn = cb_fn;
>  		user_cb->cb_arg = cb_arg;
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index bada8ad..30cb216 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -248,7 +248,7 @@ struct rte_eth_link {
>  	uint16_t link_speed;      /**< ETH_LINK_SPEED_[10, 100, 1000, 10000] */
>  	uint16_t link_duplex;     /**< ETH_LINK_[HALF_DUPLEX, FULL_DUPLEX] */
>  	uint8_t  link_status : 1; /**< 1 -> link up, 0 -> link down */
> -}__attribute__((aligned(8)));     /**< aligned for atomic64 read/write */
> +} __attribute__((aligned(8)));     /**< aligned for atomic64 read/write */
>  
>  #define ETH_LINK_SPEED_AUTONEG  0       /**< Auto-negotiate link speed. */
>  #define ETH_LINK_SPEED_10       10      /**< 10 megabits/second. */
> @@ -767,7 +767,7 @@ struct rte_eth_conf {
>  	struct rte_eth_rxmode rxmode; /**< Port RX configuration. */
>  	struct rte_eth_txmode txmode; /**< Port TX configuration. */
>  	uint32_t lpbk_mode; /**< Loopback operation mode. By default the value
> -			         is 0, meaning the loopback mode is disabled.
> +				is 0, meaning the loopback mode is disabled.
>  				 Read the datasheet of given ethernet controller
>  				 for details. The possible values of this field
>  				 are defined in implementation of each driver. */
> @@ -1591,10 +1591,10 @@ struct rte_eth_dev_data {
>  	/**< Common rx buffer size handled by all queues */
>  
>  	uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */
> -	struct ether_addr* mac_addrs;/**< Device Ethernet Link address. */
> +	struct ether_addr *mac_addrs;/**< Device Ethernet Link address. */
>  	uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR];
>  	/** bitmap array of associating Ethernet MAC addresses to pools */
> -	struct ether_addr* hash_mac_addrs;
> +	struct ether_addr *hash_mac_addrs;
>  	/** Device Ethernet MAC addresses of hash filtering. */
>  	uint8_t port_id;           /**< Device [external] port identifier. */
>  	uint8_t promiscuous   : 1, /**< RX promiscuous mode ON(1) / OFF(0). */
> @@ -2318,7 +2318,7 @@ extern int rte_eth_dev_set_mtu(uint8_t port_id, uint16_t mtu);
>   *   - (-ENOSYS) if VLAN filtering on *port_id* disabled.
>   *   - (-EINVAL) if *vlan_id* > 4095.
>   */
> -extern int rte_eth_dev_vlan_filter(uint8_t port_id, uint16_t vlan_id , int on);
> +extern int rte_eth_dev_vlan_filter(uint8_t port_id, uint16_t vlan_id, int on);
>  
>  /**
>   * Enable/Disable hardware VLAN Strip by a rx queue of an Ethernet device.
> @@ -2543,7 +2543,7 @@ rte_eth_rx_queue_count(uint8_t port_id, uint16_t queue_id)
>  	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_count, -ENOTSUP);
> -        return (*dev->dev_ops->rx_queue_count)(dev, queue_id);
> +	return (*dev->dev_ops->rx_queue_count)(dev, queue_id);
>  }
>  
>  /**
> @@ -3004,7 +3004,7 @@ int rte_eth_dev_rss_reta_query(uint8_t port,
>    *  - (-ENODEV) if *port_id* invalid.
>   *   - (-EINVAL) if bad parameter.
>   */
> -int rte_eth_dev_uc_hash_table_set(uint8_t port,struct ether_addr *addr,
> +int rte_eth_dev_uc_hash_table_set(uint8_t port, struct ether_addr *addr,
>  					uint8_t on);
>  
>   /**
> @@ -3024,7 +3024,7 @@ int rte_eth_dev_uc_hash_table_set(uint8_t port,struct ether_addr *addr,
>    *  - (-ENODEV) if *port_id* invalid.
>   *   - (-EINVAL) if bad parameter.
>   */
> -int rte_eth_dev_uc_all_hash_table_set(uint8_t port,uint8_t on);
> +int rte_eth_dev_uc_all_hash_table_set(uint8_t port, uint8_t on);
>  
>   /**
>   * Set RX L2 Filtering mode of a VF of an Ethernet device.
> @@ -3068,7 +3068,7 @@ int rte_eth_dev_set_vf_rxmode(uint8_t port, uint16_t vf, uint16_t rx_mode,
>  *   - (-EINVAL) if bad parameter.
>  */
>  int
> -rte_eth_dev_set_vf_tx(uint8_t port,uint16_t vf, uint8_t on);
> +rte_eth_dev_set_vf_tx(uint8_t port, uint16_t vf, uint8_t on);
>  
>  /**
>  * Enable or disable a VF traffic receive of an Ethernet device.
> @@ -3087,7 +3087,7 @@ rte_eth_dev_set_vf_tx(uint8_t port,uint16_t vf, uint8_t on);
>  *   - (-EINVAL) if bad parameter.
>  */
>  int
> -rte_eth_dev_set_vf_rx(uint8_t port,uint16_t vf, uint8_t on);
> +rte_eth_dev_set_vf_rx(uint8_t port, uint16_t vf, uint8_t on);
>  
>  /**
>  * Enable/Disable hardware VF VLAN filtering by an Ethernet device of
  
Pattan, Reshma Dec. 24, 2015, 5:40 p.m. UTC | #2
> -----Original Message-----
> From: Qiu, Michael
> On 12/23/2015 8:19 PM, Reshma Pattan wrote:
> > Macros RTE_PROC_PRIMARY_OR_ERR_RET and
> RTE_PROC_PRIMARY_OR_RET are
> > blocking the secondary process from using the APIs.
> > API access should be given to both secondary and primary.
> 
> Just as the log says, is it safe to do so?


Hi,

Some parts of the code still need these macros, which I am not sure yet. But as and when we identify those we have to add the macros to the needed places. 
But it is safe to remove from start of function to allow secondary process to do device configuration and queue setups for vdev. 
Please let me know if you know any of such cases where these macros should be added.

Thanks,
Reshma
  
Michael Qiu Dec. 28, 2015, 4:40 a.m. UTC | #3
On 12/25/2015 1:40 AM, Pattan, Reshma wrote:
>
>> -----Original Message-----
>> From: Qiu, Michael
>> On 12/23/2015 8:19 PM, Reshma Pattan wrote:
>>> Macros RTE_PROC_PRIMARY_OR_ERR_RET and
>> RTE_PROC_PRIMARY_OR_RET are
>>> blocking the secondary process from using the APIs.
>>> API access should be given to both secondary and primary.
>> Just as the log says, is it safe to do so?
>
> Hi,
>
> Some parts of the code still need these macros, which I am not sure yet. But as and when we identify those we have to add the macros to the needed places. 
> But it is safe to remove from start of function to allow secondary process to do device configuration and queue setups for vdev. 
> Please let me know if you know any of such cases where these macros should be added.

You you have removed almost all secondary check, and as I know, with
your patch, secondary almost has full control of a device, what's the
exact demand for secondary to have full API access?

It's a big change for DPDK I think, but patch set itself is good for me:)

Thanks,
Michael
> Thanks,
> Reshma
> 	
>
  
Pattan, Reshma Jan. 5, 2016, 4:34 p.m. UTC | #4
From: reshmapa <reshma.pattan@intel.com>

Patches 1 and 2 removes RTE_PROC_PRIMARY_OR_ERR_RET and
RTE_PROC_PRIMARY_OR_RET macro usage from rte_ether and rte_cryptodev libraries to allow API
access to secondary process.

Patch 3 allows users to configure ethdev with zero rx/tx queues, but both should not be zero.
Fix rte_eth_dev_tx_queue_config, rte_eth_dev_rx_queue_config to allocate memory for rx/tx queues
only when number of rx/tx queues are nonzero.

v3:
* Removed checkpatch fixes of lib/librte_ether/rte_ethdev.h from patch number 1.

Reshma Pattan (3):
  librte_ether: remove RTE_PROC_PRIMARY_OR_ERR_RET and
    RTE_PROC_PRIMARY_OR_RET
  librte_cryptodev: remove RTE_PROC_PRIMARY_OR_RET
  librte_ether: fix rte_eth_dev_configure

 lib/librte_cryptodev/rte_cryptodev.c |   42 ----------------
 lib/librte_ether/rte_ethdev.c        |   86 ++++++++++------------------------
 2 files changed, 25 insertions(+), 103 deletions(-)
  
Ananyev, Konstantin Jan. 6, 2016, 1:41 p.m. UTC | #5
> -----Original Message-----
> From: Pattan, Reshma
> Sent: Tuesday, January 05, 2016 4:35 PM
> To: Ananyev, Konstantin; Doherty, Declan; thomas.monjalon@6wind.com
> Cc: dev@dpdk.org; Pattan, Reshma
> Subject: [PATCH v3 0/3] fix RTE_PROC_PRIMARY_OR_ERR_RET RTE_PROC_PRIMARY_OR_RET
> 
> From: reshmapa <reshma.pattan@intel.com>
> 
> Patches 1 and 2 removes RTE_PROC_PRIMARY_OR_ERR_RET and
> RTE_PROC_PRIMARY_OR_RET macro usage from rte_ether and rte_cryptodev libraries to allow API
> access to secondary process.
> 
> Patch 3 allows users to configure ethdev with zero rx/tx queues, but both should not be zero.
> Fix rte_eth_dev_tx_queue_config, rte_eth_dev_rx_queue_config to allocate memory for rx/tx queues
> only when number of rx/tx queues are nonzero.
> 
> v3:
> * Removed checkpatch fixes of lib/librte_ether/rte_ethdev.h from patch number 1.
> 
> Reshma Pattan (3):
>   librte_ether: remove RTE_PROC_PRIMARY_OR_ERR_RET and
>     RTE_PROC_PRIMARY_OR_RET
>   librte_cryptodev: remove RTE_PROC_PRIMARY_OR_RET
>   librte_ether: fix rte_eth_dev_configure
> 
>  lib/librte_cryptodev/rte_cryptodev.c |   42 ----------------
>  lib/librte_ether/rte_ethdev.c        |   86 ++++++++++------------------------
>  2 files changed, 25 insertions(+), 103 deletions(-)
> 
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 1.7.4.1
  
Thomas Monjalon Feb. 5, 2016, 9:43 a.m. UTC | #6
Hi,

2016-01-05 16:34, Reshma Pattan:
> From: reshmapa <reshma.pattan@intel.com>
> 
> Patches 1 and 2 removes RTE_PROC_PRIMARY_OR_ERR_RET and
> RTE_PROC_PRIMARY_OR_RET macro usage from rte_ether and rte_cryptodev libraries to allow API
> access to secondary process.

I don't understand the use case.
What is the role of the primary process if queues are configured by
the secondary process?
You have not answered to Michael:
	http://dpdk.org/ml/archives/dev/2015-December/030811.html

Other question first asked by Michael
	http://dpdk.org/ml/archives/dev/2015-December/030777.html
There are some comments asserting that it is not safe for secondary.
And your answer was it is safe for vdev? And what about PCI devices?
  
Pattan, Reshma Feb. 5, 2016, 2:58 p.m. UTC | #7
Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, February 5, 2016 9:43 AM
> To: Pattan, Reshma
> Cc: Ananyev, Konstantin; Doherty, Declan; dev@dpdk.org; Qiu, Michael
> Subject: Re: [PATCH v3 0/3] fix RTE_PROC_PRIMARY_OR_ERR_RET
> RTE_PROC_PRIMARY_OR_RET
> 
> Hi,
> 
> 2016-01-05 16:34, Reshma Pattan:
> > From: reshmapa <reshma.pattan@intel.com>
> >
> > Patches 1 and 2 removes RTE_PROC_PRIMARY_OR_ERR_RET and
> > RTE_PROC_PRIMARY_OR_RET macro usage from rte_ether and rte_cryptodev
> > libraries to allow API access to secondary process.
> 
> I don't understand the use case.

These changes were added for the use case where vdev has to be configured from secondary process.
As of now,  secondary process is allowed to create vdev but not allowed to configure it.
Ex1: tcpdump feature needs these changes. As we create & configure vdev from secondary process.
Ex2: Sean Harte, initially reported this as limitation as part of his development related to containers proof-of concept. 

> What is the role of the primary process if queues are configured by the
> secondary process?

There can be use cases where  primary and secondary processes needs to configure different queues of same port or needs to configure different PCI ports.
I assume, users will be aware of PCI port & queue combinations used in primary and will not try to reconfigure them in  secondary.

> You have not answered to Michael:
> 	http://dpdk.org/ml/archives/dev/2015-December/030811.html
> 
> Other question first asked by Michael
> 	http://dpdk.org/ml/archives/dev/2015-December/030777.html
> There are some comments asserting that it is not safe for secondary.
> And your answer was it is safe for vdev? And what about PCI devices?

I assume, users will be aware of PCI port & queue combinations used in primary and will  not try to reconfigure them in  secondary.


Thanks,
Reshma
  
Thomas Monjalon Feb. 5, 2016, 3:05 p.m. UTC | #8
2016-02-05 14:58, Pattan, Reshma:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2016-01-05 16:34, Reshma Pattan:
> > > From: reshmapa <reshma.pattan@intel.com>
> > >
> > > Patches 1 and 2 removes RTE_PROC_PRIMARY_OR_ERR_RET and
> > > RTE_PROC_PRIMARY_OR_RET macro usage from rte_ether and rte_cryptodev
> > > libraries to allow API access to secondary process.
> > 
> > I don't understand the use case.
> 
> These changes were added for the use case where vdev has to be configured from secondary process.
> As of now,  secondary process is allowed to create vdev but not allowed to configure it.
> Ex1: tcpdump feature needs these changes. As we create & configure vdev from secondary process.
> Ex2: Sean Harte, initially reported this as limitation as part of his development related to containers proof-of concept. 
> 
> > What is the role of the primary process if queues are configured by the
> > secondary process?
> 
> There can be use cases where  primary and secondary processes needs to configure different queues of same port or needs to configure different PCI ports.
> I assume, users will be aware of PCI port & queue combinations used in primary and will not try to reconfigure them in  secondary.
> 
> > You have not answered to Michael:
> > 	http://dpdk.org/ml/archives/dev/2015-December/030811.html
> > 
> > Other question first asked by Michael
> > 	http://dpdk.org/ml/archives/dev/2015-December/030777.html
> > There are some comments asserting that it is not safe for secondary.
> > And your answer was it is safe for vdev? And what about PCI devices?
> 
> I assume, users will be aware of PCI port & queue combinations used in primary and will  not try to reconfigure them in  secondary.

OK, thanks, good answer.
Anyone against this idea?
  
Thomas Monjalon Feb. 24, 2016, 6:25 p.m. UTC | #9
> > From: reshmapa <reshma.pattan@intel.com>
> > 
> > Patches 1 and 2 removes RTE_PROC_PRIMARY_OR_ERR_RET and
> > RTE_PROC_PRIMARY_OR_RET macro usage from rte_ether and rte_cryptodev libraries to allow API
> > access to secondary process.
> > 
> > Patch 3 allows users to configure ethdev with zero rx/tx queues, but both should not be zero.
> > Fix rte_eth_dev_tx_queue_config, rte_eth_dev_rx_queue_config to allocate memory for rx/tx queues
> > only when number of rx/tx queues are nonzero.
> > 
> > v3:
> > * Removed checkpatch fixes of lib/librte_ether/rte_ethdev.h from patch number 1.
> > 
> > Reshma Pattan (3):
> >   librte_ether: remove RTE_PROC_PRIMARY_OR_ERR_RET and
> >     RTE_PROC_PRIMARY_OR_RET
> >   librte_cryptodev: remove RTE_PROC_PRIMARY_OR_RET
> >   librte_ether: fix rte_eth_dev_configure
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied with these titles:
- ethdev: allow full control from secondary process
- cryptodev: allow full control from secondary process
- ethdev: support unidirectional configuration
Please see how it is more informative without using the macros or function
names. The title must reflect the intent, not the details.
Thanks
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index ed971b4..5849102 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -711,10 +711,6 @@  rte_eth_dev_rx_queue_start(uint8_t port_id, uint16_t rx_queue_id)
 {
 	struct rte_eth_dev *dev;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
@@ -741,10 +737,6 @@  rte_eth_dev_rx_queue_stop(uint8_t port_id, uint16_t rx_queue_id)
 {
 	struct rte_eth_dev *dev;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
@@ -771,10 +763,6 @@  rte_eth_dev_tx_queue_start(uint8_t port_id, uint16_t tx_queue_id)
 {
 	struct rte_eth_dev *dev;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
@@ -801,10 +789,6 @@  rte_eth_dev_tx_queue_stop(uint8_t port_id, uint16_t tx_queue_id)
 {
 	struct rte_eth_dev *dev;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
@@ -874,10 +858,6 @@  rte_eth_dev_configure(uint8_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 	struct rte_eth_dev_info dev_info;
 	int diag;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	if (nb_rx_q > RTE_MAX_QUEUES_PER_PORT) {
@@ -1059,10 +1039,6 @@  rte_eth_dev_start(uint8_t port_id)
 	struct rte_eth_dev *dev;
 	int diag;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
@@ -1096,10 +1072,6 @@  rte_eth_dev_stop(uint8_t port_id)
 {
 	struct rte_eth_dev *dev;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_RET();
-
 	RTE_ETH_VALID_PORTID_OR_RET(port_id);
 	dev = &rte_eth_devices[port_id];
 
@@ -1121,10 +1093,6 @@  rte_eth_dev_set_link_up(uint8_t port_id)
 {
 	struct rte_eth_dev *dev;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
@@ -1138,10 +1106,6 @@  rte_eth_dev_set_link_down(uint8_t port_id)
 {
 	struct rte_eth_dev *dev;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
@@ -1155,10 +1119,6 @@  rte_eth_dev_close(uint8_t port_id)
 {
 	struct rte_eth_dev *dev;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_RET();
-
 	RTE_ETH_VALID_PORTID_OR_RET(port_id);
 	dev = &rte_eth_devices[port_id];
 
@@ -1183,10 +1143,6 @@  rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
 	struct rte_eth_dev *dev;
 	struct rte_eth_dev_info dev_info;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
@@ -1266,10 +1222,6 @@  rte_eth_tx_queue_setup(uint8_t port_id, uint16_t tx_queue_id,
 	struct rte_eth_dev *dev;
 	struct rte_eth_dev_info dev_info;
 
-	/* This function is only safe when called from the primary process
-	 * in a multi-process setup*/
-	RTE_PROC_PRIMARY_OR_ERR_RET(-E_RTE_SECONDARY);
-
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
 	dev = &rte_eth_devices[port_id];
@@ -2479,7 +2431,7 @@  rte_eth_dev_callback_register(uint8_t port_id,
 	/* create a new callback. */
 	if (user_cb == NULL)
 		user_cb = rte_zmalloc("INTR_USER_CALLBACK",
-		                      sizeof(struct rte_eth_dev_callback), 0);
+					sizeof(struct rte_eth_dev_callback), 0);
 	if (user_cb != NULL) {
 		user_cb->cb_fn = cb_fn;
 		user_cb->cb_arg = cb_arg;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index bada8ad..30cb216 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -248,7 +248,7 @@  struct rte_eth_link {
 	uint16_t link_speed;      /**< ETH_LINK_SPEED_[10, 100, 1000, 10000] */
 	uint16_t link_duplex;     /**< ETH_LINK_[HALF_DUPLEX, FULL_DUPLEX] */
 	uint8_t  link_status : 1; /**< 1 -> link up, 0 -> link down */
-}__attribute__((aligned(8)));     /**< aligned for atomic64 read/write */
+} __attribute__((aligned(8)));     /**< aligned for atomic64 read/write */
 
 #define ETH_LINK_SPEED_AUTONEG  0       /**< Auto-negotiate link speed. */
 #define ETH_LINK_SPEED_10       10      /**< 10 megabits/second. */
@@ -767,7 +767,7 @@  struct rte_eth_conf {
 	struct rte_eth_rxmode rxmode; /**< Port RX configuration. */
 	struct rte_eth_txmode txmode; /**< Port TX configuration. */
 	uint32_t lpbk_mode; /**< Loopback operation mode. By default the value
-			         is 0, meaning the loopback mode is disabled.
+				is 0, meaning the loopback mode is disabled.
 				 Read the datasheet of given ethernet controller
 				 for details. The possible values of this field
 				 are defined in implementation of each driver. */
@@ -1591,10 +1591,10 @@  struct rte_eth_dev_data {
 	/**< Common rx buffer size handled by all queues */
 
 	uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */
-	struct ether_addr* mac_addrs;/**< Device Ethernet Link address. */
+	struct ether_addr *mac_addrs;/**< Device Ethernet Link address. */
 	uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR];
 	/** bitmap array of associating Ethernet MAC addresses to pools */
-	struct ether_addr* hash_mac_addrs;
+	struct ether_addr *hash_mac_addrs;
 	/** Device Ethernet MAC addresses of hash filtering. */
 	uint8_t port_id;           /**< Device [external] port identifier. */
 	uint8_t promiscuous   : 1, /**< RX promiscuous mode ON(1) / OFF(0). */
@@ -2318,7 +2318,7 @@  extern int rte_eth_dev_set_mtu(uint8_t port_id, uint16_t mtu);
  *   - (-ENOSYS) if VLAN filtering on *port_id* disabled.
  *   - (-EINVAL) if *vlan_id* > 4095.
  */
-extern int rte_eth_dev_vlan_filter(uint8_t port_id, uint16_t vlan_id , int on);
+extern int rte_eth_dev_vlan_filter(uint8_t port_id, uint16_t vlan_id, int on);
 
 /**
  * Enable/Disable hardware VLAN Strip by a rx queue of an Ethernet device.
@@ -2543,7 +2543,7 @@  rte_eth_rx_queue_count(uint8_t port_id, uint16_t queue_id)
 	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_count, -ENOTSUP);
-        return (*dev->dev_ops->rx_queue_count)(dev, queue_id);
+	return (*dev->dev_ops->rx_queue_count)(dev, queue_id);
 }
 
 /**
@@ -3004,7 +3004,7 @@  int rte_eth_dev_rss_reta_query(uint8_t port,
   *  - (-ENODEV) if *port_id* invalid.
  *   - (-EINVAL) if bad parameter.
  */
-int rte_eth_dev_uc_hash_table_set(uint8_t port,struct ether_addr *addr,
+int rte_eth_dev_uc_hash_table_set(uint8_t port, struct ether_addr *addr,
 					uint8_t on);
 
  /**
@@ -3024,7 +3024,7 @@  int rte_eth_dev_uc_hash_table_set(uint8_t port,struct ether_addr *addr,
   *  - (-ENODEV) if *port_id* invalid.
  *   - (-EINVAL) if bad parameter.
  */
-int rte_eth_dev_uc_all_hash_table_set(uint8_t port,uint8_t on);
+int rte_eth_dev_uc_all_hash_table_set(uint8_t port, uint8_t on);
 
  /**
  * Set RX L2 Filtering mode of a VF of an Ethernet device.
@@ -3068,7 +3068,7 @@  int rte_eth_dev_set_vf_rxmode(uint8_t port, uint16_t vf, uint16_t rx_mode,
 *   - (-EINVAL) if bad parameter.
 */
 int
-rte_eth_dev_set_vf_tx(uint8_t port,uint16_t vf, uint8_t on);
+rte_eth_dev_set_vf_tx(uint8_t port, uint16_t vf, uint8_t on);
 
 /**
 * Enable or disable a VF traffic receive of an Ethernet device.
@@ -3087,7 +3087,7 @@  rte_eth_dev_set_vf_tx(uint8_t port,uint16_t vf, uint8_t on);
 *   - (-EINVAL) if bad parameter.
 */
 int
-rte_eth_dev_set_vf_rx(uint8_t port,uint16_t vf, uint8_t on);
+rte_eth_dev_set_vf_rx(uint8_t port, uint16_t vf, uint8_t on);
 
 /**
 * Enable/Disable hardware VF VLAN filtering by an Ethernet device of