* Re: SFQ qdisc crashes with limit of 2 packets
From: Patrick McHardy @ 2007-09-18 19:15 UTC (permalink / raw)
To: Chuck Ebbert; +Cc: Netdev, Alexey Kuznetsov
In-Reply-To: <46F0117A.4060807@trash.net>
[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]
Patrick McHardy wrote:
> Never mind, I found the reason. When enqueuing the packet, sfq_enqueue
> contains an off-by-one in the limit check (which IIRC is there for a
> reason, but I can't remember right now) and drops the packet again.
> dev_queue_xmit() calls qdisc_run() anyway and the empty qdisc is
> dequeued, which is not handled by SFQ.
>
> I see three possibilities to fix this (in my preferred order):
>
> 1) figure out why the off-by-one is there, if not needed remove
> 2) don't dequeue qdiscs even once if empty
> 3) check for NULL in sfq_dequeue
>
> So I'll try to remeber why the off-by-one is there ..
OK the off-by-one prevents an out-of-bounds array access, which
would cause a crash itself. Despite what I said above, sfq does
try to handle dequeues while empty, but forgets to update q->tail
when dropping the last packet from the only active queue, probably
because it wasn't expected that the queue length is too small to
queue even a single packet (and that really doesn't make much sense).
So one possibility for fixing this is to update q->tail in sfq_drop
when dropping the last packet, but that would still leave the qdisc
non-functional because of the off-by-one. I chose a different way:
cap the limit at SFQ_DEPTH-1 and remove the off-by-one, which should
have no effect on the max (still 127), but prevents the crash since
we can now queue at least a single packet and q->tail is properly
updated in sfq_dequeue().
CCed Alexey just to be safe, but I think the patch should be fine.
Signed-off-by: Patrick McHardy <kaber@trash.net>
[-- Attachment #2: y --]
[-- Type: text/plain, Size: 1347 bytes --]
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 9579573..cbf8089 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -270,7 +270,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
q->tail = x;
}
}
- if (++sch->q.qlen < q->limit-1) {
+ if (++sch->q.qlen < q->limit) {
sch->bstats.bytes += skb->len;
sch->bstats.packets++;
return 0;
@@ -306,7 +306,7 @@ sfq_requeue(struct sk_buff *skb, struct Qdisc* sch)
q->tail = x;
}
}
- if (++sch->q.qlen < q->limit - 1) {
+ if (++sch->q.qlen < q->limit) {
sch->qstats.requeues++;
return 0;
}
@@ -391,10 +391,10 @@ static int sfq_change(struct Qdisc *sch, struct rtattr *opt)
q->quantum = ctl->quantum ? : psched_mtu(sch->dev);
q->perturb_period = ctl->perturb_period*HZ;
if (ctl->limit)
- q->limit = min_t(u32, ctl->limit, SFQ_DEPTH);
+ q->limit = min_t(u32, ctl->limit, SFQ_DEPTH - 1);
qlen = sch->q.qlen;
- while (sch->q.qlen >= q->limit-1)
+ while (sch->q.qlen >= q->limit)
sfq_drop(sch);
qdisc_tree_decrease_qlen(sch, qlen - sch->q.qlen);
@@ -423,7 +423,7 @@ static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
q->dep[i+SFQ_DEPTH].next = i+SFQ_DEPTH;
q->dep[i+SFQ_DEPTH].prev = i+SFQ_DEPTH;
}
- q->limit = SFQ_DEPTH;
+ q->limit = SFQ_DEPTH - 1;
q->max_depth = 0;
q->tail = SFQ_DEPTH;
if (opt == NULL) {
^ permalink raw reply related
* Re: Please pull 'iwlwifi' branch of wireless-2.6
From: David Miller @ 2007-09-18 19:22 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: jeff-o2qLIJkoznsdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20070918185040.GF4940-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Tue, 18 Sep 2007 14:50:40 -0400
> Jeff & Dave,
>
> Here it is -- it's big, it's...well...beautiful in its own way...well,
> at least it seems to work... :-)
>
> There are some outstanding issues. The driver does more than it
> probably should under the covers instead of in the stack, and the
> issue of including headers with a "../../mac80211/..." path remains.
> Still, I think it would be better to get this mainlined than to keep
> it out of stream.
Jeff, if you have no objections I'll pull this into net-2.6.24
Thanks.
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: H. Peter Anvin @ 2007-09-18 19:27 UTC (permalink / raw)
To: David Miller; +Cc: mchan, vda.linux, linux-kernel, netdev
In-Reply-To: <20070918.122034.83625273.davem@davemloft.net>
David Miller wrote:
> From: "H. Peter Anvin" <hpa@zytor.com>
> Date: Tue, 18 Sep 2007 11:41:34 -0700
>
>> David Miller wrote:
>>> I don't like it because it means people have to setup full initrd's
>>> in order to do network booting with such network cards.
>>>
>> klibc could help with that, if there is interest in exploring that
>> avenue again.
>
> I appreciate the effort you put into klibc and the offer to
> make initrd's easier to build.
>
> But the point is that the initrd shouldn't be necessary in the first
> place. There becomes zero point in building these drivers statically
> into the kernel, which many of us do specifically to avoid module
> loading, initrds, and all that fuss. Because the driver is totally
> crippled even though it's been fully built into the main kernel image.
>
> I mean, it's so incredibly stupid and makes kernel development that
> much more difficult.
>
> Every new dependency, be it requiring initrd or something else,
> is one more barrier added to kernel development.
>
> I really pine for the days where everything was so simple, and initrd
> and modules were the odd ball cases, most developers built everything
> into their kernel image.
Well, what I was referring to here, of course, was the initramfs
integrated in the kernel image, so it all comes out of the kernel build
tree and produces a single bootable image. The fact that part of it
contains userspace code is in that way invisible.
That was kind of the point here, and the only reason for pushing klibc
into the kernel build tree at all. Under the "distros use external
initrd anyway" school of thought, whatever libc used for that can be
external anyway.
-hpa
^ permalink raw reply
* Re: [PATCH] phy: export phy_mii_ioctl
From: Jon Smirl @ 2007-09-18 19:29 UTC (permalink / raw)
To: Domen Puncer; +Cc: netdev, linuxppc-embedded, sven
In-Reply-To: <20070918151622.GD32628@nd47.coderock.org>
On 9/18/07, Domen Puncer <domen@coderock.org> wrote:
> More testing and getting it to work properly on Phytec pcm030 would
> be great.
Do we want to do anything about this?
[ 1.569657] net eth0: attached phy 0 to driver Generic PHY
[ 2.576013] Sending DHCP requests .<6>PHY: f0003000:00 - Link is Up
- 100/Full
[ 4.612000] ., OK
[ 6.764005] IP-Config: Got DHCP answer from 192.168.1.200, my
address is 192.168.1.5
What is happening is the printk for "<6>PHY: f0003000:00 - Link is Up
- 100/Full" is done in an interrupt and it comes in the middle of the
kernel doing DHCP and printing ... without a CR.
Two possible solutions, get rid of the link-up message or wait in in
the initial driver load until the link is up. Or we could leave it the
way it is, but some people may report this as a bug.
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* [PATCH] return useful listenq info in tcp_info and INET_DIAG_INFO
From: Rick Jones @ 2007-09-18 19:54 UTC (permalink / raw)
To: netdev
Return some useful information such as the maximum listen backlog and the
current listen backlog in the tcp_info structure and INET_DIAG_INFO.
Signed-off-by: Rick Jones <rick.jones2@hp.com>
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
---
diff -r bdcdd0e1ee9d net/ipv4/tcp.c
--- a/net/ipv4/tcp.c Sat Sep 01 07:00:31 2007 +0000
+++ b/net/ipv4/tcp.c Tue Sep 18 11:02:26 2007 -0700
@@ -2030,8 +2030,14 @@ void tcp_get_info(struct sock *sk, struc
info->tcpi_snd_mss = tp->mss_cache;
info->tcpi_rcv_mss = icsk->icsk_ack.rcv_mss;
- info->tcpi_unacked = tp->packets_out;
- info->tcpi_sacked = tp->sacked_out;
+ if (sk->sk_state == TCP_LISTEN) {
+ info->tcpi_unacked = sk->sk_ack_backlog;
+ info->tcpi_sacked = sk->sk_max_ack_backlog;
+ }
+ else {
+ info->tcpi_unacked = tp->packets_out;
+ info->tcpi_sacked = tp->sacked_out;
+ }
info->tcpi_lost = tp->lost_out;
info->tcpi_retrans = tp->retrans_out;
info->tcpi_fackets = tp->fackets_out;
diff -r bdcdd0e1ee9d net/ipv4/tcp_diag.c
--- a/net/ipv4/tcp_diag.c Sat Sep 01 07:00:31 2007 +0000
+++ b/net/ipv4/tcp_diag.c Tue Sep 18 11:02:26 2007 -0700
@@ -25,11 +25,14 @@ static void tcp_diag_get_info(struct soc
const struct tcp_sock *tp = tcp_sk(sk);
struct tcp_info *info = _info;
- if (sk->sk_state == TCP_LISTEN)
+ if (sk->sk_state == TCP_LISTEN) {
r->idiag_rqueue = sk->sk_ack_backlog;
- else
+ r->idiag_wqueue = sk->sk_max_ack_backlog;
+ }
+ else {
r->idiag_rqueue = tp->rcv_nxt - tp->copied_seq;
- r->idiag_wqueue = tp->write_seq - tp->snd_una;
+ r->idiag_wqueue = tp->write_seq - tp->snd_una;
+ }
if (info != NULL)
tcp_get_info(sk, info);
}
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: Michael Chan @ 2007-09-18 20:05 UTC (permalink / raw)
To: David Miller; +Cc: vda.linux, linux-kernel, netdev
In-Reply-To: <20070918.112337.74737433.davem@davemloft.net>
On Tue, 2007-09-18 at 11:23 -0700, David Miller wrote:
> I don't like it because it means people have to setup full initrd's
> in order to do network booting with such network cards.
>
> But the days of my opinion mattering on that issue are long gone,
> the momentum is just too greatly behind using request_firmware()
> across the board, so there is no reason for bnx2 to be any different.
>
The bnx2 firmware changes quite frequently. A new driver quite often
requires new firmware to work correctly. Splitting them up makes things
difficult for the user.
The firmware in tg3 is a lot more mature and I don't expect it to
change. I think tg3 is better suited for using request_firmware().
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: David Miller @ 2007-09-18 20:08 UTC (permalink / raw)
To: hpa; +Cc: mchan, vda.linux, linux-kernel, netdev
In-Reply-To: <46F02688.8030507@zytor.com>
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Tue, 18 Sep 2007 12:27:04 -0700
> Well, what I was referring to here, of course, was the initramfs
> integrated in the kernel image, so it all comes out of the kernel build
> tree and produces a single bootable image. The fact that part of it
> contains userspace code is in that way invisible.
>
> That was kind of the point here, and the only reason for pushing klibc
> into the kernel build tree at all. Under the "distros use external
> initrd anyway" school of thought, whatever libc used for that can be
> external anyway.
Sounds good to me :)
^ permalink raw reply
* Re: SFQ qdisc crashes with limit of 2 packets
From: David Miller @ 2007-09-18 20:09 UTC (permalink / raw)
To: kaber; +Cc: cebbert, netdev, kuznet
In-Reply-To: <46F023D0.7030307@trash.net>
From: Patrick McHardy <kaber@trash.net>
Date: Tue, 18 Sep 2007 21:15:28 +0200
> OK the off-by-one prevents an out-of-bounds array access, which
> would cause a crash itself. Despite what I said above, sfq does
> try to handle dequeues while empty, but forgets to update q->tail
> when dropping the last packet from the only active queue, probably
> because it wasn't expected that the queue length is too small to
> queue even a single packet (and that really doesn't make much sense).
>
> So one possibility for fixing this is to update q->tail in sfq_drop
> when dropping the last packet, but that would still leave the qdisc
> non-functional because of the off-by-one. I chose a different way:
> cap the limit at SFQ_DEPTH-1 and remove the off-by-one, which should
> have no effect on the max (still 127), but prevents the crash since
> we can now queue at least a single packet and q->tail is properly
> updated in sfq_dequeue().
>
> CCed Alexey just to be safe, but I think the patch should be fine.
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
I've applied this to net-2.6, thanks Patrick.
I'll hold off merging this to Linus until later today so
that if some issue is found we can address it.
Thanks.
^ permalink raw reply
* [ofa-general] Re: [PATCH net-2.6.24] Fix refcounting problem with netif_rx_reschedule()
From: David Miller @ 2007-09-18 20:15 UTC (permalink / raw)
To: rdreier; +Cc: netdev, general
In-Reply-To: <adawsunkg8i.fsf@cisco.com>
From: Roland Dreier <rdreier@cisco.com>
Date: Tue, 18 Sep 2007 10:58:37 -0700
> netif_rx_complete() takes a netdev parameter and does dev_put() on
> that netdev, so netif_rx_reschedule() needs to also take a netdev
> parameter and do dev_hold() on it to avoid reference counts from
> getting becoming negative because of unbalanced dev_put()s.
>
> This should fix the problem reported by Krishna Kumar
> <krkumar2@in.ibm.com> with IPoIB waiting forever for netdev refcounts
> to become 0 during module unload.
>
> Signed-off-by: Roland Dreier <rolandd@cisco.com>
Applied to net-2.6.24, thanks Roland.
> BTW, it looks like drivers/net/ibm_emac/ibm_emac_mal.c would not have
> built in the current net-2.6.24 tree, since its call to
> netif_rx_reschedule() was left with the netdev parameter. So that
> file does not need to be touched in this patch.
Yes, I know, this is the one NAPI driver that hasn't been converted.
It's a complicated conversion because of how the driver and the data
structures have been arranged (in short, a mess) which makes it
insanely difficult to get from a queue instance back up to a network
device or similar.
Further complicating things is that you need to setup a ppc32
cross-build environment to even build test a conversion, and I'm not
comfortable doing the surgery until I can test build the thing.
And this may be hard to believe, but other things have been more
pressing than setting up a ppc32 cross-build environment :-)
This is a hint of anyone looking for something to do that it'd
be much appreciated for someone to tackle the ibm_emac conversion.
Thanks.
^ permalink raw reply
* [PATCH] iproute2: enable setting rto_min, rtt and rttvar in tc-esque units
From: Rick Jones @ 2007-09-18 20:15 UTC (permalink / raw)
To: netdev
Enable users of ip to specify the times for rtt, rttvar and rto_min
in human-friendly terms a la "tc" while maintaining backwards
compatability with the previous "raw" mechanism. Builds upon
David Miller's uncommited patch to set rto_min.
Signed-off-by: Rick Jones <rick.jones2@hp.com>
---
include/linux/rtnetlink.h | 2 +
include/utils.h | 1 +
ip/iproute.c | 30 ++++++++++++++++-----
lib/utils.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
man/man8/ip.8 | 36 ++++++++++++++++++++-----
5 files changed, 119 insertions(+), 14 deletions(-)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 477270c..2494d2c 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -352,6 +352,8 @@ enum
#define RTAX_INITCWND RTAX_INITCWND
RTAX_FEATURES,
#define RTAX_FEATURES RTAX_FEATURES
+ RTAX_RTO_MIN,
+#define RTAX_RTO_MIN RTAX_RTO_MIN
__RTAX_MAX
};
diff --git a/include/utils.h b/include/utils.h
index a3fd335..7da2b29 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -77,6 +77,7 @@ extern int get_prefix(inet_prefix *dst, char *arg, int family);
extern int get_integer(int *val, const char *arg, int base);
extern int get_unsigned(unsigned *val, const char *arg, int base);
+extern int get_jiffies(unsigned *val, const char *arg, int base, int *raw);
#define get_byte get_u8
#define get_ushort get_u16
#define get_short get_s16
diff --git a/ip/iproute.c b/ip/iproute.c
index 6fe4a70..ebb0b2f 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -51,6 +51,7 @@ static const char *mx_names[RTAX_MAX+1] = {
[RTAX_HOPLIMIT] = "hoplimit",
[RTAX_INITCWND] = "initcwnd",
[RTAX_FEATURES] = "features",
+ [RTAX_RTO_MIN] = "rto_min",
};
static void usage(void) __attribute__((noreturn));
@@ -71,9 +72,10 @@ static void usage(void)
fprintf(stderr, "INFO_SPEC := NH OPTIONS FLAGS [ nexthop NH ]...\n");
fprintf(stderr, "NH := [ via ADDRESS ] [ dev STRING ] [ weight NUMBER ] NHFLAGS\n");
fprintf(stderr, "OPTIONS := FLAGS [ mtu NUMBER ] [ advmss NUMBER ]\n");
- fprintf(stderr, " [ rtt NUMBER ] [ rttvar NUMBER ]\n");
+ fprintf(stderr, " [ rtt TIME ] [ rttvar TIME ]\n");
fprintf(stderr, " [ window NUMBER] [ cwnd NUMBER ] [ initcwnd NUMBER ]\n");
fprintf(stderr, " [ ssthresh NUMBER ] [ realms REALM ]\n");
+ fprintf(stderr, " [ rto_min TIME ]\n");
fprintf(stderr, "TYPE := [ unicast | local | broadcast | multicast | throw |\n");
fprintf(stderr, " unreachable | prohibit | blackhole | nat ]\n");
fprintf(stderr, "TABLE_ID := [ local | main | default | all | NUMBER ]\n");
@@ -82,6 +84,7 @@ static void usage(void)
fprintf(stderr, "MP_ALGO := { rr | drr | random | wrandom }\n");
fprintf(stderr, "NHFLAGS := [ onlink | pervasive ]\n");
fprintf(stderr, "RTPROTO := [ kernel | boot | static | NUMBER ]\n");
+ fprintf(stderr, "TIME := NUMBER[s|ms|us|ns|j]\n");
exit(-1);
}
@@ -516,7 +519,8 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
if (mxlock & (1<<i))
fprintf(fp, " lock");
- if (i != RTAX_RTT && i != RTAX_RTTVAR)
+ if (i != RTAX_RTT && i != RTAX_RTTVAR &&
+ i != RTAX_RTO_MIN)
fprintf(fp, " %u", *(unsigned*)RTA_DATA(mxrta[i]));
else {
unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
@@ -524,7 +528,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
val *= 1000;
if (i == RTAX_RTT)
val /= 8;
- else
+ else if (i == RTAX_RTTVAR)
val /= 4;
if (val >= hz)
fprintf(fp, " %ums", val/hz);
@@ -689,6 +693,7 @@ int iproute_modify(int cmd, unsigned flags, int argc, char **argv)
int table_ok = 0;
int proto_ok = 0;
int type_ok = 0;
+ int raw = 0;
memset(&req, 0, sizeof(req));
@@ -796,9 +801,19 @@ int iproute_modify(int cmd, unsigned flags, int argc, char **argv)
mxlock |= (1<<RTAX_RTT);
NEXT_ARG();
}
- if (get_unsigned(&rtt, *argv, 0))
+ if (get_jiffies(&rtt, *argv, 0, &raw))
invarg("\"rtt\" value is invalid\n", *argv);
- rta_addattr32(mxrta, sizeof(mxbuf), RTAX_RTT, rtt);
+ rta_addattr32(mxrta, sizeof(mxbuf), RTAX_RTT,
+ (raw) ? rtt : rtt * 8);
+ } else if (strcmp(*argv, "rto_min") == 0) {
+ unsigned rto_min;
+ NEXT_ARG();
+ mxlock |= (1<<RTAX_RTO_MIN);
+ if (get_jiffies(&rto_min, *argv, 0, &raw))
+ invarg("\"rto_min\" value is invalid\n",
+ *argv);
+ rta_addattr32(mxrta, sizeof(mxbuf), RTAX_RTO_MIN,
+ rto_min);
} else if (matches(*argv, "window") == 0) {
unsigned win;
NEXT_ARG();
@@ -836,9 +851,10 @@ int iproute_modify(int cmd, unsigned flags, int argc, char **argv)
mxlock |= (1<<RTAX_RTTVAR);
NEXT_ARG();
}
- if (get_unsigned(&win, *argv, 0))
+ if (get_jiffies(&win, *argv, 0, &raw))
invarg("\"rttvar\" value is invalid\n", *argv);
- rta_addattr32(mxrta, sizeof(mxbuf), RTAX_RTTVAR, win);
+ rta_addattr32(mxrta, sizeof(mxbuf), RTAX_RTTVAR,
+ (raw) ? win : win * 4);
} else if (matches(*argv, "ssthresh") == 0) {
unsigned win;
NEXT_ARG();
diff --git a/lib/utils.c b/lib/utils.c
index 4f35a60..4c42dfd 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -61,6 +61,70 @@ int get_unsigned(unsigned *val, const char *arg, int base)
return 0;
}
+/*
+ * get_jiffies is "translated" from a similar routine "get_time" in
+ * tc_util.c. we don't use the exact same routine because tc passes
+ * microseconds to the kernel and the callers of get_jiffies want
+ * to pass jiffies, and have a different assumption for the units of
+ * a "raw" number.
+ */
+
+int get_jiffies(unsigned *jiffies, const char *arg, int base, int *raw)
+{
+ double t;
+ unsigned long res;
+ char *p;
+
+ if (strchr(arg,'.') != NULL) {
+ t = strtod(arg,&p);
+ if (t < 0.0)
+ return -1;
+ }
+ else {
+ res = strtoul(arg,&p,base);
+ if (res > UINT_MAX)
+ return -1;
+ t = (double)res;
+ }
+ if (p == arg)
+ return -1;
+
+ if (__iproute2_hz_internal == 0)
+ __iproute2_hz_internal = __get_hz();
+
+ *raw = 1;
+
+ if (*p) {
+ *raw = 0;
+ if (strcasecmp(p, "s") == 0 || strcasecmp(p, "sec")==0 ||
+ strcasecmp(p, "secs")==0)
+ t *= __iproute2_hz_internal;
+ else if (strcasecmp(p, "ms") == 0 || strcasecmp(p, "msec")==0 ||
+ strcasecmp(p, "msecs") == 0)
+ t *= __iproute2_hz_internal/1000.0;
+ else if (strcasecmp(p, "us") == 0 || strcasecmp(p, "usec")==0 ||
+ strcasecmp(p, "usecs") == 0)
+ t *= __iproute2_hz_internal/1000000.0;
+ else if (strcasecmp(p, "ns") == 0 || strcasecmp(p, "nsec")==0 ||
+ strcasecmp(p, "nsecs") == 0)
+ t *= __iproute2_hz_internal/1000000000.0;
+ else if (strcasecmp(p, "j") == 0 || strcasecmp(p, "hz") == 0 ||
+ strcasecmp(p,"jiffies") == 0)
+ t *= 1.0; /* allow suffix, do nothing */
+ else
+ return -1;
+ }
+
+ /* emulate ceil() without having to bring-in -lm and always be >= 1 */
+
+ *jiffies = t;
+ if (*jiffies < t)
+ *jiffies += 1;
+
+ return 0;
+
+}
+
int get_u64(__u64 *val, const char *arg, int base)
{
unsigned long long res;
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index a9132da..7181054 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -169,9 +169,9 @@ replace " | " monitor " } "
.B advmss
.IR NUMBER " ] [ "
.B rtt
-.IR NUMBER " ] [ "
+.IR TIME " ] [ "
.B rttvar
-.IR NUMBER " ] [ "
+.IR TIME " ] [ "
.B window
.IR NUMBER " ] [ "
.B cwnd
@@ -179,7 +179,9 @@ replace " | " monitor " } "
.B ssthresh
.IR REALM " ] [ "
.B realms
-.IR REALM " ]"
+.IR REALM " ] [ "
+.B rto_min
+.IR TIME " ]"
.ti -8
.IR TYPE " := [ "
@@ -301,6 +303,9 @@ throw " | " unreachable " | " prohibit " | " blackhole " | " nat " ]"
.IR KEY " := { " DOTTED_QUAD " | " NUMBER " }"
.ti -8
+.IR TIME " := " NUMBER "[s|ms|us|ns|j]"
+
+.ti -8
.BR "ip maddr" " [ " add " | " del " ]"
.IB MULTIADDR " dev " STRING
@@ -1062,12 +1067,29 @@ measured in bytes. It limits maximal data bursts that our TCP
peers are allowed to send to us.
.TP
-.BI rtt " NUMBER"
-the initial RTT ('Round Trip Time') estimate.
+.BI rtt " TIME"
+the initial RTT ('Round Trip Time') estimate. If no suffix is
+specified the units are raw values passed directly to the
+routing code to maintain compatability with previous releases.
+Otherwise if a suffix of s, sec or secs is used to specify
+seconds; ms, msec or msecs to specify milliseconds; us, usec
+or usecs to specify microseconds; ns, nsec or nsecs to specify
+nanoseconds; j, hz or jiffies to specify jiffies, the value is
+converted to what the routing code expects.
+
+
+.TP
+.BI rttvar " TIME " "(2.3.15+ only)"
+the initial RTT variance estimate. Values are specified as with
+.BI rtt
+above.
.TP
-.BI rttvar " NUMBER " "(2.3.15+ only)"
-the initial RTT variance estimate.
+.BI rto_min " TIME " "(2.6.23+ only)"
+the minimum TCP Retransmission TimeOut to use when communicating with this
+destination. Values are specified as with
+.BI rtt
+above.
.TP
.BI ssthresh " NUMBER " "(2.3.15+ only)"
^ permalink raw reply related
* Re: [PATCH net-2.6.24] Fix documentation for dev_put()/dev_hold()
From: David Miller @ 2007-09-18 20:16 UTC (permalink / raw)
To: rdreier; +Cc: netdev, general
In-Reply-To: <adasl5bkfyx.fsf_-_@cisco.com>
From: Roland Dreier <rdreier@cisco.com>
Date: Tue, 18 Sep 2007 11:04:22 -0700
> It looks like the comments for dev_put() and dev_hold() got reversed somehow.
>
> Signed-off-by: Roland Dreier <rolandd@cisco.com>
Applied, thanks Roland.
^ permalink raw reply
* Re: [PATCH 1/3] [PPP] L2TP: Disallow non-UDP datagram sockets
From: James Chapman @ 2007-09-18 20:16 UTC (permalink / raw)
To: Herbert Xu
Cc: David S. Miller, Michal Ostrowski, Paul Mackerras,
Toralf Förster, netdev
In-Reply-To: <E1IXbqW-0002OB-00@gondolin.me.apana.org.au>
Herbert Xu wrote:
> [PPP] L2TP: Disallow non-UDP datagram sockets
>
> With the addition of UDP-Lite we need to refine the socket check so that
> only genuine UDP sockets are allowed through.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Acked-by: James Chapman <jchapman@katalix.com>
^ permalink raw reply
* Re: [PATCH 2/3] [PPP] L2TP: Fix skb handling in pppol2tp_recv_core
From: James Chapman @ 2007-09-18 20:17 UTC (permalink / raw)
To: Herbert Xu
Cc: David S. Miller, Michal Ostrowski, Paul Mackerras,
Toralf Förster, netdev
In-Reply-To: <E1IXbqo-0002OW-00@gondolin.me.apana.org.au>
Herbert Xu wrote:
> [PPP] L2TP: Fix skb handling in pppol2tp_recv_core
>
> The function pppol2tp_recv_core doesn't handle non-linear packets properly.
> It also fails to check the remote offset field.
>
> This patch fixes these problems. It also removes an unnecessary check on
> the UDP header which has already been performed by the UDP layer.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Acked-by: James Chapman <jchapman@katalix.com>
--
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
^ permalink raw reply
* Re: [0/3] [PPP]: Fix pppol2tp skb bugs
From: David Miller @ 2007-09-18 20:17 UTC (permalink / raw)
To: herbert; +Cc: jchapman, mostrows, paulus, toralf.foerster, netdev
In-Reply-To: <20070918120438.GA9121@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 18 Sep 2007 20:04:38 +0800
> On Fri, Aug 31, 2007 at 05:06:25PM +0800, Herbert Xu wrote:
> >
> > I've audited ppp_generic.c and pppoe.c. I'll do pppol2tp
> > tomorrow.
>
> Tomrrow took a while to come :)
It took me two weeks to apply the original patch set so you
get a reprieve too :-)
> Here are the fixes for pppol2tp.
I'll apply these, thanks Herbert.
^ permalink raw reply
* Re: [PATCH 3/3] [PPP] L2TP: Fix skb handling in pppol2tp_xmit
From: James Chapman @ 2007-09-18 20:19 UTC (permalink / raw)
To: Herbert Xu
Cc: David S. Miller, Michal Ostrowski, Paul Mackerras,
Toralf Förster, netdev
In-Reply-To: <E1IXbqs-0002Od-00@gondolin.me.apana.org.au>
Herbert Xu wrote:
> [PPP] L2TP: Fix skb handling in pppol2tp_xmit
>
> This patch makes pppol2tp_xmit call skb_cow_head so that we don't modify
> cloned skb data. It also gets rid of skb2 we only need to preserve the
> original skb for congestion notification, which is only applicable for
> ppp_async and ppp_sync.
>
> The other semantic change made here is the removal of socket accounting
> for data tranmitted out of pppol2tp_xmit. The original code leaked any
> existing socket skb accounting. We could fix this by dropping the
> original skb owner. However, this is undesirable as the packet has not
> physically left the host yet.
>
> In fact, all other tunnels in the kernel do not account skb's passing
> through to their own socket. In partciular, ESP over UDP does not do
> so and it is the closest tunnel type to PPPoL2TP. So this patch simply
> removes the socket accounting in pppol2tp_xmit. The accounting still
> applies to control packets of course.
>
> I've also added a reminder that the outgoing checksum here doesn't work.
> I suppose existing deployments don't actually enable checksums.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
This one causes my test system to lock up. I'll investigate. Please
don't apply this patch for now.
--
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
^ permalink raw reply
* Re: [PATCH][NETNS] Cleanup list walking in setup_net and cleanup_net
From: David Miller @ 2007-09-18 20:21 UTC (permalink / raw)
To: xemul; +Cc: ebiederm, netdev, devel
In-Reply-To: <46EF871D.8050900@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Tue, 18 Sep 2007 12:06:53 +0400
> I proposed introducing a list_for_each_entry_continue_reverse macro
> to be used in setup_net() when unrolling the failed ->init callback.
>
> Here is the macro and some more cleanup in the setup_net() itself
> to remove one variable from the stack :) The same thing is for the
> cleanup_net() - the existing list_for_each_entry_reverse() is used.
>
> Minor, but the code looks nicer.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Applied, thanks Pavel.
^ permalink raw reply
* Re: [PATCH] return useful listenq info in tcp_info and INET_DIAG_INFO
From: David Miller @ 2007-09-18 20:25 UTC (permalink / raw)
To: rick.jones2; +Cc: netdev
In-Reply-To: <200709181954.MAA25525@tardy.cup.hp.com>
From: Rick Jones <rick.jones2@hp.com>
Date: Tue, 18 Sep 2007 12:54:31 -0700 (PDT)
> Return some useful information such as the maximum listen backlog and the
> current listen backlog in the tcp_info structure and INET_DIAG_INFO.
>
> Signed-off-by: Rick Jones <rick.jones2@hp.com>
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
Thanks Rick, I'll add this to net-2.6.24 with a minor coding
style fix:
> + }
> + else {
We don't split up closing braces and else clauses like this,
it eats up a precious extra screen line :-)
} else {
is the custom.
^ permalink raw reply
* Please pull 'b43' branch of wireless-2.6
From: John W. Linville @ 2007-09-18 20:28 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: jeff-o2qLIJkoznsdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, sam-uyr5N9Q2VtJg9hUCZPvPmw
Jeff & Dave,
This series adds the b43 and b43legacy drivers, as well as the ssb
bus infrastructure upon which they depend.
These drivers support the BCM43xx wireless hardware from Broadcom.
The b43 driver supports modern hardware using the "version 4" firmware,
while the b43legacy driver uses the "version 3" firmware to support
some older devices which lack the resources to run the newer (and
more feature rich) firmware. Both drivers have performed well in
-mm and Fedora rawhide, and b43 has performed well in later Fedora
7 kernels as well.
Furthermore, Michael and Larry have both demonstrated a strong
commitment to the community and their users. They both have responded
to bugs both at the kernel.org bugzilla and in distro bugzillas.
I have every confidence that they will continue to support their
drivers at the same high standard that have already set. I'd like
to take this opportunity to thank both of them for their support both
in the past and in the future as well.
No doubt there are still problems here. But again, I think we would
be better served by mainlining this code rather than continuing to
maintain it out of stream.
Thanks!
John
---
Patches are here:
http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/b43/
---
The following changes since commit 0d4cbb5e7f60b2f1a4d8b7f6ea4cc264262c7a01:
Linus Torvalds (1):
Linux 2.6.23-rc6
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git b43
Larry Finger (1):
b43legacy: add mac80211-based driver for legacy BCM43xx devices
Michael Buesch (2):
ssb: add Sonics Silicon Backplane bus support
b43: add mac80211-based driver for modern BCM43xx devices
MAINTAINERS | 22 +
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/net/wireless/Kconfig | 2 +
drivers/net/wireless/Makefile | 2 +
drivers/net/wireless/b43/Kconfig | 119 +
drivers/net/wireless/b43/Makefile | 17 +
drivers/net/wireless/b43/b43.h | 845 ++++++
drivers/net/wireless/b43/debugfs.c | 654 +++++
drivers/net/wireless/b43/debugfs.h | 89 +
drivers/net/wireless/b43/dma.c | 1494 ++++++++++
drivers/net/wireless/b43/dma.h | 337 +++
drivers/net/wireless/b43/leds.c | 299 ++
drivers/net/wireless/b43/leds.h | 55 +
drivers/net/wireless/b43/lo.c | 1261 ++++++++
drivers/net/wireless/b43/lo.h | 112 +
drivers/net/wireless/b43/main.c | 4091 ++++++++++++++++++++++++++
drivers/net/wireless/b43/main.h | 142 +
drivers/net/wireless/b43/pcmcia.c | 157 +
drivers/net/wireless/b43/pcmcia.h | 20 +
drivers/net/wireless/b43/phy.c | 4351 ++++++++++++++++++++++++++++
drivers/net/wireless/b43/phy.h | 297 ++
drivers/net/wireless/b43/pio.c | 650 +++++
drivers/net/wireless/b43/pio.h | 153 +
drivers/net/wireless/b43/sysfs.c | 235 ++
drivers/net/wireless/b43/sysfs.h | 9 +
drivers/net/wireless/b43/tables.c | 375 +++
drivers/net/wireless/b43/tables.h | 28 +
drivers/net/wireless/b43/xmit.c | 648 +++++
drivers/net/wireless/b43/xmit.h | 250 ++
drivers/net/wireless/b43legacy/Kconfig | 89 +
drivers/net/wireless/b43legacy/Makefile | 14 +
drivers/net/wireless/b43legacy/b43legacy.h | 829 ++++++
drivers/net/wireless/b43legacy/debugfs.c | 505 ++++
drivers/net/wireless/b43legacy/debugfs.h | 89 +
drivers/net/wireless/b43legacy/dma.c | 1565 ++++++++++
drivers/net/wireless/b43legacy/dma.h | 367 +++
drivers/net/wireless/b43legacy/ilt.c | 336 +++
drivers/net/wireless/b43legacy/ilt.h | 34 +
drivers/net/wireless/b43legacy/leds.c | 302 ++
drivers/net/wireless/b43legacy/leds.h | 56 +
drivers/net/wireless/b43legacy/main.c | 3805 ++++++++++++++++++++++++
drivers/net/wireless/b43legacy/main.h | 147 +
drivers/net/wireless/b43legacy/phy.c | 2265 +++++++++++++++
drivers/net/wireless/b43legacy/phy.h | 219 ++
drivers/net/wireless/b43legacy/pio.c | 668 +++++
drivers/net/wireless/b43legacy/pio.h | 172 ++
drivers/net/wireless/b43legacy/radio.c | 2131 ++++++++++++++
drivers/net/wireless/b43legacy/radio.h | 98 +
drivers/net/wireless/b43legacy/sysfs.c | 238 ++
drivers/net/wireless/b43legacy/sysfs.h | 9 +
drivers/net/wireless/b43legacy/xmit.c | 642 ++++
drivers/net/wireless/b43legacy/xmit.h | 259 ++
drivers/ssb/Kconfig | 117 +
drivers/ssb/Makefile | 18 +
drivers/ssb/b43_pci_bridge.c | 46 +
drivers/ssb/driver_chipcommon.c | 446 +++
drivers/ssb/driver_extif.c | 129 +
drivers/ssb/driver_mipscore.c | 223 ++
drivers/ssb/driver_pcicore.c | 576 ++++
drivers/ssb/main.c | 1162 ++++++++
drivers/ssb/pci.c | 740 +++++
drivers/ssb/pcihost_wrapper.c | 104 +
drivers/ssb/pcmcia.c | 271 ++
drivers/ssb/scan.c | 413 +++
drivers/ssb/ssb_private.h | 136 +
include/linux/mod_devicetable.h | 15 +
include/linux/ssb/ssb.h | 424 +++
include/linux/ssb/ssb_driver_chipcommon.h | 396 +++
include/linux/ssb/ssb_driver_extif.h | 204 ++
include/linux/ssb/ssb_driver_mips.h | 46 +
include/linux/ssb/ssb_driver_pci.h | 106 +
include/linux/ssb/ssb_regs.h | 292 ++
scripts/mod/file2alias.c | 19 +
74 files changed, 37439 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/wireless/b43/Kconfig
create mode 100644 drivers/net/wireless/b43/Makefile
create mode 100644 drivers/net/wireless/b43/b43.h
create mode 100644 drivers/net/wireless/b43/debugfs.c
create mode 100644 drivers/net/wireless/b43/debugfs.h
create mode 100644 drivers/net/wireless/b43/dma.c
create mode 100644 drivers/net/wireless/b43/dma.h
create mode 100644 drivers/net/wireless/b43/leds.c
create mode 100644 drivers/net/wireless/b43/leds.h
create mode 100644 drivers/net/wireless/b43/lo.c
create mode 100644 drivers/net/wireless/b43/lo.h
create mode 100644 drivers/net/wireless/b43/main.c
create mode 100644 drivers/net/wireless/b43/main.h
create mode 100644 drivers/net/wireless/b43/pcmcia.c
create mode 100644 drivers/net/wireless/b43/pcmcia.h
create mode 100644 drivers/net/wireless/b43/phy.c
create mode 100644 drivers/net/wireless/b43/phy.h
create mode 100644 drivers/net/wireless/b43/pio.c
create mode 100644 drivers/net/wireless/b43/pio.h
create mode 100644 drivers/net/wireless/b43/sysfs.c
create mode 100644 drivers/net/wireless/b43/sysfs.h
create mode 100644 drivers/net/wireless/b43/tables.c
create mode 100644 drivers/net/wireless/b43/tables.h
create mode 100644 drivers/net/wireless/b43/xmit.c
create mode 100644 drivers/net/wireless/b43/xmit.h
create mode 100644 drivers/net/wireless/b43legacy/Kconfig
create mode 100644 drivers/net/wireless/b43legacy/Makefile
create mode 100644 drivers/net/wireless/b43legacy/b43legacy.h
create mode 100644 drivers/net/wireless/b43legacy/debugfs.c
create mode 100644 drivers/net/wireless/b43legacy/debugfs.h
create mode 100644 drivers/net/wireless/b43legacy/dma.c
create mode 100644 drivers/net/wireless/b43legacy/dma.h
create mode 100644 drivers/net/wireless/b43legacy/ilt.c
create mode 100644 drivers/net/wireless/b43legacy/ilt.h
create mode 100644 drivers/net/wireless/b43legacy/leds.c
create mode 100644 drivers/net/wireless/b43legacy/leds.h
create mode 100644 drivers/net/wireless/b43legacy/main.c
create mode 100644 drivers/net/wireless/b43legacy/main.h
create mode 100644 drivers/net/wireless/b43legacy/phy.c
create mode 100644 drivers/net/wireless/b43legacy/phy.h
create mode 100644 drivers/net/wireless/b43legacy/pio.c
create mode 100644 drivers/net/wireless/b43legacy/pio.h
create mode 100644 drivers/net/wireless/b43legacy/radio.c
create mode 100644 drivers/net/wireless/b43legacy/radio.h
create mode 100644 drivers/net/wireless/b43legacy/sysfs.c
create mode 100644 drivers/net/wireless/b43legacy/sysfs.h
create mode 100644 drivers/net/wireless/b43legacy/xmit.c
create mode 100644 drivers/net/wireless/b43legacy/xmit.h
create mode 100644 drivers/ssb/Kconfig
create mode 100644 drivers/ssb/Makefile
create mode 100644 drivers/ssb/b43_pci_bridge.c
create mode 100644 drivers/ssb/driver_chipcommon.c
create mode 100644 drivers/ssb/driver_extif.c
create mode 100644 drivers/ssb/driver_mipscore.c
create mode 100644 drivers/ssb/driver_pcicore.c
create mode 100644 drivers/ssb/main.c
create mode 100644 drivers/ssb/pci.c
create mode 100644 drivers/ssb/pcihost_wrapper.c
create mode 100644 drivers/ssb/pcmcia.c
create mode 100644 drivers/ssb/scan.c
create mode 100644 drivers/ssb/ssb_private.h
create mode 100644 include/linux/ssb/ssb.h
create mode 100644 include/linux/ssb/ssb_driver_chipcommon.h
create mode 100644 include/linux/ssb/ssb_driver_extif.h
create mode 100644 include/linux/ssb/ssb_driver_mips.h
create mode 100644 include/linux/ssb/ssb_driver_pci.h
create mode 100644 include/linux/ssb/ssb_regs.h
--
John W. Linville
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org
^ permalink raw reply
* Re: [PATCH 3/3] [PPP] L2TP: Fix skb handling in pppol2tp_xmit
From: David Miller @ 2007-09-18 20:32 UTC (permalink / raw)
To: jchapman; +Cc: herbert, mostrows, paulus, toralf.foerster, netdev
In-Reply-To: <46F032D5.7020806@katalix.com>
From: James Chapman <jchapman@katalix.com>
Date: Tue, 18 Sep 2007 21:19:33 +0100
> This one causes my test system to lock up. I'll investigate. Please
> don't apply this patch for now.
I'll make sure not to push this until we figure out what's
wrong, thanks for checking James.
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: Sam Ravnborg @ 2007-09-18 20:35 UTC (permalink / raw)
To: David Miller; +Cc: hpa, mchan, vda.linux, linux-kernel, netdev
In-Reply-To: <20070918.130810.78161599.davem@davemloft.net>
On Tue, Sep 18, 2007 at 01:08:10PM -0700, David Miller wrote:
> From: "H. Peter Anvin" <hpa@zytor.com>
> Date: Tue, 18 Sep 2007 12:27:04 -0700
>
> > Well, what I was referring to here, of course, was the initramfs
> > integrated in the kernel image, so it all comes out of the kernel build
> > tree and produces a single bootable image. The fact that part of it
> > contains userspace code is in that way invisible.
> >
> > That was kind of the point here, and the only reason for pushing klibc
> > into the kernel build tree at all. Under the "distros use external
> > initrd anyway" school of thought, whatever libc used for that can be
> > external anyway.
>
> Sounds good to me :)
Except there seems to be great resistance to include userland code in the
kernel as demonstrated at last KS. Or this is maybe just a single vocal
person and the topic were brought up late?
Anyway - if we again consider klibc I will do my best to make the
build stuff as smooth as possible.
Sam
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: H. Peter Anvin @ 2007-09-18 20:40 UTC (permalink / raw)
To: Sam Ravnborg; +Cc: David Miller, mchan, vda.linux, linux-kernel, netdev
In-Reply-To: <20070918203530.GC27836@uranus.ravnborg.org>
Sam Ravnborg wrote:
>
> Except there seems to be great resistance to include userland code in the
> kernel as demonstrated at last KS. Or this is maybe just a single vocal
> person and the topic were brought up late?
>
> Anyway - if we again consider klibc I will do my best to make the
> build stuff as smooth as possible.
>
At least as of the last merged tree it was very smooth indeed, thanks to
your help.
-hpa
^ permalink raw reply
* [FIX NETLINK] properly check arguments to netlink_bind()
From: Holger Eitzenberger @ 2007-09-18 21:01 UTC (permalink / raw)
To: netdev
Hi,
while going through going netlink code I found out that netlink_bind()
does not properly check bind parameters. I checked both 2.6.23-rc1 as
well as 2.6.16.53, both are affected.
With a small test prog I wasn't able to crash my maschine though, but
data was accessed out of bounds.
Please apply the attached patch.
Thanks.
/holger
^ permalink raw reply
* Re: [FIX NETLINK] properly check arguments to netlink_bind()
From: Holger Eitzenberger @ 2007-09-18 21:05 UTC (permalink / raw)
To: netdev
In-Reply-To: <87fy1belij.fsf@kruemel.my-eitzenberger.de>
[-- Attachment #1: Type: text/plain, Size: 396 bytes --]
Holger Eitzenberger <holger@my-eitzenberger.de> writes:
> while going through going netlink code I found out that netlink_bind()
> does not properly check bind parameters. I checked both 2.6.23-rc1 as
> well as 2.6.16.53, both are affected.
>
> With a small test prog I wasn't able to crash my maschine though, but
> data was accessed out of bounds.
See my attached patch, thanks.
/holger
[-- Attachment #2: netlink fix --]
[-- Type: text/plain, Size: 1108 bytes --]
[NETLINK] Check for correct bind parameters
Before this change it was possible to pass less than sockaddr_nl
bytes, which might lead to arbitrary data being accessed in
netlink_bind().
Signed-off-by: Holger Eitzenberger <holger@my-eitzenberger.de>
---
commit 53ba94ab22cc3338d915d684ba1012fa0419ff14
tree 3a1b1dc6cb5dacac99722b9f96fe3ba4b2d29bde
parent f695baf2df9e0413d3521661070103711545207a
author Holger Eitzenberger <holger@my-eitzenberger.de> Mon, 17 Sep 2007 22:15:37 +0200
committer Holger Eitzenberger <holger@elmo.(none)> Mon, 17 Sep 2007 22:15:37 +0200
net/netlink/af_netlink.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 5681ce3..425543d 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -598,6 +598,9 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, int addr_len
struct sockaddr_nl *nladdr = (struct sockaddr_nl *)addr;
int err;
+ if (addr < sizeof(struct sockaddr_nl))
+ return -EINVAL;
+
if (nladdr->nl_family != AF_NETLINK)
return -EINVAL;
^ permalink raw reply related
* Re: [FIX NETLINK] properly check arguments to netlink_bind()
From: Holger Eitzenberger @ 2007-09-18 21:14 UTC (permalink / raw)
To: netdev
In-Reply-To: <876427elan.fsf@kruemel.my-eitzenberger.de>
[-- Attachment #1: Type: text/plain, Size: 95 bytes --]
The previous patchlet wasn't correct, please apply this one.
Sorry for the noise.
/holger
[-- Attachment #2: netlink bind fix --]
[-- Type: text/plain, Size: 1123 bytes --]
[NETLINK] Check for correct bind parameters
Before this change it was possible to pass less than sockaddr_nl
bytes, which might lead to arbitrary data being accessed in
netlink_bind().
Take two.
Signed-off-by: Holger Eitzenberger <holger@my-eitzenberger.de>
---
commit 3155c34167184c31afeac2a061c0e0b9cd401d56
tree b5efe4234a5835e823b6b024f8d96e56f4abfd18
parent f695baf2df9e0413d3521661070103711545207a
author Holger Eitzenberger <holger@my-eitzenberger.de> Tue, 18 Sep 2007 23:10:11 +0200
committer Holger Eitzenberger <holger@elmo.(none)> Tue, 18 Sep 2007 23:10:11 +0200
net/netlink/af_netlink.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 5681ce3..5aaa9a7 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -598,6 +598,9 @@ static int netlink_bind(struct socket *sock, struct sockaddr *addr, int addr_len
struct sockaddr_nl *nladdr = (struct sockaddr_nl *)addr;
int err;
+ if (addr_len < sizeof(struct sockaddr_nl))
+ return -EINVAL;
+
if (nladdr->nl_family != AF_NETLINK)
return -EINVAL;
^ permalink raw reply related
* Re: [PATCH 2/7] CAN: Add PF_CAN core module
From: Urs Thuermann @ 2007-09-18 21:20 UTC (permalink / raw)
To: Patrick McHardy
Cc: netdev, David Miller, Thomas Gleixner, Oliver Hartkopp,
Oliver Hartkopp, Urs Thuermann
In-Reply-To: <46EFD33D.8030905@trash.net>
Patrick McHardy <kaber@trash.net> writes:
> > +++ net-2.6.24/include/linux/can.h 2007-09-17 10:27:09.000000000 +0200
> Is this file used only from within the kernel? If so you could use
> the nicer-to-look-at u8/u16/u32 types instead of the double underscored
> ones.
No, this file contains the interface to user space.
> > +++ net-2.6.24/include/linux/can/core.h 2007-09-17 11:08:39.000000000 +0200
> > @@ -0,0 +1,78 @@
> > +
> > +extern int can_proto_register(struct can_proto *cp);
> > +extern int can_proto_unregister(struct can_proto *cp);
>
>
> The callers of the unregister function don't check the return code,
> and they can't handle errors anyways since they use it in the
> module unload path, so making it void seems more appropriate
> (and maybe a WARN_ON for the "not-found" case).
These functions have been declared returning void originally, but in
the review process with Thomas we changed them since they can fail.
Even if the current users, i.e. raw.c and bcm.c don't use the return
value, others might do. I therefore prefer to keep the int return
value. Rather we should check whether we can do something useful with
the return value in raw.c and bcm.c. At least we can put a WARN_ON
there.
> Same here, none of the callers check the return value and since
> they're all declared as void they can't propagate any errors back.
Same as above.
> > +#include "af_can.h"
>
>
> It seems most of the things declared in that file are only used within
> af_can.c. Might be easier to read the code if you'd just move it over.
af_can.h declares the interface between af_can.c and proc.c. There
should be nothing in af_can.h which is only used in af_can.c
> > +int stats_timer = 1; /* default: on */
>
>
> This seems to be only used in af_can.c, so it could be static.
> __read_mostly also seems to be approriate.
Right. Will be changed.
> There are a few more that look like they could be __read_mostly
> below, but I'll skip these since you probably know better than me.
Yes, I will check these also.
> > +HLIST_HEAD(rx_dev_list);
>
>
> Same here (static).
Can't be static since it's used in proc.c. But __read_mostly might
make sense.
What exactly is the effect of __read_mostly? Is that in a separate
ELF section? Where is that finally located?
> > + if (ret == -ENOSYS)
> > + printk(KERN_INFO "can: request_module(%s) not"
> > + " implemented.\n", module_name);
> > + else if (ret)
> > + printk(KERN_ERR "can: request_module(%s) failed\n",
> > + module_name);
>
>
> Both of these printks seem to be user-triggerable, so they should
> be rate-limited (or maybe get removed completely/changed to DBG).
Hm, I don't think DBG() would be right here, since the messages show
problems in the installation to the admin. OTOH, I see that a user
can flood the log by opening sockets with invalid proto numbers. Rate
limiting might solve this, or we should print the message only once
per proto number. I will think about this.
> > + /* check for success and correct type */
> > + cp = proto_tab[protocol];
>
>
> What prevents the module from getting unloaded again (and using
> a stale pointer)?
When the module is unloaded it calls can_proto_unregister() which
clears the pointer. Do you see a race condition here?
> > + if (!cp || cp->type != sock->type)
> > + return -EPROTONOSUPPORT;
> > +
> > + if (net != &init_net)
> > + return -EAFNOSUPPORT;
>
>
> Shouldn't this be done before attempting the module load?
Yes, you're right.
> > +int can_send(struct sk_buff *skb, int loop)
> > +{
> > + int err;
> > +
> > + if (skb->dev->type != ARPHRD_CAN) {
> > + kfree_skb(skb);
> > + return -EPERM;
>
>
> EPERM doesn't seem like the best fit, but I don't have a better
> suggestion myself at the moment.
We have chosen EPERM because a user shouldn't be allowed to open a raw
CAN socket and then send arbitrary frames to other non-CAN interfaces.
This is also because the raw and bcm protocols can be configured to
grant access to non-root users.
> > + if (!(skb->dev->flags & IFF_LOOPBACK)) {
> > + /*
> > + * If the interface is not capable to do loopback
> > + * itself, we do it here.
> > + */
> > + struct sk_buff *newskb = skb_clone(skb, GFP_ATOMIC);
> > +
> > + if (!newskb) {
> > + kfree_skb(skb);
> > + return -ENOMEM;
> > + }
> > +
> > + newskb->sk = skb->sk;
> > + newskb->ip_summed = CHECKSUM_UNNECESSARY;
> > + newskb->pkt_type = PACKET_BROADCAST;
> > + netif_rx(newskb);
>
>
> So the intention here is to send the packet to the non-loopback device
> and manually loop it, which means sending it twice?
CAN is a broadcast message network, so every frame should be (usually)
sent to all receivers, on remote hosts and to all local sockets. If
the driver for the interface is not able to loop back the frame for
local delivery, the can_send() function will do this as a fallback.
For real CAN devices it is preferred that the driver does loopback.
For vcan it makes no difference where loopback is done. The module
paramenter for vcan is therefore only useful to test and debug the CAN
core module. It is nothing a normal user will ever use.
> > +static struct dev_rcv_lists *find_dev_rcv_lists(struct net_device *dev)
> > +{
> > + struct dev_rcv_lists *d;
> > + struct hlist_node *n;
> > +
> > + /*
> > + * find receive list for this device
> > + *
> > + * The hlist_for_each_entry*() macros curse through the list
> > + * using the pointer variable n and set d to the containing
> > + * struct in each list iteration. Therefore, after list
> > + * iteration, d is unmodified when the list is empty, and it
> > + * points to last list element, when the list is non-empty
> > + * but no match in the loop body is found. I.e. d is *not*
> > + * NULL when no match is found. We can, however, use the
> > + * cursor variable n to decide if a match was found.
> > + */
> > +
> > + hlist_for_each_entry(d, n, &rx_dev_list, list) {
>
>
> On the receive path you use RCU, so this should be
> hlist_for_each_entry_rcu(), no? The bottem half disabling during
> addition/removal also seems unnecessary, but I might be missing
> something.
find_dev_rcv_lists() is called in one place from can_rcv() with RCU
lock held, as you write. The other two calls to find_dev_rcv_lists()
are from can_rx_register/unregister() functions which change the
receive lists. Therefore, we can't only use RCU but need protection
against simultanous writes. We do this with the spin_lock_bh(). The
_bh variant, because can_rcv() runs in interrupt and we need to block
that. I thought this is pretty standard.
I'll check this again tomorrow, but I have put much time in these
locking issues already, changed it quite a few times and hoped to have
got it right finally.
...
I have just looked at the code again and I think we're right here.
Please reply if you think otherwise and we must fix it.
> > + case NETDEV_REGISTER:
> > +
> > + /*
> > + * create new dev_rcv_lists for this device
> > + *
> > + * N.B. zeroing the struct is the correct initialization
> > + * for the embedded hlist_head structs.
> > + * Another list type, e.g. list_head, would require
> > + * explicit initialization.
> > + */
> > +
> > + DBG("creating new dev_rcv_lists for %s\n", dev->name);
> > +
> > + d = kzalloc(sizeof(*d),
> > + in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);
>
>
> netdevice registration should never happen from interrupt handlers.
Hm, I seem to remember we had such an occurance with hot-pluggable
devices, i.e. USB. But I may be wrong. In what context is
NETDEV_REGISTER called for e.g. USB devices? Can we safely write
just GFP_KERNEL here?
> > +static struct packet_type can_packet = {
> > + .type = __constant_htons(ETH_P_CAN),
> > + .dev = NULL,
> > + .func = can_rcv,
> > +};
>
> __read_mostly (for those below as well)?
OK.
> > + stattimer.expires = jiffies + HZ;
>
> round_jiffies?
Yes. We don't depend on exact times relative to module load time, but
we would like to have the timer expirations 1 second apart. But we
would still get this with round_jiffies(), right?
> > +static unsigned long calc_rate(unsigned long oldjif, unsigned long newjif,
> > + unsigned long count)
> > +{
> > + unsigned long ret = 0;
> > +
> > + if (oldjif == newjif)
> > + return 0;
> > +
> > + /* see can_rcv() - this should NEVER happen! */
>
>
> If I'm not mistaken this comment is outdated and should refer to
> can_stat_update().
Yep. Thanks.
> > + stattimer.expires = jiffies + HZ;
>
>
> round_jiffies?
Yes, like above.
> > + * proc read functions
> > + *
> > + * From known use-cases we expect about 10 entries in a receive list to be
> > + * printed in the proc_fs. So PAGE_SIZE is definitely enough space here.
>
>
> Would be nicer to use seq_file (for all the proc stuff).
That has already been on my TODO list. There was some problem which I
don't remember ATM, which is why I dropped it temporarily. Now on my
TODO list again.
Thank you for your feedback.
urs
^ 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