* [net-next-2.6 PATCH] net: consolidate netif_needs_gso() checks
@ 2010-06-17 0:18 Jeff Kirsher
2010-06-17 10:18 ` Herbert Xu
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Kirsher @ 2010-06-17 0:18 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, bphilips, John Fastabend, Herbert Xu, Jeff Kirsher
From: John Fastabend <john.r.fastabend@intel.com>
netif_needs_gso() is checked twice in the TX path once,
before submitting the skb to the qdisc and once after
it is dequeued from the qdisc just before calling
ndo_hard_start(). This opens a window for a user to
change the gso/tso or tx checksum settings that can
cause netif_needs_gso to be true in one check and false
in the other.
Specifically, changing TX checksum setting may cause
the warning in skb_gso_segment() to be triggered if
the checksum is calculated earlier.
This consolidates the netif_needs_gso() calls so that
the stack only checks if gso is needed in
dev_hard_start_xmit().
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
net/core/dev.c | 68 ++++++++++++++++++++++++++------------------------------
1 files changed, 32 insertions(+), 36 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 5902426..7f390b5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1895,6 +1895,22 @@ static inline void skb_orphan_try(struct sk_buff *skb)
skb_orphan(skb);
}
+/*
+ * Returns true if either:
+ * 1. skb has frag_list and the device doesn't support FRAGLIST, or
+ * 2. skb is fragmented and the device does not support SG, or if
+ * at least one of fragments is in highmem and device does not
+ * support DMA from it.
+ */
+static inline int skb_needs_linearize(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ return skb_is_nonlinear(skb) &&
+ ((skb_has_frags(skb) && !(dev->features & NETIF_F_FRAGLIST)) ||
+ (skb_shinfo(skb)->nr_frags && (!(dev->features & NETIF_F_SG) ||
+ illegal_highdma(dev, skb))));
+}
+
int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq)
{
@@ -1919,6 +1935,22 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
goto out_kfree_skb;
if (skb->next)
goto gso;
+ } else {
+ if (skb_needs_linearize(skb, dev) &&
+ __skb_linearize(skb))
+ goto out_kfree_skb;
+
+ /* If packet is not checksummed and device does not
+ * support checksumming for this protocol, complete
+ * checksumming here.
+ */
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ skb_set_transport_header(skb, skb->csum_start -
+ skb_headroom(skb));
+ if (!dev_can_checksum(dev, skb) &&
+ skb_checksum_help(skb))
+ goto out_kfree_skb;
+ }
}
rc = ops->ndo_start_xmit(skb, dev);
@@ -2089,22 +2121,6 @@ static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
return rc;
}
-/*
- * Returns true if either:
- * 1. skb has frag_list and the device doesn't support FRAGLIST, or
- * 2. skb is fragmented and the device does not support SG, or if
- * at least one of fragments is in highmem and device does not
- * support DMA from it.
- */
-static inline int skb_needs_linearize(struct sk_buff *skb,
- struct net_device *dev)
-{
- return skb_is_nonlinear(skb) &&
- ((skb_has_frags(skb) && !(dev->features & NETIF_F_FRAGLIST)) ||
- (skb_shinfo(skb)->nr_frags && (!(dev->features & NETIF_F_SG) ||
- illegal_highdma(dev, skb))));
-}
-
/**
* dev_queue_xmit - transmit a buffer
* @skb: buffer to transmit
@@ -2137,25 +2153,6 @@ int dev_queue_xmit(struct sk_buff *skb)
struct Qdisc *q;
int rc = -ENOMEM;
- /* GSO will handle the following emulations directly. */
- if (netif_needs_gso(dev, skb))
- goto gso;
-
- /* Convert a paged skb to linear, if required */
- if (skb_needs_linearize(skb, dev) && __skb_linearize(skb))
- goto out_kfree_skb;
-
- /* If packet is not checksummed and device does not support
- * checksumming for this protocol, complete checksumming here.
- */
- if (skb->ip_summed == CHECKSUM_PARTIAL) {
- skb_set_transport_header(skb, skb->csum_start -
- skb_headroom(skb));
- if (!dev_can_checksum(dev, skb) && skb_checksum_help(skb))
- goto out_kfree_skb;
- }
-
-gso:
/* Disable soft irqs for various locks below. Also
* stops preemption for RCU.
*/
@@ -2214,7 +2211,6 @@ gso:
rc = -ENETDOWN;
rcu_read_unlock_bh();
-out_kfree_skb:
kfree_skb(skb);
return rc;
out:
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [net-next-2.6 PATCH] net: consolidate netif_needs_gso() checks
2010-06-17 0:18 [net-next-2.6 PATCH] net: consolidate netif_needs_gso() checks Jeff Kirsher
@ 2010-06-17 10:18 ` Herbert Xu
2010-06-21 20:55 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Herbert Xu @ 2010-06-17 10:18 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, netdev, gospo, bphilips, John Fastabend
On Wed, Jun 16, 2010 at 05:18:12PM -0700, Jeff Kirsher wrote:
> From: John Fastabend <john.r.fastabend@intel.com>
>
> netif_needs_gso() is checked twice in the TX path once,
> before submitting the skb to the qdisc and once after
> it is dequeued from the qdisc just before calling
> ndo_hard_start(). This opens a window for a user to
> change the gso/tso or tx checksum settings that can
> cause netif_needs_gso to be true in one check and false
> in the other.
>
> Specifically, changing TX checksum setting may cause
> the warning in skb_gso_segment() to be triggered if
> the checksum is calculated earlier.
>
> This consolidates the netif_needs_gso() calls so that
> the stack only checks if gso is needed in
> dev_hard_start_xmit().
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [net-next-2.6 PATCH] net: consolidate netif_needs_gso() checks
2010-06-17 10:18 ` Herbert Xu
@ 2010-06-21 20:55 ` David Miller
0 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2010-06-21 20:55 UTC (permalink / raw)
To: herbert; +Cc: jeffrey.t.kirsher, netdev, gospo, bphilips, john.r.fastabend
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 17 Jun 2010 20:18:57 +1000
> On Wed, Jun 16, 2010 at 05:18:12PM -0700, Jeff Kirsher wrote:
>> From: John Fastabend <john.r.fastabend@intel.com>
>>
>> netif_needs_gso() is checked twice in the TX path once,
>> before submitting the skb to the qdisc and once after
>> it is dequeued from the qdisc just before calling
>> ndo_hard_start(). This opens a window for a user to
>> change the gso/tso or tx checksum settings that can
>> cause netif_needs_gso to be true in one check and false
>> in the other.
>>
>> Specifically, changing TX checksum setting may cause
>> the warning in skb_gso_segment() to be triggered if
>> the checksum is calculated earlier.
>>
>> This consolidates the netif_needs_gso() calls so that
>> the stack only checks if gso is needed in
>> dev_hard_start_xmit().
>>
>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied, thanks!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-06-21 20:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-17 0:18 [net-next-2.6 PATCH] net: consolidate netif_needs_gso() checks Jeff Kirsher
2010-06-17 10:18 ` Herbert Xu
2010-06-21 20:55 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.