* Re: [PATCH] ipv4: Cache source address in nexthop entries.
From: David Miller @ 2011-03-08 18:38 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1103081030090.1529@ja.ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Tue, 8 Mar 2011 11:57:38 +0200 (EET)
> It means, even if there are addresses on the
> concerned device it does not mean the routes on this device
> are required to use prefsrc from this device. We must
> restrict the scope according to the provided for the
> route: cfg->fc_scope.
Simple to fix, I'll remember the fc_scope value in the nexthop
and use that in the address selection call.
> As the addresses have nothing to do with the link
> state, I don't think it is correct to call fib_update_nh_saddrs
> for DEV events.
fib_update_nh_saddrs() gets called for "DEV" events since those
are what are emitted when addresses are addded and removed.
^ permalink raw reply
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: Stephen Hemminger @ 2011-03-08 18:42 UTC (permalink / raw)
To: Lucas Nussbaum; +Cc: netdev, Sangtae Ha
In-Reply-To: <20110308080926.GA22641@xanadu.blop.info>
On Tue, 8 Mar 2011 09:09:26 +0100
Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
> The Hystart slow start algorithm requires precise RTT delay measurements
> to decide when to leave slow start. However, currently, CUBIC doesn't
> enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
> and to leave slow start too early, generating bad performance since
> convergence to the optimal cwnd is slower.
>
> Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.
>
> Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>
Just to explain what RTT_STAMP does. It causes the tcp receive code
to compute the rtt using high resolution clocks rather than just
jiffies. It requires access to ktime_get_real which means accessing
clock source. This is cheap for TSC, a little expensive for HPET but
expensive for PIT. I worry that enabling it may hurt regular users
on old desktops. But without it enabling RTT_STAMP, packets that
get acked in less than a jiffie (1 - 10 ms) will
Also I should have used ktime_get rather than ktime_get_real
because real time is altered by NTP and other actions.
^ permalink raw reply
* Re: ANNOUNCE: debloat-testing kernel git tree
From: Rick Jones @ 2011-03-08 18:48 UTC (permalink / raw)
To: Pavel Machek
Cc: Tianji Li, Dave T?ht, sedat.dilek-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
bloat-devel-JXvr2/1DY2fm6VMwtOF2vx4hnT+Y9+D1,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110308065819.GA1357-+ZI9xUNit7I@public.gmane.org>
On Tue, 2011-03-08 at 07:58 +0100, Pavel Machek wrote:
> Bandwidth varies greatly with time. On cellphone, you can go from
> 5KB/sec GPRS to 300KB/sec HSDPA and back...
How quickly/often and does that trigger an event in the host that would
allow adjustment of the queue?
rick jones
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3] tcp: ioctl type SIOCOUTQNSD returns amount of data not sent
From: David Miller @ 2011-03-08 18:48 UTC (permalink / raw)
To: sledz
Cc: netdev, alan, linux-kernel, kuznet, pekkas, jmorris, yoshfuji,
kaber, m.schuknecht
In-Reply-To: <4D7619A2.4090207@dresearch.de>
From: Steffen Sledz <sledz@dresearch.de>
Date: Tue, 08 Mar 2011 12:57:22 +0100
> Anything else to do?
Be patient.
^ permalink raw reply
* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: David Miller @ 2011-03-08 18:52 UTC (permalink / raw)
To: chris.mason; +Cc: nhorman, netdev, rds-devel, venkat.x.venkatsubra
In-Reply-To: <1299530363-sup-8922@think>
From: Chris Mason <chris.mason@oracle.com>
Date: Mon, 07 Mar 2011 15:41:04 -0500
> Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
>> From: Neil Horman <nhorman@tuxdriver.com>
>> Date: Wed, 2 Mar 2011 11:28:22 -0500
>>
>> > Recently had this bug halt reported to me:
>>
>> Well, does anyone on the RDS team care about this bug fix at all?
>>
>> Stuff like this should not sit for nearly a week without any reply
>> whatsoever.
>>
>
> The patch looks good to me, but I'm surprised we haven't seen it here.
> Venkat please take a look (link to the patch below) Has it only been seen on ppc?
>
> http://permalink.gmane.org/gmane.linux.network/187933
Can I get an ACK or something, _please_?
^ permalink raw reply
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: David Miller @ 2011-03-08 18:55 UTC (permalink / raw)
To: shemminger; +Cc: lucas.nussbaum, netdev, sha2
In-Reply-To: <20110308104211.3567526f@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Tue, 8 Mar 2011 10:42:11 -0800
> On Tue, 8 Mar 2011 09:09:26 +0100
> Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
>
>> The Hystart slow start algorithm requires precise RTT delay measurements
>> to decide when to leave slow start. However, currently, CUBIC doesn't
>> enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
>> and to leave slow start too early, generating bad performance since
>> convergence to the optimal cwnd is slower.
>>
>> Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.
>>
>> Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>
>
> Just to explain what RTT_STAMP does. It causes the tcp receive code
> to compute the rtt using high resolution clocks rather than just
> jiffies. It requires access to ktime_get_real which means accessing
> clock source. This is cheap for TSC, a little expensive for HPET but
> expensive for PIT. I worry that enabling it may hurt regular users
> on old desktops. But without it enabling RTT_STAMP, packets that
> get acked in less than a jiffie (1 - 10 ms) will
>
> Also I should have used ktime_get rather than ktime_get_real
> because real time is altered by NTP and other actions.
This also means that whatever testing was originally done on Hystart
is dependent upon whatever value CONFIG_HZ had in the test kernel.
^ permalink raw reply
* Re: [Drbd-dev] [PATCH 2/2 v2] netlink: kill eff_cap from struct netlink_skb_parms
From: Patrick McHardy @ 2011-03-08 18:54 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Chris Wright, David Miller, linux-fbdev, netdev,
linux-security-module, dm-devel, drbd-dev
In-Reply-To: <20110308183246.GA17412@ioremap.net>
Am 08.03.2011 19:32, schrieb Evgeniy Polyakov:
> Hi Patrick.
>
> On Tue, Mar 08, 2011 at 03:50:47PM +0100, Patrick McHardy (kaber@trash.net) wrote:
>>> Yup, connector is very async at that place, but I wonder why the hell I
>>> ever made that decision. I believe we can replace it with pure sync call
>>> of the registered connector callback, since netlink is synchronous and
>>> no one has any problem with it.
>>
>> Are you going to do this or do you want me to take care of it?
>
> I will return back at the end of the week and will take care of this
> problem. I will not mind if you complete it first though :)
Thanks Evgeniy, I'll give it a shot.
^ permalink raw reply
* [PATCH] cnic: fix double initalization of bnx2 based cards
From: Neil Horman @ 2011-03-08 18:56 UTC (permalink / raw)
To: netdev
Cc: Neil Horman, David S. Miller, Michael Chan, Dmitry Kravkov,
Eddie Wai, Eilon Greenstein
bnx2 cards can work with the cnic driver, but when the cnic driver detects a
bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
bnx2 and bnx2x (the former being a regex subset of the later). This causes
initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
catastrophic, is definately not expected. Fix this by choosing either the bnx2
or bnx2x initalization path, not both
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: David S. Miller <davem@davemloft.net>
CC: Michael Chan <mchan@broadcom.com>
CC: Dmitry Kravkov <dmitry@broadcom.com>
CC: Eddie Wai <waie@broadcom.com>
CC: Eilon Greenstein <eilong@broadcom.com>
---
drivers/net/cnic.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
index 271a1f0..18b59ad 100644
--- a/drivers/net/cnic.c
+++ b/drivers/net/cnic.c
@@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
if (!strcmp(drvinfo.driver, "bnx2"))
cdev = init_bnx2_cnic(dev);
- if (!strcmp(drvinfo.driver, "bnx2x"))
+ else if (!strcmp(drvinfo.driver, "bnx2x"))
cdev = init_bnx2x_cnic(dev);
if (cdev) {
write_lock(&cnic_dev_lock);
--
1.7.4
^ permalink raw reply related
* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Ben Hutchings @ 2011-03-08 18:59 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, David S. Miller, Michael Chan, Dmitry Kravkov, Eddie Wai,
Eilon Greenstein
In-Reply-To: <1299610588-22356-1-git-send-email-nhorman@tuxdriver.com>
On Tue, 2011-03-08 at 13:56 -0500, Neil Horman wrote:
> bnx2 cards can work with the cnic driver, but when the cnic driver detects a
> bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
> bnx2 and bnx2x (the former being a regex subset of the later).
Since when does strcmp() do a regex match?
And why is a function named 'is_cnic_dev' doing initialisation, anyway?
Ben.
> This causes
> initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
> catastrophic, is definately not expected. Fix this by choosing either the bnx2
> or bnx2x initalization path, not both
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: David S. Miller <davem@davemloft.net>
> CC: Michael Chan <mchan@broadcom.com>
> CC: Dmitry Kravkov <dmitry@broadcom.com>
> CC: Eddie Wai <waie@broadcom.com>
> CC: Eilon Greenstein <eilong@broadcom.com>
> ---
> drivers/net/cnic.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
> index 271a1f0..18b59ad 100644
> --- a/drivers/net/cnic.c
> +++ b/drivers/net/cnic.c
> @@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
>
> if (!strcmp(drvinfo.driver, "bnx2"))
> cdev = init_bnx2_cnic(dev);
> - if (!strcmp(drvinfo.driver, "bnx2x"))
> + else if (!strcmp(drvinfo.driver, "bnx2x"))
> cdev = init_bnx2x_cnic(dev);
> if (cdev) {
> write_lock(&cnic_dev_lock);
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: Chris Mason @ 2011-03-08 18:58 UTC (permalink / raw)
To: David Miller; +Cc: nhorman, netdev, rds-devel, venkat.x.venkatsubra
In-Reply-To: <20110308.105223.71117584.davem@davemloft.net>
Excerpts from David Miller's message of 2011-03-08 13:52:23 -0500:
> From: Chris Mason <chris.mason@oracle.com>
> Date: Mon, 07 Mar 2011 15:41:04 -0500
>
> > Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
> >> From: Neil Horman <nhorman@tuxdriver.com>
> >> Date: Wed, 2 Mar 2011 11:28:22 -0500
> >>
> >> > Recently had this bug halt reported to me:
> >>
> >> Well, does anyone on the RDS team care about this bug fix at all?
> >>
> >> Stuff like this should not sit for nearly a week without any reply
> >> whatsoever.
> >>
> >
> > The patch looks good to me, but I'm surprised we haven't seen it here.
> > Venkat please take a look (link to the patch below) Has it only been seen on ppc?
> >
> > http://permalink.gmane.org/gmane.linux.network/187933
>
> Can I get an ACK or something, _please_?
Sorry, wasn't clear:
Acked-by: Chris Mason <chris.mason@oracle.com>
-chris
^ permalink raw reply
* Re: [PATCH] ipv4: Cache source address in nexthop entries.
From: David Miller @ 2011-03-08 19:03 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <20110308.103801.193702247.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Tue, 08 Mar 2011 10:38:01 -0800 (PST)
> From: Julian Anastasov <ja@ssi.bg>
> Date: Tue, 8 Mar 2011 11:57:38 +0200 (EET)
>
>> It means, even if there are addresses on the
>> concerned device it does not mean the routes on this device
>> are required to use prefsrc from this device. We must
>> restrict the scope according to the provided for the
>> route: cfg->fc_scope.
>
> Simple to fix, I'll remember the fc_scope value in the nexthop
> and use that in the address selection call.
I'll push the following into net-next-2.6, thanks Julian:
--------------------
ipv4: Fix scope value used in route src-address caching.
We have to use cfg->fc_scope not the final nh_scope value.
Reported-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/ip_fib.h | 1 +
net/ipv4/fib_semantics.c | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 0e14083..3f6c943 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -51,6 +51,7 @@ struct fib_nh {
struct fib_info *nh_parent;
unsigned nh_flags;
unsigned char nh_scope;
+ unsigned char nh_cfg_scope;
#ifdef CONFIG_IP_ROUTE_MULTIPATH
int nh_weight;
int nh_power;
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 952c737..d73d758 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -854,9 +854,10 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
}
change_nexthops(fi) {
+ nexthop_nh->nh_cfg_scope = cfg->fc_scope;
nexthop_nh->nh_saddr = inet_select_addr(nexthop_nh->nh_dev,
nexthop_nh->nh_gw,
- nexthop_nh->nh_scope);
+ nexthop_nh->nh_cfg_scope);
} endfor_nexthops(fi)
link_it:
@@ -1141,7 +1142,7 @@ void fib_update_nh_saddrs(struct net_device *dev)
continue;
nh->nh_saddr = inet_select_addr(nh->nh_dev,
nh->nh_gw,
- nh->nh_scope);
+ nh->nh_cfg_scope);
}
}
--
1.7.4.1
^ permalink raw reply related
* Re: [net-next-2.6 00/14][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2011-03-08 19:11 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1299551108-23663-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 7 Mar 2011 18:24:54 -0800
> The following series contains fixes and cleanups for e1000e and ixgbe, as
> well as support for FCoE DDP in target mode for ixgbe. In igb, added
> additional stats for OS2BMC feature.
>
> The following are changes since commit 541ac7c9b30ee2ff84ad87f27e0bc069e143afb5:
> bonding: COW before overwriting the destination MAC address
>
> and are available in the git repository at:
> master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-next-2.6 master
Pulled.
Please address the feedback given on the lack of docbook descriptions for
all of those fcoe device ops.
^ permalink raw reply
* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Michael Chan @ 2011-03-08 19:07 UTC (permalink / raw)
To: Ben Hutchings
Cc: Neil Horman, netdev@vger.kernel.org, David S. Miller,
Dmitry Kravkov, Eddie Wai, Eilon Greenstein
In-Reply-To: <1299610768.4957.1.camel@bwh-desktop>
On Tue, 2011-03-08 at 10:59 -0800, Ben Hutchings wrote:
> On Tue, 2011-03-08 at 13:56 -0500, Neil Horman wrote:
> > bnx2 cards can work with the cnic driver, but when the cnic driver detects a
> > bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
> > bnx2 and bnx2x (the former being a regex subset of the later).
>
> Since when does strcmp() do a regex match?
Yeah, strcmp() does NULL-terminated string compare, right?
>
> And why is a function named 'is_cnic_dev' doing initialisation, anyway?
Just data structure init if a supported device is detected.
>
> Ben.
>
> > This causes
> > initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
> > catastrophic, is definately not expected. Fix this by choosing either the bnx2
> > or bnx2x initalization path, not both
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: David S. Miller <davem@davemloft.net>
> > CC: Michael Chan <mchan@broadcom.com>
> > CC: Dmitry Kravkov <dmitry@broadcom.com>
> > CC: Eddie Wai <waie@broadcom.com>
> > CC: Eilon Greenstein <eilong@broadcom.com>
> > ---
> > drivers/net/cnic.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
> > index 271a1f0..18b59ad 100644
> > --- a/drivers/net/cnic.c
> > +++ b/drivers/net/cnic.c
> > @@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
> >
> > if (!strcmp(drvinfo.driver, "bnx2"))
> > cdev = init_bnx2_cnic(dev);
> > - if (!strcmp(drvinfo.driver, "bnx2x"))
> > + else if (!strcmp(drvinfo.driver, "bnx2x"))
> > cdev = init_bnx2x_cnic(dev);
> > if (cdev) {
> > write_lock(&cnic_dev_lock);
>
^ permalink raw reply
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: Sangtae Ha @ 2011-03-08 19:15 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, lucas.nussbaum, netdev
In-Reply-To: <20110308.105503.104060436.davem@davemloft.net>
Yes. I remember that CONFIG_HZ was 1000 at that time and the value of
CONFIG_HZ could affect the algorithm.
I don't think HyStart needs this extra RTT_STAMP since we only need a
rough delay estimate.
Let me check HyStart with the latest git and with different CONFIG_HZ values.
Sangtae
On Tue, Mar 8, 2011 at 1:55 PM, David Miller <davem@davemloft.net> wrote:
>
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Tue, 8 Mar 2011 10:42:11 -0800
>
> > On Tue, 8 Mar 2011 09:09:26 +0100
> > Lucas Nussbaum <lucas.nussbaum@loria.fr> wrote:
> >
> >> The Hystart slow start algorithm requires precise RTT delay measurements
> >> to decide when to leave slow start. However, currently, CUBIC doesn't
> >> enable TCP timestamps. This can cause Hystart to mis-estimate the RTT,
> >> and to leave slow start too early, generating bad performance since
> >> convergence to the optimal cwnd is slower.
> >>
> >> Timestamps are already used by TCP Illinois, LP, Vegas, Veno and Yeah.
> >>
> >> Signed-off-by: Lucas Nussbaum <lucas.nussbaum@loria.fr>
> >
> > Just to explain what RTT_STAMP does. It causes the tcp receive code
> > to compute the rtt using high resolution clocks rather than just
> > jiffies. It requires access to ktime_get_real which means accessing
> > clock source. This is cheap for TSC, a little expensive for HPET but
> > expensive for PIT. I worry that enabling it may hurt regular users
> > on old desktops. But without it enabling RTT_STAMP, packets that
> > get acked in less than a jiffie (1 - 10 ms) will
> >
> > Also I should have used ktime_get rather than ktime_get_real
> > because real time is altered by NTP and other actions.
>
> This also means that whatever testing was originally done on Hystart
> is dependent upon whatever value CONFIG_HZ had in the test kernel.
^ permalink raw reply
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: Stephen Hemminger @ 2011-03-08 19:17 UTC (permalink / raw)
To: David Miller; +Cc: lucas.nussbaum, netdev, sha2
In-Reply-To: <20110308.105503.104060436.davem@davemloft.net>
On Tue, 08 Mar 2011 10:55:03 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
Complete the sentence...
> on old desktops.
But without it enabling RTT_STAMP, packets that
get acked in less than a jiffie (1 - 10 ms) will
be processed with no rtt value (-1); this causes hystart
never to be invoked on local connections.
Maybe the following would be better, it passes 0 as RTT
when not using RTT_STAMP for quick packets.
It turns out the Cubic ends up converting rtt_us back
to jiffies anyway, so RTT_STAMP is not really needed.
----
Subject: tcp: fix RTT for quick packets in congestion control
In the congestion control interface, the callback for each ACK
includes an estimated round trip time in microseconds.
Some algorithms need high resolution (Vegas style) but most only
need jiffie resolution. If RTT is not accurate (like a retransmission)
-1 is used as a flag value.
When doing coarse resolution if RTT is less than a a jiffie
then 0 should be returned rather than no estimate. Otherwise algorithms
that expect good ack's to trigger slow start (like CUBIC Hystart)
will be confused.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/ipv4/tcp_input.c 2011-03-08 11:11:26.093183654 -0800
+++ b/net/ipv4/tcp_input.c 2011-03-08 11:11:46.641404939 -0800
@@ -3350,7 +3350,7 @@ static int tcp_clean_rtx_queue(struct so
net_invalid_timestamp()))
rtt_us = ktime_us_delta(ktime_get_real(),
last_ackt);
- else if (ca_seq_rtt > 0)
+ else if (ca_seq_rtt >= 0)
rtt_us = jiffies_to_usecs(ca_seq_rtt);
}
^ permalink raw reply
* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: David Miller @ 2011-03-08 19:22 UTC (permalink / raw)
To: chris.mason; +Cc: nhorman, netdev, rds-devel, venkat.x.venkatsubra
In-Reply-To: <1299610642-sup-5101@think>
From: Chris Mason <chris.mason@oracle.com>
Date: Tue, 08 Mar 2011 13:58:57 -0500
> Excerpts from David Miller's message of 2011-03-08 13:52:23 -0500:
>> From: Chris Mason <chris.mason@oracle.com>
>> Date: Mon, 07 Mar 2011 15:41:04 -0500
>>
>> > Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
>> >> From: Neil Horman <nhorman@tuxdriver.com>
>> >> Date: Wed, 2 Mar 2011 11:28:22 -0500
>> >>
>> >> > Recently had this bug halt reported to me:
>> >>
>> >> Well, does anyone on the RDS team care about this bug fix at all?
>> >>
>> >> Stuff like this should not sit for nearly a week without any reply
>> >> whatsoever.
>> >>
>> >
>> > The patch looks good to me, but I'm surprised we haven't seen it here.
>> > Venkat please take a look (link to the patch below) Has it only been seen on ppc?
>> >
>> > http://permalink.gmane.org/gmane.linux.network/187933
>>
>> Can I get an ACK or something, _please_?
>
> Sorry, wasn't clear:
>
> Acked-by: Chris Mason <chris.mason@oracle.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
From: David Miller @ 2011-03-08 19:29 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
In-Reply-To: <1299592626-18510-1-git-send-email-remi.denis-courmont@nokia.com>
From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Tue, 8 Mar 2011 15:56:59 +0200
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
You don't even read what I tell you, which makes reviewing your work
insanely frustrating.
I said:
Reference other changes like a man, by mentioned the SHA1 ID and
giving the commit message header line inside of ("") right
afterwards. :-)
I didn't say to put the SHA1 reference in the subject line, it's
so messy there. Put it in the commit message. Do you see other
people putting commit ID references in the commit header line?
Also, I said explicitly to reference the commit like this:
$(SHA1_ID) ("commit message header line")
so in this case it would be:
a8059512b120362b15424f152b2548fe8b11bd0c ("Phonet: implement
per-socket destination/peer address")
because when commits get backported that SHA1_ID might be different
in the tree it gets backported to, so you have to provide that
textual reference to the commit so that in such a case it can still
be matched up.
It also irks me that you persisted to provide the most terse possible
one-line commit message. Have a conversation in your commit message,
this isn't for you it's for other people trying to understand your
work.
The commit message header line just gives a top-level description.
Details like the reasoning, reference to relevant commits, etc.
belong in the commit message contents.
^ permalink raw reply
* Re: pull request: sfc-next-2.6 2011-03-07
From: David Miller @ 2011-03-08 19:33 UTC (permalink / raw)
To: bhutchings; +Cc: linux-net-drivers, netdev
In-Reply-To: <1299540194.2522.46.camel@bwh-desktop>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Mon, 07 Mar 2011 23:23:14 +0000
> The following changes since commit 07df5294a753dfac2cc9f75e6159fc25fdc22149:
>
> inet: Replace left-over references to inet->cork (2011-03-01 23:00:58 -0800)
>
> are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6.git for-davem
>
> Just one more performance optimisation.
Pulled, thanks Ben.
^ permalink raw reply
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: Lucas Nussbaum @ 2011-03-08 19:36 UTC (permalink / raw)
To: Sangtae Ha; +Cc: David Miller, shemminger, netdev
In-Reply-To: <AANLkTinT0eHbbELWDLxYyj0axpYt=GHnxLP8fMH-pWOs@mail.gmail.com>
On 08/03/11 at 14:15 -0500, Sangtae Ha wrote:
> Yes. I remember that CONFIG_HZ was 1000 at that time and the value of
> CONFIG_HZ could affect the algorithm.
> I don't think HyStart needs this extra RTT_STAMP since we only need a
> rough delay estimate.
> Let me check HyStart with the latest git and with different CONFIG_HZ values.
One of the problems I discovered was that ca->delay_min was completely
off if RTT_STAMP is disabled. For example, on a link with a 11ms RTT, I
could get delay_min = 4ms. But even with RTT_STAMP, there's the problem
that the code rounds the computed RTT value to a jiffie (in
bictcp_acked()).
My other patch mitigates the performance problem by making it harder for
Hystart to abort slow start. But after a few days working on this issue,
I'm wondering whether Hystart shouldn't be completely rewritten to work
on the usec values, or disabled by default.
--
| Lucas Nussbaum MCF Université Nancy 2 |
| lucas.nussbaum@loria.fr LORIA / AlGorille |
| http://www.loria.fr/~lnussbau/ +33 3 54 95 86 19 |
^ permalink raw reply
* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Neil Horman @ 2011-03-08 19:36 UTC (permalink / raw)
To: Michael Chan
Cc: Ben Hutchings, netdev@vger.kernel.org, David S. Miller,
Dmitry Kravkov, Eddie Wai, Eilon Greenstein
In-Reply-To: <1299611277.12601.15.camel@HP1>
On Tue, Mar 08, 2011 at 11:07:57AM -0800, Michael Chan wrote:
>
> On Tue, 2011-03-08 at 10:59 -0800, Ben Hutchings wrote:
> > On Tue, 2011-03-08 at 13:56 -0500, Neil Horman wrote:
> > > bnx2 cards can work with the cnic driver, but when the cnic driver detects a
> > > bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
> > > bnx2 and bnx2x (the former being a regex subset of the later).
> >
> > Since when does strcmp() do a regex match?
>
> Yeah, strcmp() does NULL-terminated string compare, right?
>
Sorry, poor choice of words. It doesn't do a regex match, I only ment to
illustrate that bnx2 is a substring of bnx2x.
strcmp does this:
strcmp(const char *str1, const char *str2) {
...
while (*s1 || *s2) {
...
}
...
}
Since bnx2 is a substring of bnx2 both if clauses are a match, since you'll hit
the null terminator of the shorter string in both, which means we call both
initalization functions.
I'll leave commentary on initalization in is_cnic_dev to the authors :)
Neil
>
> >
> > Ben.
> >
> > > This causes
> > > initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
> > > catastrophic, is definately not expected. Fix this by choosing either the bnx2
> > > or bnx2x initalization path, not both
> > >
> > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > CC: David S. Miller <davem@davemloft.net>
> > > CC: Michael Chan <mchan@broadcom.com>
> > > CC: Dmitry Kravkov <dmitry@broadcom.com>
> > > CC: Eddie Wai <waie@broadcom.com>
> > > CC: Eilon Greenstein <eilong@broadcom.com>
> > > ---
> > > drivers/net/cnic.c | 2 +-
> > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
> > > index 271a1f0..18b59ad 100644
> > > --- a/drivers/net/cnic.c
> > > +++ b/drivers/net/cnic.c
> > > @@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
> > >
> > > if (!strcmp(drvinfo.driver, "bnx2"))
> > > cdev = init_bnx2_cnic(dev);
> > > - if (!strcmp(drvinfo.driver, "bnx2x"))
> > > + else if (!strcmp(drvinfo.driver, "bnx2x"))
> > > cdev = init_bnx2x_cnic(dev);
> > > if (cdev) {
> > > write_lock(&cnic_dev_lock);
> >
>
>
> --
> 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
* Re: [rds-devel] [PATCH] rds: prevent BUG_ON triggering on congestion map updates
From: Neil Horman @ 2011-03-08 19:36 UTC (permalink / raw)
To: David Miller; +Cc: chris.mason, netdev, rds-devel, venkat.x.venkatsubra
In-Reply-To: <20110308.112258.183048712.davem@davemloft.net>
On Tue, Mar 08, 2011 at 11:22:58AM -0800, David Miller wrote:
> From: Chris Mason <chris.mason@oracle.com>
> Date: Tue, 08 Mar 2011 13:58:57 -0500
>
> > Excerpts from David Miller's message of 2011-03-08 13:52:23 -0500:
> >> From: Chris Mason <chris.mason@oracle.com>
> >> Date: Mon, 07 Mar 2011 15:41:04 -0500
> >>
> >> > Excerpts from David Miller's message of 2011-03-07 15:27:53 -0500:
> >> >> From: Neil Horman <nhorman@tuxdriver.com>
> >> >> Date: Wed, 2 Mar 2011 11:28:22 -0500
> >> >>
> >> >> > Recently had this bug halt reported to me:
> >> >>
> >> >> Well, does anyone on the RDS team care about this bug fix at all?
> >> >>
> >> >> Stuff like this should not sit for nearly a week without any reply
> >> >> whatsoever.
> >> >>
> >> >
> >> > The patch looks good to me, but I'm surprised we haven't seen it here.
> >> > Venkat please take a look (link to the patch below) Has it only been seen on ppc?
> >> >
> >> > http://permalink.gmane.org/gmane.linux.network/187933
> >>
> >> Can I get an ACK or something, _please_?
> >
> > Sorry, wasn't clear:
> >
> > Acked-by: Chris Mason <chris.mason@oracle.com>
>
> Applied, thanks everyone.
Thanks, Chris, Dave.
Neil
> --
> 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
* Re: [PATCH] tcp_cubic: enable TCP timestamps
From: David Miller @ 2011-03-08 19:38 UTC (permalink / raw)
To: lucas.nussbaum; +Cc: sangtae.ha, shemminger, netdev
In-Reply-To: <20110308193602.GA14526@xanadu.blop.info>
From: Lucas Nussbaum <lucas.nussbaum@loria.fr>
Date: Tue, 8 Mar 2011 20:36:02 +0100
> My other patch mitigates the performance problem by making it harder for
> Hystart to abort slow start. But after a few days working on this issue,
> I'm wondering whether Hystart shouldn't be completely rewritten to work
> on the usec values, or disabled by default.
I am of the opinion that Hystart, like TCP VEGAS, fundamentally cannot
work.
It's disabled in several major distributions because of the performance
problems you noticed, and this was on my own personal recommendation.
^ permalink raw reply
* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Ben Hutchings @ 2011-03-08 19:40 UTC (permalink / raw)
To: Neil Horman
Cc: Michael Chan, netdev@vger.kernel.org, David S. Miller,
Dmitry Kravkov, Eddie Wai, Eilon Greenstein
In-Reply-To: <20110308193601.GA19669@hmsreliant.think-freely.org>
On Tue, 2011-03-08 at 14:36 -0500, Neil Horman wrote:
> On Tue, Mar 08, 2011 at 11:07:57AM -0800, Michael Chan wrote:
> >
> > On Tue, 2011-03-08 at 10:59 -0800, Ben Hutchings wrote:
> > > On Tue, 2011-03-08 at 13:56 -0500, Neil Horman wrote:
> > > > bnx2 cards can work with the cnic driver, but when the cnic driver detects a
> > > > bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
> > > > bnx2 and bnx2x (the former being a regex subset of the later).
> > >
> > > Since when does strcmp() do a regex match?
> >
> > Yeah, strcmp() does NULL-terminated string compare, right?
> >
> Sorry, poor choice of words. It doesn't do a regex match, I only ment to
> illustrate that bnx2 is a substring of bnx2x.
>
> strcmp does this:
>
> strcmp(const char *str1, const char *str2) {
> ...
> while (*s1 || *s2) {
> ...
> }
> ...
> }
>
> Since bnx2 is a substring of bnx2 both if clauses are a match, since you'll hit
> the null terminator of the shorter string in both, which means we call both
> initalization functions.
[...]
If you don't even know what strcmp() does, what are you doing hacking on
the kernel?
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] Make CUBIC Hystart more robust to RTT variations
From: David Miller @ 2011-03-08 19:43 UTC (permalink / raw)
To: rhee; +Cc: lucas.nussbaum, xiyou.wangcong, netdev
In-Reply-To: <4D764AAC.30302@ncsu.edu>
From: Injong Rhee <rhee@ncsu.edu>
Date: Tue, 08 Mar 2011 10:26:36 -0500
> Thanks for updating CUBIC hystart. You might want to test the
> cases with more background traffic and verify whether this
> threshold is too conservative.
So let's get down to basics.
What does Hystart do specially that allows it to avoid all of the
problems that TCP VEGAS runs into.
Specifically, that if you use RTTs to make congestion control
decisions it is impossible to notice new bandwidth becomming available
fast enough.
Again, it's impossible to react fast enough. No matter what you tweak
all of your various settings to, this problem will still exist.
This is a core issue, you cannot get around it.
This is why I feel that Hystart is fundamentally flawed and we should
turn it off by default if not flat-out remove it.
Distributions are turning it off by default already, therefore it's
stupid for the upstream kernel to behave differently if that's what
%99 of the world is going to end up experiencing.
^ permalink raw reply
* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Michael Chan @ 2011-03-08 19:43 UTC (permalink / raw)
To: Neil Horman
Cc: Ben Hutchings, netdev@vger.kernel.org, David S. Miller,
Dmitry Kravkov, Eddie Wai, Eilon Greenstein
In-Reply-To: <20110308193601.GA19669@hmsreliant.think-freely.org>
On Tue, 2011-03-08 at 11:36 -0800, Neil Horman wrote:
> On Tue, Mar 08, 2011 at 11:07:57AM -0800, Michael Chan wrote:
> >
> > On Tue, 2011-03-08 at 10:59 -0800, Ben Hutchings wrote:
> > > On Tue, 2011-03-08 at 13:56 -0500, Neil Horman wrote:
> > > > bnx2 cards can work with the cnic driver, but when the cnic driver detects a
> > > > bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
> > > > bnx2 and bnx2x (the former being a regex subset of the later).
> > >
> > > Since when does strcmp() do a regex match?
> >
> > Yeah, strcmp() does NULL-terminated string compare, right?
> >
> Sorry, poor choice of words. It doesn't do a regex match, I only ment to
> illustrate that bnx2 is a substring of bnx2x.
>
> strcmp does this:
>
> strcmp(const char *str1, const char *str2) {
> ...
> while (*s1 || *s2) {
> ...
> }
> ...
> }
>
> Since bnx2 is a substring of bnx2 both if clauses are a match, since you'll hit
> the null terminator of the shorter string in both, which means we call both
> initalization functions.
No, that should not happen. When we compare NULL-terminated "bnx2" with
NULL-terminated "bnx2x", it won't match because the '\0' won't match the
'x', right?
I think the patch is good. We can avoid the unnecessary strcmp() when
we have a match with "bnx2" already. But we should not get both matches
even without the patch.
>
> I'll leave commentary on initalization in is_cnic_dev to the authors :)
>
> Neil
>
>
> >
> > >
> > > Ben.
> > >
> > > > This causes
> > > > initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
> > > > catastrophic, is definately not expected. Fix this by choosing either the bnx2
> > > > or bnx2x initalization path, not both
> > > >
> > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > CC: David S. Miller <davem@davemloft.net>
> > > > CC: Michael Chan <mchan@broadcom.com>
> > > > CC: Dmitry Kravkov <dmitry@broadcom.com>
> > > > CC: Eddie Wai <waie@broadcom.com>
> > > > CC: Eilon Greenstein <eilong@broadcom.com>
> > > > ---
> > > > drivers/net/cnic.c | 2 +-
> > > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
> > > > index 271a1f0..18b59ad 100644
> > > > --- a/drivers/net/cnic.c
> > > > +++ b/drivers/net/cnic.c
> > > > @@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
> > > >
> > > > if (!strcmp(drvinfo.driver, "bnx2"))
> > > > cdev = init_bnx2_cnic(dev);
> > > > - if (!strcmp(drvinfo.driver, "bnx2x"))
> > > > + else if (!strcmp(drvinfo.driver, "bnx2x"))
> > > > cdev = init_bnx2x_cnic(dev);
> > > > if (cdev) {
> > > > write_lock(&cnic_dev_lock);
> > >
> >
> >
> > --
> > 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
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