[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1298682048.3555.18.camel@localhost>
Date: Sat, 26 Feb 2011 01:00:48 +0000
From: Ben Hutchings <bhutchings@...arflare.com>
To: Alexander Duyck <alexander.h.duyck@...el.com>
Cc: davem@...emloft.net, jeffrey.t.kirsher@...el.com,
netdev@...r.kernel.org
Subject: Re: [net-next-2.6 PATCH 02/10] ethtool: add ntuple flow specifier
to network flow classifier
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.
> Signed-off-by: Alexander Duyck <alexander.h.duyck@...el.com>
> ---
>
> 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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists