* [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC
@ 2016-08-19 21:32 Alexander Duyck
2016-08-19 22:05 ` Greg
2016-08-21 0:21 ` Ben Hutchings
0 siblings, 2 replies; 6+ messages in thread
From: Alexander Duyck @ 2016-08-19 21:32 UTC (permalink / raw)
To: netdev; +Cc: davem
The i40e hardware has support for SCTP filtering via Rx NFC however the
default configuration expects us to include the verification tag as a part
of the filter. In order to support that I need to be able to transfer that
data through the ethtool interface via a new structure.
This patch adds a new structure to allow us to pass the verification tag
for IPv4 or IPv6 SCTP traffic.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
include/uapi/linux/ethtool.h | 50 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
index b8f38e8..12ba8ac 100644
--- a/include/uapi/linux/ethtool.h
+++ b/include/uapi/linux/ethtool.h
@@ -708,7 +708,7 @@ enum ethtool_flags {
* @pdst: Destination port
* @tos: Type-of-service
*
- * This can be used to specify a TCP/IPv4, UDP/IPv4 or SCTP/IPv4 flow.
+ * This can be used to specify a TCP/IPv4 or UDP/IPv4 flow.
*/
struct ethtool_tcpip4_spec {
__be32 ip4src;
@@ -719,6 +719,27 @@ struct ethtool_tcpip4_spec {
};
/**
+ * struct ethtool_sctpip4_spec - flow specification for SCTP/IPv4
+ * @ip4src: Source host
+ * @ip4dst: Destination host
+ * @psrc: Source port
+ * @pdst: Destination port
+ * @tos: Type-of-service
+ * @vtag: Verification tag
+ *
+ * This can be used to specify a SCTP/IPv4 flow.
+ */
+struct ethtool_sctpip4_spec {
+ __be32 ip4src;
+ __be32 ip4dst;
+ __be16 psrc;
+ __be16 pdst;
+ __u8 tos;
+ /* 3 byte hole */
+ __be32 vtag;
+};
+
+/**
* struct ethtool_ah_espip4_spec - flow specification for IPsec/IPv4
* @ip4src: Source host
* @ip4dst: Destination host
@@ -762,7 +783,7 @@ struct ethtool_usrip4_spec {
* @pdst: Destination port
* @tclass: Traffic Class
*
- * This can be used to specify a TCP/IPv6, UDP/IPv6 or SCTP/IPv6 flow.
+ * This can be used to specify a TCP/IPv6 or UDP/IPv6 flow.
*/
struct ethtool_tcpip6_spec {
__be32 ip6src[4];
@@ -773,6 +794,27 @@ struct ethtool_tcpip6_spec {
};
/**
+ * struct ethtool_sctpip6_spec - flow specification for SCTP/IPv6
+ * @ip6src: Source host
+ * @ip6dst: Destination host
+ * @psrc: Source port
+ * @pdst: Destination port
+ * @tclass: Traffic Class
+ * @vtag: Verification tag
+ *
+ * This can be used to specify a SCTP/IPv6 flow.
+ */
+struct ethtool_sctpip6_spec {
+ __be32 ip6src[4];
+ __be32 ip6dst[4];
+ __be16 psrc;
+ __be16 pdst;
+ __u8 tclass;
+ /* 3 byte hole */
+ __be32 vtag;
+};
+
+/**
* struct ethtool_ah_espip6_spec - flow specification for IPsec/IPv6
* @ip6src: Source host
* @ip6dst: Destination host
@@ -807,13 +849,13 @@ struct ethtool_usrip6_spec {
union ethtool_flow_union {
struct ethtool_tcpip4_spec tcp_ip4_spec;
struct ethtool_tcpip4_spec udp_ip4_spec;
- struct ethtool_tcpip4_spec sctp_ip4_spec;
+ struct ethtool_sctpip4_spec sctp_ip4_spec;
struct ethtool_ah_espip4_spec ah_ip4_spec;
struct ethtool_ah_espip4_spec esp_ip4_spec;
struct ethtool_usrip4_spec usr_ip4_spec;
struct ethtool_tcpip6_spec tcp_ip6_spec;
struct ethtool_tcpip6_spec udp_ip6_spec;
- struct ethtool_tcpip6_spec sctp_ip6_spec;
+ struct ethtool_sctpip6_spec sctp_ip6_spec;
struct ethtool_ah_espip6_spec ah_ip6_spec;
struct ethtool_ah_espip6_spec esp_ip6_spec;
struct ethtool_usrip6_spec usr_ip6_spec;
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC
2016-08-19 21:32 [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC Alexander Duyck
@ 2016-08-19 22:05 ` Greg
2016-08-21 0:21 ` Ben Hutchings
1 sibling, 0 replies; 6+ messages in thread
From: Greg @ 2016-08-19 22:05 UTC (permalink / raw)
To: Alexander Duyck; +Cc: netdev, davem
On Fri, 2016-08-19 at 14:32 -0700, Alexander Duyck wrote:
> The i40e hardware has support for SCTP filtering via Rx NFC however the
> default configuration expects us to include the verification tag as a part
> of the filter. In order to support that I need to be able to transfer that
> data through the ethtool interface via a new structure.
>
> This patch adds a new structure to allow us to pass the verification tag
> for IPv4 or IPv6 SCTP traffic.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> include/uapi/linux/ethtool.h | 50 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> index b8f38e8..12ba8ac 100644
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -708,7 +708,7 @@ enum ethtool_flags {
> * @pdst: Destination port
> * @tos: Type-of-service
> *
> - * This can be used to specify a TCP/IPv4, UDP/IPv4 or SCTP/IPv4 flow.
> + * This can be used to specify a TCP/IPv4 or UDP/IPv4 flow.
> */
> struct ethtool_tcpip4_spec {
> __be32 ip4src;
> @@ -719,6 +719,27 @@ struct ethtool_tcpip4_spec {
> };
>
> /**
> + * struct ethtool_sctpip4_spec - flow specification for SCTP/IPv4
> + * @ip4src: Source host
> + * @ip4dst: Destination host
> + * @psrc: Source port
> + * @pdst: Destination port
> + * @tos: Type-of-service
> + * @vtag: Verification tag
> + *
> + * This can be used to specify a SCTP/IPv4 flow.
> + */
> +struct ethtool_sctpip4_spec {
> + __be32 ip4src;
> + __be32 ip4dst;
> + __be16 psrc;
> + __be16 pdst;
> + __u8 tos;
> + /* 3 byte hole */
> + __be32 vtag;
> +};
> +
> +/**
> * struct ethtool_ah_espip4_spec - flow specification for IPsec/IPv4
> * @ip4src: Source host
> * @ip4dst: Destination host
> @@ -762,7 +783,7 @@ struct ethtool_usrip4_spec {
> * @pdst: Destination port
> * @tclass: Traffic Class
> *
> - * This can be used to specify a TCP/IPv6, UDP/IPv6 or SCTP/IPv6 flow.
> + * This can be used to specify a TCP/IPv6 or UDP/IPv6 flow.
> */
> struct ethtool_tcpip6_spec {
> __be32 ip6src[4];
> @@ -773,6 +794,27 @@ struct ethtool_tcpip6_spec {
> };
>
> /**
> + * struct ethtool_sctpip6_spec - flow specification for SCTP/IPv6
> + * @ip6src: Source host
> + * @ip6dst: Destination host
> + * @psrc: Source port
> + * @pdst: Destination port
> + * @tclass: Traffic Class
> + * @vtag: Verification tag
> + *
> + * This can be used to specify a SCTP/IPv6 flow.
> + */
> +struct ethtool_sctpip6_spec {
> + __be32 ip6src[4];
> + __be32 ip6dst[4];
> + __be16 psrc;
> + __be16 pdst;
> + __u8 tclass;
> + /* 3 byte hole */
> + __be32 vtag;
> +};
> +
> +/**
> * struct ethtool_ah_espip6_spec - flow specification for IPsec/IPv6
> * @ip6src: Source host
> * @ip6dst: Destination host
> @@ -807,13 +849,13 @@ struct ethtool_usrip6_spec {
> union ethtool_flow_union {
> struct ethtool_tcpip4_spec tcp_ip4_spec;
> struct ethtool_tcpip4_spec udp_ip4_spec;
> - struct ethtool_tcpip4_spec sctp_ip4_spec;
> + struct ethtool_sctpip4_spec sctp_ip4_spec;
> struct ethtool_ah_espip4_spec ah_ip4_spec;
> struct ethtool_ah_espip4_spec esp_ip4_spec;
> struct ethtool_usrip4_spec usr_ip4_spec;
> struct ethtool_tcpip6_spec tcp_ip6_spec;
> struct ethtool_tcpip6_spec udp_ip6_spec;
> - struct ethtool_tcpip6_spec sctp_ip6_spec;
> + struct ethtool_sctpip6_spec sctp_ip6_spec;
> struct ethtool_ah_espip6_spec ah_ip6_spec;
> struct ethtool_ah_espip6_spec esp_ip6_spec;
> struct ethtool_usrip6_spec usr_ip6_spec;
>
Looks good to me.
Reviewed-by: Greg Rose <grose@lightfleet.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC
2016-08-19 21:32 [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC Alexander Duyck
2016-08-19 22:05 ` Greg
@ 2016-08-21 0:21 ` Ben Hutchings
2016-08-21 1:56 ` Alexander Duyck
1 sibling, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2016-08-21 0:21 UTC (permalink / raw)
To: Alexander Duyck, netdev; +Cc: davem
[-- Attachment #1: Type: text/plain, Size: 782 bytes --]
On Fri, 2016-08-19 at 14:32 -0700, Alexander Duyck wrote:
> The i40e hardware has support for SCTP filtering via Rx NFC however the
> default configuration expects us to include the verification tag as a part
> of the filter. In order to support that I need to be able to transfer that
> data through the ethtool interface via a new structure.
>
> This patch adds a new structure to allow us to pass the verification tag
> for IPv4 or IPv6 SCTP traffic.
[...]
This looks like an incompatible ABI change. I suppose it could be OK
if no drivers implemented flow steering for SCTP using the previously
specified structure, but have you checked that that is the case?
Ben.
--
Ben Hutchings
It's easier to fight for one's principles than to live up to them.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC
2016-08-21 0:21 ` Ben Hutchings
@ 2016-08-21 1:56 ` Alexander Duyck
2016-08-22 13:05 ` Ben Hutchings
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Duyck @ 2016-08-21 1:56 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Alexander Duyck, Netdev, David Miller
On Sat, Aug 20, 2016 at 5:21 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Fri, 2016-08-19 at 14:32 -0700, Alexander Duyck wrote:
>> The i40e hardware has support for SCTP filtering via Rx NFC however the
>> default configuration expects us to include the verification tag as a part
>> of the filter. In order to support that I need to be able to transfer that
>> data through the ethtool interface via a new structure.
>>
>> This patch adds a new structure to allow us to pass the verification tag
>> for IPv4 or IPv6 SCTP traffic.
> [...]
>
> This looks like an incompatible ABI change. I suppose it could be OK
> if no drivers implemented flow steering for SCTP using the previously
> specified structure, but have you checked that that is the case?
>
> Ben.
Well the structure itself matches the TCP flow spec for the TCP flow
sized portion. All I am doing with this patch is adding an extension
to that which is still confined to the 52 byte limit of the flow
union. The net result should be that the new value will appear masked
if anything using the new specifier receives a rule using the old
definition and for anything using the new flow spec that sends to the
old code the extra value will be ignored. I can look into double
checking the behavior to make certain I have that right on Monday.
One thing I could do if you would like would be to spin up another
patch to force the kernel to return -EINVAL if we are masking in
fields that are out of bounds for the flow specification. That way we
can handle this a bit more concisely in the future should we end up
having to extend any other flow specifications.
- Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC
2016-08-21 1:56 ` Alexander Duyck
@ 2016-08-22 13:05 ` Ben Hutchings
2016-08-22 15:32 ` Alexander Duyck
0 siblings, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2016-08-22 13:05 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Alexander Duyck, Netdev, David Miller
[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]
On Sat, 2016-08-20 at 18:56 -0700, Alexander Duyck wrote:
> > On Sat, Aug 20, 2016 at 5:21 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> >
> > On Fri, 2016-08-19 at 14:32 -0700, Alexander Duyck wrote:
> > >
> > > The i40e hardware has support for SCTP filtering via Rx NFC however the
> > > default configuration expects us to include the verification tag as a part
> > > of the filter. In order to support that I need to be able to transfer that
> > > data through the ethtool interface via a new structure.
> > >
> > > This patch adds a new structure to allow us to pass the verification tag
> > > for IPv4 or IPv6 SCTP traffic.
> > [...]
> >
> > This looks like an incompatible ABI change. I suppose it could be OK
> > if no drivers implemented flow steering for SCTP using the previously
> > specified structure, but have you checked that that is the case?
> >
> > Ben.
>
> Well the structure itself matches the TCP flow spec for the TCP flow
> sized portion. All I am doing with this patch is adding an extension
> to that which is still confined to the 52 byte limit of the flow
> > union.
But that extension will be ignored by any drivers that implemented the
API as previously defined. (If there aren't any, as I said, this
doesn't really matter.)
With previous extensions (everything in struct ethtool_flow_ext) we've
introduced new type flags to ensure that they won't be silently
ignored. You could add a new extended-SCTP type value for the same
reason.
[...]
> One thing I could do if you would like would be to spin up another
> patch to force the kernel to return -EINVAL if we are masking in
> fields that are out of bounds for the flow specification. That way we
> can handle this a bit more concisely in the future should we end up
> > having to extend any other flow specifications.
It's too late to do that now.
Ben.
--
Ben Hutchings
Quantity is no substitute for quality, but it's the only one we've got.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC
2016-08-22 13:05 ` Ben Hutchings
@ 2016-08-22 15:32 ` Alexander Duyck
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Duyck @ 2016-08-22 15:32 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Alexander Duyck, Netdev, David Miller
On Mon, Aug 22, 2016 at 6:05 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Sat, 2016-08-20 at 18:56 -0700, Alexander Duyck wrote:
>> > On Sat, Aug 20, 2016 at 5:21 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
>> >
>> > On Fri, 2016-08-19 at 14:32 -0700, Alexander Duyck wrote:
>> > >
>> > > The i40e hardware has support for SCTP filtering via Rx NFC however the
>> > > default configuration expects us to include the verification tag as a part
>> > > of the filter. In order to support that I need to be able to transfer that
>> > > data through the ethtool interface via a new structure.
>> > >
>> > > This patch adds a new structure to allow us to pass the verification tag
>> > > for IPv4 or IPv6 SCTP traffic.
>> > [...]
>> >
>> > This looks like an incompatible ABI change. I suppose it could be OK
>> > if no drivers implemented flow steering for SCTP using the previously
>> > specified structure, but have you checked that that is the case?
>> >
>> > Ben.
>>
>> Well the structure itself matches the TCP flow spec for the TCP flow
>> sized portion. All I am doing with this patch is adding an extension
>> to that which is still confined to the 52 byte limit of the flow
>> > union.
>
> But that extension will be ignored by any drivers that implemented the
> API as previously defined. (If there aren't any, as I said, this
> doesn't really matter.)
The ixgbe driver supports sctp already, and was using the TCP flow
spec to do it.
> With previous extensions (everything in struct ethtool_flow_ext) we've
> introduced new type flags to ensure that they won't be silently
> ignored. You could add a new extended-SCTP type value for the same
> reason.
I guess I could go the extended flow route. I an just define a new
type for SCTP_V4 and SCTP_V6.
> [...]
>> One thing I could do if you would like would be to spin up another
>> patch to force the kernel to return -EINVAL if we are masking in
>> fields that are out of bounds for the flow specification. That way we
>> can handle this a bit more concisely in the future should we end up
>> > having to extend any other flow specifications.
>
> It's too late to do that now.
I disagree. For now what I can do is lock down the existing flow
specifications so nobody tries to cheat and smuggle data in on the
unused space and for the new extended specifications we make sure that
they are included in a mask verification so that if we have to add any
new fields they will already be checked for.
I'll make sure to include such a patch for v2.
Thanks.
- Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-22 15:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-19 21:32 [net-next PATCH] ethtool: Add support for SCTP verification tag in Rx NFC Alexander Duyck
2016-08-19 22:05 ` Greg
2016-08-21 0:21 ` Ben Hutchings
2016-08-21 1:56 ` Alexander Duyck
2016-08-22 13:05 ` Ben Hutchings
2016-08-22 15:32 ` Alexander Duyck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).