diff mbox

[dpdk-dev,v6,1/2] librte_ether: modify internal callback function

Message ID 1475772490-10491-2-git-send-email-bernard.iremonger@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Iremonger, Bernard Oct. 6, 2016, 4:48 p.m. UTC 
  add cb_arg parameter to the _rte_eth_dev_callback_process function.

Adding a parameter to this function allows passing information
to the application when an eth device event occurs such as
a VF to PF message.
This allows the application to decide if a particular function
is permitted.

drivers/net: add parameter to callback process function

add parameter to call of _rte_eth_dev_callback_process function
in the following PMD's:

net/bonding
net/e1000
net/i40e
net/mlx4
net/mlx5
net/nfp
net/thunderx
net/vhost
net/virtio
net/enic

app/test: add parameter to callback process function
add parameter to call of _rte_eth_dev_callback_process function

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
Signed-off-by: Alex Zelezniak <az5157@att.com>
---
 app/test/virtual_pmd.c                 |  2 +-
 drivers/net/bonding/rte_eth_bond_pmd.c |  6 +++---
 drivers/net/e1000/em_ethdev.c          |  2 +-
 drivers/net/e1000/igb_ethdev.c         |  4 ++--
 drivers/net/enic/enic_main.c           |  2 +-
 drivers/net/i40e/i40e_ethdev.c         |  4 ++--
 drivers/net/i40e/i40e_ethdev_vf.c      |  2 +-
 drivers/net/mlx4/mlx4.c                |  4 ++--
 drivers/net/mlx5/mlx5_ethdev.c         |  4 ++--
 drivers/net/nfp/nfp_net.c              |  2 +-
 drivers/net/thunderx/nicvf_ethdev.c    |  2 +-
 drivers/net/vhost/rte_eth_vhost.c      |  6 +++---
 drivers/net/virtio/virtio_ethdev.c     |  2 +-
 lib/librte_ether/rte_ethdev.c          |  5 ++++-
 lib/librte_ether/rte_ethdev.h          | 12 +++++++++++-
 15 files changed, 36 insertions(+), 23 deletions(-)

Comments

Thomas Monjalon Oct. 7, 2016, 12:29 p.m. UTC | #1 
2016-10-06 17:48, Bernard Iremonger:
> @@ -2508,7 +2508,7 @@ rte_eth_dev_callback_unregister(uint8_t port_id,
>  
>  void
>  _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
> -	enum rte_eth_event_type event)
> +	enum rte_eth_event_type event, void *cb_arg)
>  {
>  	struct rte_eth_dev_callback *cb_lst;
>  	struct rte_eth_dev_callback dev_cb;
> @@ -2519,6 +2519,9 @@ _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
>  			continue;
>  		dev_cb = *cb_lst;
>  		cb_lst->active = 1;
> +		if (cb_arg != NULL)
> +			dev_cb.cb_arg = (void *) cb_arg;
> +
>  		rte_spinlock_unlock(&rte_eth_dev_cb_lock);
>  		dev_cb.cb_fn(dev->data->port_id, dev_cb.event,
>  						dev_cb.cb_arg);
[...]
> @@ -3047,6 +3048,11 @@ typedef void (*rte_eth_dev_cb_fn)(uint8_t port_id, \
>   * @param cb_arg
>   *  Pointer to the parameters for the registered callback.
>   *
> + *  The cb_arg must not be NULL if the application requires
> + *  data to be returned when the callback is processed.
> + *  For the RTE_ETH_EVENT_VF_MBOX data is returned to the
> + *  application.

This comment is wrong.
You should say that the user data is overwritten in the case of
RTE_ETH_EVENT_VF_MBOX. And you should point to where the meaning
of this parameter is documented (ixgbe.h) or document it here.
Iremonger, Bernard Oct. 7, 2016, 4:57 p.m. UTC | #2 
Hi Thomas,

<snip>

> Subject: Re: [dpdk-dev] [PATCH v6 1/2] librte_ether: modify internal callback
> function
> 
> 2016-10-06 17:48, Bernard Iremonger:
> > @@ -2508,7 +2508,7 @@ rte_eth_dev_callback_unregister(uint8_t
> port_id,
> >
> >  void
> >  _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
> > -	enum rte_eth_event_type event)
> > +	enum rte_eth_event_type event, void *cb_arg)
> >  {
> >  	struct rte_eth_dev_callback *cb_lst;
> >  	struct rte_eth_dev_callback dev_cb;
> > @@ -2519,6 +2519,9 @@ _rte_eth_dev_callback_process(struct
> rte_eth_dev *dev,
> >  			continue;
> >  		dev_cb = *cb_lst;
> >  		cb_lst->active = 1;
> > +		if (cb_arg != NULL)
> > +			dev_cb.cb_arg = (void *) cb_arg;
> > +
> >  		rte_spinlock_unlock(&rte_eth_dev_cb_lock);
> >  		dev_cb.cb_fn(dev->data->port_id, dev_cb.event,
> >  						dev_cb.cb_arg);
> [...]
> > @@ -3047,6 +3048,11 @@ typedef void (*rte_eth_dev_cb_fn)(uint8_t
> port_id, \
> >   * @param cb_arg
> >   *  Pointer to the parameters for the registered callback.
> >   *
> > + *  The cb_arg must not be NULL if the application requires
> > + *  data to be returned when the callback is processed.
> > + *  For the RTE_ETH_EVENT_VF_MBOX data is returned to the
> > + *  application.
> 
> This comment is wrong.
> You should say that the user data is overwritten in the case of
> RTE_ETH_EVENT_VF_MBOX. And you should point to where the meaning of
> this parameter is documented (ixgbe.h) or document it here.

This is corrected in the v7 patchset.

Regards.

Bernard.
diff mbox

Patch

diff --git a/app/test/virtual_pmd.c b/app/test/virtual_pmd.c
index 4831113..65b44c6 100644
--- a/app/test/virtual_pmd.c
+++ b/app/test/virtual_pmd.c
@@ -485,7 +485,7 @@  virtual_ethdev_simulate_link_status_interrupt(uint8_t port_id,
 
 	vrtl_eth_dev->data->dev_link.link_status = link_status;
 
-	_rte_eth_dev_callback_process(vrtl_eth_dev, RTE_ETH_EVENT_INTR_LSC);
+	_rte_eth_dev_callback_process(vrtl_eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 }
 
 int
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index fb4050c..8d510e3 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1958,7 +1958,7 @@  bond_ethdev_delayed_lsc_propagation(void *arg)
 		return;
 
 	_rte_eth_dev_callback_process((struct rte_eth_dev *)arg,
-			RTE_ETH_EVENT_INTR_LSC);
+			RTE_ETH_EVENT_INTR_LSC, NULL);
 }
 
 void
@@ -2079,7 +2079,7 @@  bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
 						(void *)bonded_eth_dev);
 			else
 				_rte_eth_dev_callback_process(bonded_eth_dev,
-						RTE_ETH_EVENT_INTR_LSC);
+						RTE_ETH_EVENT_INTR_LSC, NULL);
 
 		} else {
 			if (internals->link_down_delay_ms > 0)
@@ -2088,7 +2088,7 @@  bond_ethdev_lsc_event_callback(uint8_t port_id, enum rte_eth_event_type type,
 						(void *)bonded_eth_dev);
 			else
 				_rte_eth_dev_callback_process(bonded_eth_dev,
-						RTE_ETH_EVENT_INTR_LSC);
+						RTE_ETH_EVENT_INTR_LSC, NULL);
 		}
 	}
 }
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index f767e1c..e13e858 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -1599,7 +1599,7 @@  eth_em_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 
 	eth_em_interrupt_get_status(dev);
 	eth_em_interrupt_action(dev);
-	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 }
 
 static int
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 5a1a83e..d9ab2df 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -2683,7 +2683,7 @@  eth_igb_interrupt_action(struct rte_eth_dev *dev)
 		E1000_WRITE_REG(hw, E1000_TCTL, tctl);
 		E1000_WRITE_REG(hw, E1000_RCTL, rctl);
 		E1000_WRITE_FLUSH(hw);
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 	}
 
 	return 0;
@@ -2743,7 +2743,7 @@  void igbvf_mbx_process(struct rte_eth_dev *dev)
 
 	/* PF reset VF event */
 	if (in_msg == E1000_PF_CONTROL_MSG)
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET);
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET, NULL);
 }
 
 static int
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 15a05b4..fb72491 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -436,7 +436,7 @@  enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
 	vnic_intr_return_all_credits(&enic->intr);
 
 	enic_link_update(enic);
-	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 	enic_log_q_error(enic);
 }
 
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 697800e..d947760 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -5510,7 +5510,7 @@  i40e_dev_interrupt_delayed_handler(void *param)
 
 	/* handle the link up interrupt in an alarm callback */
 	i40e_dev_link_update(dev, 0);
-	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 
 	i40e_pf_enable_irq0(hw);
 	rte_intr_enable(&(dev->pci_dev->intr_handle));
@@ -5594,7 +5594,7 @@  i40e_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 			return;
 		else
 			_rte_eth_dev_callback_process(dev,
-				RTE_ETH_EVENT_INTR_LSC);
+				RTE_ETH_EVENT_INTR_LSC, NULL);
 	}
 
 done:
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 34eb274..bd89cd9 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1341,7 +1341,7 @@  i40evf_handle_pf_event(__rte_unused struct rte_eth_dev *dev,
 	switch (pf_msg->event) {
 	case I40E_VIRTCHNL_EVENT_RESET_IMPENDING:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_RESET_IMPENDING event\n");
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET);
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET, NULL);
 		break;
 	case I40E_VIRTCHNL_EVENT_LINK_CHANGE:
 		PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE event\n");
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index 1553b2e..3e57cca 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -5448,7 +5448,7 @@  mlx4_dev_link_status_handler(void *arg)
 	ret = priv_dev_link_status_handler(priv, dev);
 	priv_unlock(priv);
 	if (ret)
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 }
 
 /**
@@ -5471,7 +5471,7 @@  mlx4_dev_interrupt_handler(struct rte_intr_handle *intr_handle, void *cb_arg)
 	ret = priv_dev_link_status_handler(priv, dev);
 	priv_unlock(priv);
 	if (ret)
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index bf4232a..c76e754 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -1069,7 +1069,7 @@  mlx5_dev_link_status_handler(void *arg)
 	ret = priv_dev_link_status_handler(priv, dev);
 	priv_unlock(priv);
 	if (ret)
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 }
 
 /**
@@ -1092,7 +1092,7 @@  mlx5_dev_interrupt_handler(struct rte_intr_handle *intr_handle, void *cb_arg)
 	ret = priv_dev_link_status_handler(priv, dev);
 	priv_unlock(priv);
 	if (ret)
-		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 }
 
 /**
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index d526f34..a2b9056 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -1219,7 +1219,7 @@  nfp_net_dev_interrupt_delayed_handler(void *param)
 	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
 
 	nfp_net_link_update(dev, 0);
-	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
+	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 
 	nfp_net_dev_link_status_print(dev);
 
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index b758c9f..c61e171 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -108,7 +108,7 @@  nicvf_interrupt(void *arg)
 			nicvf_set_eth_link_status(nic,
 					&nic->eth_dev->data->dev_link);
 		_rte_eth_dev_callback_process(nic->eth_dev,
-				RTE_ETH_EVENT_INTR_LSC);
+				RTE_ETH_EVENT_INTR_LSC, NULL);
 	}
 
 	rte_eal_alarm_set(NICVF_INTR_POLL_INTERVAL_MS * 1000,
diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index c1d09a0..18a0c10 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -559,7 +559,7 @@  new_device(int vid)
 
 	RTE_LOG(INFO, PMD, "New connection established\n");
 
-	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC);
+	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 
 	return 0;
 }
@@ -626,7 +626,7 @@  destroy_device(int vid)
 
 	RTE_LOG(INFO, PMD, "Connection closed\n");
 
-	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC);
+	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_INTR_LSC, NULL);
 }
 
 static int
@@ -655,7 +655,7 @@  vring_state_changed(int vid, uint16_t vring, int enable)
 	RTE_LOG(INFO, PMD, "vring%u is %s\n",
 			vring, enable ? "enabled" : "disabled");
 
-	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_QUEUE_STATE);
+	_rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_QUEUE_STATE, NULL);
 
 	return 0;
 }
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index b4dfc0a..2f0065a 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1103,7 +1103,7 @@  virtio_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 	if (isr & VIRTIO_PCI_ISR_CONFIG) {
 		if (virtio_dev_link_update(dev, 0) == 0)
 			_rte_eth_dev_callback_process(dev,
-						      RTE_ETH_EVENT_INTR_LSC);
+						      RTE_ETH_EVENT_INTR_LSC, NULL);
 	}
 
 }
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index c517e88..13ba70d 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2508,7 +2508,7 @@  rte_eth_dev_callback_unregister(uint8_t port_id,
 
 void
 _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
-	enum rte_eth_event_type event)
+	enum rte_eth_event_type event, void *cb_arg)
 {
 	struct rte_eth_dev_callback *cb_lst;
 	struct rte_eth_dev_callback dev_cb;
@@ -2519,6 +2519,9 @@  _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
 			continue;
 		dev_cb = *cb_lst;
 		cb_lst->active = 1;
+		if (cb_arg != NULL)
+			dev_cb.cb_arg = (void *) cb_arg;
+
 		rte_spinlock_unlock(&rte_eth_dev_cb_lock);
 		dev_cb.cb_fn(dev->data->port_id, dev_cb.event,
 						dev_cb.cb_arg);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 7218b6f..775e899 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -3026,6 +3026,7 @@  enum rte_eth_event_type {
 				/**< queue state event (enabled/disabled) */
 	RTE_ETH_EVENT_INTR_RESET,
 			/**< reset interrupt event, sent to VF on PF reset */
+	RTE_ETH_EVENT_VF_MBOX,  /**< message from the VF received by PF */
 	RTE_ETH_EVENT_MAX       /**< max value of this enum */
 };
 
@@ -3047,6 +3048,11 @@  typedef void (*rte_eth_dev_cb_fn)(uint8_t port_id, \
  * @param cb_arg
  *  Pointer to the parameters for the registered callback.
  *
+ *  The cb_arg must not be NULL if the application requires
+ *  data to be returned when the callback is processed.
+ *  For the RTE_ETH_EVENT_VF_MBOX data is returned to the
+ *  application.
+ *
  * @return
  *  - On success, zero.
  *  - On failure, a negative value.
@@ -3085,12 +3091,16 @@  int rte_eth_dev_callback_unregister(uint8_t port_id,
  *  Pointer to struct rte_eth_dev.
  * @param event
  *  Eth device interrupt event type.
+ * @param cb_arg
+ *  Update callback parameter to pass data back to user application.
+ *  This allows the user application to decide if a particular function
+ *  is permitted or not.
  *
  * @return
  *  void
  */
 void _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
-				enum rte_eth_event_type event);
+				enum rte_eth_event_type event, void *cb_arg);
 
 /**
  * When there is no rx packet coming in Rx Queue for a long time, we can