Skip to contentSkip to conversation

Workarounds and fixes for ntuple filters

PATCH In review Alexander Duyck 5 participants 24 messages Feb 25, 2011 – Mar 4, 2011

Subsystems (3)
Networking Network drivers Networking ethtool
lore
Share & tools
lore

Alexander Duyck

author
#0 lore raw

cover Workarounds and fixes for ntuple filters

This patch series is meant to address issue in the implementation of ntuple filters in both ixgbe and the kernel. The main issue being that ntuple filters didn't support the ability for drivers to retain and display filters.

This series addresses it by removing the ETHTOOL_GRXNTUPLE interface from the kernel and extends the ethtool network flow classifier interface to handle the same fields that ntuple filters provide. I have a separate set of patches I will be emailing out in the next few minutes that will contain the ethtool user-space portion of these changes.

The patches labeled with RFC are currently just for comments as I am not submitting them directly to the netdev tree and will be instead submitting them to the Jeff Kirsher's Intel network driver tree for submission.

---

Alexander Duyck (10):
[RFC] ixgbe: Add support for using the same fields as ntuple in nfc [RFC] ixgbe: add support for nfc addition and removal of filters [RFC] ixgbe: add support for displaying ntuple filters via the nfc interface [RFC] ixgbe: add basic support for settting and getting nfc controls [RFC] ixgbe: update perfect filter framework to support retaining filters [RFC] ixgbe: add support for different Rx packet buffer sizes [RFC] ethtool: remove support for ETHTOOL_GRXNTUPLE
[RFC] ixgbe: remove ntuple filtering
ethtool: add ntuple flow specifier to network flow classifier
ethtool: prevent null pointer dereference with NTUPLE set but no set_rx_ntuple

 drivers/net/ixgbe/ixgbe.h         |   29 +-
 drivers/net/ixgbe/ixgbe_82598.c   |    2 
 drivers/net/ixgbe/ixgbe_82599.c   |  661 ++++++++++++++++++++-----------------
 drivers/net/ixgbe/ixgbe_dcb_nl.c  |   47 +--
 drivers/net/ixgbe/ixgbe_ethtool.c |  504 ++++++++++++++++++++++------
 drivers/net/ixgbe/ixgbe_main.c    |   90 +++--
 drivers/net/ixgbe/ixgbe_type.h    |   25 +
 drivers/net/ixgbe/ixgbe_x540.c    |    2 
 include/linux/ethtool.h           |   27 +-
 include/linux/netdevice.h         |    3 
 net/core/dev.c                    |    5 
 net/core/ethtool.c                |  302 -----------------
 12 files changed, 884 insertions(+), 813 deletions(-)
author
#1 lore raw

PATCH 1/10 ethtool: prevent null pointer dereference with NTUPLE set but no set_rx_ntuple

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.

This issue appears to affect all kernels since 2.6.34.

Signed-off-by Alexander Duyck <[email protected]>

---

 net/core/ethtool.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
net/core/ethtool.c +3 -0
index c1a71bb..4843674 100644
@@ -893,6 +893,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;
#2 lore raw

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

Quoted 2 lines · 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.

I think it would be a bug for NETIF_F_NTUPLE to be enabled on a device that doesn't have this operation. Are there any drivers for which this is possible?

Quoted 1 line

This issue appears to affect all kernels since 2.6.34.

If this can actually happen, the fix should go to net-2.6 and [email protected]. However, I think that the null deference is impossible and this really just fixes the error code.

Ben.

Quoted 21 lines
Signed-off-by Alexander Duyck <[email protected]>

---

 net/core/ethtool.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
net/core/ethtool.c +3 -0
index c1a71bb..4843674 100644
@@ -893,6 +893,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;
Signature · 3 lines
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.
author
#3 lore raw

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

Quoted 7 lines · Ben Hutchings
Quoted 2 lines · 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.

I think it would be a bug for NETIF_F_NTUPLE to be enabled on a device that doesn't have this operation. Are there any drivers for which this is possible?

Currently there are no drivers where this is possible. However I encountered it as a result of testing the patches further on in this set.

Quoted 7 lines
Quoted 1 line

This issue appears to affect all kernels since 2.6.34.

If this can actually happen, the fix should go to net-2.6 and [email protected]. However, I think that the null deference is impossible and this really just fixes the error code.

Ben.

It cannot occur with any of the in-kernel drivers since they all set the NETIF_F_NTUPLE flag and have the function defined. However going forward I would like to have the option of using the network flow classifier interface instead of the set_rx_ntuple interface due to the fact that it supports many of the features I needed.

I believe this patch should apply to net-2.6 without any changes so if it is better placed there I will resubmit it specifically for net-2.6 and stable.

Thanks,

Alex

Quoted 22 lines
Quoted 21 lines
Signed-off-by Alexander Duyck<[email protected]>

---

  net/core/ethtool.c |    3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)
net/core/ethtool.c +3 -0
index c1a71bb..4843674 100644
@@ -893,6 +893,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;
maintainer
#4 lore raw

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

From: Alexander Duyck <[email protected]>
Date: Fri, 25 Feb 2011 16:40:13 -0800

Quoted 5 lines

It cannot occur with any of the in-kernel drivers since they all set the NETIF_F_NTUPLE flag and have the function defined. However going forward I would like to have the option of using the network flow classifier interface instead of the set_rx_ntuple interface due to the fact that it supports many of the features I needed.

This still doesn't explain to me why a driver would set the feature flag, but not actually implement the feature.

I'm not applying this patch.

When you create the situation that causes the potentially NULL dereference, then you can use that patch to show why this seemingly illogical situation can indeed occur.

Until then no driver causes this issue, therefore the problem does not exist.

#5 lore raw

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

Quoted 11 lines · David Miller

From: Alexander Duyck <[email protected]>
Date: Fri, 25 Feb 2011 16:40:13 -0800

Quoted 5 lines

It cannot occur with any of the in-kernel drivers since they all set the NETIF_F_NTUPLE flag and have the function defined.  However going forward I would like to have the option of using the network flow classifier interface instead of the set_rx_ntuple interface due to the fact that it supports many of the features I needed.

This still doesn't explain to me why a driver would set the feature flag, but not actually implement the feature.

Actually the reason I ran into this is because of the patches in the RFC set. Basically I was looking at moving the ntuple support in ixgbe over to network flow classifier rules. As such I was leaving the ntuple flag set, but using set_rxnfc via the filter rules instead.
If you recommend adding a new flag to do that I am fine with that.

Quoted 8 lines

I'm not applying this patch.

When you create the situation that causes the potentially NULL dereference, then you can use that patch to show why this seemingly illogical situation can indeed occur.

Until then no driver causes this issue, therefore the problem does not exist.

I'll do some digging late next week to see if there are any other means of encountering the issue and will get back to you if I find anything.

Thanks,

Alex

author
#6 lore raw

PATCH 2/10 ethtool: add ntuple flow specifier to network flow classifier

In reply to Alexander Duyck

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.

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.

Signed-off-by Alexander Duyck <[email protected]>

---

 include/linux/ethtool.h |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
include/linux/ethtool.h +20 -0
index aac3e2e..3d1f8e0 100644
@@ -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];
+};
+
+/**
  * 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)
  * @m_u: Masks for flow field bits to be ignored
+ * @flow_type_ext: Type of extended match to perform, e.g. %NTUPLE_FLOW_EXT
  * @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

@@ -396,8 +411,10 @@ struct ethtool_rx_flow_spec {
 		struct ethtool_ah_espip4_spec		esp_ip4_spec;
 		struct ethtool_usrip4_spec		usr_ip4_spec;
 		struct ethhdr				ether_spec;
+		struct ethtool_ntuple_spec_ext		ntuple_spec;
 		__u8					hdata[72];
 	} h_u, m_u;
+	__u32		flow_type_ext;
 	__u64		ring_cookie;
 	__u32		location;
 };

@@ -955,6 +972,9 @@ struct ethtool_ops {
 #define	IPV6_FLOW	0x11	/* hash only */
 #define	ETHER_FLOW	0x12	/* spec only (ether_spec) */
 
+/* Flow extension types for network flow classifier */
+#define NTUPLE_FLOW_EXT	0x01 /* indicates ntuple in nfc */
+
 /* L3-L4 network traffic flow hash options */
 #define	RXH_L2DA	(1 << 1)
 #define	RXH_VLAN	(1 << 2)
#7 lore raw

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

Quoted 6 lines · Alexander Duyck

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.

Quoted 9 lines

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 16 lines
Signed-off-by Alexander Duyck <[email protected]>

---

 include/linux/ethtool.h |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
include/linux/ethtool.h +2 -0
index aac3e2e..3d1f8e0 100644
@@ -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

include/linux/ethtool.h struct ethtool_usrip4_spec { · line 392

Quoted 10 lines
include/linux/ethtool.h +10 -0
+ * @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:

include/linux/ethtool.h +28 -10
@@ -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.

Signature · 3 lines
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.
Acked-by
#8 lore raw

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

Quoted 74 lines · Ben Hutchings
Quoted 6 lines · Alexander Duyck

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.

Quoted 9 lines

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 16 lines
Signed-off-by Alexander Duyck <[email protected]>

---

 include/linux/ethtool.h |   20 ++++++++++++++++++++

 1 files changed, 20 insertions(+), 0 deletions(-)
include/linux/ethtool.h +2 -0
index aac3e2e..3d1f8e0 100644
@@ -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

include/linux/ethtool.h struct ethtool_usrip4_spec { · line 392

Quoted 10 lines
include/linux/ethtool.h +10 -0
+ * @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:

include/linux/ethtool.h +8 -0
@@ -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;

include/linux/ethtool.h struct ethtool_usrip4_spec { · line 397

Quoted 10 lines
include/linux/ethtool.h +10 -0
+       __u8                                    hdata[52];
+};
+
+struct ethtool_flow_ext {
+       __be16  vlan_etype;
+       __be16  vlan_tci;
+       __be32  data[2];
+       __u32   reserved[2];
+};
+

Any chance of getting the reserved fields moved to the top of the structure? My only concern is that we might end up with a flow spec larger than 52 bytes at some point and moving the reserved fields to the front might give us a little more wiggle room future compatibility.

Quoted 43 lines
include/linux/ethtool.h +10 -11
  /**
  * 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)
---
Quoted 1 line

Ben.

This works for my purposes other than the one comment above. However if you are fine with it I am good with it since I can't think of any filters that we might need in the near future that would require more than 52 bytes.

Acked-by Alexander Duyck <[email protected]>
#9 lore raw

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

Quoted 3 lines · Alexander Duyck
Quoted 1 line · Ben Hutchings

On Fri, 2011-02-25 at 15:32 -0800, Alexander Duyck wrote:

[...]

Quoted 48 lines
Quoted 48 lines
Quoted 10 lines
include/linux/ethtool.h +2 -0
index aac3e2e..3d1f8e0 100644
@@ -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

include/linux/ethtool.h struct ethtool_usrip4_spec { · line 392

Quoted 10 lines
include/linux/ethtool.h +10 -0
+ * @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:

include/linux/ethtool.h +16 -0
@@ -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];
Quoted 8 lines

include/linux/ethtool.h struct ethtool_usrip4_spec { · line 397

Quoted 2 lines
include/linux/ethtool.h +2 -0
+};
+

Any chance of getting the reserved fields moved to the top of the structure? My only concern is that we might end up with a flow spec larger than 52 bytes at some point and moving the reserved fields to the front might give us a little more wiggle room future compatibility.

[...]

OK, so how about this:

/**
* union ethtool_flow_union - flow spec type-specific fields
* @tcp_ip4_spec: TCP/IPv4 fields to match
* @udp_ip4_spec: UDP/IPv4 fields to match
* @sctp_ip4_spec: SCTP/IPv4 fields to match
* @ah_ip4_spec: AH/IPv4 fields to match
* @esp_ip4_spec: ESP/IPv4 fields to match
* @usr_ip4_spec: User-defined IPv4 fields to match
* @ether_spec: Ethernet fields to match
*
* Note: The size of this union may shrink in future to allow for
* expansion in &struct ethtool_flow_ext.
*/

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 - flow spec common extension fields
* @vlan_etype: EtherType for vlan tagged packet to match
* @vlan_tci: VLAN tag to match
* @data: Driver-dependent data to match
*
* Note: Additional fields may be inserted before @vlan_etype in future,
* but the offset of the existing fields within the containing structure
* (&struct ethtool_rx_flow_spec) will be stable.
*/

struct ethtool_flow_ext {
	__be16	vlan_etype;
	__be16	vlan_tci;
	__be32	data[2];
};

Please can you check that these definitions won't affect the size of struct ethtool_rx_flow_spec on i386 or x86-64?

Ben.

Signature · 3 lines
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.
#10 lore raw

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

Quoted 10 lines · Ben Hutchings

/**
* struct ethtool_flow_ext - flow spec common extension fields
* @vlan_etype: EtherType for vlan tagged packet to match
* @vlan_tci: VLAN tag to match
* @data: Driver-dependent data to match
*
* Note: Additional fields may be inserted before @vlan_etype in future,
* but the offset of the existing fields within the containing structure
* (&struct ethtool_rx_flow_spec) will be stable.
*/

Quoted 5 lines
struct ethtool_flow_ext {
	__be16	vlan_etype;
	__be16	vlan_tci;
	__be32	data[2];
};

I am wondering about the semantics of these vlan_* fields. Is vlan_etype the Ethertype in the VLAN header or the type after it? Specifically, I am wondering whether these fields are limited to VLANs or can work with general Ethertypes and the 2 bytes after them.

#11 lore raw

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

Quoted 17 lines · Dimitrios Michailidis
Quoted 10 lines · Ben Hutchings

/**
* struct ethtool_flow_ext - flow spec common extension fields
* @vlan_etype: EtherType for vlan tagged packet to match
* @vlan_tci: VLAN tag to match
* @data: Driver-dependent data to match
*
* Note: Additional fields may be inserted before @vlan_etype in future,
* but the offset of the existing fields within the containing structure
* (&struct ethtool_rx_flow_spec) will be stable.
*/

Quoted 5 lines
struct ethtool_flow_ext {
	__be16	vlan_etype;
	__be16	vlan_tci;
	__be32	data[2];
};
Quoted 2 lines

I am wondering about the semantics of these vlan_* fields. Is vlan_etype the Ethertype in the VLAN header or the type after it?

It would be the the type in the VLAN tag. The nested ethertype is normally implied by flow_type to be ETH_P_IP.

This does leave the question of what this would mean:

struct ethtool_rx_flow_spec fs = {
	.flow_type = ... | FLOW_EXT,
	...
	.h_ext.vlan_tci = htons(0x1234),
	.m_ext.vlan_etype = 0xffff,
};

This says the TCI must be == 0x1234 but the type can be anything. But the type surely has to be be one assigned for use in VLAN tags. Should we leave it to the driver/hardware to determine what those valid types are, or should we reject this as valid?

Quoted 3 lines

Specifically, I am wondering
whether these fields are limited to VLANs or can work with general Ethertypes and the 2 bytes after them.

If you set flow_type = ETHER_FLOW you can then specify any combination of Ethernet header fields to match in {h_u,m_u}.ether_spec. However, so far only sfc implements it and it only allows matching on the destination MAC address.

Ben.

Signature · 3 lines
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.
#12 lore raw

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

Quoted 33 lines · Ben Hutchings
Quoted 17 lines · Dimitrios Michailidis
Quoted 10 lines · Ben Hutchings

/**
* struct ethtool_flow_ext - flow spec common extension fields
* @vlan_etype: EtherType for vlan tagged packet to match
* @vlan_tci: VLAN tag to match
* @data: Driver-dependent data to match
*
* Note: Additional fields may be inserted before @vlan_etype in future,
* but the offset of the existing fields within the containing structure
* (&struct ethtool_rx_flow_spec) will be stable.
*/

Quoted 5 lines
struct ethtool_flow_ext {
	__be16	vlan_etype;
	__be16	vlan_tci;
	__be32	data[2];
};
Quoted 2 lines

I am wondering about the semantics of these vlan_* fields. Is vlan_etype the Ethertype in the VLAN header or the type after it?

It would be the the type in the VLAN tag. The nested ethertype is normally implied by flow_type to be ETH_P_IP.

This does leave the question of what this would mean:

struct ethtool_rx_flow_spec fs = {
	.flow_type = ... | FLOW_EXT,
	...
	.h_ext.vlan_tci = htons(0x1234),
	.m_ext.vlan_etype = 0xffff,
};
Quoted 4 lines

This says the TCI must be == 0x1234 but the type can be anything. But the type surely has to be be one assigned for use in VLAN tags. Should we leave it to the driver/hardware to determine what those valid types are, or should we reject this as valid?

Right. Devices have some internal rules for what qualifies as a VLAN frame. If users are given the option to specify vlan_etype what do they get?
At least we need to specify what is expected so drivers can decide if they can support it.

author
#13 lore raw

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

Quoted 34 lines · Dimitrios Michailidis
Quoted 33 lines · Ben Hutchings
Quoted 17 lines · Dimitrios Michailidis
Quoted 10 lines · Ben Hutchings

/**
* struct ethtool_flow_ext - flow spec common extension fields
* @vlan_etype: EtherType for vlan tagged packet to match
* @vlan_tci: VLAN tag to match
* @data: Driver-dependent data to match
*
* Note: Additional fields may be inserted before @vlan_etype in future,
* but the offset of the existing fields within the containing structure
* (&struct ethtool_rx_flow_spec) will be stable.
*/

Quoted 5 lines
struct ethtool_flow_ext {
	__be16	vlan_etype;
	__be16	vlan_tci;
	__be32	data[2];
};
Quoted 2 lines

I am wondering about the semantics of these vlan_* fields. Is vlan_etype the Ethertype in the VLAN header or the type after it?

It would be the the type in the VLAN tag. The nested ethertype is normally implied by flow_type to be ETH_P_IP.

This does leave the question of what this would mean:

struct ethtool_rx_flow_spec fs = {
	.flow_type = ... | FLOW_EXT,
	...
	.h_ext.vlan_tci = htons(0x1234),
	.m_ext.vlan_etype = 0xffff,
};
Quoted 8 lines
Quoted 4 lines

This says the TCI must be == 0x1234 but the type can be anything. But the type surely has to be be one assigned for use in VLAN tags. Should we leave it to the driver/hardware to determine what those valid types are, or should we reject this as valid?

Right. Devices have some internal rules for what qualifies as a VLAN frame. If users are given the option to specify vlan_etype what do they get?
At least we need to specify what is expected so drivers can decide if they can support it.

The basic idea I had is similar to what Ben described. Basically the vlan_etype can be used to determine the Ethertype for the VLAN to be compared. The reason for this is specifically the VLAN 0 case since without the VLAN Ethertype check VLAN 0 on ixgbe hardware will match an untagged frame and that may not be a desired result. As such we can specify the VLAN Ethertype and then we will only match VLAN tagged frames without any false hits.

Thanks,

Alex

author
#14 lore raw

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

In reply to Ben Hutchings
Quoted 105 lines · Ben Hutchings
Quoted 3 lines · Alexander Duyck
Quoted 1 line · Ben Hutchings

On Fri, 2011-02-25 at 15:32 -0800, Alexander Duyck wrote:

[...]

Quoted 48 lines
Quoted 48 lines
Quoted 10 lines
include/linux/ethtool.h +2 -0
index aac3e2e..3d1f8e0 100644
@@ -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

include/linux/ethtool.h struct ethtool_usrip4_spec { · line 392

Quoted 10 lines
include/linux/ethtool.h +10 -0
+ * @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:

include/linux/ethtool.h +16 -0
@@ -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];
Quoted 8 lines

include/linux/ethtool.h struct ethtool_usrip4_spec { · line 397

Quoted 2 lines
include/linux/ethtool.h +2 -0
+};
+

Any chance of getting the reserved fields moved to the top of the structure? My only concern is that we might end up with a flow spec larger than 52 bytes at some point and moving the reserved fields to the front might give us a little more wiggle room future compatibility.

[...]

OK, so how about this:

/**
* union ethtool_flow_union - flow spec type-specific fields
* @tcp_ip4_spec: TCP/IPv4 fields to match
* @udp_ip4_spec: UDP/IPv4 fields to match
* @sctp_ip4_spec: SCTP/IPv4 fields to match
* @ah_ip4_spec: AH/IPv4 fields to match
* @esp_ip4_spec: ESP/IPv4 fields to match
* @usr_ip4_spec: User-defined IPv4 fields to match
* @ether_spec: Ethernet fields to match
*
* Note: The size of this union may shrink in future to allow for
* expansion in&struct ethtool_flow_ext.
*/

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 - flow spec common extension fields
* @vlan_etype: EtherType for vlan tagged packet to match
* @vlan_tci: VLAN tag to match
* @data: Driver-dependent data to match
*
* Note: Additional fields may be inserted before @vlan_etype in future,
* but the offset of the existing fields within the containing structure
* (&struct ethtool_rx_flow_spec) will be stable.
*/

struct ethtool_flow_ext {
	__be16	vlan_etype;
	__be16	vlan_tci;
	__be32	data[2];
};
Quoted 5 lines

Please can you check that these definitions won't affect the size of struct ethtool_rx_flow_spec on i386 or x86-64?

Ben.

I'll try to look into it next week since I am just getting caught up from being out on vacation.

As I recall when I had made my original changes they didn't have an effect on the size so this should be fine since all of the fields have a maximum alignment of 32 bits.

Thanks,

Alex

maintainer
#15 lore raw

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

In reply to Alexander Duyck

From: Alexander Duyck <[email protected]>
Date: Fri, 25 Feb 2011 15:32:49 -0800

include/linux/ethtool.h struct ethtool_rx_flow_spec { · line 420

Quoted 11 lines
include/linux/ethtool.h +2 -0
@@ -396,8 +411,10 @@ struct ethtool_rx_flow_spec {
 		struct ethtool_ah_espip4_spec		esp_ip4_spec;
 		struct ethtool_usrip4_spec		usr_ip4_spec;
 		struct ethhdr				ether_spec;
+		struct ethtool_ntuple_spec_ext		ntuple_spec;
 		__u8					hdata[72];
 	} h_u, m_u;
+	__u32		flow_type_ext;
 	__u64		ring_cookie;
 	__u32		location;
 };

How can you add this flow_type_ext member to this user visible structure without utterly breaking userspace? It changes the offsets of the ring_cookie and location members.

author
#16 lore raw

RFC PATCH 3/10 ixgbe: remove ntuple filtering

In reply to Alexander Duyck

Due to numerous issues in ntuple filters it has been decieded to move the interface over to the network flow classification interface. As a first step to achieving this I first need to remove the old ntuple interface.

In addition I am removing the requirement that ntuple filters have the same number of queues and requirements as ATR. As a result this change will make it so that all the ntuple flag does is disable ATR for now.

Signed-off-by Alexander Duyck <[email protected]>

---

 drivers/net/ixgbe/ixgbe_dcb_nl.c  |   47 +++++------
 drivers/net/ixgbe/ixgbe_ethtool.c |  162 +++----------------------------------
 drivers/net/ixgbe/ixgbe_main.c    |   45 +++-------
 3 files changed, 48 insertions(+), 206 deletions(-)
drivers/net/ixgbe/ixgbe_dcb_nl.c +23 -24
index a977df3..47fd9f6 100644
@@ -114,11 +114,12 @@ static u8 ixgbe_dcbnl_set_state(struct net_device *netdev, u8 state)
 	u8 err = 0;
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 
+	/* verify there is something to do, if not then exit */
+	if (!!state != !(adapter->flags & IXGBE_FLAG_DCB_ENABLED))
+		return err;
+
 	if (state > 0) {
 		/* Turn on DCB */
-		if (adapter->flags & IXGBE_FLAG_DCB_ENABLED)
-			goto out;
-
 		if (!(adapter->flags & IXGBE_FLAG_MSIX_ENABLED)) {
 			e_err(drv, "Enable failed, needs MSI-X\n");
 			err = 1;

@@ -138,7 +139,6 @@ static u8 ixgbe_dcbnl_set_state(struct net_device *netdev, u8 state)
 		case ixgbe_mac_82599EB:
 		case ixgbe_mac_X540:
 			adapter->flags &= ~IXGBE_FLAG_FDIR_HASH_CAPABLE;
-			adapter->flags &= ~IXGBE_FLAG_FDIR_PERFECT_CAPABLE;
 			break;
 		default:
 			break;

@@ -150,29 +150,28 @@ static u8 ixgbe_dcbnl_set_state(struct net_device *netdev, u8 state)
 			netdev->netdev_ops->ndo_open(netdev);
 	} else {
 		/* Turn off DCB */
-		if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-			if (netif_running(netdev))
-				netdev->netdev_ops->ndo_stop(netdev);
-			ixgbe_clear_interrupt_scheme(adapter);
+		if (netif_running(netdev))
+			netdev->netdev_ops->ndo_stop(netdev);
+		ixgbe_clear_interrupt_scheme(adapter);
 
-			adapter->hw.fc.requested_mode = adapter->last_lfc_mode;
-			adapter->temp_dcb_cfg.pfc_mode_enable = false;
-			adapter->dcb_cfg.pfc_mode_enable = false;
-			adapter->flags &= ~IXGBE_FLAG_DCB_ENABLED;
-			adapter->flags |= IXGBE_FLAG_RSS_ENABLED;
-			switch (adapter->hw.mac.type) {
-			case ixgbe_mac_82599EB:
-			case ixgbe_mac_X540:
+		adapter->hw.fc.requested_mode = adapter->last_lfc_mode;
+		adapter->temp_dcb_cfg.pfc_mode_enable = false;
+		adapter->dcb_cfg.pfc_mode_enable = false;
+		adapter->flags &= ~IXGBE_FLAG_DCB_ENABLED;
+		adapter->flags |= IXGBE_FLAG_RSS_ENABLED;
+		switch (adapter->hw.mac.type) {
+		case ixgbe_mac_82599EB:
+		case ixgbe_mac_X540:
+			if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE))
 				adapter->flags |= IXGBE_FLAG_FDIR_HASH_CAPABLE;
-				break;
-			default:
-				break;
-			}
-
-			ixgbe_init_interrupt_scheme(adapter);
-			if (netif_running(netdev))
-				netdev->netdev_ops->ndo_open(netdev);
+			break;
+		default:
+			break;
 		}
+
+		ixgbe_init_interrupt_scheme(adapter);
+		if (netif_running(netdev))
+			netdev->netdev_ops->ndo_open(netdev);
 	}
 out:
 	return err;
drivers/net/ixgbe/ixgbe_ethtool.c +11 -151

188 lines not on this page · raw message

drivers/net/ixgbe/ixgbe_main.c +14 -31

126 lines not on this page · raw message

author
#17 lore raw

RFC PATCH 4/10 ethtool: remove support for ETHTOOL_GRXNTUPLE

In reply to Alexander Duyck

This change is meant to remove all support for displaying an ntuple as strings via ETHTOOL_GRXNTUPLE. The reason for this change is due to the fact that multiple issues have been found including:
- Multiple buffer overruns for strings being displayed.
- Incorrect filters displayed, cleared filters with ring of -2 are displayed
- Setting get_rx_ntuple displays no rules if defined.
- Endianess wrong on displayed values.
- Hard limit of 1024 filters makes display functionality extremely limited

In order to address this I am proposing moving the displaying of rules over to the ETHTOOL_GRXCLSRLCNT/ETHTOOL_GRXCLSRULE/ETHTOOL_GRXCLSRULEALL interfaces and will have a follow-on patch to allow this.

Signed-off-by Alexander Duyck <[email protected]>

---

 include/linux/ethtool.h   |    7 -
 include/linux/netdevice.h |    3 
 net/core/dev.c            |    5 -
 net/core/ethtool.c        |  299 ---------------------------------------------
 4 files changed, 1 insertions(+), 313 deletions(-)
include/linux/ethtool.h +1 -6
index 3d1f8e0..2f2f27c 100644
@@ -250,7 +250,6 @@ enum ethtool_stringset {
 	ETH_SS_TEST		= 0,
 	ETH_SS_STATS,
 	ETH_SS_PRIV_FLAGS,
-	ETH_SS_NTUPLE_FILTERS,
 	ETH_SS_FEATURES,
 };
 

@@ -637,8 +636,6 @@ struct ethtool_rx_ntuple_flow_spec_container {
 };
 
 struct ethtool_rx_ntuple_list {
-#define ETHTOOL_MAX_NTUPLE_LIST_ENTRY 1024
-#define ETHTOOL_MAX_NTUPLE_STRING_PER_ENTRY 14
 	struct list_head	list;
 	unsigned int		count;
 };

@@ -659,7 +656,6 @@ u32 ethtool_op_get_ufo(struct net_device *dev);
 int ethtool_op_set_ufo(struct net_device *dev, u32 data);
 u32 ethtool_op_get_flags(struct net_device *dev);
 int ethtool_op_set_flags(struct net_device *dev, u32 data, u32 supported);
-void ethtool_ntuple_flush(struct net_device *dev);
 
 /**
  * &ethtool_ops - Alter and report network device settings

@@ -776,7 +772,6 @@ struct ethtool_ops {
 	int	(*reset)(struct net_device *, u32 *);
 	int	(*set_rx_ntuple)(struct net_device *,
 				 struct ethtool_rx_ntuple *);
-	int	(*get_rx_ntuple)(struct net_device *, u32 stringset, void *);
 	int	(*get_rxfh_indir)(struct net_device *,
 				  struct ethtool_rxfh_indir *);
 	int	(*set_rxfh_indir)(struct net_device *,

@@ -842,7 +837,7 @@ struct ethtool_ops {
 #define ETHTOOL_FLASHDEV	0x00000033 /* Flash firmware to device */
 #define ETHTOOL_RESET		0x00000034 /* Reset hardware */
 #define ETHTOOL_SRXNTUPLE	0x00000035 /* Add an n-tuple filter to device */
-#define ETHTOOL_GRXNTUPLE	0x00000036 /* Get n-tuple filters from device */
+/* ETHTOOL_GRXNTUPLE		0x00000036 disabled due to multiple issues */
 #define ETHTOOL_GSSET_INFO	0x00000037 /* Get string set info */
 #define ETHTOOL_GRXFHINDIR	0x00000038 /* Get RX flow hash indir'n table */
 #define ETHTOOL_SRXFHINDIR	0x00000039 /* Set RX flow hash indir'n table */
include/linux/netdevice.h +0 -3
index ffe56c1..f20fabe 100644
@@ -1246,9 +1246,6 @@ struct net_device {
 	/* max exchange id for FCoE LRO by ddp */
 	unsigned int		fcoe_ddp_xid;
 #endif
-	/* n-tuple filter list attached to this device */
-	struct ethtool_rx_ntuple_list ethtool_ntuple_list;
-
 	/* phy device may attach itself for hardware timestamping */
 	struct phy_device *phydev;
net/core/dev.c +0 -5
index 69a3c08..c788c98 100644
@@ -5886,8 +5886,6 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 
 	dev->gso_max_size = GSO_MAX_SIZE;
 
-	INIT_LIST_HEAD(&dev->ethtool_ntuple_list.list);
-	dev->ethtool_ntuple_list.count = 0;
 	INIT_LIST_HEAD(&dev->napi_list);
 	INIT_LIST_HEAD(&dev->unreg_list);
 	INIT_LIST_HEAD(&dev->link_watch_list);

@@ -5951,9 +5949,6 @@ void free_netdev(struct net_device *dev)
 	/* Flush device addresses */
 	dev_addr_flush(dev);
 
-	/* Clear ethtool n-tuple list */
-	ethtool_ntuple_flush(dev);
-
 	list_for_each_entry_safe(p, n, &dev->napi_list, dev_list)
 		netif_napi_del(p);
net/core/ethtool.c +0 -299

329 lines not on this page · raw message

author
#18 lore raw

RFC PATCH 5/10 ixgbe: add support for different Rx packet buffer sizes

In reply to Alexander Duyck

This change adds support for hardware that can have different Rx packet buffer sizes.

Signed-off-by Alexander Duyck <[email protected]>

---

 drivers/net/ixgbe/ixgbe_82598.c |    2 ++
 drivers/net/ixgbe/ixgbe_82599.c |    2 ++
 drivers/net/ixgbe/ixgbe_type.h  |    1 +
 drivers/net/ixgbe/ixgbe_x540.c  |    2 ++
 4 files changed, 7 insertions(+), 0 deletions(-)
drivers/net/ixgbe/ixgbe_82598.c +2 -0
index d0f1d9d..d2dc50c 100644
@@ -37,6 +37,7 @@
 #define IXGBE_82598_RAR_ENTRIES   16
 #define IXGBE_82598_MC_TBL_SIZE  128
 #define IXGBE_82598_VFT_TBL_SIZE 128
+#define IXGBE_82598_RX_PB_SIZE   512
 
 static s32 ixgbe_setup_copper_link_82598(struct ixgbe_hw *hw,
                                          ixgbe_link_speed speed,

@@ -123,6 +124,7 @@ static s32 ixgbe_get_invariants_82598(struct ixgbe_hw *hw)
 	mac->mcft_size = IXGBE_82598_MC_TBL_SIZE;
 	mac->vft_size = IXGBE_82598_VFT_TBL_SIZE;
 	mac->num_rar_entries = IXGBE_82598_RAR_ENTRIES;
+	mac->rx_pb_size = IXGBE_82598_RX_PB_SIZE;
 	mac->max_rx_queues = IXGBE_82598_MAX_RX_QUEUES;
 	mac->max_tx_queues = IXGBE_82598_MAX_TX_QUEUES;
 	mac->max_msix_vectors = ixgbe_get_pcie_msix_count_82598(hw);
drivers/net/ixgbe/ixgbe_82599.c +2 -0
index a21f581..c9bfa64 100644
@@ -38,6 +38,7 @@
 #define IXGBE_82599_RAR_ENTRIES   128
 #define IXGBE_82599_MC_TBL_SIZE   128
 #define IXGBE_82599_VFT_TBL_SIZE  128
+#define IXGBE_82599_RX_PB_SIZE    512
 
 static void ixgbe_disable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw);
 static void ixgbe_enable_tx_laser_multispeed_fiber(struct ixgbe_hw *hw);

@@ -168,6 +169,7 @@ static s32 ixgbe_get_invariants_82599(struct ixgbe_hw *hw)
 	mac->vft_size = IXGBE_82599_VFT_TBL_SIZE;
 	mac->num_rar_entries = IXGBE_82599_RAR_ENTRIES;
 	mac->max_rx_queues = IXGBE_82599_MAX_RX_QUEUES;
+	mac->rx_pb_size = IXGBE_82599_RX_PB_SIZE;
 	mac->max_tx_queues = IXGBE_82599_MAX_TX_QUEUES;
 	mac->max_msix_vectors = ixgbe_get_pcie_msix_count_generic(hw);
drivers/net/ixgbe/ixgbe_type.h +1 -0
index ab65d13..2f8ba73 100644
@@ -2571,6 +2571,7 @@ struct ixgbe_mac_info {
 	u32                             vft_size;
 	u32                             num_rar_entries;
 	u32                             rar_highwater;
+	u32                             rx_pb_size;
 	u32                             max_tx_queues;
 	u32                             max_rx_queues;
 	u32                             max_msix_vectors;
drivers/net/ixgbe/ixgbe_x540.c +2 -0
index f2518b0..fa095f8 100644
@@ -38,6 +38,7 @@
 #define IXGBE_X540_RAR_ENTRIES   128
 #define IXGBE_X540_MC_TBL_SIZE   128
 #define IXGBE_X540_VFT_TBL_SIZE  128
+#define IXGBE_X540_RX_PB_SIZE    384
 
 static s32 ixgbe_update_flash_X540(struct ixgbe_hw *hw);
 static s32 ixgbe_poll_flash_update_done_X540(struct ixgbe_hw *hw);

@@ -62,6 +63,7 @@ static s32 ixgbe_get_invariants_X540(struct ixgbe_hw *hw)
 	mac->vft_size = IXGBE_X540_VFT_TBL_SIZE;
 	mac->num_rar_entries = IXGBE_X540_RAR_ENTRIES;
 	mac->max_rx_queues = IXGBE_X540_MAX_RX_QUEUES;
+	mac->rx_pb_size = IXGBE_X540_RX_PB_SIZE;
 	mac->max_tx_queues = IXGBE_X540_MAX_TX_QUEUES;
 	mac->max_msix_vectors = ixgbe_get_pcie_msix_count_generic(hw);
author
#19 lore raw

RFC PATCH 6/10 ixgbe: update perfect filter framework to support retaining filters

In reply to Alexander Duyck

This change is meant to update the internal framework of ixgbe so that perfect filters can be stored and tracked via software.

Signed-off-by Alexander Duyck <[email protected]>

---

 drivers/net/ixgbe/ixgbe.h       |   18 +
 drivers/net/ixgbe/ixgbe_82599.c |  659 +++++++++++++++++++++------------------
 drivers/net/ixgbe/ixgbe_main.c  |    2 
 drivers/net/ixgbe/ixgbe_type.h  |   24 -
 4 files changed, 376 insertions(+), 327 deletions(-)
drivers/net/ixgbe/ixgbe.h +12 -6
index 12769b5..0cd75d6 100644
@@ -526,16 +526,22 @@ extern void ixgbe_alloc_rx_buffers(struct ixgbe_ring *, u16);
 extern void ixgbe_write_eitr(struct ixgbe_q_vector *);
 extern int ethtool_ioctl(struct ifreq *ifr);
 extern s32 ixgbe_reinit_fdir_tables_82599(struct ixgbe_hw *hw);
-extern s32 ixgbe_init_fdir_signature_82599(struct ixgbe_hw *hw, u32 pballoc);
-extern s32 ixgbe_init_fdir_perfect_82599(struct ixgbe_hw *hw, u32 pballoc);
+extern s32 ixgbe_init_fdir_signature_82599(struct ixgbe_hw *hw, u32 fdirctrl);
+extern s32 ixgbe_init_fdir_perfect_82599(struct ixgbe_hw *hw, u32 fdirctrl);
 extern s32 ixgbe_fdir_add_signature_filter_82599(struct ixgbe_hw *hw,
 						 union ixgbe_atr_hash_dword input,
 						 union ixgbe_atr_hash_dword common,
                                                  u8 queue);
-extern s32 ixgbe_fdir_add_perfect_filter_82599(struct ixgbe_hw *hw,
-                                      union ixgbe_atr_input *input,
-                                      struct ixgbe_atr_input_masks *input_masks,
-                                      u16 soft_id, u8 queue);
+extern s32 ixgbe_fdir_set_input_mask_82599(struct ixgbe_hw *hw,
+					   union ixgbe_atr_input *input_mask);
+extern s32 ixgbe_fdir_write_perfect_filter_82599(struct ixgbe_hw *hw,
+						 union ixgbe_atr_input *input,
+						 u16 soft_id, u8 queue);
+extern s32 ixgbe_fdir_erase_perfect_filter_82599(struct ixgbe_hw *hw,
+						 union ixgbe_atr_input *input,
+						 u16 soft_id);
+extern void ixgbe_atr_compute_perfect_hash_82599(union ixgbe_atr_input *input,
+						 union ixgbe_atr_input *mask);
 extern void ixgbe_configure_rscctl(struct ixgbe_adapter *adapter,
                                    struct ixgbe_ring *ring);
 extern void ixgbe_clear_rscctl(struct ixgbe_adapter *adapter,
drivers/net/ixgbe/ixgbe_82599.c +355 -304

803 lines not on this page · raw message

drivers/net/ixgbe/ixgbe_main.c +1 -1
index 929f059..6b6f602 100644
@@ -5137,7 +5137,7 @@ static int __devinit ixgbe_sw_init(struct ixgbe_adapter *adapter)
 		adapter->atr_sample_rate = 20;
 		adapter->ring_feature[RING_F_FDIR].indices =
 							 IXGBE_MAX_FDIR_INDICES;
-		adapter->fdir_pballoc = 0;
+		adapter->fdir_pballoc = IXGBE_FDIR_PBALLOC_64K;
 #ifdef IXGBE_FCOE
 		adapter->flags |= IXGBE_FLAG_FCOE_CAPABLE;
 		adapter->flags &= ~IXGBE_FLAG_FCOE_ENABLED;
drivers/net/ixgbe/ixgbe_type.h +8 -16
index 2f8ba73..efcf65e 100644
@@ -1922,9 +1922,10 @@
 #endif
 
 enum ixgbe_fdir_pballoc_type {
-	IXGBE_FDIR_PBALLOC_64K = 0,
-	IXGBE_FDIR_PBALLOC_128K,
-	IXGBE_FDIR_PBALLOC_256K,
+	IXGBE_FDIR_PBALLOC_NONE = 0,
+	IXGBE_FDIR_PBALLOC_64K  = 1,
+	IXGBE_FDIR_PBALLOC_128K = 2,
+	IXGBE_FDIR_PBALLOC_256K = 3,
 };
 #define IXGBE_FDIR_PBALLOC_SIZE_SHIFT           16
 

@@ -1978,7 +1979,7 @@ enum ixgbe_fdir_pballoc_type {
 #define IXGBE_FDIRCMD_CMD_ADD_FLOW              0x00000001
 #define IXGBE_FDIRCMD_CMD_REMOVE_FLOW           0x00000002
 #define IXGBE_FDIRCMD_CMD_QUERY_REM_FILT        0x00000003
-#define IXGBE_FDIRCMD_CMD_QUERY_REM_HASH        0x00000007
+#define IXGBE_FDIRCMD_FILTER_VALID              0x00000004
 #define IXGBE_FDIRCMD_FILTER_UPDATE             0x00000008
 #define IXGBE_FDIRCMD_IPv6DMATCH                0x00000010
 #define IXGBE_FDIRCMD_L4TYPE_UDP                0x00000020

@@ -1997,6 +1998,7 @@ enum ixgbe_fdir_pballoc_type {
 #define IXGBE_FDIR_INIT_DONE_POLL               10
 #define IXGBE_FDIRCMD_CMD_POLL                  10
 
+#define IXGBE_FDIR_DROP_QUEUE                   127
 /* Transmit Descriptor - Advanced */
 union ixgbe_adv_tx_desc {
 	struct {

@@ -2183,7 +2185,7 @@ union ixgbe_atr_input {
 	 * src_port   - 2 bytes
 	 * dst_port   - 2 bytes
 	 * flex_bytes - 2 bytes
-	 * rsvd0      - 2 bytes - space reserved must be 0.
+	 * bkt_hash   - 2 bytes
 	 */
 	struct {
 		u8     vm_pool;

@@ -2194,7 +2196,7 @@ union ixgbe_atr_input {
 		__be16 src_port;
 		__be16 dst_port;
 		__be16 flex_bytes;
-		__be16 rsvd0;
+		__be16 bkt_hash;
 	} formatted;
 	__be32 dword_stream[11];
 };

@@ -2215,16 +2217,6 @@ union ixgbe_atr_hash_dword {
 	__be32 dword;
 };
 
-struct ixgbe_atr_input_masks {
-	__be16 rsvd0;
-	__be16 vlan_id_mask;
-	__be32 dst_ip_mask[4];
-	__be32 src_ip_mask[4];
-	__be16 src_port_mask;
-	__be16 dst_port_mask;
-	__be16 flex_mask;
-};
-
 enum ixgbe_eeprom_type {
 	ixgbe_eeprom_uninitialized = 0,
 	ixgbe_eeprom_spi,
author
#20 lore raw

RFC PATCH 7/10 ixgbe: add basic support for settting and getting nfc controls

In reply to Alexander Duyck

This change adds basic suppport for the obtaining of RSS ring counts and setting of RSS hash options.

Signed-off-by Alexander Duyck <[email protected]>

---

 drivers/net/ixgbe/ixgbe_ethtool.c |   60 +++++++++++++++++++++++++++++++++++++
 1 files changed, 60 insertions(+), 0 deletions(-)
drivers/net/ixgbe/ixgbe_ethtool.c +60 -0
index 1b40c02..6c17e45 100644
@@ -2299,6 +2299,65 @@ static int ixgbe_set_flags(struct net_device *netdev, u32 data)
 	return 0;
 }
 
+static int ixgbe_get_rss_hash_opts(struct ixgbe_adapter *adapter,
+				   struct ethtool_rxnfc *cmd)
+{
+	cmd->data = 0;
+
+	/* if RSS is disabled then report no hashing */
+	if (!(adapter->flags & IXGBE_FLAG_RSS_ENABLED))
+		return 0;
+
+	/* Report default options for RSS on ixgbe */
+	switch (cmd->flow_type) {
+	case TCP_V4_FLOW:
+		cmd->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
+	case UDP_V4_FLOW:
+	case SCTP_V4_FLOW:
+	case AH_ESP_V4_FLOW:
+	case AH_V4_FLOW:
+	case ESP_V4_FLOW:
+	case IPV4_FLOW:
+		cmd->data |= RXH_IP_SRC | RXH_IP_DST;
+		break;
+	case TCP_V6_FLOW:
+		cmd->data |= RXH_L4_B_0_1 | RXH_L4_B_2_3;
+	case UDP_V6_FLOW:
+	case SCTP_V6_FLOW:
+	case AH_ESP_V6_FLOW:
+	case AH_V6_FLOW:
+	case ESP_V6_FLOW:
+	case IPV6_FLOW:
+		cmd->data |= RXH_IP_SRC | RXH_IP_DST;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ixgbe_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
+			   void *rule_locs)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(dev);
+	int ret = -EOPNOTSUPP;
+
+	switch (cmd->cmd) {
+	case ETHTOOL_GRXFH:
+		ret = ixgbe_get_rss_hash_opts(adapter, cmd);
+		break;
+	case ETHTOOL_GRXRINGS:
+		cmd->data = adapter->num_rx_queues;
+		ret = 0;
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
 static const struct ethtool_ops ixgbe_ethtool_ops = {
 	.get_settings           = ixgbe_get_settings,
 	.set_settings           = ixgbe_set_settings,

@@ -2334,6 +2393,7 @@ static const struct ethtool_ops ixgbe_ethtool_ops = {
 	.set_coalesce           = ixgbe_set_coalesce,
 	.get_flags              = ethtool_op_get_flags,
 	.set_flags              = ixgbe_set_flags,
+	.get_rxnfc		= ixgbe_get_rxnfc,
 };
 
 void ixgbe_set_ethtool_ops(struct net_device *netdev)
author
#21 lore raw

RFC PATCH 8/10 ixgbe: add support for displaying ntuple filters via the nfc interface

In reply to Alexander Duyck

This code adds support for displaying the filters that were added via the nfc interface. This is primarily to test the interface for now, but I am also looking into the feasability of moving all of the ntuple filter code in ixgbe over to the nfc interface since it seems to be better implemented.

Signed-off-by Alexander Duyck <[email protected]>

---

 drivers/net/ixgbe/ixgbe.h         |   11 ++++
 drivers/net/ixgbe/ixgbe_ethtool.c |   95 +++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+), 0 deletions(-)
drivers/net/ixgbe/ixgbe.h +11 -0
index 0cd75d6..903c828 100644
@@ -466,6 +466,17 @@ struct ixgbe_adapter {
 	DECLARE_BITMAP(active_vfs, IXGBE_MAX_VF_FUNCTIONS);
 	unsigned int num_vfs;
 	struct vf_data_storage *vfinfo;
+
+	struct hlist_head fdir_filter_list;
+	union ixgbe_atr_input fdir_mask;
+	int fdir_filter_count;
+};
+
+struct ixgbe_fdir_filter {
+	struct  hlist_node fdir_node;
+	union ixgbe_atr_input filter;
+	u16 sw_idx;
+	u16 action;
 };
 
 enum ixbge_state_t {
drivers/net/ixgbe/ixgbe_ethtool.c +95 -0

107 lines not on this page · raw message

author
#22 lore raw

RFC PATCH 9/10 ixgbe: add support for nfc addition and removal of filters

In reply to Alexander Duyck

This change is meant to allow for nfc to insert and remove filters in order to test the ethtool interface which includes it's own rules manager.

Signed-off-by Alexander Duyck <[email protected]>

---

 drivers/net/ixgbe/ixgbe_ethtool.c |  232 +++++++++++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_main.c    |   45 +++++++
 2 files changed, 277 insertions(+), 0 deletions(-)
drivers/net/ixgbe/ixgbe_ethtool.c +232 -0
index 145e018..a91e8db 100644
@@ -2453,6 +2453,237 @@ static int ixgbe_get_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd,
 	return ret;
 }
 
+static int ixgbe_update_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
+					   struct ixgbe_fdir_filter *input,
+					   u16 sw_idx)
+{
+	struct ixgbe_hw *hw = &adapter->hw;
+	struct hlist_node *node, *node2, *parent;
+	struct ixgbe_fdir_filter *rule;
+	int err = -EINVAL;
+
+	parent = NULL;
+	rule = NULL;
+
+	hlist_for_each_entry_safe(rule, node, node2,
+				  &adapter->fdir_filter_list, fdir_node) {
+		/* hash found, or no matching entry */
+		if (rule->sw_idx >= sw_idx)
+			break;
+		parent = node;
+	}
+
+	/* if there is an old rule occupying our place remove it */
+	if (rule && (rule->sw_idx == sw_idx)) {
+		if (!input || (rule->filter.formatted.bkt_hash !=
+			       input->filter.formatted.bkt_hash)) {
+			err = ixgbe_fdir_erase_perfect_filter_82599(hw,
+								    &rule->filter,
+								    sw_idx);
+		}
+
+		hlist_del(&rule->fdir_node);
+		kfree(rule);
+		adapter->fdir_filter_count--;
+	}
+
+	/* stop here if there was not an input filter provided */
+	if (!input)
+		return err;
+
+	/* reset error since we are now adding a filter */
+	err = 0;
+
+	/* initialize node and set software index */
+	INIT_HLIST_NODE(&input->fdir_node);
+
+	/* add filter to the list */
+	if (parent)
+		hlist_add_after(parent, &input->fdir_node);
+	else
+		hlist_add_head(&input->fdir_node,
+			       &adapter->fdir_filter_list);
+
+	/* update counts */
+	adapter->fdir_filter_count++;
+
+	return err;
+}
+
+static int ixgbe_flowspec_to_flow_type(struct ethtool_rx_flow_spec *fsp,
+				       u8 *flow_type)
+{
+	switch (fsp->flow_type) {
+	case TCP_V4_FLOW:
+		*flow_type = IXGBE_ATR_FLOW_TYPE_TCPV4;
+		break;
+	case UDP_V4_FLOW:
+		*flow_type = IXGBE_ATR_FLOW_TYPE_UDPV4;
+		break;
+	case SCTP_V4_FLOW:
+		*flow_type = IXGBE_ATR_FLOW_TYPE_SCTPV4;
+		break;
+	case IP_USER_FLOW:
+		switch (fsp->h_u.usr_ip4_spec.proto) {
+		case IPPROTO_TCP:
+			*flow_type = IXGBE_ATR_FLOW_TYPE_TCPV4;
+			break;
+		case IPPROTO_UDP:
+			*flow_type = IXGBE_ATR_FLOW_TYPE_UDPV4;
+			break;
+		case IPPROTO_SCTP:
+			*flow_type = IXGBE_ATR_FLOW_TYPE_SCTPV4;
+			break;
+		case 0:
+			if (!fsp->m_u.usr_ip4_spec.proto) {
+				*flow_type = IXGBE_ATR_FLOW_TYPE_IPV4;
+				break;
+			}
+		default:
+			return 0;
+		}
+		break;
+	default:
+		return 0;
+	}
+
+	return 1;
+}
+
+static int ixgbe_add_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
+					struct ethtool_rxnfc *cmd)
+{
+	struct ethtool_rx_flow_spec *fsp =
+		(struct ethtool_rx_flow_spec *)&cmd->fs;
+	struct ixgbe_hw *hw = &adapter->hw;
+	struct ixgbe_fdir_filter *input;
+	union ixgbe_atr_input mask;
+	int err;
+
+	if (!(adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE))
+		return -EOPNOTSUPP;
+
+	/*
+	 * Don't allow programming if the action is a queue greater than
+	 * the number of online Rx queues.
+	 */
+	if ((fsp->ring_cookie != RX_CLS_FLOW_DISC) &&
+	    (fsp->ring_cookie >= adapter->num_rx_queues))
+		return -EINVAL;
+
+	input = kzalloc(sizeof(*input), GFP_ATOMIC);
+	if (!input)
+		return -ENOMEM;
+
+	memset(&mask, 0, sizeof(union ixgbe_atr_input));
+
+	/* set SW index */
+	input->sw_idx = fsp->location;
+
+	/* record flow type */
+	if (!ixgbe_flowspec_to_flow_type(fsp,
+					 &input->filter.formatted.flow_type)) {
+		e_err(drv, "Unrecognized flow type\n");
+		goto err_out;
+	}
+
+	mask.formatted.flow_type = IXGBE_ATR_L4TYPE_IPV6_MASK |
+				   IXGBE_ATR_L4TYPE_MASK;
+
+	if (input->filter.formatted.flow_type == IXGBE_ATR_FLOW_TYPE_IPV4)
+		mask.formatted.flow_type &= IXGBE_ATR_L4TYPE_IPV6_MASK;
+
+	/* Copy input into formatted structures */
+	input->filter.formatted.src_ip[0] = fsp->h_u.tcp_ip4_spec.ip4src;
+	mask.formatted.src_ip[0] = fsp->m_u.tcp_ip4_spec.ip4src;
+	input->filter.formatted.dst_ip[0] = fsp->h_u.tcp_ip4_spec.ip4dst;
+	mask.formatted.dst_ip[0] = fsp->m_u.tcp_ip4_spec.ip4dst;
+	input->filter.formatted.src_port = fsp->h_u.tcp_ip4_spec.psrc;
+	mask.formatted.src_port = fsp->m_u.tcp_ip4_spec.psrc;
+	input->filter.formatted.dst_port = fsp->h_u.tcp_ip4_spec.pdst;
+	mask.formatted.dst_port = fsp->m_u.tcp_ip4_spec.pdst;
+
+	/* determine if we need to drop or route the packet */
+	if (fsp->ring_cookie == RX_CLS_FLOW_DISC)
+		input->action = IXGBE_FDIR_DROP_QUEUE;
+	else
+		input->action = fsp->ring_cookie;
+
+	spin_lock(&adapter->fdir_perfect_lock);
+
+	if (hlist_empty(&adapter->fdir_filter_list)) {
+		/* save mask and program input mask into HW */
+		memcpy(&adapter->fdir_mask, &mask, sizeof(mask));
+		err = ixgbe_fdir_set_input_mask_82599(hw, &mask);
+		if (err) {
+			e_err(drv, "Error writing mask\n");
+			goto err_out_w_lock;
+		}
+	} else if (memcmp(&adapter->fdir_mask, &mask, sizeof(mask))) {
+		e_err(drv, "Only one mask supported per port\n");
+		goto err_out_w_lock;
+	}
+
+	/* apply mask and compute/store hash */
+	ixgbe_atr_compute_perfect_hash_82599(&input->filter, &mask);
+
+	/* Don't exceed the available space for filters in the HW */
+	if (adapter->fdir_filter_count >=
+	    ((1024 << adapter->fdir_pballoc) - 2))
+		goto err_out_w_lock;
+
+	/* program filters to filter memory */
+	err = ixgbe_fdir_write_perfect_filter_82599(hw,
+				&input->filter, input->sw_idx,
+				adapter->rx_ring[input->action]->reg_idx);
+	if (err)
+		goto err_out_w_lock;
+
+	ixgbe_update_ethtool_fdir_entry(adapter, input, input->sw_idx);
+
+	spin_unlock(&adapter->fdir_perfect_lock);
+
+	return err;
+err_out_w_lock:
+	spin_unlock(&adapter->fdir_perfect_lock);
+err_out:
+	kfree(input);
+	return -1;
+}
+
+static int ixgbe_del_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
+					struct ethtool_rxnfc *cmd)
+{
+	struct ethtool_rx_flow_spec *fsp =
+		(struct ethtool_rx_flow_spec *)&cmd->fs;
+	int err;
+
+	spin_lock(&adapter->fdir_perfect_lock);
+	err = ixgbe_update_ethtool_fdir_entry(adapter, NULL, fsp->location);
+	spin_unlock(&adapter->fdir_perfect_lock);
+
+	return err;
+}
+
+static int ixgbe_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(dev);
+	int ret = -EOPNOTSUPP;
+
+	switch (cmd->cmd) {
+	case ETHTOOL_SRXCLSRLINS:
+		ret = ixgbe_add_ethtool_fdir_entry(adapter, cmd);
+		break;
+	case ETHTOOL_SRXCLSRLDEL:
+		ret = ixgbe_del_ethtool_fdir_entry(adapter, cmd);
+		break;
+	default:
+		break;
+	}
+
+	return ret;
+}
+
 static const struct ethtool_ops ixgbe_ethtool_ops = {
 	.get_settings           = ixgbe_get_settings,
 	.set_settings           = ixgbe_set_settings,

@@ -2489,6 +2720,7 @@ static const struct ethtool_ops ixgbe_ethtool_ops = {
 	.get_flags              = ethtool_op_get_flags,
 	.set_flags              = ixgbe_set_flags,
 	.get_rxnfc		= ixgbe_get_rxnfc,
+	.set_rxnfc		= ixgbe_set_rxnfc,
 };
 
 void ixgbe_set_ethtool_ops(struct net_device *netdev)
drivers/net/ixgbe/ixgbe_main.c +45 -0

69 lines not on this page · raw message

author
#23 lore raw

RFC PATCH 10/10 ixgbe: Add support for using the same fields as ntuple in nfc

In reply to Alexander Duyck

This change is meant to make use of the NTUPLE_FLOW_EXT to allow the network flow classifier interface to support the same type of options in terms of VLAN and User-defined as the ntuple interface.

Signed-off-by Alexander Duyck <[email protected]>

---

 drivers/net/ixgbe/ixgbe_ethtool.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
drivers/net/ixgbe/ixgbe_ethtool.c +19 -0
index a91e8db..494e1bf 100644
@@ -2388,6 +2388,13 @@ static int ixgbe_get_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
 	fsp->m_u.tcp_ip4_spec.ip4src = mask->formatted.src_ip[0];
 	fsp->h_u.tcp_ip4_spec.ip4dst = rule->filter.formatted.dst_ip[0];
 	fsp->m_u.tcp_ip4_spec.ip4dst = mask->formatted.dst_ip[0];
+	fsp->h_u.ntuple_spec.vlan_tci = rule->filter.formatted.vlan_id;
+	fsp->m_u.ntuple_spec.vlan_tci = mask->formatted.vlan_id;
+	fsp->h_u.ntuple_spec.vlan_etype = rule->filter.formatted.flex_bytes;
+	fsp->m_u.ntuple_spec.vlan_etype = mask->formatted.flex_bytes;
+	*(u8 *)fsp->h_u.ntuple_spec.data = rule->filter.formatted.vm_pool;
+	*(u8 *)fsp->m_u.ntuple_spec.data = mask->formatted.vm_pool;
+	fsp->flow_type_ext = NTUPLE_FLOW_EXT;
 
 	/* record action */
 	if (rule->action == IXGBE_FDIR_DROP_QUEUE)

@@ -2603,6 +2610,18 @@ static int ixgbe_add_ethtool_fdir_entry(struct ixgbe_adapter *adapter,
 	input->filter.formatted.dst_port = fsp->h_u.tcp_ip4_spec.pdst;
 	mask.formatted.dst_port = fsp->m_u.tcp_ip4_spec.pdst;
 
+	if (fsp->flow_type_ext == NTUPLE_FLOW_EXT) {
+		input->filter.formatted.vm_pool =
+				*(unsigned char *)fsp->h_u.ntuple_spec.data;
+		mask.formatted.vm_pool =
+				*(unsigned char *)fsp->m_u.ntuple_spec.data;
+		input->filter.formatted.vlan_id = fsp->h_u.ntuple_spec.vlan_tci;
+		mask.formatted.vlan_id = fsp->m_u.ntuple_spec.vlan_tci;
+		input->filter.formatted.flex_bytes =
+						fsp->h_u.ntuple_spec.vlan_etype;
+		mask.formatted.flex_bytes = fsp->m_u.ntuple_spec.vlan_etype;
+	}
+
 	/* determine if we need to drop or route the packet */
 	if (fsp->ring_cookie == RX_CLS_FLOW_DISC)
 		input->action = IXGBE_FDIR_DROP_QUEUE;