* [PATCH] [IPV6] Defer IPv6 device initialization until a valid qdisc is specified
From: Mitsuru Chinen @ 2007-10-09 7:21 UTC (permalink / raw)
To: netdev; +Cc: YOSHIFUJI Hideaki
To judge the timing for DAD, netif_carrier_ok() is used. However,
there is a possibility that dev->qdisc stays noop_qdisc even if
netif_carrier_ok() returns true. In that case, DAD NS is not sent out.
We need to defer the IPv6 device initialization until a valid qdisc
is specified.
Signed-off-by: Mitsuru Chinen <mitch@linux.vnet.ibm.com>
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
net/ipv6/addrconf.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 6d5c3c2..da8c79c 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -74,6 +74,7 @@
#include <net/tcp.h>
#include <net/ip.h>
#include <net/netlink.h>
+#include <net/pkt_sched.h>
#include <linux/if_tunnel.h>
#include <linux/rtnetlink.h>
@@ -213,6 +214,12 @@ static struct ipv6_devconf ipv6_devconf_dflt __read_mostly = {
const struct in6_addr in6addr_any = IN6ADDR_ANY_INIT;
const struct in6_addr in6addr_loopback = IN6ADDR_LOOPBACK_INIT;
+/* Check if a valid qdisc is available */
+static inline int addrconf_qdisc_ok(struct net_device *dev)
+{
+ return (dev->qdisc != &noop_qdisc);
+}
+
static void addrconf_del_timer(struct inet6_ifaddr *ifp)
{
if (del_timer(&ifp->timer))
@@ -384,7 +391,7 @@ static struct inet6_dev * ipv6_add_dev(struct net_device *dev)
}
#endif
- if (netif_running(dev) && netif_carrier_ok(dev))
+ if (netif_running(dev) && addrconf_qdisc_ok(dev))
ndev->if_flags |= IF_READY;
ipv6_mc_init_dev(ndev);
@@ -2283,7 +2290,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
break;
if (event == NETDEV_UP) {
- if (!netif_carrier_ok(dev)) {
+ if (!addrconf_qdisc_ok(dev)) {
/* device is not ready yet. */
printk(KERN_INFO
"ADDRCONF(NETDEV_UP): %s: "
@@ -2295,7 +2302,7 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
if (idev)
idev->if_flags |= IF_READY;
} else {
- if (!netif_carrier_ok(dev)) {
+ if (!addrconf_qdisc_ok(dev)) {
/* device is still not ready. */
break;
}
--
1.5.2.2
^ permalink raw reply related
* Re: [ofa-general] Re: [PATCH V6 0/9] net/bonding: ADD IPoIB support for the bonding driver
From: Moni Shoua @ 2007-10-09 7:24 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Jay Vosburgh, netdev, OpenFabrics General
In-Reply-To: <23084.1191348619@death>
Jay Vosburgh wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
>
>> Moni Shoua wrote:
>>> Jay Vosburgh wrote:
>>>> ACK patches 3 - 9.
>>>>
>>>> Roland, are you comfortable with the IB changes in patches 1 and 2?
>>>>
>>>> Jeff, when Roland acks patches 1 and 2, please apply all 9.
>>>>
>>>> -J
>>> Hi Jeff,
>>> Roland acked the IPoIB patches. If you haven't done so already can you please apply them?
>>> I'm not sure when 2.6.24 is going to open and I'm afraid to miss it.
>> hrm, I don't see them in my inbox for some reason. can someone bounce
>> them to me? or give me a git tree to pull from?
>
> Moni, can you repost the patch series to Jeff, and put the
> appropriate "Acked-by" lines in for myself (patches 3 - 8) and Roland
> (patches 1 and 2)? You can probably leave off the netdev and
> openfabrics lists, but cc me.
>
> -J
>
> ---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>
Hi Jeff,
I don't commits of the patches in http://git.kernel.org/?p=linux/kernel/git/jgarzik/netdev-2.6.git;a=summary
(I hope that I'm looking in the right place).
Did you get them?
thanks
MoniS
^ permalink raw reply
* Re: raw PF_PACKET protocol selection
From: Joakim Tjernlund @ 2007-10-09 7:27 UTC (permalink / raw)
To: Evgeniy Polyakov; +Cc: 'Herbert Xu', netdev
In-Reply-To: <20071009071338.GA15057@2ka.mipt.ru>
On Tue, 2007-10-09 at 11:13 +0400, Evgeniy Polyakov wrote:
> On Tue, Oct 09, 2007 at 08:08:22AM +0200, Joakim Tjernlund (joakim.tjernlund@transmode.se) wrote:
> > > Your program works fine here. You did run it as root, right?
> >
> > Yes and ETH_P_ALL is the only protocol that prints anything
> > I am on 2.6.22
>
> ETH_P_ARP works too.
>
> > > Did you try stracing it?
> >
> > Just did and now it works, it didn't yesterday :(
> > But if I change protocol to ETH_P_MOBITEX, I don't get any
> > pkgs(I did change protocol on sending side too)
>
> Did you change eth_type_trans() to catch your proto?
>
Just fond out something:
if I redirect my prog like so:
./sniff > log
and press Ctrl-C after a packet has been sent to it,
it does NOT work. I don't get ANY output in my "log" file, not
even the printf("---------\n") appears.
But if I run whithout redirect it works(at least with ETH_P_BPQ)
Anyone else see this too?
Jocke
^ permalink raw reply
* Re: raw PF_PACKET protocol selection
From: Evgeniy Polyakov @ 2007-10-09 7:34 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: 'Herbert Xu', netdev
In-Reply-To: <1191914858.6682.75.camel@gentoo-jocke.transmode.se>
On Tue, Oct 09, 2007 at 09:27:38AM +0200, Joakim Tjernlund (joakim.tjernlund@transmode.se) wrote:
> > Did you change eth_type_trans() to catch your proto?
> >
>
> Just fond out something:
> if I redirect my prog like so:
> ./sniff > log
> and press Ctrl-C after a packet has been sent to it,
> it does NOT work. I don't get ANY output in my "log" file, not
> even the printf("---------\n") appears.
> But if I run whithout redirect it works(at least with ETH_P_BPQ)
> Anyone else see this too?
I only tested with IP and ARP packets - I can not say when packet was
actually received and written to log, but it does start filling up, but
maybe not immediately - it can be output buffering in shell though.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: raw PF_PACKET protocol selection
From: Joakim Tjernlund @ 2007-10-09 7:51 UTC (permalink / raw)
To: Evgeniy Polyakov; +Cc: 'Herbert Xu', netdev
In-Reply-To: <20071009073437.GA1951@2ka.mipt.ru>
On Tue, 2007-10-09 at 11:34 +0400, Evgeniy Polyakov wrote:
> On Tue, Oct 09, 2007 at 09:27:38AM +0200, Joakim Tjernlund (joakim.tjernlund@transmode.se) wrote:
> > > Did you change eth_type_trans() to catch your proto?
> > >
> >
> > Just fond out something:
> > if I redirect my prog like so:
> > ./sniff > log
> > and press Ctrl-C after a packet has been sent to it,
> > it does NOT work. I don't get ANY output in my "log" file, not
> > even the printf("---------\n") appears.
> > But if I run whithout redirect it works(at least with ETH_P_BPQ)
> > Anyone else see this too?
>
> I only tested with IP and ARP packets - I can not say when packet was
> actually received and written to log, but it does start filling up, but
> maybe not immediately - it can be output buffering in shell though.
Did you receive many packets? Seems like when I receive just 1 or 2 pkgs
I get the empty log. If I strace ./sniff > log I see that recvfrom gets
pkgs, but there are no trace of writes. I guess this
is a bash(3.2_p17) or glibc(2.5.-r4) bug?
^ permalink raw reply
* Re: raw PF_PACKET protocol selection
From: Herbert Xu @ 2007-10-09 7:56 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: Evgeniy Polyakov, netdev
In-Reply-To: <1191914858.6682.75.camel@gentoo-jocke.transmode.se>
On Tue, Oct 09, 2007 at 09:27:38AM +0200, Joakim Tjernlund wrote:
>
> Just fond out something:
> if I redirect my prog like so:
> ./sniff > log
> and press Ctrl-C after a packet has been sent to it,
> it does NOT work. I don't get ANY output in my "log" file, not
> even the printf("---------\n") appears.
> But if I run whithout redirect it works(at least with ETH_P_BPQ)
> Anyone else see this too?
Um, this is what we call buffering.
You either need to turn buffering off with setbuf(3) or you
should install a SIGINT handler to flush the output before
exiting.
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: [PATCHES] TX batching
From: Krishna Kumar2 @ 2007-10-09 8:14 UTC (permalink / raw)
To: hadi
Cc: David Miller, gaagaan, general, herbert, jagana, jeff,
Evgeniy Polyakov, kaber, kumarkr, mcarlson, mchan, netdev,
peter.p.waskiewicz.jr, randy.dunlap, rdreier, rick.jones2,
Robert.Olsson, shemminger, sri, tgraf, xma
In-Reply-To: <1191852320.4352.73.camel@localhost>
J Hadi Salim <j.hadi123@gmail.com> wrote on 10/08/2007 07:35:20 PM:
> I dont see something from Krishna's approach that i can take and reuse.
> This maybe because my old approaches have evolved from the same path.
> There is a long list but as a sample: i used to do a lot more work while
> holding the queue lock which i have now moved post queue lock; i dont
> have any speacial interfaces/tricks just for batching, i provide hints
> to the core of how much the driver can take etc etc. I have offered
> Krishna co-authorship if he makes the IPOIB driver to work on my
> patches, that offer still stands if he chooses to take it.
My feeling is that since the approaches are very different, it would
be a good idea to test the two for performance. Do you mind me doing
that? Ofcourse others and/or you are more than welcome to do the same.
I had sent a note to you yesterday about this, please let me know
either way.
******************* Previous mail ******************
Hi Jamal,
If you don't mind, I am trying to run your approach vs mine to get some
results
for comparison.
For starters, I am having issues with iperf when using your infrastructure
code with
my IPoIB driver - about 100MB is sent and then everything stops for some
reason.
The changes in the IPoIB driver that I made to support batching is to set
BTX, set
xmit_win, and dynamically reduce xmit_win on every xmit and increase
xmit_win on
every xmit completion. Is there anything else that is required from the
driver?
thanks,
- KK
^ permalink raw reply
* Re: raw PF_PACKET protocol selection
From: Evgeniy Polyakov @ 2007-10-09 8:17 UTC (permalink / raw)
To: Joakim Tjernlund; +Cc: 'Herbert Xu', netdev
In-Reply-To: <1191916285.6682.83.camel@gentoo-jocke.transmode.se>
On Tue, Oct 09, 2007 at 09:51:25AM +0200, Joakim Tjernlund (joakim.tjernlund@transmode.se) wrote:
> On Tue, 2007-10-09 at 11:34 +0400, Evgeniy Polyakov wrote:
> > On Tue, Oct 09, 2007 at 09:27:38AM +0200, Joakim Tjernlund (joakim.tjernlund@transmode.se) wrote:
> > > > Did you change eth_type_trans() to catch your proto?
> > > >
> > >
> > > Just fond out something:
> > > if I redirect my prog like so:
> > > ./sniff > log
> > > and press Ctrl-C after a packet has been sent to it,
> > > it does NOT work. I don't get ANY output in my "log" file, not
> > > even the printf("---------\n") appears.
> > > But if I run whithout redirect it works(at least with ETH_P_BPQ)
> > > Anyone else see this too?
> >
> > I only tested with IP and ARP packets - I can not say when packet was
> > actually received and written to log, but it does start filling up, but
> > maybe not immediately - it can be output buffering in shell though.
>
> Did you receive many packets? Seems like when I receive just 1 or 2 pkgs
> I get the empty log. If I strace ./sniff > log I see that recvfrom gets
> pkgs, but there are no trace of writes. I guess this
> is a bash(3.2_p17) or glibc(2.5.-r4) bug?
I received 1396 bytes of logs before terminated, which is 27 ARP packets,
so there is quite big number of packet there.
Your application works correctly (although you swapped source and
destination ethernet fields) - buffered writing is not a bug,
if you do not like it, use write(2), mmap(2) or turn buffering off as
Herbert suggested. To get packets with your own ethernet protocol number
you have to change eth_type_trans() function in kernel, which parses
ethernet header and returns protocol number, under some conditions it
will just return your number automatically, but you should check it.
--
Evgeniy Polyakov
^ permalink raw reply
* Re: raw PF_PACKET protocol selection
From: Joakim Tjernlund @ 2007-10-09 9:00 UTC (permalink / raw)
To: Evgeniy Polyakov; +Cc: 'Herbert Xu', netdev
In-Reply-To: <20071009081752.GA31369@2ka.mipt.ru>
On Tue, 2007-10-09 at 12:17 +0400, Evgeniy Polyakov wrote:
> On Tue, Oct 09, 2007 at 09:51:25AM +0200, Joakim Tjernlund (joakim.tjernlund@transmode.se) wrote:
> > On Tue, 2007-10-09 at 11:34 +0400, Evgeniy Polyakov wrote:
> > > On Tue, Oct 09, 2007 at 09:27:38AM +0200, Joakim Tjernlund (joakim.tjernlund@transmode.se) wrote:
> > > > > Did you change eth_type_trans() to catch your proto?
> > > > >
> > > >
> > > > Just fond out something:
> > > > if I redirect my prog like so:
> > > > ./sniff > log
> > > > and press Ctrl-C after a packet has been sent to it,
> > > > it does NOT work. I don't get ANY output in my "log" file, not
> > > > even the printf("---------\n") appears.
> > > > But if I run whithout redirect it works(at least with ETH_P_BPQ)
> > > > Anyone else see this too?
> > >
> > > I only tested with IP and ARP packets - I can not say when packet was
> > > actually received and written to log, but it does start filling up, but
> > > maybe not immediately - it can be output buffering in shell though.
> >
> > Did you receive many packets? Seems like when I receive just 1 or 2 pkgs
> > I get the empty log. If I strace ./sniff > log I see that recvfrom gets
> > pkgs, but there are no trace of writes. I guess this
> > is a bash(3.2_p17) or glibc(2.5.-r4) bug?
>
> I received 1396 bytes of logs before terminated, which is 27 ARP packets,
> so there is quite big number of packet there.
> Your application works correctly (although you swapped source and
> destination ethernet fields) - buffered writing is not a bug,
> if you do not like it, use write(2), mmap(2) or turn buffering off as
> Herbert suggested. To get packets with your own ethernet protocol number
> you have to change eth_type_trans() function in kernel, which parses
> ethernet header and returns protocol number, under some conditions it
> will just return your number automatically, but you should check it.
I thought that flushing was done automatically when SIGINT happened
but I was apperently wrong. Sorry for the noise and thanks for your
help. I have added setvbuf calls to make it unbuffered.
Jocke
^ permalink raw reply
* Hi dear
From: Angela @ 2007-10-08 20:17 UTC (permalink / raw)
To: netdev
Hey, my name isAngela. Im looking for a fiend and dying to meet you. Catch my photo and call me. My photo http://10ur.net/logo.JPG
^ permalink raw reply
* net-2.6.24 rebased...
From: David Miller @ 2007-10-09 9:41 UTC (permalink / raw)
To: netdev; +Cc: akpm
The TCP bug fix that went into Linus's tree today created some
conflicts with net-2.6.24, so I rebased:
kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.24.git
I took the opportunity to integrate fix patches into the patches which
introduced those bugs or build failures.
BTW, when the tree gets this big I tend to move the pre-rebase tree
to:
kernel.org:/pub/scm/linux/kernel/git/davem/bak-net-2.6.24.git
It can be useful if a mistake is made during a rebase and we need
to figure out what happened.
9.1MB, 739 changesets, keep those patches flowing!
Enjoy
^ permalink raw reply
* [PATCH][NET-2.6.24] Remove double dev->flags checking when calling dev_close()
From: Pavel Emelyanov @ 2007-10-09 10:50 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
The unregister_netdevice() and dev_change_net_namespace()
both check for dev->flags to be IFF_UP before calling the
dev_close(), but the dev_close() checks for IFF_UP itself,
so remove those unneeded checks.
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/net/core/dev.c b/net/core/dev.c
index e7e728a..1e169a5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3893,8 +3893,7 @@ void unregister_netdevice(struct net_dev
BUG_ON(dev->reg_state != NETREG_REGISTERED);
/* If device is running, close it first. */
- if (dev->flags & IFF_UP)
- dev_close(dev);
+ dev_close(dev);
/* And unlink it from device chain. */
unlist_netdevice(dev);
@@ -4018,8 +4017,7 @@ int dev_change_net_namespace(struct net_
*/
/* If device is running close it first. */
- if (dev->flags & IFF_UP)
- dev_close(dev);
+ dev_close(dev);
/* And unlink it from device chain */
err = -ENODEV;
^ permalink raw reply related
* [ofa-general] RE: [PATCH 2/3][NET_BATCH] net core use batching
From: Krishna Kumar2 @ 2007-10-09 10:58 UTC (permalink / raw)
To: Waskiewicz Jr, Peter P
Cc: jagana, johnpol, herbert, gaagaan, Robert.Olsson, mcarlson,
rdreier, hadi, kaber, randy.dunlap, jeff, general, mchan, tgraf,
netdev, shemminger, David Miller, sri
In-Reply-To: <D5C1322C3E673F459512FB59E0DDC32903BC14AC@orsmsx414.amr.corp.intel.com>
Hi Peter,
"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@intel.com> wrote on
10/09/2007 04:03:42 AM:
> > true, that needs some resolution. Heres a hand-waving thought:
> > Assuming all packets of a specific map end up in the same
> > qdiscn queue, it seems feasible to ask the qdisc scheduler to
> > give us enough packages (ive seen people use that terms to
> > refer to packets) for each hardware ring's available space.
> > With the patches i posted, i do that via
> > dev->xmit_win that assumes only one view of the driver; essentially a
> > single ring.
> > If that is doable, then it is up to the driver to say "i have
> > space for 5 in ring[0], 10 in ring[1] 0 in ring[2]" based on
> > what scheduling scheme the driver implements - the dev->blist
> > can stay the same. Its a handwave, so there may be issues
> > there and there could be better ways to handle this.
> >
> > Note: The other issue that needs resolving that i raised
> > earlier was in regards to multiqueue running on multiple cpus
> > servicing different rings concurently.
>
> I can see the qdisc being modified to send batches per queue_mapping.
> This shouldn't be too difficult, and if we had the xmit_win per queue
> (in the subqueue struct like Dave pointed out).
I hope my understanding of multiqueue is correct for this mail to make
sense :-)
Isn't it enough that the multiqueue+batching drivers handle skbs
belonging to different queue's themselves, instead of qdisc having
to figure that out? This will reduce costs for most skbs that are
neither batched nor sent to multiqueue devices.
Eg, driver can keep processing skbs and put to the correct tx_queue
as long as mapping remains the same. If the mapping changes, it posts
earlier skbs (with the correct lock) and then iterates for the other
skbs that have the next different mapping, and so on.
(This is required only if driver is supposed to transmit >1 skb in one
call, otherwise it is not an issue)
Alternatively, supporting drivers could return a different code on
mapping change, like: NETDEV_TX_MAPPING_CHANGED (for batching only)
so that qdisc_run() could retry. Would that work?
Secondly having xmit_win per queue: would it help in multiple skb
case? Currently there is no way to tell qdisc to dequeue skbs from
a particular band - it returns skb from highest priority band.
thanks,
- KK
^ permalink raw reply
* Re: [PATCH 2/3][NET_BATCH] net core use batching
From: David Miller @ 2007-10-09 11:02 UTC (permalink / raw)
To: krkumar2
Cc: peter.p.waskiewicz.jr, gaagaan, general, hadi, herbert, jagana,
jeff, johnpol, kaber, mcarlson, mchan, netdev, randy.dunlap,
rdreier, rick.jones2, Robert.Olsson, shemminger, sri, tgraf, xma
In-Reply-To: <OF8571378C.BCC92C60-ON6525736F.003A8E75-6525736F.003C488C@in.ibm.com>
From: Krishna Kumar2 <krkumar2@in.ibm.com>
Date: Tue, 9 Oct 2007 16:28:27 +0530
> Isn't it enough that the multiqueue+batching drivers handle skbs
> belonging to different queue's themselves, instead of qdisc having
> to figure that out? This will reduce costs for most skbs that are
> neither batched nor sent to multiqueue devices.
>
> Eg, driver can keep processing skbs and put to the correct tx_queue
> as long as mapping remains the same. If the mapping changes, it posts
> earlier skbs (with the correct lock) and then iterates for the other
> skbs that have the next different mapping, and so on.
The complexity in most of these suggestions is beginning to drive me a
bit crazy :-)
This should be the simplest thing in the world, when TX queue has
space, give it packets. Period.
When I hear suggestions like "have the driver pick the queue in
->hard_start_xmit() and return some special status if the queue
becomes different"..... you know, I really begin to wonder :-)
If we have to go back, get into the queueing layer locks, have these
special cases, and whatnot, what's the point?
This code should eventually be able to run lockless all the way to the
TX queue handling code of the driver. The queueing code should know
what TX queue the packet will be bound for, and always precisely
invoke the driver in a state where the driver can accept the packet.
Ignore LLTX, it sucks, it was a big mistake, and we will get rid of
it.
^ permalink raw reply
* [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Krishna Kumar2 @ 2007-10-09 11:20 UTC (permalink / raw)
To: David Miller
Cc: jagana, johnpol, gaagaan, jeff, Robert.Olsson, mcarlson, rdreier,
peter.p.waskiewicz.jr, hadi, netdev, general, mchan, tgraf,
randy.dunlap, sri, shemminger, kaber, herbert
In-Reply-To: <20071009.040255.71088090.davem@davemloft.net>
Hi Dave,
David Miller <davem@davemloft.net> wrote on 10/09/2007 04:32:55 PM:
> > Isn't it enough that the multiqueue+batching drivers handle skbs
> > belonging to different queue's themselves, instead of qdisc having
> > to figure that out? This will reduce costs for most skbs that are
> > neither batched nor sent to multiqueue devices.
> >
> > Eg, driver can keep processing skbs and put to the correct tx_queue
> > as long as mapping remains the same. If the mapping changes, it posts
> > earlier skbs (with the correct lock) and then iterates for the other
> > skbs that have the next different mapping, and so on.
>
> The complexity in most of these suggestions is beginning to drive me a
> bit crazy :-)
>
> This should be the simplest thing in the world, when TX queue has
> space, give it packets. Period.
>
> When I hear suggestions like "have the driver pick the queue in
> ->hard_start_xmit() and return some special status if the queue
> becomes different"..... you know, I really begin to wonder :-)
>
> If we have to go back, get into the queueing layer locks, have these
> special cases, and whatnot, what's the point?
I understand your point, but the qdisc code itself needs almost no
change, as small as:
qdisc_restart()
{
...
case NETDEV_TX_MAPPING_CHANGED:
/*
* Driver sent some skbs from one mapping, and found others
* are for different queue_mapping. Try again.
*/
ret = 1; /* guaranteed to have atleast 1 skb in batch list */
break;
...
}
Alternatively if the driver does all the dirty work, qdisc needs no
change at all. However, I am not sure if this addresses all the
concerns raised by you, Peter, Jamal, others.
> This code should eventually be able to run lockless all the way to the
> TX queue handling code of the driver. The queueing code should know
> what TX queue the packet will be bound for, and always precisely
> invoke the driver in a state where the driver can accept the packet.
This sounds like a good idea :)
I need to think more on this, esp as my batching sends multiple skbs of
possibly different mappings to device, and those skbs stay in batch list
if driver couldn't send them out.
thanks,
- KK
^ permalink raw reply
* [ofa-general] Re: [PATCH 2/3][NET_BATCH] net core use batching
From: Krishna Kumar2 @ 2007-10-09 11:21 UTC (permalink / raw)
To: David Miller
Cc: jagana, johnpol, gaagaan, jeff, Robert.Olsson, mcarlson, rdreier,
peter.p.waskiewicz.jr, hadi, netdev, general, mchan, tgraf,
randy.dunlap, sri, shemminger, kaber, herbert
In-Reply-To: <20071009.040255.71088090.davem@davemloft.net>
David Miller <davem@davemloft.net> wrote on 10/09/2007 04:32:55 PM:
> Ignore LLTX, it sucks, it was a big mistake, and we will get rid of
> it.
Great, this will make life easy. Any idea how long that would take?
It seems simple enough to do.
thanks,
- KK
^ permalink raw reply
* Re: [PATCH 2/3][NET_BATCH] net core use batching
From: David Miller @ 2007-10-09 11:24 UTC (permalink / raw)
To: krkumar2
Cc: gaagaan, general, hadi, herbert, jagana, jeff, johnpol, kaber,
mcarlson, mchan, netdev, peter.p.waskiewicz.jr, randy.dunlap,
rdreier, rick.jones2, Robert.Olsson, shemminger, sri, tgraf, xma
In-Reply-To: <OF20E88706.D91FCF10-ON6525736F.003E50CD-6525736F.003E5E6F@in.ibm.com>
From: Krishna Kumar2 <krkumar2@in.ibm.com>
Date: Tue, 9 Oct 2007 16:51:14 +0530
> David Miller <davem@davemloft.net> wrote on 10/09/2007 04:32:55 PM:
>
> > Ignore LLTX, it sucks, it was a big mistake, and we will get rid of
> > it.
>
> Great, this will make life easy. Any idea how long that would take?
> It seems simple enough to do.
I'd say we can probably try to get rid of it in 2.6.25, this is
assuming we get driver authors to cooperate and do the conversions
or alternatively some other motivated person.
I can just threaten to do them all and that should get the driver
maintainers going :-)
^ permalink raw reply
* Re: [PATCH][NET-2.6.24] Remove double dev->flags checking when calling dev_close()
From: Jeff Garzik @ 2007-10-09 11:33 UTC (permalink / raw)
To: Pavel Emelyanov; +Cc: David Miller, Linux Netdev List, devel
In-Reply-To: <470B5D0E.9090202@openvz.org>
Pavel Emelyanov wrote:
> The unregister_netdevice() and dev_change_net_namespace()
> both check for dev->flags to be IFF_UP before calling the
> dev_close(), but the dev_close() checks for IFF_UP itself,
> so remove those unneeded checks.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>
> ---
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index e7e728a..1e169a5 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3893,8 +3893,7 @@ void unregister_netdevice(struct net_dev
> BUG_ON(dev->reg_state != NETREG_REGISTERED);
>
> /* If device is running, close it first. */
> - if (dev->flags & IFF_UP)
> - dev_close(dev);
> + dev_close(dev);
>
> /* And unlink it from device chain. */
> unlist_netdevice(dev);
> @@ -4018,8 +4017,7 @@ int dev_change_net_namespace(struct net_
> */
>
> /* If device is running close it first. */
> - if (dev->flags & IFF_UP)
> - dev_close(dev);
> + dev_close(dev);
>
> /* And unlink it from device chain */
> err = -ENODEV;
One side effect of this patch: might_sleep() is now called unconditionally.
If that is irrelevant... ACK.
^ permalink raw reply
* Re: [PATCH][NET-2.6.24] Remove double dev->flags checking when calling dev_close()
From: Pavel Emelyanov @ 2007-10-09 11:44 UTC (permalink / raw)
To: Jeff Garzik; +Cc: David Miller, Linux Netdev List, devel
In-Reply-To: <470B66FA.80006@garzik.org>
Jeff Garzik wrote:
> Pavel Emelyanov wrote:
>> The unregister_netdevice() and dev_change_net_namespace()
>> both check for dev->flags to be IFF_UP before calling the
>> dev_close(), but the dev_close() checks for IFF_UP itself,
>> so remove those unneeded checks.
>>
>> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>>
>> ---
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index e7e728a..1e169a5 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3893,8 +3893,7 @@ void unregister_netdevice(struct net_dev
>> BUG_ON(dev->reg_state != NETREG_REGISTERED);
>>
>> /* If device is running, close it first. */
>> - if (dev->flags & IFF_UP)
>> - dev_close(dev);
>> + dev_close(dev);
>>
>> /* And unlink it from device chain. */
>> unlist_netdevice(dev);
>> @@ -4018,8 +4017,7 @@ int dev_change_net_namespace(struct net_
>> */
>>
>> /* If device is running close it first. */
>> - if (dev->flags & IFF_UP)
>> - dev_close(dev);
>> + dev_close(dev);
>>
>> /* And unlink it from device chain */
>> err = -ENODEV;
>
> One side effect of this patch: might_sleep() is now called unconditionally.
That's not a big deal - see, the synchronize_net() is called further
in both places unconditionally (!) and also calls might_sleep(), so
this is OK.
> If that is irrelevant... ACK.
Thanks :)
>
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH][NETNS] Don't memset() netns to zero manually
From: Pavel Emelyanov @ 2007-10-09 11:48 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, devel
The newly created net namespace is set to 0 with memset()
in setup_net(). The setup_net() is also called for the
init_net_ns(), which is zeroed naturally as a global var.
So remove this memset and allocate new nets with the
kmem_cache_zalloc().
Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
---
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 0e0ca6d..6f71db8 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -24,7 +24,7 @@ EXPORT_SYMBOL_GPL(init_net);
static struct net *net_alloc(void)
{
- return kmem_cache_alloc(net_cachep, GFP_KERNEL);
+ return kmem_cache_zalloc(net_cachep, GFP_KERNEL);
}
static void net_free(struct net *net)
@@ -90,7 +90,6 @@ static int setup_net(struct net *net)
struct pernet_operations *ops;
int error;
- memset(net, 0, sizeof(struct net));
atomic_set(&net->count, 1);
atomic_set(&net->use_count, 0);
^ permalink raw reply related
* Re: [RFC][PATCH 1/2] TCP: fix lost retransmit detection
From: Ilpo Järvinen @ 2007-10-09 12:19 UTC (permalink / raw)
To: TAKANO Ryousei; +Cc: Netdev, y-kodama
In-Reply-To: <20071009.152834.38612348.takano@axe-inc.co.jp>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 2629 bytes --]
On Tue, 9 Oct 2007, TAKANO Ryousei wrote:
> From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
> Subject: Re: [RFC][PATCH 1/2] TCP: fix lost retransmit detection
> Date: Mon, 8 Oct 2007 14:11:55 +0300 (EEST)
>
> > On Sun, 7 Oct 2007, TAKANO Ryousei wrote:
> >
> > > From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
> > >
> > > > Just couple of thoughts, not that this change itself is incorrect...
...Added this line back, it's important. :-)
> > > > In case sacktag uses fastpath, this code won't be executed for the skb's
> > > > that we would like to check (those with SACKED_RETRANS set, that are
> > > > below the fastpath_skb_hint). We will eventually deal with the whole queue
> > > > when fastpath_skb_hint gets set to NULL, with the next cumulative ACK that
> > > > fully ACKs an skb at the latest. Maybe there's a need for a larger surgery
> > > > than this to fix it. I think we need additional field to tcp_sock to avoid
> > > > doing a full-walk per ACK:
> > >
> > > I think the problem occurs in slowpath. For example, in case when the receiver
> > > detects and sends back a new SACK block, the sender may fail to detect loss
> > > of a retransmitted packet.
> >
> > ...No, the slow path is very likely to _correct_ the problem! The problem
> > occurs because fastpath _skips_ processing of skbs below fastpath_skb_hint
> > whereas slowpath starts from tcp_write_queue_head. The retransmitted skbs
> > we're interested in reside exactly on that skipped range.
> >
> Sorry, my explanaton is insufficient.
> This problem occurs even if we are in the slowpath.
>
> [...long explination snip...]
>
> Is it corrent?
Yes, we're dealing with two related, but different problems here. I
understood well what you're trying to fix, and even acknowledged your
change (see what I added back from my original reply above). The far
worse problem which I describe, however, occurs with fastpath only (well,
very unlikely to occur with slow-path but it could in theory), has been
like this since the hints were added. And it's preventing the execution
of this portion you fixed (until slowpath is taken).
...What makes the other problem even nastier is the fact that it
becomes more and more significant when fastpath (or it's equivalent)
skips more and more skb processing. It's good that we noticed it
now...
I tried to do a fix to that other problem, it seems that the solution
will be intertwined with the problem you're describing so that I in
fact ended removing the code block where your key modification is and
placing somewhat similar sequence gathering elsewhere... Posting it
shortly.
--
i.
^ permalink raw reply
* [RFC PATCH net-2.6.24 0/3]: Attempt to fix lost_retrans brokeness
From: Ilpo Järvinen @ 2007-10-09 12:19 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Stephen Hemminger, TAKANO Ryousei
Lost_retrans handling of sacktag was found to be flawed, two
problems that were found have an intertwined solution. Fastpath
problem has existed since hints got added and the other problem
has probably been there even longer than that. ...This change
may add non-trivial processing cost.
Initial sketch, only compile tested. This will become more and
more useful, when sacktag starts to process less and less skbs,
which hopefully happens quite soon... :-) Sadly enough it will
probably then be consuming part of the benefits we're able to
achieve by less skb walking...
First one is trivial, so Dave might want to apply it already.
--
i.
^ permalink raw reply
* [RFC PATCH] [TCP]: Fix lost_retrans loop vs fastpath problems
From: Ilpo Järvinen @ 2007-10-09 12:20 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Stephen Hemminger, TAKANO Ryousei
In-Reply-To: <11919324023846-git-send-email-ilpo.jarvinen@helsinki.fi>
Detection implemented with lost_retrans must work also when
fastpath is taken, yet most of the queue is skipped including
(very likely) those retransmitted skb's we're interested in.
This problem appeared when the hints got added, which removed
a need to always walk over the whole write queue head.
Therefore decicion for the lost_retrans worker loop entry must
be separated from the sacktag processing more than it was
necessary before.
It turns out to be problematic to optimize the worker loop
very heavily because ack_seqs of skb may have a number of
discontinuity points. Maybe similar approach as currently is
implemented could be attempted but that's becoming more and
more complex because the trend is towards less skb walking
in sacktag marker.
Maybe after(highest_sack_end_seq, tp->high_seq) checking is not
sufficiently accurate and causes entry too often in no-work-to-do
cases. Since that's not known, I've separated solution to that
from this patch.
Noticed because of report against a related problem from TAKANO
Ryousei <takano@axe-inc.co.jp>. He also provided a patch to
that part of the problem. This patch includes solution to it
(though this patch has to use somewhat different placement).
TAKANO's description and patch is available here:
http://marc.info/?l=linux-netdev&m=119149311913288&w=2
...In short, TAKANO's problem is that end_seq the loop is using
not necessarily the largest SACK block's end_seq because the
current ACK may still have higher SACK blocks which are later
by the loop.
Cc: TAKANO Ryousei <takano@axe-inc.co.jp>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 37 ++++++++++++++++++++++---------------
1 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 56aa34a..b90c2fc 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1109,27 +1109,34 @@ static int tcp_is_sackblock_valid(struct tcp_sock *tp, int is_dsack,
/* Check for lost retransmit. This superb idea is borrowed from "ratehalving".
* Event "C". Later note: FACK people cheated me again 8), we have to account
* for reordering! Ugly, but should help.
+ *
+ * Search retransmitted skbs from write_queue that were sent when snd_nxt was
+ * less than what is now known to be received by the other end (derived from
+ * SACK blocks by the caller).
*/
-static int tcp_mark_lost_retrans(struct sock *sk, u32 lost_retrans)
+static int tcp_mark_lost_retrans(struct sock *sk, u32 received_upto)
{
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *skb;
int flag = 0;
+ int cnt = 0;
tcp_for_write_queue(skb, sk) {
u32 ack_seq = TCP_SKB_CB(skb)->ack_seq;
if (skb == tcp_send_head(sk))
break;
- if (after(TCP_SKB_CB(skb)->seq, lost_retrans))
+ if (cnt == tp->retrans_out)
break;
if (!after(TCP_SKB_CB(skb)->end_seq, tp->snd_una))
continue;
- if ((TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_RETRANS) &&
- after(lost_retrans, ack_seq) &&
+ if (!(TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_RETRANS))
+ continue;
+
+ if (after(received_upto, ack_seq) &&
(tcp_is_fack(tp) ||
- !before(lost_retrans,
+ !before(received_upto,
ack_seq + tp->reordering * tp->mss_cache))) {
TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_RETRANS;
tp->retrans_out -= tcp_skb_pcount(skb);
@@ -1143,6 +1150,8 @@ static int tcp_mark_lost_retrans(struct sock *sk, u32 lost_retrans)
flag |= FLAG_DATA_SACKED;
NET_INC_STATS_BH(LINUX_MIB_TCPLOSTRETRANSMIT);
}
+ } else {
+ cnt += tcp_skb_pcount(skb);
}
}
return flag;
@@ -1193,7 +1202,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
int num_sacks = (ptr[1] - TCPOLEN_SACK_BASE)>>3;
int reord = tp->packets_out;
int prior_fackets;
- u32 lost_retrans = 0;
+ u32 highest_sack_end_seq = 0;
int flag = 0;
int found_dup_sack = 0;
int cached_fack_count;
@@ -1383,11 +1392,6 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
continue;
}
- if ((sacked&TCPCB_SACKED_RETRANS) &&
- after(end_seq, TCP_SKB_CB(skb)->ack_seq) &&
- (!lost_retrans || after(end_seq, lost_retrans)))
- lost_retrans = end_seq;
-
if (!in_sack)
continue;
@@ -1441,9 +1445,10 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (fack_count > tp->fackets_out)
tp->fackets_out = fack_count;
- if (after(TCP_SKB_CB(skb)->seq,
- tp->highest_sack))
+ if (after(TCP_SKB_CB(skb)->seq, tp->highest_sack)) {
tp->highest_sack = TCP_SKB_CB(skb)->seq;
+ highest_sack_end_seq = TCP_SKB_CB(skb)->end_seq;
+ }
} else {
if (dup_sack && (sacked&TCPCB_RETRANS))
reord = min(fack_count, reord);
@@ -1463,8 +1468,10 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
}
}
- if (lost_retrans && icsk->icsk_ca_state == TCP_CA_Recovery)
- flag |= tcp_mark_lost_retrans(sk, lost_retrans);
+ if (tp->retrans_out && highest_sack_end_seq &&
+ after(highest_sack_end_seq, tp->high_seq) &&
+ icsk->icsk_ca_state == TCP_CA_Recovery)
+ flag |= tcp_mark_lost_retrans(sk, highest_sack_end_seq);
tcp_verify_left_out(tp);
--
1.5.0.6
^ permalink raw reply related
* [PATCH] [TCP]: Separate lost_retrans loop into own function
From: Ilpo Järvinen @ 2007-10-09 12:20 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Stephen Hemminger, TAKANO Ryousei
In-Reply-To: <11919324023597-git-send-email-ilpo.jarvinen@helsinki.fi>
Follows own function for each task principle, this is really
somewhat separate task being done in sacktag. Also reduces
indentation.
In addition, added ack_seq local var to break some long
lines & fixed coding style things.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_input.c | 80 +++++++++++++++++++++++++++-----------------------
1 files changed, 43 insertions(+), 37 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index a04e78e..56aa34a 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1106,6 +1106,47 @@ static int tcp_is_sackblock_valid(struct tcp_sock *tp, int is_dsack,
return !before(start_seq, end_seq - tp->max_window);
}
+/* Check for lost retransmit. This superb idea is borrowed from "ratehalving".
+ * Event "C". Later note: FACK people cheated me again 8), we have to account
+ * for reordering! Ugly, but should help.
+ */
+static int tcp_mark_lost_retrans(struct sock *sk, u32 lost_retrans)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct sk_buff *skb;
+ int flag = 0;
+
+ tcp_for_write_queue(skb, sk) {
+ u32 ack_seq = TCP_SKB_CB(skb)->ack_seq;
+
+ if (skb == tcp_send_head(sk))
+ break;
+ if (after(TCP_SKB_CB(skb)->seq, lost_retrans))
+ break;
+ if (!after(TCP_SKB_CB(skb)->end_seq, tp->snd_una))
+ continue;
+
+ if ((TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_RETRANS) &&
+ after(lost_retrans, ack_seq) &&
+ (tcp_is_fack(tp) ||
+ !before(lost_retrans,
+ ack_seq + tp->reordering * tp->mss_cache))) {
+ TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_RETRANS;
+ tp->retrans_out -= tcp_skb_pcount(skb);
+
+ /* clear lost hint */
+ tp->retransmit_skb_hint = NULL;
+
+ if (!(TCP_SKB_CB(skb)->sacked & (TCPCB_LOST|TCPCB_SACKED_ACKED))) {
+ tp->lost_out += tcp_skb_pcount(skb);
+ TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
+ flag |= FLAG_DATA_SACKED;
+ NET_INC_STATS_BH(LINUX_MIB_TCPLOSTRETRANSMIT);
+ }
+ }
+ }
+ return flag;
+}
static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
struct tcp_sack_block_wire *sp, int num_sacks,
@@ -1422,43 +1463,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
}
}
- /* Check for lost retransmit. This superb idea is
- * borrowed from "ratehalving". Event "C".
- * Later note: FACK people cheated me again 8),
- * we have to account for reordering! Ugly,
- * but should help.
- */
- if (lost_retrans && icsk->icsk_ca_state == TCP_CA_Recovery) {
- struct sk_buff *skb;
-
- tcp_for_write_queue(skb, sk) {
- if (skb == tcp_send_head(sk))
- break;
- if (after(TCP_SKB_CB(skb)->seq, lost_retrans))
- break;
- if (!after(TCP_SKB_CB(skb)->end_seq, tp->snd_una))
- continue;
- if ((TCP_SKB_CB(skb)->sacked&TCPCB_SACKED_RETRANS) &&
- after(lost_retrans, TCP_SKB_CB(skb)->ack_seq) &&
- (tcp_is_fack(tp) ||
- !before(lost_retrans,
- TCP_SKB_CB(skb)->ack_seq + tp->reordering *
- tp->mss_cache))) {
- TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_RETRANS;
- tp->retrans_out -= tcp_skb_pcount(skb);
-
- /* clear lost hint */
- tp->retransmit_skb_hint = NULL;
-
- if (!(TCP_SKB_CB(skb)->sacked&(TCPCB_LOST|TCPCB_SACKED_ACKED))) {
- tp->lost_out += tcp_skb_pcount(skb);
- TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
- flag |= FLAG_DATA_SACKED;
- NET_INC_STATS_BH(LINUX_MIB_TCPLOSTRETRANSMIT);
- }
- }
- }
- }
+ if (lost_retrans && icsk->icsk_ca_state == TCP_CA_Recovery)
+ flag |= tcp_mark_lost_retrans(sk, lost_retrans);
tcp_verify_left_out(tp);
--
1.5.0.6
^ permalink raw reply related
* [RFC PATCH] [TCP]: Limit processing lost_retrans loop to work-to-do cases
From: Ilpo Järvinen @ 2007-10-09 12:20 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Stephen Hemminger, TAKANO Ryousei
In-Reply-To: <119193240269-git-send-email-ilpo.jarvinen@helsinki.fi>
This addition of lost_retrans_low to tcp_sock might be
unnecessary, it's not clear how often lost_retrans worker is
executed when there wasn't work to do.
Cc: TAKANO Ryousei <takano@axe-inc.co.jp>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/linux/tcp.h | 2 ++
net/ipv4/tcp_input.c | 15 +++++++++++----
net/ipv4/tcp_output.c | 2 ++
3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 9ff456e..c5b94c1 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -348,6 +348,8 @@ struct tcp_sock {
int lost_cnt_hint;
int retransmit_cnt_hint;
+ u32 lost_retrans_low; /* Sent seq after any rxmit (lowest) */
+
u16 advmss; /* Advertised MSS */
u16 prior_ssthresh; /* ssthresh saved at recovery start */
u32 lost_out; /* Lost packets */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b90c2fc..e7bc720 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1112,7 +1112,8 @@ static int tcp_is_sackblock_valid(struct tcp_sock *tp, int is_dsack,
*
* Search retransmitted skbs from write_queue that were sent when snd_nxt was
* less than what is now known to be received by the other end (derived from
- * SACK blocks by the caller).
+ * SACK blocks by the caller). Also calculate the lowest snd_nxt among the
+ * remaining retransmitted skbs to avoid some costly processing per ACKs.
*/
static int tcp_mark_lost_retrans(struct sock *sk, u32 received_upto)
{
@@ -1120,6 +1121,7 @@ static int tcp_mark_lost_retrans(struct sock *sk, u32 received_upto)
struct sk_buff *skb;
int flag = 0;
int cnt = 0;
+ u32 new_low_seq = 0;
tcp_for_write_queue(skb, sk) {
u32 ack_seq = TCP_SKB_CB(skb)->ack_seq;
@@ -1150,10 +1152,15 @@ static int tcp_mark_lost_retrans(struct sock *sk, u32 received_upto)
flag |= FLAG_DATA_SACKED;
NET_INC_STATS_BH(LINUX_MIB_TCPLOSTRETRANSMIT);
}
- } else {
+ } else if (!new_low_seq || before(ack_seq, new_low_seq)) {
+ new_low_seq = ack_seq;
cnt += tcp_skb_pcount(skb);
}
}
+
+ if (tp->retrans_out)
+ tp->lost_retrans_low = new_low_seq;
+
return flag;
}
@@ -1468,8 +1475,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
}
}
- if (tp->retrans_out && highest_sack_end_seq &&
- after(highest_sack_end_seq, tp->high_seq) &&
+ if (tp->retrans_out &&
+ after(highest_sack_end_seq, tp->lost_retrans_low) &&
icsk->icsk_ca_state == TCP_CA_Recovery)
flag |= tcp_mark_lost_retrans(sk, highest_sack_end_seq);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 5329675..324b420 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1914,6 +1914,8 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb)
printk(KERN_DEBUG "retrans_out leaked.\n");
}
#endif
+ if (!tp->retrans_out)
+ tp->lost_retrans_low = tp->snd_nxt;
TCP_SKB_CB(skb)->sacked |= TCPCB_RETRANS;
tp->retrans_out += tcp_skb_pcount(skb);
--
1.5.0.6
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox