netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] [BRIDGE] Set features based on slave's ones (was Ethernet Bridging: Enable Hardware Checksumming)
From: Stephen Hemminger @ 2005-05-19 20:39 UTC (permalink / raw)
  To: Catalin(ux aka Dino) BOIE; +Cc: netdev, davem
In-Reply-To: <Pine.LNX.4.62.0505191654400.24043@webhosting.rdsbv.ro>

On Thu, 19 May 2005 18:28:26 +0300 (EEST)
"Catalin(ux aka Dino) BOIE" <util@deuroconsult.ro> wrote:

> Hello!
> 
> The attached patch, makes the bridge to select features (almost all) only 
> if _all_ devices supports them.
> The patch was tested (create a bridge, add an interface, remove the 
> interface and always check features flags).
> 
> What do you think?
> 
> Signed-off-by: Catalin(ux aka Dino) BOIE <catab at umbrella.ro>

Okay, but just open code this, don't need BR_FEAT_MASK
and br_features_change.

^ permalink raw reply

* Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming
From: Nivedita Singhvi @ 2005-05-19 20:43 UTC (permalink / raw)
  To: David S. Miller; +Cc: herbert, jdmason, shemminger, netdev
In-Reply-To: <20050519.115141.21926463.davem@davemloft.net>

David S. Miller wrote:

> I think instead we should look at ways to propagate
> the hardware device features to the bridge.  Even if
> a bridge is composed of multiple devices, we just
> advertise the subset of features actually supported.

Just for some background:

In the typical Xen virtual environment, the primary
interface is typically bridged with virtual interfaces
that talk to other partitions.

Since currently the bridge doesn't advertise them,
this results in the loss of hw checksum offload,
segmentation offload, and sendfile exploitation
(sendfile() falls down to the usual tcp_sendmsg()
case) etc on the primary partition traffic talking
to remote hosts.

This can be rather painful, and performance comparisons
with other OSs which don't do this are even more of
an apples to oranges comparison since this is
somewhat silently happening.

Hence the need for enhancing the bridging environment
somewhat :).

thanks,
Nivedita

^ permalink raw reply

* Re: [PATCH] [BRIDGE] Set features based on slave's ones (was Ethernet Bridging: Enable Hardware Checksumming)
From: Stephen Hemminger @ 2005-05-19 21:20 UTC (permalink / raw)
  To: Catalin(ux aka Dino) BOIE; +Cc: netdev, davem
In-Reply-To: <Pine.LNX.4.62.0505191654400.24043@webhosting.rdsbv.ro>

Also NETIF_F_LLTX should always be set on the bridge, independent of what devices are in it.

^ permalink raw reply

* Re: [IPSEC] Verify key payload in verify_one_algo
From: Herbert Xu @ 2005-05-19 21:34 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev
In-Reply-To: <20050519.124059.85410159.davem@davemloft.net>

On Thu, May 19, 2005 at 12:40:59PM -0700, David S. Miller wrote:
> 
> I gather from these two patches that once you found the
> first problem you went around auditing alg_key_len usage
> and these were the only two bugs you spotted.  Right?

Correct.

Cheers,
-- 
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

* Re: assertion (!atomic_read(&sk->sk_rmem_alloc)) failed at net/netlink/af_netlink.c (122)
From: Herbert Xu @ 2005-05-19 21:34 UTC (permalink / raw)
  To: David S. Miller; +Cc: tommy.christensen, chamas, netdev
In-Reply-To: <20050519.124921.115910096.davem@davemloft.net>

On Thu, May 19, 2005 at 12:49:21PM -0700, David S. Miller wrote:
> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Thu, 12 May 2005 21:07:38 +1000
> 
> > On Thu, May 12, 2005 at 12:58:03PM +0200, Tommy Christensen wrote:
> > > 
> > > Signed-off-by: Tommy S. Christensen <tommy.christensen@tpack.net>
> > 
> > Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> Hmmm, should I undo Tommy's one-liner skb_orphan() change
> then add this one instead?

Please do since the first one had a subtle bug.

Cheers,
-- 
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

* Re: [PATCH] netlink: defer socket destruction a bit
From: Herbert Xu @ 2005-05-19 21:38 UTC (permalink / raw)
  To: David S. Miller; +Cc: tommy.christensen, netdev, chamas
In-Reply-To: <20050519.130859.115641546.davem@davemloft.net>

On Thu, May 19, 2005 at 01:08:59PM -0700, David S. Miller wrote:
> 
> Ok, I think I got all the patches straight.  All of Tommy's
> patches combined together look like this diff in my tree.
> Please double check it.

Yes this is the one.

> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -735,11 +735,15 @@ static inline int do_one_broadcast(struc
>  
>  	sock_hold(sk);
>  	if (p->skb2 == NULL) {
> -		if (atomic_read(&p->skb->users) != 1) {
> +		if (skb_shared(p->skb)) {
>  			p->skb2 = skb_clone(p->skb, p->allocation);
>  		} else {
> -			p->skb2 = p->skb;
> -			atomic_inc(&p->skb->users);
> +			p->skb2 = skb_get(p->skb);
> +			/*
> +			 * skb ownership may have been set when
> +			 * delivered to a previous socket.
> +			 */
> +			skb_orphan(p->skb2);
>  		}
>  	}
>  	if (p->skb2 == NULL) {
> @@ -785,11 +789,12 @@ int netlink_broadcast(struct sock *ssk, 
>  	sk_for_each_bound(sk, node, &nl_table[ssk->sk_protocol].mc_list)
>  		do_one_broadcast(sk, &info);
>  
> +	kfree_skb(skb);
> +
>  	netlink_unlock_table();
>  
>  	if (info.skb2)
>  		kfree_skb(info.skb2);
> -	kfree_skb(skb);
>  
>  	if (info.delivered) {
>  		if (info.congested && (allocation & __GFP_WAIT))
-- 
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

* Re: [PATCH] [BRIDGE] Set features based on slave's ones (was Ethernet Bridging: Enable Hardware Checksumming)
From: Jon Mason @ 2005-05-19 21:40 UTC (permalink / raw)
  To: Catalin(ux aka Dino) BOIE; +Cc: netdev, davem
In-Reply-To: <Pine.LNX.4.62.0505191654400.24043@webhosting.rdsbv.ro>

On Thu, May 19, 2005 at 06:28:26PM +0300, Catalin(ux aka Dino) BOIE wrote:
> Hello!
> 
> The attached patch, makes the bridge to select features (almost all) only 
> if _all_ devices supports them.
> The patch was tested (create a bridge, add an interface, remove the 
> interface and always check features flags).
> 
> What do you think?
> 
> Signed-off-by: Catalin(ux aka Dino) BOIE <catab at umbrella.ro>
> ---
> Catalin(ux aka Dino) BOIE
> catab at deuroconsult.ro
> http://kernel.umbrella.ro/

> --- bridge1/net/bridge/br_private.h	2005-03-02 09:37:50.000000000 +0200
> +++ linux/net/bridge/br_private.h	2005-05-19 18:00:05.000000000 +0300
> @@ -27,6 +27,10 @@
>  #define BR_PORT_BITS	10
>  #define BR_MAX_PORTS	(1<<BR_PORT_BITS)
>  
> +#define BR_FEAT_MASK	(NETIF_F_HW_CSUM | NETIF_F_SG \
> +			| NETIF_F_FRAGLIST | NETIF_F_IP_CSUM \
> +			| NETIF_F_HIGHDMA | NETIF_F_TSO)
> +
>  typedef struct bridge_id bridge_id;
>  typedef struct mac_addr mac_addr;
>  typedef __u16 port_id;
> --- bridge1/net/bridge/br_device.c	2005-03-02 09:37:30.000000000 +0200
> +++ linux/net/bridge/br_device.c	2005-05-19 17:14:21.000000000 +0300
> @@ -107,4 +107,5 @@ void br_dev_setup(struct net_device *dev
>  	dev->tx_queue_len = 0;
>  	dev->set_mac_address = NULL;
>  	dev->priv_flags = IFF_EBRIDGE;
> +	dev->features = BR_FEAT_MASK;
>  }
> --- bridge1/net/bridge/br_if.c	2005-03-02 09:38:33.000000000 +0200
> +++ linux/net/bridge/br_if.c	2005-05-19 18:21:39.000000000 +0300
> @@ -314,6 +314,27 @@ int br_min_mtu(const struct net_bridge *
>  	return mtu;
>  }
>  
> +/*
> + * If slave device (@dev) doesn't support special features,
> + * turn them off globally.
> + */
> +static inline void br_features_change(struct net_bridge *br, struct net_device *dev)
> +{
> +	br->dev->features &= dev->features | ~BR_FEAT_MASK;

~BR_FEAT_MASK isn't necessary. Either you wanted to do an '&' just
before it, so that BR_FEAT_MASK features are the only ones that you want
or you are enabling features that aren't necessarily supported by the
adapter/driver (like LLTX and HW_VLAN stuff).

> +}
> +
> +/*
> + * Recomputes features using slave's features
> + */
> +static void br_features_recompute(struct net_bridge *br)
> +{
> +	struct net_bridge_port *p;
> +
> +	br->dev->features |= BR_FEAT_MASK;

The OR is not needed.  Just re-initialize features to BR_FEAT_MASK and
everything will take case of itself.

> +	list_for_each_entry(p, &br->port_list, list)
> +		br_features_change(br, p->dev);
> +}
> +
>  /* called with RTNL */
>  int br_add_if(struct net_bridge *br, struct net_device *dev)
>  {
> @@ -332,9 +353,10 @@ int br_add_if(struct net_bridge *br, str
>  	if (IS_ERR(p = new_nbp(br, dev, br_initial_port_cost(dev))))
>  		return PTR_ERR(p);
>  
> +	br_features_change(br, dev);
> +
>   	if ((err = br_fdb_insert(br, p, dev->dev_addr, 1)))
>  		destroy_nbp(p);
> - 
>  	else if ((err = br_sysfs_addif(p)))
>  		del_nbp(p);
>  	else {
> @@ -368,6 +390,7 @@ int br_del_if(struct net_bridge *br, str
>  
>  	spin_lock_bh(&br->lock);
>  	br_stp_recalculate_bridge_id(br);
> +	br_features_recompute(br);
>  	spin_unlock_bh(&br->lock);
>  
>  	return 0;

^ permalink raw reply

* Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming
From: David S. Miller @ 2005-05-19 21:48 UTC (permalink / raw)
  To: shemminger; +Cc: jdmason, netdev
In-Reply-To: <20050519133333.07a992e6@dxpl.pdx.osdl.net>

From: Stephen Hemminger <shemminger@osdl.org>
Date: Thu, 19 May 2005 13:33:33 -0700

> The bridge doesn't need locking, or checksumming and can allow highdma
> buffers; all of which are handled by net/core/dev.c if needed.

As discuesed elsewhere, this handling by net/core/dev.c makes
TCP sending much more expensive when it is actually used.

Furthermore, I just found another hole in the idea to propagate
sub-device features into the bridge device.

If one device has NETIF_F_HW_CSUM and the others have NETIF_F_IP_CSUM,
both bits will be set in the bridge device and things will entirely
break.  The two checksumming on output schemes are different, and all
of the stack assumes that only one of these two bits are set.

I have such a setup in two of my sparc64 systems (sunhme does
NETIF_F_HW_CSUM, and tg3 does NETIF_F_IP_CSUM).  Also, my PowerMAC G5
has this problem too, the onboard sungem chip does NETIF_F_HW_CSUM and
the tg3 I have in a PCI slot does NETIF_F_P_CSUM.  So given that
half the machines I have powered on right here could trigger the
problem, it's far from theoretical :-)

There are multiple spots that want to do this kind of stuff
now (bridging, vlan, bonding) which indicates that some sort
of common infrastructure should be written to implement this
kind of stuff.

^ permalink raw reply

* Re: [IPV4/IPV6] Keep wmem accounting separate in ip*_push_pending_frames
From: Herbert Xu @ 2005-05-19 21:48 UTC (permalink / raw)
  To: David S. Miller; +Cc: johnpol, netdev
In-Reply-To: <20050519.123530.55508893.davem@davemloft.net>

On Thu, May 19, 2005 at 12:35:30PM -0700, David S. Miller wrote:
> 
> Ok, based upon this I am adding the following patch to my
> tree.  Thanks.

Thanks Dave, this looks good to me.

> --- 1/net/ipv4/netfilter/ip_conntrack_core.c.~1~	2005-05-18 22:45:26.000000000 -0700
> +++ 2/net/ipv4/netfilter/ip_conntrack_core.c	2005-05-19 12:32:26.000000000 -0700
> @@ -940,37 +940,25 @@
>  struct sk_buff *
>  ip_ct_gather_frags(struct sk_buff *skb, u_int32_t user)
>  {
> -	struct sock *sk = skb->sk;
>  #ifdef CONFIG_NETFILTER_DEBUG
>  	unsigned int olddebug = skb->nf_debug;
>  #endif
>  
> -	if (sk) {
> -		sock_hold(sk);
> -		skb_orphan(skb);
> -	}
> +	skb_orphan(skb);
>  
>  	local_bh_disable(); 
>  	skb = ip_defrag(skb, user);
>  	local_bh_enable();
>  
> -	if (!skb) {
> -		if (sk)
> -			sock_put(sk);
> -		return skb;
> -	}
> -
> -	if (sk) {
> -		skb_set_owner_w(skb, sk);
> -		sock_put(sk);
> -	}
> -
> -	ip_send_check(skb->nh.iph);
> -	skb->nfcache |= NFC_ALTERED;
> +	if (skb) {
> +		ip_send_check(skb->nh.iph);
> +		skb->nfcache |= NFC_ALTERED;
>  #ifdef CONFIG_NETFILTER_DEBUG
> -	/* Packet path as if nothing had happened. */
> -	skb->nf_debug = olddebug;
> +		/* Packet path as if nothing had happened. */
> +		skb->nf_debug = olddebug;
>  #endif
> +	}
> +
>  	return skb;
>  }
-- 
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

* [PATCH] (1/3) netem - reinsert for duplication
From: Stephen Hemminger @ 2005-05-19 22:12 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, netem

Handle duplication of packets in netem by re-inserting at top of qdisc tree.
This avoid problems with qlen accounting with nested qdisc. This recursion
requires no additional locking but will potentially increase stack depth.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

Index: netem-2.6.12-rc4/net/sched/sch_netem.c
===================================================================
--- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
+++ netem-2.6.12-rc4/net/sched/sch_netem.c
@@ -203,42 +203,47 @@ static int netem_run(struct Qdisc *sch)
 	return 0;
 }
 
+/*
+ * Insert one skb into qdisc.
+ * Note: parent depends on return value to account for queue length.
+ * 	NET_XMIT_DROP: queue length didn't change.
+ *      NET_XMIT_SUCCESS: one skb was queued.
+ */
 static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 {
 	struct netem_sched_data *q = qdisc_priv(sch);
+	struct sk_buff *skb2;
 	int ret;
+	int count = 1;
 
 	pr_debug("netem_enqueue skb=%p\n", skb);
 
+	/* Random duplication */
+	if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor))
+		++count;
+
 	/* Random packet drop 0 => none, ~0 => all */
-	if (q->loss && q->loss >= get_crandom(&q->loss_cor)) {
-		pr_debug("netem_enqueue: random loss\n");
+	if (q->loss && q->loss >= get_crandom(&q->loss_cor))
+		--count;
+
+	if (count == 0) {
 		sch->qstats.drops++;
 		kfree_skb(skb);
-		return 0;	/* lie about loss so TCP doesn't know */
+		return NET_XMIT_DROP;
 	}
 
-	/* Random duplication */
-	if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor)) {
-		struct sk_buff *skb2;
+	/*
+	 * If we need to duplicate packet, then re-insert at top of the
+	 * qdisc tree, since parent queuer expects that only one
+	 * skb will be queued.
+	 */
+	if (count > 1 && (skb2 = skb_clone(skb, GFP_ATOMIC)) != NULL) {
+		struct Qdisc *rootq = sch->dev->qdisc;
+		u32 dupsave = q->duplicate; /* prevent duplicating a dup... */
+		q->duplicate = 0;
 
-		skb2 = skb_clone(skb, GFP_ATOMIC);
-		if (skb2 && netem_delay(sch, skb2) == NET_XMIT_SUCCESS) {
-			struct Qdisc *qp;
-
-			/* Since one packet can generate two packets in the
-			 * queue, the parent's qlen accounting gets confused,
-			 * so fix it.
-			 */
-			qp = qdisc_lookup(sch->dev, TC_H_MAJ(sch->parent));
-			if (qp)
-				qp->q.qlen++;
-
-			sch->q.qlen++;
-			sch->bstats.bytes += skb2->len;
-			sch->bstats.packets++;
-		} else
-			sch->qstats.drops++;
+		rootq->enqueue(skb2, rootq);
+		q->duplicate = dupsave;
 	}
 
 	/* If doing simple delay then gap == 0 so all packets

^ permalink raw reply

* [PATCH] (2/3) netem: use only inner qdisc -- no private skbuff queue
From: Stephen Hemminger @ 2005-05-19 22:12 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, netem

Netem works better if there if packets are just queued in the inner discipline
rather than having a separate delayed queue. Change to use the dequeue/requeue
to peek like TBF does.

By doing this potential qlen problems with the old method are avoided. The problems
happened when the netem_run that moved packets from the inner discipline to the nested
discipline failed (because inner queue was full). This happened in dequeue, so the
effective qlen of the netem would be decreased (because of the drop), but there was
no way to keep the outer qdisc (caller of netem dequeue) in sync.

The problem window is still there since this patch doesn't address the issue of
requeue failing in netem_dequeue, but that shouldn't happen since the sequence dequeue/requeue
should always work.  Long term correct fix is to implement qdisc->peek in all the qdisc's
to allow for this (needed by several other qdisc's as well).

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

Index: netem-2.6.12-rc4/net/sched/sch_netem.c
===================================================================
--- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
+++ netem-2.6.12-rc4/net/sched/sch_netem.c
@@ -53,7 +53,6 @@
 
 struct netem_sched_data {
 	struct Qdisc	*qdisc;
-	struct sk_buff_head delayed;
 	struct timer_list timer;
 
 	u32 latency;
@@ -137,72 +136,6 @@ static long tabledist(unsigned long mu, 
 	return  x / NETEM_DIST_SCALE + (sigma / NETEM_DIST_SCALE) * t + mu;
 }
 
-/* Put skb in the private delayed queue. */
-static int netem_delay(struct Qdisc *sch, struct sk_buff *skb)
-{
-	struct netem_sched_data *q = qdisc_priv(sch);
-	psched_tdiff_t td;
-	psched_time_t now;
-	
-	PSCHED_GET_TIME(now);
-	td = tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist);
-	
-	/* Always queue at tail to keep packets in order */
-	if (likely(q->delayed.qlen < q->limit)) {
-		struct netem_skb_cb *cb = (struct netem_skb_cb *)skb->cb;
-	
-		PSCHED_TADD2(now, td, cb->time_to_send);
-
-		pr_debug("netem_delay: skb=%p now=%llu tosend=%llu\n", skb, 
-			 now, cb->time_to_send);
-	
-		__skb_queue_tail(&q->delayed, skb);
-		return NET_XMIT_SUCCESS;
-	}
-
-	pr_debug("netem_delay: queue over limit %d\n", q->limit);
-	sch->qstats.overlimits++;
-	kfree_skb(skb);
-	return NET_XMIT_DROP;
-}
-
-/*
- *  Move a packet that is ready to send from the delay holding
- *  list to the underlying qdisc.
- */
-static int netem_run(struct Qdisc *sch)
-{
-	struct netem_sched_data *q = qdisc_priv(sch);
-	struct sk_buff *skb;
-	psched_time_t now;
-
-	PSCHED_GET_TIME(now);
-
-	skb = skb_peek(&q->delayed);
-	if (skb) {
-		const struct netem_skb_cb *cb
-			= (const struct netem_skb_cb *)skb->cb;
-		long delay 
-			= PSCHED_US2JIFFIE(PSCHED_TDIFF(cb->time_to_send, now));
-		pr_debug("netem_run: skb=%p delay=%ld\n", skb, delay);
-
-		/* if more time remaining? */
-		if (delay > 0) {
-			mod_timer(&q->timer, jiffies + delay);
-			return 1;
-		}
-
-		__skb_unlink(skb, &q->delayed);
-		
-		if (q->qdisc->enqueue(skb, q->qdisc)) {
-			sch->q.qlen--;
-			sch->qstats.drops++;
-		} 
-	}
-
-	return 0;
-}
-
 /*
  * Insert one skb into qdisc.
  * Note: parent depends on return value to account for queue length.
@@ -212,6 +145,7 @@ static int netem_run(struct Qdisc *sch)
 static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
 {
 	struct netem_sched_data *q = qdisc_priv(sch);
+	struct netem_skb_cb *cb = (struct netem_skb_cb *)skb->cb;
 	struct sk_buff *skb2;
 	int ret;
 	int count = 1;
@@ -246,18 +180,24 @@ static int netem_enqueue(struct sk_buff 
 		q->duplicate = dupsave;
 	}
 
-	/* If doing simple delay then gap == 0 so all packets
-	 * go into the delayed holding queue
-	 * otherwise if doing out of order only "1 out of gap"
-	 * packets will be delayed.
+	/* 
+	 * Do re-ordering by putting one out of N packets at the front
+	 * of the queue.
+	 * gap == 0 is special case for no-reordering.
 	 */
-	if (q->counter < q->gap) {
+	if (q->gap == 0 || q->counter != q->gap) {
+		psched_time_t now;
+		PSCHED_GET_TIME(now);
+		PSCHED_TADD2(now, 
+			     tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist),
+			     cb->time_to_send);
+		
 		++q->counter;
 		ret = q->qdisc->enqueue(skb, q->qdisc);
 	} else {
 		q->counter = 0;
-		ret = netem_delay(sch, skb);
-		netem_run(sch);
+		PSCHED_GET_TIME(cb->time_to_send);
+		ret = q->qdisc->ops->requeue(skb, q->qdisc);
 	}
 
 	if (likely(ret == NET_XMIT_SUCCESS)) {
@@ -301,22 +241,33 @@ static struct sk_buff *netem_dequeue(str
 {
 	struct netem_sched_data *q = qdisc_priv(sch);
 	struct sk_buff *skb;
-	int pending;
-
-	pending = netem_run(sch);
 
 	skb = q->qdisc->dequeue(q->qdisc);
 	if (skb) {
-		pr_debug("netem_dequeue: return skb=%p\n", skb);
-		sch->q.qlen--;
-		sch->flags &= ~TCQ_F_THROTTLED;
-	}
-	else if (pending) {
-		pr_debug("netem_dequeue: throttling\n");
+		const struct netem_skb_cb *cb
+			= (const struct netem_skb_cb *)skb->cb;
+		psched_time_t now;
+		long delay;
+
+		/* if more time remaining? */
+		PSCHED_GET_TIME(now);
+		delay = PSCHED_US2JIFFIE(PSCHED_TDIFF(cb->time_to_send, now));
+		pr_debug("netem_run: skb=%p delay=%ld\n", skb, delay);
+		if (delay <= 0) {
+			pr_debug("netem_dequeue: return skb=%p\n", skb);
+			sch->q.qlen--;
+			sch->flags &= ~TCQ_F_THROTTLED;
+			return skb;
+		}
+
+		mod_timer(&q->timer, jiffies + delay);
 		sch->flags |= TCQ_F_THROTTLED;
-	} 
 
-	return skb;
+		if (q->qdisc->ops->requeue(skb, q->qdisc) != 0)
+			sch->qstats.drops++;
+	}
+
+	return NULL;
 }
 
 static void netem_watchdog(unsigned long arg)
@@ -333,8 +284,6 @@ static void netem_reset(struct Qdisc *sc
 	struct netem_sched_data *q = qdisc_priv(sch);
 
 	qdisc_reset(q->qdisc);
-	skb_queue_purge(&q->delayed);
-
 	sch->q.qlen = 0;
 	sch->flags &= ~TCQ_F_THROTTLED;
 	del_timer_sync(&q->timer);
@@ -460,7 +409,6 @@ static int netem_init(struct Qdisc *sch,
 	if (!opt)
 		return -EINVAL;
 
-	skb_queue_head_init(&q->delayed);
 	init_timer(&q->timer);
 	q->timer.function = netem_watchdog;
 	q->timer.data = (unsigned long) sch;

^ permalink raw reply

* [PATCH] (3/3) netem: allow random reordering
From: Stephen Hemminger @ 2005-05-19 22:12 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, netem

Enhance the reorder feature of netem to allow random percent to be reordered.
Has expected backwards compatibility behaviour.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>

Index: netem-2.6.12-rc4/include/linux/pkt_sched.h
===================================================================
--- netem-2.6.12-rc4.orig/include/linux/pkt_sched.h
+++ netem-2.6.12-rc4/include/linux/pkt_sched.h
@@ -427,6 +427,7 @@ enum
 	TCA_NETEM_UNSPEC,
 	TCA_NETEM_CORR,
 	TCA_NETEM_DELAY_DIST,
+	TCA_NETEM_REORDER,
 	__TCA_NETEM_MAX,
 };
 
@@ -437,7 +438,7 @@ struct tc_netem_qopt
 	__u32	latency;	/* added delay (us) */
 	__u32   limit;		/* fifo limit (packets) */
 	__u32	loss;		/* random packet loss (0=none ~0=100%) */
-	__u32	gap;		/* re-ordering gap (0 for delay all) */
+	__u32	gap;		/* re-ordering gap (0 for none) */
 	__u32   duplicate;	/* random packet dup  (0=none ~0=100%) */
 	__u32	jitter;		/* random jitter in latency (us) */
 };
@@ -449,6 +450,12 @@ struct tc_netem_corr
 	__u32	dup_corr;	/* duplicate correlation  */
 };
 
+struct tc_netem_reorder
+{
+	__u32	probability;
+	__u32	correlation;
+};
+
 #define NETEM_DIST_SCALE	8192
 
 #endif
Index: netem-2.6.12-rc4/net/sched/sch_netem.c
===================================================================
--- netem-2.6.12-rc4.orig/net/sched/sch_netem.c
+++ netem-2.6.12-rc4/net/sched/sch_netem.c
@@ -62,11 +62,12 @@ struct netem_sched_data {
 	u32 gap;
 	u32 jitter;
 	u32 duplicate;
+	u32 reorder;
 
 	struct crndstate {
 		unsigned long last;
 		unsigned long rho;
-	} delay_cor, loss_cor, dup_cor;
+	} delay_cor, loss_cor, dup_cor, reorder_cor;
 
 	struct disttable {
 		u32  size;
@@ -185,18 +186,18 @@ static int netem_enqueue(struct sk_buff 
 	 * of the queue.
 	 * gap == 0 is special case for no-reordering.
 	 */
-	if (q->gap == 0 || q->counter != q->gap) {
+	if (q->gap == 0 && q->counter < q->gap && 
+	    q->reorder < get_crandom(&q->reorder_cor)) {
 		psched_time_t now;
 		PSCHED_GET_TIME(now);
-		PSCHED_TADD2(now, 
-			     tabledist(q->latency, q->jitter, &q->delay_cor, q->delay_dist),
+		PSCHED_TADD2(now, tabledist(q->latency, q->jitter, 
+					    &q->delay_cor, q->delay_dist),
 			     cb->time_to_send);
-		
 		++q->counter;
 		ret = q->qdisc->enqueue(skb, q->qdisc);
 	} else {
-		q->counter = 0;
 		PSCHED_GET_TIME(cb->time_to_send);
+		q->counter = 0;
 		ret = q->qdisc->ops->requeue(skb, q->qdisc);
 	}
 
@@ -351,6 +352,19 @@ static int get_correlation(struct Qdisc 
 	return 0;
 }
 
+static int get_reorder(struct Qdisc *sch, const struct rtattr *attr)
+{
+	struct netem_sched_data *q = qdisc_priv(sch);
+	const struct tc_netem_reorder *r = RTA_DATA(attr);
+
+	if (RTA_PAYLOAD(attr) != sizeof(*r))
+		return -EINVAL;
+
+	q->reorder = r->probability;
+	init_crandom(&q->reorder_cor, r->correlation);
+	return 0;
+}
+
 static int netem_change(struct Qdisc *sch, struct rtattr *opt)
 {
 	struct netem_sched_data *q = qdisc_priv(sch);
@@ -371,9 +385,15 @@ static int netem_change(struct Qdisc *sc
 	q->jitter = qopt->jitter;
 	q->limit = qopt->limit;
 	q->gap = qopt->gap;
+	q->counter = 0;
 	q->loss = qopt->loss;
 	q->duplicate = qopt->duplicate;
 
+	/* for compatiablity with earlier versions.
+	 * if gap is set, need to assume 100% probablity
+	 */
+	q->reorder = ~0;
+
 	/* Handle nested options after initial queue options.
 	 * Should have put all options in nested format but too late now.
 	 */ 
@@ -395,6 +415,11 @@ static int netem_change(struct Qdisc *sc
 			if (ret)
 				return ret;
 		}
+		if (tb[TCA_NETEM_REORDER-1]) {
+			ret = get_reorder(sch, tb[TCA_NETEM_REORDER-1]);
+			if (ret)
+				return ret;
+		}
 	}
 
 
@@ -412,7 +437,6 @@ static int netem_init(struct Qdisc *sch,
 	init_timer(&q->timer);
 	q->timer.function = netem_watchdog;
 	q->timer.data = (unsigned long) sch;
-	q->counter = 0;
 
 	q->qdisc = qdisc_create_dflt(sch->dev, &pfifo_qdisc_ops);
 	if (!q->qdisc) {
@@ -444,6 +468,7 @@ static int netem_dump(struct Qdisc *sch,
 	struct rtattr *rta = (struct rtattr *) b;
 	struct tc_netem_qopt qopt;
 	struct tc_netem_corr cor;
+	struct tc_netem_reorder reorder;
 
 	qopt.latency = q->latency;
 	qopt.jitter = q->jitter;
@@ -457,6 +482,11 @@ static int netem_dump(struct Qdisc *sch,
 	cor.loss_corr = q->loss_cor.rho;
 	cor.dup_corr = q->dup_cor.rho;
 	RTA_PUT(skb, TCA_NETEM_CORR, sizeof(cor), &cor);
+
+	reorder.probability = q->reorder;
+	reorder.correlation = q->reorder_cor.rho;
+	RTA_PUT(skb, TCA_NETEM_REORDER, sizeof(reorder), &reorder);
+
 	rta->rta_len = skb->tail - b;
 
 	return skb->len;

^ permalink raw reply

* tg3 crash.
From: raz ben jehuda @ 2005-05-19 22:41 UTC (permalink / raw)
  To: davem; +Cc: netdev

Hie david.
did you take a look at the code i sent you and everyone else ?

Raz.

^ permalink raw reply

* Re: [PATCH] netlink: defer socket destruction a bit
From: Tommy Christensen @ 2005-05-19 23:15 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David S. Miller, netdev, chamas
In-Reply-To: <20050519213839.GC24024@gondor.apana.org.au>

Herbert Xu wrote:
> On Thu, May 19, 2005 at 01:08:59PM -0700, David S. Miller wrote:
> 
>>Ok, I think I got all the patches straight.  All of Tommy's
>>patches combined together look like this diff in my tree.
>>Please double check it.
> 
> Yes this is the one.

Yup. This combination fixes both problems.

Thanks,
Tommy

^ permalink raw reply

* Re: [PATCH] netlink: defer socket destruction a bit
From: David S. Miller @ 2005-05-19 23:16 UTC (permalink / raw)
  To: tommy.christensen; +Cc: herbert, netdev, chamas
In-Reply-To: <428D1E19.9030204@tpack.net>

From: Tommy Christensen <tommy.christensen@tpack.net>
Subject: Re: [PATCH] netlink: defer socket destruction a bit
Date: Fri, 20 May 2005 01:15:37 +0200

> Herbert Xu wrote:
> > On Thu, May 19, 2005 at 01:08:59PM -0700, David S. Miller wrote:
> > 
> >>Ok, I think I got all the patches straight.  All of Tommy's
> >>patches combined together look like this diff in my tree.
> >>Please double check it.
> > 
> > Yes this is the one.
> 
> Yup. This combination fixes both problems.

Thanks a lot for double checking, both of you :-)

^ permalink raw reply

* Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming
From: Jon Mason @ 2005-05-19 23:23 UTC (permalink / raw)
  To: David S. Miller; +Cc: shemminger, netdev
In-Reply-To: <20050519.144800.71090042.davem@davemloft.net>

On Thursday 19 May 2005 04:48 pm, David S. Miller wrote:
> From: Stephen Hemminger <shemminger@osdl.org>
> Date: Thu, 19 May 2005 13:33:33 -0700
>
> > The bridge doesn't need locking, or checksumming and can allow highdma
> > buffers; all of which are handled by net/core/dev.c if needed.
>
> As discuesed elsewhere, this handling by net/core/dev.c makes
> TCP sending much more expensive when it is actually used.
>
> Furthermore, I just found another hole in the idea to propagate
> sub-device features into the bridge device.
>
> If one device has NETIF_F_HW_CSUM and the others have NETIF_F_IP_CSUM,
> both bits will be set in the bridge device and things will entirely
> break.  The two checksumming on output schemes are different, and all
> of the stack assumes that only one of these two bits are set.
>
> I have such a setup in two of my sparc64 systems (sunhme does
> NETIF_F_HW_CSUM, and tg3 does NETIF_F_IP_CSUM).  Also, my PowerMAC G5
> has this problem too, the onboard sungem chip does NETIF_F_HW_CSUM and
> the tg3 I have in a PCI slot does NETIF_F_IP_CSUM.  So given that
> half the machines I have powered on right here could trigger the
> problem, it's far from theoretical :-)
>
> There are multiple spots that want to do this kind of stuff
> now (bridging, vlan, bonding) which indicates that some sort
> of common infrastructure should be written to implement this
> kind of stuff.

Since NETIF_F_HW_CSUM encompasses NETIF_F_IP_CSUM, perhaps it would be better 
to use the latter until such time as a new infrastructure can be implimented.

Thanks,
Jon

^ permalink raw reply

* Re: [Netem] [PATCH] (1/3) netem - reinsert for duplication
From: Stephen Hemminger @ 2005-05-19 23:32 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, netdev
In-Reply-To: <20050519151250.17ab5c81@dxpl.pdx.osdl.net>

On Thu, 19 May 2005 15:12:50 -0700
Stephen Hemminger <shemminger@osdl.org> wrote:

> Handle duplication of packets in netem by re-inserting at top of qdisc tree.
> This avoid problems with qlen accounting with nested qdisc. This recursion
> requires no additional locking but will potentially increase stack depth.
> 
> Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
> 

Hold off on applying these patches until next week, I am seeing some timing
anomalies when using this for TCP testing, and not sure if these patches are related.

^ permalink raw reply

* Re: [PATCH] Ethernet Bridging: Enable Hardware Checksumming
From: David S. Miller @ 2005-05-19 23:36 UTC (permalink / raw)
  To: jdmason; +Cc: shemminger, netdev
In-Reply-To: <200505191823.31777.jdmason@us.ibm.com>

From: Jon Mason <jdmason@us.ibm.com>
Date: Thu, 19 May 2005 18:23:31 -0500

> Since NETIF_F_HW_CSUM encompasses NETIF_F_IP_CSUM, perhaps it would
> be better to use the latter until such time as a new infrastructure
> can be implimented.

Probably, that is the best idea, it is again another
definition of "subset".

^ permalink raw reply

* Re: [Netem] [PATCH] (1/3) netem - reinsert for duplication
From: David S. Miller @ 2005-05-19 23:37 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <20050519163235.2f5db193@dxpl.pdx.osdl.net>

From: Stephen Hemminger <shemminger@osdl.org>
Date: Thu, 19 May 2005 16:32:35 -0700

> Hold off on applying these patches until next week, I am seeing some
> timing anomalies when using this for TCP testing, and not sure if
> these patches are related.

Ok.

^ permalink raw reply

* RE: [PATCH] Super TSO
From: Leonid Grossman @ 2005-05-20  0:15 UTC (permalink / raw)
  To: 'David S. Miller'; +Cc: netdev
In-Reply-To: <20050517.192416.108741581.davem@davemloft.net>

A somewhat related thought, while we are at it...

It would arguably make sense to allow a NIC to set max TSO size that the
card is 
willing to support (rather than assume/enforce 64k). Some NICs support
bigger max 
TSO size today, but it is even more important to allow a NIC to limit
TSO to a smaller size.

One likely scenario where this feature is desirable is a system with highly
fragmented memory. 
In this case, the number of physical fragments per TSO frame could be always
so high that it will be cheaper 
(on a given platform) to copy the frame than to DMA it. 

In this case, 64k TSO becomes a liability and it will make sense to limit
it.
TSO "sweet spot" will be captured anyways - at least on fast networks, going

from 1.5k to 9k typically doubles throughput, while going from 9k to 64k
adds no more than another 10% 
(plus a little bit of free %cpu, but not that much).

> -----Original Message-----
> From: netdev-bounce@oss.sgi.com 
> [mailto:netdev-bounce@oss.sgi.com] On Behalf Of David S. Miller
> Sent: Tuesday, May 17, 2005 7:24 PM
> To: netdev@oss.sgi.com
> Subject: [PATCH] Super TSO
> 
> 
> Ok, this implements the idea Herbert and I kicked around last week.
> 
> Basically, we build as large TSO segments as humanly possible.
> Then we slice and dice them to fit the congestion and send 
> windows at transmit time.
> 
> It works remarkably well.  Even application limited cases 
> like "scp" behave nicely.
> 
> During straight streaming tests like "bw_tcp", I see full
> "65535 MOD mss" sized frames going out to my onboard tg3 card 
> once the congestion and send window grow large enough.
> 
> I did some cross-continent (San Francisco --> east coast of 
> US) transfers to make sure TSO stays on when packet drops 
> occur and that performance doesn't suffer compared to TSO 
> being off.  It all looks fine so far.
> 
> There were many bugs discovered along the way.  For example, 
> the old code would use the TSO mss to do nagle calculations.  
> It also would mess up the packet sizing for TSO when SACK 
> blocks were being advertised.
> 
> So we don't have to calculate this "(65535 - headers) % mss" 
> thing all the time, I keep a cache of three pieces of state.  
> MSS, number of SACKS being advertized, and if TSO is enabled 
> or not.  If any of these change, we recalculate the packet 
> building goal size.
> 
> I moved all of the "should we send" logic privately into the 
> file net/ipv4/tcp_output.c, and I am sure there are many 
> simplifications possible.  There are several spurious 
> tcp_current_mss() calls scattered about in certain code paths 
> as well.  For example, some places would call it in order to 
> decide whether to call tcp_write_xmit() or not, then 
> tcp_write_xmit() does it once more.  But there are call sites 
> that do not do this, so tcp_write_xmit() has to do it for those cases.
> 
> Comments?
> 
> --- 1/include/linux/tcp.h.~1~	2005-05-16 22:29:56.000000000 -0700
> +++ 2/include/linux/tcp.h	2005-05-17 11:15:46.000000000 -0700
> @@ -280,13 +280,15 @@ struct tcp_sock {
>  	__u32	snd_wnd;	/* The window we expect to 
> receive	*/
>  	__u32	max_window;	/* Maximal window ever seen 
> from peer	*/
>  	__u32	pmtu_cookie;	/* Last pmtu seen by socket	
> 	*/
> -	__u32	mss_cache;	/* Cached effective mss, not 
> including SACKS */
> -	__u16	mss_cache_std;	/* Like mss_cache, but without TSO */
> +	__u16	mss_cache;	/* Cached effective mss, not 
> including SACKS */
> +	__u16	xmit_size_goal;	/* Goal for segmenting output 
> packets	*/
> +	__u32	xmit_size_cache;/* Cache for keeping 
> xmit_size_goal uptodate */
>  	__u16	ext_header_len;	/* Network protocol overhead 
> (IP/IPv6 options) */
>  	__u8	ca_state;	/* State of fast-retransmit 
> machine 	*/
>  	__u8	retransmits;	/* Number of unrecovered RTO 
> timeouts.	*/
>  
>  	__u16	advmss;		/* Advertised MSS		
> 	*/
> +	__u16	__pad0;
>  	__u32	window_clamp;	/* Maximal window to advertise	
> 	*/
>  	__u32	rcv_ssthresh;	/* Current window clamp		
> 	*/
>  
> --- 1/include/net/tcp.h.~1~	2005-05-16 22:29:56.000000000 -0700
> +++ 2/include/net/tcp.h	2005-05-17 16:44:32.000000000 -0700
> @@ -817,11 +817,18 @@ static inline int tcp_ack_scheduled(stru
>  	return tp->ack.pending&TCP_ACK_SCHED;
>  }
>  
> -static __inline__ void tcp_dec_quickack_mode(struct tcp_sock *tp)
> +static __inline__ void tcp_dec_quickack_mode(struct tcp_sock *tp, 
> +unsigned int pkts)
>  {
> -	if (tp->ack.quick && --tp->ack.quick == 0) {
> -		/* Leaving quickack mode we deflate ATO. */
> -		tp->ack.ato = TCP_ATO_MIN;
> +	if (tp->ack.quick) {
> +		if (pkts > tp->ack.quick)
> +			tp->ack.quick = 0;
> +		else
> +			tp->ack.quick -= pkts;
> +
> +		if (!tp->ack.quick) {
> +			/* Leaving quickack mode we deflate ATO. */
> +			tp->ack.ato = TCP_ATO_MIN;
> +		}
>  	}
>  }
>  
> @@ -939,7 +946,14 @@ extern __u32 cookie_v4_init_sequence(str
>  
>  /* tcp_output.c */
>  
> -extern int tcp_write_xmit(struct sock *, int nonagle);
> +extern void __tcp_data_snd_check(struct sock *sk, struct 
> sk_buff *skb); 
> +extern void __tcp_push_pending_frames(struct sock *sk,
> +				      struct tcp_sock *tp,
> +				      unsigned int cur_mss,
> +				      int nonagle);
> +extern int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp); 
> +extern int tcp_tso_mince(struct sock *sk, struct tcp_sock *tp,
> +			 struct sk_buff *skb);
>  extern int tcp_retransmit_skb(struct sock *, struct sk_buff 
> *);  extern void tcp_xmit_retransmit_queue(struct sock *);  
> extern void tcp_simple_retransmit(struct sock *); @@ -951,7 
> +965,7 @@ extern int  tcp_write_wakeup(struct sock  extern 
> void tcp_send_fin(struct sock *sk);  extern void 
> tcp_send_active_reset(struct sock *sk, int priority);  extern 
> int  tcp_send_synack(struct sock *); -extern void 
> tcp_push_one(struct sock *, unsigned mss_now);
> +extern void tcp_push_one(struct sock *, unsigned int mss_now);
>  extern void tcp_send_ack(struct sock *sk);  extern void 
> tcp_send_delayed_ack(struct sock *sk);
>  
> @@ -1054,7 +1068,7 @@ static inline void 
> tcp_reset_xmit_timer(  static inline void 
> tcp_initialize_rcv_mss(struct sock *sk)  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> -	unsigned int hint = min(tp->advmss, tp->mss_cache_std);
> +	unsigned int hint = min(tp->advmss, tp->mss_cache);
>  
>  	hint = min(hint, tp->rcv_wnd/2);
>  	hint = min(hint, TCP_MIN_RCVMSS);
> @@ -1353,23 +1367,23 @@ static inline void tcp_cwnd_validate(str  }
>  
>  /* Set slow start threshould and cwnd not falling to slow 
> start */ -static inline void __tcp_enter_cwr(struct tcp_sock *tp)
> +static inline void __tcp_enter_cwr(struct tcp_sock *tp, unsigned int 
> +pkts)
>  {
>  	tp->undo_marker = 0;
>  	tp->snd_ssthresh = tcp_recalc_ssthresh(tp);
>  	tp->snd_cwnd = min(tp->snd_cwnd,
> -			   tcp_packets_in_flight(tp) + 1U);
> +			   tcp_packets_in_flight(tp) + pkts);
>  	tp->snd_cwnd_cnt = 0;
>  	tp->high_seq = tp->snd_nxt;
>  	tp->snd_cwnd_stamp = tcp_time_stamp;
>  	TCP_ECN_queue_cwr(tp);
>  }
>  
> -static inline void tcp_enter_cwr(struct tcp_sock *tp)
> +static inline void tcp_enter_cwr(struct tcp_sock *tp, unsigned int 
> +pkts)
>  {
>  	tp->prior_ssthresh = 0;
>  	if (tp->ca_state < TCP_CA_CWR) {
> -		__tcp_enter_cwr(tp);
> +		__tcp_enter_cwr(tp, pkts);
>  		tcp_set_ca_state(tp, TCP_CA_CWR);
>  	}
>  }
> @@ -1397,74 +1411,6 @@ static __inline__ void tcp_minshall_upda
>  		tp->snd_sml = TCP_SKB_CB(skb)->end_seq;  }
>  
> -/* Return 0, if packet can be sent now without violation 
> Nagle's rules:
> -   1. It is full sized.
> -   2. Or it contains FIN.
> -   3. Or TCP_NODELAY was set.
> -   4. Or TCP_CORK is not set, and all sent packets are ACKed.
> -      With Minshall's modification: all sent small packets are ACKed.
> - */
> -
> -static __inline__ int
> -tcp_nagle_check(const struct tcp_sock *tp, const struct 
> sk_buff *skb, 
> -		unsigned mss_now, int nonagle)
> -{
> -	return (skb->len < mss_now &&
> -		!(TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN) &&
> -		((nonagle&TCP_NAGLE_CORK) ||
> -		 (!nonagle &&
> -		  tp->packets_out &&
> -		  tcp_minshall_check(tp))));
> -}
> -
> -extern void tcp_set_skb_tso_segs(struct sock *, struct sk_buff *);
> -
> -/* This checks if the data bearing packet SKB (usually 
> sk->sk_send_head)
> - * should be put on the wire right now.
> - */
> -static __inline__ int tcp_snd_test(struct sock *sk,
> -				   struct sk_buff *skb,
> -				   unsigned cur_mss, int nonagle)
> -{
> -	struct tcp_sock *tp = tcp_sk(sk);
> -	int pkts = tcp_skb_pcount(skb);
> -
> -	if (!pkts) {
> -		tcp_set_skb_tso_segs(sk, skb);
> -		pkts = tcp_skb_pcount(skb);
> -	}
> -
> -	/*	RFC 1122 - section 4.2.3.4
> -	 *
> -	 *	We must queue if
> -	 *
> -	 *	a) The right edge of this frame exceeds the window
> -	 *	b) There are packets in flight and we have a 
> small segment
> -	 *	   [SWS avoidance and Nagle algorithm]
> -	 *	   (part of SWS is done on packetization)
> -	 *	   Minshall version sounds: there are no _small_
> -	 *	   segments in flight. (tcp_nagle_check)
> -	 *	c) We have too many packets 'in flight'
> -	 *
> -	 * 	Don't use the nagle rule for urgent data (or
> -	 *	for the final FIN -DaveM).
> -	 *
> -	 *	Also, Nagle rule does not apply to frames, which
> -	 *	sit in the middle of queue (they have no chances
> -	 *	to get new data) and if room at tail of skb is
> -	 *	not enough to save something seriously (<32 for now).
> -	 */
> -
> -	/* Don't be strict about the congestion window for the
> -	 * final FIN frame.  -DaveM
> -	 */
> -	return (((nonagle&TCP_NAGLE_PUSH) || tp->urg_mode
> -		 || !tcp_nagle_check(tp, skb, cur_mss, nonagle)) &&
> -		(((tcp_packets_in_flight(tp) + (pkts-1)) < 
> tp->snd_cwnd) ||
> -		 (TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN)) &&
> -		!after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + 
> tp->snd_wnd));
> -}
> -
>  static __inline__ void tcp_check_probe_timer(struct sock 
> *sk, struct tcp_sock *tp)  {
>  	if (!tp->packets_out && !tp->pending)
> @@ -1477,42 +1423,12 @@ static __inline__ int tcp_skb_is_last(co
>  	return skb->next == (struct sk_buff *)&sk->sk_write_queue;  }
>  
> -/* Push out any pending frames which were held back due to
> - * TCP_CORK or attempt at coalescing tiny packets.
> - * The socket must be locked by the caller.
> - */
> -static __inline__ void __tcp_push_pending_frames(struct sock *sk,
> -						 struct tcp_sock *tp,
> -						 unsigned cur_mss,
> -						 int nonagle)
> -{
> -	struct sk_buff *skb = sk->sk_send_head;
> -
> -	if (skb) {
> -		if (!tcp_skb_is_last(sk, skb))
> -			nonagle = TCP_NAGLE_PUSH;
> -		if (!tcp_snd_test(sk, skb, cur_mss, nonagle) ||
> -		    tcp_write_xmit(sk, nonagle))
> -			tcp_check_probe_timer(sk, tp);
> -	}
> -	tcp_cwnd_validate(sk, tp);
> -}
> -
>  static __inline__ void tcp_push_pending_frames(struct sock *sk,
>  					       struct tcp_sock *tp)
>  {
>  	__tcp_push_pending_frames(sk, tp, tcp_current_mss(sk, 
> 1), tp->nonagle);  }
>  
> -static __inline__ int tcp_may_send_now(struct sock *sk, 
> struct tcp_sock *tp) -{
> -	struct sk_buff *skb = sk->sk_send_head;
> -
> -	return (skb &&
> -		tcp_snd_test(sk, skb, tcp_current_mss(sk, 1),
> -			     tcp_skb_is_last(sk, skb) ? 
> TCP_NAGLE_PUSH : tp->nonagle));
> -}
> -
>  static __inline__ void tcp_init_wl(struct tcp_sock *tp, u32 
> ack, u32 seq)  {
>  	tp->snd_wl1 = seq;
> @@ -1986,7 +1902,7 @@ static inline void 
> tcp_westwood_update_r  static inline __u32 
> __tcp_westwood_bw_rttmin(const struct tcp_sock *tp)  {
>          return max((tp->westwood.bw_est) * (tp->westwood.rtt_min) /
> -		   (__u32) (tp->mss_cache_std),
> +		   (__u32) (tp->mss_cache),
>  		   2U);
>  }
>  
> --- 1/include/net/sock.h.~1~	2005-05-16 22:29:56.000000000 -0700
> +++ 2/include/net/sock.h	2005-05-17 12:06:44.000000000 -0700
> @@ -1130,13 +1130,16 @@ static inline void 
> sk_stream_moderate_sn  static inline struct sk_buff 
> *sk_stream_alloc_pskb(struct sock *sk,
>  						   int size, 
> int mem, int gfp)
>  {
> -	struct sk_buff *skb = alloc_skb(size + 
> sk->sk_prot->max_header, gfp);
> +	struct sk_buff *skb;
> +	int hdr_len;
>  
> +	hdr_len = SKB_DATA_ALIGN(sk->sk_prot->max_header);
> +	skb = alloc_skb(size + hdr_len, gfp);
>  	if (skb) {
>  		skb->truesize += mem;
>  		if (sk->sk_forward_alloc >= (int)skb->truesize ||
>  		    sk_stream_mem_schedule(sk, skb->truesize, 0)) {
> -			skb_reserve(skb, sk->sk_prot->max_header);
> +			skb_reserve(skb, hdr_len);
>  			return skb;
>  		}
>  		__kfree_skb(skb);
> --- 1/net/ipv4/tcp_output.c.~1~	2005-05-16 
> 22:29:56.000000000 -0700
> +++ 2/net/ipv4/tcp_output.c	2005-05-17 19:14:23.000000000 -0700
> @@ -141,11 +141,11 @@ static inline void tcp_event_data_sent(s
>  		tp->ack.pingpong = 1;
>  }
>  
> -static __inline__ void tcp_event_ack_sent(struct sock *sk)
> +static __inline__ void tcp_event_ack_sent(struct sock *sk, 
> unsigned int 
> +pkts)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
>  
> -	tcp_dec_quickack_mode(tp);
> +	tcp_dec_quickack_mode(tp, pkts);
>  	tcp_clear_xmit_timer(sk, TCP_TIME_DACK);  }
>  
> @@ -361,7 +361,7 @@ static int tcp_transmit_skb(struct sock 
>  		tp->af_specific->send_check(sk, th, skb->len, skb);
>  
>  		if (tcb->flags & TCPCB_FLAG_ACK)
> -			tcp_event_ack_sent(sk);
> +			tcp_event_ack_sent(sk, tcp_skb_pcount(skb));
>  
>  		if (skb->len != tcp_header_size)
>  			tcp_event_data_sent(tp, skb, sk);
> @@ -372,7 +372,7 @@ static int tcp_transmit_skb(struct sock 
>  		if (err <= 0)
>  			return err;
>  
> -		tcp_enter_cwr(tp);
> +		tcp_enter_cwr(tp, tcp_skb_pcount(skb));
>  
>  		/* NET_XMIT_CN is special. It does not guarantee,
>  		 * that this packet is lost. It tells that 
> device @@ -419,32 +419,11 @@ static inline void tcp_tso_set_push(stru
>  		TCP_SKB_CB(skb)->flags |= TCPCB_FLAG_PSH;  }
>  
> -/* Send _single_ skb sitting at the send head. This function requires
> - * true push pending frames to setup probe timer etc.
> - */
> -void tcp_push_one(struct sock *sk, unsigned cur_mss) -{
> -	struct tcp_sock *tp = tcp_sk(sk);
> -	struct sk_buff *skb = sk->sk_send_head;
> -
> -	if (tcp_snd_test(sk, skb, cur_mss, TCP_NAGLE_PUSH)) {
> -		/* Send it out now. */
> -		TCP_SKB_CB(skb)->when = tcp_time_stamp;
> -		tcp_tso_set_push(skb);
> -		if (!tcp_transmit_skb(sk, skb_clone(skb, 
> sk->sk_allocation))) {
> -			sk->sk_send_head = NULL;
> -			tp->snd_nxt = TCP_SKB_CB(skb)->end_seq;
> -			tcp_packets_out_inc(sk, tp, skb);
> -			return;
> -		}
> -	}
> -}
> -
>  void tcp_set_skb_tso_segs(struct sock *sk, struct sk_buff *skb)  {
>  	struct tcp_sock *tp = tcp_sk(sk);
>  
> -	if (skb->len <= tp->mss_cache_std ||
> +	if (skb->len <= tp->mss_cache ||
>  	    !(sk->sk_route_caps & NETIF_F_TSO)) {
>  		/* Avoid the costly divide in the normal
>  		 * non-TSO case.
> @@ -454,10 +433,10 @@ void tcp_set_skb_tso_segs(struct sock *s
>  	} else {
>  		unsigned int factor;
>  
> -		factor = skb->len + (tp->mss_cache_std - 1);
> -		factor /= tp->mss_cache_std;
> +		factor = skb->len + (tp->mss_cache - 1);
> +		factor /= tp->mss_cache;
>  		skb_shinfo(skb)->tso_segs = factor;
> -		skb_shinfo(skb)->tso_size = tp->mss_cache_std;
> +		skb_shinfo(skb)->tso_size = tp->mss_cache;
>  	}
>  }
>  
> @@ -662,7 +641,7 @@ unsigned int tcp_sync_mss(struct sock *s
>  
>  	/* And store cached results */
>  	tp->pmtu_cookie = pmtu;
> -	tp->mss_cache = tp->mss_cache_std = mss_now;
> +	tp->mss_cache = mss_now;
>  
>  	return mss_now;
>  }
> @@ -674,59 +653,274 @@ unsigned int tcp_sync_mss(struct sock *s
>   * cannot be large. However, taking into account rare use of 
> URG, this
>   * is not a big flaw.
>   */
> +static inline u32 compute_xmit_cache(u32 mss, int sacks, int 
> +tso_enabled) {
> +	u32 ret = (mss << 16) | sacks;
> +
> +	if (tso_enabled)
> +		ret |= (1 << 8);
> +
> +	return ret;
> +}
>  
> -unsigned int tcp_current_mss(struct sock *sk, int large)
> +unsigned int tcp_current_mss(struct sock *sk, int large_allowed)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
>  	struct dst_entry *dst = __sk_dst_get(sk);
> -	unsigned int do_large, mss_now;
> +	u32 mss_now;
> +	u32 xmit_cache;
> +	int doing_tso = 0;
> +
> +	mss_now = tp->mss_cache;
> +
> +	if (large_allowed &&
> +	    (sk->sk_route_caps & NETIF_F_TSO) &&
> +	    !tp->urg_mode)
> +		doing_tso = 1;
>  
> -	mss_now = tp->mss_cache_std;
>  	if (dst) {
>  		u32 mtu = dst_mtu(dst);
>  		if (mtu != tp->pmtu_cookie)
>  			mss_now = tcp_sync_mss(sk, mtu);
>  	}
>  
> -	do_large = (large &&
> -		    (sk->sk_route_caps & NETIF_F_TSO) &&
> -		    !tp->urg_mode);
> -
> -	if (do_large) {
> -		unsigned int large_mss, factor, limit;
> -
> -		large_mss = 65535 - tp->af_specific->net_header_len -
> -			tp->ext_header_len - tp->tcp_header_len;
> -
> -		if (tp->max_window && large_mss > (tp->max_window>>1))
> -			large_mss = max((tp->max_window>>1),
> -					68U - tp->tcp_header_len);
> -
> -		factor = large_mss / mss_now;
> -
> -		/* Always keep large mss multiple of real mss, but
> -		 * do not exceed 1/tso_win_divisor of the 
> congestion window
> -		 * so we can keep the ACK clock ticking and minimize
> -		 * bursting.
> -		 */
> -		limit = tp->snd_cwnd;
> -		if (sysctl_tcp_tso_win_divisor)
> -			limit /= sysctl_tcp_tso_win_divisor;
> -		limit = max(1U, limit);
> -		if (factor > limit)
> -			factor = limit;
> -
> -		tp->mss_cache = mss_now * factor;
> -
> -		mss_now = tp->mss_cache;
> -	}
> +	/* If the MSS, the TSO state, or the number of SACK blocks
> +	 * changes, we have to recompute tp->xmit_size_goal.
> +	 */
> +	xmit_cache = compute_xmit_cache(mss_now, tp->rx_opt.eff_sacks,
> +					doing_tso);
>  
>  	if (tp->rx_opt.eff_sacks)
>  		mss_now -= (TCPOLEN_SACK_BASE_ALIGNED +
>  			    (tp->rx_opt.eff_sacks * 
> TCPOLEN_SACK_PERBLOCK));
> +
> +	if (tp->xmit_size_cache != xmit_cache) {
> +		u16 xmit_size_goal = mss_now;
> +
> +		if (doing_tso) {
> +			xmit_size_goal = 65535 -
> +				tp->af_specific->net_header_len -
> +				tp->ext_header_len - tp->tcp_header_len;
> +
> +			if (tp->rx_opt.eff_sacks)
> +				xmit_size_goal -= 
> (TCPOLEN_SACK_BASE_ALIGNED +
> +						   
> (tp->rx_opt.eff_sacks *
> +						    
> TCPOLEN_SACK_PERBLOCK));
> +
> +			xmit_size_goal -= (xmit_size_goal % mss_now);
> +		}
> +		tp->xmit_size_goal = xmit_size_goal;
> +		tp->xmit_size_cache = xmit_cache;
> +	}
> +
>  	return mss_now;
>  }
>  
> +/* This must be invoked the first time we consider transmitting
> + * SKB onto the wire.
> + */
> +static inline int tcp_init_tso_segs(struct sock *sk, struct sk_buff 
> +*skb) {
> +	int tso_segs = tcp_skb_pcount(skb);
> +
> +	if (!tso_segs) {
> +		tcp_set_skb_tso_segs(sk, skb);
> +		tso_segs = tcp_skb_pcount(skb);
> +	}
> +	return tso_segs;
> +}
> +
> +/* Return 0, if packet can be sent now without violation 
> Nagle's rules:
> + * 1. It is full sized.
> + * 2. Or it contains FIN.
> + * 3. Or TCP_NODELAY was set.
> + * 4. Or TCP_CORK is not set, and all sent packets are ACKed.
> + *    With Minshall's modification: all sent small packets are ACKed.
> + */
> +
> +static inline int tcp_nagle_check(const struct tcp_sock *tp,
> +				  const struct sk_buff *skb, 
> +				  unsigned mss_now, int nonagle)
> +{
> +	return (skb->len < mss_now &&
> +		!(TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN) &&
> +		((nonagle&TCP_NAGLE_CORK) ||
> +		 (!nonagle &&
> +		  tp->packets_out &&
> +		  tcp_minshall_check(tp))));
> +}
> +
> +/* Return non-zero if the Nagle test allows this packet to be
> + * sent now.
> + */
> +static inline int tcp_nagle_test(struct tcp_sock *tp, struct sk_buff 
> +*skb, unsigned int cur_mss, int nonagle) {
> +	/* Nagle rule does not apply to frames, which
> +	 * sit in the middle of queue (they have no chances
> +	 * to get new data) and if room at tail of skb is
> +	 * not enough to save something seriously (<32 for now).
> +	 *
> +	 * This is implemented in the callers, where they modify
> +	 * the 'nonagle' argument based upon the location of SKB
> +	 * in the send queue.
> +	 */
> +	if (nonagle & TCP_NAGLE_PUSH)
> +		return 1;
> +
> +	/* Don't use the nagle rule for urgent data (or
> +	 * for the final FIN -DaveM).
> +	 */
> +	if (tp->urg_mode ||
> +	    (TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN))
> +		return 1;
> +
> +	if (!tcp_nagle_check(tp, skb, cur_mss, nonagle))
> +		return 1;
> +
> +	return 0;
> +}
> +
> +/* Can at least one segment of SKB be sent right now, according
> + * to the congestion window rules?  If so, return how many segments
> + * are allowed.
> + */
> +static inline unsigned int tcp_cwnd_test(struct tcp_sock *tp, struct 
> +sk_buff *skb) {
> +	u32 in_flight, cwnd;
> +
> +	/* Don't be strict about the congestion window for the
> +	 * final FIN frame.  -DaveM
> +	 */
> +	if (TCP_SKB_CB(skb)->flags & TCPCB_FLAG_FIN)
> +		return 1;
> +
> +	in_flight = tcp_packets_in_flight(tp);
> +	cwnd = tp->snd_cwnd;
> +	if (in_flight < cwnd)
> +		return (cwnd - in_flight);
> +
> +	return 0;
> +}
> +
> +/* Does at least the first segment of SKB fit into the congestion
> + * window?
> + */
> +static inline int tcp_snd_wnd_test(struct tcp_sock *tp, 
> struct sk_buff 
> +*skb, unsigned int cur_mss) {
> +	u32 end_seq = TCP_SKB_CB(skb)->end_seq;
> +
> +	if (skb->len > cur_mss)
> +		end_seq = TCP_SKB_CB(skb)->seq + cur_mss;
> +
> +	return !after(end_seq, tp->snd_una + tp->snd_wnd); }
> +
> +/* This checks if the data bearing packet SKB (usually
> + * sk->sk_send_head) should be put on the wire right now.  If so, it
> + * returns the number of packets allowed by the congestion window.
> + */
> +static unsigned int tcp_snd_test(struct sock *sk, struct 
> sk_buff *skb,
> +				 unsigned cur_mss, int nonagle)
> +{
> +	struct tcp_sock *tp = tcp_sk(sk);
> +	unsigned int cwnd_quota;
> +
> +	if (!tcp_nagle_test(tp, skb, cur_mss, nonagle))
> +		return 0;
> +
> +	cwnd_quota = tcp_cwnd_test(tp, skb);
> +	if (cwnd_quota &&
> +	    !tcp_snd_wnd_test(tp, skb, cur_mss))
> +		cwnd_quota = 0;
> +
> +	return cwnd_quota;
> +}
> +
> +int tcp_may_send_now(struct sock *sk, struct tcp_sock *tp) {
> +	struct sk_buff *skb = sk->sk_send_head;
> +
> +	return (skb &&
> +		tcp_snd_test(sk, skb, tcp_current_mss(sk, 1),
> +			     tcp_skb_is_last(sk, skb) ? 
> TCP_NAGLE_PUSH : tp->nonagle)); }
> +
> +/* Trim TSO SKB to LEN bytes, put the remaining data into a 
> new packet
> + * which is put after SKB on the list.  It is very much like
> + * tcp_fragment() except that it may make several kinds of 
> assumptions
> + * in order to speed up the splitting operation.  In particular, we
> + * know that all the data is in scatter-gather pages, and that the
> + * packet has never been sent out before (and thus is not cloned).
> + */
> +static int tso_fragment(struct sock *sk, struct sk_buff 
> *skb, unsigned 
> +int len) {
> +	struct sk_buff *buff;
> +	int nlen = skb->len - len;
> +	u16 flags;
> +
> +	/* All of a TSO frame must be composed of paged data.  */
> +	BUG_ON(skb->len != skb->data_len);
> +
> +	buff = sk_stream_alloc_pskb(sk, 0, 0, GFP_ATOMIC);
> +	if (unlikely(buff == NULL))
> +		return -ENOMEM;
> +	sk_charge_skb(sk, buff);
> +
> +	buff->truesize += nlen;
> +	skb->truesize -= nlen;
> +
> +	/* Correct the sequence numbers. */
> +	TCP_SKB_CB(buff)->seq = TCP_SKB_CB(skb)->seq + len;
> +	TCP_SKB_CB(buff)->end_seq = TCP_SKB_CB(skb)->end_seq;
> +	TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(buff)->seq;
> +
> +	/* PSH and FIN should only be set in the second packet. */
> +	flags = TCP_SKB_CB(skb)->flags;
> +	TCP_SKB_CB(skb)->flags = flags & 
> ~(TCPCB_FLAG_FIN|TCPCB_FLAG_PSH);
> +	TCP_SKB_CB(buff)->flags = flags;
> +
> +	/* This packet was never sent out yet, so no SACK bits. */
> +	TCP_SKB_CB(buff)->sacked = 0;
> +
> +	buff->ip_summed = skb->ip_summed = CHECKSUM_HW;
> +	skb_split(skb, buff, len);
> +
> +	/* Fix up tso_factor for both original and new SKB.  */
> +	tcp_set_skb_tso_segs(sk, skb);
> +	tcp_set_skb_tso_segs(sk, buff);
> +
> +	/* Link BUFF into the send queue. */
> +	__skb_append(skb, buff);
> +
> +	return 0;
> +}
> +
> +/* Split TSO frame SKB into one MSS sized packet, then the rest.
> + * This is called during SACK processing when some SACK information
> + * hits a TSO packet.
> + */
> +int tcp_tso_mince(struct sock *sk, struct tcp_sock *tp, 
> struct sk_buff 
> +*skb) {
> +	unsigned int mss_now = tcp_current_mss(sk, 1);
> +
> +	BUG_ON(tcp_skb_pcount(skb) <= 1);
> +
> +	/* We cannot use tso_fragment() in this case, because
> +	 * the packet has been sent once already and thus may
> +	 * be cloned or have other non-trivial details to deal
> +	 * with.
> +	 */
> +	return tcp_fragment(sk, skb, mss_now); }
> +
> +static inline unsigned int tso_size_limit(u32 window, unsigned int 
> +mss_now, unsigned int cwnd) {
> +	u32 cwnd_len;
> +
> +	cwnd_len = mss_now * cwnd;
> +	return min(window, cwnd_len);
> +}
> +
>  /* This routine writes packets to the network.  It advances the
>   * send_head.  This happens as incoming acks open up the remote
>   * window for us.
> @@ -734,57 +928,167 @@ unsigned int tcp_current_mss(struct sock
>   * Returns 1, if no segments are in flight and we have 
> queued segments, but
>   * cannot send anything now because of SWS or another problem.
>   */
> -int tcp_write_xmit(struct sock *sk, int nonagle)
> +static int tcp_write_xmit(struct sock *sk, int nonagle)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> -	unsigned int mss_now;
> +	unsigned int mss_now, cwnd_quota, sent_pkts, tso_segs;
> +	struct sk_buff *skb;
>  
>  	/* If we are closed, the bytes will have to remain here.
>  	 * In time closedown will finish, we empty the write 
> queue and all
>  	 * will be happy.
>  	 */
> -	if (sk->sk_state != TCP_CLOSE) {
> -		struct sk_buff *skb;
> -		int sent_pkts = 0;
> +	if (unlikely(sk->sk_state == TCP_CLOSE))
> +		return 0;
>  
> -		/* Account for SACKS, we may need to fragment 
> due to this.
> -		 * It is just like the real MSS changing on us 
> midstream.
> -		 * We also handle things correctly when the 
> user adds some
> -		 * IP options mid-stream.  Silly to do, but cover it.
> -		 */
> -		mss_now = tcp_current_mss(sk, 1);
> +	/* Account for SACKS, we may need to fragment due to this.
> +	 * It is just like the real MSS changing on us midstream.
> +	 * We also handle things correctly when the user adds some
> +	 * IP options mid-stream.  Silly to do, but cover it.
> +	 */
> +	mss_now = tcp_current_mss(sk, 1);
> +	skb = sk->sk_send_head;
> +	if (unlikely(!skb))
> +		return 0;
> +
> +	tso_segs = tcp_init_tso_segs(sk, skb);
> +	cwnd_quota = tcp_cwnd_test(tp, skb);
> +	sent_pkts = 0;
> +
> +	while (cwnd_quota) {
> +		u32 end_seq, window_seq;
> +
> +		if (!tcp_nagle_test(tp, skb, mss_now,
> +				    (tcp_skb_is_last(sk, skb) ?
> +				     nonagle : TCP_NAGLE_PUSH)))
> +			break;
> +
> +		end_seq = TCP_SKB_CB(skb)->end_seq;
> +		window_seq = tp->snd_una + tp->snd_wnd;
> +		if (skb->len > mss_now)
> +			end_seq = TCP_SKB_CB(skb)->seq + mss_now;
> +		if (after(end_seq, window_seq))
> +			break;
> +
> +		BUG_ON(!tso_segs);
> +
> +		if (tso_segs > 1) {
> +			u32 limit = tso_size_limit(window_seq -
> +						   TCP_SKB_CB(skb)->seq,
> +						   mss_now, cwnd_quota);
>  
> -		while ((skb = sk->sk_send_head) &&
> -		       tcp_snd_test(sk, skb, mss_now,
> -			       	    tcp_skb_is_last(sk, skb) ? nonagle :
> -				    			       
> TCP_NAGLE_PUSH)) {
> -			if (skb->len > mss_now) {
> -				if (tcp_fragment(sk, skb, mss_now))
> +			if (skb->len > limit) {
> +				if (tso_fragment(sk, skb, limit))
>  					break;
>  			}
> -
> -			TCP_SKB_CB(skb)->when = tcp_time_stamp;
> -			tcp_tso_set_push(skb);
> -			if (tcp_transmit_skb(sk, skb_clone(skb, 
> GFP_ATOMIC)))
> +		} else if (skb->len > mss_now) {
> +			if (tcp_fragment(sk, skb, mss_now))
>  				break;
> +		}
>  
> -			/* Advance the send_head.  This one is sent out.
> -			 * This call will increment packets_out.
> -			 */
> -			update_send_head(sk, tp, skb);
> +		TCP_SKB_CB(skb)->when = tcp_time_stamp;
> +		tcp_tso_set_push(skb);
> +		if (tcp_transmit_skb(sk, skb_clone(skb, GFP_ATOMIC)))
> +			break;
>  
> -			tcp_minshall_update(tp, mss_now, skb);
> -			sent_pkts = 1;
> -		}
> +		/* Advance the send_head.  This one is sent out.
> +		 * This call will increment packets_out.
> +		 */
> +		update_send_head(sk, tp, skb);
>  
> -		if (sent_pkts) {
> -			tcp_cwnd_validate(sk, tp);
> -			return 0;
> +		tcp_minshall_update(tp, mss_now, skb);
> +		sent_pkts++;
> +
> +		cwnd_quota -= tcp_skb_pcount(skb);
> +		skb = sk->sk_send_head;
> +		if (!skb)
> +			break;
> +		tso_segs = tcp_init_tso_segs(sk, skb);
> +	}
> +
> +	if (sent_pkts) {
> +		tcp_cwnd_validate(sk, tp);
> +		return 0;
> +	}
> +
> +	return !tp->packets_out && sk->sk_send_head; }
> +
> +/* Push out any pending frames which were held back due to
> + * TCP_CORK or attempt at coalescing tiny packets.
> + * The socket must be locked by the caller.
> + */
> +void __tcp_push_pending_frames(struct sock *sk,
> +			       struct tcp_sock *tp,
> +			       unsigned int cur_mss,
> +			       int nonagle)
> +{
> +	struct sk_buff *skb = sk->sk_send_head;
> +
> +	if (skb) {
> +		if (!tcp_skb_is_last(sk, skb))
> +			nonagle = TCP_NAGLE_PUSH;
> +		if (tcp_write_xmit(sk, nonagle))
> +			tcp_check_probe_timer(sk, tp);
> +	}
> +	tcp_cwnd_validate(sk, tp);
> +}
> +
> +/* As ACKs arrive and the send and congestion windows potentially
> + * open up, we call this to try and make write queue transmit
> + * progress.
> + *
> + * The caller has the socket locked, and has verified that
> + * sk->sk_send_head is not NULL.
> + */
> +void __tcp_data_snd_check(struct sock *sk, struct sk_buff *skb) {
> +	struct tcp_sock *tp = tcp_sk(sk);
> +
> +	if (tcp_write_xmit(sk, tp->nonagle))
> +		tcp_check_probe_timer(sk, tp);
> +}
> +
> +/* Send _single_ skb sitting at the send head. This function requires
> + * true push pending frames to setup probe timer etc.  Caller makes
> + * sure that sk->sk_send_head is non-NULL.
> + */
> +void tcp_push_one(struct sock *sk, unsigned int cur_mss) {
> +	struct tcp_sock *tp = tcp_sk(sk);
> +	struct sk_buff *skb = sk->sk_send_head;
> +	unsigned int tso_segs, cwnd_quota;
> +
> +	tso_segs = tcp_init_tso_segs(sk, skb);
> +	cwnd_quota = tcp_snd_test(sk, skb, cur_mss, TCP_NAGLE_PUSH);
> +	if (cwnd_quota) {
> +		u32 window_seq;
> +
> +		window_seq = tp->snd_una + tp->snd_wnd;
> +		BUG_ON(!tso_segs);
> +
> +		if (tso_segs > 1) {
> +			u32 limit = tso_size_limit(window_seq -
> +						   TCP_SKB_CB(skb)->seq,
> +						   cur_mss, cwnd_quota);
> +
> +			if (skb->len > limit) {
> +				if (tso_fragment(sk, skb, limit))
> +					return;
> +			}
> +		} else if (skb->len > cur_mss) {
> +			if (tcp_fragment(sk, skb, cur_mss))
> +				return;
>  		}
>  
> -		return !tp->packets_out && sk->sk_send_head;
> +		/* Send it out now. */
> +		TCP_SKB_CB(skb)->when = tcp_time_stamp;
> +		tcp_tso_set_push(skb);
> +		if (!tcp_transmit_skb(sk, skb_clone(skb, 
> sk->sk_allocation))) {
> +			update_send_head(sk, tp, skb);
> +			return;
> +		}
>  	}
> -	return 0;
>  }
>  
>  /* This function returns the amount that we can raise the @@ 
> -1041,12 +1345,6 @@ int tcp_retransmit_skb(struct sock *sk, 
>  		if (before(TCP_SKB_CB(skb)->end_seq, tp->snd_una))
>  			BUG();
>  
> -		if (sk->sk_route_caps & NETIF_F_TSO) {
> -			sk->sk_route_caps &= ~NETIF_F_TSO;
> -			sock_set_flag(sk, SOCK_NO_LARGESEND);
> -			tp->mss_cache = tp->mss_cache_std;
> -		}
> -
>  		if (tcp_trim_head(sk, skb, tp->snd_una - 
> TCP_SKB_CB(skb)->seq))
>  			return -ENOMEM;
>  	}
> @@ -1671,13 +1969,6 @@ int tcp_write_wakeup(struct sock *sk)
>  				TCP_SKB_CB(skb)->flags |= 
> TCPCB_FLAG_PSH;
>  				if (tcp_fragment(sk, skb, seg_size))
>  					return -1;
> -				/* SWS override triggered 
> forced fragmentation.
> -				 * Disable TSO, the connection 
> is too sick. */
> -				if (sk->sk_route_caps & NETIF_F_TSO) {
> -					sock_set_flag(sk, 
> SOCK_NO_LARGESEND);
> -					sk->sk_route_caps &= 
> ~NETIF_F_TSO;
> -					tp->mss_cache = 
> tp->mss_cache_std;
> -				}
>  			} else if (!tcp_skb_pcount(skb))
>  				tcp_set_skb_tso_segs(sk, skb);
>  
> --- 1/net/ipv4/tcp.c.~1~	2005-05-16 22:29:56.000000000 -0700
> +++ 2/net/ipv4/tcp.c	2005-05-17 12:07:26.000000000 -0700
> @@ -634,7 +634,7 @@ static ssize_t do_tcp_sendpages(struct s
>  			 size_t psize, int flags)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
> -	int mss_now;
> +	int mss_now, size_goal;
>  	int err;
>  	ssize_t copied;
>  	long timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT); 
> @@ -647,6 +647,7 @@ static ssize_t do_tcp_sendpages(struct s
>  	clear_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
>  
>  	mss_now = tcp_current_mss(sk, !(flags&MSG_OOB));
> +	size_goal = tp->xmit_size_goal;
>  	copied = 0;
>  
>  	err = -EPIPE;
> @@ -660,7 +661,7 @@ static ssize_t do_tcp_sendpages(struct s
>  		int offset = poffset % PAGE_SIZE;
>  		int size = min_t(size_t, psize, PAGE_SIZE - offset);
>  
> -		if (!sk->sk_send_head || (copy = mss_now - 
> skb->len) <= 0) {
> +		if (!sk->sk_send_head || (copy = size_goal - 
> skb->len) <= 0) {
>  new_segment:
>  			if (!sk_stream_memory_free(sk))
>  				goto wait_for_sndbuf;
> @@ -671,7 +672,7 @@ new_segment:
>  				goto wait_for_memory;
>  
>  			skb_entail(sk, tp, skb);
> -			copy = mss_now;
> +			copy = size_goal;
>  		}
>  
>  		if (copy > size)
> @@ -712,7 +713,7 @@ new_segment:
>  		if (!(psize -= copy))
>  			goto out;
>  
> -		if (skb->len != mss_now || (flags & MSG_OOB))
> +		if (skb->len != size_goal || (flags & MSG_OOB))
>  			continue;
>  
>  		if (forced_push(tp)) {
> @@ -732,6 +733,7 @@ wait_for_memory:
>  			goto do_error;
>  
>  		mss_now = tcp_current_mss(sk, !(flags&MSG_OOB));
> +		size_goal = tp->xmit_size_goal;
>  	}
>  
>  out:
> @@ -773,15 +775,11 @@ ssize_t tcp_sendpage(struct socket *sock
>  
>  static inline int select_size(struct sock *sk, struct 
> tcp_sock *tp)  {
> -	int tmp = tp->mss_cache_std;
> +	int tmp = tp->mss_cache;
>  
> -	if (sk->sk_route_caps & NETIF_F_SG) {
> -		int pgbreak = SKB_MAX_HEAD(MAX_TCP_HEADER);
> +	if (sk->sk_route_caps & NETIF_F_SG)
> +		tmp = 0;
>  
> -		if (tmp >= pgbreak &&
> -		    tmp <= pgbreak + (MAX_SKB_FRAGS - 1) * PAGE_SIZE)
> -			tmp = pgbreak;
> -	}
>  	return tmp;
>  }
>  
> @@ -792,7 +790,7 @@ int tcp_sendmsg(struct kiocb *iocb, stru
>  	struct tcp_sock *tp = tcp_sk(sk);
>  	struct sk_buff *skb;
>  	int iovlen, flags;
> -	int mss_now;
> +	int mss_now, size_goal;
>  	int err, copied;
>  	long timeo;
>  
> @@ -811,6 +809,7 @@ int tcp_sendmsg(struct kiocb *iocb, stru
>  	clear_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
>  
>  	mss_now = tcp_current_mss(sk, !(flags&MSG_OOB));
> +	size_goal = tp->xmit_size_goal;
>  
>  	/* Ok commence sending. */
>  	iovlen = msg->msg_iovlen;
> @@ -833,7 +832,7 @@ int tcp_sendmsg(struct kiocb *iocb, stru
>  			skb = sk->sk_write_queue.prev;
>  
>  			if (!sk->sk_send_head ||
> -			    (copy = mss_now - skb->len) <= 0) {
> +			    (copy = size_goal - skb->len) <= 0) {
>  
>  new_segment:
>  				/* Allocate new segment. If the 
> interface is SG, @@ -856,7 +855,7 @@ new_segment:
>  					skb->ip_summed = CHECKSUM_HW;
>  
>  				skb_entail(sk, tp, skb);
> -				copy = mss_now;
> +				copy = size_goal;
>  			}
>  
>  			/* Try to append data to the end of 
> skb. */ @@ -891,11 +890,6 @@ new_segment:
>  					tcp_mark_push(tp, skb);
>  					goto new_segment;
>  				} else if (page) {
> -					/* If page is cached, align
> -					 * offset to L1 cache boundary
> -					 */
> -					off = (off + 
> L1_CACHE_BYTES - 1) &
> -					      ~(L1_CACHE_BYTES - 1);
>  					if (off == PAGE_SIZE) {
>  						put_page(page);
>  						TCP_PAGE(sk) = 
> page = NULL;
> @@ -956,7 +950,7 @@ new_segment:
>  			if ((seglen -= copy) == 0 && iovlen == 0)
>  				goto out;
>  
> -			if (skb->len != mss_now || (flags & MSG_OOB))
> +			if (skb->len != size_goal || (flags & MSG_OOB))
>  				continue;
>  
>  			if (forced_push(tp)) {
> @@ -976,6 +970,7 @@ wait_for_memory:
>  				goto do_error;
>  
>  			mss_now = tcp_current_mss(sk, !(flags&MSG_OOB));
> +			size_goal = tp->xmit_size_goal;
>  		}
>  	}
>  
> @@ -2135,7 +2130,7 @@ void tcp_get_info(struct sock *sk, struc
>  
>  	info->tcpi_rto = jiffies_to_usecs(tp->rto);
>  	info->tcpi_ato = jiffies_to_usecs(tp->ack.ato);
> -	info->tcpi_snd_mss = tp->mss_cache_std;
> +	info->tcpi_snd_mss = tp->mss_cache;
>  	info->tcpi_rcv_mss = tp->ack.rcv_mss;
>  
>  	info->tcpi_unacked = tp->packets_out;
> @@ -2185,7 +2180,7 @@ int tcp_getsockopt(struct sock *sk, int 
>  
>  	switch (optname) {
>  	case TCP_MAXSEG:
> -		val = tp->mss_cache_std;
> +		val = tp->mss_cache;
>  		if (!val && ((1 << sk->sk_state) & (TCPF_CLOSE 
> | TCPF_LISTEN)))
>  			val = tp->rx_opt.user_mss;
>  		break;
> --- 1/net/ipv4/tcp_input.c.~1~	2005-05-16 
> 22:29:56.000000000 -0700
> +++ 2/net/ipv4/tcp_input.c	2005-05-17 16:49:26.000000000 -0700
> @@ -805,10 +805,10 @@ __u32 tcp_init_cwnd(struct tcp_sock *tp,
>  	__u32 cwnd = (dst ? dst_metric(dst, RTAX_INITCWND) : 0);
>  
>  	if (!cwnd) {
> -		if (tp->mss_cache_std > 1460)
> +		if (tp->mss_cache > 1460)
>  			cwnd = 2;
>  		else
> -			cwnd = (tp->mss_cache_std > 1095) ? 3 : 4;
> +			cwnd = (tp->mss_cache > 1095) ? 3 : 4;
>  	}
>  	return min_t(__u32, cwnd, tp->snd_cwnd_clamp);  } @@ 
> -974,14 +974,6 @@ tcp_sacktag_write_queue(struct sock *sk,
>  	int flag = 0;
>  	int i;
>  
> -	/* So, SACKs for already sent large segments will be lost.
> -	 * Not good, but alternative is to resegment the queue. */
> -	if (sk->sk_route_caps & NETIF_F_TSO) {
> -		sk->sk_route_caps &= ~NETIF_F_TSO;
> -		sock_set_flag(sk, SOCK_NO_LARGESEND);
> -		tp->mss_cache = tp->mss_cache_std;
> -	}
> -
>  	if (!tp->sacked_out)
>  		tp->fackets_out = 0;
>  	prior_fackets = tp->fackets_out;
> @@ -1038,6 +1030,15 @@ tcp_sacktag_write_queue(struct sock *sk,
>  			if(!before(TCP_SKB_CB(skb)->seq, end_seq))
>  				break;
>  
> +			/* Even if mincing a TSO frame fails, we
> +			 * continue anyways.  We will end up with
> +			 * more coarse SACK information, but it is
> +			 * better than ignoring all the SACK information
> +			 * altogether.
> +			 */
> +			if (tcp_skb_pcount(skb) > 1)
> +				tcp_tso_mince(sk, tp, skb);
> +
>  			fack_count += tcp_skb_pcount(skb);
>  
>  			in_sack = !after(start_seq, 
> TCP_SKB_CB(skb)->seq) && @@ -1142,7 +1143,7 @@ 
> tcp_sacktag_write_queue(struct sock *sk,
>  			    (IsFack(tp) ||
>  			     !before(lost_retrans,
>  				     TCP_SKB_CB(skb)->ack_seq + 
> tp->reordering *
> -				     tp->mss_cache_std))) {
> +				     tp->mss_cache))) {
>  				TCP_SKB_CB(skb)->sacked &= 
> ~TCPCB_SACKED_RETRANS;
>  				tp->retrans_out -= tcp_skb_pcount(skb);
>  
> @@ -1782,7 +1783,7 @@ static void tcp_try_to_open(struct sock 
>  		tp->retrans_stamp = 0;
>  
>  	if (flag&FLAG_ECE)
> -		tcp_enter_cwr(tp);
> +		tcp_enter_cwr(tp, 1);
>  
>  	if (tp->ca_state != TCP_CA_CWR) {
>  		int state = TCP_CA_Open;
> @@ -2170,7 +2171,7 @@ static void vegas_cong_avoid(struct tcp_
>  		 * is the cwnd during the previous RTT.
>  		 */
>  		old_wnd = (tp->vegas.beg_snd_nxt - 
> tp->vegas.beg_snd_una) /
> -			tp->mss_cache_std;
> +			tp->mss_cache;
>  		old_snd_cwnd = tp->vegas.beg_snd_cwnd;
>  
>  		/* Save the extent of the current window so we 
> can use this @@ -2799,19 +2800,19 @@ static void 
> westwood_dupack_update(struc  {
>  	struct tcp_sock *tp = tcp_sk(sk);
>  
> -	tp->westwood.accounted += tp->mss_cache_std;
> -	tp->westwood.cumul_ack = tp->mss_cache_std;
> +	tp->westwood.accounted += tp->mss_cache;
> +	tp->westwood.cumul_ack = tp->mss_cache;
>  }
>  
>  static inline int westwood_may_change_cumul(struct tcp_sock *tp)  {
> -	return (tp->westwood.cumul_ack > tp->mss_cache_std);
> +	return (tp->westwood.cumul_ack > tp->mss_cache);
>  }
>  
>  static inline void westwood_partial_update(struct tcp_sock *tp)  {
>  	tp->westwood.accounted -= tp->westwood.cumul_ack;
> -	tp->westwood.cumul_ack = tp->mss_cache_std;
> +	tp->westwood.cumul_ack = tp->mss_cache;
>  }
>  
>  static inline void westwood_complete_update(struct tcp_sock 
> *tp) @@ -3952,7 +3953,7 @@ static void tcp_new_space(struct sock *s
>  	    !(sk->sk_userlocks & SOCK_SNDBUF_LOCK) &&
>  	    !tcp_memory_pressure &&
>  	    atomic_read(&tcp_memory_allocated) < sysctl_tcp_mem[0]) {
> - 		int sndmem = max_t(u32, tp->rx_opt.mss_clamp, 
> tp->mss_cache_std) +
> + 		int sndmem = max_t(u32, tp->rx_opt.mss_clamp, 
> tp->mss_cache) +
>  			MAX_TCP_HEADER + 16 + sizeof(struct sk_buff),
>  		    demanded = max_t(unsigned int, tp->snd_cwnd,
>  						   tp->reordering + 1);
> @@ -3975,16 +3976,6 @@ static inline void tcp_check_space(struc
>  	}
>  }
>  
> -static void __tcp_data_snd_check(struct sock *sk, struct 
> sk_buff *skb) -{
> -	struct tcp_sock *tp = tcp_sk(sk);
> -
> -	if (after(TCP_SKB_CB(skb)->end_seq, tp->snd_una + 
> tp->snd_wnd) ||
> -	    tcp_packets_in_flight(tp) >= tp->snd_cwnd ||
> -	    tcp_write_xmit(sk, tp->nonagle))
> -		tcp_check_probe_timer(sk, tp);
> -}
> -
>  static __inline__ void tcp_data_snd_check(struct sock *sk)  {
>  	struct sk_buff *skb = sk->sk_send_head;
> --- 1/net/ipv4/tcp_ipv4.c.~1~	2005-05-16 22:29:56.000000000 -0700
> +++ 2/net/ipv4/tcp_ipv4.c	2005-05-17 11:27:14.000000000 -0700
> @@ -2060,7 +2060,8 @@ static int tcp_v4_init_sock(struct sock 
>  	 */
>  	tp->snd_ssthresh = 0x7fffffff;	/* Infinity */
>  	tp->snd_cwnd_clamp = ~0;
> -	tp->mss_cache_std = tp->mss_cache = 536;
> +	tp->mss_cache = 536;
> +	tp->xmit_size_cache = ~0;
>  
>  	tp->reordering = sysctl_tcp_reordering;
>  
> --- 1/net/ipv6/tcp_ipv6.c.~1~	2005-05-16 22:29:56.000000000 -0700
> +++ 2/net/ipv6/tcp_ipv6.c	2005-05-17 11:27:25.000000000 -0700
> @@ -2021,7 +2021,8 @@ static int tcp_v6_init_sock(struct sock 
>  	 */
>  	tp->snd_ssthresh = 0x7fffffff;
>  	tp->snd_cwnd_clamp = ~0;
> -	tp->mss_cache_std = tp->mss_cache = 536;
> +	tp->mss_cache = 536;
> +	tp->xmit_size_cache = ~0;
>  
>  	tp->reordering = sysctl_tcp_reordering;
> 
> 

^ permalink raw reply

* Re: [PATCH] Super TSO
From: David S. Miller @ 2005-05-20  0:21 UTC (permalink / raw)
  To: leonid.grossman; +Cc: netdev
In-Reply-To: <200505200015.j4K0FNVG005262@guinness.s2io.com>

From: "Leonid Grossman" <leonid.grossman@neterion.com>
Date: Thu, 19 May 2005 17:15:18 -0700

> A somewhat related thought, while we are at it...
> 
> It would arguably make sense to allow a NIC to set max TSO size that the
> card is 
> willing to support (rather than assume/enforce 64k). Some NICs support
> bigger max 
> TSO size today, but it is even more important to allow a NIC to limit
> TSO to a smaller size.

We can never use a maximum greater than 65535 because that
is the limit of the representation of the ipv4 header length
field.  These TSO packets have to be processed by the entire
IP output path, including things like the firewalling and
packet scheduler modules.

So it must look like a valid IPV4 packet.

> One likely scenario where this feature is desirable is a system with highly
> fragmented memory. 
> In this case, the number of physical fragments per TSO frame could be always
> so high that it will be cheaper 
> (on a given platform) to copy the frame than to DMA it. 

We always chop up the user data into individual system pages when we
build TSO frames, so I can't see how any kind of memory fragmentation
could be an issue.

So this kind of nullifies your argument for smaller TSO size
limits as specified by the card.

The segmenter in the TCP code has the most knowledge about how it
should segment, based upon numerous factors, many if not all of which
the networking card is totally unaware of.

^ permalink raw reply

* RE: [PATCH] Super TSO
From: Leonid Grossman @ 2005-05-20  0:36 UTC (permalink / raw)
  To: 'David S.Miller'; +Cc: netdev
In-Reply-To: <20050519.172107.59468102.davem@davemloft.net>

 

> -----Original Message-----
> From: David S.Miller [mailto:davem@davemloft.net] 
> Sent: Thursday, May 19, 2005 5:21 PM
> To: leonid.grossman@neterion.com
> Cc: netdev@oss.sgi.com
> Subject: Re: [PATCH] Super TSO
> 
> From: "Leonid Grossman" <leonid.grossman@neterion.com>
> Date: Thu, 19 May 2005 17:15:18 -0700
> 
> > A somewhat related thought, while we are at it...
> > 
> > It would arguably make sense to allow a NIC to set max TSO 
> size that 
> > the card is willing to support (rather than assume/enforce 
> 64k). Some 
> > NICs support bigger max TSO size today, but it is even more 
> important 
> > to allow a NIC to limit TSO to a smaller size.
> 
> We can never use a maximum greater than 65535 because that is 
> the limit of the representation of the ipv4 header length 
> field.  These TSO packets have to be processed by the entire 
> IP output path, including things like the firewalling and 
> packet scheduler modules.
> 
> So it must look like a valid IPV4 packet.
> 
> > One likely scenario where this feature is desirable is a 
> system with 
> > highly fragmented memory.
> > In this case, the number of physical fragments per TSO 
> frame could be 
> > always so high that it will be cheaper (on a given 
> platform) to copy 
> > the frame than to DMA it.
> 
> We always chop up the user data into individual system pages 
> when we build TSO frames, so I can't see how any kind of 
> memory fragmentation could be an issue.

This is exactly what I wanted to hear :-)
If the TSO implementation guarantees that the payload comes (for the most
part) 
in physically continuous pages, then the number of fragments 
will never get out of whack, and my argument indeed becomes invalid.

> 
> So this kind of nullifies your argument for smaller TSO size 
> limits as specified by the card.
> 
> The segmenter in the TCP code has the most knowledge about 
> how it should segment, based upon numerous factors, many if 
> not all of which the networking card is totally unaware of.

Sure. On the other hand, the TCP code is unaware of the copy vs. DMA costs
on a particular NIC 
(well, this is actually more specific to a system than to a NIC). 
But I guess as long as both the packet size and the number of fragments will
not get very big at the same time,
A NIC will be OK.


> 

^ permalink raw reply

* Re: [PATCH] Super TSO
From: Rick Jones @ 2005-05-20  0:45 UTC (permalink / raw)
  To: netdev
In-Reply-To: <200505200015.j4K0FNVG005262@guinness.s2io.com>


> In this case, 64k TSO becomes a liability and it will make sense to limit it.
>  TSO "sweet spot" will be captured anyways - at least on fast networks, going
>  from 1.5k to 9k typically doubles throughput, while going from 9k to 64k
> adds no more than another 10% (plus a little bit of free %cpu, but not that
> much).


On the surface, that just sounds like something adhering to the laws of
diminishing returns.  As you increase the TSO size, you are shrinking the 
per-send costs, but the per-byte costs (if there is a copy from user to kernel) 
and the ack costs remain the same.

If that 1500 to 9K to 64K is an MTU (and thus real MSS) change than the per-send 
and ack costs are what are diminishing but the per-byte costs remain the same.

Asymptotes and all that...

rick jones

^ permalink raw reply

* Re: [PATCH] Super TSO
From: David S. Miller @ 2005-05-20  0:55 UTC (permalink / raw)
  To: leonid.grossman; +Cc: netdev
In-Reply-To: <200505200036.j4K0aVVG008391@guinness.s2io.com>

From: "Leonid Grossman" <leonid.grossman@neterion.com>
Date: Thu, 19 May 2005 17:36:25 -0700

> > > One likely scenario where this feature is desirable is a 
> > system with 
> > > highly fragmented memory.
> > > In this case, the number of physical fragments per TSO 
> > frame could be 
> > > always so high that it will be cheaper (on a given 
> > platform) to copy 
> > > the frame than to DMA it.
> > 
> > We always chop up the user data into individual system pages 
> > when we build TSO frames, so I can't see how any kind of 
> > memory fragmentation could be an issue.
> 
> This is exactly what I wanted to hear :-)
> If the TSO implementation guarantees that the payload comes (for the most
> part) 
> in physically continuous pages, then the number of fragments 
> will never get out of whack, and my argument indeed becomes invalid.

You misunderstand me.  The TCP segmenter splits the incoming
user data into page size'd chunks.  So a 64K packet uses
64K / PAGE_SIZE individual pages.

The only thing the driver author needs to be aware of wrt. this
is to never wake up the TX netif queue until at least
"MAX_SKB_FRAGS + 1" transmit descriptors are available.

You'll see that every driver setting NETIF_F_SG implements
this test.

> Sure. On the other hand, the TCP code is unaware of the copy vs. DMA
> costs on a particular NIC (well, this is actually more specific to a
> system than to a NIC).  But I guess as long as both the packet size
> and the number of fragments will not get very big at the same time,
> A NIC will be OK.

They can and will be as large as "MAX_SKB_FRAGS + 1".

^ permalink raw reply

* RE: [PATCH] Super TSO
From: Leonid Grossman @ 2005-05-20  1:25 UTC (permalink / raw)
  To: 'David S.Miller'; +Cc: netdev
In-Reply-To: <20050519.175542.07642115.davem@davemloft.net>


> > > We always chop up the user data into individual system 
> pages when we 
> > > build TSO frames, so I can't see how any kind of memory 
> > > fragmentation could be an issue.
> > 
> > This is exactly what I wanted to hear :-) If the TSO implementation 
> > guarantees that the payload comes (for the most
> > part)
> > in physically continuous pages, then the number of fragments will 
> > never get out of whack, and my argument indeed becomes invalid.
> 
> You misunderstand me.  The TCP segmenter splits the incoming 
> user data into page size'd chunks.  So a 64K packet uses 64K 
> / PAGE_SIZE individual pages.


Yes, I think I'm actually with you. As long as each page is physically
continuous 
and therefore requires one descriptor per page, the total number of
descriptors per packet 
will be only 64K / PAGE_SIZE (plus some for headers), and this is fully
manageable from a NIC prospective.

> 
> The only thing the driver author needs to be aware of wrt. 
> this is to never wake up the TX netif queue until at least 
> "MAX_SKB_FRAGS + 1" transmit descriptors are available.
> 
> You'll see that every driver setting NETIF_F_SG implements this test.
> 
> > Sure. On the other hand, the TCP code is unaware of the 
> copy vs. DMA 
> > costs on a particular NIC (well, this is actually more 
> specific to a 
> > system than to a NIC).  But I guess as long as both the packet size 
> > and the number of fragments will not get very big at the 
> same time, A 
> > NIC will be OK.
> 
> They can and will be as large as "MAX_SKB_FRAGS + 1".
> 
> 

^ permalink raw reply


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).