From: John Fastabend <john.fastabend@gmail•com>
To: "Patil, Kiran" <kiran.patil@intel•com>,
David Miller <davem@davemloft•net>,
jeffrey.t.kirsher@intel•com
Cc: netdev@vger•kernel.org, nhorman@redhat•com, sassmann@redhat•com,
jogreene@redhat•com
Subject: Re: [net-next 08/15] i40e: Allow user to change input set mask for flow director
Date: Wed, 4 May 2016 20:22:30 -0700 [thread overview]
Message-ID: <572ABC76.2080600@gmail.com> (raw)
In-Reply-To: <572AB43E.2020007@intel.com>
On 16-05-04 07:47 PM, Patil, Kiran wrote:
> On 4/26/2016 8:48 PM, David Miller wrote:
>> From: Jeff Kirsher <jeffrey.t.kirsher@intel•com>
>> Date: Tue, 26 Apr 2016 13:55:41 -0700
>>
>>> From: Kiran Patil <kiran.patil@intel•com>
>>>
>>> This patch implements feature, which allows user to change
>>> input set mask for flow director using side-band channel.
>>> This patch adds definition of FLOW_TYPE_MASK into the header file.
>>> With this patch, user can now specify less than 4 tuple(src ip, dsp ip,
>>> src port, dst port) for flow type TCP4/UDP4.
>>>
>>> Change-Id: I90052508f8c172c0da5a4b78b949704b4a59ea78
>>> Signed-off-by: Kiran Patil <kiran.patil@intel•com>
>>> Tested-by: Andrew Bowers <andrewx.bowers@intel•com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel•com>
>>
>> If you want to do this, you have to define the semantics generically
>> in include/uapi/linux/ethtool.h so that other drivers can implement
>> this too.
>>
>> Please don't ever implement private, driver specific, interpretations
>> of the generic ethtool facilitites.
>>
>> The semantics and interpretations of the values must absolutely be
>> consistent across all drivers in the tree.
>>
>> Otherwise the user experience is terrible.
>>
>> Thanks.
>>
> This is not new feature implemented in i40e driver. This is the original
> feature of ethtool which allows user to specify subset of tuple for
> setting up flow director.
>
> i40e driver using it same way as ixgbe.
>
> Please let us know if I misinterpreted your response.
>
> Would you recommend that we re-submit patch with better patch
> description (indicating that it is not new feature but just enabling
> feature).
>
> Thanks,
> -- Kiran P.
At least define FLOW_TYPE_MASK in ethtool.h then its not
sort of hobbled together in the driver and others can use it
instead of the normal ~FLOW_EXT which I see other drivers used.
Another benefit if its near the definition of the flow types
you have a chance of someone seeing it if they add a flag
past 0xff.
And maybe do it as a separate patch. So you aren't adding normal
driver ethtool implementation and a new #define for all drivers
in the same patch.
.John
next prev parent reply other threads:[~2016-05-05 3:22 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-26 20:55 [net-next 00/15][pull request] 40GbE Intel Wired LAN Driver Updates 2016-04-26 Jeff Kirsher
2016-04-26 20:55 ` [net-next 01/15] i40e/i40evf: Clean up feature flags Jeff Kirsher
2016-04-26 20:55 ` [net-next 02/15] i40e/i40evf: Add support for IPIP and SIT offloads Jeff Kirsher
2016-04-26 20:55 ` [net-next 03/15] i40e: Add support for configuring VF RSS Jeff Kirsher
2016-04-26 20:55 ` [net-next 04/15] i40evf: Don't Panic Jeff Kirsher
2016-04-26 20:55 ` [net-next 05/15] i40e: Code cleanup in i40e_add_fdir_ethtool Jeff Kirsher
2016-04-26 20:55 ` [net-next 06/15] i40e: Specify AQ event opcode to wait for Jeff Kirsher
2016-04-26 20:55 ` [net-next 07/15] i40evf: Allow PF driver to configure RSS Jeff Kirsher
2016-04-26 20:55 ` [net-next 08/15] i40e: Allow user to change input set mask for flow director Jeff Kirsher
2016-04-27 3:48 ` David Miller
2016-05-05 2:47 ` Patil, Kiran
2016-05-05 3:22 ` John Fastabend [this message]
2016-05-05 15:00 ` Patil, Kiran
2016-05-05 3:57 ` David Miller
2016-05-05 3:52 ` Alexander Duyck
2016-04-26 20:55 ` [net-next 09/15] i40e: Add device capability which defines if update is available Jeff Kirsher
2016-04-26 20:55 ` [net-next 10/15] i40e: Add DeviceID for X722 QSFP+ Jeff Kirsher
2016-04-26 20:55 ` [net-next 11/15] i40e: Remove zero check Jeff Kirsher
2016-04-26 20:55 ` [net-next 12/15] i40e/i40evf: Only offload VLAN tag if enabled Jeff Kirsher
2016-04-26 20:55 ` [net-next 13/15] i40e: Add promiscuous on VLAN support Jeff Kirsher
2016-04-26 20:55 ` [net-next 14/15] i40e: Add VF promiscuous mode driver support Jeff Kirsher
2016-04-26 20:55 ` [net-next 15/15] i40evf: Add driver support for promiscuous mode Jeff Kirsher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=572ABC76.2080600@gmail.com \
--to=john.fastabend@gmail$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=jeffrey.t.kirsher@intel$(echo .)com \
--cc=jogreene@redhat$(echo .)com \
--cc=kiran.patil@intel$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nhorman@redhat$(echo .)com \
--cc=sassmann@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox