[dpdk-dev,v7,2/2] net/ixgbe: add callback to user app on VF to PF mbox msg

Message ID 1475858784-5303-3-git-send-email-bernard.iremonger@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Iremonger, Bernard Oct. 7, 2016, 4:46 p.m. UTC
  call _rte_eth_dev_callback_process from ixgbe_rcv_msg_from_vf function.

The callback asks the user application if it is allowed to perform
the function.

If the cb_param.retval is RTE_PMD_IXGBE_MB_EVENT_PROCEED then continue,
if 0, do nothing and send ACK to VF
if > 1, do nothing and send NAK to VF.

Signed-off-by: Alex Zelezniak <az5157@att.com>
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 drivers/net/ixgbe/ixgbe_pf.c      | 42 +++++++++++++++++++++++++++++++++------
 drivers/net/ixgbe/rte_pmd_ixgbe.h | 20 +++++++++++++++++++
 2 files changed, 56 insertions(+), 6 deletions(-)
  

Comments

Thomas Monjalon Oct. 7, 2016, 10:44 p.m. UTC | #1
2016-10-07 17:46, Bernard Iremonger:
> +struct rte_pmd_ixgbe_mb_event_param {
> +	uint16_t vfid;     /**< Virtual Function number */
> +	uint16_t msg_type; /**< message type */
> +	uint16_t retval;   /**< return value */
> +	void *userdata;    /**< pointer to user data */

Generally speaking, the user data is a pointer passed by the application
when registering the callback and must be untouched.
It should be the name of the parameter that you are overriding with
this structure.
So this "userdata" pointer could probably be better defined.

By the way, it is far from trivial to understand how to write the callback.
I think it deserves more explanations.
  
Iremonger, Bernard Oct. 10, 2016, 2:31 p.m. UTC | #2
Hi Thomas,

<snip>

> Subject: Re: [dpdk-dev] [PATCH v7 2/2] net/ixgbe: add callback to user app
> on VF to PF mbox msg
> 
> 2016-10-07 17:46, Bernard Iremonger:
> > +struct rte_pmd_ixgbe_mb_event_param {
> > +	uint16_t vfid;     /**< Virtual Function number */
> > +	uint16_t msg_type; /**< message type */
> > +	uint16_t retval;   /**< return value */
> > +	void *userdata;    /**< pointer to user data */
> 
> Generally speaking, the user data is a pointer passed by the application when
> registering the callback and must be untouched.
> It should be the name of the parameter that you are overriding with this
> structure.
> So this "userdata" pointer could probably be better defined.
> 
> By the way, it is far from trivial to understand how to write the callback.
> I think it deserves more explanations.
> 

I will clarify the description of the struct  rte_pmd_ixgbe_mb_event_param, and rename the member userdate to msg in a v8 patchset.

Regards,

Bernard.
  

Patch

diff --git a/drivers/net/ixgbe/ixgbe_pf.c b/drivers/net/ixgbe/ixgbe_pf.c
index 56393ff..2a177b8 100644
--- a/drivers/net/ixgbe/ixgbe_pf.c
+++ b/drivers/net/ixgbe/ixgbe_pf.c
@@ -1,7 +1,7 @@ 
 /*-
  *   BSD LICENSE
  *
- *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   Copyright(c) 2010-2016 Intel Corporation. All rights reserved.
  *   All rights reserved.
  *
  *   Redistribution and use in source and binary forms, with or without
@@ -51,6 +51,7 @@ 
 
 #include "base/ixgbe_common.h"
 #include "ixgbe_ethdev.h"
+#include "rte_pmd_ixgbe.h"
 
 #define IXGBE_MAX_VFTA     (128)
 #define IXGBE_VF_MSG_SIZE_DEFAULT 1
@@ -660,6 +661,7 @@  ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_vf_info *vfinfo =
 		*IXGBE_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private);
+	struct rte_pmd_ixgbe_mb_event_param cb_param;
 
 	retval = ixgbe_read_mbx(hw, msgbuf, mbx_size, vf);
 	if (retval) {
@@ -674,27 +676,54 @@  ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)
 	/* flush the ack before we write any messages back */
 	IXGBE_WRITE_FLUSH(hw);
 
+	/**
+	 * initialise structure to send to user application
+	 * will return response from user in retval field
+	 */
+	cb_param.retval = RTE_PMD_IXGBE_MB_EVENT_PROCEED;
+	cb_param.vfid = vf;
+	cb_param.msg_type = msgbuf[0] & 0xFFFF;
+	cb_param.userdata = (void *)msgbuf;
+
 	/* perform VF reset */
 	if (msgbuf[0] == IXGBE_VF_RESET) {
 		int ret = ixgbe_vf_reset(dev, vf, msgbuf);
 
 		vfinfo[vf].clear_to_send = true;
+
+		/* notify application about VF reset */
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_VF_MBOX, &cb_param);
 		return ret;
 	}
 
+	/**
+	 * ask user application if we allowed to perform those functions
+	 * if we get cb_param.retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED
+	 * then business as usual,
+	 * if 0, do nothing and send ACK to VF
+	 * if cb_param.retval > 1, do nothing and send NAK to VF
+	 */
+	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_VF_MBOX, &cb_param);
+
+	retval = cb_param.retval;
+
 	/* check & process VF to PF mailbox message */
 	switch ((msgbuf[0] & 0xFFFF)) {
 	case IXGBE_VF_SET_MAC_ADDR:
-		retval = ixgbe_vf_set_mac_addr(dev, vf, msgbuf);
+		if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+			retval = ixgbe_vf_set_mac_addr(dev, vf, msgbuf);
 		break;
 	case IXGBE_VF_SET_MULTICAST:
-		retval = ixgbe_vf_set_multicast(dev, vf, msgbuf);
+		if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+			retval = ixgbe_vf_set_multicast(dev, vf, msgbuf);
 		break;
 	case IXGBE_VF_SET_LPE:
-		retval = ixgbe_set_vf_lpe(dev, vf, msgbuf);
+		if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+			retval = ixgbe_set_vf_lpe(dev, vf, msgbuf);
 		break;
 	case IXGBE_VF_SET_VLAN:
-		retval = ixgbe_vf_set_vlan(dev, vf, msgbuf);
+		if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+			retval = ixgbe_vf_set_vlan(dev, vf, msgbuf);
 		break;
 	case IXGBE_VF_API_NEGOTIATE:
 		retval = ixgbe_negotiate_vf_api(dev, vf, msgbuf);
@@ -704,7 +733,8 @@  ixgbe_rcv_msg_from_vf(struct rte_eth_dev *dev, uint16_t vf)
 		msg_size = IXGBE_VF_GET_QUEUE_MSG_SIZE;
 		break;
 	case IXGBE_VF_UPDATE_XCAST_MODE:
-		retval = ixgbe_set_vf_mc_promisc(dev, vf, msgbuf);
+		if (retval == RTE_PMD_IXGBE_MB_EVENT_PROCEED)
+			retval = ixgbe_set_vf_mc_promisc(dev, vf, msgbuf);
 		break;
 	default:
 		PMD_DRV_LOG(DEBUG, "Unhandled Msg %8.8x", (unsigned)msgbuf[0]);
diff --git a/drivers/net/ixgbe/rte_pmd_ixgbe.h b/drivers/net/ixgbe/rte_pmd_ixgbe.h
index 2689668..8665cea 100644
--- a/drivers/net/ixgbe/rte_pmd_ixgbe.h
+++ b/drivers/net/ixgbe/rte_pmd_ixgbe.h
@@ -179,4 +179,24 @@  int rte_pmd_ixgbe_set_vf_split_drop_en(uint8_t port, uint16_t vf, uint8_t on);
  */
 int
 rte_pmd_ixgbe_set_vf_vlan_stripq(uint8_t port, uint16_t vf, uint8_t on);
+
+/**
+ * Response sent back to ixgbe driver from user app after callback
+ */
+enum rte_pmd_ixgbe_mb_event_rsp {
+	RTE_PMD_IXGBE_MB_EVENT_NOOP_ACK,  /**< skip mbox request and ACK */
+	RTE_PMD_IXGBE_MB_EVENT_NOOP_NACK, /**< skip mbox request and NACK */
+	RTE_PMD_IXGBE_MB_EVENT_PROCEED,  /**< proceed with mbox request  */
+	RTE_PMD_IXGBE_MB_EVENT_MAX       /**< max value of this enum */
+};
+
+/**
+ * Data sent to the user application when the callback is executed.
+ */
+struct rte_pmd_ixgbe_mb_event_param {
+	uint16_t vfid;     /**< Virtual Function number */
+	uint16_t msg_type; /**< message type */
+	uint16_t retval;   /**< return value */
+	void *userdata;    /**< pointer to user data */
+};
 #endif /* _PMD_IXGBE_H_ */