All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry
@ 2016-07-27  5:49 Kiran Patil
  2016-07-27  6:01 ` Jeff Kirsher
  2016-07-27 15:53 ` Jeff Kirsher
  0 siblings, 2 replies; 6+ messages in thread
From: Kiran Patil @ 2016-07-27  5:49 UTC (permalink / raw)
  To: intel-wired-lan

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 <kiran.patil@intel.com>
Change-Id: I90052508f8c172c0da5a4b78b949704b4a59ea78
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 116 +++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index c912e04..e7613b2 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -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) {
-- 
1.8.3.1


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

* [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry
  2016-07-27  5:49 [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry Kiran Patil
@ 2016-07-27  6:01 ` Jeff Kirsher
  2016-07-27 15:52   ` Wyborny, Carolyn
  2016-07-27 15:53 ` Jeff Kirsher
  1 sibling, 1 reply; 6+ messages in thread
From: Jeff Kirsher @ 2016-07-27  6:01 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, 2016-07-26 at 22:49 -0700, Kiran Patil wrote:
> 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 <kiran.patil@intel.com>
> Change-Id: I90052508f8c172c0da5a4b78b949704b4a59ea78
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> ---
> ?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>

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

* [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry
  2016-07-27  6:01 ` Jeff Kirsher
@ 2016-07-27 15:52   ` Wyborny, Carolyn
  2016-08-02 18:25     ` John Fastabend
  0 siblings, 1 reply; 6+ messages in thread
From: Wyborny, Carolyn @ 2016-07-27 15:52 UTC (permalink / raw)
  To: intel-wired-lan

> -----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 <kiran.patil@intel.com>; intel-wired-lan at lists.osuosl.org;
> Bowers, AndrewX <andrewx.bowers@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part
> of adding flow director entry
> 
> On Tue, 2016-07-26 at 22:49 -0700, Kiran Patil wrote:
> > 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 <kiran.patil@intel.com>
> > Change-Id: I90052508f8c172c0da5a4b78b949704b4a59ea78
> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> > ---
> > ?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.

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

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

* [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry
  2016-07-27  5:49 [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry Kiran Patil
  2016-07-27  6:01 ` Jeff Kirsher
@ 2016-07-27 15:53 ` Jeff Kirsher
  1 sibling, 0 replies; 6+ messages in thread
From: Jeff Kirsher @ 2016-07-27 15:53 UTC (permalink / raw)
  To: intel-wired-lan

On Tue, 2016-07-26 at 22:49 -0700, Kiran Patil wrote:
> 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 <kiran.patil@intel.com>
> Change-Id: I90052508f8c172c0da5a4b78b949704b4a59ea78
> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
> ---
> ?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>

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

* [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry
  2016-07-27 15:52   ` Wyborny, Carolyn
@ 2016-08-02 18:25     ` John Fastabend
  2016-08-02 19:16       ` Patil, Kiran
  0 siblings, 1 reply; 6+ messages in thread
From: John Fastabend @ 2016-08-02 18:25 UTC (permalink / raw)
  To: intel-wired-lan

On 16-07-27 08:52 AM, Wyborny, Carolyn wrote:
>> -----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 <kiran.patil@intel.com>; intel-wired-lan at lists.osuosl.org;
>> Bowers, AndrewX <andrewx.bowers@intel.com>
>> Subject: Re: [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part
>> of adding flow director entry
>>
>> On Tue, 2016-07-26 at 22:49 -0700, Kiran Patil wrote:
>>> 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 <kiran.patil@intel.com>
>>> Change-Id: I90052508f8c172c0da5a4b78b949704b4a59ea78
>>> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
>>> ---
>>>  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.
> 
> 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



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

* [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry
  2016-08-02 18:25     ` John Fastabend
@ 2016-08-02 19:16       ` Patil, Kiran
  0 siblings, 0 replies; 6+ messages in thread
From: Patil, Kiran @ 2016-08-02 19:16 UTC (permalink / raw)
  To: intel-wired-lan

On 8/2/2016 11:25 AM, John Fastabend wrote:
> On 16-07-27 08:52 AM, Wyborny, Carolyn wrote:
>>> -----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 <kiran.patil@intel.com>; intel-wired-lan at lists.osuosl.org;
>>> Bowers, AndrewX <andrewx.bowers@intel.com>
>>> Subject: Re: [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part
>>> of adding flow director entry
>>>
>>> On Tue, 2016-07-26 at 22:49 -0700, Kiran Patil wrote:
>>>> 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 <kiran.patil@intel.com>
>>>> Change-Id: I90052508f8c172c0da5a4b78b949704b4a59ea78
>>>> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
>>>> ---
>>>>   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.
>>
>> 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
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
> may be harder to pull into distributions if you just wanted a simple
> fix like this.
>
> .John
>
>


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

end of thread, other threads:[~2016-08-02 19:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-27  5:49 [Intel-wired-lan] [PATCH V0] i40e: Program device correctly as part of adding flow director entry Kiran Patil
2016-07-27  6:01 ` Jeff Kirsher
2016-07-27 15:52   ` Wyborny, Carolyn
2016-08-02 18:25     ` John Fastabend
2016-08-02 19:16       ` Patil, Kiran
2016-07-27 15:53 ` Jeff Kirsher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.