* [RFC PATCH 0/3] Drop get_headlen functions in favor of generic function
@ 2014-09-04 23:13 Alexander Duyck
2014-09-04 23:13 ` [RFC PATCH 1/3] net: Add function for parsing the header length out of linear ethernet frames Alexander Duyck
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Alexander Duyck @ 2014-09-04 23:13 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet
This series replaced igb_get_headlen and ixgbe_get_headlen with a generic
function named eth_get_headlen.
This is a first pass at this and has been compile tested only. I'm mainly
looking for feedback on any issues with the approach I have taken or any
complains about the eth_get_headlen function or the changes to __skb_get_poff
in the first patch.
I'll try running this through some stress/performance testing tomorrow to
see how it compares to the original functions.
---
Alexander Duyck (3):
net: Add function for parsing the header length out of linear ethernet frames
igb: use new eth_get_headlen interface
ixgbe: use new eth_get_headlen interface
drivers/net/ethernet/intel/igb/igb_main.c | 109 -----------------------
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 116 -------------------------
include/linux/etherdevice.h | 1
include/linux/skbuff.h | 2
include/net/flow_keys.h | 2
net/core/flow_dissector.c | 41 ++++++---
net/ethernet/eth.c | 27 ++++++
7 files changed, 60 insertions(+), 238 deletions(-)
--
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/3] net: Add function for parsing the header length out of linear ethernet frames
2014-09-04 23:13 [RFC PATCH 0/3] Drop get_headlen functions in favor of generic function Alexander Duyck
@ 2014-09-04 23:13 ` Alexander Duyck
2014-09-04 23:36 ` Eric Dumazet
2014-09-04 23:13 ` [RFC PATCH 2/3] igb: use new eth_get_headlen interface Alexander Duyck
2014-09-04 23:13 ` [RFC PATCH 3/3] ixgbe: " Alexander Duyck
2 siblings, 1 reply; 6+ messages in thread
From: Alexander Duyck @ 2014-09-04 23:13 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet
This patch updates some of the flow_dissector api so that it can be used to
parse the length of ethernet buffers stored in fragments. Most of the
changes needed were to __skb_get_poff as it needed to be updated to support
sending a linear buffer instead of a skb.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
include/linux/etherdevice.h | 1 +
include/linux/skbuff.h | 2 ++
include/net/flow_keys.h | 2 ++
net/core/flow_dissector.c | 41 ++++++++++++++++++++++++++---------------
net/ethernet/eth.c | 27 +++++++++++++++++++++++++++
5 files changed, 58 insertions(+), 15 deletions(-)
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 9c5529d..733980f 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -29,6 +29,7 @@
#include <asm/bitsperlong.h>
#ifdef __KERNEL__
+u32 eth_get_headlen(void *data, unsigned int max_len);
__be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev);
extern const struct header_ops eth_header_ops;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 02529fc..77d4d6f 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3217,6 +3217,8 @@ bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off);
int skb_checksum_setup(struct sk_buff *skb, bool recalculate);
u32 __skb_get_poff(const struct sk_buff *skb);
+u32 ___skb_get_poff(const struct sk_buff *skb, void *data,
+ const struct flow_keys *keys, int hlen);
/**
* skb_head_is_locked - Determine if the skb->head is locked down
diff --git a/include/net/flow_keys.h b/include/net/flow_keys.h
index 9a03f73..7ee2df0 100644
--- a/include/net/flow_keys.h
+++ b/include/net/flow_keys.h
@@ -40,4 +40,6 @@ static inline __be32 skb_flow_get_ports(const struct sk_buff *skb, int thoff, u8
return __skb_flow_get_ports(skb, thoff, ip_proto, NULL, 0);
}
u32 flow_hash_from_keys(struct flow_keys *keys);
+unsigned int flow_get_hlen(const unsigned char *data, unsigned int max_len,
+ __be16 protocol);
#endif
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 12f48ca..f55667e 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -13,6 +13,7 @@
#include <linux/if_pppox.h>
#include <linux/ppp_defs.h>
#include <net/flow_keys.h>
+#include <scsi/fc/fc_fcoe.h>
/* copy saddr & daddr, possibly using 64bit load/store
* Equivalent to : flow->src = iph->saddr;
@@ -118,7 +119,7 @@ ipv6:
nhoff += sizeof(struct ipv6hdr);
flow_label = ip6_flowlabel(iph);
- if (flow_label) {
+ if (flow_label && skb) {
/* Awesome, IPv6 packet has a flow label so we can
* use that to represent the ports without any
* further dissection.
@@ -165,6 +166,9 @@ ipv6:
return false;
}
}
+ case htons(ETH_P_FCOE):
+ flow->thoff = (u16)(nhoff + FCOE_HEADER_LEN);
+ /* fall through */
default:
return false;
}
@@ -316,26 +320,18 @@ u16 __skb_tx_hash(const struct net_device *dev, struct sk_buff *skb,
}
EXPORT_SYMBOL(__skb_tx_hash);
-/* __skb_get_poff() returns the offset to the payload as far as it could
- * be dissected. The main user is currently BPF, so that we can dynamically
- * truncate packets without needing to push actual payload to the user
- * space and can analyze headers only, instead.
- */
-u32 __skb_get_poff(const struct sk_buff *skb)
+u32 ___skb_get_poff(const struct sk_buff *skb, void *data,
+ const struct flow_keys *keys, int hlen)
{
- struct flow_keys keys;
- u32 poff = 0;
-
- if (!skb_flow_dissect(skb, &keys))
- return 0;
+ u32 poff = keys->thoff;
- poff += keys.thoff;
- switch (keys.ip_proto) {
+ switch (keys->ip_proto) {
case IPPROTO_TCP: {
const struct tcphdr *tcph;
struct tcphdr _tcph;
- tcph = skb_header_pointer(skb, poff, sizeof(_tcph), &_tcph);
+ tcph = __skb_header_pointer(skb, poff, sizeof(_tcph),
+ data, hlen, &_tcph);
if (!tcph)
return poff;
@@ -369,6 +365,21 @@ u32 __skb_get_poff(const struct sk_buff *skb)
return poff;
}
+/* __skb_get_poff() returns the offset to the payload as far as it could
+ * be dissected. The main user is currently BPF, so that we can dynamically
+ * truncate packets without needing to push actual payload to the user
+ * space and can analyze headers only, instead.
+ */
+u32 __skb_get_poff(const struct sk_buff *skb)
+{
+ struct flow_keys keys;
+
+ if (!skb_flow_dissect(skb, &keys))
+ return 0;
+
+ return ___skb_get_poff(skb, skb->data, &keys, skb->len);
+}
+
static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
{
#ifdef CONFIG_XPS
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index 5cebca1..f0299ac 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -146,6 +146,33 @@ int eth_rebuild_header(struct sk_buff *skb)
EXPORT_SYMBOL(eth_rebuild_header);
/**
+ * eth_get_headlen - determine the the length of header for an ethernet frame
+ * @data: pointer to start of frame
+ * @len: total length of frame
+ *
+ * Make a best effort attempt to pull the length for all of the headers for
+ * a given frame in a linear buffer.
+ */
+u32 eth_get_headlen(void *data, unsigned int len)
+{
+ const struct ethhdr *eth = (const struct ethhdr *)data;
+ struct flow_keys keys;
+
+ /* this should never happen, but better safe than sorry */
+ if (len < sizeof(*eth))
+ return len;
+
+ /* parse any remaining L2/L3 headers, check for L4 */
+ if (!__skb_flow_dissect(NULL, &keys, data,
+ eth->h_proto, sizeof(*eth), len))
+ return max_t(u32, keys.thoff, sizeof(*eth));
+
+ /* parse for any L4 headers */
+ return min_t(u32, ___skb_get_poff(NULL, data, &keys, len), len);
+}
+EXPORT_SYMBOL(eth_get_headlen);
+
+/**
* eth_type_trans - determine the packet's protocol ID.
* @skb: received socket data
* @dev: receiving network device
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 2/3] igb: use new eth_get_headlen interface
2014-09-04 23:13 [RFC PATCH 0/3] Drop get_headlen functions in favor of generic function Alexander Duyck
2014-09-04 23:13 ` [RFC PATCH 1/3] net: Add function for parsing the header length out of linear ethernet frames Alexander Duyck
@ 2014-09-04 23:13 ` Alexander Duyck
2014-09-04 23:13 ` [RFC PATCH 3/3] ixgbe: " Alexander Duyck
2 siblings, 0 replies; 6+ messages in thread
From: Alexander Duyck @ 2014-09-04 23:13 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet
Update igb to drop the igb_get_headlen function in favor of eth_get_headlen.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 109 -----------------------------
1 file changed, 1 insertion(+), 108 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 89de7fe..4c023f0 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6769,113 +6769,6 @@ static bool igb_is_non_eop(struct igb_ring *rx_ring,
}
/**
- * igb_get_headlen - determine size of header for LRO/GRO
- * @data: pointer to the start of the headers
- * @max_len: total length of section to find headers in
- *
- * This function is meant to determine the length of headers that will
- * be recognized by hardware for LRO, and GRO offloads. The main
- * motivation of doing this is to only perform one pull for IPv4 TCP
- * packets so that we can do basic things like calculating the gso_size
- * based on the average data per packet.
- **/
-static unsigned int igb_get_headlen(unsigned char *data,
- unsigned int max_len)
-{
- union {
- unsigned char *network;
- /* l2 headers */
- struct ethhdr *eth;
- struct vlan_hdr *vlan;
- /* l3 headers */
- struct iphdr *ipv4;
- struct ipv6hdr *ipv6;
- } hdr;
- __be16 protocol;
- u8 nexthdr = 0; /* default to not TCP */
- u8 hlen;
-
- /* this should never happen, but better safe than sorry */
- if (max_len < ETH_HLEN)
- return max_len;
-
- /* initialize network frame pointer */
- hdr.network = data;
-
- /* set first protocol and move network header forward */
- protocol = hdr.eth->h_proto;
- hdr.network += ETH_HLEN;
-
- /* handle any vlan tag if present */
- if (protocol == htons(ETH_P_8021Q)) {
- if ((hdr.network - data) > (max_len - VLAN_HLEN))
- return max_len;
-
- protocol = hdr.vlan->h_vlan_encapsulated_proto;
- hdr.network += VLAN_HLEN;
- }
-
- /* handle L3 protocols */
- if (protocol == htons(ETH_P_IP)) {
- if ((hdr.network - data) > (max_len - sizeof(struct iphdr)))
- return max_len;
-
- /* access ihl as a u8 to avoid unaligned access on ia64 */
- hlen = (hdr.network[0] & 0x0F) << 2;
-
- /* verify hlen meets minimum size requirements */
- if (hlen < sizeof(struct iphdr))
- return hdr.network - data;
-
- /* record next protocol if header is present */
- if (!(hdr.ipv4->frag_off & htons(IP_OFFSET)))
- nexthdr = hdr.ipv4->protocol;
- } else if (protocol == htons(ETH_P_IPV6)) {
- if ((hdr.network - data) > (max_len - sizeof(struct ipv6hdr)))
- return max_len;
-
- /* record next protocol */
- nexthdr = hdr.ipv6->nexthdr;
- hlen = sizeof(struct ipv6hdr);
- } else {
- return hdr.network - data;
- }
-
- /* relocate pointer to start of L4 header */
- hdr.network += hlen;
-
- /* finally sort out TCP */
- if (nexthdr == IPPROTO_TCP) {
- if ((hdr.network - data) > (max_len - sizeof(struct tcphdr)))
- return max_len;
-
- /* access doff as a u8 to avoid unaligned access on ia64 */
- hlen = (hdr.network[12] & 0xF0) >> 2;
-
- /* verify hlen meets minimum size requirements */
- if (hlen < sizeof(struct tcphdr))
- return hdr.network - data;
-
- hdr.network += hlen;
- } else if (nexthdr == IPPROTO_UDP) {
- if ((hdr.network - data) > (max_len - sizeof(struct udphdr)))
- return max_len;
-
- hdr.network += sizeof(struct udphdr);
- }
-
- /* If everything has gone correctly hdr.network should be the
- * data section of the packet and will be the end of the header.
- * If not then it probably represents the end of the last recognized
- * header.
- */
- if ((hdr.network - data) < max_len)
- return hdr.network - data;
- else
- return max_len;
-}
-
-/**
* igb_pull_tail - igb specific version of skb_pull_tail
* @rx_ring: rx descriptor ring packet is being transacted on
* @rx_desc: pointer to the EOP Rx descriptor
@@ -6919,7 +6812,7 @@ static void igb_pull_tail(struct igb_ring *rx_ring,
/* we need the header to contain the greater of either ETH_HLEN or
* 60 bytes if the skb->len is less than 60 for skb_pad.
*/
- pull_len = igb_get_headlen(va, IGB_RX_HDR_LEN);
+ pull_len = eth_get_headlen(va, IGB_RX_HDR_LEN);
/* align pull length to size of long to optimize memcpy performance */
skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 3/3] ixgbe: use new eth_get_headlen interface
2014-09-04 23:13 [RFC PATCH 0/3] Drop get_headlen functions in favor of generic function Alexander Duyck
2014-09-04 23:13 ` [RFC PATCH 1/3] net: Add function for parsing the header length out of linear ethernet frames Alexander Duyck
2014-09-04 23:13 ` [RFC PATCH 2/3] igb: use new eth_get_headlen interface Alexander Duyck
@ 2014-09-04 23:13 ` Alexander Duyck
2 siblings, 0 replies; 6+ messages in thread
From: Alexander Duyck @ 2014-09-04 23:13 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet
Update ixgbe to drop the ixgbe_get_headlen function in favor of eth_get_headlen.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 116 -------------------------
1 file changed, 1 insertion(+), 115 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 53fbf06..09c3746 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1521,120 +1521,6 @@ void ixgbe_alloc_rx_buffers(struct ixgbe_ring *rx_ring, u16 cleaned_count)
ixgbe_release_rx_desc(rx_ring, i);
}
-/**
- * ixgbe_get_headlen - determine size of header for RSC/LRO/GRO/FCOE
- * @data: pointer to the start of the headers
- * @max_len: total length of section to find headers in
- *
- * This function is meant to determine the length of headers that will
- * be recognized by hardware for LRO, GRO, and RSC offloads. The main
- * motivation of doing this is to only perform one pull for IPv4 TCP
- * packets so that we can do basic things like calculating the gso_size
- * based on the average data per packet.
- **/
-static unsigned int ixgbe_get_headlen(unsigned char *data,
- unsigned int max_len)
-{
- union {
- unsigned char *network;
- /* l2 headers */
- struct ethhdr *eth;
- struct vlan_hdr *vlan;
- /* l3 headers */
- struct iphdr *ipv4;
- struct ipv6hdr *ipv6;
- } hdr;
- __be16 protocol;
- u8 nexthdr = 0; /* default to not TCP */
- u8 hlen;
-
- /* this should never happen, but better safe than sorry */
- if (max_len < ETH_HLEN)
- return max_len;
-
- /* initialize network frame pointer */
- hdr.network = data;
-
- /* set first protocol and move network header forward */
- protocol = hdr.eth->h_proto;
- hdr.network += ETH_HLEN;
-
- /* handle any vlan tag if present */
- if (protocol == htons(ETH_P_8021Q)) {
- if ((hdr.network - data) > (max_len - VLAN_HLEN))
- return max_len;
-
- protocol = hdr.vlan->h_vlan_encapsulated_proto;
- hdr.network += VLAN_HLEN;
- }
-
- /* handle L3 protocols */
- if (protocol == htons(ETH_P_IP)) {
- if ((hdr.network - data) > (max_len - sizeof(struct iphdr)))
- return max_len;
-
- /* access ihl as a u8 to avoid unaligned access on ia64 */
- hlen = (hdr.network[0] & 0x0F) << 2;
-
- /* verify hlen meets minimum size requirements */
- if (hlen < sizeof(struct iphdr))
- return hdr.network - data;
-
- /* record next protocol if header is present */
- if (!(hdr.ipv4->frag_off & htons(IP_OFFSET)))
- nexthdr = hdr.ipv4->protocol;
- } else if (protocol == htons(ETH_P_IPV6)) {
- if ((hdr.network - data) > (max_len - sizeof(struct ipv6hdr)))
- return max_len;
-
- /* record next protocol */
- nexthdr = hdr.ipv6->nexthdr;
- hlen = sizeof(struct ipv6hdr);
-#ifdef IXGBE_FCOE
- } else if (protocol == htons(ETH_P_FCOE)) {
- if ((hdr.network - data) > (max_len - FCOE_HEADER_LEN))
- return max_len;
- hlen = FCOE_HEADER_LEN;
-#endif
- } else {
- return hdr.network - data;
- }
-
- /* relocate pointer to start of L4 header */
- hdr.network += hlen;
-
- /* finally sort out TCP/UDP */
- if (nexthdr == IPPROTO_TCP) {
- if ((hdr.network - data) > (max_len - sizeof(struct tcphdr)))
- return max_len;
-
- /* access doff as a u8 to avoid unaligned access on ia64 */
- hlen = (hdr.network[12] & 0xF0) >> 2;
-
- /* verify hlen meets minimum size requirements */
- if (hlen < sizeof(struct tcphdr))
- return hdr.network - data;
-
- hdr.network += hlen;
- } else if (nexthdr == IPPROTO_UDP) {
- if ((hdr.network - data) > (max_len - sizeof(struct udphdr)))
- return max_len;
-
- hdr.network += sizeof(struct udphdr);
- }
-
- /*
- * If everything has gone correctly hdr.network should be the
- * data section of the packet and will be the end of the header.
- * If not then it probably represents the end of the last recognized
- * header.
- */
- if ((hdr.network - data) < max_len)
- return hdr.network - data;
- else
- return max_len;
-}
-
static void ixgbe_set_rsc_gso_size(struct ixgbe_ring *ring,
struct sk_buff *skb)
{
@@ -1793,7 +1679,7 @@ static void ixgbe_pull_tail(struct ixgbe_ring *rx_ring,
* we need the header to contain the greater of either ETH_HLEN or
* 60 bytes if the skb->len is less than 60 for skb_pad.
*/
- pull_len = ixgbe_get_headlen(va, IXGBE_RX_HDR_SIZE);
+ pull_len = eth_get_headlen(va, IXGBE_RX_HDR_SIZE);
/* align pull length to size of long to optimize memcpy performance */
skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/3] net: Add function for parsing the header length out of linear ethernet frames
2014-09-04 23:13 ` [RFC PATCH 1/3] net: Add function for parsing the header length out of linear ethernet frames Alexander Duyck
@ 2014-09-04 23:36 ` Eric Dumazet
2014-09-05 1:00 ` Alexander Duyck
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2014-09-04 23:36 UTC (permalink / raw)
To: Alexander Duyck; +Cc: netdev, davem
On Thu, 2014-09-04 at 19:13 -0400, Alexander Duyck wrote:
> This patch updates some of the flow_dissector api so that it can be used to
> parse the length of ethernet buffers stored in fragments. Most of the
> changes needed were to __skb_get_poff as it needed to be updated to support
> sending a linear buffer instead of a skb.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> include/linux/etherdevice.h | 1 +
> include/linux/skbuff.h | 2 ++
> include/net/flow_keys.h | 2 ++
> net/core/flow_dissector.c | 41 ++++++++++++++++++++++++++---------------
> net/ethernet/eth.c | 27 +++++++++++++++++++++++++++
> 5 files changed, 58 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index 9c5529d..733980f 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -29,6 +29,7 @@
> #include <asm/bitsperlong.h>
>
> #ifdef __KERNEL__
> +u32 eth_get_headlen(void *data, unsigned int max_len);
> __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev);
> extern const struct header_ops eth_header_ops;
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 02529fc..77d4d6f 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3217,6 +3217,8 @@ bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off);
> int skb_checksum_setup(struct sk_buff *skb, bool recalculate);
>
> u32 __skb_get_poff(const struct sk_buff *skb);
> +u32 ___skb_get_poff(const struct sk_buff *skb, void *data,
> + const struct flow_keys *keys, int hlen);
>
> /**
> * skb_head_is_locked - Determine if the skb->head is locked down
> diff --git a/include/net/flow_keys.h b/include/net/flow_keys.h
> index 9a03f73..7ee2df0 100644
> --- a/include/net/flow_keys.h
> +++ b/include/net/flow_keys.h
> @@ -40,4 +40,6 @@ static inline __be32 skb_flow_get_ports(const struct sk_buff *skb, int thoff, u8
> return __skb_flow_get_ports(skb, thoff, ip_proto, NULL, 0);
> }
> u32 flow_hash_from_keys(struct flow_keys *keys);
> +unsigned int flow_get_hlen(const unsigned char *data, unsigned int max_len,
> + __be16 protocol);
> #endif
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 12f48ca..f55667e 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -13,6 +13,7 @@
> #include <linux/if_pppox.h>
> #include <linux/ppp_defs.h>
> #include <net/flow_keys.h>
> +#include <scsi/fc/fc_fcoe.h>
>
> /* copy saddr & daddr, possibly using 64bit load/store
> * Equivalent to : flow->src = iph->saddr;
> @@ -118,7 +119,7 @@ ipv6:
> nhoff += sizeof(struct ipv6hdr);
>
> flow_label = ip6_flowlabel(iph);
> - if (flow_label) {
> + if (flow_label && skb) {
Hmpff... undocumented bit here....
> /* Awesome, IPv6 packet has a flow label so we can
> * use that to represent the ports without any
> * further dissection.
> @@ -165,6 +166,9 @@ ipv6:
> return false;
> }
> }
> + case htons(ETH_P_FCOE):
> + flow->thoff = (u16)(nhoff + FCOE_HEADER_LEN);
> + /* fall through */
> default:
> return false;
> }
> @@ -316,26 +320,18 @@ u16 __skb_tx_hash(const struct net_device *dev, struct sk_buff *skb,
> }
> EXPORT_SYMBOL(__skb_tx_hash);
>
> -/* __skb_get_poff() returns the offset to the payload as far as it could
> - * be dissected. The main user is currently BPF, so that we can dynamically
> - * truncate packets without needing to push actual payload to the user
> - * space and can analyze headers only, instead.
> - */
> -u32 __skb_get_poff(const struct sk_buff *skb)
> +u32 ___skb_get_poff(const struct sk_buff *skb, void *data,
> + const struct flow_keys *keys, int hlen)
> {
> - struct flow_keys keys;
> - u32 poff = 0;
> -
> - if (!skb_flow_dissect(skb, &keys))
> - return 0;
> + u32 poff = keys->thoff;
>
> - poff += keys.thoff;
> - switch (keys.ip_proto) {
> + switch (keys->ip_proto) {
> case IPPROTO_TCP: {
> const struct tcphdr *tcph;
> struct tcphdr _tcph;
>
> - tcph = skb_header_pointer(skb, poff, sizeof(_tcph), &_tcph);
> + tcph = __skb_header_pointer(skb, poff, sizeof(_tcph),
> + data, hlen, &_tcph);
> if (!tcph)
> return poff;
>
> @@ -369,6 +365,21 @@ u32 __skb_get_poff(const struct sk_buff *skb)
> return poff;
> }
>
> +/* __skb_get_poff() returns the offset to the payload as far as it could
> + * be dissected. The main user is currently BPF, so that we can dynamically
> + * truncate packets without needing to push actual payload to the user
> + * space and can analyze headers only, instead.
> + */
> +u32 __skb_get_poff(const struct sk_buff *skb)
> +{
> + struct flow_keys keys;
> +
> + if (!skb_flow_dissect(skb, &keys))
> + return 0;
> +
> + return ___skb_get_poff(skb, skb->data, &keys, skb->len);
hlen is not skb->len, but skb_headlen(skb)
> +}
> +
> static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
> {
> #ifdef CONFIG_XPS
> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index 5cebca1..f0299ac 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -146,6 +146,33 @@ int eth_rebuild_header(struct sk_buff *skb)
> EXPORT_SYMBOL(eth_rebuild_header);
>
> /**
> + * eth_get_headlen - determine the the length of header for an ethernet frame
> + * @data: pointer to start of frame
> + * @len: total length of frame
> + *
> + * Make a best effort attempt to pull the length for all of the headers for
> + * a given frame in a linear buffer.
> + */
> +u32 eth_get_headlen(void *data, unsigned int len)
> +{
> + const struct ethhdr *eth = (const struct ethhdr *)data;
> + struct flow_keys keys;
> +
> + /* this should never happen, but better safe than sorry */
> + if (len < sizeof(*eth))
> + return len;
> +
> + /* parse any remaining L2/L3 headers, check for L4 */
> + if (!__skb_flow_dissect(NULL, &keys, data,
> + eth->h_proto, sizeof(*eth), len))
> + return max_t(u32, keys.thoff, sizeof(*eth));
Not sure keys.thoff is valid at this point ?
> +
> + /* parse for any L4 headers */
> + return min_t(u32, ___skb_get_poff(NULL, data, &keys, len), len);
> +}
> +EXPORT_SYMBOL(eth_get_headlen);
> +
> +/**
> * eth_type_trans - determine the packet's protocol ID.
> * @skb: received socket data
> * @dev: receiving network device
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 1/3] net: Add function for parsing the header length out of linear ethernet frames
2014-09-04 23:36 ` Eric Dumazet
@ 2014-09-05 1:00 ` Alexander Duyck
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Duyck @ 2014-09-05 1:00 UTC (permalink / raw)
To: Eric Dumazet, Alexander Duyck; +Cc: netdev, davem
On 09/04/2014 04:36 PM, Eric Dumazet wrote:
> On Thu, 2014-09-04 at 19:13 -0400, Alexander Duyck wrote:
>> This patch updates some of the flow_dissector api so that it can be used to
>> parse the length of ethernet buffers stored in fragments. Most of the
>> changes needed were to __skb_get_poff as it needed to be updated to support
>> sending a linear buffer instead of a skb.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
>> ---
>> include/linux/etherdevice.h | 1 +
>> include/linux/skbuff.h | 2 ++
>> include/net/flow_keys.h | 2 ++
>> net/core/flow_dissector.c | 41 ++++++++++++++++++++++++++---------------
>> net/ethernet/eth.c | 27 +++++++++++++++++++++++++++
>> 5 files changed, 58 insertions(+), 15 deletions(-)
>>
>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>> index 12f48ca..f55667e 100644
>> --- a/net/core/flow_dissector.c
>> +++ b/net/core/flow_dissector.c
>> @@ -13,6 +13,7 @@
>> #include <linux/if_pppox.h>
>> #include <linux/ppp_defs.h>
>> #include <net/flow_keys.h>
>> +#include <scsi/fc/fc_fcoe.h>
>>
>> /* copy saddr & daddr, possibly using 64bit load/store
>> * Equivalent to : flow->src = iph->saddr;
>> @@ -118,7 +119,7 @@ ipv6:
>> nhoff += sizeof(struct ipv6hdr);
>>
>> flow_label = ip6_flowlabel(iph);
>> - if (flow_label) {
>> + if (flow_label && skb) {
> Hmpff... undocumented bit here....
I'll add a comment. It is basically just forcing it to continue parsing
for the case where skb is NULL which currently only applies to
eth_get_headlen.
>
> @@ -369,6 +365,21 @@ u32 __skb_get_poff(const struct sk_buff *skb)
> return poff;
> }
>
> +/* __skb_get_poff() returns the offset to the payload as far as it could
> + * be dissected. The main user is currently BPF, so that we can dynamically
> + * truncate packets without needing to push actual payload to the user
> + * space and can analyze headers only, instead.
> + */
> +u32 __skb_get_poff(const struct sk_buff *skb)
> +{
> + struct flow_keys keys;
> +
> + if (!skb_flow_dissect(skb, &keys))
> + return 0;
> +
> + return ___skb_get_poff(skb, skb->data, &keys, skb->len);
> hlen is not skb->len, but skb_headlen(skb)
My bad. I will fix that for the next one.
>> +}
>> +
>> static inline int get_xps_queue(struct net_device *dev, struct sk_buff *skb)
>> {
>> #ifdef CONFIG_XPS
>> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
>> index 5cebca1..f0299ac 100644
>> --- a/net/ethernet/eth.c
>> +++ b/net/ethernet/eth.c
>> @@ -146,6 +146,33 @@ int eth_rebuild_header(struct sk_buff *skb)
>> EXPORT_SYMBOL(eth_rebuild_header);
>>
>> /**
>> + * eth_get_headlen - determine the the length of header for an ethernet frame
>> + * @data: pointer to start of frame
>> + * @len: total length of frame
>> + *
>> + * Make a best effort attempt to pull the length for all of the headers for
>> + * a given frame in a linear buffer.
>> + */
>> +u32 eth_get_headlen(void *data, unsigned int len)
>> +{
>> + const struct ethhdr *eth = (const struct ethhdr *)data;
>> + struct flow_keys keys;
>> +
>> + /* this should never happen, but better safe than sorry */
>> + if (len < sizeof(*eth))
>> + return len;
>> +
>> + /* parse any remaining L2/L3 headers, check for L4 */
>> + if (!__skb_flow_dissect(NULL, &keys, data,
>> + eth->h_proto, sizeof(*eth), len))
>> + return max_t(u32, keys.thoff, sizeof(*eth));
> Not sure keys.thoff is valid at this point ?
>
It should be 0 if it didn't find any value. __skb_flow_dissect starts
by doing a memset 0 on the keys. So I am using the max_t to force it to
at least include the Ethernet header in the length to pull as otherwise
we will trigger errors on eth_type_trans.
Thanks,
Alex
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-09-05 1:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-04 23:13 [RFC PATCH 0/3] Drop get_headlen functions in favor of generic function Alexander Duyck
2014-09-04 23:13 ` [RFC PATCH 1/3] net: Add function for parsing the header length out of linear ethernet frames Alexander Duyck
2014-09-04 23:36 ` Eric Dumazet
2014-09-05 1:00 ` Alexander Duyck
2014-09-04 23:13 ` [RFC PATCH 2/3] igb: use new eth_get_headlen interface Alexander Duyck
2014-09-04 23:13 ` [RFC PATCH 3/3] ixgbe: " Alexander Duyck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).