Thread (24 messages) 24 messages, 5 authors, 2011-03-04

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

From: Ben Hutchings <hidden>
Date: 2011-02-26 01:00:52

On Fri, 2011-02-25 at 15:32 -0800, Alexander Duyck wrote:
This change is meant to add an ntuple define type to the rx network flow
classification specifiers.  The idea is to allow ntuple to be displayed and
possibly configured via the network flow classification interface.  To do
this I added a ntuple_flow_spec_ext to the lsit of supported filters, and
added a flow_type_ext value to the structure in an unused hole within the
ethtool_rx_flow_spec structure.
There's a hole there on 64-bit architectures.  Unfortunately, on i386
and other architectures where u64 is not 64-bit-aligned, there isn't.
We actually need to add compat handling for the commands that use it.

Also, we don't want these flags to be ignored by older kernel versions
and drivers - they should reject specs that they don't understand.  So
any extension flags need to be added to flow_type.
Due to the fact that the flow specifier structures are only 4 byte aligned
instead of 8 I had to break the user data field into 2 sections.  In
addition I added the vlan ethertype field since this is what ixgbe was
using the user-data for currently and it allows for the fields to stay 4
byte aligned while occupying space at the end of the flow_spec.

In order to guarantee byte ordering I also thought it best to keep all
fields in the flow_spec area a big endian value, as such I added vlan, vlan
ethertype, and data as big endian values.
It's not important that byte order is consistent across architectures.
quoted hunk ↗ jump to hunk
Signed-off-by: Alexander Duyck <redacted>
---

 include/linux/ethtool.h |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index aac3e2e..3d1f8e0 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -378,10 +378,25 @@ struct ethtool_usrip4_spec {
 };
 
 /**
+ * struct ethtool_ntuple_spec_ext - flow spec extension for ntuple in nfc
+ * @unused: space unused by extension
+ * @vlan_etype: EtherType for vlan tagged packet to match
+ * @vlan_tci: VLAN tag to match
+ * @data: Driver-dependent data to match
+ */
+struct ethtool_ntuple_spec_ext {
+	__be32	unused[15];
+	__be16	vlan_etype;
+	__be16	vlan_tci;
+	__be32	data[2];
+};
[...]

This is a really nasty way to reclaim space in the union.

Let's name the union, shrink it and insert the extra fields that way:
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -377,27 +377,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[52];
+};
+
+struct ethtool_flow_ext {
+	__be16	vlan_etype;
+	__be16	vlan_tci;
+	__be32	data[2];
+	__u32	reserved[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;
 	__u64		ring_cookie;
 	__u32		location;
 };
@@ -954,6 +970,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)
---
Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help