Skip to contentSkip to conversation

i40e: Program device correctly as part of adding flow director entry

PATCH New Kiran Patil 4 participants 6 messages Jul 27, 2016 – Aug 2, 2016

Subsystems (2)
Network drivers Intel Ethernet
lore
Share & tools
lore

Kiran Patil

author
#0 lore raw

PATCH 1/1 i40e: Program device correctly as part of adding flow director entry

This patch fixes the problem where driver was not programming underlying device correctly when user specifies less than full tuple set as part of adding flow director entry (for flow-type tcp4/udp4) using ethtool.

Signed-off-by Kiran Patil <[email protected]>
Change-Id I90052508f8c172c0da5a4b78b949704b4a59ea78
Tested-by Andrew Bowers <[email protected]>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 116 +++++++++++++++++++++++++
 1 file changed, 116 insertions(+)
drivers/net/ethernet/intel/i40e/i40e_ethtool.c +116 -0
index c912e04..e7613b2 100644
@@ -2563,6 +2563,116 @@ static int i40e_del_fdir_entry(struct i40e_vsi *vsi,
 }
 
 /**
+ * i40e_handle_input_set - Detect and handle input set changes
+ * @vsi: pointer to the targeted VSI
+ * @fsp: pointer to RX flow classification spec
+ *
+ * Reads register, detect change in input set based on existing register
+ * value and what user has passed. Update input set mask register if needed.
+ **/
+static int i40e_handle_input_set(struct i40e_vsi *vsi,
+				 struct ethtool_rx_flow_spec *fsp)
+{
+	bool inset_mask_change = false;
+	struct i40e_pf *pf;
+	u64 val;
+	u8 idx;
+
+	if (unlikely(!vsi))
+		return -EINVAL;
+
+	pf = vsi->back;
+	switch (fsp->flow_type & ~FLOW_EXT) {
+	case TCP_V4_FLOW:
+		idx = I40E_FILTER_PCTYPE_NONF_IPV4_TCP;
+		break;
+	case UDP_V4_FLOW:
+		idx = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
+		break;
+	default:
+		/* for all other flow types */
+		return 0;
+	}
+
+	val = ((u64)i40e_read_rx_ctl(&pf->hw,
+				     I40E_PRTQF_FD_INSET(idx, 1)) << 32) |
+	       (u64)i40e_read_rx_ctl(&pf->hw,
+				     I40E_PRTQF_FD_INSET(idx, 0));
+
+	/* Default input set (TCP/UDP/SCTP) contains following
+	 * fields: srcip + dest ip + src port + dest port
+	 * For SCTP, there is one extra field, "verification tag"
+	 */
+	if (val & I40E_L3_SRC_MASK) {
+		if (!fsp->h_u.tcp_ip4_spec.ip4src) {
+			val &= ~I40E_L3_SRC_MASK;
+			inset_mask_change = true;
+		}
+	} else {
+		if (fsp->h_u.tcp_ip4_spec.ip4src) {
+			val |= I40E_L3_SRC_MASK;
+			inset_mask_change = true;
+		}
+	}
+	if (val & I40E_L3_DST_MASK) {
+		if (!fsp->h_u.tcp_ip4_spec.ip4dst) {
+			val &= ~I40E_L3_DST_MASK;
+			inset_mask_change = true;
+		}
+	} else {
+		if (fsp->h_u.tcp_ip4_spec.ip4dst) {
+			val |= I40E_L3_DST_MASK;
+			inset_mask_change = true;
+		}
+	}
+	if (val & I40E_L4_SRC_MASK) {
+		if (!fsp->h_u.tcp_ip4_spec.psrc) {
+			val &= ~I40E_L4_SRC_MASK;
+			inset_mask_change = true;
+		}
+	} else {
+		if (fsp->h_u.tcp_ip4_spec.psrc) {
+			val |= I40E_L4_SRC_MASK;
+			inset_mask_change = true;
+		}
+	}
+	if (val & I40E_L4_DST_MASK) {
+		if (!fsp->h_u.tcp_ip4_spec.pdst) {
+			val &= ~I40E_L4_DST_MASK;
+			inset_mask_change = true;
+		}
+	} else {
+		if (fsp->h_u.tcp_ip4_spec.pdst) {
+			val |= I40E_L4_DST_MASK;
+			inset_mask_change = true;
+		}
+	}
+
+	if (inset_mask_change) {
+		if (pf->flags & I40E_FLAG_MFP_ENABLED) {
+			netif_err(pf, drv, vsi->netdev, "Change of input set is not supported when MFP mode is enabled\n");
+			return -EOPNOTSUPP;
+		}
+		if (pf->fdir_pf_active_filters) {
+			netif_err(pf, drv, vsi->netdev, "Change of input set is not supported when there are existing filters. Please delete them and re-try\n");
+			return -EOPNOTSUPP;
+		}
+
+		if (I40E_DEBUG_FD & pf->hw.debug_mask)
+			netif_info(pf, drv, vsi->netdev, "FD_INSET mask is changing to 0x%016llx\n",
+				   val);
+		/* Update input mask register since input set mask changed */
+		i40e_write_rx_ctl(&pf->hw, I40E_PRTQF_FD_INSET(idx, 1),
+				  (u32)(val >> 32));
+		i40e_write_rx_ctl(&pf->hw, I40E_PRTQF_FD_INSET(idx, 0),
+				  (u32)val);
+		netif_info(pf, drv, vsi->netdev, "Input set mask change has been successful. Please note that this and all other interfaces on (related and derived from) this part are affected as well.\n");
+	}
+
+	return 0;
+}
+
+/**
  * i40e_add_fdir_ethtool - Add/Remove Flow Director filters
  * @vsi: pointer to the targeted VSI
  * @cmd: command to get or set RX flow classification rules

@@ -2637,6 +2747,12 @@ static int i40e_add_fdir_ethtool(struct i40e_vsi *vsi,
 	input->dst_ip[0] = fsp->h_u.tcp_ip4_spec.ip4src;
 	input->src_ip[0] = fsp->h_u.tcp_ip4_spec.ip4dst;
 
+	ret = i40e_handle_input_set(vsi, fsp);
+	if (ret) {
+		netif_err(pf, drv, vsi->netdev, "Unable to handle change in input set mask\n");
+		goto free_input;
+	}
+
 	if (ntohl(fsp->m_ext.data[1])) {
 		vf_id = ntohl(fsp->h_ext.data[1]);
 		if (vf_id >= pf->num_alloc_vfs) {
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH 1/1 i40e: Program device correctly as part of adding flow director entry

Quoted 5 lines · Kiran Patil

This patch fixes the problem where driver was not programming underlying device correctly when user specifies less than full tuple set as part of adding flow director entry (for flow-type tcp4/udp4) using ethtool.

Quoted 7 lines
Signed-off-by Kiran Patil <[email protected]>
Change-Id I90052508f8c172c0da5a4b78b949704b4a59ea78
Tested-by Andrew Bowers <[email protected]>

---
?drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 116
+++++++++++++++++++++++++
?1 file changed, 116 insertions(+)

Andrew, I know your tested-by is already on this, but I also know it was
not tested while in my dev-queue branch of my next-queue tree. ?Can you
please verify this patch in my tree? ?No need to send another tested-by,
just let me know if your tested-by still stands.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20160726/514e49c4/attachment.asc>

#2 lore raw

PATCH 1/1 i40e: Program device correctly as part of adding flow director entry

Quoted 23 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jeff Kirsher
Sent: Tuesday, July 26, 2016 11:02 PM
To: Patil, Kiran <[email protected]>; intel-wired-lan at lists.osuosl.org; Bowers, AndrewX <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry

Quoted 5 lines · Kiran Patil

This patch fixes the problem where driver was not programming underlying device correctly when user specifies less than full tuple set as part of adding flow director entry (for flow-type tcp4/udp4) using ethtool.

Quoted 7 lines
Signed-off-by Kiran Patil <[email protected]>
Change-Id I90052508f8c172c0da5a4b78b949704b4a59ea78
Tested-by Andrew Bowers <[email protected]>

---
?drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 116
+++++++++++++++++++++++++
?1 file changed, 116 insertions(+)

Quoted 4 lines

Andrew, I know your tested-by is already on this, but I also know it was not tested while in my dev-queue branch of my next-queue tree. ?Can you please verify this patch in my tree? ?No need to send another tested-by, just let me know if your tested-by still stands.

Please drop this patch. The i40e team is redesigning this feature with a couple of others and it will be submitted at a future time.

Thanks,

Carolyn

#3 lore raw

PATCH 1/1 i40e: Program device correctly as part of adding flow director entry

Quoted 28 lines · Carolyn
Quoted 23 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jeff Kirsher
Sent: Tuesday, July 26, 2016 11:02 PM
To: Patil, Kiran <[email protected]>; intel-wired-lan at lists.osuosl.org; Bowers, AndrewX <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry

Quoted 5 lines · Kiran Patil

This patch fixes the problem where driver was not programming underlying device correctly when user specifies less than full tuple set as part of adding flow director entry (for flow-type tcp4/udp4) using ethtool.

Quoted 7 lines
Signed-off-by Kiran Patil <[email protected]>
Change-Id I90052508f8c172c0da5a4b78b949704b4a59ea78
Tested-by Andrew Bowers <[email protected]>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 116
+++++++++++++++++++++++++
 1 file changed, 116 insertions(+)
Quoted 4 lines

Andrew, I know your tested-by is already on this, but I also know it was not tested while in my dev-queue branch of my next-queue tree. Can you please verify this patch in my tree? No need to send another tested-by, just let me know if your tested-by still stands.

Quoted 7 lines

Please drop this patch. The i40e team is redesigning this feature with a couple of others and it will be submitted at a future time.

Thanks,

Carolyn
_____________

But wasn't this a bugfix? Shouldn't the bug fix go out now or can I expect the redesign shortly. Another thing to consider is a redesign may be harder to pull into distributions if you just wanted a simple fix like this.

.John

#4 lore raw

PATCH 1/1 i40e: Program device correctly as part of adding flow director entry

Quoted 32 lines · John Fastabend
Quoted 31 lines · Carolyn
Quoted 23 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Jeff Kirsher
Sent: Tuesday, July 26, 2016 11:02 PM
To: Patil, Kiran <[email protected]>; intel-wired-lan at lists.osuosl.org; Bowers, AndrewX <[email protected]>
Subject: Re: [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry

Quoted 5 lines · Kiran Patil

This patch fixes the problem where driver was not programming underlying device correctly when user specifies less than full tuple set as part of adding flow director entry (for flow-type tcp4/udp4) using ethtool.

Quoted 7 lines
Signed-off-by Kiran Patil <[email protected]>
Change-Id I90052508f8c172c0da5a4b78b949704b4a59ea78
Tested-by Andrew Bowers <[email protected]>
---
  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 116
+++++++++++++++++++++++++
  1 file changed, 116 insertions(+)
Quoted 4 lines

Andrew, I know your tested-by is already on this, but I also know it was not tested while in my dev-queue branch of my next-queue tree. Can you please verify this patch in my tree? No need to send another tested-by, just let me know if your tested-by still stands.

Please drop this patch. The i40e team is redesigning this feature with a couple of others and it will be submitted at a future time.

Quoted 7 lines
Quoted 4 lines

Thanks,

Carolyn
_____________

But wasn't this a bugfix? Shouldn't the bug fix go out now or can I expect the redesign shortly. Another thing to consider is a redesign

Redesign will address generic problem, implementation of flow-director (w.r.t. mask) whereas this patch is isolated and fixing the bug and it will still be required.
-- Kiran

Quoted 6 lines

may be harder to pull into distributions if you just wanted a simple fix like this.

.John

#5 lore raw

PATCH 1/1 i40e: Program device correctly as part of adding flow director entry

In reply to Kiran Patil
Quoted 5 lines · Kiran Patil

This patch fixes the problem where driver was not programming underlying device correctly when user specifies less than full tuple set as part of adding flow director entry (for flow-type tcp4/udp4) using ethtool.

Quoted 7 lines
Signed-off-by Kiran Patil <[email protected]>
Change-Id I90052508f8c172c0da5a4b78b949704b4a59ea78
Tested-by Andrew Bowers <[email protected]>

---
?drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 116
+++++++++++++++++++++++++
?1 file changed, 116 insertions(+)

For public consumption, dropping this patch because a different approach is
being taken to resolve this issue.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20160727/a3cc89db/attachment.asc>