[net-next-2.6 PATCH 0/2] Start of work to cleanup ntuple filters

[net-next-2.6 PATCH 0/2] Start of work to cleanup ntuple filters

[Alexander Duyck]

This patch series is the start of my work to cleanup ntuple filters and the
way they are implemented in ixgbe.  These first two patches lay the groundwork
for moving ixgbe over to using the network flow classifier interface instead
of the ntuple interface.

This will eventually allow for ixgbe to be able to maintain its own rule list,
add support for removal of filters, and will correct the multiple display
issues seen with ntuple filters.

---

Alexander Duyck (2):
      ethtool: add ntuple flow specifier data to network flow classifier
      ethtool: prevent null pointer dereference with NTUPLE set but no set_rx_ntuple


 include/linux/ethtool.h |   52 +++++++++++++++++++++++++++++------------------
 net/core/ethtool.c      |    3 +++
 net/socket.c            |   14 ++++++-------
 3 files changed, 42 insertions(+), 27 deletions(-)

-- 

[net-next-2.6 PATCH 1/2] ethtool: prevent null pointer dereference with NTUPLE set but no set_rx_ntuple

[Alexander Duyck]

This change is meant to prevent a possible null pointer dereference if
NETIF_F_NTUPLE is defined but the set_rx_ntuple function pointer is not.

The main motivation behind this patch is to eventually replace the ntuple
interfaces entirely with the network flow classifier interfaces.  This
allows the device drivers to maintain the ntuple check internally while
using the network flow classifier interface for setting up and displaying
rules.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 net/core/ethtool.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 1b7fa98..704e176 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -910,6 +910,9 @@ static noinline_for_stack int ethtool_set_rx_ntuple(struct net_device *dev,
 	struct ethtool_rx_ntuple_flow_spec_container *fsc = NULL;
 	int ret;
 
+	if (!ops->set_rx_ntuple)
+		return -EOPNOTSUPP;
+
 	if (!(dev->features & NETIF_F_NTUPLE))
 		return -EINVAL;
 

Re: [net-next-2.6 PATCH 1/2] ethtool: prevent null pointer dereference with NTUPLE set but no set_rx_ntuple

[David Miller]

From: Alexander Duyck <alexander.h.duyck@intel.com>
Date: Fri, 08 Apr 2011 15:07:22 -0700

> This change is meant to prevent a possible null pointer dereference if
> NETIF_F_NTUPLE is defined but the set_rx_ntuple function pointer is not.
> 
> The main motivation behind this patch is to eventually replace the ntuple
> interfaces entirely with the network flow classifier interfaces.  This
> allows the device drivers to maintain the ntuple check internally while
> using the network flow classifier interface for setting up and displaying
> rules.
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>

Applied.

[net-next-2.6 PATCH 2/2] ethtool: add ntuple flow specifier data to network flow classifier

[Alexander Duyck]

This change is meant to add an ntuple data extensions to the rx network flow
classification specifiers.  The idea is to allow ntuple to be displayed via
the network flow classification interface.

This work is based on input from Ben Hutchings.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---

 include/linux/ethtool.h |   52 +++++++++++++++++++++++++++++------------------
 net/socket.c            |   14 ++++++-------
 2 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index c04d131..0423d9f 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -380,27 +380,43 @@ struct ethtool_usrip4_spec {
 	__u8    proto;
 };
 
+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_ah_espip4_spec		ah_ip4_spec;
+	struct ethtool_ah_espip4_spec		esp_ip4_spec;
+	struct ethtool_usrip4_spec		usr_ip4_spec;
+	struct ethhdr				ether_spec;
+	__u8					hdata[60];
+};
+
+struct ethtool_flow_ext {
+	__be16	vlan_etype;
+	__be16	vlan_tci;
+	__be32	data[2];
+};
+
 /**
  * struct ethtool_rx_flow_spec - specification for RX flow filter
  * @flow_type: Type of match to perform, e.g. %TCP_V4_FLOW
  * @h_u: Flow fields to match (dependent on @flow_type)
+ * @h_ext: Additional fields to match
  * @m_u: Masks for flow field bits to be ignored
+ * @m_ext: Masks for additional field bits to be ignored.
+ *	Note, all additional fields must be ignored unless @flow_type
+ *	includes the %FLOW_EXT flag.
  * @ring_cookie: RX ring/queue index to deliver to, or %RX_CLS_FLOW_DISC
  *	if packets should be discarded
  * @location: Index of filter in hardware table
  */
 struct ethtool_rx_flow_spec {
 	__u32		flow_type;
-	union {
-		struct ethtool_tcpip4_spec		tcp_ip4_spec;
-		struct ethtool_tcpip4_spec		udp_ip4_spec;
-		struct ethtool_tcpip4_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 ethhdr				ether_spec;
-		__u8					hdata[72];
-	} h_u, m_u;
+	union ethtool_flow_union h_u;
+	struct ethtool_flow_ext h_ext;
+	union ethtool_flow_union m_u;
+	struct ethtool_flow_ext m_ext;
+	__u32		flow_type_ext;
 	__u64		ring_cookie;
 	__u32		location;
 };
@@ -458,16 +474,10 @@ struct ethtool_rxnfc {
 
 struct compat_ethtool_rx_flow_spec {
 	u32		flow_type;
-	union {
-		struct ethtool_tcpip4_spec		tcp_ip4_spec;
-		struct ethtool_tcpip4_spec		udp_ip4_spec;
-		struct ethtool_tcpip4_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 ethhdr				ether_spec;
-		u8					hdata[72];
-	} h_u, m_u;
+	union ethtool_flow_union h_u;
+	struct ethtool_flow_ext h_ext;
+	union ethtool_flow_union m_u;
+	struct ethtool_flow_ext m_ext;
 	compat_u64	ring_cookie;
 	u32		location;
 };
@@ -1072,6 +1082,8 @@ struct ethtool_ops {
 #define	IPV4_FLOW	0x10	/* hash only */
 #define	IPV6_FLOW	0x11	/* hash only */
 #define	ETHER_FLOW	0x12	/* spec only (ether_spec) */
+/* Flag to enable additional fields in struct ethtool_rx_flow_spec */
+#define	FLOW_EXT	0x80000000
 
 /* L3-L4 network traffic flow hash options */
 #define	RXH_L2DA	(1 << 1)
diff --git a/net/socket.c b/net/socket.c
index 5212447..575c84f 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2643,13 +2643,13 @@ static int ethtool_ioctl(struct net *net, struct compat_ifreq __user *ifr32)
 		return -EFAULT;
 
 	if (convert_in) {
-		/* We expect there to be holes between fs.m_u and
+		/* We expect there to be holes between fs.m_ext and
 		 * fs.ring_cookie and at the end of fs, but nowhere else.
 		 */
-		BUILD_BUG_ON(offsetof(struct compat_ethtool_rxnfc, fs.m_u) +
-			     sizeof(compat_rxnfc->fs.m_u) !=
-			     offsetof(struct ethtool_rxnfc, fs.m_u) +
-			     sizeof(rxnfc->fs.m_u));
+		BUILD_BUG_ON(offsetof(struct compat_ethtool_rxnfc, fs.m_ext) +
+			     sizeof(compat_rxnfc->fs.m_ext) !=
+			     offsetof(struct ethtool_rxnfc, fs.m_ext) +
+			     sizeof(rxnfc->fs.m_ext));
 		BUILD_BUG_ON(
 			offsetof(struct compat_ethtool_rxnfc, fs.location) -
 			offsetof(struct compat_ethtool_rxnfc, fs.ring_cookie) !=
@@ -2657,7 +2657,7 @@ static int ethtool_ioctl(struct net *net, struct compat_ifreq __user *ifr32)
 			offsetof(struct ethtool_rxnfc, fs.ring_cookie));
 
 		if (copy_in_user(rxnfc, compat_rxnfc,
-				 (void *)(&rxnfc->fs.m_u + 1) -
+				 (void *)(&rxnfc->fs.m_ext + 1) -
 				 (void *)rxnfc) ||
 		    copy_in_user(&rxnfc->fs.ring_cookie,
 				 &compat_rxnfc->fs.ring_cookie,
@@ -2674,7 +2674,7 @@ static int ethtool_ioctl(struct net *net, struct compat_ifreq __user *ifr32)
 
 	if (convert_out) {
 		if (copy_in_user(compat_rxnfc, rxnfc,
-				 (const void *)(&rxnfc->fs.m_u + 1) -
+				 (const void *)(&rxnfc->fs.m_ext + 1) -
 				 (const void *)rxnfc) ||
 		    copy_in_user(&compat_rxnfc->fs.ring_cookie,
 				 &rxnfc->fs.ring_cookie,

Re: [net-next-2.6 PATCH 2/2] ethtool: add ntuple flow specifier data to network flow classifier

[Ben Hutchings]

On Fri, 2011-04-08 at 15:07 -0700, Alexander Duyck wrote:
> This change is meant to add an ntuple data extensions to the rx network flow
> classification specifiers.  The idea is to allow ntuple to be displayed via
> the network flow classification interface.

Thanks for carrying on with this.

[...]
>  /**
>   * struct ethtool_rx_flow_spec - specification for RX flow filter
>   * @flow_type: Type of match to perform, e.g. %TCP_V4_FLOW
>   * @h_u: Flow fields to match (dependent on @flow_type)
> + * @h_ext: Additional fields to match
>   * @m_u: Masks for flow field bits to be ignored
> + * @m_ext: Masks for additional field bits to be ignored.
> + *	Note, all additional fields must be ignored unless @flow_type
> + *	includes the %FLOW_EXT flag.
>   * @ring_cookie: RX ring/queue index to deliver to, or %RX_CLS_FLOW_DISC
>   *	if packets should be discarded
>   * @location: Index of filter in hardware table
>   */
>  struct ethtool_rx_flow_spec {
>  	__u32		flow_type;
> -	union {
> -		struct ethtool_tcpip4_spec		tcp_ip4_spec;
> -		struct ethtool_tcpip4_spec		udp_ip4_spec;
> -		struct ethtool_tcpip4_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 ethhdr				ether_spec;
> -		__u8					hdata[72];
> -	} h_u, m_u;
> +	union ethtool_flow_union h_u;
> +	struct ethtool_flow_ext h_ext;
> +	union ethtool_flow_union m_u;
> +	struct ethtool_flow_ext m_ext;
> +	__u32		flow_type_ext;
[...]

You can't add flow_type_ext here.  I assume this is an oversight, since
it isn't mentioned anywhere else.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [net-next-2.6 PATCH 2/2] ethtool: add ntuple flow specifier data to network flow classifier

[Alexander Duyck]

On 4/8/2011 3:21 PM, Ben Hutchings wrote:
> On Fri, 2011-04-08 at 15:07 -0700, Alexander Duyck wrote:
>> This change is meant to add an ntuple data extensions to the rx network flow
>> classification specifiers.  The idea is to allow ntuple to be displayed via
>> the network flow classification interface.
>
> Thanks for carrying on with this.
>
> [...]
>>   /**
>>    * struct ethtool_rx_flow_spec - specification for RX flow filter
>>    * @flow_type: Type of match to perform, e.g. %TCP_V4_FLOW
>>    * @h_u: Flow fields to match (dependent on @flow_type)
>> + * @h_ext: Additional fields to match
>>    * @m_u: Masks for flow field bits to be ignored
>> + * @m_ext: Masks for additional field bits to be ignored.
>> + *	Note, all additional fields must be ignored unless @flow_type
>> + *	includes the %FLOW_EXT flag.
>>    * @ring_cookie: RX ring/queue index to deliver to, or %RX_CLS_FLOW_DISC
>>    *	if packets should be discarded
>>    * @location: Index of filter in hardware table
>>    */
>>   struct ethtool_rx_flow_spec {
>>   	__u32		flow_type;
>> -	union {
>> -		struct ethtool_tcpip4_spec		tcp_ip4_spec;
>> -		struct ethtool_tcpip4_spec		udp_ip4_spec;
>> -		struct ethtool_tcpip4_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 ethhdr				ether_spec;
>> -		__u8					hdata[72];
>> -	} h_u, m_u;
>> +	union ethtool_flow_union h_u;
>> +	struct ethtool_flow_ext h_ext;
>> +	union ethtool_flow_union m_u;
>> +	struct ethtool_flow_ext m_ext;
>> +	__u32		flow_type_ext;
> [...]
>
> You can't add flow_type_ext here.  I assume this is an oversight, since
> it isn't mentioned anywhere else.
>
> Ben.
>

Yeah, I forgot and left it in there.  It will be removed and resubmitted.

Thanks,

Alex