Paul Durrant
2013-Dec-02 17:35 UTC
[PATCH net v4] xen-netback: fix fragment detection in checksum setup
The code to detect fragments in checksum_setup() was missing for IPv4 and
too eager for IPv6. (It transpires that Windows seems to send IPv6 packets
with a fragment header even if they are not a fragment - i.e. offset is zero,
and M bit is not set).
This patch also incorporates a fix to callers of maybe_pull_tail() where
skb->network_header was being erroneously added to the length argument.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
cc: David Miller <davem@davemloft.net>
---
v4
- Re-work maybe_pull_tail() to have a failure path and update mac and
network header pointers on success.
- Re-work callers of maybe_pull_tail() to handle failures and allow for
movement of skb header pointers.
- Incorporate fixes from patch "Fix pull size in checksum_setup_ip"
since
it also modifies callers of maybe_pull_tail().
- Remove error messages in checksum routines that could be triggered by
frontends
v3
- Use defined values for ''fragment offset'' and ''more
fragments''
v2
- Added comments noting what fragment/offset masks mean
drivers/net/xen-netback/netback.c | 215 ++++++++++++++++++++-----------------
include/linux/ipv6.h | 1 +
include/net/ipv6.h | 3 +-
3 files changed, 120 insertions(+), 99 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 64f0e0d..e3d7216 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1149,49 +1149,62 @@ static int xenvif_set_skb_gso(struct xenvif *vif,
return 0;
}
-static inline void maybe_pull_tail(struct sk_buff *skb, unsigned int len)
+static inline bool maybe_pull_tail(struct sk_buff *skb, unsigned int len,
+ unsigned int max)
{
- if (skb_is_nonlinear(skb) && skb_headlen(skb) < len) {
- /* If we need to pullup then pullup to the max, so we
- * won''t need to do it again.
- */
- int target = min_t(int, skb->len, MAX_TCP_HEADER);
- __pskb_pull_tail(skb, target - skb_headlen(skb));
- }
+ if (skb_headlen(skb) >= len)
+ return true;
+
+ /* If we need to pullup then pullup to the max, so we
+ * won''t need to do it again.
+ */
+ if (max > skb->len)
+ max = skb->len;
+
+ __pskb_pull_tail(skb, max - skb_headlen(skb));
+
+ return skb_headlen(skb) >= len;
}
+/* This value should be large enough to cover a tagged ethernet header plus
+ * maximally sized IP and TCP or UDP headers.
+ */
+#define MAX_IP_HDR_LEN 128
+
static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
int recalculate_partial_csum)
{
- struct iphdr *iph = (void *)skb->data;
- unsigned int header_size;
unsigned int off;
+ bool fragment;
int err = -EPROTO;
- off = sizeof(struct iphdr);
+ fragment = false;
- header_size = skb->network_header + off + MAX_IPOPTLEN;
- maybe_pull_tail(skb, header_size);
+ if (!maybe_pull_tail(skb, sizeof(struct iphdr), MAX_IP_HDR_LEN))
+ goto out;
+
+ if (ip_hdr(skb)->frag_off & htons(IP_OFFSET | IP_MF))
+ fragment = true;
- off = iph->ihl * 4;
+ off = ip_hdrlen(skb);
- switch (iph->protocol) {
+ switch (ip_hdr(skb)->protocol) {
case IPPROTO_TCP:
if (!skb_partial_csum_set(skb, off,
offsetof(struct tcphdr, check)))
goto out;
if (recalculate_partial_csum) {
- struct tcphdr *tcph = tcp_hdr(skb);
-
- header_size = skb->network_header +
- off +
- sizeof(struct tcphdr);
- maybe_pull_tail(skb, header_size);
-
- tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
- skb->len - off,
- IPPROTO_TCP, 0);
+ if (!maybe_pull_tail(skb,
+ off + sizeof(struct tcphdr),
+ MAX_IP_HDR_LEN))
+ goto out;
+
+ tcp_hdr(skb)->check + ~csum_tcpudp_magic(ip_hdr(skb)->saddr,
+ ip_hdr(skb)->daddr,
+ skb->len - off,
+ IPPROTO_TCP, 0);
}
break;
case IPPROTO_UDP:
@@ -1200,24 +1213,19 @@ static int checksum_setup_ip(struct xenvif *vif, struct
sk_buff *skb,
goto out;
if (recalculate_partial_csum) {
- struct udphdr *udph = udp_hdr(skb);
-
- header_size = skb->network_header +
- off +
- sizeof(struct udphdr);
- maybe_pull_tail(skb, header_size);
-
- udph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
- skb->len - off,
- IPPROTO_UDP, 0);
+ if (!maybe_pull_tail(skb,
+ off + sizeof(struct udphdr),
+ MAX_IP_HDR_LEN))
+ goto out;
+
+ udp_hdr(skb)->check + ~csum_tcpudp_magic(ip_hdr(skb)->saddr,
+ ip_hdr(skb)->daddr,
+ skb->len - off,
+ IPPROTO_UDP, 0);
}
break;
default:
- if (net_ratelimit())
- netdev_err(vif->dev,
- "Attempting to checksum a non-TCP/UDP packet, "
- "dropping a protocol %d packet\n",
- iph->protocol);
goto out;
}
@@ -1227,75 +1235,93 @@ out:
return err;
}
+/* This value should be large enough to cover a tagged ethernet header plus
+ * an IPv6 header, all options, and a maximal TCP or UDP header.
+ */
+#define MAX_IPV6_HDR_LEN 256
+
+#define OPT_HDR(type, skb, off) \
+ (type *)(skb_network_header(skb) + (off))
+
static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
int recalculate_partial_csum)
{
int err = -EPROTO;
- struct ipv6hdr *ipv6h = (void *)skb->data;
u8 nexthdr;
- unsigned int header_size;
unsigned int off;
+ unsigned int len;
bool fragment;
bool done;
+ fragment = false;
done = false;
off = sizeof(struct ipv6hdr);
- header_size = skb->network_header + off;
- maybe_pull_tail(skb, header_size);
+ if (!maybe_pull_tail(skb, off, MAX_IPV6_HDR_LEN))
+ goto out;
- nexthdr = ipv6h->nexthdr;
+ nexthdr = ipv6_hdr(skb)->nexthdr;
- while ((off <= sizeof(struct ipv6hdr) + ntohs(ipv6h->payload_len))
&&
- !done) {
+ len = sizeof(struct ipv6hdr) + ntohs(ipv6_hdr(skb)->payload_len);
+ while (off <= len && !done) {
switch (nexthdr) {
case IPPROTO_DSTOPTS:
case IPPROTO_HOPOPTS:
case IPPROTO_ROUTING: {
- struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
+ struct ipv6_opt_hdr *hp;
- header_size = skb->network_header +
- off +
- sizeof(struct ipv6_opt_hdr);
- maybe_pull_tail(skb, header_size);
+ if (!maybe_pull_tail(skb,
+ off +
+ sizeof(struct ipv6_opt_hdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+ hp = OPT_HDR(struct ipv6_opt_hdr, skb, off);
nexthdr = hp->nexthdr;
off += ipv6_optlen(hp);
break;
}
case IPPROTO_AH: {
- struct ip_auth_hdr *hp = (void *)(skb->data + off);
+ struct ip_auth_hdr *hp;
+
+ if (!maybe_pull_tail(skb,
+ off +
+ sizeof(struct ip_auth_hdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+
+ hp = OPT_HDR(struct ip_auth_hdr, skb, off);
+ nexthdr = hp->nexthdr;
+ off += ipv6_authlen(hp);
+ break;
+ }
+ case IPPROTO_FRAGMENT: {
+ struct frag_hdr *hp;
+
+ if (!maybe_pull_tail(skb,
+ off +
+ sizeof(struct frag_hdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+
+ hp = OPT_HDR(struct frag_hdr, skb, off);
- header_size = skb->network_header +
- off +
- sizeof(struct ip_auth_hdr);
- maybe_pull_tail(skb, header_size);
+ if (hp->frag_off & htons(IP6_OFFSET | IP6_MF))
+ fragment = true;
nexthdr = hp->nexthdr;
- off += (hp->hdrlen+2)<<2;
+ off += sizeof(struct frag_hdr);
break;
}
- case IPPROTO_FRAGMENT:
- fragment = true;
- /* fall through */
default:
done = true;
break;
}
}
- if (!done) {
- if (net_ratelimit())
- netdev_err(vif->dev, "Failed to parse packet header\n");
+ if (!done || fragment)
goto out;
- }
-
- if (fragment) {
- if (net_ratelimit())
- netdev_err(vif->dev, "Packet is a fragment!\n");
- goto out;
- }
switch (nexthdr) {
case IPPROTO_TCP:
@@ -1304,17 +1330,16 @@ static int checksum_setup_ipv6(struct xenvif *vif,
struct sk_buff *skb,
goto out;
if (recalculate_partial_csum) {
- struct tcphdr *tcph = tcp_hdr(skb);
-
- header_size = skb->network_header +
- off +
- sizeof(struct tcphdr);
- maybe_pull_tail(skb, header_size);
-
- tcph->check = ~csum_ipv6_magic(&ipv6h->saddr,
- &ipv6h->daddr,
- skb->len - off,
- IPPROTO_TCP, 0);
+ if (!maybe_pull_tail(skb,
+ off + sizeof(struct tcphdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+
+ tcp_hdr(skb)->check + ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+ &ipv6_hdr(skb)->daddr,
+ skb->len - off,
+ IPPROTO_TCP, 0);
}
break;
case IPPROTO_UDP:
@@ -1323,25 +1348,19 @@ static int checksum_setup_ipv6(struct xenvif *vif,
struct sk_buff *skb,
goto out;
if (recalculate_partial_csum) {
- struct udphdr *udph = udp_hdr(skb);
-
- header_size = skb->network_header +
- off +
- sizeof(struct udphdr);
- maybe_pull_tail(skb, header_size);
-
- udph->check = ~csum_ipv6_magic(&ipv6h->saddr,
- &ipv6h->daddr,
- skb->len - off,
- IPPROTO_UDP, 0);
+ if (!maybe_pull_tail(skb,
+ off + sizeof(struct udphdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+
+ udp_hdr(skb)->check + ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+ &ipv6_hdr(skb)->daddr,
+ skb->len - off,
+ IPPROTO_UDP, 0);
}
break;
default:
- if (net_ratelimit())
- netdev_err(vif->dev,
- "Attempting to checksum a non-TCP/UDP packet, "
- "dropping a protocol %d packet\n",
- nexthdr);
goto out;
}
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 5d89d1b..c56c350 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -4,6 +4,7 @@
#include <uapi/linux/ipv6.h>
#define ipv6_optlen(p) (((p)->hdrlen+1) << 3)
+#define ipv6_authlen(p) (((p)->hdrlen+2) << 2)
/*
* This structure contains configuration options per IPv6 link.
*/
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index eb198ac..488316e 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -110,7 +110,8 @@ struct frag_hdr {
__be32 identification;
};
-#define IP6_MF 0x0001
+#define IP6_MF 0x0001
+#define IP6_OFFSET 0xFFF8
#include <net/sock.h>
--
1.7.10.4
Paul Durrant
2013-Dec-02 17:41 UTC
[PATCH net v4] xen-netback: fix fragment detection in checksum setup
The code to detect fragments in checksum_setup() was missing for IPv4 and
too eager for IPv6. (It transpires that Windows seems to send IPv6 packets
with a fragment header even if they are not a fragment - i.e. offset is zero,
and M bit is not set).
This patch also incorporates a fix to callers of maybe_pull_tail() where
skb->network_header was being erroneously added to the length argument.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
cc: David Miller <davem@davemloft.net>
---
v4
- Re-work maybe_pull_tail() to have a failure path and update mac and
network header pointers on success.
- Re-work callers of maybe_pull_tail() to handle failures and allow for
movement of skb header pointers.
- Incorporate fixes from patch "Fix pull size in checksum_setup_ip"
since
it also modifies callers of maybe_pull_tail().
- Remove error messages in checksum routines that could be triggered by
frontends
v3
- Use defined values for ''fragment offset'' and ''more
fragments''
v2
- Added comments noting what fragment/offset masks mean
drivers/net/xen-netback/netback.c | 215 ++++++++++++++++++++-----------------
include/linux/ipv6.h | 1 +
include/net/ipv6.h | 3 +-
3 files changed, 120 insertions(+), 99 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c
b/drivers/net/xen-netback/netback.c
index 64f0e0d..e3d7216 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -1149,49 +1149,62 @@ static int xenvif_set_skb_gso(struct xenvif *vif,
return 0;
}
-static inline void maybe_pull_tail(struct sk_buff *skb, unsigned int len)
+static inline bool maybe_pull_tail(struct sk_buff *skb, unsigned int len,
+ unsigned int max)
{
- if (skb_is_nonlinear(skb) && skb_headlen(skb) < len) {
- /* If we need to pullup then pullup to the max, so we
- * won''t need to do it again.
- */
- int target = min_t(int, skb->len, MAX_TCP_HEADER);
- __pskb_pull_tail(skb, target - skb_headlen(skb));
- }
+ if (skb_headlen(skb) >= len)
+ return true;
+
+ /* If we need to pullup then pullup to the max, so we
+ * won''t need to do it again.
+ */
+ if (max > skb->len)
+ max = skb->len;
+
+ __pskb_pull_tail(skb, max - skb_headlen(skb));
+
+ return skb_headlen(skb) >= len;
}
+/* This value should be large enough to cover a tagged ethernet header plus
+ * maximally sized IP and TCP or UDP headers.
+ */
+#define MAX_IP_HDR_LEN 128
+
static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb,
int recalculate_partial_csum)
{
- struct iphdr *iph = (void *)skb->data;
- unsigned int header_size;
unsigned int off;
+ bool fragment;
int err = -EPROTO;
- off = sizeof(struct iphdr);
+ fragment = false;
- header_size = skb->network_header + off + MAX_IPOPTLEN;
- maybe_pull_tail(skb, header_size);
+ if (!maybe_pull_tail(skb, sizeof(struct iphdr), MAX_IP_HDR_LEN))
+ goto out;
+
+ if (ip_hdr(skb)->frag_off & htons(IP_OFFSET | IP_MF))
+ fragment = true;
- off = iph->ihl * 4;
+ off = ip_hdrlen(skb);
- switch (iph->protocol) {
+ switch (ip_hdr(skb)->protocol) {
case IPPROTO_TCP:
if (!skb_partial_csum_set(skb, off,
offsetof(struct tcphdr, check)))
goto out;
if (recalculate_partial_csum) {
- struct tcphdr *tcph = tcp_hdr(skb);
-
- header_size = skb->network_header +
- off +
- sizeof(struct tcphdr);
- maybe_pull_tail(skb, header_size);
-
- tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
- skb->len - off,
- IPPROTO_TCP, 0);
+ if (!maybe_pull_tail(skb,
+ off + sizeof(struct tcphdr),
+ MAX_IP_HDR_LEN))
+ goto out;
+
+ tcp_hdr(skb)->check + ~csum_tcpudp_magic(ip_hdr(skb)->saddr,
+ ip_hdr(skb)->daddr,
+ skb->len - off,
+ IPPROTO_TCP, 0);
}
break;
case IPPROTO_UDP:
@@ -1200,24 +1213,19 @@ static int checksum_setup_ip(struct xenvif *vif, struct
sk_buff *skb,
goto out;
if (recalculate_partial_csum) {
- struct udphdr *udph = udp_hdr(skb);
-
- header_size = skb->network_header +
- off +
- sizeof(struct udphdr);
- maybe_pull_tail(skb, header_size);
-
- udph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
- skb->len - off,
- IPPROTO_UDP, 0);
+ if (!maybe_pull_tail(skb,
+ off + sizeof(struct udphdr),
+ MAX_IP_HDR_LEN))
+ goto out;
+
+ udp_hdr(skb)->check + ~csum_tcpudp_magic(ip_hdr(skb)->saddr,
+ ip_hdr(skb)->daddr,
+ skb->len - off,
+ IPPROTO_UDP, 0);
}
break;
default:
- if (net_ratelimit())
- netdev_err(vif->dev,
- "Attempting to checksum a non-TCP/UDP packet, "
- "dropping a protocol %d packet\n",
- iph->protocol);
goto out;
}
@@ -1227,75 +1235,93 @@ out:
return err;
}
+/* This value should be large enough to cover a tagged ethernet header plus
+ * an IPv6 header, all options, and a maximal TCP or UDP header.
+ */
+#define MAX_IPV6_HDR_LEN 256
+
+#define OPT_HDR(type, skb, off) \
+ (type *)(skb_network_header(skb) + (off))
+
static int checksum_setup_ipv6(struct xenvif *vif, struct sk_buff *skb,
int recalculate_partial_csum)
{
int err = -EPROTO;
- struct ipv6hdr *ipv6h = (void *)skb->data;
u8 nexthdr;
- unsigned int header_size;
unsigned int off;
+ unsigned int len;
bool fragment;
bool done;
+ fragment = false;
done = false;
off = sizeof(struct ipv6hdr);
- header_size = skb->network_header + off;
- maybe_pull_tail(skb, header_size);
+ if (!maybe_pull_tail(skb, off, MAX_IPV6_HDR_LEN))
+ goto out;
- nexthdr = ipv6h->nexthdr;
+ nexthdr = ipv6_hdr(skb)->nexthdr;
- while ((off <= sizeof(struct ipv6hdr) + ntohs(ipv6h->payload_len))
&&
- !done) {
+ len = sizeof(struct ipv6hdr) + ntohs(ipv6_hdr(skb)->payload_len);
+ while (off <= len && !done) {
switch (nexthdr) {
case IPPROTO_DSTOPTS:
case IPPROTO_HOPOPTS:
case IPPROTO_ROUTING: {
- struct ipv6_opt_hdr *hp = (void *)(skb->data + off);
+ struct ipv6_opt_hdr *hp;
- header_size = skb->network_header +
- off +
- sizeof(struct ipv6_opt_hdr);
- maybe_pull_tail(skb, header_size);
+ if (!maybe_pull_tail(skb,
+ off +
+ sizeof(struct ipv6_opt_hdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+ hp = OPT_HDR(struct ipv6_opt_hdr, skb, off);
nexthdr = hp->nexthdr;
off += ipv6_optlen(hp);
break;
}
case IPPROTO_AH: {
- struct ip_auth_hdr *hp = (void *)(skb->data + off);
+ struct ip_auth_hdr *hp;
+
+ if (!maybe_pull_tail(skb,
+ off +
+ sizeof(struct ip_auth_hdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+
+ hp = OPT_HDR(struct ip_auth_hdr, skb, off);
+ nexthdr = hp->nexthdr;
+ off += ipv6_authlen(hp);
+ break;
+ }
+ case IPPROTO_FRAGMENT: {
+ struct frag_hdr *hp;
+
+ if (!maybe_pull_tail(skb,
+ off +
+ sizeof(struct frag_hdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+
+ hp = OPT_HDR(struct frag_hdr, skb, off);
- header_size = skb->network_header +
- off +
- sizeof(struct ip_auth_hdr);
- maybe_pull_tail(skb, header_size);
+ if (hp->frag_off & htons(IP6_OFFSET | IP6_MF))
+ fragment = true;
nexthdr = hp->nexthdr;
- off += (hp->hdrlen+2)<<2;
+ off += sizeof(struct frag_hdr);
break;
}
- case IPPROTO_FRAGMENT:
- fragment = true;
- /* fall through */
default:
done = true;
break;
}
}
- if (!done) {
- if (net_ratelimit())
- netdev_err(vif->dev, "Failed to parse packet header\n");
+ if (!done || fragment)
goto out;
- }
-
- if (fragment) {
- if (net_ratelimit())
- netdev_err(vif->dev, "Packet is a fragment!\n");
- goto out;
- }
switch (nexthdr) {
case IPPROTO_TCP:
@@ -1304,17 +1330,16 @@ static int checksum_setup_ipv6(struct xenvif *vif,
struct sk_buff *skb,
goto out;
if (recalculate_partial_csum) {
- struct tcphdr *tcph = tcp_hdr(skb);
-
- header_size = skb->network_header +
- off +
- sizeof(struct tcphdr);
- maybe_pull_tail(skb, header_size);
-
- tcph->check = ~csum_ipv6_magic(&ipv6h->saddr,
- &ipv6h->daddr,
- skb->len - off,
- IPPROTO_TCP, 0);
+ if (!maybe_pull_tail(skb,
+ off + sizeof(struct tcphdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+
+ tcp_hdr(skb)->check + ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+ &ipv6_hdr(skb)->daddr,
+ skb->len - off,
+ IPPROTO_TCP, 0);
}
break;
case IPPROTO_UDP:
@@ -1323,25 +1348,19 @@ static int checksum_setup_ipv6(struct xenvif *vif,
struct sk_buff *skb,
goto out;
if (recalculate_partial_csum) {
- struct udphdr *udph = udp_hdr(skb);
-
- header_size = skb->network_header +
- off +
- sizeof(struct udphdr);
- maybe_pull_tail(skb, header_size);
-
- udph->check = ~csum_ipv6_magic(&ipv6h->saddr,
- &ipv6h->daddr,
- skb->len - off,
- IPPROTO_UDP, 0);
+ if (!maybe_pull_tail(skb,
+ off + sizeof(struct udphdr),
+ MAX_IPV6_HDR_LEN))
+ goto out;
+
+ udp_hdr(skb)->check + ~csum_ipv6_magic(&ipv6_hdr(skb)->saddr,
+ &ipv6_hdr(skb)->daddr,
+ skb->len - off,
+ IPPROTO_UDP, 0);
}
break;
default:
- if (net_ratelimit())
- netdev_err(vif->dev,
- "Attempting to checksum a non-TCP/UDP packet, "
- "dropping a protocol %d packet\n",
- nexthdr);
goto out;
}
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 5d89d1b..c56c350 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -4,6 +4,7 @@
#include <uapi/linux/ipv6.h>
#define ipv6_optlen(p) (((p)->hdrlen+1) << 3)
+#define ipv6_authlen(p) (((p)->hdrlen+2) << 2)
/*
* This structure contains configuration options per IPv6 link.
*/
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index eb198ac..488316e 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -110,7 +110,8 @@ struct frag_hdr {
__be32 identification;
};
-#define IP6_MF 0x0001
+#define IP6_MF 0x0001
+#define IP6_OFFSET 0xFFF8
#include <net/sock.h>
--
1.7.10.4
David Miller
2013-Dec-02 18:46 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
You sent two copies of this patch, is there a difference between them?
Paul Durrant
2013-Dec-02 20:13 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
> -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev- > owner@vger.kernel.org] On Behalf Of David Miller > Sent: 02 December 2013 18:46 > To: Paul Durrant > Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan Kiss; Wei Liu; > Ian Campbell; David Vrabel > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in checksum > setup > > > You sent two copies of this patch, is there a difference between > them?Sorry. The SMTP server was playing up. The two copies are identical. Paul
David Miller
2013-Dec-02 22:25 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
From: Paul Durrant <paul.durrant@citrix.com> Date: Mon, 2 Dec 2013 17:35:15 +0000> The code to detect fragments in checksum_setup() was missing for IPv4 and > too eager for IPv6. (It transpires that Windows seems to send IPv6 packets > with a fragment header even if they are not a fragment - i.e. offset is zero, > and M bit is not set). > This patch also incorporates a fix to callers of maybe_pull_tail() where > skb->network_header was being erroneously added to the length argument. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: David Vrabel <david.vrabel@citrix.com> > cc: David Miller <davem@davemloft.net>This addresses my concerned, Ian and Wei please review.
Wei Liu
2013-Dec-03 14:02 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
On Mon, Dec 02, 2013 at 05:35:15PM +0000, Paul Durrant wrote:> The code to detect fragments in checksum_setup() was missing for IPv4 and > too eager for IPv6. (It transpires that Windows seems to send IPv6 packets > with a fragment header even if they are not a fragment - i.e. offset is zero, > and M bit is not set). > This patch also incorporates a fix to callers of maybe_pull_tail() where > skb->network_header was being erroneously added to the length argument. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: David Vrabel <david.vrabel@citrix.com> > cc: David Miller <davem@davemloft.net> > --- > v4 > - Re-work maybe_pull_tail() to have a failure path and update mac and > network header pointers on success. > - Re-work callers of maybe_pull_tail() to handle failures and allow for > movement of skb header pointers. > - Incorporate fixes from patch "Fix pull size in checksum_setup_ip" since > it also modifies callers of maybe_pull_tail(). > - Remove error messages in checksum routines that could be triggered by frontends > > v3 > - Use defined values for ''fragment offset'' and ''more fragments'' > > v2 > - Added comments noting what fragment/offset masks mean > > drivers/net/xen-netback/netback.c | 215 ++++++++++++++++++++----------------- > include/linux/ipv6.h | 1 + > include/net/ipv6.h | 3 +- > 3 files changed, 120 insertions(+), 99 deletions(-) > > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c > index 64f0e0d..e3d7216 100644 > --- a/drivers/net/xen-netback/netback.c > +++ b/drivers/net/xen-netback/netback.c > @@ -1149,49 +1149,62 @@ static int xenvif_set_skb_gso(struct xenvif *vif, > return 0; > } > > -static inline void maybe_pull_tail(struct sk_buff *skb, unsigned int len) > +static inline bool maybe_pull_tail(struct sk_buff *skb, unsigned int len, > + unsigned int max) > { > - if (skb_is_nonlinear(skb) && skb_headlen(skb) < len) { > - /* If we need to pullup then pullup to the max, so we > - * won''t need to do it again. > - */ > - int target = min_t(int, skb->len, MAX_TCP_HEADER); > - __pskb_pull_tail(skb, target - skb_headlen(skb)); > - } > + if (skb_headlen(skb) >= len) > + return true; > + > + /* If we need to pullup then pullup to the max, so we > + * won''t need to do it again. > + */ > + if (max > skb->len) > + max = skb->len; > + > + __pskb_pull_tail(skb, max - skb_headlen(skb)); > + > + return skb_headlen(skb) >= len; > } > > +/* This value should be large enough to cover a tagged ethernet header plus > + * maximally sized IP and TCP or UDP headers. > + */ > +#define MAX_IP_HDR_LEN 128 > + > static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb, > int recalculate_partial_csum) > { > - struct iphdr *iph = (void *)skb->data; > - unsigned int header_size; > unsigned int off; > + bool fragment; > int err = -EPROTO; > > - off = sizeof(struct iphdr); > + fragment = false; > > - header_size = skb->network_header + off + MAX_IPOPTLEN; > - maybe_pull_tail(skb, header_size); > + if (!maybe_pull_tail(skb, sizeof(struct iphdr), MAX_IP_HDR_LEN)) > + goto out; > +I think you need to correctly update err to reflect this failure. Using -EPROTO will wrongly blame frontend while it is backend that''s failing to process the packet. Wei.
Paul Durrant
2013-Dec-03 14:05 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 03 December 2013 14:03 > To: Paul Durrant > Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan Kiss; Wei Liu; > Ian Campbell; David Vrabel; David Miller > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in checksum > setup > > On Mon, Dec 02, 2013 at 05:35:15PM +0000, Paul Durrant wrote: > > The code to detect fragments in checksum_setup() was missing for IPv4 > and > > too eager for IPv6. (It transpires that Windows seems to send IPv6 packets > > with a fragment header even if they are not a fragment - i.e. offset is zero, > > and M bit is not set). > > This patch also incorporates a fix to callers of maybe_pull_tail() where > > skb->network_header was being erroneously added to the length > argument. > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > > Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com> > > Cc: Wei Liu <wei.liu2@citrix.com> > > Cc: Ian Campbell <ian.campbell@citrix.com> > > Cc: David Vrabel <david.vrabel@citrix.com> > > cc: David Miller <davem@davemloft.net> > > --- > > v4 > > - Re-work maybe_pull_tail() to have a failure path and update mac and > > network header pointers on success. > > - Re-work callers of maybe_pull_tail() to handle failures and allow for > > movement of skb header pointers. > > - Incorporate fixes from patch "Fix pull size in checksum_setup_ip" since > > it also modifies callers of maybe_pull_tail(). > > - Remove error messages in checksum routines that could be triggered by > frontends > > > > v3 > > - Use defined values for ''fragment offset'' and ''more fragments'' > > > > v2 > > - Added comments noting what fragment/offset masks mean > > > > drivers/net/xen-netback/netback.c | 215 ++++++++++++++++++++------- > ---------- > > include/linux/ipv6.h | 1 + > > include/net/ipv6.h | 3 +- > > 3 files changed, 120 insertions(+), 99 deletions(-) > > > > diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen- > netback/netback.c > > index 64f0e0d..e3d7216 100644 > > --- a/drivers/net/xen-netback/netback.c > > +++ b/drivers/net/xen-netback/netback.c > > @@ -1149,49 +1149,62 @@ static int xenvif_set_skb_gso(struct xenvif *vif, > > return 0; > > } > > > > -static inline void maybe_pull_tail(struct sk_buff *skb, unsigned int len) > > +static inline bool maybe_pull_tail(struct sk_buff *skb, unsigned int len, > > + unsigned int max) > > { > > - if (skb_is_nonlinear(skb) && skb_headlen(skb) < len) { > > - /* If we need to pullup then pullup to the max, so we > > - * won''t need to do it again. > > - */ > > - int target = min_t(int, skb->len, MAX_TCP_HEADER); > > - __pskb_pull_tail(skb, target - skb_headlen(skb)); > > - } > > + if (skb_headlen(skb) >= len) > > + return true; > > + > > + /* If we need to pullup then pullup to the max, so we > > + * won''t need to do it again. > > + */ > > + if (max > skb->len) > > + max = skb->len; > > + > > + __pskb_pull_tail(skb, max - skb_headlen(skb)); > > + > > + return skb_headlen(skb) >= len; > > } > > > > +/* This value should be large enough to cover a tagged ethernet header > plus > > + * maximally sized IP and TCP or UDP headers. > > + */ > > +#define MAX_IP_HDR_LEN 128 > > + > > static int checksum_setup_ip(struct xenvif *vif, struct sk_buff *skb, > > int recalculate_partial_csum) > > { > > - struct iphdr *iph = (void *)skb->data; > > - unsigned int header_size; > > unsigned int off; > > + bool fragment; > > int err = -EPROTO; > > > > - off = sizeof(struct iphdr); > > + fragment = false; > > > > - header_size = skb->network_header + off + MAX_IPOPTLEN; > > - maybe_pull_tail(skb, header_size); > > + if (!maybe_pull_tail(skb, sizeof(struct iphdr), MAX_IP_HDR_LEN)) > > + goto out; > > + > > I think you need to correctly update err to reflect this failure. > Using -EPROTO will wrongly blame frontend while it is backend that''s > failing to process the packet. >But a failure should only occur if the packet is malformed, so that would be a frontend error wouldn''t it? Paul
Wei Liu
2013-Dec-03 14:28 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
On Tue, Dec 03, 2013 at 02:05:17PM +0000, Paul Durrant wrote: [...]> > > > > > - header_size = skb->network_header + off + MAX_IPOPTLEN; > > > - maybe_pull_tail(skb, header_size); > > > + if (!maybe_pull_tail(skb, sizeof(struct iphdr), MAX_IP_HDR_LEN)) > > > + goto out; > > > + > > > > I think you need to correctly update err to reflect this failure. > > Using -EPROTO will wrongly blame frontend while it is backend that''s > > failing to process the packet. > > > > But a failure should only occur if the packet is malformed, so that would be a frontend error wouldn''t it? >__pskb_pull_tail may fail due to malloc failure. However the return value of __pskb_pull_tail cannot reflect the wether the failure is due to malformed packet or OOM. Not sure what''s the best solution here. What''s the malformed packet you were talking about? Wei.> Paul
Paul Durrant
2013-Dec-03 14:34 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 03 December 2013 14:29 > To: Paul Durrant > Cc: Wei Liu; xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan Kiss; > Ian Campbell; David Vrabel; David Miller > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in checksum > setup > > On Tue, Dec 03, 2013 at 02:05:17PM +0000, Paul Durrant wrote: > [...] > > > > > > > > - header_size = skb->network_header + off + MAX_IPOPTLEN; > > > > - maybe_pull_tail(skb, header_size); > > > > + if (!maybe_pull_tail(skb, sizeof(struct iphdr), MAX_IP_HDR_LEN)) > > > > + goto out; > > > > + > > > > > > I think you need to correctly update err to reflect this failure. > > > Using -EPROTO will wrongly blame frontend while it is backend that''s > > > failing to process the packet. > > > > > > > But a failure should only occur if the packet is malformed, so that would be > a frontend error wouldn''t it? > > > > __pskb_pull_tail may fail due to malloc failure. > > However the return value of __pskb_pull_tail cannot reflect the wether > the failure is due to malformed packet or OOM. Not sure what''s the best > solution here. What''s the malformed packet you were talking about? >For example, the pull would fail if the packet had an either_type of IP but didn''t contain an IP header, or perhaps an IPv6 packet that had an incomplete option header sequence. I would have thought such a packet was a more likely cause of failure than OOM, so -EPROTO seems a reasonable best guess. Paul
Wei Liu
2013-Dec-03 14:57 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
On Tue, Dec 03, 2013 at 02:34:56PM +0000, Paul Durrant wrote:> > -----Original Message----- > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > Sent: 03 December 2013 14:29 > > To: Paul Durrant > > Cc: Wei Liu; xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan Kiss; > > Ian Campbell; David Vrabel; David Miller > > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in checksum > > setup > > > > On Tue, Dec 03, 2013 at 02:05:17PM +0000, Paul Durrant wrote: > > [...] > > > > > > > > > > - header_size = skb->network_header + off + MAX_IPOPTLEN; > > > > > - maybe_pull_tail(skb, header_size); > > > > > + if (!maybe_pull_tail(skb, sizeof(struct iphdr), MAX_IP_HDR_LEN)) > > > > > + goto out; > > > > > + > > > > > > > > I think you need to correctly update err to reflect this failure. > > > > Using -EPROTO will wrongly blame frontend while it is backend that''s > > > > failing to process the packet. > > > > > > > > > > But a failure should only occur if the packet is malformed, so that would be > > a frontend error wouldn''t it? > > > > > > > __pskb_pull_tail may fail due to malloc failure. > > > > However the return value of __pskb_pull_tail cannot reflect the wether > > the failure is due to malformed packet or OOM. Not sure what''s the best > > solution here. What''s the malformed packet you were talking about? > > > > For example, the pull would fail if the packet had an either_type of > IP but didn''t contain an IP header, or perhaps an IPv6 packet that had > an incomplete option header sequence. I would have thought such a > packet was a more likely cause of failure than OOM, so -EPROTO seems a > reasonable best guess.How? __pskb_pull_tail doesn''t seem to care about upper layer protocols. And maybe_pull_tail has already done some lenght comparisions. Wei.> > Paul
Paul Durrant
2013-Dec-03 15:05 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 03 December 2013 14:58 > To: Paul Durrant > Cc: Wei Liu; xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan Kiss; > Ian Campbell; David Vrabel; David Miller > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in checksum > setup > > On Tue, Dec 03, 2013 at 02:34:56PM +0000, Paul Durrant wrote: > > > -----Original Message----- > > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > > Sent: 03 December 2013 14:29 > > > To: Paul Durrant > > > Cc: Wei Liu; xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan > Kiss; > > > Ian Campbell; David Vrabel; David Miller > > > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in > checksum > > > setup > > > > > > On Tue, Dec 03, 2013 at 02:05:17PM +0000, Paul Durrant wrote: > > > [...] > > > > > > > > > > > > - header_size = skb->network_header + off + > MAX_IPOPTLEN; > > > > > > - maybe_pull_tail(skb, header_size); > > > > > > + if (!maybe_pull_tail(skb, sizeof(struct iphdr), > MAX_IP_HDR_LEN)) > > > > > > + goto out; > > > > > > + > > > > > > > > > > I think you need to correctly update err to reflect this failure. > > > > > Using -EPROTO will wrongly blame frontend while it is backend that''s > > > > > failing to process the packet. > > > > > > > > > > > > > But a failure should only occur if the packet is malformed, so that would > be > > > a frontend error wouldn''t it? > > > > > > > > > > __pskb_pull_tail may fail due to malloc failure. > > > > > > However the return value of __pskb_pull_tail cannot reflect the wether > > > the failure is due to malformed packet or OOM. Not sure what''s the best > > > solution here. What''s the malformed packet you were talking about? > > > > > > > For example, the pull would fail if the packet had an either_type of > > IP but didn''t contain an IP header, or perhaps an IPv6 packet that had > > an incomplete option header sequence. I would have thought such a > > packet was a more likely cause of failure than OOM, so -EPROTO seems a > > reasonable best guess. > > How? __pskb_pull_tail doesn''t seem to care about upper layer protocols. > And maybe_pull_tail has already done some lenght comparisions. >No, __pskb_pull_tail() doesn''t care but the final check in maybe_pull_tail() means it will return false if skb_headlen() is not at least as big as what it was asked for. So if we try to pull up an IP header and there''s fewer bytes than that available then we hit the error condition. Or maybe I''m missing something. Paul
Wei Liu
2013-Dec-03 15:13 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
On Tue, Dec 03, 2013 at 03:05:40PM +0000, Paul Durrant wrote:> > -----Original Message----- > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > Sent: 03 December 2013 14:58 > > To: Paul Durrant > > Cc: Wei Liu; xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan Kiss; > > Ian Campbell; David Vrabel; David Miller > > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in checksum > > setup > > > > On Tue, Dec 03, 2013 at 02:34:56PM +0000, Paul Durrant wrote: > > > > -----Original Message----- > > > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > > > Sent: 03 December 2013 14:29 > > > > To: Paul Durrant > > > > Cc: Wei Liu; xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan > > Kiss; > > > > Ian Campbell; David Vrabel; David Miller > > > > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in > > checksum > > > > setup > > > > > > > > On Tue, Dec 03, 2013 at 02:05:17PM +0000, Paul Durrant wrote: > > > > [...] > > > > > > > > > > > > > > - header_size = skb->network_header + off + > > MAX_IPOPTLEN; > > > > > > > - maybe_pull_tail(skb, header_size); > > > > > > > + if (!maybe_pull_tail(skb, sizeof(struct iphdr), > > MAX_IP_HDR_LEN)) > > > > > > > + goto out; > > > > > > > + > > > > > > > > > > > > I think you need to correctly update err to reflect this failure. > > > > > > Using -EPROTO will wrongly blame frontend while it is backend that''s > > > > > > failing to process the packet. > > > > > > > > > > > > > > > > But a failure should only occur if the packet is malformed, so that would > > be > > > > a frontend error wouldn''t it? > > > > > > > > > > > > > __pskb_pull_tail may fail due to malloc failure. > > > > > > > > However the return value of __pskb_pull_tail cannot reflect the wether > > > > the failure is due to malformed packet or OOM. Not sure what''s the best > > > > solution here. What''s the malformed packet you were talking about? > > > > > > > > > > For example, the pull would fail if the packet had an either_type of > > > IP but didn''t contain an IP header, or perhaps an IPv6 packet that had > > > an incomplete option header sequence. I would have thought such a > > > packet was a more likely cause of failure than OOM, so -EPROTO seems a > > > reasonable best guess. > > > > How? __pskb_pull_tail doesn''t seem to care about upper layer protocols. > > And maybe_pull_tail has already done some lenght comparisions. > > > > No, __pskb_pull_tail() doesn''t care but the final check in > maybe_pull_tail() means it will return false if skb_headlen() is not > at least as big as what it was asked for. So if we try to pull up an > IP header and there''s fewer bytes than that available then we hit the > error condition. Or maybe I''m missing something.OK, we''er still on the same boat here. ;-) Would it make sense to make maybe_pull_tail to return int to reflect __pskb_pull_fail? In that case we can distinguish backend failure and frontend failure. I pay extra attention to this as we often have no access to frontend and we probably don''t want to blame frontend for non-existent misbehavior. Wei.> > Paul
Paul Durrant
2013-Dec-03 15:15 UTC
Re: [PATCH net v4] xen-netback: fix fragment detection in checksum setup
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 03 December 2013 15:13 > To: Paul Durrant > Cc: Wei Liu; xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan Kiss; > Ian Campbell; David Vrabel; David Miller > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in checksum > setup > > On Tue, Dec 03, 2013 at 03:05:40PM +0000, Paul Durrant wrote: > > > -----Original Message----- > > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > > Sent: 03 December 2013 14:58 > > > To: Paul Durrant > > > Cc: Wei Liu; xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan > Kiss; > > > Ian Campbell; David Vrabel; David Miller > > > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in > checksum > > > setup > > > > > > On Tue, Dec 03, 2013 at 02:34:56PM +0000, Paul Durrant wrote: > > > > > -----Original Message----- > > > > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > > > > Sent: 03 December 2013 14:29 > > > > > To: Paul Durrant > > > > > Cc: Wei Liu; xen-devel@lists.xen.org; netdev@vger.kernel.org; Zoltan > > > Kiss; > > > > > Ian Campbell; David Vrabel; David Miller > > > > > Subject: Re: [PATCH net v4] xen-netback: fix fragment detection in > > > checksum > > > > > setup > > > > > > > > > > On Tue, Dec 03, 2013 at 02:05:17PM +0000, Paul Durrant wrote: > > > > > [...] > > > > > > > > > > > > > > > > - header_size = skb->network_header + off + > > > MAX_IPOPTLEN; > > > > > > > > - maybe_pull_tail(skb, header_size); > > > > > > > > + if (!maybe_pull_tail(skb, sizeof(struct iphdr), > > > MAX_IP_HDR_LEN)) > > > > > > > > + goto out; > > > > > > > > + > > > > > > > > > > > > > > I think you need to correctly update err to reflect this failure. > > > > > > > Using -EPROTO will wrongly blame frontend while it is backend > that''s > > > > > > > failing to process the packet. > > > > > > > > > > > > > > > > > > > But a failure should only occur if the packet is malformed, so that > would > > > be > > > > > a frontend error wouldn''t it? > > > > > > > > > > > > > > > > __pskb_pull_tail may fail due to malloc failure. > > > > > > > > > > However the return value of __pskb_pull_tail cannot reflect the > wether > > > > > the failure is due to malformed packet or OOM. Not sure what''s the > best > > > > > solution here. What''s the malformed packet you were talking about? > > > > > > > > > > > > > For example, the pull would fail if the packet had an either_type of > > > > IP but didn''t contain an IP header, or perhaps an IPv6 packet that had > > > > an incomplete option header sequence. I would have thought such a > > > > packet was a more likely cause of failure than OOM, so -EPROTO seems > a > > > > reasonable best guess. > > > > > > How? __pskb_pull_tail doesn''t seem to care about upper layer protocols. > > > And maybe_pull_tail has already done some lenght comparisions. > > > > > > > No, __pskb_pull_tail() doesn''t care but the final check in > > maybe_pull_tail() means it will return false if skb_headlen() is not > > at least as big as what it was asked for. So if we try to pull up an > > IP header and there''s fewer bytes than that available then we hit the > > error condition. Or maybe I''m missing something. > > OK, we''er still on the same boat here. ;-) >Good. I was beginning to worry. :-)> Would it make sense to make maybe_pull_tail to return int to reflect > __pskb_pull_fail? In that case we can distinguish backend failure and > frontend failure. > > I pay extra attention to this as we often have no access to frontend and > we probably don''t want to blame frontend for non-existent misbehavior. >Ok. That sounds fair enough. I''ll do that. Paul