[dpdk-dev] [PATCH v3 2/4] ethdev: redefine the mirror type

[Jingjing Wu <jingjing.wu@intel.com>]

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