[dpdk-dev] [PATCH v2] ethdev: fix byte order inconsistence between fdir flow and mask

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

Fixed issue of byte order in ethdev library that the structure
for setting fdir's mask and flow entry is inconsist and made
inputs of mask be in big endian.

Fixes: 76c6f89e80d4 ("ixgbe: support new flow director masks")
Fixes: 2d4c1a9ea2ac ("ethdev: add new flow director masks")

Reported-by: Yaacov Hazan <yaacovh@mellanox.com>
Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
v2 changes:
  fix typo and reword API doc.

 app/test-pmd/cmdline.c               |  6 ++---
 doc/guides/rel_notes/release_2_3.rst |  6 +++++
 drivers/net/ixgbe/ixgbe_fdir.c       | 47 ++++++++++++++++++++++--------------
 lib/librte_ether/rte_eth_ctrl.h      | 17 ++++++++++---
 4 files changed, 51 insertions(+), 25 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 73298c9..13194c9 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -8687,13 +8687,13 @@ cmd_flow_director_mask_parsed(void *parsed_result,
 			return;
 		}
 
-		mask->vlan_tci_mask = res->vlan_mask;
+		mask->vlan_tci_mask = rte_cpu_to_be_16(res->vlan_mask);
 		IPV4_ADDR_TO_UINT(res->ipv4_src, mask->ipv4_mask.src_ip);
 		IPV4_ADDR_TO_UINT(res->ipv4_dst, mask->ipv4_mask.dst_ip);
 		IPV6_ADDR_TO_ARRAY(res->ipv6_src, mask->ipv6_mask.src_ip);
 		IPV6_ADDR_TO_ARRAY(res->ipv6_dst, mask->ipv6_mask.dst_ip);
-		mask->src_port_mask = res->port_src;
-		mask->dst_port_mask = res->port_dst;
+		mask->src_port_mask = rte_cpu_to_be_16(res->port_src);
+		mask->dst_port_mask = rte_cpu_to_be_16(res->port_dst);
 	}
 
 	cmd_reconfig_device_queue(res->port_id, 1, 1);
diff --git a/doc/guides/rel_notes/release_2_3.rst b/doc/guides/rel_notes/release_2_3.rst
index 99de186..77e4edd 100644
--- a/doc/guides/rel_notes/release_2_3.rst
+++ b/doc/guides/rel_notes/release_2_3.rst
@@ -19,6 +19,10 @@ Drivers
 Libraries
 ~~~~~~~~~
 
+* ** fix byte order inconsistence between fdir flow and mask **
+
+  Fixed issue in ethdev library that the structure for setting
+  fdir's mask and flow entry is inconsist in byte order.
 
 Examples
 ~~~~~~~~
@@ -39,6 +43,8 @@ API Changes
 ABI Changes
 -----------
 
+* The fields in ethdev structure ``rte_eth_fdir_masks`` were
+  changed to be in big endian.
 
 Shared Library Versions
 -----------------------
diff --git a/drivers/net/ixgbe/ixgbe_fdir.c b/drivers/net/ixgbe/ixgbe_fdir.c
index e03219b..2c874b1 100644
--- a/drivers/net/ixgbe/ixgbe_fdir.c
+++ b/drivers/net/ixgbe/ixgbe_fdir.c
@@ -309,6 +309,7 @@ fdir_set_input_mask_82599(struct rte_eth_dev *dev,
 	uint32_t fdiripv6m; /* IPv6 source and destination masks. */
 	uint16_t dst_ipv6m = 0;
 	uint16_t src_ipv6m = 0;
+	volatile uint32_t *reg;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -322,16 +323,16 @@ fdir_set_input_mask_82599(struct rte_eth_dev *dev,
 		/* use the L4 protocol mask for raw IPv4/IPv6 traffic */
 		fdirm |= IXGBE_FDIRM_L4P;
 
-	if (input_mask->vlan_tci_mask == 0x0FFF)
+	if (input_mask->vlan_tci_mask == rte_cpu_to_be_16(0x0FFF))
 		/* mask VLAN Priority */
 		fdirm |= IXGBE_FDIRM_VLANP;
-	else if (input_mask->vlan_tci_mask == 0xE000)
+	else if (input_mask->vlan_tci_mask == rte_cpu_to_be_16(0xE000))
 		/* mask VLAN ID */
 		fdirm |= IXGBE_FDIRM_VLANID;
 	else if (input_mask->vlan_tci_mask == 0)
 		/* mask VLAN ID and Priority */
 		fdirm |= IXGBE_FDIRM_VLANID | IXGBE_FDIRM_VLANP;
-	else if (input_mask->vlan_tci_mask != 0xEFFF) {
+	else if (input_mask->vlan_tci_mask != rte_cpu_to_be_16(0xEFFF)) {
 		PMD_INIT_LOG(ERR, "invalid vlan_tci_mask");
 		return -EINVAL;
 	}
@@ -340,19 +341,26 @@ fdir_set_input_mask_82599(struct rte_eth_dev *dev,
 	IXGBE_WRITE_REG(hw, IXGBE_FDIRM, fdirm);
 
 	/* store the TCP/UDP port masks, bit reversed from port layout */
-	fdirtcpm = reverse_fdir_bitmasks(input_mask->dst_port_mask,
-					 input_mask->src_port_mask);
+	fdirtcpm = reverse_fdir_bitmasks(
+			rte_be_to_cpu_16(input_mask->dst_port_mask),
+			rte_be_to_cpu_16(input_mask->src_port_mask));
 
-	/* write all the same so that UDP, TCP and SCTP use the same mask */
+	/* write all the same so that UDP, TCP and SCTP use the same mask
+	 * (little-endian)
+	 */
 	IXGBE_WRITE_REG(hw, IXGBE_FDIRTCPM, ~fdirtcpm);
 	IXGBE_WRITE_REG(hw, IXGBE_FDIRUDPM, ~fdirtcpm);
 	IXGBE_WRITE_REG(hw, IXGBE_FDIRSCTPM, ~fdirtcpm);
 	info->mask.src_port_mask = input_mask->src_port_mask;
 	info->mask.dst_port_mask = input_mask->dst_port_mask;
 
-	/* Store source and destination IPv4 masks (big-endian) */
-	IXGBE_WRITE_REG(hw, IXGBE_FDIRSIP4M, ~(input_mask->ipv4_mask.src_ip));
-	IXGBE_WRITE_REG(hw, IXGBE_FDIRDIP4M, ~(input_mask->ipv4_mask.dst_ip));
+	/* Store source and destination IPv4 masks (big-endian),
+	 * can not use IXGBE_WRITE_REG.
+	 */
+	reg = IXGBE_PCI_REG_ADDR(hw, IXGBE_FDIRSIP4M);
+	*reg = ~(input_mask->ipv4_mask.src_ip);
+	reg = IXGBE_PCI_REG_ADDR(hw, IXGBE_FDIRDIP4M);
+	*reg = ~(input_mask->ipv4_mask.dst_ip);
 	info->mask.src_ipv4_mask = input_mask->ipv4_mask.src_ip;
 	info->mask.dst_ipv4_mask = input_mask->ipv4_mask.dst_ip;
 
@@ -401,16 +409,16 @@ fdir_set_input_mask_x550(struct rte_eth_dev *dev,
 	/* some bits must be set for mac vlan or tunnel mode */
 	fdirm |= IXGBE_FDIRM_L4P | IXGBE_FDIRM_L3P;
 
-	if (input_mask->vlan_tci_mask == 0x0FFF)
+	if (input_mask->vlan_tci_mask == rte_cpu_to_be_16(0x0FFF))
 		/* mask VLAN Priority */
 		fdirm |= IXGBE_FDIRM_VLANP;
-	else if (input_mask->vlan_tci_mask == 0xE000)
+	else if (input_mask->vlan_tci_mask == rte_cpu_to_be_16(0xE000))
 		/* mask VLAN ID */
 		fdirm |= IXGBE_FDIRM_VLANID;
 	else if (input_mask->vlan_tci_mask == 0)
 		/* mask VLAN ID and Priority */
 		fdirm |= IXGBE_FDIRM_VLANID | IXGBE_FDIRM_VLANP;
-	else if (input_mask->vlan_tci_mask != 0xEFFF) {
+	else if (input_mask->vlan_tci_mask != rte_cpu_to_be_16(0xEFFF)) {
 		PMD_INIT_LOG(ERR, "invalid vlan_tci_mask");
 		return -EINVAL;
 	}
@@ -444,7 +452,7 @@ fdir_set_input_mask_x550(struct rte_eth_dev *dev,
 		info->mask.tunnel_type_mask =
 			input_mask->tunnel_type_mask;
 
-		switch (input_mask->tunnel_id_mask & 0xFFFFFFFF) {
+		switch (rte_be_to_cpu_32(input_mask->tunnel_id_mask)) {
 		case 0x0:
 			/* Mask vxlan id */
 			fdiripv6m |= IXGBE_FDIRIP6M_TNI_VNI;
@@ -904,13 +912,16 @@ fdir_write_perfect_filter_82599(struct ixgbe_hw *hw,
 	u32 addr_low, addr_high;
 	u32 tunnel_type = 0;
 	int err = 0;
+	volatile uint32_t *reg;
 
 	if (mode == RTE_FDIR_MODE_PERFECT) {
-		/* record the IPv4 address (big-endian) */
-		IXGBE_WRITE_REG(hw, IXGBE_FDIRIPSA,
-				input->formatted.src_ip[0]);
-		IXGBE_WRITE_REG(hw, IXGBE_FDIRIPDA,
-				input->formatted.dst_ip[0]);
+		/* record the IPv4 address (big-endian)
+		 * can not use IXGBE_WRITE_REG.
+		 */
+		reg = IXGBE_PCI_REG_ADDR(hw, IXGBE_FDIRIPSA);
+		*reg = input->formatted.src_ip[0];
+		reg = IXGBE_PCI_REG_ADDR(hw, IXGBE_FDIRIPDA);
+		*reg = input->formatted.dst_ip[0];
 
 		/* record source and destination port (little-endian)*/
 		fdirport = IXGBE_NTOHS(input->formatted.dst_port);
diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index ce224ad..d433e0b 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -501,6 +501,7 @@ struct rte_eth_tunnel_flow {
 
 /**
  * An union contains the inputs for all types of flow
+ * Items in flows need to be in big endian
  */
 union rte_eth_fdir_flow {
 	struct rte_eth_l2_flow     l2_flow;
@@ -588,14 +589,22 @@ struct rte_eth_fdir_filter {
  *  to match the various fields of RX packet headers.
  */
 struct rte_eth_fdir_masks {
-	uint16_t vlan_tci_mask;
+	uint16_t vlan_tci_mask;   /**< Bit mask for vlan_tci in big endian */
+	/** Bit mask for ipv4 flow in big endian. */
 	struct rte_eth_ipv4_flow   ipv4_mask;
+	/** Bit maks for ipv6 flow in big endian. */
 	struct rte_eth_ipv6_flow   ipv6_mask;
+	/** Bit mask for L4 source port in big endian. */
 	uint16_t src_port_mask;
+	/** Bit mask for L4 destination port in big endian. */
 	uint16_t dst_port_mask;
-	uint8_t mac_addr_byte_mask;  /** Per byte MAC address mask */
-	uint32_t tunnel_id_mask;  /** tunnel ID mask */
-	uint8_t tunnel_type_mask;
+	/** 6 bit mask for proper 6 bytes of Mac address, bit 0 matches the
+	    first byte on the wire */
+	uint8_t mac_addr_byte_mask;
+	/** Bit mask for tunnel ID in big endian. */
+	uint32_t tunnel_id_mask;
+	uint8_t tunnel_type_mask; /**< 1 - Match tunnel type,
+				       0 - Ignore tunnel type. */
 };
 
 /**
-- 
2.4.0