Add support for RX packet classification in a network device
Subsystems (3)
PATCH 2/3 Add support for RX packet classification in a network device
From: Santwona Behera <[email protected]>
Subject: [PATCH 2/3] [Ethtool] Add RX pkt classification interface
--- include/linux/ethtool.h | 95 +++++++++++++++++++++++++++++++++++++++++------ net/core/ethtool.c | 60 ++++++++++++++++++++++++----- 2 files changed, 133 insertions(+), 22 deletions(-)
include/linux/ethtool.h +83 -12
@@ -55,12 +55,13 @@ struct ethtool_drvinfo {char bus_info[ETHTOOL_BUSINFO_LEN ];/* Bus info for this IF. */ /* For PCI devices, use pci_name(pci_dev). */ char reserved1[32 ]; -char reserved2[12 ]; +char reserved2[8 ];__u32 n_priv_flags;/* number of flags valid in ETHTOOL_GPFLAGS */ __u32 n_stats;/* number of u64's from ETHTOOL_GSTATS */ __u32 testinfo_len;__u32 eedump_len;/* Size of data from ETHTOOL_GEEPROM (bytes) */ __u32 regdump_len;/* Size of data from ETHTOOL_GREGS (bytes) */ +__u32 n_rx_rules;/* number of rx classification rules */ };#define SOPASS_MAX 6
@@ -287,10 +288,71 @@ enum ethtool_flags { ETH_FLAG_LRO = (1 << 15), /* LRO is enabled */ }; -struct ethtool_rxnfc { -__u32 cmd; +struct ethtool_tcpip4_spec { +__u32 ip4src; +__u32 ip4dst; +__u16 psrc; +__u16 pdst; +__u8 tos; +}; + +struct ethtool_ah_espip4_spec { +__u32 ip4src; +__u32 ip4dst; +__u32 spi; +__u8 tos; +}; + +struct ethtool_rawip4_spec { +__u32 ip4src; +__u32 ip4dst; +__u8 hdata[64 ]; +}; + +struct ethtool_ether_spec { +__u16 ether_type; +__u8 frame_size; +__u8 eframe[16 ]; +}; + +#define ETH_RX_NFC_IP4 1 +#define ETH_RX_NFC_IP6 2 + +struct ethtool_usrip4_spec { +__u32 ip4src; +__u32 ip4dst; +__u32 l4_4_bytes; +__u8 tos; +__u8 ip_ver; +__u8 proto; +}; + +struct ethtool_rx_flow_spec {__u32 flow_type; -__u64 data; +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_rawip4_spec raw_ip4_spec; +struct ethtool_ether_spec ether_spec; +struct ethtool_usrip4_spec usr_ip4_spec; +__u8 hdata[64 ]; + } h_u, m_u;/* entry, mask */ +__u64 ring_cookie; +__u32 location; +}; + +struct ethtool_rxnfc { +__u32 cmd; +__u32 flow_type; +/* The rx flow hash value or the rule DB size */ +__u64 data; +struct ethtool_rx_flow_spec fs; +__u32 rule_cnt; +__u32 rule_locs[0 ]; };#ifdef __KERNEL__
@@ -417,8 +479,9 @@ struct ethtool_ops {/* the following hooks are obsolete */ int (*self_test_count)(struct net_device *);/* use get_sset_count */ int (*get_stats_count)(struct net_device *);/* use get_sset_count */ -int (*get_rxhash)(struct net_device *,struct ethtool_rxnfc *); - int (*set_rxhash)(struct net_device *, struct ethtool_rxnfc *); +int (*get_rxrule_cnt)(struct net_device *); +int (*get_rxnfc)(struct net_device *,struct ethtool_rxnfc *,void *); + int (*set_rxnfc)(struct net_device *, struct ethtool_rxnfc *); };#endif /* __KERNEL__ */
@@ -467,6 +530,11 @@ struct ethtool_ops {#define ETHTOOL_GRXFH 0x00000029/* Get RX flow hash configuration */ #define ETHTOOL_SRXFH 0x0000002a/* Set RX flow hash configuration */ +#define ETHTOOL_GRXRINGS 0x0000002b/* Get RX rings available for LB */ +#define ETHTOOL_GRXCLSRULE 0x0000002c/* Get RX classification rule */ +#define ETHTOOL_GRXCLSRLALL 0x0000002d/* Get all RX classification rule */ +#define ETHTOOL_SRXCLSRLDEL 0x0000002e/* Delete RX classification rule */ +#define ETHTOOL_SRXCLSRLINS 0x0000002f/* Insert RX classification rule */ /* compatibility with older code */ #define SPARC_ETH_GSET ETHTOOL_GSET
@@ -558,14 +626,16 @@ struct ethtool_ops {#define TCP_V4_FLOW 0x01#define UDP_V4_FLOW 0x02#define SCTP_V4_FLOW 0x03 -#define AH_ESP_V4_FLOW 0x04 -#define TCP_V6_FLOW 0x05 -#define UDP_V6_FLOW 0x06 -#define SCTP_V6_FLOW 0x07 -#define AH_ESP_V6_FLOW 0x08 +#define AH_V4_FLOW 0x04 +#define ESP_V4_FLOW 0x05 +#define TCP_V6_FLOW 0x06 +#define UDP_V6_FLOW 0x07 +#define SCTP_V6_FLOW 0x08 +#define AH_V6_FLOW 0x09 +#define ESP_V6_FLOW 0x0a +#define IP_USER_FLOW 0x0b/* L3-L4 network traffic flow hash options */ -#define RXH_DEV_PORT (1 << 0)#define RXH_L2DA (1 << 1)#define RXH_VLAN (1 << 2)#define RXH_L3_PROTO (1 << 3)
@@ -575,5 +645,6 @@ struct ethtool_ops {#define RXH_L4_B_2_3 (1 << 7)/* dst port in case of TCP/UDP/SCTP */ #define RXH_DISCARD (1 << 31) +#define RX_CLS_FLOW_DISC 0xffffffffffffffffULL#endif /* _LINUX_ETHTOOL_H */
net/core/ethtool.c +50 -10
106 lines not on this page · raw message
Signature · 1 line
1.6.0.4
Quoted 7 lines · On Mon, 2008-12-22 at 10:45 -0800, [email protected] wrote:
include/linux/ethtool.h +0
-0@@ -55,12 +55,13 @@ struct ethtool_drvinfo {char bus_info[ETHTOOL_BUSINFO_LEN ];/* Bus info for this IF. */ /* For PCI devices, use pci_name(pci_dev). */
include/linux/ethtool.h struct ethtool_drvinfo { · line 65
Quoted 10 lines
include/linux/ethtool.h +2
-1char reserved1[32 ]; -char reserved2[12 ]; +char reserved2[8 ];__u32 n_priv_flags;/* number of flags valid in ETHTOOL_GPFLAGS */ __u32 n_stats;/* number of u64's from ETHTOOL_GSTATS */ __u32 testinfo_len;__u32 eedump_len;/* Size of data from ETHTOOL_GEEPROM (bytes) */ __u32 regdump_len;/* Size of data from ETHTOOL_GREGS (bytes) */ +__u32 n_rx_rules;/* number of rx classification rules */ };
This shifts all the fields between n_priv_flags and regdump_len inclusive. What is the point of reserving space in the structure if we then go and move fields around elsewhere?
Also, why do you think n_rx_rules is driver or hardware information? The maximum number of RX filters is not necessarily a static property. Consider hardware that has separate limited-size sets of layer-2 and layer-3 filters, or that has a single set but needs more storage for some types of filters.
The important value is the current number of rules which is dynamic and does not belong here.
[...]
Quoted 13 lines
include/linux/ethtool.h +4
-5@@ -558,14 +626,16 @@ struct ethtool_ops {#define TCP_V4_FLOW 0x01#define UDP_V4_FLOW 0x02#define SCTP_V4_FLOW 0x03 -#define AH_ESP_V4_FLOW 0x04 -#define TCP_V6_FLOW 0x05 -#define UDP_V6_FLOW 0x06 -#define SCTP_V6_FLOW 0x07 -#define AH_ESP_V6_FLOW 0x08 +#define AH_V4_FLOW 0x04 +#define ESP_V4_FLOW 0x05 +#define TCP_V6_FLOW 0x06 +#define UDP_V6_FLOW 0x07
include/linux/ethtool.h struct ethtool_ops { · line 641
Quoted 10 lines
include/linux/ethtool.h +4
-1+#define SCTP_V6_FLOW 0x08 +#define AH_V6_FLOW 0x09 +#define ESP_V6_FLOW 0x0a +#define IP_USER_FLOW 0x0b/* L3-L4 network traffic flow hash options */ -#define RXH_DEV_PORT (1 << 0)#define RXH_L2DA (1 << 1)#define RXH_VLAN (1 << 2)#define RXH_L3_PROTO (1 << 3)
[...]
No, you can't do this. Leave the existing definitions unchanged and only add new ones.
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.
Quoted 29 lines · Ben Hutchings
Quoted 7 lines · On Mon, 2008-12-22 at 10:45 -0800, [email protected] wrote:
include/linux/ethtool.h +0
-0@@ -55,12 +55,13 @@ struct ethtool_drvinfo {char bus_info[ETHTOOL_BUSINFO_LEN ];/* Bus info for this IF. */ /* For PCI devices, use pci_name(pci_dev). */ include/linux/ethtool.h struct ethtool_drvinfo { · line 65
Quoted 10 lines
include/linux/ethtool.h +2
-1char reserved1[32 ]; -char reserved2[12 ]; +char reserved2[8 ];__u32 n_priv_flags;/* number of flags valid in ETHTOOL_GPFLAGS */ __u32 n_stats;/* number of u64's from ETHTOOL_GSTATS */ __u32 testinfo_len;__u32 eedump_len;/* Size of data from ETHTOOL_GEEPROM (bytes) */ __u32 regdump_len;/* Size of data from ETHTOOL_GREGS (bytes) */ +__u32 n_rx_rules;/* number of rx classification rules */ };This shifts all the fields between
n_priv_flagsandregdump_leninclusive. What is the point of reserving space in the structure if we then go and move fields around elsewhere?Also, why do you think
n_rx_rulesis driver or hardware information? The maximum number of RX filters is not necessarily a static property. Consider hardware that has separate limited-size sets of layer-2 and layer-3 filters, or that has a single set but needs more storage for some types of filters.
Quoted 4 lines
The important value is the current number of rules which is dynamic and does not belong here.
[...]
OK, I will move this to the ethtool_rxnfc struct.
Quoted 25 lines
Quoted 23 lines
include/linux/ethtool.h +8
-6@@ -558,14 +626,16 @@ struct ethtool_ops {#define TCP_V4_FLOW 0x01#define UDP_V4_FLOW 0x02#define SCTP_V4_FLOW 0x03 -#define AH_ESP_V4_FLOW 0x04 -#define TCP_V6_FLOW 0x05 -#define UDP_V6_FLOW 0x06 -#define SCTP_V6_FLOW 0x07 -#define AH_ESP_V6_FLOW 0x08 +#define AH_V4_FLOW 0x04 +#define ESP_V4_FLOW 0x05 +#define TCP_V6_FLOW 0x06 +#define UDP_V6_FLOW 0x07 +#define SCTP_V6_FLOW 0x08 +#define AH_V6_FLOW 0x09 +#define ESP_V6_FLOW 0x0a +#define IP_USER_FLOW 0x0b/* L3-L4 network traffic flow hash options */ -#define RXH_DEV_PORT (1 << 0)#define RXH_L2DA (1 << 1)#define RXH_VLAN (1 << 2)#define RXH_L3_PROTO (1 << 3)[...]
Quoted 2 lines
No, you can't do this. Leave the existing definitions unchanged and only add new ones.
The original code/patch was not quite correct where the AH_ESP_V4_FLOW was being used to represent AH flows. So my goal here was to remove that and add 2 separate flow types for AH and ESP. I have two ways of achieving this without changing the existing definitions completely:
1. I change AH_ESP_Vx_FLOW defines to AH_Vx_FLOW defines and add 2 new defines for ESP_Vx_FLOW at the end, with values 0x9 and 0xa.
2. I keep the AH_ESP_Vx_FLOW defines as is (but this will be dead code as it will not be used) and add 2 new AH_Vx_FLOW defines and 2 new ESP_Vx_FLOW defines at the end with values 0x9, 0xa, 0xb, 0xc.
Please let me know which one is more desirable.
rgds,
--santwona
On Mon, 2008-12-22 at 15:04 -0800, [email protected] wrote:
[...]
Quoted 28 lines
Quoted 25 lines
Quoted 23 lines
include/linux/ethtool.h +8
-6@@ -558,14 +626,16 @@ struct ethtool_ops {#define TCP_V4_FLOW 0x01#define UDP_V4_FLOW 0x02#define SCTP_V4_FLOW 0x03 -#define AH_ESP_V4_FLOW 0x04 -#define TCP_V6_FLOW 0x05 -#define UDP_V6_FLOW 0x06 -#define SCTP_V6_FLOW 0x07 -#define AH_ESP_V6_FLOW 0x08 +#define AH_V4_FLOW 0x04 +#define ESP_V4_FLOW 0x05 +#define TCP_V6_FLOW 0x06 +#define UDP_V6_FLOW 0x07 +#define SCTP_V6_FLOW 0x08 +#define AH_V6_FLOW 0x09 +#define ESP_V6_FLOW 0x0a +#define IP_USER_FLOW 0x0b/* L3-L4 network traffic flow hash options */ -#define RXH_DEV_PORT (1 << 0)#define RXH_L2DA (1 << 1)#define RXH_VLAN (1 << 2)#define RXH_L3_PROTO (1 << 3)[...]
Quoted 2 lines
No, you can't do this. Leave the existing definitions unchanged and only add new ones.
Quoted 7 lines
The original code/patch was not quite correct where the
AH_ESP_V4_FLOWwas being used to represent AH flows. So my goal here was to remove that and add 2 separate flow types for AH and ESP. I have two ways of achieving this without changing the existing definitions completely:1. I change
AH_ESP_Vx_FLOWdefines toAH_Vx_FLOWdefines and add 2 new defines forESP_Vx_FLOWat the end, with values0x9and0xa.
If AH_ESP_Vx_FLOW has only ever been implemented as AH-only then perhaps this is reasonable.
Quoted 3 lines
2. I keep the
AH_ESP_Vx_FLOWdefines as is (but this will be dead code as it will not be used) and add 2 newAH_Vx_FLOWdefines and 2 newESP_Vx_FLOWdefines at the end with values0x9,0xa,0xb,0xc.
This is safest.
Also you are probably right to remove RXH_DEV_PORT as that appears to be an niu quirk that shouldn't be exposed.
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.
Quoted 10 lines
Quoted 3 lines
1. I change
AH_ESP_Vx_FLOWdefines toAH_Vx_FLOWdefines and add 2 new defines forESP_Vx_FLOWat the end, with values0x9and0xa.If
AH_ESP_Vx_FLOWhas only ever been implemented as AH-only then perhaps this is reasonable.Quoted 1 line
2. I keep the
AH_ESP_Vx_FLOWdefines as is (but this will be dead code as it will not be used) and add 2 newAH_Vx_FLOWdefines and 2 newESP_Vx_FLOWdefines at the end with values0x9,0xa,0xb,0xc.This is safest.
OK, I will re-implement it this way then.
Quoted 3 lines
Also you are probably right to remove
RXH_DEV_PORTas that appears to be an niu quirk that shouldn't be exposed.
That was the reasoning behind removing RXH_DEV_PORT.
rgds,
--santwona