DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/3] enable mirror functionality in i40e driver
@ 2015-05-13  8:47 Jingjing Wu
  2015-05-13  8:47 ` [dpdk-dev] [PATCH 1/3] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
                   ` (3 more replies)
  0 siblings, 4 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-05-13  8:47 UTC (permalink / raw)
  To: dev

This patch set enables mirror functionality in i40e driver, and
redefines structure and macros used to configure mirror.

Jingjing Wu (3):
  ethdev: rename rte_eth_vmdq_mirror_conf
  ethdev: redefine the mirror type
  i40e: enable mirror functionality in i40e driver

 app/test-pmd/cmdline.c              |  60 ++++---
 lib/librte_ether/rte_ethdev.c       |  28 ++-
 lib/librte_ether/rte_ethdev.h       |  30 ++--
 lib/librte_pmd_i40e/i40e_ethdev.c   | 334 ++++++++++++++++++++++++++++++++++++
 lib/librte_pmd_i40e/i40e_ethdev.h   |  23 +++
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c |  72 +++++---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.h |   4 +-
 7 files changed, 464 insertions(+), 87 deletions(-)

-- 
1.9.3

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

* [dpdk-dev] [PATCH 1/3] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-05-13  8:47 [dpdk-dev] [PATCH 0/3] enable mirror functionality in i40e driver Jingjing Wu
@ 2015-05-13  8:47 ` Jingjing Wu
  2015-06-02  2:50   ` Liu, Jijiang
  2015-05-13  8:47 ` [dpdk-dev] [PATCH 2/3] ethdev: redefine the mirror type Jingjing Wu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 30+ messages in thread
From: Jingjing Wu @ 2015-05-13  8:47 UTC (permalink / raw)
  To: dev

This patch renames rte_eth_vmdq_mirror_conf to rte_eth_mirror_conf
and move the maximum number check from ethdev level to driver

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 app/test-pmd/cmdline.c              | 18 +++++++++---------
 lib/librte_ether/rte_ethdev.c       | 18 ++----------------
 lib/librte_ether/rte_ethdev.h       | 19 +++++++++++--------
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 11 +++++++----
 lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  4 +++-
 5 files changed, 32 insertions(+), 38 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f01db2a..9c1fb43 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -6604,9 +6604,9 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 {
 	int ret,nb_item,i;
 	struct cmd_set_mirror_mask_result *res = parsed_result;
-	struct rte_eth_vmdq_mirror_conf mr_conf;
+	struct rte_eth_mirror_conf mr_conf;
 
-	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
+	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
 
 	unsigned int vlan_list[ETH_VMDQ_MAX_VLAN_FILTERS];
 
@@ -6622,7 +6622,7 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 		if (nb_item <= 0)
 			return;
 
-		for(i=0; i < nb_item; i++) {
+		for (i = 0; i < nb_item; i++) {
 			if (vlan_list[i] > ETHER_MAX_VLAN_ID) {
 				printf("Invalid vlan_id: must be < 4096\n");
 				return;
@@ -6634,10 +6634,10 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 	}
 
 	if(!strcmp(res->on, "on"))
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
 	if(ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
@@ -6711,9 +6711,9 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 {
 	int ret;
 	struct cmd_set_mirror_link_result *res = parsed_result;
-	struct rte_eth_vmdq_mirror_conf mr_conf;
+	struct rte_eth_mirror_conf mr_conf;
 
-	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
+	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
 	if(!strcmp(res->what, "uplink-mirror")) {
 		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
 	}else if(!strcmp(res->what, "downlink-mirror"))
@@ -6722,10 +6722,10 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 	mr_conf.dst_pool = res->dstpool_id;
 
 	if(!strcmp(res->on, "on"))
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
 
 	/* check the return value and print it if is < 0 */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 024fe8b..43c7295 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3034,7 +3034,7 @@ int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t tx_rate,
 
 int
 rte_eth_mirror_rule_set(uint8_t port_id,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id, uint8_t on)
 {
 	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
@@ -3051,7 +3051,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 
 	if (mirror_conf->dst_pool >= ETH_64_POOLS) {
 		PMD_DEBUG_TRACE("Invalid dst pool, pool id must"
-			"be 0-%d\n",ETH_64_POOLS - 1);
+			"be 0-%d\n", ETH_64_POOLS - 1);
 		return -EINVAL;
 	}
 
@@ -3062,13 +3062,6 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -EINVAL;
 	}
 
-	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
-	{
-		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-%d\n",
-			ETH_VMDQ_NUM_MIRROR_RULE - 1);
-		return -EINVAL;
-	}
-
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
 
@@ -3085,13 +3078,6 @@ rte_eth_mirror_rule_reset(uint8_t port_id, uint8_t rule_id)
 		return -ENODEV;
 	}
 
-	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
-	{
-		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-%d\n",
-			ETH_VMDQ_NUM_MIRROR_RULE-1);
-		return -EINVAL;
-	}
-
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_reset, -ENOTSUP);
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 16dbe00..562b190 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -467,8 +467,10 @@ struct rte_eth_rss_conf {
 #define ETH_VMDQ_ACCEPT_BROADCAST   0x0008 /**< accept broadcast packets. */
 #define ETH_VMDQ_ACCEPT_MULTICAST   0x0010 /**< multicast promiscuous. */
 
-/* Definitions used for VMDQ mirror rules setting */
-#define ETH_VMDQ_NUM_MIRROR_RULE     4 /**< Maximum nb. of mirror rules. . */
+/* Definitions used for mirror rules setting */
+#define ETH_MAX_NUM_MIRROR_RULE        16  /**< Maximum nb. of mirror rules. */
+#define ETH_MIRROR_MAX_NUM_VLANS       64
+/** Maximum nb. of vlan per mirror rule*/
 
 #define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring. */
 #define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring. */
@@ -480,18 +482,19 @@ struct rte_eth_rss_conf {
  */
 struct rte_eth_vlan_mirror {
 	uint64_t vlan_mask; /**< mask for valid VLAN ID. */
-	uint16_t vlan_id[ETH_VMDQ_MAX_VLAN_FILTERS];
+	uint16_t vlan_id[ETH_MIRROR_MAX_NUM_VLANS];
 	/** VLAN ID list for vlan mirror. */
 };
 
 /**
  * A structure used to configure traffic mirror of an Ethernet port.
  */
-struct rte_eth_vmdq_mirror_conf {
+struct rte_eth_mirror_conf {
 	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to set */
-	uint8_t dst_pool; /**< Destination pool for this mirror rule. */
+	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
 	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
-	struct rte_eth_vlan_mirror vlan; /**< VLAN ID setting for VLAN mirroring */
+	struct rte_eth_vlan_mirror vlan;
+	/** VLAN ID setting for VLAN mirroring */
 };
 
 /**
@@ -1211,7 +1214,7 @@ typedef int (*eth_set_vf_rate_limit_t)(struct rte_eth_dev *dev,
 /**< @internal Set VF TX rate */
 
 typedef int (*eth_mirror_rule_set_t)(struct rte_eth_dev *dev,
-				  struct rte_eth_vmdq_mirror_conf *mirror_conf,
+				  struct rte_eth_mirror_conf *mirror_conf,
 				  uint8_t rule_id,
 				  uint8_t on);
 /**< @internal Add a traffic mirroring rule on an Ethernet device */
@@ -3168,7 +3171,7 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
  *   - (-EINVAL) if the mr_conf information is not correct.
  */
 int rte_eth_mirror_rule_set(uint8_t port_id,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id,
 			uint8_t on);
 
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 5f9a1cf..2e19314 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -209,7 +209,7 @@ static int ixgbe_set_pool_tx(struct rte_eth_dev *dev,uint16_t pool,uint8_t on);
 static int ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 		uint64_t pool_mask,uint8_t vlan_on);
 static int ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
-		struct rte_eth_vmdq_mirror_conf *mirror_conf,
+		struct rte_eth_mirror_conf *mirror_conf,
 		uint8_t rule_id, uint8_t on);
 static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev,
 		uint8_t	rule_id);
@@ -3388,7 +3388,7 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 
 static int
 ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id, uint8_t on)
 {
 	uint32_t mr_ctl,vlvf;
@@ -3412,7 +3412,10 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	if (ixgbe_vmdq_mode_check(hw) < 0)
-		return (-ENOTSUP);
+		return -ENOTSUP;
+
+	if (rule_id >= IXGBE_MAX_NUM_MIRROR_RULE)
+		return -EINVAL;
 
 	/* Check if vlan mask is valid */
 	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) && (on)) {
@@ -3526,7 +3529,7 @@ ixgbe_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t rule_id)
 		return (-ENOTSUP);
 
 	memset(&mr_info->mr_conf[rule_id], 0,
-		sizeof(struct rte_eth_vmdq_mirror_conf));
+		sizeof(struct rte_eth_mirror_conf));
 
 	/* clear PFVMCTL register */
 	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl);
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
index 419ea5d..6422000 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
@@ -177,8 +177,10 @@ struct ixgbe_uta_info {
 	uint32_t uta_shadow[IXGBE_MAX_UTA];
 };
 
+#define IXGBE_MAX_NUM_MIRROR_RULE 4  /* Maximum nb. of mirror rules. */
+
 struct ixgbe_mirror_info {
-	struct rte_eth_vmdq_mirror_conf mr_conf[ETH_VMDQ_NUM_MIRROR_RULE];
+	struct rte_eth_mirror_conf mr_conf[IXGBE_MAX_NUM_MIRROR_RULE];
 	/**< store PF mirror rules configuration*/
 };
 
-- 
1.9.3

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

* [dpdk-dev] [PATCH 2/3] ethdev: redefine the mirror type
  2015-05-13  8:47 [dpdk-dev] [PATCH 0/3] enable mirror functionality in i40e driver Jingjing Wu
  2015-05-13  8:47 ` [dpdk-dev] [PATCH 1/3] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
@ 2015-05-13  8:47 ` Jingjing Wu
  2015-05-13  8:47 ` [dpdk-dev] [PATCH 3/3] i40e: enable mirror functionality in i40e driver Jingjing Wu
  2015-06-02  7:55 ` [dpdk-dev] [PATCH v2 0/4] " Jingjing Wu
  3 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-05-13  8:47 UTC (permalink / raw)
  To: dev

This path renames the mirror type in rte_eth_mirror_conf and macros,
and reworks the mirror set in ixgbe dirvers by using new definition.
It also fixes some coding style.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 app/test-pmd/cmdline.c              | 42 ++++++++++++++-----------
 lib/librte_ether/rte_ethdev.c       | 14 +++++++--
 lib/librte_ether/rte_ethdev.h       | 11 ++++---
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 63 ++++++++++++++++++++++---------------
 4 files changed, 78 insertions(+), 52 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 9c1fb43..b863e0b 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -412,7 +412,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Set rate limit for queues in VF of a port\n\n"
 
 			"set port (port_id) mirror-rule (rule_id)"
-			"(pool-mirror|vlan-mirror)\n"
+			"(pool-mirror-up|pool-mirror-down|vlan-mirror)"
 			" (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)\n"
 			"   Set pool or vlan type mirror rule on a port.\n"
 			"   e.g., 'set port 0 mirror-rule 0 vlan-mirror 0,1"
@@ -6583,7 +6583,8 @@ cmdline_parse_token_num_t cmd_mirror_mask_ruleid =
 				rule_id, UINT8);
 cmdline_parse_token_string_t cmd_mirror_mask_what =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_mirror_mask_result,
-				what, "pool-mirror#vlan-mirror");
+				what, "pool-mirror-up#pool-mirror-down"
+				      "#vlan-mirror");
 cmdline_parse_token_string_t cmd_mirror_mask_value =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_mirror_mask_result,
 				value, NULL);
@@ -6612,13 +6613,16 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 
 	mr_conf.dst_pool = res->dstpool_id;
 
-	if (!strcmp(res->what, "pool-mirror")) {
-		mr_conf.pool_mask = strtoull(res->value,NULL,16);
-		mr_conf.rule_type_mask = ETH_VMDQ_POOL_MIRROR;
-	} else if(!strcmp(res->what, "vlan-mirror")) {
-		mr_conf.rule_type_mask = ETH_VMDQ_VLAN_MIRROR;
-		nb_item = parse_item_list(res->value, "core",
-					ETH_VMDQ_MAX_VLAN_FILTERS,vlan_list,1);
+	if (!strcmp(res->what, "pool-mirror-up")) {
+		mr_conf.pool_mask = strtoull(res->value, NULL, 16);
+		mr_conf.rule_type = ETH_MIRROR_VIRTUAL_POOL_UP;
+	} else if (!strcmp(res->what, "pool-mirror-down")) {
+		mr_conf.pool_mask = strtoull(res->value, NULL, 16);
+		mr_conf.rule_type = ETH_MIRROR_VIRTUAL_POOL_DOWN;
+	} else if (!strcmp(res->what, "vlan-mirror")) {
+		mr_conf.rule_type = ETH_MIRROR_VLAN;
+		nb_item = parse_item_list(res->value, "vlan",
+				ETH_MIRROR_MAX_NUM_VLANS, vlan_list, 1);
 		if (nb_item <= 0)
 			return;
 
@@ -6633,21 +6637,21 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 		}
 	}
 
-	if(!strcmp(res->on, "on"))
+	if (!strcmp(res->on, "on"))
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
-	if(ret < 0)
+	if (ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
 }
 
 cmdline_parse_inst_t cmd_set_mirror_mask = {
 		.f = cmd_set_mirror_mask_parsed,
 		.data = NULL,
-		.help_str = "set port X mirror-rule Y pool-mirror|vlan-mirror "
-				"pool_mask|vlan_id[,vlan_id]* dst-pool Z on|off",
+		.help_str = "set port X mirror-rule Y pool-mirror-up|pool-mirror-down|vlan-mirror"
+			    "pool_mask|vlan_id[,vlan_id]* dst-pool Z on|off",
 		.tokens = {
 			(void *)&cmd_mirror_mask_set,
 			(void *)&cmd_mirror_mask_port,
@@ -6714,14 +6718,14 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 	struct rte_eth_mirror_conf mr_conf;
 
 	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
-	if(!strcmp(res->what, "uplink-mirror")) {
-		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
-	}else if(!strcmp(res->what, "downlink-mirror"))
-		mr_conf.rule_type_mask = ETH_VMDQ_DOWNLIN_MIRROR;
+	if (!strcmp(res->what, "uplink-mirror")) {
+		mr_conf.rule_type = ETH_MIRROR_UPLINK_PORT;
+	} else if (!strcmp(res->what, "downlink-mirror"))
+		mr_conf.rule_type = ETH_MIRROR_DOWNLINK_PORT;
 
 	mr_conf.dst_pool = res->dstpool_id;
 
-	if(!strcmp(res->on, "on"))
+	if (!strcmp(res->on, "on"))
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
@@ -6729,7 +6733,7 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 						res->rule_id, 0);
 
 	/* check the return value and print it if is < 0 */
-	if(ret < 0)
+	if (ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
 
 }
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 43c7295..dc3b47e 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3044,7 +3044,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -ENODEV;
 	}
 
-	if (mirror_conf->rule_type_mask == 0) {
+	if (mirror_conf->rule_type == 0) {
 		PMD_DEBUG_TRACE("mirror rule type can not be 0.\n");
 		return -EINVAL;
 	}
@@ -3055,12 +3055,20 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -EINVAL;
 	}
 
-	if ((mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) &&
-		(mirror_conf->pool_mask == 0)) {
+	if ((mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP ||
+	     mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_DOWN) &&
+	    (mirror_conf->pool_mask == 0)) {
 		PMD_DEBUG_TRACE("Invalid mirror pool, pool mask can not"
 				"be 0.\n");
 		return -EINVAL;
 	}
+	/* Check if vlan mask is valid */
+	if (mirror_conf->rule_type == ETH_MIRROR_VLAN &&
+	    mirror_conf->vlan.vlan_mask == 0) {
+		PMD_DEBUG_TRACE("Invalid vlan mask, vlan mask can not"
+				"be 0.\n");
+		return -EINVAL;
+	}
 
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 562b190..92bc24e 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -472,10 +472,11 @@ struct rte_eth_rss_conf {
 #define ETH_MIRROR_MAX_NUM_VLANS       64
 /** Maximum nb. of vlan per mirror rule*/
 
-#define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring. */
-#define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring. */
-#define ETH_VMDQ_DOWNLIN_MIRROR 0x0004 /**< Downlink Port Mirroring. */
-#define ETH_VMDQ_VLAN_MIRROR    0x0008 /**< VLAN Mirroring. */
+#define ETH_MIRROR_VIRTUAL_POOL_UP     1  /**< Virtual Pool uplink Mirroring. */
+#define ETH_MIRROR_VIRTUAL_POOL_DOWN   2  /**< Virtual Pool downlink Mirroring. */
+#define ETH_MIRROR_UPLINK_PORT         3  /**< Uplink Port Mirroring. */
+#define ETH_MIRROR_DOWNLINK_PORT       4  /**< Downlink Port Mirroring. */
+#define ETH_MIRROR_VLAN                5  /**< VLAN Mirroring. */
 
 /**
  * A structure used to configure VLAN traffic mirror of an Ethernet port.
@@ -490,7 +491,7 @@ struct rte_eth_vlan_mirror {
  * A structure used to configure traffic mirror of an Ethernet port.
  */
 struct rte_eth_mirror_conf {
-	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to set */
+	uint8_t rule_type; /**< Mirroring rule type  we want to set */
 	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
 	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
 	struct rte_eth_vlan_mirror vlan;
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 2e19314..52ffc6a 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -3386,6 +3386,11 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 	return ret;
 }
 
+#define IXGBE_MRCTL_VPME  0x01 /* Virtual Pool Mirroring. */
+#define IXGBE_MRCTL_UPME  0x02 /* Uplink Port Mirroring. */
+#define IXGBE_MRCTL_DPME  0x04 /* Downlink Port Mirroring. */
+#define IXGBE_MRCTL_VLME  0x08 /* VLAN Mirroring. */
+
 static int
 ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
@@ -3410,6 +3415,7 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			(IXGBE_DEV_PRIVATE_TO_PFDATA(dev->data->dev_private));
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint8_t mirror_type;
 
 	if (ixgbe_vmdq_mode_check(hw) < 0)
 		return -ENOTSUP;
@@ -3417,28 +3423,24 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 	if (rule_id >= IXGBE_MAX_NUM_MIRROR_RULE)
 		return -EINVAL;
 
-	/* Check if vlan mask is valid */
-	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) && (on)) {
-		if (mirror_conf->vlan.vlan_mask == 0)
-			return (-EINVAL);
-	}
-
-	/* Check if vlan id is valid and find conresponding VLAN ID index in VLVF */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) {
+	switch (mirror_conf->rule_type) {
+	case ETH_MIRROR_VLAN:
+		mirror_type = IXGBE_MRCTL_VLME;
+		/* Check if vlan id is valid and find conresponding VLAN ID index in VLVF */
 		for (i = 0;i < IXGBE_VLVF_ENTRIES; i++) {
 			if (mirror_conf->vlan.vlan_mask & (1ULL << i)) {
 				/* search vlan id related pool vlan filter index */
 				reg_index = ixgbe_find_vlvf_slot(hw,
 						mirror_conf->vlan.vlan_id[i]);
 				if(reg_index < 0)
-					return (-EINVAL);
+					return -EINVAL;
 				vlvf = IXGBE_READ_REG(hw, IXGBE_VLVF(reg_index));
 				if ((vlvf & IXGBE_VLVF_VIEN) &&
-					((vlvf & IXGBE_VLVF_VLANID_MASK)
-						== mirror_conf->vlan.vlan_id[i]))
+				    ((vlvf & IXGBE_VLVF_VLANID_MASK) ==
+				      mirror_conf->vlan.vlan_id[i]))
 					vlan_mask |= (1ULL << reg_index);
 				else
-					return (-EINVAL);
+					return -EINVAL;
 			}
 		}
 
@@ -3460,13 +3462,13 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			for(i = 0 ;i < ETH_VMDQ_MAX_VLAN_FILTERS; i++)
 				mr_info->mr_conf[rule_id].vlan.vlan_id[i] = 0;
 		}
-	}
-
-	/*
-	 * if enable pool mirror, write related pool mask register,if disable
-	 * pool mirror, clear PFMRVM register
-	 */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) {
+		break;
+	case ETH_MIRROR_VIRTUAL_POOL_UP:
+		mirror_type = IXGBE_MRCTL_VPME;
+		/*
+		 * if enable pool mirror, write related pool mask register,if disable
+		 * pool mirror, clear PFMRVM register
+		 */
 		if (on) {
 			mp_lsb = mirror_conf->pool_mask & 0xFFFFFFFF;
 			mp_msb = mirror_conf->pool_mask >> pool_mask_offset;
@@ -3478,32 +3480,43 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			mp_msb = 0;
 			mr_info->mr_conf[rule_id].pool_mask = 0;
 		}
+		break;
+	case ETH_MIRROR_UPLINK_PORT:
+		mirror_type = IXGBE_MRCTL_UPME;
+		break;
+	case ETH_MIRROR_DOWNLINK_PORT:
+		mirror_type = IXGBE_MRCTL_DPME;
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "unsupported mirror type %d.",
+			mirror_conf->rule_type);
+		return -EINVAL;
 	}
 
 	/* read  mirror control register and recalculate it */
 	mr_ctl = IXGBE_READ_REG(hw,IXGBE_MRCTL(rule_id));
 
 	if (on) {
-		mr_ctl |= mirror_conf->rule_type_mask;
+		mr_ctl |= mirror_type;
 		mr_ctl &= mirror_rule_mask;
 		mr_ctl |= mirror_conf->dst_pool << dst_pool_offset;
 	} else
-		mr_ctl &= ~(mirror_conf->rule_type_mask & mirror_rule_mask);
+		mr_ctl &= ~(mirror_conf->rule_type & mirror_rule_mask);
 
-	mr_info->mr_conf[rule_id].rule_type_mask = (uint8_t)(mr_ctl & mirror_rule_mask);
+	mr_info->mr_conf[rule_id].rule_type = mirror_conf->rule_type;
 	mr_info->mr_conf[rule_id].dst_pool = mirror_conf->dst_pool;
 
 	/* write mirrror control  register */
 	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl);
 
-        /* write pool mirrror control  register */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) {
+	/* write pool mirrror control  register */
+	if (mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP) {
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVM(rule_id), mp_lsb);
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVM(rule_id + rule_mr_offset),
 				mp_msb);
 	}
 	/* write VLAN mirrror control  register */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) {
+	if (mirror_conf->rule_type == ETH_MIRROR_VLAN) {
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVLAN(rule_id), mv_lsb);
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVLAN(rule_id + rule_mr_offset),
 				mv_msb);
-- 
1.9.3

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

* [dpdk-dev] [PATCH 3/3] i40e: enable mirror functionality in i40e driver
  2015-05-13  8:47 [dpdk-dev] [PATCH 0/3] enable mirror functionality in i40e driver Jingjing Wu
  2015-05-13  8:47 ` [dpdk-dev] [PATCH 1/3] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
  2015-05-13  8:47 ` [dpdk-dev] [PATCH 2/3] ethdev: redefine the mirror type Jingjing Wu
@ 2015-05-13  8:47 ` Jingjing Wu
  2015-06-02  7:55 ` [dpdk-dev] [PATCH v2 0/4] " Jingjing Wu
  3 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-05-13  8:47 UTC (permalink / raw)
  To: dev

enable mirror functionality in i40e driver
.mirror_rule_set
.mirror_rule_reset

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 lib/librte_pmd_i40e/i40e_ethdev.c | 334 ++++++++++++++++++++++++++++++++++++++
 lib/librte_pmd_i40e/i40e_ethdev.h |  23 +++
 2 files changed, 357 insertions(+)

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 43762f2..981e7ac 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -211,6 +211,10 @@ static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 				void *arg);
 static void i40e_configure_registers(struct i40e_hw *hw);
 static void i40e_hw_init(struct i40e_hw *hw);
+static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
+			struct rte_eth_mirror_conf *mirror_conf,
+			uint8_t sw_id, uint8_t on);
+static int i40e_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t sw_id);
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
 #define RTE_PCI_DEV_ID_DECL_I40E(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
@@ -262,6 +266,8 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.udp_tunnel_add               = i40e_dev_udp_tunnel_add,
 	.udp_tunnel_del               = i40e_dev_udp_tunnel_del,
 	.filter_ctrl                  = i40e_dev_filter_ctrl,
+	.mirror_rule_set              = i40e_mirror_rule_set,
+	.mirror_rule_reset            = i40e_mirror_rule_reset,
 };
 
 static struct eth_driver rte_i40e_pmd = {
@@ -563,6 +569,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 	/* enable uio intr after callback register */
 	rte_intr_enable(&(pci_dev->intr_handle));
 
+	/* initialize mirror rule list */
+	TAILQ_INIT(&pf->mirror_list);
+
 	return 0;
 
 err_mac_alloc:
@@ -925,6 +934,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_vsi *main_vsi = pf->main_vsi;
+	struct i40e_mirror_rule *p_mirror;
 	int i;
 
 	/* Disable all queues */
@@ -949,6 +959,13 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	/* Set link down */
 	i40e_dev_set_link_down(dev);
 
+	/* Remove all mirror rules */
+	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
+		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
+		rte_free(p_mirror);
+	}
+	pf->nb_mirror_rule = 0;
+
 }
 
 static void
@@ -5714,3 +5731,320 @@ i40e_configure_registers(struct i40e_hw *hw)
 			"0x%"PRIx32, reg_table[i].val, reg_table[i].addr);
 	}
 }
+
+/**
+ * i40e_aq_add_mirror_rule
+ * @hw: pointer to the hardware structure
+ * @seid: VEB seid to add mirror rule to
+ * @dst_id: destination vsi seid
+ * @entries: Buffer which contains the entities to be mirrored
+ * @count: number of entities contained in the buffer
+ * @rule_id:the rule_id of the rule to be added
+ *
+ * Add a mirror rule for a given veb.
+ *
+ **/
+static enum
+i40e_status_code i40e_aq_add_mirror_rule(struct i40e_hw *hw,
+			uint16_t seid, uint16_t dst_id,
+			uint16_t rule_type, uint16_t *entries,
+			uint16_t count, uint16_t *rule_id)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_add_delete_mirror_rule *cmd =
+		(struct i40e_aqc_add_delete_mirror_rule *)&desc.params.raw;
+	struct i40e_aqc_add_delete_mirror_rule_completion *resp =
+		(struct i40e_aqc_add_delete_mirror_rule_completion *)
+		&desc.params.raw;
+	uint16_t buff_len;
+	enum i40e_status_code status;
+
+	i40e_fill_default_direct_cmd_desc(&desc,
+					  i40e_aqc_opc_add_mirror_rule);
+
+	buff_len = sizeof(uint16_t) * count;
+	desc.datalen = rte_cpu_to_le_16(buff_len);
+	if (buff_len > 0)
+		desc.flags |= rte_cpu_to_le_16(
+			(uint16_t)(I40E_AQ_FLAG_BUF | I40E_AQ_FLAG_RD));
+	cmd->rule_type = rte_cpu_to_le_16(rule_type <<
+				I40E_AQC_MIRROR_RULE_TYPE_SHIFT);
+	cmd->num_entries = rte_cpu_to_le_16(count);
+	cmd->seid = rte_cpu_to_le_16(seid);
+	cmd->destination = rte_cpu_to_le_16(dst_id);
+
+	status = i40e_asq_send_command(hw, &desc, entries, buff_len, NULL);
+	PMD_DRV_LOG(INFO, "i40e_aq_add_mirror_rule, aq_status %d,"
+			 "rule_id = %u"
+			 " mirror_rules_used = %u, mirror_rules_free = %u,",
+			 hw->aq.asq_last_status, resp->rule_id,
+			 resp->mirror_rules_used, resp->mirror_rules_free);
+	*rule_id = rte_le_to_cpu_16(resp->rule_id);
+
+	return status;
+}
+
+/**
+ * i40e_aq_del_mirror_rule
+ * @hw: pointer to the hardware structure
+ * @seid: VEB seid to add mirror rule to
+ * @entries: Buffer which contains the entities to be mirrored
+ * @count: number of entities contained in the buffer
+ * @rule_id:the rule_id of the rule to be delete
+ *
+ * Delete a mirror rule for a given veb.
+ *
+ **/
+static enum
+i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw *hw,
+		uint16_t seid, uint16_t rule_type, uint16_t *entries,
+		uint16_t count, uint16_t rule_id)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_add_delete_mirror_rule *cmd =
+		(struct i40e_aqc_add_delete_mirror_rule *)&desc.params.raw;
+	uint16_t buff_len = 0;
+	enum i40e_status_code status;
+	void *buff = NULL;
+
+	i40e_fill_default_direct_cmd_desc(&desc,
+					  i40e_aqc_opc_delete_mirror_rule);
+
+	if (rule_type == I40E_AQC_MIRROR_RULE_TYPE_VLAN) {
+		desc.flags |= rte_cpu_to_le_16((uint16_t)(I40E_AQ_FLAG_BUF |
+							  I40E_AQ_FLAG_RD));
+		cmd->num_entries = count;
+		buff_len = sizeof(uint16_t) * count;
+		desc.datalen = rte_cpu_to_le_16(buff_len);
+		buff = (void *)entries;
+	} else
+		/* rule id is filled in destination field for deleting mirror rule */
+		cmd->destination = rte_cpu_to_le_16(rule_id);
+
+	cmd->rule_type = rte_cpu_to_le_16(rule_type <<
+				I40E_AQC_MIRROR_RULE_TYPE_SHIFT);
+	cmd->seid = rte_cpu_to_le_16(seid);
+
+	status = i40e_asq_send_command(hw, &desc, buff, buff_len, NULL);
+
+	return status;
+}
+
+/**
+ * i40e_mirror_rule_set
+ * @dev: pointer to the hardware structure
+ * @mirror_conf: mirror rule info
+ * @sw_id: mirror rule's sw_id
+ * @on: enable/disable
+ *
+ * set a mirror rule.
+ *
+ **/
+static int
+i40e_mirror_rule_set(struct rte_eth_dev *dev,
+			struct rte_eth_mirror_conf *mirror_conf,
+			uint8_t sw_id, uint8_t on)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_mirror_rule *it, *mirr_rule = NULL;
+	struct i40e_mirror_rule *parent = NULL;
+	uint16_t seid, dst_seid, rule_id;
+	uint16_t i, j = 0;
+	int ret;
+
+	PMD_DRV_LOG(DEBUG, "i40e_mirror_rule_set: sw_id = %d.", sw_id);
+
+	if (pf->main_vsi->veb == NULL || pf->vfs == NULL) {
+		PMD_DRV_LOG(ERR, "mirror rule can not be configured"
+			" without veb or vfs.");
+		return -ENOSYS;
+	}
+	if (pf->nb_mirror_rule > I40E_MAX_NUM_MIRROR_RULE) {
+		PMD_DRV_LOG(ERR, "mirror table is full.");
+		return -ENOSYS;
+	}
+	if (mirror_conf->dst_pool > pf->vf_num) {
+		PMD_DRV_LOG(ERR, "invalid destination pool %u.",
+				 mirror_conf->dst_pool);
+		return -EINVAL;
+	}
+
+	seid = pf->main_vsi->veb->seid;
+
+	TAILQ_FOREACH(it, &pf->mirror_list, rules) {
+		if (sw_id <= it->index) {
+			mirr_rule = it;
+			break;
+		}
+		parent = it;
+	}
+	if (mirr_rule && sw_id == mirr_rule->index) {
+		if (on) {
+			PMD_DRV_LOG(ERR, "mirror rule exists.");
+			return -EEXIST;
+		} else {
+			ret = i40e_aq_del_mirror_rule(hw, seid,
+					mirr_rule->rule_type,
+					mirr_rule->entries,
+					mirr_rule->num_entries, mirr_rule->id);
+			if (ret < 0) {
+				PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
+						   " ret = %d, aq_err = %d.",
+						   ret, hw->aq.asq_last_status);
+				return -ENOSYS;
+			}
+			TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
+			rte_free(mirr_rule);
+			pf->nb_mirror_rule--;
+			return 0;
+		}
+	} else if (!on) {
+		PMD_DRV_LOG(ERR, "mirror rule doesn't exist.");
+		return -ENOENT;
+	}
+
+	mirr_rule = rte_zmalloc("i40e_mirror_rule",
+				sizeof(struct i40e_mirror_rule) , 0);
+	if (!mirr_rule) {
+		PMD_DRV_LOG(ERR, "failed to allocate memory");
+		return I40E_ERR_NO_MEMORY;
+	}
+	switch (mirror_conf->rule_type) {
+	case ETH_MIRROR_VLAN:
+		for (i = 0, j = 0; i < ETH_MIRROR_MAX_NUM_VLANS; i++) {
+			if (mirror_conf->vlan.vlan_mask & (1ULL << i)) {
+				mirr_rule->entries[j] =
+					mirror_conf->vlan.vlan_id[i];
+				j++;
+			}
+		}
+		if (j == 0) {
+			PMD_DRV_LOG(ERR, "vlan is not specified.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_VLAN;
+		break;
+	case ETH_MIRROR_VIRTUAL_POOL_UP:
+	case ETH_MIRROR_VIRTUAL_POOL_DOWN:
+		/* check if the specified pool bit is out of range */
+		if (mirror_conf->pool_mask > (uint64_t)(1ULL << (pf->vf_num + 1))) {
+			PMD_DRV_LOG(ERR, "pool mask is out of range.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		for (i = 0, j = 0; i < pf->vf_num; i++) {
+			if (mirror_conf->pool_mask & (1ULL << i)) {
+				mirr_rule->entries[j] = pf->vfs[i].vsi->seid;
+				j++;
+			}
+		}
+		if (mirror_conf->pool_mask & (1ULL << pf->vf_num)) {
+			/* add pf vsi to entries */
+			mirr_rule->entries[j] = pf->main_vsi_seid;
+			j++;
+		}
+		if (j == 0) {
+			PMD_DRV_LOG(ERR, "pool is not specified.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		/* egress and ingress in aq commands means from switch but not port */
+		mirr_rule->rule_type =
+			(mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP) ?
+			I40E_AQC_MIRROR_RULE_TYPE_VPORT_EGRESS :
+			I40E_AQC_MIRROR_RULE_TYPE_VPORT_INGRESS;
+		break;
+	case ETH_MIRROR_UPLINK_PORT:
+		/* egress and ingress in aq commands means from switch but not port*/
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_ALL_EGRESS;
+		break;
+	case ETH_MIRROR_DOWNLINK_PORT:
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_ALL_INGRESS;
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "unsupported mirror type %d.",
+			mirror_conf->rule_type);
+		rte_free(mirr_rule);
+		return -EINVAL;
+	}
+
+	/* If the dst_pool is equal to vf_num, consider it as PF */
+	if (mirror_conf->dst_pool == pf->vf_num)
+		dst_seid = pf->main_vsi_seid;
+	else
+		dst_seid = pf->vfs[mirror_conf->dst_pool].vsi->seid;
+
+	ret = i40e_aq_add_mirror_rule(hw, seid, dst_seid,
+				      mirr_rule->rule_type, mirr_rule->entries,
+				      j, &rule_id);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "failed to add mirror rule:"
+				   " ret = %d, aq_err = %d.",
+				   ret, hw->aq.asq_last_status);
+		rte_free(mirr_rule);
+		return -ENOSYS;
+	}
+
+	mirr_rule->index = sw_id;
+	mirr_rule->num_entries = j;
+	mirr_rule->id = rule_id;
+	mirr_rule->dst_vsi_seid = dst_seid;
+
+	if (parent)
+		TAILQ_INSERT_AFTER(&pf->mirror_list, parent, mirr_rule, rules);
+	else
+		TAILQ_INSERT_HEAD(&pf->mirror_list, mirr_rule, rules);
+
+	pf->nb_mirror_rule++;
+	return 0;
+}
+
+/**
+ * i40e_mirror_rule_reset
+ * @dev: pointer to the device
+ * @sw_id: mirror rule's sw_id
+ *
+ * reset a mirror rule.
+ *
+ **/
+static int
+i40e_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t sw_id)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_mirror_rule *it, *mirr_rule = NULL;
+	uint16_t seid;
+	int ret;
+
+	PMD_DRV_LOG(DEBUG, "i40e_mirror_rule_reset: sw_id = %d.", sw_id);
+
+	seid = pf->main_vsi->veb->seid;
+
+	TAILQ_FOREACH(it, &pf->mirror_list, rules) {
+		if (sw_id == it->index) {
+			mirr_rule = it;
+			break;
+		}
+	}
+	if (mirr_rule) {
+		ret = i40e_aq_del_mirror_rule(hw, seid,
+				mirr_rule->rule_type,
+				mirr_rule->entries,
+				mirr_rule->num_entries, mirr_rule->id);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
+					   " status = %d, aq_err = %d.",
+					   ret, hw->aq.asq_last_status);
+			return -ENOSYS;
+		}
+		TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
+		rte_free(mirr_rule);
+		pf->nb_mirror_rule--;
+	} else {
+		PMD_DRV_LOG(ERR, "mirror rule doesn't exist.");
+		return -ENOENT;
+	}
+	return 0;
+}
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.h b/lib/librte_pmd_i40e/i40e_ethdev.h
index b9bed5a..ffd70d3 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.h
+++ b/lib/librte_pmd_i40e/i40e_ethdev.h
@@ -323,6 +323,27 @@ struct i40e_fdir_info {
 	struct i40e_fdir_flex_mask flex_mask[I40E_FILTER_PCTYPE_MAX];
 };
 
+#define I40E_MIRROR_MAX_ENTRIES_PER_RULE   64
+#define I40E_MAX_NUM_MIRROR_RULE           32
+/*
+ * Mirror rule structure
+ */
+struct i40e_mirror_rule {
+	TAILQ_ENTRY(i40e_mirror_rule) rules;
+	uint8_t rule_type;
+	uint16_t index;          /* the sw index of mirror rule */
+	uint16_t id;             /* the rule id assigned by firmware */
+	uint16_t dst_vsi_seid;   /* destination vsi for this mirror rule. */
+	uint16_t num_entries;
+	/* the info stores depend on the rule type.
+	    If type is I40E_MIRROR_TYPE_VLAN, vlan ids are stored here.
+	    If type is I40E_MIRROR_TYPE_VPORT_*, vsi's seid are stored.
+	 */
+	uint16_t entries[I40E_MIRROR_MAX_ENTRIES_PER_RULE];
+};
+
+TAILQ_HEAD(i40e_mirror_rule_list, i40e_mirror_rule);
+
 /*
  * Structure to store private data specific for PF instance.
  */
@@ -362,6 +383,8 @@ struct i40e_pf {
 	struct i40e_vmdq_info *vmdq;
 
 	struct i40e_fdir_info fdir; /* flow director info */
+	struct i40e_mirror_rule_list mirror_list;
+	uint16_t nb_mirror_rule;   /* The number of mirror rules */
 };
 
 enum pending_msg {
-- 
1.9.3

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

* Re: [dpdk-dev] [PATCH 1/3] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-05-13  8:47 ` [dpdk-dev] [PATCH 1/3] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
@ 2015-06-02  2:50   ` Liu, Jijiang
  0 siblings, 0 replies; 30+ messages in thread
From: Liu, Jijiang @ 2015-06-02  2:50 UTC (permalink / raw)
  To: Wu, Jingjing; +Cc: dev



> -----Original Message-----
> From: Wu, Jingjing
> Sent: Wednesday, May 13, 2015 4:47 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Jiajia, SunX; Zhang, Helin; Liu, Jijiang
> Subject: [PATCH 1/3] ethdev: rename rte_eth_vmdq_mirror_conf
> 
> This patch renames rte_eth_vmdq_mirror_conf to rte_eth_mirror_conf and
> move the maximum number check from ethdev level to driver
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  app/test-pmd/cmdline.c              | 18 +++++++++---------
>  lib/librte_ether/rte_ethdev.c       | 18 ++----------------
>  lib/librte_ether/rte_ethdev.h       | 19 +++++++++++--------
>  lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 11 +++++++----
> lib/librte_pmd_ixgbe/ixgbe_ethdev.h |  4 +++-
>  5 files changed, 32 insertions(+), 38 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> f01db2a..9c1fb43 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -6604,9 +6604,9 @@ cmd_set_mirror_mask_parsed(void
> *parsed_result,  {
>  	int ret,nb_item,i;
>  	struct cmd_set_mirror_mask_result *res = parsed_result;
> -	struct rte_eth_vmdq_mirror_conf mr_conf;
> +	struct rte_eth_mirror_conf mr_conf;
> 
> -	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
> +	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
> 
>  	unsigned int vlan_list[ETH_VMDQ_MAX_VLAN_FILTERS];
> 
> @@ -6622,7 +6622,7 @@ cmd_set_mirror_mask_parsed(void
> *parsed_result,
>  		if (nb_item <= 0)
>  			return;
> 
> -		for(i=0; i < nb_item; i++) {
> +		for (i = 0; i < nb_item; i++) {
>  			if (vlan_list[i] > ETHER_MAX_VLAN_ID) {
>  				printf("Invalid vlan_id: must be < 4096\n");
>  				return;
> @@ -6634,10 +6634,10 @@ cmd_set_mirror_mask_parsed(void
> *parsed_result,
>  	}
> 
>  	if(!strcmp(res->on, "on"))
> -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
>  						res->rule_id, 1);
>  	else
> -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
>  						res->rule_id, 0);
>  	if(ret < 0)
>  		printf("mirror rule add error: (%s)\n", strerror(-ret)); @@ -
> 6711,9 +6711,9 @@ cmd_set_mirror_link_parsed(void *parsed_result,  {
>  	int ret;
>  	struct cmd_set_mirror_link_result *res = parsed_result;
> -	struct rte_eth_vmdq_mirror_conf mr_conf;
> +	struct rte_eth_mirror_conf mr_conf;
> 
> -	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
> +	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
>  	if(!strcmp(res->what, "uplink-mirror")) {
>  		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
>  	}else if(!strcmp(res->what, "downlink-mirror")) @@ -6722,10
> +6722,10 @@ cmd_set_mirror_link_parsed(void *parsed_result,
>  	mr_conf.dst_pool = res->dstpool_id;
> 
>  	if(!strcmp(res->on, "on"))
> -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
>  						res->rule_id, 1);
>  	else
> -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
>  						res->rule_id, 0);
> 
>  	/* check the return value and print it if is < 0 */ diff --git
> a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index
> 024fe8b..43c7295 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -3034,7 +3034,7 @@ int rte_eth_set_vf_rate_limit(uint8_t port_id,
> uint16_t vf, uint16_t tx_rate,
> 
>  int
>  rte_eth_mirror_rule_set(uint8_t port_id,
> -			struct rte_eth_vmdq_mirror_conf *mirror_conf,
> +			struct rte_eth_mirror_conf *mirror_conf,
>  			uint8_t rule_id, uint8_t on)
>  {
>  	struct rte_eth_dev *dev = &rte_eth_devices[port_id]; @@ -3051,7
> +3051,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
> 
>  	if (mirror_conf->dst_pool >= ETH_64_POOLS) {
>  		PMD_DEBUG_TRACE("Invalid dst pool, pool id must"
> -			"be 0-%d\n",ETH_64_POOLS - 1);
> +			"be 0-%d\n", ETH_64_POOLS - 1);
>  		return -EINVAL;
>  	}
> 
> @@ -3062,13 +3062,6 @@ rte_eth_mirror_rule_set(uint8_t port_id,
>  		return -EINVAL;
>  	}
> 
> -	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
> -	{
> -		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-
> %d\n",
> -			ETH_VMDQ_NUM_MIRROR_RULE - 1);
> -		return -EINVAL;
> -	}
> -
>  	dev = &rte_eth_devices[port_id];
>  	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -
> ENOTSUP);
> 
> @@ -3085,13 +3078,6 @@ rte_eth_mirror_rule_reset(uint8_t port_id,
> uint8_t rule_id)
>  		return -ENODEV;
>  	}
> 
> -	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
> -	{
> -		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-
> %d\n",
> -			ETH_VMDQ_NUM_MIRROR_RULE-1);
> -		return -EINVAL;
> -	}
> -
>  	dev = &rte_eth_devices[port_id];
>  	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_reset, -
> ENOTSUP);
> 
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 16dbe00..562b190 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -467,8 +467,10 @@ struct rte_eth_rss_conf {
>  #define ETH_VMDQ_ACCEPT_BROADCAST   0x0008 /**< accept broadcast
> packets. */
>  #define ETH_VMDQ_ACCEPT_MULTICAST   0x0010 /**< multicast
> promiscuous. */
> 
> -/* Definitions used for VMDQ mirror rules setting */
> -#define ETH_VMDQ_NUM_MIRROR_RULE     4 /**< Maximum nb. of mirror
> rules. . */
> +/* Definitions used for mirror rules setting */
> +#define ETH_MAX_NUM_MIRROR_RULE        16  /**< Maximum nb. of
> mirror rules. */
> +#define ETH_MIRROR_MAX_NUM_VLANS       64
> +/** Maximum nb. of vlan per mirror rule*/
> 
>  #define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring.
> */
>  #define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring.
> */ @@ -480,18 +482,19 @@ struct rte_eth_rss_conf {
>   */
>  struct rte_eth_vlan_mirror {
>  	uint64_t vlan_mask; /**< mask for valid VLAN ID. */
> -	uint16_t vlan_id[ETH_VMDQ_MAX_VLAN_FILTERS];
> +	uint16_t vlan_id[ETH_MIRROR_MAX_NUM_VLANS];
>  	/** VLAN ID list for vlan mirror. */

The comment doesn't follow coding rule, which should be /**< VLAN ID list for vlan mirror. */
>  };
> 
>  /**
>   * A structure used to configure traffic mirror of an Ethernet port.
>   */
> -struct rte_eth_vmdq_mirror_conf {
> +struct rte_eth_mirror_conf {
>  	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to
> set */
> -	uint8_t dst_pool; /**< Destination pool for this mirror rule. */
> +	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
>  	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
> -	struct rte_eth_vlan_mirror vlan; /**< VLAN ID setting for VLAN
> mirroring */
> +	struct rte_eth_vlan_mirror vlan;
> +	/** VLAN ID setting for VLAN mirroring */
The comment doesn't follow coding rule, which should be /**< VLAN ID setting for VLAN mirroring*/,

Could you please check other places?
>  };
> 
>  /**
> @@ -1211,7 +1214,7 @@ typedef int (*eth_set_vf_rate_limit_t)(struct
> rte_eth_dev *dev,  /**< @internal Set VF TX rate */
> 
>  typedef int (*eth_mirror_rule_set_t)(struct rte_eth_dev *dev,
> -				  struct rte_eth_vmdq_mirror_conf
> *mirror_conf,
> +				  struct rte_eth_mirror_conf *mirror_conf,
>  				  uint8_t rule_id,
>  				  uint8_t on);
>  /**< @internal Add a traffic mirroring rule on an Ethernet device */ @@ -
> 3168,7 +3171,7 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t
> vlan_id,
>   *   - (-EINVAL) if the mr_conf information is not correct.
>   */
>  int rte_eth_mirror_rule_set(uint8_t port_id,
> -			struct rte_eth_vmdq_mirror_conf *mirror_conf,
> +			struct rte_eth_mirror_conf *mirror_conf,
>  			uint8_t rule_id,
>  			uint8_t on);
> 
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index 5f9a1cf..2e19314 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -209,7 +209,7 @@ static int ixgbe_set_pool_tx(struct rte_eth_dev
> *dev,uint16_t pool,uint8_t on);  static int ixgbe_set_pool_vlan_filter(struct
> rte_eth_dev *dev, uint16_t vlan,
>  		uint64_t pool_mask,uint8_t vlan_on);
>  static int ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
> -		struct rte_eth_vmdq_mirror_conf *mirror_conf,
> +		struct rte_eth_mirror_conf *mirror_conf,
>  		uint8_t rule_id, uint8_t on);
>  static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev,
>  		uint8_t	rule_id);
> @@ -3388,7 +3388,7 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev
> *dev, uint16_t vlan,
> 
>  static int
>  ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
> -			struct rte_eth_vmdq_mirror_conf *mirror_conf,
> +			struct rte_eth_mirror_conf *mirror_conf,
>  			uint8_t rule_id, uint8_t on)
>  {
>  	uint32_t mr_ctl,vlvf;
> @@ -3412,7 +3412,10 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
>  		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> 
>  	if (ixgbe_vmdq_mode_check(hw) < 0)
> -		return (-ENOTSUP);
> +		return -ENOTSUP;
> +
> +	if (rule_id >= IXGBE_MAX_NUM_MIRROR_RULE)
> +		return -EINVAL;
> 
>  	/* Check if vlan mask is valid */
>  	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) &&
> (on)) { @@ -3526,7 +3529,7 @@ ixgbe_mirror_rule_reset(struct rte_eth_dev
> *dev, uint8_t rule_id)
>  		return (-ENOTSUP);
> 
>  	memset(&mr_info->mr_conf[rule_id], 0,
> -		sizeof(struct rte_eth_vmdq_mirror_conf));
> +		sizeof(struct rte_eth_mirror_conf));
> 
>  	/* clear PFVMCTL register */
>  	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl); diff --git
> a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
> b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
> index 419ea5d..6422000 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.h
> @@ -177,8 +177,10 @@ struct ixgbe_uta_info {
>  	uint32_t uta_shadow[IXGBE_MAX_UTA];
>  };
> 
> +#define IXGBE_MAX_NUM_MIRROR_RULE 4  /* Maximum nb. of mirror
> rules. */
> +
>  struct ixgbe_mirror_info {
> -	struct rte_eth_vmdq_mirror_conf
> mr_conf[ETH_VMDQ_NUM_MIRROR_RULE];
> +	struct rte_eth_mirror_conf
> mr_conf[IXGBE_MAX_NUM_MIRROR_RULE];
>  	/**< store PF mirror rules configuration*/  };
> 
> --
> 1.9.3

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

* [dpdk-dev] [PATCH v2 0/4] enable mirror functionality in i40e driver
  2015-05-13  8:47 [dpdk-dev] [PATCH 0/3] enable mirror functionality in i40e driver Jingjing Wu
                   ` (2 preceding siblings ...)
  2015-05-13  8:47 ` [dpdk-dev] [PATCH 3/3] i40e: enable mirror functionality in i40e driver Jingjing Wu
@ 2015-06-02  7:55 ` Jingjing Wu
  2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
                     ` (4 more replies)
  3 siblings, 5 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-02  7:55 UTC (permalink / raw)
  To: dev

This patch set enables mirror functionality in i40e driver, and redefines structure and macros used to configure mirror.

v2 changes:
 - correct comments style
 - add doc change

Jingjing Wu (4):
  ethdev: rename rte_eth_vmdq_mirror_conf
  ethdev: redefine the mirror type
  i40e: enable mirror functionality in i40e driver
  doc: modify the command about mirror in testpmd guide

 app/test-pmd/cmdline.c                      |  62 +++---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
 drivers/net/i40e/i40e_ethdev.c              | 334 ++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_ethdev.h              |  23 ++
 drivers/net/ixgbe/ixgbe_ethdev.c            |  72 +++---
 drivers/net/ixgbe/ixgbe_ethdev.h            |   4 +-
 lib/librte_ether/rte_ethdev.c               |  28 +--
 lib/librte_ether/rte_ethdev.h               |  32 +--
 8 files changed, 472 insertions(+), 91 deletions(-)

-- 
1.9.3

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

* [dpdk-dev] [PATCH v2 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-06-02  7:55 ` [dpdk-dev] [PATCH v2 0/4] " Jingjing Wu
@ 2015-06-02  7:55   ` Jingjing Wu
  2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 2/4] ethdev: redefine the mirror type Jingjing Wu
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-02  7:55 UTC (permalink / raw)
  To: dev

rename rte_eth_vmdq_mirror_conf to rte_eth_mirror_conf and move
the maximum rule id check from ethdev level to driver

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 app/test-pmd/cmdline.c           | 22 +++++++++++-----------
 drivers/net/ixgbe/ixgbe_ethdev.c | 11 +++++++----
 drivers/net/ixgbe/ixgbe_ethdev.h |  4 +++-
 lib/librte_ether/rte_ethdev.c    | 18 ++----------------
 lib/librte_ether/rte_ethdev.h    | 21 ++++++++++++---------
 5 files changed, 35 insertions(+), 41 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f01db2a..a084b73 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -6604,11 +6604,11 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 {
 	int ret,nb_item,i;
 	struct cmd_set_mirror_mask_result *res = parsed_result;
-	struct rte_eth_vmdq_mirror_conf mr_conf;
+	struct rte_eth_mirror_conf mr_conf;
 
-	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
+	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
 
-	unsigned int vlan_list[ETH_VMDQ_MAX_VLAN_FILTERS];
+	unsigned int vlan_list[ETH_MIRROR_MAX_NUM_VLANS];
 
 	mr_conf.dst_pool = res->dstpool_id;
 
@@ -6618,11 +6618,11 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 	} else if(!strcmp(res->what, "vlan-mirror")) {
 		mr_conf.rule_type_mask = ETH_VMDQ_VLAN_MIRROR;
 		nb_item = parse_item_list(res->value, "core",
-					ETH_VMDQ_MAX_VLAN_FILTERS,vlan_list,1);
+					ETH_MIRROR_MAX_NUM_VLANS, vlan_list, 1);
 		if (nb_item <= 0)
 			return;
 
-		for(i=0; i < nb_item; i++) {
+		for (i = 0; i < nb_item; i++) {
 			if (vlan_list[i] > ETHER_MAX_VLAN_ID) {
 				printf("Invalid vlan_id: must be < 4096\n");
 				return;
@@ -6634,10 +6634,10 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 	}
 
 	if(!strcmp(res->on, "on"))
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
 	if(ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
@@ -6711,9 +6711,9 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 {
 	int ret;
 	struct cmd_set_mirror_link_result *res = parsed_result;
-	struct rte_eth_vmdq_mirror_conf mr_conf;
+	struct rte_eth_mirror_conf mr_conf;
 
-	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
+	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
 	if(!strcmp(res->what, "uplink-mirror")) {
 		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
 	}else if(!strcmp(res->what, "downlink-mirror"))
@@ -6722,10 +6722,10 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 	mr_conf.dst_pool = res->dstpool_id;
 
 	if(!strcmp(res->on, "on"))
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
 
 	/* check the return value and print it if is < 0 */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0d9f9b2..5b6af92 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -209,7 +209,7 @@ static int ixgbe_set_pool_tx(struct rte_eth_dev *dev,uint16_t pool,uint8_t on);
 static int ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 		uint64_t pool_mask,uint8_t vlan_on);
 static int ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
-		struct rte_eth_vmdq_mirror_conf *mirror_conf,
+		struct rte_eth_mirror_conf *mirror_conf,
 		uint8_t rule_id, uint8_t on);
 static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev,
 		uint8_t	rule_id);
@@ -3388,7 +3388,7 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 
 static int
 ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id, uint8_t on)
 {
 	uint32_t mr_ctl,vlvf;
@@ -3412,7 +3412,10 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	if (ixgbe_vmdq_mode_check(hw) < 0)
-		return (-ENOTSUP);
+		return -ENOTSUP;
+
+	if (rule_id >= IXGBE_MAX_NUM_MIRROR_RULE)
+		return -EINVAL;
 
 	/* Check if vlan mask is valid */
 	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) && (on)) {
@@ -3526,7 +3529,7 @@ ixgbe_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t rule_id)
 		return (-ENOTSUP);
 
 	memset(&mr_info->mr_conf[rule_id], 0,
-		sizeof(struct rte_eth_vmdq_mirror_conf));
+		sizeof(struct rte_eth_mirror_conf));
 
 	/* clear PFVMCTL register */
 	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl);
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 19237b8..f193ce6 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -177,8 +177,10 @@ struct ixgbe_uta_info {
 	uint32_t uta_shadow[IXGBE_MAX_UTA];
 };
 
+#define IXGBE_MAX_NUM_MIRROR_RULE 4  /* Maximum nb. of mirror rules. */
+
 struct ixgbe_mirror_info {
-	struct rte_eth_vmdq_mirror_conf mr_conf[ETH_VMDQ_NUM_MIRROR_RULE];
+	struct rte_eth_mirror_conf mr_conf[IXGBE_MAX_NUM_MIRROR_RULE];
 	/**< store PF mirror rules configuration*/
 };
 
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 024fe8b..43c7295 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3034,7 +3034,7 @@ int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t tx_rate,
 
 int
 rte_eth_mirror_rule_set(uint8_t port_id,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id, uint8_t on)
 {
 	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
@@ -3051,7 +3051,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 
 	if (mirror_conf->dst_pool >= ETH_64_POOLS) {
 		PMD_DEBUG_TRACE("Invalid dst pool, pool id must"
-			"be 0-%d\n",ETH_64_POOLS - 1);
+			"be 0-%d\n", ETH_64_POOLS - 1);
 		return -EINVAL;
 	}
 
@@ -3062,13 +3062,6 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -EINVAL;
 	}
 
-	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
-	{
-		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-%d\n",
-			ETH_VMDQ_NUM_MIRROR_RULE - 1);
-		return -EINVAL;
-	}
-
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
 
@@ -3085,13 +3078,6 @@ rte_eth_mirror_rule_reset(uint8_t port_id, uint8_t rule_id)
 		return -ENODEV;
 	}
 
-	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
-	{
-		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-%d\n",
-			ETH_VMDQ_NUM_MIRROR_RULE-1);
-		return -EINVAL;
-	}
-
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_reset, -ENOTSUP);
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 16dbe00..31d3170 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -467,8 +467,10 @@ struct rte_eth_rss_conf {
 #define ETH_VMDQ_ACCEPT_BROADCAST   0x0008 /**< accept broadcast packets. */
 #define ETH_VMDQ_ACCEPT_MULTICAST   0x0010 /**< multicast promiscuous. */
 
-/* Definitions used for VMDQ mirror rules setting */
-#define ETH_VMDQ_NUM_MIRROR_RULE     4 /**< Maximum nb. of mirror rules. . */
+/* Definitions used for mirror rules setting */
+#define ETH_MAX_NUM_MIRROR_RULE        16  /**< Maximum nb. of mirror rules. */
+/** Maximum nb. of vlan per mirror rule */
+#define ETH_MIRROR_MAX_NUM_VLANS       64
 
 #define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring. */
 #define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring. */
@@ -480,18 +482,19 @@ struct rte_eth_rss_conf {
  */
 struct rte_eth_vlan_mirror {
 	uint64_t vlan_mask; /**< mask for valid VLAN ID. */
-	uint16_t vlan_id[ETH_VMDQ_MAX_VLAN_FILTERS];
-	/** VLAN ID list for vlan mirror. */
+	/** VLAN ID list for vlan mirroring. */
+	uint16_t vlan_id[ETH_MIRROR_MAX_NUM_VLANS];
 };
 
 /**
  * A structure used to configure traffic mirror of an Ethernet port.
  */
-struct rte_eth_vmdq_mirror_conf {
+struct rte_eth_mirror_conf {
 	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to set */
-	uint8_t dst_pool; /**< Destination pool for this mirror rule. */
+	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
 	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
-	struct rte_eth_vlan_mirror vlan; /**< VLAN ID setting for VLAN mirroring */
+	/** VLAN ID setting for VLAN mirroring. */
+	struct rte_eth_vlan_mirror vlan;
 };
 
 /**
@@ -1211,7 +1214,7 @@ typedef int (*eth_set_vf_rate_limit_t)(struct rte_eth_dev *dev,
 /**< @internal Set VF TX rate */
 
 typedef int (*eth_mirror_rule_set_t)(struct rte_eth_dev *dev,
-				  struct rte_eth_vmdq_mirror_conf *mirror_conf,
+				  struct rte_eth_mirror_conf *mirror_conf,
 				  uint8_t rule_id,
 				  uint8_t on);
 /**< @internal Add a traffic mirroring rule on an Ethernet device */
@@ -3168,7 +3171,7 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
  *   - (-EINVAL) if the mr_conf information is not correct.
  */
 int rte_eth_mirror_rule_set(uint8_t port_id,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id,
 			uint8_t on);
 
-- 
1.9.3

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

* [dpdk-dev] [PATCH v2 2/4] ethdev: redefine the mirror type
  2015-06-02  7:55 ` [dpdk-dev] [PATCH v2 0/4] " Jingjing Wu
  2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
@ 2015-06-02  7:55   ` Jingjing Wu
  2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 3/4] i40e: enable mirror functionality in i40e driver Jingjing Wu
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-02  7:55 UTC (permalink / raw)
  To: dev

This path renames the mirror type in rte_eth_mirror_conf and macros,
and rework the mirror set in ixgbe dirvers by using new definition.
It also fixes some coding style.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 app/test-pmd/cmdline.c           | 42 +++++++++++++++------------
 drivers/net/ixgbe/ixgbe_ethdev.c | 63 ++++++++++++++++++++++++----------------
 lib/librte_ether/rte_ethdev.c    | 14 +++++++--
 lib/librte_ether/rte_ethdev.h    | 11 +++----
 4 files changed, 78 insertions(+), 52 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a084b73..e606795 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -412,7 +412,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Set rate limit for queues in VF of a port\n\n"
 
 			"set port (port_id) mirror-rule (rule_id)"
-			"(pool-mirror|vlan-mirror)\n"
+			" (pool-mirror-up|pool-mirror-down|vlan-mirror)"
 			" (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)\n"
 			"   Set pool or vlan type mirror rule on a port.\n"
 			"   e.g., 'set port 0 mirror-rule 0 vlan-mirror 0,1"
@@ -6583,7 +6583,8 @@ cmdline_parse_token_num_t cmd_mirror_mask_ruleid =
 				rule_id, UINT8);
 cmdline_parse_token_string_t cmd_mirror_mask_what =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_mirror_mask_result,
-				what, "pool-mirror#vlan-mirror");
+				what, "pool-mirror-up#pool-mirror-down"
+				      "#vlan-mirror");
 cmdline_parse_token_string_t cmd_mirror_mask_value =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_mirror_mask_result,
 				value, NULL);
@@ -6612,13 +6613,16 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 
 	mr_conf.dst_pool = res->dstpool_id;
 
-	if (!strcmp(res->what, "pool-mirror")) {
-		mr_conf.pool_mask = strtoull(res->value,NULL,16);
-		mr_conf.rule_type_mask = ETH_VMDQ_POOL_MIRROR;
-	} else if(!strcmp(res->what, "vlan-mirror")) {
-		mr_conf.rule_type_mask = ETH_VMDQ_VLAN_MIRROR;
-		nb_item = parse_item_list(res->value, "core",
-					ETH_MIRROR_MAX_NUM_VLANS, vlan_list, 1);
+	if (!strcmp(res->what, "pool-mirror-up")) {
+		mr_conf.pool_mask = strtoull(res->value, NULL, 16);
+		mr_conf.rule_type = ETH_MIRROR_VIRTUAL_POOL_UP;
+	} else if (!strcmp(res->what, "pool-mirror-down")) {
+		mr_conf.pool_mask = strtoull(res->value, NULL, 16);
+		mr_conf.rule_type = ETH_MIRROR_VIRTUAL_POOL_DOWN;
+	} else if (!strcmp(res->what, "vlan-mirror")) {
+		mr_conf.rule_type = ETH_MIRROR_VLAN;
+		nb_item = parse_item_list(res->value, "vlan",
+				ETH_MIRROR_MAX_NUM_VLANS, vlan_list, 1);
 		if (nb_item <= 0)
 			return;
 
@@ -6633,21 +6637,21 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 		}
 	}
 
-	if(!strcmp(res->on, "on"))
+	if (!strcmp(res->on, "on"))
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
-	if(ret < 0)
+	if (ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
 }
 
 cmdline_parse_inst_t cmd_set_mirror_mask = {
 		.f = cmd_set_mirror_mask_parsed,
 		.data = NULL,
-		.help_str = "set port X mirror-rule Y pool-mirror|vlan-mirror "
-				"pool_mask|vlan_id[,vlan_id]* dst-pool Z on|off",
+		.help_str = "set port X mirror-rule Y pool-mirror-up|pool-mirror-down|vlan-mirror"
+			    " pool_mask|vlan_id[,vlan_id]* dst-pool Z on|off",
 		.tokens = {
 			(void *)&cmd_mirror_mask_set,
 			(void *)&cmd_mirror_mask_port,
@@ -6714,14 +6718,14 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 	struct rte_eth_mirror_conf mr_conf;
 
 	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
-	if(!strcmp(res->what, "uplink-mirror")) {
-		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
-	}else if(!strcmp(res->what, "downlink-mirror"))
-		mr_conf.rule_type_mask = ETH_VMDQ_DOWNLIN_MIRROR;
+	if (!strcmp(res->what, "uplink-mirror"))
+		mr_conf.rule_type = ETH_MIRROR_UPLINK_PORT;
+	else
+		mr_conf.rule_type = ETH_MIRROR_DOWNLINK_PORT;
 
 	mr_conf.dst_pool = res->dstpool_id;
 
-	if(!strcmp(res->on, "on"))
+	if (!strcmp(res->on, "on"))
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
@@ -6729,7 +6733,7 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 						res->rule_id, 0);
 
 	/* check the return value and print it if is < 0 */
-	if(ret < 0)
+	if (ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
 
 }
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 5b6af92..a0a3d03 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3386,6 +3386,11 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 	return ret;
 }
 
+#define IXGBE_MRCTL_VPME  0x01 /* Virtual Pool Mirroring. */
+#define IXGBE_MRCTL_UPME  0x02 /* Uplink Port Mirroring. */
+#define IXGBE_MRCTL_DPME  0x04 /* Downlink Port Mirroring. */
+#define IXGBE_MRCTL_VLME  0x08 /* VLAN Mirroring. */
+
 static int
 ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
@@ -3410,6 +3415,7 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			(IXGBE_DEV_PRIVATE_TO_PFDATA(dev->data->dev_private));
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint8_t mirror_type;
 
 	if (ixgbe_vmdq_mode_check(hw) < 0)
 		return -ENOTSUP;
@@ -3417,28 +3423,24 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 	if (rule_id >= IXGBE_MAX_NUM_MIRROR_RULE)
 		return -EINVAL;
 
-	/* Check if vlan mask is valid */
-	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) && (on)) {
-		if (mirror_conf->vlan.vlan_mask == 0)
-			return (-EINVAL);
-	}
-
-	/* Check if vlan id is valid and find conresponding VLAN ID index in VLVF */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) {
+	switch (mirror_conf->rule_type) {
+	case ETH_MIRROR_VLAN:
+		mirror_type = IXGBE_MRCTL_VLME;
+		/* Check if vlan id is valid and find conresponding VLAN ID index in VLVF */
 		for (i = 0;i < IXGBE_VLVF_ENTRIES; i++) {
 			if (mirror_conf->vlan.vlan_mask & (1ULL << i)) {
 				/* search vlan id related pool vlan filter index */
 				reg_index = ixgbe_find_vlvf_slot(hw,
 						mirror_conf->vlan.vlan_id[i]);
 				if(reg_index < 0)
-					return (-EINVAL);
+					return -EINVAL;
 				vlvf = IXGBE_READ_REG(hw, IXGBE_VLVF(reg_index));
 				if ((vlvf & IXGBE_VLVF_VIEN) &&
-					((vlvf & IXGBE_VLVF_VLANID_MASK)
-						== mirror_conf->vlan.vlan_id[i]))
+				    ((vlvf & IXGBE_VLVF_VLANID_MASK) ==
+				      mirror_conf->vlan.vlan_id[i]))
 					vlan_mask |= (1ULL << reg_index);
 				else
-					return (-EINVAL);
+					return -EINVAL;
 			}
 		}
 
@@ -3460,13 +3462,13 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			for(i = 0 ;i < ETH_VMDQ_MAX_VLAN_FILTERS; i++)
 				mr_info->mr_conf[rule_id].vlan.vlan_id[i] = 0;
 		}
-	}
-
-	/*
-	 * if enable pool mirror, write related pool mask register,if disable
-	 * pool mirror, clear PFMRVM register
-	 */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) {
+		break;
+	case ETH_MIRROR_VIRTUAL_POOL_UP:
+		mirror_type = IXGBE_MRCTL_VPME;
+		/*
+		 * if enable pool mirror, write related pool mask register,if disable
+		 * pool mirror, clear PFMRVM register
+		 */
 		if (on) {
 			mp_lsb = mirror_conf->pool_mask & 0xFFFFFFFF;
 			mp_msb = mirror_conf->pool_mask >> pool_mask_offset;
@@ -3478,32 +3480,43 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			mp_msb = 0;
 			mr_info->mr_conf[rule_id].pool_mask = 0;
 		}
+		break;
+	case ETH_MIRROR_UPLINK_PORT:
+		mirror_type = IXGBE_MRCTL_UPME;
+		break;
+	case ETH_MIRROR_DOWNLINK_PORT:
+		mirror_type = IXGBE_MRCTL_DPME;
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "unsupported mirror type %d.",
+			mirror_conf->rule_type);
+		return -EINVAL;
 	}
 
 	/* read  mirror control register and recalculate it */
 	mr_ctl = IXGBE_READ_REG(hw,IXGBE_MRCTL(rule_id));
 
 	if (on) {
-		mr_ctl |= mirror_conf->rule_type_mask;
+		mr_ctl |= mirror_type;
 		mr_ctl &= mirror_rule_mask;
 		mr_ctl |= mirror_conf->dst_pool << dst_pool_offset;
 	} else
-		mr_ctl &= ~(mirror_conf->rule_type_mask & mirror_rule_mask);
+		mr_ctl &= ~(mirror_conf->rule_type & mirror_rule_mask);
 
-	mr_info->mr_conf[rule_id].rule_type_mask = (uint8_t)(mr_ctl & mirror_rule_mask);
+	mr_info->mr_conf[rule_id].rule_type = mirror_conf->rule_type;
 	mr_info->mr_conf[rule_id].dst_pool = mirror_conf->dst_pool;
 
 	/* write mirrror control  register */
 	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl);
 
-        /* write pool mirrror control  register */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) {
+	/* write pool mirrror control  register */
+	if (mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP) {
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVM(rule_id), mp_lsb);
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVM(rule_id + rule_mr_offset),
 				mp_msb);
 	}
 	/* write VLAN mirrror control  register */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) {
+	if (mirror_conf->rule_type == ETH_MIRROR_VLAN) {
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVLAN(rule_id), mv_lsb);
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVLAN(rule_id + rule_mr_offset),
 				mv_msb);
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 43c7295..dc3b47e 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3044,7 +3044,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -ENODEV;
 	}
 
-	if (mirror_conf->rule_type_mask == 0) {
+	if (mirror_conf->rule_type == 0) {
 		PMD_DEBUG_TRACE("mirror rule type can not be 0.\n");
 		return -EINVAL;
 	}
@@ -3055,12 +3055,20 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -EINVAL;
 	}
 
-	if ((mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) &&
-		(mirror_conf->pool_mask == 0)) {
+	if ((mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP ||
+	     mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_DOWN) &&
+	    (mirror_conf->pool_mask == 0)) {
 		PMD_DEBUG_TRACE("Invalid mirror pool, pool mask can not"
 				"be 0.\n");
 		return -EINVAL;
 	}
+	/* Check if vlan mask is valid */
+	if (mirror_conf->rule_type == ETH_MIRROR_VLAN &&
+	    mirror_conf->vlan.vlan_mask == 0) {
+		PMD_DEBUG_TRACE("Invalid vlan mask, vlan mask can not"
+				"be 0.\n");
+		return -EINVAL;
+	}
 
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 31d3170..3c9be54 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -472,10 +472,11 @@ struct rte_eth_rss_conf {
 /** Maximum nb. of vlan per mirror rule */
 #define ETH_MIRROR_MAX_NUM_VLANS       64
 
-#define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring. */
-#define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring. */
-#define ETH_VMDQ_DOWNLIN_MIRROR 0x0004 /**< Downlink Port Mirroring. */
-#define ETH_VMDQ_VLAN_MIRROR    0x0008 /**< VLAN Mirroring. */
+#define ETH_MIRROR_VIRTUAL_POOL_UP     1  /**< Virtual Pool uplink Mirroring. */
+#define ETH_MIRROR_VIRTUAL_POOL_DOWN   2  /**< Virtual Pool downlink Mirroring. */
+#define ETH_MIRROR_UPLINK_PORT         3  /**< Uplink Port Mirroring. */
+#define ETH_MIRROR_DOWNLINK_PORT       4  /**< Downlink Port Mirroring. */
+#define ETH_MIRROR_VLAN                5  /**< VLAN Mirroring. */
 
 /**
  * A structure used to configure VLAN traffic mirror of an Ethernet port.
@@ -490,7 +491,7 @@ struct rte_eth_vlan_mirror {
  * A structure used to configure traffic mirror of an Ethernet port.
  */
 struct rte_eth_mirror_conf {
-	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to set */
+	uint8_t rule_type; /**< Mirroring rule type  we want to set */
 	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
 	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
 	/** VLAN ID setting for VLAN mirroring. */
-- 
1.9.3

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

* [dpdk-dev] [PATCH v2 3/4] i40e: enable mirror functionality in i40e driver
  2015-06-02  7:55 ` [dpdk-dev] [PATCH v2 0/4] " Jingjing Wu
  2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
  2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 2/4] ethdev: redefine the mirror type Jingjing Wu
@ 2015-06-02  7:55   ` Jingjing Wu
  2015-06-02  7:56   ` [dpdk-dev] [PATCH v2 4/4] doc: modify the command about mirror in testpmd guide Jingjing Wu
  2015-06-05  8:16   ` [dpdk-dev] [PATCH v3 0/4] enable mirror functionality in i40e driver Jingjing Wu
  4 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-02  7:55 UTC (permalink / raw)
  To: dev

enable mirror functionality in i40e driver
.mirror_rule_set
.mirror_rule_reset

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 334 +++++++++++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_ethdev.h |  23 +++
 2 files changed, 357 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index da6c0b5..f89e268 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -211,6 +211,10 @@ static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 				void *arg);
 static void i40e_configure_registers(struct i40e_hw *hw);
 static void i40e_hw_init(struct i40e_hw *hw);
+static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
+			struct rte_eth_mirror_conf *mirror_conf,
+			uint8_t sw_id, uint8_t on);
+static int i40e_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t sw_id);
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
 #define RTE_PCI_DEV_ID_DECL_I40E(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
@@ -262,6 +266,8 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.udp_tunnel_add               = i40e_dev_udp_tunnel_add,
 	.udp_tunnel_del               = i40e_dev_udp_tunnel_del,
 	.filter_ctrl                  = i40e_dev_filter_ctrl,
+	.mirror_rule_set              = i40e_mirror_rule_set,
+	.mirror_rule_reset            = i40e_mirror_rule_reset,
 };
 
 static struct eth_driver rte_i40e_pmd = {
@@ -563,6 +569,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 	/* enable uio intr after callback register */
 	rte_intr_enable(&(pci_dev->intr_handle));
 
+	/* initialize mirror rule list */
+	TAILQ_INIT(&pf->mirror_list);
+
 	return 0;
 
 err_mac_alloc:
@@ -929,6 +938,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_vsi *main_vsi = pf->main_vsi;
+	struct i40e_mirror_rule *p_mirror;
 	int i;
 
 	/* Disable all queues */
@@ -953,6 +963,13 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	/* Set link down */
 	i40e_dev_set_link_down(dev);
 
+	/* Remove all mirror rules */
+	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
+		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
+		rte_free(p_mirror);
+	}
+	pf->nb_mirror_rule = 0;
+
 }
 
 static void
@@ -5697,3 +5714,320 @@ i40e_configure_registers(struct i40e_hw *hw)
 			"0x%"PRIx32, reg_table[i].val, reg_table[i].addr);
 	}
 }
+
+/**
+ * i40e_aq_add_mirror_rule
+ * @hw: pointer to the hardware structure
+ * @seid: VEB seid to add mirror rule to
+ * @dst_id: destination vsi seid
+ * @entries: Buffer which contains the entities to be mirrored
+ * @count: number of entities contained in the buffer
+ * @rule_id:the rule_id of the rule to be added
+ *
+ * Add a mirror rule for a given veb.
+ *
+ **/
+static enum
+i40e_status_code i40e_aq_add_mirror_rule(struct i40e_hw *hw,
+			uint16_t seid, uint16_t dst_id,
+			uint16_t rule_type, uint16_t *entries,
+			uint16_t count, uint16_t *rule_id)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_add_delete_mirror_rule *cmd =
+		(struct i40e_aqc_add_delete_mirror_rule *)&desc.params.raw;
+	struct i40e_aqc_add_delete_mirror_rule_completion *resp =
+		(struct i40e_aqc_add_delete_mirror_rule_completion *)
+		&desc.params.raw;
+	uint16_t buff_len;
+	enum i40e_status_code status;
+
+	i40e_fill_default_direct_cmd_desc(&desc,
+					  i40e_aqc_opc_add_mirror_rule);
+
+	buff_len = sizeof(uint16_t) * count;
+	desc.datalen = rte_cpu_to_le_16(buff_len);
+	if (buff_len > 0)
+		desc.flags |= rte_cpu_to_le_16(
+			(uint16_t)(I40E_AQ_FLAG_BUF | I40E_AQ_FLAG_RD));
+	cmd->rule_type = rte_cpu_to_le_16(rule_type <<
+				I40E_AQC_MIRROR_RULE_TYPE_SHIFT);
+	cmd->num_entries = rte_cpu_to_le_16(count);
+	cmd->seid = rte_cpu_to_le_16(seid);
+	cmd->destination = rte_cpu_to_le_16(dst_id);
+
+	status = i40e_asq_send_command(hw, &desc, entries, buff_len, NULL);
+	PMD_DRV_LOG(INFO, "i40e_aq_add_mirror_rule, aq_status %d,"
+			 "rule_id = %u"
+			 " mirror_rules_used = %u, mirror_rules_free = %u,",
+			 hw->aq.asq_last_status, resp->rule_id,
+			 resp->mirror_rules_used, resp->mirror_rules_free);
+	*rule_id = rte_le_to_cpu_16(resp->rule_id);
+
+	return status;
+}
+
+/**
+ * i40e_aq_del_mirror_rule
+ * @hw: pointer to the hardware structure
+ * @seid: VEB seid to add mirror rule to
+ * @entries: Buffer which contains the entities to be mirrored
+ * @count: number of entities contained in the buffer
+ * @rule_id:the rule_id of the rule to be delete
+ *
+ * Delete a mirror rule for a given veb.
+ *
+ **/
+static enum
+i40e_status_code i40e_aq_del_mirror_rule(struct i40e_hw *hw,
+		uint16_t seid, uint16_t rule_type, uint16_t *entries,
+		uint16_t count, uint16_t rule_id)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_add_delete_mirror_rule *cmd =
+		(struct i40e_aqc_add_delete_mirror_rule *)&desc.params.raw;
+	uint16_t buff_len = 0;
+	enum i40e_status_code status;
+	void *buff = NULL;
+
+	i40e_fill_default_direct_cmd_desc(&desc,
+					  i40e_aqc_opc_delete_mirror_rule);
+
+	if (rule_type == I40E_AQC_MIRROR_RULE_TYPE_VLAN) {
+		desc.flags |= rte_cpu_to_le_16((uint16_t)(I40E_AQ_FLAG_BUF |
+							  I40E_AQ_FLAG_RD));
+		cmd->num_entries = count;
+		buff_len = sizeof(uint16_t) * count;
+		desc.datalen = rte_cpu_to_le_16(buff_len);
+		buff = (void *)entries;
+	} else
+		/* rule id is filled in destination field for deleting mirror rule */
+		cmd->destination = rte_cpu_to_le_16(rule_id);
+
+	cmd->rule_type = rte_cpu_to_le_16(rule_type <<
+				I40E_AQC_MIRROR_RULE_TYPE_SHIFT);
+	cmd->seid = rte_cpu_to_le_16(seid);
+
+	status = i40e_asq_send_command(hw, &desc, buff, buff_len, NULL);
+
+	return status;
+}
+
+/**
+ * i40e_mirror_rule_set
+ * @dev: pointer to the hardware structure
+ * @mirror_conf: mirror rule info
+ * @sw_id: mirror rule's sw_id
+ * @on: enable/disable
+ *
+ * set a mirror rule.
+ *
+ **/
+static int
+i40e_mirror_rule_set(struct rte_eth_dev *dev,
+			struct rte_eth_mirror_conf *mirror_conf,
+			uint8_t sw_id, uint8_t on)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_mirror_rule *it, *mirr_rule = NULL;
+	struct i40e_mirror_rule *parent = NULL;
+	uint16_t seid, dst_seid, rule_id;
+	uint16_t i, j = 0;
+	int ret;
+
+	PMD_DRV_LOG(DEBUG, "i40e_mirror_rule_set: sw_id = %d.", sw_id);
+
+	if (pf->main_vsi->veb == NULL || pf->vfs == NULL) {
+		PMD_DRV_LOG(ERR, "mirror rule can not be configured"
+			" without veb or vfs.");
+		return -ENOSYS;
+	}
+	if (pf->nb_mirror_rule > I40E_MAX_NUM_MIRROR_RULE) {
+		PMD_DRV_LOG(ERR, "mirror table is full.");
+		return -ENOSYS;
+	}
+	if (mirror_conf->dst_pool > pf->vf_num) {
+		PMD_DRV_LOG(ERR, "invalid destination pool %u.",
+				 mirror_conf->dst_pool);
+		return -EINVAL;
+	}
+
+	seid = pf->main_vsi->veb->seid;
+
+	TAILQ_FOREACH(it, &pf->mirror_list, rules) {
+		if (sw_id <= it->index) {
+			mirr_rule = it;
+			break;
+		}
+		parent = it;
+	}
+	if (mirr_rule && sw_id == mirr_rule->index) {
+		if (on) {
+			PMD_DRV_LOG(ERR, "mirror rule exists.");
+			return -EEXIST;
+		} else {
+			ret = i40e_aq_del_mirror_rule(hw, seid,
+					mirr_rule->rule_type,
+					mirr_rule->entries,
+					mirr_rule->num_entries, mirr_rule->id);
+			if (ret < 0) {
+				PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
+						   " ret = %d, aq_err = %d.",
+						   ret, hw->aq.asq_last_status);
+				return -ENOSYS;
+			}
+			TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
+			rte_free(mirr_rule);
+			pf->nb_mirror_rule--;
+			return 0;
+		}
+	} else if (!on) {
+		PMD_DRV_LOG(ERR, "mirror rule doesn't exist.");
+		return -ENOENT;
+	}
+
+	mirr_rule = rte_zmalloc("i40e_mirror_rule",
+				sizeof(struct i40e_mirror_rule) , 0);
+	if (!mirr_rule) {
+		PMD_DRV_LOG(ERR, "failed to allocate memory");
+		return I40E_ERR_NO_MEMORY;
+	}
+	switch (mirror_conf->rule_type) {
+	case ETH_MIRROR_VLAN:
+		for (i = 0, j = 0; i < ETH_MIRROR_MAX_NUM_VLANS; i++) {
+			if (mirror_conf->vlan.vlan_mask & (1ULL << i)) {
+				mirr_rule->entries[j] =
+					mirror_conf->vlan.vlan_id[i];
+				j++;
+			}
+		}
+		if (j == 0) {
+			PMD_DRV_LOG(ERR, "vlan is not specified.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_VLAN;
+		break;
+	case ETH_MIRROR_VIRTUAL_POOL_UP:
+	case ETH_MIRROR_VIRTUAL_POOL_DOWN:
+		/* check if the specified pool bit is out of range */
+		if (mirror_conf->pool_mask > (uint64_t)(1ULL << (pf->vf_num + 1))) {
+			PMD_DRV_LOG(ERR, "pool mask is out of range.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		for (i = 0, j = 0; i < pf->vf_num; i++) {
+			if (mirror_conf->pool_mask & (1ULL << i)) {
+				mirr_rule->entries[j] = pf->vfs[i].vsi->seid;
+				j++;
+			}
+		}
+		if (mirror_conf->pool_mask & (1ULL << pf->vf_num)) {
+			/* add pf vsi to entries */
+			mirr_rule->entries[j] = pf->main_vsi_seid;
+			j++;
+		}
+		if (j == 0) {
+			PMD_DRV_LOG(ERR, "pool is not specified.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		/* egress and ingress in aq commands means from switch but not port */
+		mirr_rule->rule_type =
+			(mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP) ?
+			I40E_AQC_MIRROR_RULE_TYPE_VPORT_EGRESS :
+			I40E_AQC_MIRROR_RULE_TYPE_VPORT_INGRESS;
+		break;
+	case ETH_MIRROR_UPLINK_PORT:
+		/* egress and ingress in aq commands means from switch but not port*/
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_ALL_EGRESS;
+		break;
+	case ETH_MIRROR_DOWNLINK_PORT:
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_ALL_INGRESS;
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "unsupported mirror type %d.",
+			mirror_conf->rule_type);
+		rte_free(mirr_rule);
+		return -EINVAL;
+	}
+
+	/* If the dst_pool is equal to vf_num, consider it as PF */
+	if (mirror_conf->dst_pool == pf->vf_num)
+		dst_seid = pf->main_vsi_seid;
+	else
+		dst_seid = pf->vfs[mirror_conf->dst_pool].vsi->seid;
+
+	ret = i40e_aq_add_mirror_rule(hw, seid, dst_seid,
+				      mirr_rule->rule_type, mirr_rule->entries,
+				      j, &rule_id);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "failed to add mirror rule:"
+				   " ret = %d, aq_err = %d.",
+				   ret, hw->aq.asq_last_status);
+		rte_free(mirr_rule);
+		return -ENOSYS;
+	}
+
+	mirr_rule->index = sw_id;
+	mirr_rule->num_entries = j;
+	mirr_rule->id = rule_id;
+	mirr_rule->dst_vsi_seid = dst_seid;
+
+	if (parent)
+		TAILQ_INSERT_AFTER(&pf->mirror_list, parent, mirr_rule, rules);
+	else
+		TAILQ_INSERT_HEAD(&pf->mirror_list, mirr_rule, rules);
+
+	pf->nb_mirror_rule++;
+	return 0;
+}
+
+/**
+ * i40e_mirror_rule_reset
+ * @dev: pointer to the device
+ * @sw_id: mirror rule's sw_id
+ *
+ * reset a mirror rule.
+ *
+ **/
+static int
+i40e_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t sw_id)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_mirror_rule *it, *mirr_rule = NULL;
+	uint16_t seid;
+	int ret;
+
+	PMD_DRV_LOG(DEBUG, "i40e_mirror_rule_reset: sw_id = %d.", sw_id);
+
+	seid = pf->main_vsi->veb->seid;
+
+	TAILQ_FOREACH(it, &pf->mirror_list, rules) {
+		if (sw_id == it->index) {
+			mirr_rule = it;
+			break;
+		}
+	}
+	if (mirr_rule) {
+		ret = i40e_aq_del_mirror_rule(hw, seid,
+				mirr_rule->rule_type,
+				mirr_rule->entries,
+				mirr_rule->num_entries, mirr_rule->id);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
+					   " status = %d, aq_err = %d.",
+					   ret, hw->aq.asq_last_status);
+			return -ENOSYS;
+		}
+		TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
+		rte_free(mirr_rule);
+		pf->nb_mirror_rule--;
+	} else {
+		PMD_DRV_LOG(ERR, "mirror rule doesn't exist.");
+		return -ENOENT;
+	}
+	return 0;
+}
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index baac4a0..f364221 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -323,6 +323,27 @@ struct i40e_fdir_info {
 	struct i40e_fdir_flex_mask flex_mask[I40E_FILTER_PCTYPE_MAX];
 };
 
+#define I40E_MIRROR_MAX_ENTRIES_PER_RULE   64
+#define I40E_MAX_NUM_MIRROR_RULE           32
+/*
+ * Mirror rule structure
+ */
+struct i40e_mirror_rule {
+	TAILQ_ENTRY(i40e_mirror_rule) rules;
+	uint8_t rule_type;
+	uint16_t index;          /* the sw index of mirror rule */
+	uint16_t id;             /* the rule id assigned by firmware */
+	uint16_t dst_vsi_seid;   /* destination vsi for this mirror rule. */
+	uint16_t num_entries;
+	/* the info stores depend on the rule type.
+	    If type is I40E_MIRROR_TYPE_VLAN, vlan ids are stored here.
+	    If type is I40E_MIRROR_TYPE_VPORT_*, vsi's seid are stored.
+	 */
+	uint16_t entries[I40E_MIRROR_MAX_ENTRIES_PER_RULE];
+};
+
+TAILQ_HEAD(i40e_mirror_rule_list, i40e_mirror_rule);
+
 /*
  * Structure to store private data specific for PF instance.
  */
@@ -362,6 +383,8 @@ struct i40e_pf {
 	struct i40e_vmdq_info *vmdq;
 
 	struct i40e_fdir_info fdir; /* flow director info */
+	struct i40e_mirror_rule_list mirror_list;
+	uint16_t nb_mirror_rule;   /* The number of mirror rules */
 };
 
 enum pending_msg {
-- 
1.9.3

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

* [dpdk-dev] [PATCH v2 4/4] doc: modify the command about mirror in testpmd guide
  2015-06-02  7:55 ` [dpdk-dev] [PATCH v2 0/4] " Jingjing Wu
                     ` (2 preceding siblings ...)
  2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 3/4] i40e: enable mirror functionality in i40e driver Jingjing Wu
@ 2015-06-02  7:56   ` Jingjing Wu
  2015-06-05  8:16   ` [dpdk-dev] [PATCH v3 0/4] enable mirror functionality in i40e driver Jingjing Wu
  4 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-02  7:56 UTC (permalink / raw)
  To: dev

This patch modifies the mirror command about type extension and adds link mirror command.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 761172e..2cd3461 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -772,9 +772,13 @@ set port (port_id) vf (vf_id) rate (rate_value) queue_mask (queue_mask)
 set port - mirror rule
 ~~~~~~~~~~~~~~~~~~~~~~
 
-Set port or vlan type mirror rule for a port.
+Set pool or vlan type mirror rule for a port:
 
-set port (port_id) mirror-rule (rule_id) (pool-mirror|vlan-mirror) (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)
+set port (port_id) mirror-rule (rule_id) (pool-mirror-up|pool-mirror-down|vlan-mirror) (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)
+
+Set link mirror rule for a port:
+
+set port (port_id) mirror-rule (rule_id) (uplink-mirror|downlink-mirror) dst-pool (pool_id) (on|off)
 
 For example to enable mirror traffic with vlan 0,1 to pool 0:
 
-- 
1.9.3

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

* [dpdk-dev] [PATCH v3 0/4] enable mirror functionality in i40e driver
  2015-06-02  7:55 ` [dpdk-dev] [PATCH v2 0/4] " Jingjing Wu
                     ` (3 preceding siblings ...)
  2015-06-02  7:56   ` [dpdk-dev] [PATCH v2 4/4] doc: modify the command about mirror in testpmd guide Jingjing Wu
@ 2015-06-05  8:16   ` Jingjing Wu
  2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
                       ` (4 more replies)
  4 siblings, 5 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-05  8:16 UTC (permalink / raw)
  To: dev

This patch set enables mirror functionality in i40e driver, and redefines structure and macros used to configure mirror.
 
v2 changes:
 - correct comments style
 - add doc change
 
v3 changes:
 - change the mirror rule type to support bit mask and avoid ABI broken
 - fix code style

Jingjing Wu (4):
  ethdev: rename rte_eth_vmdq_mirror_conf
  ethdev: redefine the mirror type
  i40e: enable mirror functionality in i40e driver
  doc: modify the command about mirror in testpmd guide

 app/test-pmd/cmdline.c                      |  62 +++---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
 drivers/net/i40e/i40e_ethdev.c              | 334 ++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_ethdev.h              |  23 ++
 drivers/net/ixgbe/ixgbe_ethdev.c            |  64 ++++--
 drivers/net/ixgbe/ixgbe_ethdev.h            |   4 +-
 lib/librte_ether/rte_ethdev.c               |  28 +--
 lib/librte_ether/rte_ethdev.h               |  30 +--
 8 files changed, 467 insertions(+), 86 deletions(-)

-- 
1.9.3

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

* [dpdk-dev] [PATCH v3 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-06-05  8:16   ` [dpdk-dev] [PATCH v3 0/4] enable mirror functionality in i40e driver Jingjing Wu
@ 2015-06-05  8:16     ` Jingjing Wu
  2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 2/4] ethdev: redefine the mirror type Jingjing Wu
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-05  8:16 UTC (permalink / raw)
  To: dev

rename rte_eth_vmdq_mirror_conf to rte_eth_mirror_conf and move
the maximum rule id check from ethdev level to driver

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 app/test-pmd/cmdline.c           | 22 +++++++++++-----------
 drivers/net/ixgbe/ixgbe_ethdev.c | 11 +++++++----
 drivers/net/ixgbe/ixgbe_ethdev.h |  4 +++-
 lib/librte_ether/rte_ethdev.c    | 18 ++----------------
 lib/librte_ether/rte_ethdev.h    | 19 ++++++++++---------
 5 files changed, 33 insertions(+), 41 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f01db2a..d693bde 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -6604,11 +6604,11 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 {
 	int ret,nb_item,i;
 	struct cmd_set_mirror_mask_result *res = parsed_result;
-	struct rte_eth_vmdq_mirror_conf mr_conf;
+	struct rte_eth_mirror_conf mr_conf;
 
-	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
+	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
 
-	unsigned int vlan_list[ETH_VMDQ_MAX_VLAN_FILTERS];
+	unsigned int vlan_list[ETH_MIRROR_MAX_VLANS];
 
 	mr_conf.dst_pool = res->dstpool_id;
 
@@ -6618,11 +6618,11 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 	} else if(!strcmp(res->what, "vlan-mirror")) {
 		mr_conf.rule_type_mask = ETH_VMDQ_VLAN_MIRROR;
 		nb_item = parse_item_list(res->value, "core",
-					ETH_VMDQ_MAX_VLAN_FILTERS,vlan_list,1);
+					ETH_MIRROR_MAX_VLANS, vlan_list, 1);
 		if (nb_item <= 0)
 			return;
 
-		for(i=0; i < nb_item; i++) {
+		for (i = 0; i < nb_item; i++) {
 			if (vlan_list[i] > ETHER_MAX_VLAN_ID) {
 				printf("Invalid vlan_id: must be < 4096\n");
 				return;
@@ -6634,10 +6634,10 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 	}
 
 	if(!strcmp(res->on, "on"))
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
 	if(ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
@@ -6711,9 +6711,9 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 {
 	int ret;
 	struct cmd_set_mirror_link_result *res = parsed_result;
-	struct rte_eth_vmdq_mirror_conf mr_conf;
+	struct rte_eth_mirror_conf mr_conf;
 
-	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
+	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
 	if(!strcmp(res->what, "uplink-mirror")) {
 		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
 	}else if(!strcmp(res->what, "downlink-mirror"))
@@ -6722,10 +6722,10 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 	mr_conf.dst_pool = res->dstpool_id;
 
 	if(!strcmp(res->on, "on"))
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
 
 	/* check the return value and print it if is < 0 */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0d9f9b2..9e767fa 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -209,7 +209,7 @@ static int ixgbe_set_pool_tx(struct rte_eth_dev *dev,uint16_t pool,uint8_t on);
 static int ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 		uint64_t pool_mask,uint8_t vlan_on);
 static int ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
-		struct rte_eth_vmdq_mirror_conf *mirror_conf,
+		struct rte_eth_mirror_conf *mirror_conf,
 		uint8_t rule_id, uint8_t on);
 static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev,
 		uint8_t	rule_id);
@@ -3388,7 +3388,7 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 
 static int
 ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id, uint8_t on)
 {
 	uint32_t mr_ctl,vlvf;
@@ -3412,7 +3412,10 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	if (ixgbe_vmdq_mode_check(hw) < 0)
-		return (-ENOTSUP);
+		return -ENOTSUP;
+
+	if (rule_id >= IXGBE_MAX_MIRROR_RULES)
+		return -EINVAL;
 
 	/* Check if vlan mask is valid */
 	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) && (on)) {
@@ -3526,7 +3529,7 @@ ixgbe_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t rule_id)
 		return (-ENOTSUP);
 
 	memset(&mr_info->mr_conf[rule_id], 0,
-		sizeof(struct rte_eth_vmdq_mirror_conf));
+		sizeof(struct rte_eth_mirror_conf));
 
 	/* clear PFVMCTL register */
 	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl);
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 19237b8..755b674 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -177,8 +177,10 @@ struct ixgbe_uta_info {
 	uint32_t uta_shadow[IXGBE_MAX_UTA];
 };
 
+#define IXGBE_MAX_MIRROR_RULES 4  /* Maximum nb. of mirror rules. */
+
 struct ixgbe_mirror_info {
-	struct rte_eth_vmdq_mirror_conf mr_conf[ETH_VMDQ_NUM_MIRROR_RULE];
+	struct rte_eth_mirror_conf mr_conf[IXGBE_MAX_MIRROR_RULES];
 	/**< store PF mirror rules configuration*/
 };
 
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 024fe8b..43c7295 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3034,7 +3034,7 @@ int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t tx_rate,
 
 int
 rte_eth_mirror_rule_set(uint8_t port_id,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id, uint8_t on)
 {
 	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
@@ -3051,7 +3051,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 
 	if (mirror_conf->dst_pool >= ETH_64_POOLS) {
 		PMD_DEBUG_TRACE("Invalid dst pool, pool id must"
-			"be 0-%d\n",ETH_64_POOLS - 1);
+			"be 0-%d\n", ETH_64_POOLS - 1);
 		return -EINVAL;
 	}
 
@@ -3062,13 +3062,6 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -EINVAL;
 	}
 
-	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
-	{
-		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-%d\n",
-			ETH_VMDQ_NUM_MIRROR_RULE - 1);
-		return -EINVAL;
-	}
-
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
 
@@ -3085,13 +3078,6 @@ rte_eth_mirror_rule_reset(uint8_t port_id, uint8_t rule_id)
 		return -ENODEV;
 	}
 
-	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
-	{
-		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-%d\n",
-			ETH_VMDQ_NUM_MIRROR_RULE-1);
-		return -EINVAL;
-	}
-
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_reset, -ENOTSUP);
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 16dbe00..ae22fea 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -467,8 +467,8 @@ struct rte_eth_rss_conf {
 #define ETH_VMDQ_ACCEPT_BROADCAST   0x0008 /**< accept broadcast packets. */
 #define ETH_VMDQ_ACCEPT_MULTICAST   0x0010 /**< multicast promiscuous. */
 
-/* Definitions used for VMDQ mirror rules setting */
-#define ETH_VMDQ_NUM_MIRROR_RULE     4 /**< Maximum nb. of mirror rules. . */
+/** Maximum nb. of vlan per mirror rule */
+#define ETH_MIRROR_MAX_VLANS       64
 
 #define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring. */
 #define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring. */
@@ -480,18 +480,19 @@ struct rte_eth_rss_conf {
  */
 struct rte_eth_vlan_mirror {
 	uint64_t vlan_mask; /**< mask for valid VLAN ID. */
-	uint16_t vlan_id[ETH_VMDQ_MAX_VLAN_FILTERS];
-	/** VLAN ID list for vlan mirror. */
+	/** VLAN ID list for vlan mirroring. */
+	uint16_t vlan_id[ETH_MIRROR_MAX_VLANS];
 };
 
 /**
  * A structure used to configure traffic mirror of an Ethernet port.
  */
-struct rte_eth_vmdq_mirror_conf {
+struct rte_eth_mirror_conf {
 	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to set */
-	uint8_t dst_pool; /**< Destination pool for this mirror rule. */
+	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
 	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
-	struct rte_eth_vlan_mirror vlan; /**< VLAN ID setting for VLAN mirroring */
+	/** VLAN ID setting for VLAN mirroring. */
+	struct rte_eth_vlan_mirror vlan;
 };
 
 /**
@@ -1211,7 +1212,7 @@ typedef int (*eth_set_vf_rate_limit_t)(struct rte_eth_dev *dev,
 /**< @internal Set VF TX rate */
 
 typedef int (*eth_mirror_rule_set_t)(struct rte_eth_dev *dev,
-				  struct rte_eth_vmdq_mirror_conf *mirror_conf,
+				  struct rte_eth_mirror_conf *mirror_conf,
 				  uint8_t rule_id,
 				  uint8_t on);
 /**< @internal Add a traffic mirroring rule on an Ethernet device */
@@ -3168,7 +3169,7 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
  *   - (-EINVAL) if the mr_conf information is not correct.
  */
 int rte_eth_mirror_rule_set(uint8_t port_id,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id,
 			uint8_t on);
 
-- 
1.9.3

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

* [dpdk-dev] [PATCH v3 2/4] ethdev: redefine the mirror type
  2015-06-05  8:16   ` [dpdk-dev] [PATCH v3 0/4] enable mirror functionality in i40e driver Jingjing Wu
  2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
@ 2015-06-05  8:16     ` Jingjing Wu
  2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 3/4] i40e: enable mirror functionality in i40e driver Jingjing Wu
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-05  8:16 UTC (permalink / raw)
  To: dev

This path renames the mirror type in rte_eth_mirror_conf and macros,
and rework the mirror set in ixgbe dirvers by using new definition.
It also fixes some coding style.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 app/test-pmd/cmdline.c           | 42 +++++++++++++++++--------------
 drivers/net/ixgbe/ixgbe_ethdev.c | 53 ++++++++++++++++++++++++++--------------
 lib/librte_ether/rte_ethdev.c    | 14 ++++++++---
 lib/librte_ether/rte_ethdev.h    | 11 +++++----
 4 files changed, 74 insertions(+), 46 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index d693bde..6d4474b 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -412,7 +412,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Set rate limit for queues in VF of a port\n\n"
 
 			"set port (port_id) mirror-rule (rule_id)"
-			"(pool-mirror|vlan-mirror)\n"
+			" (pool-mirror-up|pool-mirror-down|vlan-mirror)"
 			" (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)\n"
 			"   Set pool or vlan type mirror rule on a port.\n"
 			"   e.g., 'set port 0 mirror-rule 0 vlan-mirror 0,1"
@@ -6583,7 +6583,8 @@ cmdline_parse_token_num_t cmd_mirror_mask_ruleid =
 				rule_id, UINT8);
 cmdline_parse_token_string_t cmd_mirror_mask_what =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_mirror_mask_result,
-				what, "pool-mirror#vlan-mirror");
+				what, "pool-mirror-up#pool-mirror-down"
+				      "#vlan-mirror");
 cmdline_parse_token_string_t cmd_mirror_mask_value =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_mirror_mask_result,
 				value, NULL);
@@ -6612,13 +6613,16 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 
 	mr_conf.dst_pool = res->dstpool_id;
 
-	if (!strcmp(res->what, "pool-mirror")) {
-		mr_conf.pool_mask = strtoull(res->value,NULL,16);
-		mr_conf.rule_type_mask = ETH_VMDQ_POOL_MIRROR;
-	} else if(!strcmp(res->what, "vlan-mirror")) {
-		mr_conf.rule_type_mask = ETH_VMDQ_VLAN_MIRROR;
-		nb_item = parse_item_list(res->value, "core",
-					ETH_MIRROR_MAX_VLANS, vlan_list, 1);
+	if (!strcmp(res->what, "pool-mirror-up")) {
+		mr_conf.pool_mask = strtoull(res->value, NULL, 16);
+		mr_conf.rule_type = ETH_MIRROR_VIRTUAL_POOL_UP;
+	} else if (!strcmp(res->what, "pool-mirror-down")) {
+		mr_conf.pool_mask = strtoull(res->value, NULL, 16);
+		mr_conf.rule_type = ETH_MIRROR_VIRTUAL_POOL_DOWN;
+	} else if (!strcmp(res->what, "vlan-mirror")) {
+		mr_conf.rule_type = ETH_MIRROR_VLAN;
+		nb_item = parse_item_list(res->value, "vlan",
+				ETH_MIRROR_MAX_VLANS, vlan_list, 1);
 		if (nb_item <= 0)
 			return;
 
@@ -6633,21 +6637,21 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 		}
 	}
 
-	if(!strcmp(res->on, "on"))
+	if (!strcmp(res->on, "on"))
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
-	if(ret < 0)
+	if (ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
 }
 
 cmdline_parse_inst_t cmd_set_mirror_mask = {
 		.f = cmd_set_mirror_mask_parsed,
 		.data = NULL,
-		.help_str = "set port X mirror-rule Y pool-mirror|vlan-mirror "
-				"pool_mask|vlan_id[,vlan_id]* dst-pool Z on|off",
+		.help_str = "set port X mirror-rule Y pool-mirror-up|pool-mirror-down|vlan-mirror"
+			    " pool_mask|vlan_id[,vlan_id]* dst-pool Z on|off",
 		.tokens = {
 			(void *)&cmd_mirror_mask_set,
 			(void *)&cmd_mirror_mask_port,
@@ -6714,14 +6718,14 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 	struct rte_eth_mirror_conf mr_conf;
 
 	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
-	if(!strcmp(res->what, "uplink-mirror")) {
-		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
-	}else if(!strcmp(res->what, "downlink-mirror"))
-		mr_conf.rule_type_mask = ETH_VMDQ_DOWNLIN_MIRROR;
+	if (!strcmp(res->what, "uplink-mirror"))
+		mr_conf.rule_type = ETH_MIRROR_UPLINK_PORT;
+	else
+		mr_conf.rule_type = ETH_MIRROR_DOWNLINK_PORT;
 
 	mr_conf.dst_pool = res->dstpool_id;
 
-	if(!strcmp(res->on, "on"))
+	if (!strcmp(res->on, "on"))
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
@@ -6729,7 +6733,7 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 						res->rule_id, 0);
 
 	/* check the return value and print it if is < 0 */
-	if(ret < 0)
+	if (ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
 
 }
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 9e767fa..8d01494 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3386,6 +3386,15 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 	return ret;
 }
 
+#define IXGBE_MRCTL_VPME  0x01 /* Virtual Pool Mirroring. */
+#define IXGBE_MRCTL_UPME  0x02 /* Uplink Port Mirroring. */
+#define IXGBE_MRCTL_DPME  0x04 /* Downlink Port Mirroring. */
+#define IXGBE_MRCTL_VLME  0x08 /* VLAN Mirroring. */
+#define IXGBE_INVALID_MIRROR_TYPE(mirror_type) \
+	((mirror_type) & ~(uint8_t)(ETH_MIRROR_VIRTUAL_POOL_UP | \
+	ETH_MIRROR_VIRTUAL_POOL_DOWN | ETH_MIRROR_UPLINK_PORT | \
+	ETH_MIRROR_DOWNLINK_PORT | ETH_MIRROR_VLAN))
+
 static int
 ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
@@ -3410,6 +3419,7 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			(IXGBE_DEV_PRIVATE_TO_PFDATA(dev->data->dev_private));
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint8_t mirror_type = 0;
 
 	if (ixgbe_vmdq_mode_check(hw) < 0)
 		return -ENOTSUP;
@@ -3417,28 +3427,28 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 	if (rule_id >= IXGBE_MAX_MIRROR_RULES)
 		return -EINVAL;
 
-	/* Check if vlan mask is valid */
-	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) && (on)) {
-		if (mirror_conf->vlan.vlan_mask == 0)
-			return (-EINVAL);
-	}
+	if (IXGBE_INVALID_MIRROR_TYPE(mirror_conf->rule_type))
+		PMD_DRV_LOG(ERR, "unsupported mirror type 0x%x.",
+			mirror_conf->rule_type);
+		return -EINVAL;
 
-	/* Check if vlan id is valid and find conresponding VLAN ID index in VLVF */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) {
+	if (mirror_conf->rule_type & ETH_MIRROR_VLAN) {
+		mirror_type |= IXGBE_MRCTL_VLME;
+		/* Check if vlan id is valid and find conresponding VLAN ID index in VLVF */
 		for (i = 0;i < IXGBE_VLVF_ENTRIES; i++) {
 			if (mirror_conf->vlan.vlan_mask & (1ULL << i)) {
 				/* search vlan id related pool vlan filter index */
 				reg_index = ixgbe_find_vlvf_slot(hw,
 						mirror_conf->vlan.vlan_id[i]);
 				if(reg_index < 0)
-					return (-EINVAL);
+					return -EINVAL;
 				vlvf = IXGBE_READ_REG(hw, IXGBE_VLVF(reg_index));
 				if ((vlvf & IXGBE_VLVF_VIEN) &&
-					((vlvf & IXGBE_VLVF_VLANID_MASK)
-						== mirror_conf->vlan.vlan_id[i]))
+				    ((vlvf & IXGBE_VLVF_VLANID_MASK) ==
+				      mirror_conf->vlan.vlan_id[i]))
 					vlan_mask |= (1ULL << reg_index);
 				else
-					return (-EINVAL);
+					return -EINVAL;
 			}
 		}
 
@@ -3466,7 +3476,8 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 	 * if enable pool mirror, write related pool mask register,if disable
 	 * pool mirror, clear PFMRVM register
 	 */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) {
+	if (mirror_conf->rule_type & ETH_MIRROR_VIRTUAL_POOL_UP) {
+		mirror_type |= IXGBE_MRCTL_VPME;
 		if (on) {
 			mp_lsb = mirror_conf->pool_mask & 0xFFFFFFFF;
 			mp_msb = mirror_conf->pool_mask >> pool_mask_offset;
@@ -3479,31 +3490,35 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			mr_info->mr_conf[rule_id].pool_mask = 0;
 		}
 	}
+	if (mirror_conf->rule_type & ETH_MIRROR_UPLINK_PORT)
+		mirror_type |= IXGBE_MRCTL_UPME;
+	if (mirror_conf->rule_type & ETH_MIRROR_DOWNLINK_PORT)
+		mirror_type |= IXGBE_MRCTL_DPME;
 
 	/* read  mirror control register and recalculate it */
-	mr_ctl = IXGBE_READ_REG(hw,IXGBE_MRCTL(rule_id));
+	mr_ctl = IXGBE_READ_REG(hw, IXGBE_MRCTL(rule_id));
 
 	if (on) {
-		mr_ctl |= mirror_conf->rule_type_mask;
+		mr_ctl |= mirror_type;
 		mr_ctl &= mirror_rule_mask;
 		mr_ctl |= mirror_conf->dst_pool << dst_pool_offset;
 	} else
-		mr_ctl &= ~(mirror_conf->rule_type_mask & mirror_rule_mask);
+		mr_ctl &= ~(mirror_conf->rule_type & mirror_rule_mask);
 
-	mr_info->mr_conf[rule_id].rule_type_mask = (uint8_t)(mr_ctl & mirror_rule_mask);
+	mr_info->mr_conf[rule_id].rule_type = mirror_conf->rule_type;
 	mr_info->mr_conf[rule_id].dst_pool = mirror_conf->dst_pool;
 
 	/* write mirrror control  register */
 	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl);
 
-        /* write pool mirrror control  register */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) {
+	/* write pool mirrror control  register */
+	if (mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP) {
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVM(rule_id), mp_lsb);
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVM(rule_id + rule_mr_offset),
 				mp_msb);
 	}
 	/* write VLAN mirrror control  register */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) {
+	if (mirror_conf->rule_type == ETH_MIRROR_VLAN) {
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVLAN(rule_id), mv_lsb);
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVLAN(rule_id + rule_mr_offset),
 				mv_msb);
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 43c7295..34f1900 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3044,7 +3044,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -ENODEV;
 	}
 
-	if (mirror_conf->rule_type_mask == 0) {
+	if (mirror_conf->rule_type == 0) {
 		PMD_DEBUG_TRACE("mirror rule type can not be 0.\n");
 		return -EINVAL;
 	}
@@ -3055,12 +3055,20 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -EINVAL;
 	}
 
-	if ((mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) &&
-		(mirror_conf->pool_mask == 0)) {
+	if ((mirror_conf->rule_type & (ETH_MIRROR_VIRTUAL_POOL_UP |
+	     ETH_MIRROR_VIRTUAL_POOL_DOWN)) &&
+	    (mirror_conf->pool_mask == 0)) {
 		PMD_DEBUG_TRACE("Invalid mirror pool, pool mask can not"
 				"be 0.\n");
 		return -EINVAL;
 	}
+	/* Check if vlan mask is valid */
+	if ((mirror_conf->rule_type & ETH_MIRROR_VLAN) &&
+	    mirror_conf->vlan.vlan_mask == 0) {
+		PMD_DEBUG_TRACE("Invalid vlan mask, vlan mask can not"
+				"be 0.\n");
+		return -EINVAL;
+	}
 
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index ae22fea..d113c8a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -470,10 +470,11 @@ struct rte_eth_rss_conf {
 /** Maximum nb. of vlan per mirror rule */
 #define ETH_MIRROR_MAX_VLANS       64
 
-#define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring. */
-#define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring. */
-#define ETH_VMDQ_DOWNLIN_MIRROR 0x0004 /**< Downlink Port Mirroring. */
-#define ETH_VMDQ_VLAN_MIRROR    0x0008 /**< VLAN Mirroring. */
+#define ETH_MIRROR_VIRTUAL_POOL_UP     0x01  /**< Virtual Pool uplink Mirroring. */
+#define ETH_MIRROR_UPLINK_PORT         0x02  /**< Uplink Port Mirroring. */
+#define ETH_MIRROR_DOWNLINK_PORT       0x04  /**< Downlink Port Mirroring. */
+#define ETH_MIRROR_VLAN                0x08  /**< VLAN Mirroring. */
+#define ETH_MIRROR_VIRTUAL_POOL_DOWN   0x10  /**< Virtual Pool downlink Mirroring. */
 
 /**
  * A structure used to configure VLAN traffic mirror of an Ethernet port.
@@ -488,7 +489,7 @@ struct rte_eth_vlan_mirror {
  * A structure used to configure traffic mirror of an Ethernet port.
  */
 struct rte_eth_mirror_conf {
-	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to set */
+	uint8_t rule_type; /**< Mirroring rule type */
 	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
 	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
 	/** VLAN ID setting for VLAN mirroring. */
-- 
1.9.3

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

* [dpdk-dev] [PATCH v3 3/4] i40e: enable mirror functionality in i40e driver
  2015-06-05  8:16   ` [dpdk-dev] [PATCH v3 0/4] enable mirror functionality in i40e driver Jingjing Wu
  2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
  2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 2/4] ethdev: redefine the mirror type Jingjing Wu
@ 2015-06-05  8:16     ` Jingjing Wu
  2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 4/4] doc: modify the command about mirror in testpmd guide Jingjing Wu
  2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
  4 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-05  8:16 UTC (permalink / raw)
  To: dev

enable mirror functionality in i40e driver
.mirror_rule_set
.mirror_rule_reset

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 334 +++++++++++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_ethdev.h |  23 +++
 2 files changed, 357 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index da6c0b5..d0e83b5 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -211,6 +211,10 @@ static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 				void *arg);
 static void i40e_configure_registers(struct i40e_hw *hw);
 static void i40e_hw_init(struct i40e_hw *hw);
+static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
+			struct rte_eth_mirror_conf *mirror_conf,
+			uint8_t sw_id, uint8_t on);
+static int i40e_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t sw_id);
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
 #define RTE_PCI_DEV_ID_DECL_I40E(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
@@ -262,6 +266,8 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.udp_tunnel_add               = i40e_dev_udp_tunnel_add,
 	.udp_tunnel_del               = i40e_dev_udp_tunnel_del,
 	.filter_ctrl                  = i40e_dev_filter_ctrl,
+	.mirror_rule_set              = i40e_mirror_rule_set,
+	.mirror_rule_reset            = i40e_mirror_rule_reset,
 };
 
 static struct eth_driver rte_i40e_pmd = {
@@ -563,6 +569,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 	/* enable uio intr after callback register */
 	rte_intr_enable(&(pci_dev->intr_handle));
 
+	/* initialize mirror rule list */
+	TAILQ_INIT(&pf->mirror_list);
+
 	return 0;
 
 err_mac_alloc:
@@ -929,6 +938,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_vsi *main_vsi = pf->main_vsi;
+	struct i40e_mirror_rule *p_mirror;
 	int i;
 
 	/* Disable all queues */
@@ -953,6 +963,13 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	/* Set link down */
 	i40e_dev_set_link_down(dev);
 
+	/* Remove all mirror rules */
+	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
+		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
+		rte_free(p_mirror);
+	}
+	pf->nb_mirror_rule = 0;
+
 }
 
 static void
@@ -5697,3 +5714,320 @@ i40e_configure_registers(struct i40e_hw *hw)
 			"0x%"PRIx32, reg_table[i].val, reg_table[i].addr);
 	}
 }
+
+/**
+ * i40e_aq_add_mirror_rule
+ * @hw: pointer to the hardware structure
+ * @seid: VEB seid to add mirror rule to
+ * @dst_id: destination vsi seid
+ * @entries: Buffer which contains the entities to be mirrored
+ * @count: number of entities contained in the buffer
+ * @rule_id:the rule_id of the rule to be added
+ *
+ * Add a mirror rule for a given veb.
+ *
+ **/
+static enum i40e_status_code
+i40e_aq_add_mirror_rule(struct i40e_hw *hw,
+			uint16_t seid, uint16_t dst_id,
+			uint16_t rule_type, uint16_t *entries,
+			uint16_t count, uint16_t *rule_id)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_add_delete_mirror_rule *cmd =
+		(struct i40e_aqc_add_delete_mirror_rule *)&desc.params.raw;
+	struct i40e_aqc_add_delete_mirror_rule_completion *resp =
+		(struct i40e_aqc_add_delete_mirror_rule_completion *)
+		&desc.params.raw;
+	uint16_t buff_len;
+	enum i40e_status_code status;
+
+	i40e_fill_default_direct_cmd_desc(&desc,
+					  i40e_aqc_opc_add_mirror_rule);
+
+	buff_len = sizeof(uint16_t) * count;
+	desc.datalen = rte_cpu_to_le_16(buff_len);
+	if (buff_len > 0)
+		desc.flags |= rte_cpu_to_le_16(
+			(uint16_t)(I40E_AQ_FLAG_BUF | I40E_AQ_FLAG_RD));
+	cmd->rule_type = rte_cpu_to_le_16(rule_type <<
+				I40E_AQC_MIRROR_RULE_TYPE_SHIFT);
+	cmd->num_entries = rte_cpu_to_le_16(count);
+	cmd->seid = rte_cpu_to_le_16(seid);
+	cmd->destination = rte_cpu_to_le_16(dst_id);
+
+	status = i40e_asq_send_command(hw, &desc, entries, buff_len, NULL);
+	PMD_DRV_LOG(INFO, "i40e_aq_add_mirror_rule, aq_status %d,"
+			 "rule_id = %u"
+			 " mirror_rules_used = %u, mirror_rules_free = %u,",
+			 hw->aq.asq_last_status, resp->rule_id,
+			 resp->mirror_rules_used, resp->mirror_rules_free);
+	*rule_id = rte_le_to_cpu_16(resp->rule_id);
+
+	return status;
+}
+
+/**
+ * i40e_aq_del_mirror_rule
+ * @hw: pointer to the hardware structure
+ * @seid: VEB seid to add mirror rule to
+ * @entries: Buffer which contains the entities to be mirrored
+ * @count: number of entities contained in the buffer
+ * @rule_id:the rule_id of the rule to be delete
+ *
+ * Delete a mirror rule for a given veb.
+ *
+ **/
+static enum i40e_status_code
+i40e_aq_del_mirror_rule(struct i40e_hw *hw,
+		uint16_t seid, uint16_t rule_type, uint16_t *entries,
+		uint16_t count, uint16_t rule_id)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_add_delete_mirror_rule *cmd =
+		(struct i40e_aqc_add_delete_mirror_rule *)&desc.params.raw;
+	uint16_t buff_len = 0;
+	enum i40e_status_code status;
+	void *buff = NULL;
+
+	i40e_fill_default_direct_cmd_desc(&desc,
+					  i40e_aqc_opc_delete_mirror_rule);
+
+	if (rule_type == I40E_AQC_MIRROR_RULE_TYPE_VLAN) {
+		desc.flags |= rte_cpu_to_le_16((uint16_t)(I40E_AQ_FLAG_BUF |
+							  I40E_AQ_FLAG_RD));
+		cmd->num_entries = count;
+		buff_len = sizeof(uint16_t) * count;
+		desc.datalen = rte_cpu_to_le_16(buff_len);
+		buff = (void *)entries;
+	} else
+		/* rule id is filled in destination field for deleting mirror rule */
+		cmd->destination = rte_cpu_to_le_16(rule_id);
+
+	cmd->rule_type = rte_cpu_to_le_16(rule_type <<
+				I40E_AQC_MIRROR_RULE_TYPE_SHIFT);
+	cmd->seid = rte_cpu_to_le_16(seid);
+
+	status = i40e_asq_send_command(hw, &desc, buff, buff_len, NULL);
+
+	return status;
+}
+
+/**
+ * i40e_mirror_rule_set
+ * @dev: pointer to the hardware structure
+ * @mirror_conf: mirror rule info
+ * @sw_id: mirror rule's sw_id
+ * @on: enable/disable
+ *
+ * set a mirror rule.
+ *
+ **/
+static int
+i40e_mirror_rule_set(struct rte_eth_dev *dev,
+			struct rte_eth_mirror_conf *mirror_conf,
+			uint8_t sw_id, uint8_t on)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_mirror_rule *it, *mirr_rule = NULL;
+	struct i40e_mirror_rule *parent = NULL;
+	uint16_t seid, dst_seid, rule_id;
+	uint16_t i, j = 0;
+	int ret;
+
+	PMD_DRV_LOG(DEBUG, "i40e_mirror_rule_set: sw_id = %d.", sw_id);
+
+	if (pf->main_vsi->veb == NULL || pf->vfs == NULL) {
+		PMD_DRV_LOG(ERR, "mirror rule can not be configured"
+			" without veb or vfs.");
+		return -ENOSYS;
+	}
+	if (pf->nb_mirror_rule > I40E_MAX_MIRROR_RULES) {
+		PMD_DRV_LOG(ERR, "mirror table is full.");
+		return -ENOSPC;
+	}
+	if (mirror_conf->dst_pool > pf->vf_num) {
+		PMD_DRV_LOG(ERR, "invalid destination pool %u.",
+				 mirror_conf->dst_pool);
+		return -EINVAL;
+	}
+
+	seid = pf->main_vsi->veb->seid;
+
+	TAILQ_FOREACH(it, &pf->mirror_list, rules) {
+		if (sw_id <= it->index) {
+			mirr_rule = it;
+			break;
+		}
+		parent = it;
+	}
+	if (mirr_rule && sw_id == mirr_rule->index) {
+		if (on) {
+			PMD_DRV_LOG(ERR, "mirror rule exists.");
+			return -EEXIST;
+		} else {
+			ret = i40e_aq_del_mirror_rule(hw, seid,
+					mirr_rule->rule_type,
+					mirr_rule->entries,
+					mirr_rule->num_entries, mirr_rule->id);
+			if (ret < 0) {
+				PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
+						   " ret = %d, aq_err = %d.",
+						   ret, hw->aq.asq_last_status);
+				return -ENOSYS;
+			}
+			TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
+			rte_free(mirr_rule);
+			pf->nb_mirror_rule--;
+			return 0;
+		}
+	} else if (!on) {
+		PMD_DRV_LOG(ERR, "mirror rule doesn't exist.");
+		return -ENOENT;
+	}
+
+	mirr_rule = rte_zmalloc("i40e_mirror_rule",
+				sizeof(struct i40e_mirror_rule) , 0);
+	if (!mirr_rule) {
+		PMD_DRV_LOG(ERR, "failed to allocate memory");
+		return I40E_ERR_NO_MEMORY;
+	}
+	switch (mirror_conf->rule_type) {
+	case ETH_MIRROR_VLAN:
+		for (i = 0, j = 0; i < ETH_MIRROR_MAX_VLANS; i++) {
+			if (mirror_conf->vlan.vlan_mask & (1ULL << i)) {
+				mirr_rule->entries[j] =
+					mirror_conf->vlan.vlan_id[i];
+				j++;
+			}
+		}
+		if (j == 0) {
+			PMD_DRV_LOG(ERR, "vlan is not specified.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_VLAN;
+		break;
+	case ETH_MIRROR_VIRTUAL_POOL_UP:
+	case ETH_MIRROR_VIRTUAL_POOL_DOWN:
+		/* check if the specified pool bit is out of range */
+		if (mirror_conf->pool_mask > (uint64_t)(1ULL << (pf->vf_num + 1))) {
+			PMD_DRV_LOG(ERR, "pool mask is out of range.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		for (i = 0, j = 0; i < pf->vf_num; i++) {
+			if (mirror_conf->pool_mask & (1ULL << i)) {
+				mirr_rule->entries[j] = pf->vfs[i].vsi->seid;
+				j++;
+			}
+		}
+		if (mirror_conf->pool_mask & (1ULL << pf->vf_num)) {
+			/* add pf vsi to entries */
+			mirr_rule->entries[j] = pf->main_vsi_seid;
+			j++;
+		}
+		if (j == 0) {
+			PMD_DRV_LOG(ERR, "pool is not specified.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		/* egress and ingress in aq commands means from switch but not port */
+		mirr_rule->rule_type =
+			(mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP) ?
+			I40E_AQC_MIRROR_RULE_TYPE_VPORT_EGRESS :
+			I40E_AQC_MIRROR_RULE_TYPE_VPORT_INGRESS;
+		break;
+	case ETH_MIRROR_UPLINK_PORT:
+		/* egress and ingress in aq commands means from switch but not port*/
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_ALL_EGRESS;
+		break;
+	case ETH_MIRROR_DOWNLINK_PORT:
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_ALL_INGRESS;
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "unsupported mirror type %d.",
+			mirror_conf->rule_type);
+		rte_free(mirr_rule);
+		return -EINVAL;
+	}
+
+	/* If the dst_pool is equal to vf_num, consider it as PF */
+	if (mirror_conf->dst_pool == pf->vf_num)
+		dst_seid = pf->main_vsi_seid;
+	else
+		dst_seid = pf->vfs[mirror_conf->dst_pool].vsi->seid;
+
+	ret = i40e_aq_add_mirror_rule(hw, seid, dst_seid,
+				      mirr_rule->rule_type, mirr_rule->entries,
+				      j, &rule_id);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "failed to add mirror rule:"
+				   " ret = %d, aq_err = %d.",
+				   ret, hw->aq.asq_last_status);
+		rte_free(mirr_rule);
+		return -ENOSYS;
+	}
+
+	mirr_rule->index = sw_id;
+	mirr_rule->num_entries = j;
+	mirr_rule->id = rule_id;
+	mirr_rule->dst_vsi_seid = dst_seid;
+
+	if (parent)
+		TAILQ_INSERT_AFTER(&pf->mirror_list, parent, mirr_rule, rules);
+	else
+		TAILQ_INSERT_HEAD(&pf->mirror_list, mirr_rule, rules);
+
+	pf->nb_mirror_rule++;
+	return 0;
+}
+
+/**
+ * i40e_mirror_rule_reset
+ * @dev: pointer to the device
+ * @sw_id: mirror rule's sw_id
+ *
+ * reset a mirror rule.
+ *
+ **/
+static int
+i40e_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t sw_id)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_mirror_rule *it, *mirr_rule = NULL;
+	uint16_t seid;
+	int ret;
+
+	PMD_DRV_LOG(DEBUG, "i40e_mirror_rule_reset: sw_id = %d.", sw_id);
+
+	seid = pf->main_vsi->veb->seid;
+
+	TAILQ_FOREACH(it, &pf->mirror_list, rules) {
+		if (sw_id == it->index) {
+			mirr_rule = it;
+			break;
+		}
+	}
+	if (mirr_rule) {
+		ret = i40e_aq_del_mirror_rule(hw, seid,
+				mirr_rule->rule_type,
+				mirr_rule->entries,
+				mirr_rule->num_entries, mirr_rule->id);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
+					   " status = %d, aq_err = %d.",
+					   ret, hw->aq.asq_last_status);
+			return -ENOSYS;
+		}
+		TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
+		rte_free(mirr_rule);
+		pf->nb_mirror_rule--;
+	} else {
+		PMD_DRV_LOG(ERR, "mirror rule doesn't exist.");
+		return -ENOENT;
+	}
+	return 0;
+}
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index baac4a0..723d7a4 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -323,6 +323,27 @@ struct i40e_fdir_info {
 	struct i40e_fdir_flex_mask flex_mask[I40E_FILTER_PCTYPE_MAX];
 };
 
+#define I40E_MIRROR_MAX_ENTRIES_PER_RULE   64
+#define I40E_MAX_MIRROR_RULES           64
+/*
+ * Mirror rule structure
+ */
+struct i40e_mirror_rule {
+	TAILQ_ENTRY(i40e_mirror_rule) rules;
+	uint8_t rule_type;
+	uint16_t index;          /* the sw index of mirror rule */
+	uint16_t id;             /* the rule id assigned by firmware */
+	uint16_t dst_vsi_seid;   /* destination vsi for this mirror rule. */
+	uint16_t num_entries;
+	/* the info stores depend on the rule type.
+	    If type is I40E_MIRROR_TYPE_VLAN, vlan ids are stored here.
+	    If type is I40E_MIRROR_TYPE_VPORT_*, vsi's seid are stored.
+	 */
+	uint16_t entries[I40E_MIRROR_MAX_ENTRIES_PER_RULE];
+};
+
+TAILQ_HEAD(i40e_mirror_rule_list, i40e_mirror_rule);
+
 /*
  * Structure to store private data specific for PF instance.
  */
@@ -362,6 +383,8 @@ struct i40e_pf {
 	struct i40e_vmdq_info *vmdq;
 
 	struct i40e_fdir_info fdir; /* flow director info */
+	struct i40e_mirror_rule_list mirror_list;
+	uint16_t nb_mirror_rule;   /* The number of mirror rules */
 };
 
 enum pending_msg {
-- 
1.9.3

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

* [dpdk-dev] [PATCH v3 4/4] doc: modify the command about mirror in testpmd guide
  2015-06-05  8:16   ` [dpdk-dev] [PATCH v3 0/4] enable mirror functionality in i40e driver Jingjing Wu
                       ` (2 preceding siblings ...)
  2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 3/4] i40e: enable mirror functionality in i40e driver Jingjing Wu
@ 2015-06-05  8:16     ` Jingjing Wu
  2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
  4 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-05  8:16 UTC (permalink / raw)
  To: dev

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 761172e..2cd3461 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -772,9 +772,13 @@ set port (port_id) vf (vf_id) rate (rate_value) queue_mask (queue_mask)
 set port - mirror rule
 ~~~~~~~~~~~~~~~~~~~~~~
 
-Set port or vlan type mirror rule for a port.
+Set pool or vlan type mirror rule for a port:
 
-set port (port_id) mirror-rule (rule_id) (pool-mirror|vlan-mirror) (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)
+set port (port_id) mirror-rule (rule_id) (pool-mirror-up|pool-mirror-down|vlan-mirror) (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)
+
+Set link mirror rule for a port:
+
+set port (port_id) mirror-rule (rule_id) (uplink-mirror|downlink-mirror) dst-pool (pool_id) (on|off)
 
 For example to enable mirror traffic with vlan 0,1 to pool 0:
 
-- 
1.9.3

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

* [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver
  2015-06-05  8:16   ` [dpdk-dev] [PATCH v3 0/4] enable mirror functionality in i40e driver Jingjing Wu
                       ` (3 preceding siblings ...)
  2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 4/4] doc: modify the command about mirror in testpmd guide Jingjing Wu
@ 2015-06-10  6:24     ` Jingjing Wu
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
                         ` (6 more replies)
  4 siblings, 7 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-10  6:24 UTC (permalink / raw)
  To: dev

This patch set enables mirror functionality in i40e driver, and redefines structure and macros used to configure mirror.
 
v2 changes:
 - correct comments style
 - add doc change
 
v3 changes:
 - change the mirror rule type to support bit mask
 - fix code style

v4 changes:
 - correct the rule type check ixgbe

Jingjing Wu (4):
  ethdev: rename rte_eth_vmdq_mirror_conf
  ethdev: rename and extend the mirror type
  i40e: enable mirror functionality in i40e driver
  doc: modify the command about mirror in testpmd guide

 app/test-pmd/cmdline.c                      |  62 +++---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
 drivers/net/i40e/i40e_ethdev.c              | 334 ++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_ethdev.h              |  23 ++
 drivers/net/ixgbe/ixgbe_ethdev.c            |  62 ++++--
 drivers/net/ixgbe/ixgbe_ethdev.h            |   4 +-
 lib/librte_ether/rte_ethdev.c               |  28 +--
 lib/librte_ether/rte_ethdev.h               |  30 +--
 8 files changed, 466 insertions(+), 85 deletions(-)

-- 
1.9.3

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

* [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
@ 2015-06-10  6:24       ` Jingjing Wu
  2015-06-26  7:03         ` Wu, Jingjing
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 2/4] ethdev: rename and extend the mirror type Jingjing Wu
                         ` (5 subsequent siblings)
  6 siblings, 1 reply; 30+ messages in thread
From: Jingjing Wu @ 2015-06-10  6:24 UTC (permalink / raw)
  To: dev

rename rte_eth_vmdq_mirror_conf to rte_eth_mirror_conf and move
the maximum rule id check from ethdev level to driver

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 app/test-pmd/cmdline.c           | 22 +++++++++++-----------
 drivers/net/ixgbe/ixgbe_ethdev.c | 11 +++++++----
 drivers/net/ixgbe/ixgbe_ethdev.h |  4 +++-
 lib/librte_ether/rte_ethdev.c    | 18 ++----------------
 lib/librte_ether/rte_ethdev.h    | 19 ++++++++++---------
 5 files changed, 33 insertions(+), 41 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f01db2a..d693bde 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -6604,11 +6604,11 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 {
 	int ret,nb_item,i;
 	struct cmd_set_mirror_mask_result *res = parsed_result;
-	struct rte_eth_vmdq_mirror_conf mr_conf;
+	struct rte_eth_mirror_conf mr_conf;
 
-	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
+	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
 
-	unsigned int vlan_list[ETH_VMDQ_MAX_VLAN_FILTERS];
+	unsigned int vlan_list[ETH_MIRROR_MAX_VLANS];
 
 	mr_conf.dst_pool = res->dstpool_id;
 
@@ -6618,11 +6618,11 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 	} else if(!strcmp(res->what, "vlan-mirror")) {
 		mr_conf.rule_type_mask = ETH_VMDQ_VLAN_MIRROR;
 		nb_item = parse_item_list(res->value, "core",
-					ETH_VMDQ_MAX_VLAN_FILTERS,vlan_list,1);
+					ETH_MIRROR_MAX_VLANS, vlan_list, 1);
 		if (nb_item <= 0)
 			return;
 
-		for(i=0; i < nb_item; i++) {
+		for (i = 0; i < nb_item; i++) {
 			if (vlan_list[i] > ETHER_MAX_VLAN_ID) {
 				printf("Invalid vlan_id: must be < 4096\n");
 				return;
@@ -6634,10 +6634,10 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 	}
 
 	if(!strcmp(res->on, "on"))
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
 	if(ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
@@ -6711,9 +6711,9 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 {
 	int ret;
 	struct cmd_set_mirror_link_result *res = parsed_result;
-	struct rte_eth_vmdq_mirror_conf mr_conf;
+	struct rte_eth_mirror_conf mr_conf;
 
-	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
+	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
 	if(!strcmp(res->what, "uplink-mirror")) {
 		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
 	}else if(!strcmp(res->what, "downlink-mirror"))
@@ -6722,10 +6722,10 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 	mr_conf.dst_pool = res->dstpool_id;
 
 	if(!strcmp(res->on, "on"))
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
-		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
+		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
 
 	/* check the return value and print it if is < 0 */
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0d9f9b2..9e767fa 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -209,7 +209,7 @@ static int ixgbe_set_pool_tx(struct rte_eth_dev *dev,uint16_t pool,uint8_t on);
 static int ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 		uint64_t pool_mask,uint8_t vlan_on);
 static int ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
-		struct rte_eth_vmdq_mirror_conf *mirror_conf,
+		struct rte_eth_mirror_conf *mirror_conf,
 		uint8_t rule_id, uint8_t on);
 static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev,
 		uint8_t	rule_id);
@@ -3388,7 +3388,7 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 
 static int
 ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id, uint8_t on)
 {
 	uint32_t mr_ctl,vlvf;
@@ -3412,7 +3412,10 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
 	if (ixgbe_vmdq_mode_check(hw) < 0)
-		return (-ENOTSUP);
+		return -ENOTSUP;
+
+	if (rule_id >= IXGBE_MAX_MIRROR_RULES)
+		return -EINVAL;
 
 	/* Check if vlan mask is valid */
 	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) && (on)) {
@@ -3526,7 +3529,7 @@ ixgbe_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t rule_id)
 		return (-ENOTSUP);
 
 	memset(&mr_info->mr_conf[rule_id], 0,
-		sizeof(struct rte_eth_vmdq_mirror_conf));
+		sizeof(struct rte_eth_mirror_conf));
 
 	/* clear PFVMCTL register */
 	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl);
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 19237b8..755b674 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -177,8 +177,10 @@ struct ixgbe_uta_info {
 	uint32_t uta_shadow[IXGBE_MAX_UTA];
 };
 
+#define IXGBE_MAX_MIRROR_RULES 4  /* Maximum nb. of mirror rules. */
+
 struct ixgbe_mirror_info {
-	struct rte_eth_vmdq_mirror_conf mr_conf[ETH_VMDQ_NUM_MIRROR_RULE];
+	struct rte_eth_mirror_conf mr_conf[IXGBE_MAX_MIRROR_RULES];
 	/**< store PF mirror rules configuration*/
 };
 
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 024fe8b..43c7295 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3034,7 +3034,7 @@ int rte_eth_set_vf_rate_limit(uint8_t port_id, uint16_t vf, uint16_t tx_rate,
 
 int
 rte_eth_mirror_rule_set(uint8_t port_id,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id, uint8_t on)
 {
 	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
@@ -3051,7 +3051,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 
 	if (mirror_conf->dst_pool >= ETH_64_POOLS) {
 		PMD_DEBUG_TRACE("Invalid dst pool, pool id must"
-			"be 0-%d\n",ETH_64_POOLS - 1);
+			"be 0-%d\n", ETH_64_POOLS - 1);
 		return -EINVAL;
 	}
 
@@ -3062,13 +3062,6 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -EINVAL;
 	}
 
-	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
-	{
-		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-%d\n",
-			ETH_VMDQ_NUM_MIRROR_RULE - 1);
-		return -EINVAL;
-	}
-
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
 
@@ -3085,13 +3078,6 @@ rte_eth_mirror_rule_reset(uint8_t port_id, uint8_t rule_id)
 		return -ENODEV;
 	}
 
-	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
-	{
-		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-%d\n",
-			ETH_VMDQ_NUM_MIRROR_RULE-1);
-		return -EINVAL;
-	}
-
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_reset, -ENOTSUP);
 
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 16dbe00..ae22fea 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -467,8 +467,8 @@ struct rte_eth_rss_conf {
 #define ETH_VMDQ_ACCEPT_BROADCAST   0x0008 /**< accept broadcast packets. */
 #define ETH_VMDQ_ACCEPT_MULTICAST   0x0010 /**< multicast promiscuous. */
 
-/* Definitions used for VMDQ mirror rules setting */
-#define ETH_VMDQ_NUM_MIRROR_RULE     4 /**< Maximum nb. of mirror rules. . */
+/** Maximum nb. of vlan per mirror rule */
+#define ETH_MIRROR_MAX_VLANS       64
 
 #define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring. */
 #define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring. */
@@ -480,18 +480,19 @@ struct rte_eth_rss_conf {
  */
 struct rte_eth_vlan_mirror {
 	uint64_t vlan_mask; /**< mask for valid VLAN ID. */
-	uint16_t vlan_id[ETH_VMDQ_MAX_VLAN_FILTERS];
-	/** VLAN ID list for vlan mirror. */
+	/** VLAN ID list for vlan mirroring. */
+	uint16_t vlan_id[ETH_MIRROR_MAX_VLANS];
 };
 
 /**
  * A structure used to configure traffic mirror of an Ethernet port.
  */
-struct rte_eth_vmdq_mirror_conf {
+struct rte_eth_mirror_conf {
 	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to set */
-	uint8_t dst_pool; /**< Destination pool for this mirror rule. */
+	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
 	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
-	struct rte_eth_vlan_mirror vlan; /**< VLAN ID setting for VLAN mirroring */
+	/** VLAN ID setting for VLAN mirroring. */
+	struct rte_eth_vlan_mirror vlan;
 };
 
 /**
@@ -1211,7 +1212,7 @@ typedef int (*eth_set_vf_rate_limit_t)(struct rte_eth_dev *dev,
 /**< @internal Set VF TX rate */
 
 typedef int (*eth_mirror_rule_set_t)(struct rte_eth_dev *dev,
-				  struct rte_eth_vmdq_mirror_conf *mirror_conf,
+				  struct rte_eth_mirror_conf *mirror_conf,
 				  uint8_t rule_id,
 				  uint8_t on);
 /**< @internal Add a traffic mirroring rule on an Ethernet device */
@@ -3168,7 +3169,7 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
  *   - (-EINVAL) if the mr_conf information is not correct.
  */
 int rte_eth_mirror_rule_set(uint8_t port_id,
-			struct rte_eth_vmdq_mirror_conf *mirror_conf,
+			struct rte_eth_mirror_conf *mirror_conf,
 			uint8_t rule_id,
 			uint8_t on);
 
-- 
1.9.3

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

* [dpdk-dev] [PATCH v4 2/4] ethdev: rename and extend the mirror type
  2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
@ 2015-06-10  6:24       ` Jingjing Wu
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 3/4] i40e: enable mirror functionality in i40e driver Jingjing Wu
                         ` (4 subsequent siblings)
  6 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-10  6:24 UTC (permalink / raw)
  To: dev

This path renames the mirror type in rte_eth_mirror_conf and macros,
and rework the mirror set in ixgbe dirvers by using new definition.
It also fixes some coding style.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 app/test-pmd/cmdline.c           | 42 ++++++++++++++++++---------------
 drivers/net/ixgbe/ixgbe_ethdev.c | 51 ++++++++++++++++++++++++++--------------
 lib/librte_ether/rte_ethdev.c    | 14 ++++++++---
 lib/librte_ether/rte_ethdev.h    | 11 +++++----
 4 files changed, 73 insertions(+), 45 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index d693bde..6d4474b 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -412,7 +412,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Set rate limit for queues in VF of a port\n\n"
 
 			"set port (port_id) mirror-rule (rule_id)"
-			"(pool-mirror|vlan-mirror)\n"
+			" (pool-mirror-up|pool-mirror-down|vlan-mirror)"
 			" (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)\n"
 			"   Set pool or vlan type mirror rule on a port.\n"
 			"   e.g., 'set port 0 mirror-rule 0 vlan-mirror 0,1"
@@ -6583,7 +6583,8 @@ cmdline_parse_token_num_t cmd_mirror_mask_ruleid =
 				rule_id, UINT8);
 cmdline_parse_token_string_t cmd_mirror_mask_what =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_mirror_mask_result,
-				what, "pool-mirror#vlan-mirror");
+				what, "pool-mirror-up#pool-mirror-down"
+				      "#vlan-mirror");
 cmdline_parse_token_string_t cmd_mirror_mask_value =
 	TOKEN_STRING_INITIALIZER(struct cmd_set_mirror_mask_result,
 				value, NULL);
@@ -6612,13 +6613,16 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 
 	mr_conf.dst_pool = res->dstpool_id;
 
-	if (!strcmp(res->what, "pool-mirror")) {
-		mr_conf.pool_mask = strtoull(res->value,NULL,16);
-		mr_conf.rule_type_mask = ETH_VMDQ_POOL_MIRROR;
-	} else if(!strcmp(res->what, "vlan-mirror")) {
-		mr_conf.rule_type_mask = ETH_VMDQ_VLAN_MIRROR;
-		nb_item = parse_item_list(res->value, "core",
-					ETH_MIRROR_MAX_VLANS, vlan_list, 1);
+	if (!strcmp(res->what, "pool-mirror-up")) {
+		mr_conf.pool_mask = strtoull(res->value, NULL, 16);
+		mr_conf.rule_type = ETH_MIRROR_VIRTUAL_POOL_UP;
+	} else if (!strcmp(res->what, "pool-mirror-down")) {
+		mr_conf.pool_mask = strtoull(res->value, NULL, 16);
+		mr_conf.rule_type = ETH_MIRROR_VIRTUAL_POOL_DOWN;
+	} else if (!strcmp(res->what, "vlan-mirror")) {
+		mr_conf.rule_type = ETH_MIRROR_VLAN;
+		nb_item = parse_item_list(res->value, "vlan",
+				ETH_MIRROR_MAX_VLANS, vlan_list, 1);
 		if (nb_item <= 0)
 			return;
 
@@ -6633,21 +6637,21 @@ cmd_set_mirror_mask_parsed(void *parsed_result,
 		}
 	}
 
-	if(!strcmp(res->on, "on"))
+	if (!strcmp(res->on, "on"))
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 0);
-	if(ret < 0)
+	if (ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
 }
 
 cmdline_parse_inst_t cmd_set_mirror_mask = {
 		.f = cmd_set_mirror_mask_parsed,
 		.data = NULL,
-		.help_str = "set port X mirror-rule Y pool-mirror|vlan-mirror "
-				"pool_mask|vlan_id[,vlan_id]* dst-pool Z on|off",
+		.help_str = "set port X mirror-rule Y pool-mirror-up|pool-mirror-down|vlan-mirror"
+			    " pool_mask|vlan_id[,vlan_id]* dst-pool Z on|off",
 		.tokens = {
 			(void *)&cmd_mirror_mask_set,
 			(void *)&cmd_mirror_mask_port,
@@ -6714,14 +6718,14 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 	struct rte_eth_mirror_conf mr_conf;
 
 	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
-	if(!strcmp(res->what, "uplink-mirror")) {
-		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
-	}else if(!strcmp(res->what, "downlink-mirror"))
-		mr_conf.rule_type_mask = ETH_VMDQ_DOWNLIN_MIRROR;
+	if (!strcmp(res->what, "uplink-mirror"))
+		mr_conf.rule_type = ETH_MIRROR_UPLINK_PORT;
+	else
+		mr_conf.rule_type = ETH_MIRROR_DOWNLINK_PORT;
 
 	mr_conf.dst_pool = res->dstpool_id;
 
-	if(!strcmp(res->on, "on"))
+	if (!strcmp(res->on, "on"))
 		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
 						res->rule_id, 1);
 	else
@@ -6729,7 +6733,7 @@ cmd_set_mirror_link_parsed(void *parsed_result,
 						res->rule_id, 0);
 
 	/* check the return value and print it if is < 0 */
-	if(ret < 0)
+	if (ret < 0)
 		printf("mirror rule add error: (%s)\n", strerror(-ret));
 
 }
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 9e767fa..3573493 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3386,6 +3386,14 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
 	return ret;
 }
 
+#define IXGBE_MRCTL_VPME  0x01 /* Virtual Pool Mirroring. */
+#define IXGBE_MRCTL_UPME  0x02 /* Uplink Port Mirroring. */
+#define IXGBE_MRCTL_DPME  0x04 /* Downlink Port Mirroring. */
+#define IXGBE_MRCTL_VLME  0x08 /* VLAN Mirroring. */
+#define IXGBE_INVALID_MIRROR_TYPE(mirror_type) \
+	((mirror_type) & ~(uint8_t)(ETH_MIRROR_VIRTUAL_POOL_UP | \
+	ETH_MIRROR_UPLINK_PORT | ETH_MIRROR_DOWNLINK_PORT | ETH_MIRROR_VLAN))
+
 static int
 ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			struct rte_eth_mirror_conf *mirror_conf,
@@ -3410,6 +3418,7 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			(IXGBE_DEV_PRIVATE_TO_PFDATA(dev->data->dev_private));
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint8_t mirror_type = 0;
 
 	if (ixgbe_vmdq_mode_check(hw) < 0)
 		return -ENOTSUP;
@@ -3417,28 +3426,29 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 	if (rule_id >= IXGBE_MAX_MIRROR_RULES)
 		return -EINVAL;
 
-	/* Check if vlan mask is valid */
-	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) && (on)) {
-		if (mirror_conf->vlan.vlan_mask == 0)
-			return (-EINVAL);
+	if (IXGBE_INVALID_MIRROR_TYPE(mirror_conf->rule_type)) {
+		PMD_DRV_LOG(ERR, "unsupported mirror type 0x%x.",
+			mirror_conf->rule_type);
+		return -EINVAL;
 	}
 
-	/* Check if vlan id is valid and find conresponding VLAN ID index in VLVF */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) {
+	if (mirror_conf->rule_type & ETH_MIRROR_VLAN) {
+		mirror_type |= IXGBE_MRCTL_VLME;
+		/* Check if vlan id is valid and find conresponding VLAN ID index in VLVF */
 		for (i = 0;i < IXGBE_VLVF_ENTRIES; i++) {
 			if (mirror_conf->vlan.vlan_mask & (1ULL << i)) {
 				/* search vlan id related pool vlan filter index */
 				reg_index = ixgbe_find_vlvf_slot(hw,
 						mirror_conf->vlan.vlan_id[i]);
 				if(reg_index < 0)
-					return (-EINVAL);
+					return -EINVAL;
 				vlvf = IXGBE_READ_REG(hw, IXGBE_VLVF(reg_index));
 				if ((vlvf & IXGBE_VLVF_VIEN) &&
-					((vlvf & IXGBE_VLVF_VLANID_MASK)
-						== mirror_conf->vlan.vlan_id[i]))
+				    ((vlvf & IXGBE_VLVF_VLANID_MASK) ==
+				      mirror_conf->vlan.vlan_id[i]))
 					vlan_mask |= (1ULL << reg_index);
 				else
-					return (-EINVAL);
+					return -EINVAL;
 			}
 		}
 
@@ -3466,7 +3476,8 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 	 * if enable pool mirror, write related pool mask register,if disable
 	 * pool mirror, clear PFMRVM register
 	 */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) {
+	if (mirror_conf->rule_type & ETH_MIRROR_VIRTUAL_POOL_UP) {
+		mirror_type |= IXGBE_MRCTL_VPME;
 		if (on) {
 			mp_lsb = mirror_conf->pool_mask & 0xFFFFFFFF;
 			mp_msb = mirror_conf->pool_mask >> pool_mask_offset;
@@ -3479,31 +3490,35 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
 			mr_info->mr_conf[rule_id].pool_mask = 0;
 		}
 	}
+	if (mirror_conf->rule_type & ETH_MIRROR_UPLINK_PORT)
+		mirror_type |= IXGBE_MRCTL_UPME;
+	if (mirror_conf->rule_type & ETH_MIRROR_DOWNLINK_PORT)
+		mirror_type |= IXGBE_MRCTL_DPME;
 
 	/* read  mirror control register and recalculate it */
-	mr_ctl = IXGBE_READ_REG(hw,IXGBE_MRCTL(rule_id));
+	mr_ctl = IXGBE_READ_REG(hw, IXGBE_MRCTL(rule_id));
 
 	if (on) {
-		mr_ctl |= mirror_conf->rule_type_mask;
+		mr_ctl |= mirror_type;
 		mr_ctl &= mirror_rule_mask;
 		mr_ctl |= mirror_conf->dst_pool << dst_pool_offset;
 	} else
-		mr_ctl &= ~(mirror_conf->rule_type_mask & mirror_rule_mask);
+		mr_ctl &= ~(mirror_conf->rule_type & mirror_rule_mask);
 
-	mr_info->mr_conf[rule_id].rule_type_mask = (uint8_t)(mr_ctl & mirror_rule_mask);
+	mr_info->mr_conf[rule_id].rule_type = mirror_conf->rule_type;
 	mr_info->mr_conf[rule_id].dst_pool = mirror_conf->dst_pool;
 
 	/* write mirrror control  register */
 	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl);
 
-        /* write pool mirrror control  register */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) {
+	/* write pool mirrror control  register */
+	if (mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP) {
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVM(rule_id), mp_lsb);
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVM(rule_id + rule_mr_offset),
 				mp_msb);
 	}
 	/* write VLAN mirrror control  register */
-	if (mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR) {
+	if (mirror_conf->rule_type == ETH_MIRROR_VLAN) {
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVLAN(rule_id), mv_lsb);
 		IXGBE_WRITE_REG(hw, IXGBE_VMRVLAN(rule_id + rule_mr_offset),
 				mv_msb);
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 43c7295..34f1900 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3044,7 +3044,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -ENODEV;
 	}
 
-	if (mirror_conf->rule_type_mask == 0) {
+	if (mirror_conf->rule_type == 0) {
 		PMD_DEBUG_TRACE("mirror rule type can not be 0.\n");
 		return -EINVAL;
 	}
@@ -3055,12 +3055,20 @@ rte_eth_mirror_rule_set(uint8_t port_id,
 		return -EINVAL;
 	}
 
-	if ((mirror_conf->rule_type_mask & ETH_VMDQ_POOL_MIRROR) &&
-		(mirror_conf->pool_mask == 0)) {
+	if ((mirror_conf->rule_type & (ETH_MIRROR_VIRTUAL_POOL_UP |
+	     ETH_MIRROR_VIRTUAL_POOL_DOWN)) &&
+	    (mirror_conf->pool_mask == 0)) {
 		PMD_DEBUG_TRACE("Invalid mirror pool, pool mask can not"
 				"be 0.\n");
 		return -EINVAL;
 	}
+	/* Check if vlan mask is valid */
+	if ((mirror_conf->rule_type & ETH_MIRROR_VLAN) &&
+	    mirror_conf->vlan.vlan_mask == 0) {
+		PMD_DEBUG_TRACE("Invalid vlan mask, vlan mask can not"
+				"be 0.\n");
+		return -EINVAL;
+	}
 
 	dev = &rte_eth_devices[port_id];
 	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index ae22fea..f0496f0 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -470,10 +470,11 @@ struct rte_eth_rss_conf {
 /** Maximum nb. of vlan per mirror rule */
 #define ETH_MIRROR_MAX_VLANS       64
 
-#define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring. */
-#define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring. */
-#define ETH_VMDQ_DOWNLIN_MIRROR 0x0004 /**< Downlink Port Mirroring. */
-#define ETH_VMDQ_VLAN_MIRROR    0x0008 /**< VLAN Mirroring. */
+#define ETH_MIRROR_VIRTUAL_POOL_UP     0x01  /**< Virtual Pool uplink Mirroring. */
+#define ETH_MIRROR_UPLINK_PORT         0x02  /**< Uplink Port Mirroring. */
+#define ETH_MIRROR_DOWNLINK_PORT       0x04  /**< Downlink Port Mirroring. */
+#define ETH_MIRROR_VLAN                0x08  /**< VLAN Mirroring. */
+#define ETH_MIRROR_VIRTUAL_POOL_DOWN   0x10  /**< Virtual Pool downlink Mirroring. */
 
 /**
  * A structure used to configure VLAN traffic mirror of an Ethernet port.
@@ -488,7 +489,7 @@ struct rte_eth_vlan_mirror {
  * A structure used to configure traffic mirror of an Ethernet port.
  */
 struct rte_eth_mirror_conf {
-	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to set */
+	uint8_t rule_type; /**< Mirroring rule type */
 	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
 	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
 	/** VLAN ID setting for VLAN mirroring. */
-- 
1.9.3

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

* [dpdk-dev] [PATCH v4 3/4] i40e: enable mirror functionality in i40e driver
  2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 2/4] ethdev: rename and extend the mirror type Jingjing Wu
@ 2015-06-10  6:24       ` Jingjing Wu
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 4/4] doc: modify the command about mirror in testpmd guide Jingjing Wu
                         ` (3 subsequent siblings)
  6 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-10  6:24 UTC (permalink / raw)
  To: dev

enable mirror functionality in i40e driver
.mirror_rule_set
.mirror_rule_reset

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 334 +++++++++++++++++++++++++++++++++++++++++
 drivers/net/i40e/i40e_ethdev.h |  23 +++
 2 files changed, 357 insertions(+)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index da6c0b5..d0e83b5 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -211,6 +211,10 @@ static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 				void *arg);
 static void i40e_configure_registers(struct i40e_hw *hw);
 static void i40e_hw_init(struct i40e_hw *hw);
+static int i40e_mirror_rule_set(struct rte_eth_dev *dev,
+			struct rte_eth_mirror_conf *mirror_conf,
+			uint8_t sw_id, uint8_t on);
+static int i40e_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t sw_id);
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
 #define RTE_PCI_DEV_ID_DECL_I40E(vend, dev) {RTE_PCI_DEVICE(vend, dev)},
@@ -262,6 +266,8 @@ static const struct eth_dev_ops i40e_eth_dev_ops = {
 	.udp_tunnel_add               = i40e_dev_udp_tunnel_add,
 	.udp_tunnel_del               = i40e_dev_udp_tunnel_del,
 	.filter_ctrl                  = i40e_dev_filter_ctrl,
+	.mirror_rule_set              = i40e_mirror_rule_set,
+	.mirror_rule_reset            = i40e_mirror_rule_reset,
 };
 
 static struct eth_driver rte_i40e_pmd = {
@@ -563,6 +569,9 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
 	/* enable uio intr after callback register */
 	rte_intr_enable(&(pci_dev->intr_handle));
 
+	/* initialize mirror rule list */
+	TAILQ_INIT(&pf->mirror_list);
+
 	return 0;
 
 err_mac_alloc:
@@ -929,6 +938,7 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct i40e_vsi *main_vsi = pf->main_vsi;
+	struct i40e_mirror_rule *p_mirror;
 	int i;
 
 	/* Disable all queues */
@@ -953,6 +963,13 @@ i40e_dev_stop(struct rte_eth_dev *dev)
 	/* Set link down */
 	i40e_dev_set_link_down(dev);
 
+	/* Remove all mirror rules */
+	while ((p_mirror = TAILQ_FIRST(&pf->mirror_list))) {
+		TAILQ_REMOVE(&pf->mirror_list, p_mirror, rules);
+		rte_free(p_mirror);
+	}
+	pf->nb_mirror_rule = 0;
+
 }
 
 static void
@@ -5697,3 +5714,320 @@ i40e_configure_registers(struct i40e_hw *hw)
 			"0x%"PRIx32, reg_table[i].val, reg_table[i].addr);
 	}
 }
+
+/**
+ * i40e_aq_add_mirror_rule
+ * @hw: pointer to the hardware structure
+ * @seid: VEB seid to add mirror rule to
+ * @dst_id: destination vsi seid
+ * @entries: Buffer which contains the entities to be mirrored
+ * @count: number of entities contained in the buffer
+ * @rule_id:the rule_id of the rule to be added
+ *
+ * Add a mirror rule for a given veb.
+ *
+ **/
+static enum i40e_status_code
+i40e_aq_add_mirror_rule(struct i40e_hw *hw,
+			uint16_t seid, uint16_t dst_id,
+			uint16_t rule_type, uint16_t *entries,
+			uint16_t count, uint16_t *rule_id)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_add_delete_mirror_rule *cmd =
+		(struct i40e_aqc_add_delete_mirror_rule *)&desc.params.raw;
+	struct i40e_aqc_add_delete_mirror_rule_completion *resp =
+		(struct i40e_aqc_add_delete_mirror_rule_completion *)
+		&desc.params.raw;
+	uint16_t buff_len;
+	enum i40e_status_code status;
+
+	i40e_fill_default_direct_cmd_desc(&desc,
+					  i40e_aqc_opc_add_mirror_rule);
+
+	buff_len = sizeof(uint16_t) * count;
+	desc.datalen = rte_cpu_to_le_16(buff_len);
+	if (buff_len > 0)
+		desc.flags |= rte_cpu_to_le_16(
+			(uint16_t)(I40E_AQ_FLAG_BUF | I40E_AQ_FLAG_RD));
+	cmd->rule_type = rte_cpu_to_le_16(rule_type <<
+				I40E_AQC_MIRROR_RULE_TYPE_SHIFT);
+	cmd->num_entries = rte_cpu_to_le_16(count);
+	cmd->seid = rte_cpu_to_le_16(seid);
+	cmd->destination = rte_cpu_to_le_16(dst_id);
+
+	status = i40e_asq_send_command(hw, &desc, entries, buff_len, NULL);
+	PMD_DRV_LOG(INFO, "i40e_aq_add_mirror_rule, aq_status %d,"
+			 "rule_id = %u"
+			 " mirror_rules_used = %u, mirror_rules_free = %u,",
+			 hw->aq.asq_last_status, resp->rule_id,
+			 resp->mirror_rules_used, resp->mirror_rules_free);
+	*rule_id = rte_le_to_cpu_16(resp->rule_id);
+
+	return status;
+}
+
+/**
+ * i40e_aq_del_mirror_rule
+ * @hw: pointer to the hardware structure
+ * @seid: VEB seid to add mirror rule to
+ * @entries: Buffer which contains the entities to be mirrored
+ * @count: number of entities contained in the buffer
+ * @rule_id:the rule_id of the rule to be delete
+ *
+ * Delete a mirror rule for a given veb.
+ *
+ **/
+static enum i40e_status_code
+i40e_aq_del_mirror_rule(struct i40e_hw *hw,
+		uint16_t seid, uint16_t rule_type, uint16_t *entries,
+		uint16_t count, uint16_t rule_id)
+{
+	struct i40e_aq_desc desc;
+	struct i40e_aqc_add_delete_mirror_rule *cmd =
+		(struct i40e_aqc_add_delete_mirror_rule *)&desc.params.raw;
+	uint16_t buff_len = 0;
+	enum i40e_status_code status;
+	void *buff = NULL;
+
+	i40e_fill_default_direct_cmd_desc(&desc,
+					  i40e_aqc_opc_delete_mirror_rule);
+
+	if (rule_type == I40E_AQC_MIRROR_RULE_TYPE_VLAN) {
+		desc.flags |= rte_cpu_to_le_16((uint16_t)(I40E_AQ_FLAG_BUF |
+							  I40E_AQ_FLAG_RD));
+		cmd->num_entries = count;
+		buff_len = sizeof(uint16_t) * count;
+		desc.datalen = rte_cpu_to_le_16(buff_len);
+		buff = (void *)entries;
+	} else
+		/* rule id is filled in destination field for deleting mirror rule */
+		cmd->destination = rte_cpu_to_le_16(rule_id);
+
+	cmd->rule_type = rte_cpu_to_le_16(rule_type <<
+				I40E_AQC_MIRROR_RULE_TYPE_SHIFT);
+	cmd->seid = rte_cpu_to_le_16(seid);
+
+	status = i40e_asq_send_command(hw, &desc, buff, buff_len, NULL);
+
+	return status;
+}
+
+/**
+ * i40e_mirror_rule_set
+ * @dev: pointer to the hardware structure
+ * @mirror_conf: mirror rule info
+ * @sw_id: mirror rule's sw_id
+ * @on: enable/disable
+ *
+ * set a mirror rule.
+ *
+ **/
+static int
+i40e_mirror_rule_set(struct rte_eth_dev *dev,
+			struct rte_eth_mirror_conf *mirror_conf,
+			uint8_t sw_id, uint8_t on)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_mirror_rule *it, *mirr_rule = NULL;
+	struct i40e_mirror_rule *parent = NULL;
+	uint16_t seid, dst_seid, rule_id;
+	uint16_t i, j = 0;
+	int ret;
+
+	PMD_DRV_LOG(DEBUG, "i40e_mirror_rule_set: sw_id = %d.", sw_id);
+
+	if (pf->main_vsi->veb == NULL || pf->vfs == NULL) {
+		PMD_DRV_LOG(ERR, "mirror rule can not be configured"
+			" without veb or vfs.");
+		return -ENOSYS;
+	}
+	if (pf->nb_mirror_rule > I40E_MAX_MIRROR_RULES) {
+		PMD_DRV_LOG(ERR, "mirror table is full.");
+		return -ENOSPC;
+	}
+	if (mirror_conf->dst_pool > pf->vf_num) {
+		PMD_DRV_LOG(ERR, "invalid destination pool %u.",
+				 mirror_conf->dst_pool);
+		return -EINVAL;
+	}
+
+	seid = pf->main_vsi->veb->seid;
+
+	TAILQ_FOREACH(it, &pf->mirror_list, rules) {
+		if (sw_id <= it->index) {
+			mirr_rule = it;
+			break;
+		}
+		parent = it;
+	}
+	if (mirr_rule && sw_id == mirr_rule->index) {
+		if (on) {
+			PMD_DRV_LOG(ERR, "mirror rule exists.");
+			return -EEXIST;
+		} else {
+			ret = i40e_aq_del_mirror_rule(hw, seid,
+					mirr_rule->rule_type,
+					mirr_rule->entries,
+					mirr_rule->num_entries, mirr_rule->id);
+			if (ret < 0) {
+				PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
+						   " ret = %d, aq_err = %d.",
+						   ret, hw->aq.asq_last_status);
+				return -ENOSYS;
+			}
+			TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
+			rte_free(mirr_rule);
+			pf->nb_mirror_rule--;
+			return 0;
+		}
+	} else if (!on) {
+		PMD_DRV_LOG(ERR, "mirror rule doesn't exist.");
+		return -ENOENT;
+	}
+
+	mirr_rule = rte_zmalloc("i40e_mirror_rule",
+				sizeof(struct i40e_mirror_rule) , 0);
+	if (!mirr_rule) {
+		PMD_DRV_LOG(ERR, "failed to allocate memory");
+		return I40E_ERR_NO_MEMORY;
+	}
+	switch (mirror_conf->rule_type) {
+	case ETH_MIRROR_VLAN:
+		for (i = 0, j = 0; i < ETH_MIRROR_MAX_VLANS; i++) {
+			if (mirror_conf->vlan.vlan_mask & (1ULL << i)) {
+				mirr_rule->entries[j] =
+					mirror_conf->vlan.vlan_id[i];
+				j++;
+			}
+		}
+		if (j == 0) {
+			PMD_DRV_LOG(ERR, "vlan is not specified.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_VLAN;
+		break;
+	case ETH_MIRROR_VIRTUAL_POOL_UP:
+	case ETH_MIRROR_VIRTUAL_POOL_DOWN:
+		/* check if the specified pool bit is out of range */
+		if (mirror_conf->pool_mask > (uint64_t)(1ULL << (pf->vf_num + 1))) {
+			PMD_DRV_LOG(ERR, "pool mask is out of range.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		for (i = 0, j = 0; i < pf->vf_num; i++) {
+			if (mirror_conf->pool_mask & (1ULL << i)) {
+				mirr_rule->entries[j] = pf->vfs[i].vsi->seid;
+				j++;
+			}
+		}
+		if (mirror_conf->pool_mask & (1ULL << pf->vf_num)) {
+			/* add pf vsi to entries */
+			mirr_rule->entries[j] = pf->main_vsi_seid;
+			j++;
+		}
+		if (j == 0) {
+			PMD_DRV_LOG(ERR, "pool is not specified.");
+			rte_free(mirr_rule);
+			return -EINVAL;
+		}
+		/* egress and ingress in aq commands means from switch but not port */
+		mirr_rule->rule_type =
+			(mirror_conf->rule_type == ETH_MIRROR_VIRTUAL_POOL_UP) ?
+			I40E_AQC_MIRROR_RULE_TYPE_VPORT_EGRESS :
+			I40E_AQC_MIRROR_RULE_TYPE_VPORT_INGRESS;
+		break;
+	case ETH_MIRROR_UPLINK_PORT:
+		/* egress and ingress in aq commands means from switch but not port*/
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_ALL_EGRESS;
+		break;
+	case ETH_MIRROR_DOWNLINK_PORT:
+		mirr_rule->rule_type = I40E_AQC_MIRROR_RULE_TYPE_ALL_INGRESS;
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "unsupported mirror type %d.",
+			mirror_conf->rule_type);
+		rte_free(mirr_rule);
+		return -EINVAL;
+	}
+
+	/* If the dst_pool is equal to vf_num, consider it as PF */
+	if (mirror_conf->dst_pool == pf->vf_num)
+		dst_seid = pf->main_vsi_seid;
+	else
+		dst_seid = pf->vfs[mirror_conf->dst_pool].vsi->seid;
+
+	ret = i40e_aq_add_mirror_rule(hw, seid, dst_seid,
+				      mirr_rule->rule_type, mirr_rule->entries,
+				      j, &rule_id);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "failed to add mirror rule:"
+				   " ret = %d, aq_err = %d.",
+				   ret, hw->aq.asq_last_status);
+		rte_free(mirr_rule);
+		return -ENOSYS;
+	}
+
+	mirr_rule->index = sw_id;
+	mirr_rule->num_entries = j;
+	mirr_rule->id = rule_id;
+	mirr_rule->dst_vsi_seid = dst_seid;
+
+	if (parent)
+		TAILQ_INSERT_AFTER(&pf->mirror_list, parent, mirr_rule, rules);
+	else
+		TAILQ_INSERT_HEAD(&pf->mirror_list, mirr_rule, rules);
+
+	pf->nb_mirror_rule++;
+	return 0;
+}
+
+/**
+ * i40e_mirror_rule_reset
+ * @dev: pointer to the device
+ * @sw_id: mirror rule's sw_id
+ *
+ * reset a mirror rule.
+ *
+ **/
+static int
+i40e_mirror_rule_reset(struct rte_eth_dev *dev, uint8_t sw_id)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_mirror_rule *it, *mirr_rule = NULL;
+	uint16_t seid;
+	int ret;
+
+	PMD_DRV_LOG(DEBUG, "i40e_mirror_rule_reset: sw_id = %d.", sw_id);
+
+	seid = pf->main_vsi->veb->seid;
+
+	TAILQ_FOREACH(it, &pf->mirror_list, rules) {
+		if (sw_id == it->index) {
+			mirr_rule = it;
+			break;
+		}
+	}
+	if (mirr_rule) {
+		ret = i40e_aq_del_mirror_rule(hw, seid,
+				mirr_rule->rule_type,
+				mirr_rule->entries,
+				mirr_rule->num_entries, mirr_rule->id);
+		if (ret < 0) {
+			PMD_DRV_LOG(ERR, "failed to remove mirror rule:"
+					   " status = %d, aq_err = %d.",
+					   ret, hw->aq.asq_last_status);
+			return -ENOSYS;
+		}
+		TAILQ_REMOVE(&pf->mirror_list, mirr_rule, rules);
+		rte_free(mirr_rule);
+		pf->nb_mirror_rule--;
+	} else {
+		PMD_DRV_LOG(ERR, "mirror rule doesn't exist.");
+		return -ENOENT;
+	}
+	return 0;
+}
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index baac4a0..723d7a4 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -323,6 +323,27 @@ struct i40e_fdir_info {
 	struct i40e_fdir_flex_mask flex_mask[I40E_FILTER_PCTYPE_MAX];
 };
 
+#define I40E_MIRROR_MAX_ENTRIES_PER_RULE   64
+#define I40E_MAX_MIRROR_RULES           64
+/*
+ * Mirror rule structure
+ */
+struct i40e_mirror_rule {
+	TAILQ_ENTRY(i40e_mirror_rule) rules;
+	uint8_t rule_type;
+	uint16_t index;          /* the sw index of mirror rule */
+	uint16_t id;             /* the rule id assigned by firmware */
+	uint16_t dst_vsi_seid;   /* destination vsi for this mirror rule. */
+	uint16_t num_entries;
+	/* the info stores depend on the rule type.
+	    If type is I40E_MIRROR_TYPE_VLAN, vlan ids are stored here.
+	    If type is I40E_MIRROR_TYPE_VPORT_*, vsi's seid are stored.
+	 */
+	uint16_t entries[I40E_MIRROR_MAX_ENTRIES_PER_RULE];
+};
+
+TAILQ_HEAD(i40e_mirror_rule_list, i40e_mirror_rule);
+
 /*
  * Structure to store private data specific for PF instance.
  */
@@ -362,6 +383,8 @@ struct i40e_pf {
 	struct i40e_vmdq_info *vmdq;
 
 	struct i40e_fdir_info fdir; /* flow director info */
+	struct i40e_mirror_rule_list mirror_list;
+	uint16_t nb_mirror_rule;   /* The number of mirror rules */
 };
 
 enum pending_msg {
-- 
1.9.3

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

* [dpdk-dev] [PATCH v4 4/4] doc: modify the command about mirror in testpmd guide
  2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
                         ` (2 preceding siblings ...)
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 3/4] i40e: enable mirror functionality in i40e driver Jingjing Wu
@ 2015-06-10  6:24       ` Jingjing Wu
  2015-06-10  8:33       ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jiajia, SunX
                         ` (2 subsequent siblings)
  6 siblings, 0 replies; 30+ messages in thread
From: Jingjing Wu @ 2015-06-10  6:24 UTC (permalink / raw)
  To: dev

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 761172e..2cd3461 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -772,9 +772,13 @@ set port (port_id) vf (vf_id) rate (rate_value) queue_mask (queue_mask)
 set port - mirror rule
 ~~~~~~~~~~~~~~~~~~~~~~
 
-Set port or vlan type mirror rule for a port.
+Set pool or vlan type mirror rule for a port:
 
-set port (port_id) mirror-rule (rule_id) (pool-mirror|vlan-mirror) (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)
+set port (port_id) mirror-rule (rule_id) (pool-mirror-up|pool-mirror-down|vlan-mirror) (poolmask|vlanid[,vlanid]*) dst-pool (pool_id) (on|off)
+
+Set link mirror rule for a port:
+
+set port (port_id) mirror-rule (rule_id) (uplink-mirror|downlink-mirror) dst-pool (pool_id) (on|off)
 
 For example to enable mirror traffic with vlan 0,1 to pool 0:
 
-- 
1.9.3

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

* Re: [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver
  2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
                         ` (3 preceding siblings ...)
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 4/4] doc: modify the command about mirror in testpmd guide Jingjing Wu
@ 2015-06-10  8:33       ` Jiajia, SunX
  2015-06-12  8:18       ` Liu, Jijiang
  2015-06-12  8:44       ` Zhang, Helin
  6 siblings, 0 replies; 30+ messages in thread
From: Jiajia, SunX @ 2015-06-10  8:33 UTC (permalink / raw)
  To: Wu, Jingjing, dev

Tested-by: Jiajia Sunx <sunx.jiajia@intel.com>

- Tested Commit: 94ef2964148a4540db21034f5c3669ab81fbdc76
- OS: Fedora20 3.18.9
- GCC: gcc version 4.8.3 20140911
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection [8086:10fb]
- NIC: Intel Corporation Ethernet Controller XL710 for 40GbE QSFP+ [8086:1583]
- Default x86_64-native-linuxapp-gcc configuration
- Prerequisites: vt-d enable in bios and IOMMU enable in kernel
- Total 8 cases, 8 passed, 0 failed

2VM prerequisites:

    VF0 ./x86_64-default-linuxapp-gcc/app/testpmd -c f -n 4 --  -i
    VF0 testpmd-> set fwd rxonly
    VF0 testpmd-> start
 
    VF1 ./x86_64-default-linuxapp-gcc/app/testpmd -c f -n 4 --  -i
    VF1 testpmd-> set fwd mac
    VF1 testpmd-> start

 - Case: Mirror Traffic between 2VMs with Pool mirroring
   Description: Verify the pool mirror rule
	Set up common 2VM prerequisites.

	Add one mirror rule that will mirror VM0 income traffic to VM1::

    		PF testpmd-> set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on

	Send 10 packets to VM0 and verify the packets has been mirrored to VM1 and
	forwarded the packet.

	After test need reset mirror rule::

    		PF testpmd-> reset port 0 mirror-rule 0
  

 - Case: Mirror Traffic between 2VMs with Uplink mirroring
   Description: Verify the uplink mirror rule
	Set up common 2VM prerequisites.

	Add one mirror rule that will mirror VM0 income traffic to VM1::

    		PF testpmd-> set port 0 mirror-rule 0 uplink-mirror dst-pool 1 on

	Send 10 packets to VM0 and verify the packets has been mirrored to VM1 and
	forwarded the packet.

	After test need reset mirror rule::
    
		PF testpmd-> reset port 0 mirror-rule 0
  
 - Case: Mirror Traffic between 2VMs with Downlink mirroring
   Description: Verify the downlink mirror rule
	Run testpmd on VM0 and VM1 and start traffic forward on the VM hosts::

	    VF0 ./x86_64-default-linuxapp-gcc/app/testpmd -c f -n 4 --  -i
   	    VF1 ./x86_64-default-linuxapp-gcc/app/testpmd -c f -n 4 --  -i

	Add one mirror rule that will mirror VM0 outcome traffic to VM1::

          PF testpmd-> set port 0 mirror-rule 0 downlink-mirror dst-pool 1 on

	Make sure VM1 in receive only mode, VM0 send 16 packets, and verify the VM0
	packets has been mirrored to VM1::
    
    	    VF1 testpmd-> set fwd rxonly
          VF1 testpmd-> start
          VF0 testpmd-> start tx_first 
  
	Note: don't let VF1 fwd packets since downlink mirror will mirror back the
	packets to received packets, which will be an infinite loop.

	After test need reset mirror rule::

         PF testpmd-> reset port 0 mirror-rule 0  
  
 - Case: Mirror Traffic between VMs with Vlan mirroring
   Description: Verify the vlan mirror rule
	Set up common 2VM prerequisites.

	Add rx vlan-id 0 on VF0, add one mirror rule that will mirror VM0 income
	traffic with specified vlan to VM1::
		PF testpmd-> rx_vlan add 3 port 0 vf 0x1
    		PF testpmd-> set port 0 mirror-rule 0 vlan-mirror 3 dst-pool 1 on

	Send 10 packets with vlan-id0/vm0 MAC to VM0 and verify the packets has been
	mirrored to VM1 and forwarded the packet.

	After test need reset mirror rule::
    
		PF testpmd-> reset port 0 mirror-rule 0

 - Case: Mirror Traffic between 2VMs with Vlan & Pool mirroring
   Description: Verify the misc mirror rules of vlan and pool
	Set up common 2VM prerequisites.

	Add rx vlan-id 3 of VF1, and 2 mirror rules, one is VM0 income traffic to VM1,
	one is VM1 vlan income traffic to VM0::

    		PF testpmd-> rx_vlan add 3 port 0 vf 0x2
    		PF testpmd-> set port 0 mirror-rule 0 pool-mirror-up 0x1 dst-pool 1 on
    		PF testpmd-> set port 0 mirror-rule 1 vlan-mirror 3 dst-pool 0 on
  
	Send 2 flows one by one, first 10 packets with VM0 mac, and the second 100
	packets with VM1 vlan and mac, and verify the first 10 packets has been
	mirrored first to VM1, second 100 packets go to VM0 and the packets have been
	forwarded.

	After test need reset mirror rule::

    		PF testpmd-> reset port 0 mirror-rule 0
    		PF testpmd-> reset port 0 mirror-rule 1

 - Case: Mirror Traffic between 2VMs with Uplink & Downlink mirroring
   Description: Verify the misc mirror rules of uplink and downlink
	Run testpmd on VM0 and VM1 and start traffic forward on the VM hosts::
    
    		VF0 ./x86_64-default-linuxapp-gcc/app/testpmd -c f -n 4 --  -i
    		VF1 ./x86_64-default-linuxapp-gcc/app/testpmd -c f -n 4 --  -i
	Add 2 mirror rules that will mirror VM0 outcome and income traffic to VM1::
  
    		PF testpmd-> set port 0 mirror-rule 0 downlink-mirror dst-pool 1 on
    		PF testpmd-> set port 0 mirror-rule 0 uplink-mirror dst-pool 1 on

	Make sure VM1 in receive only mode, VM0 first send 16 packets, and verify the
	VM0 packets has been mirrored to VM1:: 
    
    		VF1 testpmd-> set fwd rxonly
    		VF1 testpmd-> start
    		VF0 testpmd-> start tx_first 

	Note: don't let VF1 fwd packets since downlink mirror will mirror back the
	packets to received packets, which will be an infinite loop.

	Send 10 packets to VF0 with VF0 MAC from ixia, verify that all VF0 received
	packets and transmitted packets will mirror to VF1::

   		VF0 testpmd-> stop 
   		VF0 testpmd-> set fwd mac
       	VF0 testpmd-> start  

	After test need reset mirror rule::
    
    		PF testpmd-> reset port 0 mirror-rule 0  

 - Case: Mirror Traffic between 2VMs with Vlan & Pool & Uplink & Downlink mirroring
   Description: Verify the misc mirror rules of vlan and pool and uplink and downlink
	Run testpmd on VM0 and VM1 and start traffic forward on the VM hosts::
    
    		VF0 ./x86_64-default-linuxapp-gcc/app/testpmd -c f -n 4 --  -i
    		VF1 ./x86_64-default-linuxapp-gcc/app/testpmd -c f -n 4 --  -i
	Add rx vlan-id 0 on VF0 and add 4 mirror rules::
    		PF testpmd-> reset port 0 mirror-rule 1
    		PF testpmd-> set port 0 mirror-rule 0 downlink-mirror dst-pool 1 on
    		PF testpmd-> set port 0 mirror-rule 1 uplink-mirror dst-pool 1 on
    		PF testpmd-> rx_vlan add 0 port 0 vf 0x2
    		PF testpmd-> set port 0 mirror-rule 2 vlan-mirror 0 dst-pool 0 on
    		PF testpmd-> set port 0 mirror-rule 3 pool-mirror-up 0x1 dst-pool 1 on
  
	Make sure VM1 in receive only mode, VM0 first send 16 packets, and verify the
	VM0 packets has been mirrored to VM1, VF1, RX, 16packets (downlink mirror)::

    		VF1 testpmd-> set fwd rxonly
    		VF1 testpmd-> start
    		VF0 testpmd-> start tx_first 
  
	Note: don't let VF1 fwd packets since downlink mirror will mirror back the
	packets to received packets, which will be an infinite loop.

	Send 1 packet to VF0 with VF0 MAC from ixia, check if VF0 RX 1 packet and TX 1
	packet, and VF1 has 2 packets mirror from VF0(uplink mirror/downlink/pool)::

    		VF0 testpmd-> stop 
    		VF0 testpmd-> set fwd mac
    		VF0 testpmd-> start  

	Send 1 packet with VM1 vlan id and mac, and verify that VF0 have 1 RX packet, 1
	TX packet, and VF1 have 2 packets(downlink mirror)::
    
    		VF0 testpmd-> stop 
    		VF0 testpmd-> set fwd mac 
    		VF0 testpmd-> start 
  
	After test need reset mirror rule::

    		PF testpmd-> reset port 0 mirror-rule 0
    		PF testpmd-> reset port 0 mirror-rule 1  
    		PF testpmd-> reset port 0 mirror-rule 2
    		PF testpmd-> reset port 0 mirror-rule 3  

 - Case: Negative input to commands
   Decription:Input invalid commands on PF/VF to make sure the commands can't work::

	Both niantic and Fortville:
   		PF testpmd-> set port 0 vf 65 tx on
   		PF testpmd-> set port 2 vf -1 tx off
    		PF testpmd-> set port 0 vf 0 rx oneee
    		PF testpmd-> set port 0 vf 0 rx offdd
    		PF testpmd-> set port 0 vf 64 rxmode BAM on
    		PF testpmd-> set port 0 vf 64 rxmode BAM off
    		PF testpmd-> set port 0 uta 00:11:22:33:44 on
    		PF testpmd-> set port 7 uta 00:55:44:33:22:11 off
    		PF testpmd-> set port 0 vf 34 rxmode ROPE on 
    		PF testpmd-> mac_addr add port 0 vf 65 00:55:44:33:22:11
    		PF testpmd-> mac_addr add port 5 vf 0 00:55:44:88:22:11
    		PF testpmd-> reset port 0xff mirror-rule 0
		PF testpmd-> set port 0 mirror-rule 0 downlink-mirror 0xf dst-pool 2 off
	Just for niantic:
    		PF testpmd-> set port 0 mirror-rule 0xf uplink-mirror dst-pool 1 on
    		PF testpmd-> set port 0 mirror-rule 2 vlan-mirror 9 dst-pool 1 on
    		PF testpmd-> reset port 0 mirror-rule 4
    

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Wednesday, June 10, 2015 2:24 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Liu, Jijiang; Jiajia, SunX; Zhang, Helin
> Subject: [PATCH v4 0/4] enable mirror functionality in i40e driver
> 
> This patch set enables mirror functionality in i40e driver, and
> redefines structure and macros used to configure mirror.
> 
> v2 changes:
>  - correct comments style
>  - add doc change
> 
> v3 changes:
>  - change the mirror rule type to support bit mask
>  - fix code style
> 
> v4 changes:
>  - correct the rule type check ixgbe
> 
> Jingjing Wu (4):
>   ethdev: rename rte_eth_vmdq_mirror_conf
>   ethdev: rename and extend the mirror type
>   i40e: enable mirror functionality in i40e driver
>   doc: modify the command about mirror in testpmd guide
> 
>  app/test-pmd/cmdline.c                      |  62 +++---
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
>  drivers/net/i40e/i40e_ethdev.c              | 334
> ++++++++++++++++++++++++++++
>  drivers/net/i40e/i40e_ethdev.h              |  23 ++
>  drivers/net/ixgbe/ixgbe_ethdev.c            |  62 ++++--
>  drivers/net/ixgbe/ixgbe_ethdev.h            |   4 +-
>  lib/librte_ether/rte_ethdev.c               |  28 +--
>  lib/librte_ether/rte_ethdev.h               |  30 +--
>  8 files changed, 466 insertions(+), 85 deletions(-)
> 
> --
> 1.9.3

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

* Re: [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver
  2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
                         ` (4 preceding siblings ...)
  2015-06-10  8:33       ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jiajia, SunX
@ 2015-06-12  8:18       ` Liu, Jijiang
  2015-06-12  8:44       ` Zhang, Helin
  6 siblings, 0 replies; 30+ messages in thread
From: Liu, Jijiang @ 2015-06-12  8:18 UTC (permalink / raw)
  To: Wu, Jingjing, dev


Acked-By: Jijiang Liu <Jijiang.liu@intel.com>

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Wednesday, June 10, 2015 2:24 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Liu, Jijiang; Jiajia, SunX; Zhang, Helin
> Subject: [PATCH v4 0/4] enable mirror functionality in i40e driver
> 
> This patch set enables mirror functionality in i40e driver, and redefines structure
> and macros used to configure mirror.
> 
> v2 changes:
>  - correct comments style
>  - add doc change
> 
> v3 changes:
>  - change the mirror rule type to support bit mask
>  - fix code style
> 
> v4 changes:
>  - correct the rule type check ixgbe
> 
> Jingjing Wu (4):
>   ethdev: rename rte_eth_vmdq_mirror_conf
>   ethdev: rename and extend the mirror type
>   i40e: enable mirror functionality in i40e driver
>   doc: modify the command about mirror in testpmd guide
> 
>  app/test-pmd/cmdline.c                      |  62 +++---
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
>  drivers/net/i40e/i40e_ethdev.c              | 334 ++++++++++++++++++++++++++++
>  drivers/net/i40e/i40e_ethdev.h              |  23 ++
>  drivers/net/ixgbe/ixgbe_ethdev.c            |  62 ++++--
>  drivers/net/ixgbe/ixgbe_ethdev.h            |   4 +-
>  lib/librte_ether/rte_ethdev.c               |  28 +--
>  lib/librte_ether/rte_ethdev.h               |  30 +--
>  8 files changed, 466 insertions(+), 85 deletions(-)
> 
> --
> 1.9.3

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

* Re: [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver
  2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
                         ` (5 preceding siblings ...)
  2015-06-12  8:18       ` Liu, Jijiang
@ 2015-06-12  8:44       ` Zhang, Helin
  2015-07-07 15:46         ` Thomas Monjalon
  6 siblings, 1 reply; 30+ messages in thread
From: Zhang, Helin @ 2015-06-12  8:44 UTC (permalink / raw)
  To: Wu, Jingjing, dev

Acked-by: Helin Zhang <helin.zhang@intel.com>

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Wednesday, June 10, 2015 2:24 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Liu, Jijiang; Jiajia, SunX; Zhang, Helin
> Subject: [PATCH v4 0/4] enable mirror functionality in i40e driver
> 
> This patch set enables mirror functionality in i40e driver, and redefines structure
> and macros used to configure mirror.
> 
> v2 changes:
>  - correct comments style
>  - add doc change
> 
> v3 changes:
>  - change the mirror rule type to support bit mask
>  - fix code style
> 
> v4 changes:
>  - correct the rule type check ixgbe
> 
> Jingjing Wu (4):
>   ethdev: rename rte_eth_vmdq_mirror_conf
>   ethdev: rename and extend the mirror type
>   i40e: enable mirror functionality in i40e driver
>   doc: modify the command about mirror in testpmd guide
> 
>  app/test-pmd/cmdline.c                      |  62 +++---
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +-
>  drivers/net/i40e/i40e_ethdev.c              | 334
> ++++++++++++++++++++++++++++
>  drivers/net/i40e/i40e_ethdev.h              |  23 ++
>  drivers/net/ixgbe/ixgbe_ethdev.c            |  62 ++++--
>  drivers/net/ixgbe/ixgbe_ethdev.h            |   4 +-
>  lib/librte_ether/rte_ethdev.c               |  28 +--
>  lib/librte_ether/rte_ethdev.h               |  30 +--
>  8 files changed, 466 insertions(+), 85 deletions(-)
> 
> --
> 1.9.3

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

* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
@ 2015-06-26  7:03         ` Wu, Jingjing
  2015-07-06  1:27           ` Wu, Jingjing
  2015-07-07 14:51           ` Thomas Monjalon
  0 siblings, 2 replies; 30+ messages in thread
From: Wu, Jingjing @ 2015-06-26  7:03 UTC (permalink / raw)
  To: 'nhorman@tuxdriver.com'; +Cc: dev

Hi, Neil

About this patch I have an ABI concern about it. 
This patch just renamed a struct rte_eth_vmdq_mirror_conf to rte_eth_mirror_conf, the size and its elements don't change. As my understanding, it will not break the ABI. And I also tested it. 
But when I use the script ./scripts/validate-abi.sh to check. A low severity problem is reported in symbol "rte_eth_mirror_rule_set"
 - Change: "Base type of 2nd parameter mirror_conf has been changed from struct rte_eth_vmdq_mirror_conf to struct rte_eth_mirror_conf."
 - Effect: "Replacement of parameter base type may indicate a change in its semantic meaning."

So, I'm not sure whether this patch meet the ABI policy?

Additional, about the validate-abi.sh, does it mean we need to fix all the problems it reports? Or we can decide case by case. Can a Low Severity problem be acceptable?

Look forward to your reply.

Thanks

Jingjing

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Wednesday, June 10, 2015 2:25 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Liu, Jijiang; Jiajia, SunX; Zhang, Helin
> Subject: [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
> 
> rename rte_eth_vmdq_mirror_conf to rte_eth_mirror_conf and move the
> maximum rule id check from ethdev level to driver
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  app/test-pmd/cmdline.c           | 22 +++++++++++-----------
>  drivers/net/ixgbe/ixgbe_ethdev.c | 11 +++++++----
> drivers/net/ixgbe/ixgbe_ethdev.h |  4 +++-
>  lib/librte_ether/rte_ethdev.c    | 18 ++----------------
>  lib/librte_ether/rte_ethdev.h    | 19 ++++++++++---------
>  5 files changed, 33 insertions(+), 41 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> f01db2a..d693bde 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -6604,11 +6604,11 @@ cmd_set_mirror_mask_parsed(void
> *parsed_result,  {
>  	int ret,nb_item,i;
>  	struct cmd_set_mirror_mask_result *res = parsed_result;
> -	struct rte_eth_vmdq_mirror_conf mr_conf;
> +	struct rte_eth_mirror_conf mr_conf;
> 
> -	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
> +	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
> 
> -	unsigned int vlan_list[ETH_VMDQ_MAX_VLAN_FILTERS];
> +	unsigned int vlan_list[ETH_MIRROR_MAX_VLANS];
> 
>  	mr_conf.dst_pool = res->dstpool_id;
> 
> @@ -6618,11 +6618,11 @@ cmd_set_mirror_mask_parsed(void
> *parsed_result,
>  	} else if(!strcmp(res->what, "vlan-mirror")) {
>  		mr_conf.rule_type_mask = ETH_VMDQ_VLAN_MIRROR;
>  		nb_item = parse_item_list(res->value, "core",
> -
> 	ETH_VMDQ_MAX_VLAN_FILTERS,vlan_list,1);
> +					ETH_MIRROR_MAX_VLANS, vlan_list,
> 1);
>  		if (nb_item <= 0)
>  			return;
> 
> -		for(i=0; i < nb_item; i++) {
> +		for (i = 0; i < nb_item; i++) {
>  			if (vlan_list[i] > ETHER_MAX_VLAN_ID) {
>  				printf("Invalid vlan_id: must be < 4096\n");
>  				return;
> @@ -6634,10 +6634,10 @@ cmd_set_mirror_mask_parsed(void
> *parsed_result,
>  	}
> 
>  	if(!strcmp(res->on, "on"))
> -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
>  						res->rule_id, 1);
>  	else
> -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
>  						res->rule_id, 0);
>  	if(ret < 0)
>  		printf("mirror rule add error: (%s)\n", strerror(-ret)); @@ -
> 6711,9 +6711,9 @@ cmd_set_mirror_link_parsed(void *parsed_result,  {
>  	int ret;
>  	struct cmd_set_mirror_link_result *res = parsed_result;
> -	struct rte_eth_vmdq_mirror_conf mr_conf;
> +	struct rte_eth_mirror_conf mr_conf;
> 
> -	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
> +	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
>  	if(!strcmp(res->what, "uplink-mirror")) {
>  		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
>  	}else if(!strcmp(res->what, "downlink-mirror")) @@ -6722,10
> +6722,10 @@ cmd_set_mirror_link_parsed(void *parsed_result,
>  	mr_conf.dst_pool = res->dstpool_id;
> 
>  	if(!strcmp(res->on, "on"))
> -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
>  						res->rule_id, 1);
>  	else
> -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
>  						res->rule_id, 0);
> 
>  	/* check the return value and print it if is < 0 */ diff --git
> a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 0d9f9b2..9e767fa 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -209,7 +209,7 @@ static int ixgbe_set_pool_tx(struct rte_eth_dev
> *dev,uint16_t pool,uint8_t on);  static int ixgbe_set_pool_vlan_filter(struct
> rte_eth_dev *dev, uint16_t vlan,
>  		uint64_t pool_mask,uint8_t vlan_on);
>  static int ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
> -		struct rte_eth_vmdq_mirror_conf *mirror_conf,
> +		struct rte_eth_mirror_conf *mirror_conf,
>  		uint8_t rule_id, uint8_t on);
>  static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev,
>  		uint8_t	rule_id);
> @@ -3388,7 +3388,7 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev
> *dev, uint16_t vlan,
> 
>  static int
>  ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
> -			struct rte_eth_vmdq_mirror_conf *mirror_conf,
> +			struct rte_eth_mirror_conf *mirror_conf,
>  			uint8_t rule_id, uint8_t on)
>  {
>  	uint32_t mr_ctl,vlvf;
> @@ -3412,7 +3412,10 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
>  		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> 
>  	if (ixgbe_vmdq_mode_check(hw) < 0)
> -		return (-ENOTSUP);
> +		return -ENOTSUP;
> +
> +	if (rule_id >= IXGBE_MAX_MIRROR_RULES)
> +		return -EINVAL;
> 
>  	/* Check if vlan mask is valid */
>  	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR)
> && (on)) { @@ -3526,7 +3529,7 @@ ixgbe_mirror_rule_reset(struct
> rte_eth_dev *dev, uint8_t rule_id)
>  		return (-ENOTSUP);
> 
>  	memset(&mr_info->mr_conf[rule_id], 0,
> -		sizeof(struct rte_eth_vmdq_mirror_conf));
> +		sizeof(struct rte_eth_mirror_conf));
> 
>  	/* clear PFVMCTL register */
>  	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl); diff --git
> a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 19237b8..755b674 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -177,8 +177,10 @@ struct ixgbe_uta_info {
>  	uint32_t uta_shadow[IXGBE_MAX_UTA];
>  };
> 
> +#define IXGBE_MAX_MIRROR_RULES 4  /* Maximum nb. of mirror rules. */
> +
>  struct ixgbe_mirror_info {
> -	struct rte_eth_vmdq_mirror_conf
> mr_conf[ETH_VMDQ_NUM_MIRROR_RULE];
> +	struct rte_eth_mirror_conf mr_conf[IXGBE_MAX_MIRROR_RULES];
>  	/**< store PF mirror rules configuration*/  };
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 024fe8b..43c7295 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -3034,7 +3034,7 @@ int rte_eth_set_vf_rate_limit(uint8_t port_id,
> uint16_t vf, uint16_t tx_rate,
> 
>  int
>  rte_eth_mirror_rule_set(uint8_t port_id,
> -			struct rte_eth_vmdq_mirror_conf *mirror_conf,
> +			struct rte_eth_mirror_conf *mirror_conf,
>  			uint8_t rule_id, uint8_t on)
>  {
>  	struct rte_eth_dev *dev = &rte_eth_devices[port_id]; @@ -3051,7
> +3051,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
> 
>  	if (mirror_conf->dst_pool >= ETH_64_POOLS) {
>  		PMD_DEBUG_TRACE("Invalid dst pool, pool id must"
> -			"be 0-%d\n",ETH_64_POOLS - 1);
> +			"be 0-%d\n", ETH_64_POOLS - 1);
>  		return -EINVAL;
>  	}
> 
> @@ -3062,13 +3062,6 @@ rte_eth_mirror_rule_set(uint8_t port_id,
>  		return -EINVAL;
>  	}
> 
> -	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
> -	{
> -		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-
> %d\n",
> -			ETH_VMDQ_NUM_MIRROR_RULE - 1);
> -		return -EINVAL;
> -	}
> -
>  	dev = &rte_eth_devices[port_id];
>  	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -
> ENOTSUP);
> 
> @@ -3085,13 +3078,6 @@ rte_eth_mirror_rule_reset(uint8_t port_id,
> uint8_t rule_id)
>  		return -ENODEV;
>  	}
> 
> -	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
> -	{
> -		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-
> %d\n",
> -			ETH_VMDQ_NUM_MIRROR_RULE-1);
> -		return -EINVAL;
> -	}
> -
>  	dev = &rte_eth_devices[port_id];
>  	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_reset, -
> ENOTSUP);
> 
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 16dbe00..ae22fea 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -467,8 +467,8 @@ struct rte_eth_rss_conf {
>  #define ETH_VMDQ_ACCEPT_BROADCAST   0x0008 /**< accept broadcast
> packets. */
>  #define ETH_VMDQ_ACCEPT_MULTICAST   0x0010 /**< multicast
> promiscuous. */
> 
> -/* Definitions used for VMDQ mirror rules setting */
> -#define ETH_VMDQ_NUM_MIRROR_RULE     4 /**< Maximum nb. of mirror
> rules. . */
> +/** Maximum nb. of vlan per mirror rule */
> +#define ETH_MIRROR_MAX_VLANS       64
> 
>  #define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring. */
>  #define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring.
> */ @@ -480,18 +480,19 @@ struct rte_eth_rss_conf {
>   */
>  struct rte_eth_vlan_mirror {
>  	uint64_t vlan_mask; /**< mask for valid VLAN ID. */
> -	uint16_t vlan_id[ETH_VMDQ_MAX_VLAN_FILTERS];
> -	/** VLAN ID list for vlan mirror. */
> +	/** VLAN ID list for vlan mirroring. */
> +	uint16_t vlan_id[ETH_MIRROR_MAX_VLANS];
>  };
> 
>  /**
>   * A structure used to configure traffic mirror of an Ethernet port.
>   */
> -struct rte_eth_vmdq_mirror_conf {
> +struct rte_eth_mirror_conf {
>  	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to
> set */
> -	uint8_t dst_pool; /**< Destination pool for this mirror rule. */
> +	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
>  	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
> -	struct rte_eth_vlan_mirror vlan; /**< VLAN ID setting for VLAN
> mirroring */
> +	/** VLAN ID setting for VLAN mirroring. */
> +	struct rte_eth_vlan_mirror vlan;
>  };
> 
>  /**
> @@ -1211,7 +1212,7 @@ typedef int (*eth_set_vf_rate_limit_t)(struct
> rte_eth_dev *dev,  /**< @internal Set VF TX rate */
> 
>  typedef int (*eth_mirror_rule_set_t)(struct rte_eth_dev *dev,
> -				  struct rte_eth_vmdq_mirror_conf
> *mirror_conf,
> +				  struct rte_eth_mirror_conf *mirror_conf,
>  				  uint8_t rule_id,
>  				  uint8_t on);
>  /**< @internal Add a traffic mirroring rule on an Ethernet device */ @@ -
> 3168,7 +3169,7 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port, uint16_t
> vlan_id,
>   *   - (-EINVAL) if the mr_conf information is not correct.
>   */
>  int rte_eth_mirror_rule_set(uint8_t port_id,
> -			struct rte_eth_vmdq_mirror_conf *mirror_conf,
> +			struct rte_eth_mirror_conf *mirror_conf,
>  			uint8_t rule_id,
>  			uint8_t on);
> 
> --
> 1.9.3

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

* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-06-26  7:03         ` Wu, Jingjing
@ 2015-07-06  1:27           ` Wu, Jingjing
  2015-07-07 14:51           ` Thomas Monjalon
  1 sibling, 0 replies; 30+ messages in thread
From: Wu, Jingjing @ 2015-07-06  1:27 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Hi, Thomas

Any suggestions about this patch? Do I need rework it by using macro RTE_NEXT_ABI?
Thanks a lot!

Jingjing

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wu, Jingjing
> Sent: Friday, June 26, 2015 3:03 PM
> To: 'nhorman@tuxdriver.com'
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename
> rte_eth_vmdq_mirror_conf
> 
> Hi, Neil
> 
> About this patch I have an ABI concern about it.
> This patch just renamed a struct rte_eth_vmdq_mirror_conf to
> rte_eth_mirror_conf, the size and its elements don't change. As my
> understanding, it will not break the ABI. And I also tested it.
> But when I use the script ./scripts/validate-abi.sh to check. A low severity
> problem is reported in symbol "rte_eth_mirror_rule_set"
>  - Change: "Base type of 2nd parameter mirror_conf has been changed from
> struct rte_eth_vmdq_mirror_conf to struct rte_eth_mirror_conf."
>  - Effect: "Replacement of parameter base type may indicate a change in its
> semantic meaning."
> 
> So, I'm not sure whether this patch meet the ABI policy?
> 
> Additional, about the validate-abi.sh, does it mean we need to fix all the
> problems it reports? Or we can decide case by case. Can a Low Severity
> problem be acceptable?
> 
> Look forward to your reply.
> 
> Thanks
> 
> Jingjing
> 
> > -----Original Message-----
> > From: Wu, Jingjing
> > Sent: Wednesday, June 10, 2015 2:25 PM
> > To: dev@dpdk.org
> > Cc: Wu, Jingjing; Liu, Jijiang; Jiajia, SunX; Zhang, Helin
> > Subject: [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
> >
> > rename rte_eth_vmdq_mirror_conf to rte_eth_mirror_conf and move the
> > maximum rule id check from ethdev level to driver
> >
> > Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> > ---
> >  app/test-pmd/cmdline.c           | 22 +++++++++++-----------
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 11 +++++++----
> > drivers/net/ixgbe/ixgbe_ethdev.h |  4 +++-
> >  lib/librte_ether/rte_ethdev.c    | 18 ++----------------
> >  lib/librte_ether/rte_ethdev.h    | 19 ++++++++++---------
> >  5 files changed, 33 insertions(+), 41 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > f01db2a..d693bde 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -6604,11 +6604,11 @@ cmd_set_mirror_mask_parsed(void
> > *parsed_result,  {
> >  	int ret,nb_item,i;
> >  	struct cmd_set_mirror_mask_result *res = parsed_result;
> > -	struct rte_eth_vmdq_mirror_conf mr_conf;
> > +	struct rte_eth_mirror_conf mr_conf;
> >
> > -	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
> > +	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
> >
> > -	unsigned int vlan_list[ETH_VMDQ_MAX_VLAN_FILTERS];
> > +	unsigned int vlan_list[ETH_MIRROR_MAX_VLANS];
> >
> >  	mr_conf.dst_pool = res->dstpool_id;
> >
> > @@ -6618,11 +6618,11 @@ cmd_set_mirror_mask_parsed(void
> > *parsed_result,
> >  	} else if(!strcmp(res->what, "vlan-mirror")) {
> >  		mr_conf.rule_type_mask = ETH_VMDQ_VLAN_MIRROR;
> >  		nb_item = parse_item_list(res->value, "core",
> > -
> > 	ETH_VMDQ_MAX_VLAN_FILTERS,vlan_list,1);
> > +					ETH_MIRROR_MAX_VLANS, vlan_list,
> > 1);
> >  		if (nb_item <= 0)
> >  			return;
> >
> > -		for(i=0; i < nb_item; i++) {
> > +		for (i = 0; i < nb_item; i++) {
> >  			if (vlan_list[i] > ETHER_MAX_VLAN_ID) {
> >  				printf("Invalid vlan_id: must be < 4096\n");
> >  				return;
> > @@ -6634,10 +6634,10 @@ cmd_set_mirror_mask_parsed(void
> > *parsed_result,
> >  	}
> >
> >  	if(!strcmp(res->on, "on"))
> > -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> > +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
> >  						res->rule_id, 1);
> >  	else
> > -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> > +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
> >  						res->rule_id, 0);
> >  	if(ret < 0)
> >  		printf("mirror rule add error: (%s)\n", strerror(-ret)); @@ -
> > 6711,9 +6711,9 @@ cmd_set_mirror_link_parsed(void *parsed_result,  {
> >  	int ret;
> >  	struct cmd_set_mirror_link_result *res = parsed_result;
> > -	struct rte_eth_vmdq_mirror_conf mr_conf;
> > +	struct rte_eth_mirror_conf mr_conf;
> >
> > -	memset(&mr_conf,0,sizeof(struct rte_eth_vmdq_mirror_conf));
> > +	memset(&mr_conf, 0, sizeof(struct rte_eth_mirror_conf));
> >  	if(!strcmp(res->what, "uplink-mirror")) {
> >  		mr_conf.rule_type_mask = ETH_VMDQ_UPLINK_MIRROR;
> >  	}else if(!strcmp(res->what, "downlink-mirror")) @@ -6722,10
> > +6722,10 @@ cmd_set_mirror_link_parsed(void *parsed_result,
> >  	mr_conf.dst_pool = res->dstpool_id;
> >
> >  	if(!strcmp(res->on, "on"))
> > -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> > +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
> >  						res->rule_id, 1);
> >  	else
> > -		ret = rte_eth_mirror_rule_set(res->port_id,&mr_conf,
> > +		ret = rte_eth_mirror_rule_set(res->port_id, &mr_conf,
> >  						res->rule_id, 0);
> >
> >  	/* check the return value and print it if is < 0 */ diff --git
> > a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> > index 0d9f9b2..9e767fa 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> > @@ -209,7 +209,7 @@ static int ixgbe_set_pool_tx(struct rte_eth_dev
> > *dev,uint16_t pool,uint8_t on);  static int
> > ixgbe_set_pool_vlan_filter(struct rte_eth_dev *dev, uint16_t vlan,
> >  		uint64_t pool_mask,uint8_t vlan_on);  static int
> > ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
> > -		struct rte_eth_vmdq_mirror_conf *mirror_conf,
> > +		struct rte_eth_mirror_conf *mirror_conf,
> >  		uint8_t rule_id, uint8_t on);
> >  static int ixgbe_mirror_rule_reset(struct rte_eth_dev *dev,
> >  		uint8_t	rule_id);
> > @@ -3388,7 +3388,7 @@ ixgbe_set_pool_vlan_filter(struct rte_eth_dev
> > *dev, uint16_t vlan,
> >
> >  static int
> >  ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
> > -			struct rte_eth_vmdq_mirror_conf *mirror_conf,
> > +			struct rte_eth_mirror_conf *mirror_conf,
> >  			uint8_t rule_id, uint8_t on)
> >  {
> >  	uint32_t mr_ctl,vlvf;
> > @@ -3412,7 +3412,10 @@ ixgbe_mirror_rule_set(struct rte_eth_dev *dev,
> >  		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> >
> >  	if (ixgbe_vmdq_mode_check(hw) < 0)
> > -		return (-ENOTSUP);
> > +		return -ENOTSUP;
> > +
> > +	if (rule_id >= IXGBE_MAX_MIRROR_RULES)
> > +		return -EINVAL;
> >
> >  	/* Check if vlan mask is valid */
> >  	if ((mirror_conf->rule_type_mask & ETH_VMDQ_VLAN_MIRROR)
> && (on)) {
> > @@ -3526,7 +3529,7 @@ ixgbe_mirror_rule_reset(struct rte_eth_dev
> *dev,
> > uint8_t rule_id)
> >  		return (-ENOTSUP);
> >
> >  	memset(&mr_info->mr_conf[rule_id], 0,
> > -		sizeof(struct rte_eth_vmdq_mirror_conf));
> > +		sizeof(struct rte_eth_mirror_conf));
> >
> >  	/* clear PFVMCTL register */
> >  	IXGBE_WRITE_REG(hw, IXGBE_MRCTL(rule_id), mr_ctl); diff --git
> > a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> > index 19237b8..755b674 100644
> > --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> > +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> > @@ -177,8 +177,10 @@ struct ixgbe_uta_info {
> >  	uint32_t uta_shadow[IXGBE_MAX_UTA];
> >  };
> >
> > +#define IXGBE_MAX_MIRROR_RULES 4  /* Maximum nb. of mirror rules.
> */
> > +
> >  struct ixgbe_mirror_info {
> > -	struct rte_eth_vmdq_mirror_conf
> > mr_conf[ETH_VMDQ_NUM_MIRROR_RULE];
> > +	struct rte_eth_mirror_conf mr_conf[IXGBE_MAX_MIRROR_RULES];
> >  	/**< store PF mirror rules configuration*/  };
> >
> > diff --git a/lib/librte_ether/rte_ethdev.c
> > b/lib/librte_ether/rte_ethdev.c index 024fe8b..43c7295 100644
> > --- a/lib/librte_ether/rte_ethdev.c
> > +++ b/lib/librte_ether/rte_ethdev.c
> > @@ -3034,7 +3034,7 @@ int rte_eth_set_vf_rate_limit(uint8_t port_id,
> > uint16_t vf, uint16_t tx_rate,
> >
> >  int
> >  rte_eth_mirror_rule_set(uint8_t port_id,
> > -			struct rte_eth_vmdq_mirror_conf *mirror_conf,
> > +			struct rte_eth_mirror_conf *mirror_conf,
> >  			uint8_t rule_id, uint8_t on)
> >  {
> >  	struct rte_eth_dev *dev = &rte_eth_devices[port_id]; @@ -3051,7
> > +3051,7 @@ rte_eth_mirror_rule_set(uint8_t port_id,
> >
> >  	if (mirror_conf->dst_pool >= ETH_64_POOLS) {
> >  		PMD_DEBUG_TRACE("Invalid dst pool, pool id must"
> > -			"be 0-%d\n",ETH_64_POOLS - 1);
> > +			"be 0-%d\n", ETH_64_POOLS - 1);
> >  		return -EINVAL;
> >  	}
> >
> > @@ -3062,13 +3062,6 @@ rte_eth_mirror_rule_set(uint8_t port_id,
> >  		return -EINVAL;
> >  	}
> >
> > -	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
> > -	{
> > -		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-
> > %d\n",
> > -			ETH_VMDQ_NUM_MIRROR_RULE - 1);
> > -		return -EINVAL;
> > -	}
> > -
> >  	dev = &rte_eth_devices[port_id];
> >  	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -
> ENOTSUP);
> >
> > @@ -3085,13 +3078,6 @@ rte_eth_mirror_rule_reset(uint8_t port_id,
> > uint8_t rule_id)
> >  		return -ENODEV;
> >  	}
> >
> > -	if(rule_id >= ETH_VMDQ_NUM_MIRROR_RULE)
> > -	{
> > -		PMD_DEBUG_TRACE("Invalid rule_id, rule_id must be 0-
> > %d\n",
> > -			ETH_VMDQ_NUM_MIRROR_RULE-1);
> > -		return -EINVAL;
> > -	}
> > -
> >  	dev = &rte_eth_devices[port_id];
> >  	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_reset, -
> ENOTSUP);
> >
> > diff --git a/lib/librte_ether/rte_ethdev.h
> > b/lib/librte_ether/rte_ethdev.h index 16dbe00..ae22fea 100644
> > --- a/lib/librte_ether/rte_ethdev.h
> > +++ b/lib/librte_ether/rte_ethdev.h
> > @@ -467,8 +467,8 @@ struct rte_eth_rss_conf {
> >  #define ETH_VMDQ_ACCEPT_BROADCAST   0x0008 /**< accept broadcast
> > packets. */
> >  #define ETH_VMDQ_ACCEPT_MULTICAST   0x0010 /**< multicast
> > promiscuous. */
> >
> > -/* Definitions used for VMDQ mirror rules setting */
> > -#define ETH_VMDQ_NUM_MIRROR_RULE     4 /**< Maximum nb. of
> mirror
> > rules. . */
> > +/** Maximum nb. of vlan per mirror rule */
> > +#define ETH_MIRROR_MAX_VLANS       64
> >
> >  #define ETH_VMDQ_POOL_MIRROR    0x0001 /**< Virtual Pool Mirroring.
> */
> >  #define ETH_VMDQ_UPLINK_MIRROR  0x0002 /**< Uplink Port Mirroring.
> > */ @@ -480,18 +480,19 @@ struct rte_eth_rss_conf {
> >   */
> >  struct rte_eth_vlan_mirror {
> >  	uint64_t vlan_mask; /**< mask for valid VLAN ID. */
> > -	uint16_t vlan_id[ETH_VMDQ_MAX_VLAN_FILTERS];
> > -	/** VLAN ID list for vlan mirror. */
> > +	/** VLAN ID list for vlan mirroring. */
> > +	uint16_t vlan_id[ETH_MIRROR_MAX_VLANS];
> >  };
> >
> >  /**
> >   * A structure used to configure traffic mirror of an Ethernet port.
> >   */
> > -struct rte_eth_vmdq_mirror_conf {
> > +struct rte_eth_mirror_conf {
> >  	uint8_t rule_type_mask; /**< Mirroring rule type mask we want to
> set
> > */
> > -	uint8_t dst_pool; /**< Destination pool for this mirror rule. */
> > +	uint8_t dst_pool;  /**< Destination pool for this mirror rule. */
> >  	uint64_t pool_mask; /**< Bitmap of pool for pool mirroring */
> > -	struct rte_eth_vlan_mirror vlan; /**< VLAN ID setting for VLAN
> > mirroring */
> > +	/** VLAN ID setting for VLAN mirroring. */
> > +	struct rte_eth_vlan_mirror vlan;
> >  };
> >
> >  /**
> > @@ -1211,7 +1212,7 @@ typedef int (*eth_set_vf_rate_limit_t)(struct
> > rte_eth_dev *dev,  /**< @internal Set VF TX rate */
> >
> >  typedef int (*eth_mirror_rule_set_t)(struct rte_eth_dev *dev,
> > -				  struct rte_eth_vmdq_mirror_conf
> > *mirror_conf,
> > +				  struct rte_eth_mirror_conf *mirror_conf,
> >  				  uint8_t rule_id,
> >  				  uint8_t on);
> >  /**< @internal Add a traffic mirroring rule on an Ethernet device */
> > @@ -
> > 3168,7 +3169,7 @@ rte_eth_dev_set_vf_vlan_filter(uint8_t port,
> > uint16_t vlan_id,
> >   *   - (-EINVAL) if the mr_conf information is not correct.
> >   */
> >  int rte_eth_mirror_rule_set(uint8_t port_id,
> > -			struct rte_eth_vmdq_mirror_conf *mirror_conf,
> > +			struct rte_eth_mirror_conf *mirror_conf,
> >  			uint8_t rule_id,
> >  			uint8_t on);
> >
> > --
> > 1.9.3

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

* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-06-26  7:03         ` Wu, Jingjing
  2015-07-06  1:27           ` Wu, Jingjing
@ 2015-07-07 14:51           ` Thomas Monjalon
  2015-07-08  0:58             ` Wu, Jingjing
  2015-07-08 10:31             ` Mcnamara, John
  1 sibling, 2 replies; 30+ messages in thread
From: Thomas Monjalon @ 2015-07-07 14:51 UTC (permalink / raw)
  To: Wu, Jingjing; +Cc: dev

2015-06-26 07:03, Wu, Jingjing:
> Hi, Neil
> 
> About this patch I have an ABI concern about it. 
> This patch just renamed a struct rte_eth_vmdq_mirror_conf to
> rte_eth_mirror_conf, the size and its elements don't change.
> As my understanding, it will not break the ABI. And I also tested it.
> But when I use the script ./scripts/validate-abi.sh to check.
> A low severity problem is reported in symbol "rte_eth_mirror_rule_set"
>  - Change: "Base type of 2nd parameter mirror_conf has been changed
>  from struct rte_eth_vmdq_mirror_conf to struct rte_eth_mirror_conf."
>  - Effect: "Replacement of parameter base type may indicate a change
>  in its semantic meaning."
> 
> So, I'm not sure whether this patch meet the ABI policy?

I think it's OK.

> Additional, about the validate-abi.sh, does it mean we need to fix
> all the problems it reports? Or we can decide case by case.
> Can a Low Severity problem be acceptable?

We have to decide case by case.
It makes ABI checking impossible to automate.
That's why any help is welcome to check the git HEAD for ABI violation.

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

* Re: [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver
  2015-06-12  8:44       ` Zhang, Helin
@ 2015-07-07 15:46         ` Thomas Monjalon
  0 siblings, 0 replies; 30+ messages in thread
From: Thomas Monjalon @ 2015-07-07 15:46 UTC (permalink / raw)
  To: Wu, Jingjing; +Cc: dev

> > This patch set enables mirror functionality in i40e driver, and redefines structure
> > and macros used to configure mirror.
> > 
> > v2 changes:
> >  - correct comments style
> >  - add doc change
> > 
> > v3 changes:
> >  - change the mirror rule type to support bit mask
> >  - fix code style
> > 
> > v4 changes:
> >  - correct the rule type check ixgbe
> > 
> > Jingjing Wu (4):
> >   ethdev: rename rte_eth_vmdq_mirror_conf
> >   ethdev: rename and extend the mirror type
> >   i40e: enable mirror functionality in i40e driver
> >   doc: modify the command about mirror in testpmd guide
> 
> Acked-by: Helin Zhang <helin.zhang@intel.com>

Applied, thanks

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

* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-07-07 14:51           ` Thomas Monjalon
@ 2015-07-08  0:58             ` Wu, Jingjing
  2015-07-08 10:31             ` Mcnamara, John
  1 sibling, 0 replies; 30+ messages in thread
From: Wu, Jingjing @ 2015-07-08  0:58 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

Thanks for the clarification.

Jingjing

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, July 07, 2015 10:51 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; nhorman@tuxdriver.com
> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename
> rte_eth_vmdq_mirror_conf
> 
> 2015-06-26 07:03, Wu, Jingjing:
> > Hi, Neil
> >
> > About this patch I have an ABI concern about it.
> > This patch just renamed a struct rte_eth_vmdq_mirror_conf to
> > rte_eth_mirror_conf, the size and its elements don't change.
> > As my understanding, it will not break the ABI. And I also tested it.
> > But when I use the script ./scripts/validate-abi.sh to check.
> > A low severity problem is reported in symbol "rte_eth_mirror_rule_set"
> >  - Change: "Base type of 2nd parameter mirror_conf has been changed
> > from struct rte_eth_vmdq_mirror_conf to struct rte_eth_mirror_conf."
> >  - Effect: "Replacement of parameter base type may indicate a change
> > in its semantic meaning."
> >
> > So, I'm not sure whether this patch meet the ABI policy?
> 
> I think it's OK.
> 
> > Additional, about the validate-abi.sh, does it mean we need to fix all
> > the problems it reports? Or we can decide case by case.
> > Can a Low Severity problem be acceptable?
> 
> We have to decide case by case.
> It makes ABI checking impossible to automate.
> That's why any help is welcome to check the git HEAD for ABI violation.

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

* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-07-07 14:51           ` Thomas Monjalon
  2015-07-08  0:58             ` Wu, Jingjing
@ 2015-07-08 10:31             ` Mcnamara, John
  2015-07-08 10:35               ` Bruce Richardson
  1 sibling, 1 reply; 30+ messages in thread
From: Mcnamara, John @ 2015-07-08 10:31 UTC (permalink / raw)
  To: Thomas Monjalon, Wu, Jingjing; +Cc: dev

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, July 7, 2015 3:51 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename
> rte_eth_vmdq_mirror_conf
> 
> > Additional, about the validate-abi.sh, does it mean we need to fix all
> > the problems it reports? Or we can decide case by case.
> > Can a Low Severity problem be acceptable?
> 
> We have to decide case by case.
> It makes ABI checking impossible to automate.
> That's why any help is welcome to check the git HEAD for ABI violation.

Hi,

Automated testing for ABI breaks is tricky since it requires some interpretation of the errors and the severity level.

However, each report file contains a 2 line summary at the top that can be read and parsed. For example (with long lines wrapped):

    head -2 compat_reports/librte_mbuf.so/v2.0.0_to_ABI_CHECK/compat_report.html

    <!-- kind:binary;verdict:compatible;affected:0;added:1;removed:0;
         type_problems_high:0;type_problems_medium:0;type_problems_low:0;
         interface_problems_high:0;interface_problems_medium:0;
         interface_problems_low:0;changed_constants:0;tool_version:1.99.8.5 -->
    <!-- kind:source;verdict:compatible;affected:0;added:1;removed:0;
         type_problems_high:0;type_problems_medium:0;type_problems_low:0;
         interface_problems_high:0;interface_problems_medium:0;
         interface_problems_low:0;changed_constants:0;tool_version:1.99.8.5 -->

This still requires interpretation but it can be used to search for categories of incompatibility.

To test if a patchset breaks the API it is possible to do something like the following. Say, for example that you have committed 3 commits to your local master, you could tag and run the checker like this:

    git checkout master

    git tag -f ABI_CHECK_AFTER
    git tag -f ABI_CHECK_BEFORE HEAD~3

    ./scripts/validate-abi.sh ABI_CHECK_BEFORE ABI_CHECK_AFTER x86_64-native-linuxapp-gcc

    grep -lr "kind:binary;verdict:incompatible" compat_reports/

The grep could be extended to match other categories that you are interested in. However, it is still probably better to examine the reports manually.

You can also use something this to run a git bisect to find a commit that introduced an ABI incompatibility:

    # Tag the head and some known good point in the history.
    git checkout master
    git tag -f ABI_CHECK_END
    git tag -f ABI_CHECK_START v2.0.0

    # Start git bisect with bad and good.
    git bisect start ABI_CHECK_END ABI_CHECK_START

    # Run bisect with a script.
    git bisect run ~/abi_bisect.sh

Where the ~/abi_bisect.sh script that searches for ABI incompatibilities looks like this:

    #!/bin/sh

    git tag -f ABI_CHECK_END

    rm -rf compat_reports/

    ./scripts/validate-abi.sh ABI_CHECK_START ABI_CHECK_END x86_64-native-linuxapp-gcc

    ! grep -lr "kind:binary;verdict:incompatible" compat_reports/

John.
-- 

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

* Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf
  2015-07-08 10:31             ` Mcnamara, John
@ 2015-07-08 10:35               ` Bruce Richardson
  0 siblings, 0 replies; 30+ messages in thread
From: Bruce Richardson @ 2015-07-08 10:35 UTC (permalink / raw)
  To: Mcnamara, John; +Cc: dev

On Wed, Jul 08, 2015 at 10:31:15AM +0000, Mcnamara, John wrote:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Tuesday, July 7, 2015 3:51 PM
> > To: Wu, Jingjing
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: rename
> > rte_eth_vmdq_mirror_conf
> > 
> > > Additional, about the validate-abi.sh, does it mean we need to fix all
> > > the problems it reports? Or we can decide case by case.
> > > Can a Low Severity problem be acceptable?
> > 
> > We have to decide case by case.
> > It makes ABI checking impossible to automate.
> > That's why any help is welcome to check the git HEAD for ABI violation.
> 
> Hi,
> 
> Automated testing for ABI breaks is tricky since it requires some interpretation of the errors and the severity level.
> 
> However, each report file contains a 2 line summary at the top that can be read and parsed. For example (with long lines wrapped):
> 
>     head -2 compat_reports/librte_mbuf.so/v2.0.0_to_ABI_CHECK/compat_report.html
> 
>     <!-- kind:binary;verdict:compatible;affected:0;added:1;removed:0;
>          type_problems_high:0;type_problems_medium:0;type_problems_low:0;
>          interface_problems_high:0;interface_problems_medium:0;
>          interface_problems_low:0;changed_constants:0;tool_version:1.99.8.5 -->
>     <!-- kind:source;verdict:compatible;affected:0;added:1;removed:0;
>          type_problems_high:0;type_problems_medium:0;type_problems_low:0;
>          interface_problems_high:0;interface_problems_medium:0;
>          interface_problems_low:0;changed_constants:0;tool_version:1.99.8.5 -->
> 
> This still requires interpretation but it can be used to search for categories of incompatibility.
> 
> To test if a patchset breaks the API it is possible to do something like the following. Say, for example that you have committed 3 commits to your local master, you could tag and run the checker like this:
> 
>     git checkout master
> 
>     git tag -f ABI_CHECK_AFTER
>     git tag -f ABI_CHECK_BEFORE HEAD~3
> 
>     ./scripts/validate-abi.sh ABI_CHECK_BEFORE ABI_CHECK_AFTER x86_64-native-linuxapp-gcc
> 
>     grep -lr "kind:binary;verdict:incompatible" compat_reports/
> 
> The grep could be extended to match other categories that you are interested in. However, it is still probably better to examine the reports manually.
> 
> You can also use something this to run a git bisect to find a commit that introduced an ABI incompatibility:
> 
>     # Tag the head and some known good point in the history.
>     git checkout master
>     git tag -f ABI_CHECK_END
>     git tag -f ABI_CHECK_START v2.0.0
> 
>     # Start git bisect with bad and good.
>     git bisect start ABI_CHECK_END ABI_CHECK_START
> 
>     # Run bisect with a script.
>     git bisect run ~/abi_bisect.sh
> 
> Where the ~/abi_bisect.sh script that searches for ABI incompatibilities looks like this:
> 
>     #!/bin/sh
> 
>     git tag -f ABI_CHECK_END
> 
>     rm -rf compat_reports/
> 
>     ./scripts/validate-abi.sh ABI_CHECK_START ABI_CHECK_END x86_64-native-linuxapp-gcc
> 
>     ! grep -lr "kind:binary;verdict:incompatible" compat_reports/
> 
> John.
> -- 

I'd mod this post up as "+1 informative", except that this isn't slashdot. :-)

Very helpful, John. Thanks.

/Bruce

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

end of thread, other threads:[~2015-07-08 10:35 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-13  8:47 [dpdk-dev] [PATCH 0/3] enable mirror functionality in i40e driver Jingjing Wu
2015-05-13  8:47 ` [dpdk-dev] [PATCH 1/3] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
2015-06-02  2:50   ` Liu, Jijiang
2015-05-13  8:47 ` [dpdk-dev] [PATCH 2/3] ethdev: redefine the mirror type Jingjing Wu
2015-05-13  8:47 ` [dpdk-dev] [PATCH 3/3] i40e: enable mirror functionality in i40e driver Jingjing Wu
2015-06-02  7:55 ` [dpdk-dev] [PATCH v2 0/4] " Jingjing Wu
2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 2/4] ethdev: redefine the mirror type Jingjing Wu
2015-06-02  7:55   ` [dpdk-dev] [PATCH v2 3/4] i40e: enable mirror functionality in i40e driver Jingjing Wu
2015-06-02  7:56   ` [dpdk-dev] [PATCH v2 4/4] doc: modify the command about mirror in testpmd guide Jingjing Wu
2015-06-05  8:16   ` [dpdk-dev] [PATCH v3 0/4] enable mirror functionality in i40e driver Jingjing Wu
2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 2/4] ethdev: redefine the mirror type Jingjing Wu
2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 3/4] i40e: enable mirror functionality in i40e driver Jingjing Wu
2015-06-05  8:16     ` [dpdk-dev] [PATCH v3 4/4] doc: modify the command about mirror in testpmd guide Jingjing Wu
2015-06-10  6:24     ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jingjing Wu
2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 1/4] ethdev: rename rte_eth_vmdq_mirror_conf Jingjing Wu
2015-06-26  7:03         ` Wu, Jingjing
2015-07-06  1:27           ` Wu, Jingjing
2015-07-07 14:51           ` Thomas Monjalon
2015-07-08  0:58             ` Wu, Jingjing
2015-07-08 10:31             ` Mcnamara, John
2015-07-08 10:35               ` Bruce Richardson
2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 2/4] ethdev: rename and extend the mirror type Jingjing Wu
2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 3/4] i40e: enable mirror functionality in i40e driver Jingjing Wu
2015-06-10  6:24       ` [dpdk-dev] [PATCH v4 4/4] doc: modify the command about mirror in testpmd guide Jingjing Wu
2015-06-10  8:33       ` [dpdk-dev] [PATCH v4 0/4] enable mirror functionality in i40e driver Jiajia, SunX
2015-06-12  8:18       ` Liu, Jijiang
2015-06-12  8:44       ` Zhang, Helin
2015-07-07 15:46         ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).