* Re: DM9000_IRQ_FLAGS
From: Ben Dooks @ 2007-12-20 9:45 UTC (permalink / raw)
To: Daniel Mack; +Cc: netdev
In-Reply-To: <20071211191823.GD15295@buzzloop.caiaq.de>
On Tue, Dec 11, 2007 at 08:18:23PM +0100, Daniel Mack wrote:
> Hi,
>
> on Toradex' Colibri, a PXA270 based board with a DM9000 ethernet
> controller, this driver won't work due to unsuitable DM9000_IRQ_FLAGS.
> If I understood the code behind request_irq() correctly, it's not
> recommended to register an IRQ without any of the IRQT_* flags set.
>
> Is there any concerns about applying the patch below?
Yes, that will possibly break all systems using level-triggered
interrupts.
Probably the best solution is to pass the data via the platform
information being fed to the device.
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* A short question about net git tree and patches
From: David Shwatrz @ 2007-12-20 9:20 UTC (permalink / raw)
To: netdev
Hello,
I have a short question regarding the net git tree and patches:
I want to write and send patches against the most recent and
bleeding edge kernel networking code.
I see in:
http://kernel.org/pub/scm/linux/kernel/git/davem/?C=M;O=A
that there are 3 git trees which can be candidates for git-clone and
making patches against;
these are:
netdev-2.6.git, net-2.6.25.git and net-2.6.git.
It seems to me that net-2.6.git is the most suitable one to work against;
am I right ?
what is the difference, in short, between the three repositories?
Regards,
DS
^ permalink raw reply
* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
From: Jarek Poplawski @ 2007-12-20 9:12 UTC (permalink / raw)
To: Satoru SATOH; +Cc: netdev
In-Reply-To: <d72b7ace0712191931w37f02645u8d5eb5eef9c1fba4@mail.gmail.com>
On 20-12-2007 04:31, Satoru SATOH wrote:
> "ip route show" does not print correct value when larger rto_min is
> set (e.g. 3sec).
>
> This problem is because of overflow in print_route() and
> the patch below is a workaround fix for that.
>
...
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -510,16 +510,16 @@ int print_route(const struct sockaddr_nl *who,
> struct nlmsghdr *n, void *arg)
> fprintf(fp, " %u",
> *(unsigned*)RTA_DATA(mxrta[i]));
> else {
> unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> + unsigned hz1 = hz / 1000;
...
> + if (val >= hz1)
> + fprintf(fp, " %ums", val/hz1);
...
Probably I miss something or my iproute sources are too old, but:
does this work with hz < 1000?
Regards,
Jarek P.
^ permalink raw reply
* Re: [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2007-12-20 8:35 UTC (permalink / raw)
To: satoru.satoh; +Cc: netdev, yoshfuji
In-Reply-To: <d72b7ace0712191931w37f02645u8d5eb5eef9c1fba4@mail.gmail.com>
In article <d72b7ace0712191931w37f02645u8d5eb5eef9c1fba4@mail.gmail.com> (at Thu, 20 Dec 2007 12:31:27 +0900), "Satoru SATOH" <satoru.satoh@gmail.com> says:
> diff --git a/ip/iproute.c b/ip/iproute.c
> index f4200ae..fa722c6 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -510,16 +510,16 @@ int print_route(const struct sockaddr_nl *who,
> struct nlmsghdr *n, void *arg)
> fprintf(fp, " %u",
> *(unsigned*)RTA_DATA(mxrta[i]));
> else {
> unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
> + unsigned hz1 = hz / 1000;
>
> - val *= 1000;
> if (i == RTAX_RTT)
I think this is incorrect; hz might not be 1000; e.g. 250 etc.
--yoshfuji
^ permalink raw reply
* Re: [PATCH net-2.6.25 1/3] Uninline the __inet_hash function
From: David Miller @ 2007-12-20 8:30 UTC (permalink / raw)
To: dada1; +Cc: xemul, netdev, devel
In-Reply-To: <476951A8.4040200@cosmosbay.com>
From: Eric Dumazet <dada1@cosmosbay.com>
Date: Wed, 19 Dec 2007 18:15:20 +0100
> Pavel Emelyanov a écrit :
> > That's not truth, if I get you right. The __inet_hash() is called
> > with 0, from all the places except for the inet_hash() one.
>
> OK, but on cases with 0, sk->sk_state is != TCP_LISTEN, unless I am mistaken.
This is true.
^ permalink raw reply
* Re: [PATCH][IPV4] ip_gre: set mac_header correctly in receive path
From: David Miller @ 2007-12-20 8:11 UTC (permalink / raw)
To: timo.teras; +Cc: kuznet, netdev
In-Reply-To: <47695EA1.10103@iki.fi>
From: Timo_Teräs <timo.teras@iki.fi>
Date: Wed, 19 Dec 2007 20:10:41 +0200
> From: Timo Teras <timo.teras@iki.fi>
>
> mac_header update in ipgre_recv() was incorrectly changed to
> skb_reset_mac_header() when it was introduced.
>
> Signed-off-by: Timo Teras <timo.teras@iki.fi>
Patch applied, thanks.
> ---
> This replaces my earlier patch titled "ip_gre: use skb->{mac,
> network}_header consistently". Apparently I hadn't done my homework how
> to use *_header correctly. And I should have done a bit more testing to
> figure out the previous patch does not work.
>
> But the main problem was the receive path in the first place, and this
> patch fixes it.
>
> The bug was introduced in commit 459a98ed881802dee55897441bc7f77af614368e.
> There might be other similar incorrect replaces.
That commit has two other identical bad conversions, I'll
fix them up.
^ permalink raw reply
* Re: [PATCH] XFRM: Audit function arguments misordered
From: David Miller @ 2007-12-20 8:01 UTC (permalink / raw)
To: paul.moore; +Cc: netdev, linux-audit
In-Reply-To: <20071219192937.15561.52837.stgit@flek.americas.hpqcorp.net>
From: Paul Moore <paul.moore@hp.com>
Date: Wed, 19 Dec 2007 14:29:38 -0500
> In several places the arguments to the xfrm_audit_start() function are in the
> wrong order resulting in incorrect user information being reported. This
> patch corrects this by pacing the arguments in the correct order.
>
> Signed-off-by: Paul Moore <paul.moore@hp.com>
Applied, thanks for fixing this bug.
^ permalink raw reply
* Re: [PATCH] One more XFRM audit fix
From: David Miller @ 2007-12-20 8:00 UTC (permalink / raw)
To: paul.moore; +Cc: netdev, linux-audit
In-Reply-To: <20071219192309.15561.9843.stgit@flek.americas.hpqcorp.net>
From: Paul Moore <paul.moore@hp.com>
Date: Wed, 19 Dec 2007 14:29:31 -0500
> The following patch is backed against David's net-2.6 tree and is pretty
> trivial. I know we're late in the 2.6.24 cycle but I think this is worth
> merging, if you guys don't feel that way let me know and I'll resubmit it
> for 2.6.25.
Where is that patch? Or do you mean the fix you emailed
seperately today (which I will apply, thanks)?
> As a side note, I'm unable to actually test the patch because I can't get
> the kernel to compile (M=net/xfrm works just fine). The problem I keep
> seeing is below:
>
> make[3]: *** No rule to make target \
> `/blah/kernels/net-2.6_xfrm-auid-secid-fix/include/linux/ticable.h', \
> needed by \
> `/blah/kernels/net-2.6_xfrm-auid-secid-fix/usr/include/linux/ticable.h'. \
> Stop.
Remove ticable.h from include/linux/Kbuild
This is already cured in Linus's tree.
^ permalink raw reply
* Re: [PATCH 00/29] Swap over NFS -v15
From: Peter Zijlstra @ 2007-12-20 8:00 UTC (permalink / raw)
To: Bill Davidsen
Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
trond.myklebust
In-Reply-To: <476999B7.1000203@tmr.com>
[-- Attachment #1: Type: text/plain, Size: 989 bytes --]
On Wed, 2007-12-19 at 17:22 -0500, Bill Davidsen wrote:
> Peter Zijlstra wrote:
> > Hi,
> >
> > Another posting of the full swap over NFS series.
> >
> > Andrew/Linus, could we start thinking of sticking this in -mm?
> >
>
> Two questions:
> 1 - what is the memory use impact on the system which don't do swap over
> NFS, such as embedded systems, and
It should have little to no impact if not used.
> 2 - what is the advantage of this code over the two existing network
> swap approaches,
> swapping to NFS mounted file and
This is not actually possible with a recent kernel, current swapfile
support requires a blockdevice.
> swap to NBD device?
> I've used the NFS file when a program was running out of memory and that
> seemed to work, people in UNYUUG have reported that the nbd swap works,
> so what's better here?
swap over NBD works sometimes, its rather easy to deadlock, and its
impossible to recover from a broken connection.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH 2.6.25 0/9]: SCTP: Update ADD-IP implementation to conform to spec
From: David Miller @ 2007-12-20 7:58 UTC (permalink / raw)
To: vladislav.yasevich; +Cc: netdev, lksctp-developers
In-Reply-To: <476984DB.4020403@hp.com>
From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Wed, 19 Dec 2007 15:53:47 -0500
> Not sure if you got the PATCH 7/9 resend, but it looks like netdev ate that
> too.
>
> I made this patch set available here:
> master.kernel.org:/pub/scm/linux/kernel/git/vxy/lksctp-dev.git addip
I got the patch, there is probably some keyword in there
that is making it get consumed by the majordomo regexp
filters we have in place.
^ permalink raw reply
* Re: TSO trimming question
From: David Miller @ 2007-12-20 7:55 UTC (permalink / raw)
To: herbert; +Cc: ilpo.jarvinen, netdev
In-Reply-To: <E1J5C38-0004kh-00@gondolin.me.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 20 Dec 2007 11:26:18 +0800
> Ilpo J??rvinen <ilpo.jarvinen@helsinki.fi> wrote:
> >
> >> is the limitting factor? For latter IMHO this would be necessary:
> >>
> >> if (skb->len > limit)
> >> limit -= limit % mss_now;
>
> Good catch! But how about putting this logic into tcp_window_allows
> since then you can avoid the divide for the case when we're not
> bound by the receiver window.
Because the purpose of this calculation is different, as I
described in my other reply, I don't think this change is
correct, regardless of where it is placed :-)
^ permalink raw reply
* Re: TSO trimming question
From: David Miller @ 2007-12-20 7:54 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev
In-Reply-To: <Pine.LNX.4.64.0712192336130.27945@kivilampi-30.cs.helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Wed, 19 Dec 2007 23:46:33 +0200 (EET)
> I'm not fully sure what's purpose of this code in tcp_write_xmit:
>
> if (skb->len < limit) {
> unsigned int trim = skb->len % mss_now;
>
> if (trim)
> limit = skb->len - trim;
> }
>
> Is it used to make sure we send only multiples of mss_now here and leave
> the left-over into another skb? Or does it try to make sure that
> tso_fragment result honors multiple of mss_now boundaries when snd_wnd
> is the limitting factor? For latter IMHO this would be necessary:
>
> if (skb->len > limit)
> limit -= limit % mss_now;
The purpose of the test is to make sure we process tail sub-mss chunks
correctly wrt. Nagle, which most closely matches the first purpose
you've listed.
So I think the calculation really does belong where it is.
Because of the way that the sendmsg() super-skb formation logic
works, we always will tack on more data and grow the tail
SKB before creating a new one. So any sub-mss chunk at the
end of a TSO frame really is at the end of the write queue
and really should get nagle processing.
Actually, there is an exception, which is when we run out of
skb_frag_list slots. In that case we'll potentially have breaks at
odd boundaries in the middle of the queue. But this can only happen
in exceptional cases (user does tons of 1-byte sendfile()'s over
random non-consequetive locations of a file) or outright bugs
(MAX_SKB_FRAGS is defined incorrectly, for example) and thus this
situation is not worth coding for.
sendmsg()'s goal, when TSO is enabled, is to append to the write
queue tail SKB, and it does so until the MAX_SKB_FRAGS limit
is reached.
^ permalink raw reply
* Re: [IPSEC]: Avoid undefined shift operation when testing algorithm ID
From: Jarek Poplawski @ 2007-12-20 7:58 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20071220042937.GA18679@gondor.apana.org.au>
On 20-12-2007 05:29, Herbert Xu wrote:
> Hi Dave:
>
> I had wanted to fix this for ages but kept putting it off and then
> forgetting about it :) So before I forget again,
>
> [IPSEC]: Avoid undefined shift operation when testing algorithm ID
>
> The aalgos/ealgos fields are only 32 bits wide. However, af_key tries
> to test them with the expression 1 << id where id can be as large as
> 253. This produces different behaviour on different architectures.
>
> The following patch explicitly checks whether ID is greater than 31
> and fails the check if that's the case.
>
> We cannot easily extend the mask to be longer than 32 bits due to
> exposure to user-space. Besides, this whole interface is obsolete
> anyway in favour of the xfrm_user interface which doesn't use this
> bit mask in templates (well not within the kernel anyway).
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index 878039b..26d5e63 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -2784,12 +2784,22 @@ static struct sadb_msg *pfkey_get_base_msg(struct sk_buff *skb, int *errp)
>
> static inline int aalg_tmpl_set(struct xfrm_tmpl *t, struct xfrm_algo_desc *d)
> {
> - return t->aalgos & (1 << d->desc.sadb_alg_id);
> + unsigned int id = d->desc.sadb_alg_id;
> +
> + if (id >= sizeof(t->aalgos) * 8)
> + return 0;
> +
> + return (t->aalgos >> id) & 1;
> }
Hi,
you probably have forgotten to mention in the changelog the returned
value is changed to 0/1 btw?
But, since you've mentioned different architectures, maybe it's the
good moment to find out why you and/or Linux doesn't seem to use
something like CHAR_BIT instead of this 8?
Thanks,
Jarek P.
^ permalink raw reply
* Re: [IPSEC]: Avoid undefined shift operation when testing algorithm ID
From: David Miller @ 2007-12-20 7:44 UTC (permalink / raw)
To: herbert; +Cc: netdev
In-Reply-To: <20071220042937.GA18679@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 20 Dec 2007 12:29:37 +0800
> [IPSEC]: Avoid undefined shift operation when testing algorithm ID
>
> The aalgos/ealgos fields are only 32 bits wide. However, af_key tries
> to test them with the expression 1 << id where id can be as large as
> 253. This produces different behaviour on different architectures.
>
> The following patch explicitly checks whether ID is greater than 31
> and fails the check if that's the case.
>
> We cannot easily extend the mask to be longer than 32 bits due to
> exposure to user-space. Besides, this whole interface is obsolete
> anyway in favour of the xfrm_user interface which doesn't use this
> bit mask in templates (well not within the kernel anyway).
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks for fixing this bug, patch applied!
^ permalink raw reply
* Re: [PATCH] remove not used code: target hw address (tha) definition and assignment in arp_process().
From: David Miller @ 2007-12-20 7:39 UTC (permalink / raw)
To: markryde; +Cc: netdev
In-Reply-To: <dac45060712192329v48bcd3td4f16f83ea13ba9e@mail.gmail.com>
From: "Mark Ryden" <markryde@gmail.com>
Date: Thu, 20 Dec 2007 09:29:47 +0200
> In arp_process() (net/ipv4/arp.c), there is unused code: definition
> and assignment of tha (target hw address ).
Patch applied, thanks.
> Signed-off-by:
> markryde@gmail.com
Please format your signed off line correctly, it should
have read like the following, all on one single line:
Signed-off-by: Mark Ryden <markryde@gmail.com>
Also, your patch was not rooted properly:
--- a/a/net/ipv4/arp.c
+++ b/b/net/ipv4/arp.c
This should have been:
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
Please correct these issues for future patch submissions.
Thanks.
^ permalink raw reply
* [PATCH] remove not used code: target hw address (tha) definition and assignment in arp_process().
From: Mark Ryden @ 2007-12-20 7:29 UTC (permalink / raw)
To: davem, netdev
[-- Attachment #1: Type: text/plain, Size: 157 bytes --]
Hello,
In arp_process() (net/ipv4/arp.c), there is unused code: definition
and assignment of tha (target hw address ).
Signed-off-by:
markryde@gmail.com
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patchGit.diff --]
[-- Type: text/x-patch; name=patchGit.diff, Size: 585 bytes --]
diff --git a/a/net/ipv4/arp.c b/b/net/ipv4/arp.c
index 14dec92..6f26cfe 100644
--- a/a/net/ipv4/arp.c
+++ b/b/net/ipv4/arp.c
@@ -706,7 +706,7 @@ static int arp_process(struct sk_buff *skb)
struct arphdr *arp;
unsigned char *arp_ptr;
struct rtable *rt;
- unsigned char *sha, *tha;
+ unsigned char *sha;
__be32 sip, tip;
u16 dev_type = dev->type;
int addr_type;
@@ -771,7 +771,6 @@ static int arp_process(struct sk_buff *skb)
arp_ptr += dev->addr_len;
memcpy(&sip, arp_ptr, 4);
arp_ptr += 4;
- tha = arp_ptr;
arp_ptr += dev->addr_len;
memcpy(&tip, arp_ptr, 4);
/*
^ permalink raw reply related
* [IPSEC]: Avoid undefined shift operation when testing algorithm ID
From: Herbert Xu @ 2007-12-20 4:29 UTC (permalink / raw)
To: David S. Miller, netdev
Hi Dave:
I had wanted to fix this for ages but kept putting it off and then
forgetting about it :) So before I forget again,
[IPSEC]: Avoid undefined shift operation when testing algorithm ID
The aalgos/ealgos fields are only 32 bits wide. However, af_key tries
to test them with the expression 1 << id where id can be as large as
253. This produces different behaviour on different architectures.
The following patch explicitly checks whether ID is greater than 31
and fails the check if that's the case.
We cannot easily extend the mask to be longer than 32 bits due to
exposure to user-space. Besides, this whole interface is obsolete
anyway in favour of the xfrm_user interface which doesn't use this
bit mask in templates (well not within the kernel anyway).
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 878039b..26d5e63 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2784,12 +2784,22 @@ static struct sadb_msg *pfkey_get_base_msg(struct sk_buff *skb, int *errp)
static inline int aalg_tmpl_set(struct xfrm_tmpl *t, struct xfrm_algo_desc *d)
{
- return t->aalgos & (1 << d->desc.sadb_alg_id);
+ unsigned int id = d->desc.sadb_alg_id;
+
+ if (id >= sizeof(t->aalgos) * 8)
+ return 0;
+
+ return (t->aalgos >> id) & 1;
}
static inline int ealg_tmpl_set(struct xfrm_tmpl *t, struct xfrm_algo_desc *d)
{
- return t->ealgos & (1 << d->desc.sadb_alg_id);
+ unsigned int id = d->desc.sadb_alg_id;
+
+ if (id >= sizeof(t->ealgos) * 8)
+ return 0;
+
+ return (t->ealgos >> id) & 1;
}
static int count_ah_combs(struct xfrm_tmpl *t)
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* [PATCH] [IPROUTE]: A workaround to make larger rto_min printed correctly
From: Satoru SATOH @ 2007-12-20 3:31 UTC (permalink / raw)
To: netdev
"ip route show" does not print correct value when larger rto_min is
set (e.g. 3sec).
This problem is because of overflow in print_route() and
the patch below is a workaround fix for that.
[root test]# ./iproute2.git.org/ip/ip route show dev eth1
192.168.140.0/24 proto kernel scope link src 192.168.140.130
169.254.0.0/16 scope link
[root test]# ./iproute2.git.org/ip/ip route change 192.168.140.0/24
dev eth1 rto_min 3s
[root test]# ./iproute2.git.org/ip/ip route show dev eth1
192.168.140.0/24 scope link rto_min lock 2ms <-- wrong
169.254.0.0/16 scope link
[root test]# ./iproute2.git/ip/ip route show dev eth1 # patched version
192.168.140.0/24 scope link rto_min lock 3000ms <-- correct
169.254.0.0/16 scope link
[root test]#
Signed-off-by: Satoru SATOH <satoru.satoh@gmail.com>
ip/iproute.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ip/iproute.c b/ip/iproute.c
index f4200ae..fa722c6 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -510,16 +510,16 @@ int print_route(const struct sockaddr_nl *who,
struct nlmsghdr *n, void *arg)
fprintf(fp, " %u",
*(unsigned*)RTA_DATA(mxrta[i]));
else {
unsigned val = *(unsigned*)RTA_DATA(mxrta[i]);
+ unsigned hz1 = hz / 1000;
- val *= 1000;
if (i == RTAX_RTT)
val /= 8;
else if (i == RTAX_RTTVAR)
val /= 4;
- if (val >= hz)
- fprintf(fp, " %ums", val/hz);
+ if (val >= hz1)
+ fprintf(fp, " %ums", val/hz1);
else
- fprintf(fp, " %.2fms", (float)val/hz);
+ fprintf(fp, " %.2fms", (float)val/hz1);
}
}
}
^ permalink raw reply related
* Re: TSO trimming question
From: Herbert Xu @ 2007-12-20 3:26 UTC (permalink / raw)
To: Ilpo J??rvinen; +Cc: davem, netdev
In-Reply-To: <Pine.LNX.4.64.0712200000050.27945@kivilampi-30.cs.helsinki.fi>
Ilpo J??rvinen <ilpo.jarvinen@helsinki.fi> wrote:
>
>> is the limitting factor? For latter IMHO this would be necessary:
>>
>> if (skb->len > limit)
>> limit -= limit % mss_now;
Good catch! But how about putting this logic into tcp_window_allows
since then you can avoid the divide for the case when we're not
bound by the receiver window.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] e1000: Use deferrable timer for watchdog
From: Kok, Auke @ 2007-12-19 23:00 UTC (permalink / raw)
To: Parag Warudkar; +Cc: netdev, linux-kernel, akpm, Arjan van de Ven
In-Reply-To: <82e4877d0712191435r5d3163f2i684270b6b41f4fd0@mail.gmail.com>
Parag Warudkar wrote:
> On Dec 19, 2007 4:38 PM, Kok, Auke <auke-jan.h.kok@intel.com> wrote:
>> Parag Warudkar wrote:
>>> On 12/19/07, Kok, Auke <auke-jan.h.kok@intel.com> wrote:
>> why would this patch reduce wakeups even more than round_jiffies()? Does it make
>> our ~2 second update interval not reliable? can you quantify "shows it reduces" ?
>> Or timer only runs once every two seconds...
>
> Without the patch - here is what powertop reports steady on my desktop -
>
> Wakeups-from-idle per second : 8.5 interval: 1.9s
> no ACPI power usage estimate available
>
> Top causes for wakeups:
> 28.6% ( 4.0) <kernel core> : clocksource_register (clocksource_watchdog)
> 14.3% ( 2.0) automount : futex_wait (hrtimer_wakeup)
> 14.3% ( 2.0) ntpd : do_setitimer (it_real_fn)
> 14.3% ( 2.0) ntpdate : do_adjtimex (sync_cmos_clock)
> 7.1% ( 1.0) <interrupt> : PS/2 keyboard/mouse/touchpad
> 7.1% ( 1.0) <interrupt> : eth0
> 7.1% ( 1.0) ip : e1000_intr_msi (e1000_watchdog)
>
> $> stop network; rmmod e1000e
> $> patch e1000e/netdev.c ; rebuild ; insmod
> $> Wait for things to settle
>
> With the patch here is what it shows steadily -
>
> Wakeups-from-idle per second : 7.5 interval: 5.8s
> no ACPI power usage estimate available
>
> Top causes for wakeups:
> 32.4% ( 2.2) <kernel core> : clocksource_register (clocksource_watchdog)
> 17.6% ( 1.2) ntpd : do_setitimer (it_real_fn)
> 14.7% ( 1.0) ntpdate : do_adjtimex (sync_cmos_clock)
> 8.8% ( 0.6) <interrupt> : eth0
> 5.9% ( 0.4) events/1 : __netdev_watchdog_up (dev_watchdog)
> 5.9% ( 0.4) <kernel core> : neigh_table_init_no_netlink
> (neigh_periodic_ 5.9% ( 0.4) <kernel module> :
> neigh_table_init_no_netlink (neigh_periodic_timer)
>
> So no longer e1000_watchdog is waking up the CPU for its own sake - it
> still runs but when the CPU is already out of IDLE to run something
> else that needs to be run undeferred.
> Wakeups from IDLE are down by 1 - from 8.5 to 7.5 .
>
>> maybe I just don't understand the effect of timer_set_deferrable() - we're already
>> deferring it ourselves when we want to. If that is not working then I suggest that
>> we fix that first instead of postponing the critical first run of the e1000
>> watchdog task.
>
> There is of course a difference between round_jiffies() and
> timer_set_deferrable() if that's what you were referring to.
> round_jiffies() will make the timer run at whatever rounded value no
> matter if the CPU is already IDLE or not. Making the timer deferrable
> makes it run only when the CPU is NOT IDLE - that is to say it is busy
> running something else - another non-deferrable timer for instance.
>
>> People in the datacenter really don't want to see more delays when bringing up
>> link, and we get frequent calls about it already being long on gigabit (not even
>> minding spanning tree). Adding 25% to that time isn't going to down very nicely
>> with them.
>>
> Well but when the machine is coming up the CPU is not going to be IDLE
> and your initial timer will likely run when it wants to - i.e.
> deferable timers won't be deferred if the CPU is not IDLE.
> On the other hand Data center people do care about power consumption
> and they would much rather make sure they don't lose network links on
> Production boxes - so a properly configured machine/network should not
> need to bring up the link more than a small number of times if at all.
> Lastly e1000 is also sold with many desktop machines (like mine) and
> those people will surely appreciate lesser wakeups.
>
> I don't have GigE connection where my desktop is located and with
> 100Mbps I don't notice any measurable delay in bringing up the link -
> may be you could try with this patch and see exactly how longer if at
> all it takes to bring up the link on a GigE connected machine.
OK, I think that would be an interesting venture and I'm willing to see if I can
get those numbers.
I'm just wondering if round_jiffies() is largely obsolete because of this. It
might just make things worse
Auke
^ permalink raw reply
* Re: [PATCH] e1000: Use deferrable timer for watchdog
From: Parag Warudkar @ 2007-12-19 22:35 UTC (permalink / raw)
To: Kok, Auke; +Cc: netdev, linux-kernel, akpm, Arjan van de Ven
In-Reply-To: <47698F4E.40209@intel.com>
On Dec 19, 2007 4:38 PM, Kok, Auke <auke-jan.h.kok@intel.com> wrote:
> Parag Warudkar wrote:
> > On 12/19/07, Kok, Auke <auke-jan.h.kok@intel.com> wrote:
>
> why would this patch reduce wakeups even more than round_jiffies()? Does it make
> our ~2 second update interval not reliable? can you quantify "shows it reduces" ?
> Or timer only runs once every two seconds...
Without the patch - here is what powertop reports steady on my desktop -
Wakeups-from-idle per second : 8.5 interval: 1.9s
no ACPI power usage estimate available
Top causes for wakeups:
28.6% ( 4.0) <kernel core> : clocksource_register (clocksource_watchdog)
14.3% ( 2.0) automount : futex_wait (hrtimer_wakeup)
14.3% ( 2.0) ntpd : do_setitimer (it_real_fn)
14.3% ( 2.0) ntpdate : do_adjtimex (sync_cmos_clock)
7.1% ( 1.0) <interrupt> : PS/2 keyboard/mouse/touchpad
7.1% ( 1.0) <interrupt> : eth0
7.1% ( 1.0) ip : e1000_intr_msi (e1000_watchdog)
$> stop network; rmmod e1000e
$> patch e1000e/netdev.c ; rebuild ; insmod
$> Wait for things to settle
With the patch here is what it shows steadily -
Wakeups-from-idle per second : 7.5 interval: 5.8s
no ACPI power usage estimate available
Top causes for wakeups:
32.4% ( 2.2) <kernel core> : clocksource_register (clocksource_watchdog)
17.6% ( 1.2) ntpd : do_setitimer (it_real_fn)
14.7% ( 1.0) ntpdate : do_adjtimex (sync_cmos_clock)
8.8% ( 0.6) <interrupt> : eth0
5.9% ( 0.4) events/1 : __netdev_watchdog_up (dev_watchdog)
5.9% ( 0.4) <kernel core> : neigh_table_init_no_netlink
(neigh_periodic_ 5.9% ( 0.4) <kernel module> :
neigh_table_init_no_netlink (neigh_periodic_timer)
So no longer e1000_watchdog is waking up the CPU for its own sake - it
still runs but when the CPU is already out of IDLE to run something
else that needs to be run undeferred.
Wakeups from IDLE are down by 1 - from 8.5 to 7.5 .
>
> maybe I just don't understand the effect of timer_set_deferrable() - we're already
> deferring it ourselves when we want to. If that is not working then I suggest that
> we fix that first instead of postponing the critical first run of the e1000
> watchdog task.
There is of course a difference between round_jiffies() and
timer_set_deferrable() if that's what you were referring to.
round_jiffies() will make the timer run at whatever rounded value no
matter if the CPU is already IDLE or not. Making the timer deferrable
makes it run only when the CPU is NOT IDLE - that is to say it is busy
running something else - another non-deferrable timer for instance.
>
> People in the datacenter really don't want to see more delays when bringing up
> link, and we get frequent calls about it already being long on gigabit (not even
> minding spanning tree). Adding 25% to that time isn't going to down very nicely
> with them.
>
Well but when the machine is coming up the CPU is not going to be IDLE
and your initial timer will likely run when it wants to - i.e.
deferable timers won't be deferred if the CPU is not IDLE.
On the other hand Data center people do care about power consumption
and they would much rather make sure they don't lose network links on
Production boxes - so a properly configured machine/network should not
need to bring up the link more than a small number of times if at all.
Lastly e1000 is also sold with many desktop machines (like mine) and
those people will surely appreciate lesser wakeups.
I don't have GigE connection where my desktop is located and with
100Mbps I don't notice any measurable delay in bringing up the link -
may be you could try with this patch and see exactly how longer if at
all it takes to bring up the link on a GigE connected machine.
Parag
^ permalink raw reply
* [RFC PATCH] LSM: Add inet_sys_snd_skb() LSM hook
From: Paul Moore @ 2007-12-19 22:21 UTC (permalink / raw)
To: netdev
In-Reply-To: <20071219220539.1626.46073.stgit@flek.americas.hpqcorp.net>
Add an inet_sys_snd_skb() LSM hook to allow the LSM to provide packet level
access control for all outbound packets. Using the existing postroute_last
netfilter hook turns out to be problematic as it is can be invoked multiple
times for a single packet, e.g. individual IPsec transforms, adding unwanted
overhead and complicating the security policy.
---
include/linux/security.h | 11 +++++++++++
net/ipv4/ip_output.c | 7 +++++++
net/ipv6/ip6_output.c | 5 +++++
security/dummy.c | 8 +++++++-
security/security.c | 6 ++++++
5 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index db19c92..1b8d332 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -876,6 +876,10 @@ struct request_sock;
* Sets the connection's peersid to the secmark on skb.
* @req_classify_flow:
* Sets the flow's sid to the openreq sid.
+ * @inet_sys_snd_skb:
+ * Check permissions on outgoing network packets.
+ * @skb is the packet to check
+ * @family is the packet's address family
*
* Security hooks for XFRM operations.
*
@@ -1416,6 +1420,7 @@ struct security_operations {
void (*inet_csk_clone)(struct sock *newsk, const struct request_sock *req);
void (*inet_conn_established)(struct sock *sk, struct sk_buff *skb);
void (*req_classify_flow)(const struct request_sock *req, struct flowi *fl);
+ int (*inet_sys_snd_skb)(struct sk_buff *skb, int family);
#endif /* CONFIG_SECURITY_NETWORK */
#ifdef CONFIG_SECURITY_NETWORK_XFRM
@@ -2328,6 +2333,7 @@ void security_sk_free(struct sock *sk);
void security_sk_clone(const struct sock *sk, struct sock *newsk);
void security_sk_classify_flow(struct sock *sk, struct flowi *fl);
void security_req_classify_flow(const struct request_sock *req, struct flowi *fl);
+int security_inet_sys_snd_skb(struct sk_buff *skb, int family);
void security_sock_graft(struct sock*sk, struct socket *parent);
int security_inet_conn_request(struct sock *sk,
struct sk_buff *skb, struct request_sock *req);
@@ -2471,6 +2477,11 @@ static inline void security_req_classify_flow(const struct request_sock *req, st
{
}
+static inline int security_inet_sys_snd_skb(struct sk_buff *skb, int family)
+{
+ return 0;
+}
+
static inline void security_sock_graft(struct sock* sk, struct socket *parent)
{
}
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index fd99fbd..82a7297 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -204,6 +204,8 @@ static inline int ip_skb_dst_mtu(struct sk_buff *skb)
static int ip_finish_output(struct sk_buff *skb)
{
+ int err;
+
#if defined(CONFIG_NETFILTER) && defined(CONFIG_XFRM)
/* Policy lookup after SNAT yielded a new policy */
if (skb->dst->xfrm != NULL) {
@@ -211,6 +213,11 @@ static int ip_finish_output(struct sk_buff *skb)
return dst_output(skb);
}
#endif
+
+ err = security_inet_sys_snd_skb(skb, AF_INET);
+ if (err)
+ return err;
+
if (skb->len > ip_skb_dst_mtu(skb) && !skb_is_gso(skb))
return ip_fragment(skb, ip_finish_output2);
else
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 6338a9c..44ddf32 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -72,8 +72,13 @@ static __inline__ void ipv6_select_ident(struct sk_buff *skb, struct frag_hdr *f
static int ip6_output_finish(struct sk_buff *skb)
{
+ int err;
struct dst_entry *dst = skb->dst;
+ err = security_inet_sys_snd_skb(skb, AF_INET6);
+ if (err)
+ return err;
+
if (dst->hh)
return neigh_hh_output(dst->hh, skb);
else if (dst->neighbour)
diff --git a/security/dummy.c b/security/dummy.c
index 0b62f95..384979a 100644
--- a/security/dummy.c
+++ b/security/dummy.c
@@ -848,6 +848,11 @@ static inline void dummy_req_classify_flow(const struct request_sock *req,
struct flowi *fl)
{
}
+
+static inline int dummy_inet_sys_snd_skb(struct sk_buff *skb, int family)
+{
+ return 0;
+}
#endif /* CONFIG_SECURITY_NETWORK */
#ifdef CONFIG_SECURITY_NETWORK_XFRM
@@ -1122,7 +1127,8 @@ void security_fixup_ops (struct security_operations *ops)
set_to_dummy_if_null(ops, inet_csk_clone);
set_to_dummy_if_null(ops, inet_conn_established);
set_to_dummy_if_null(ops, req_classify_flow);
- #endif /* CONFIG_SECURITY_NETWORK */
+ set_to_dummy_if_null(ops, inet_sys_snd_skb);
+#endif /* CONFIG_SECURITY_NETWORK */
#ifdef CONFIG_SECURITY_NETWORK_XFRM
set_to_dummy_if_null(ops, xfrm_policy_alloc_security);
set_to_dummy_if_null(ops, xfrm_policy_clone_security);
diff --git a/security/security.c b/security/security.c
index 3bdcada..7f55459 100644
--- a/security/security.c
+++ b/security/security.c
@@ -961,6 +961,12 @@ void security_req_classify_flow(const struct request_sock *req, struct flowi *fl
}
EXPORT_SYMBOL(security_req_classify_flow);
+int security_inet_sys_snd_skb(struct sk_buff *skb, int family)
+{
+ return security_ops->inet_sys_snd_skb(skb, family);
+}
+EXPORT_SYMBOL(security_inet_sys_snd_skb);
+
void security_sock_graft(struct sock *sk, struct socket *parent)
{
security_ops->sock_graft(sk, parent);
^ permalink raw reply related
* [RFC PATCH] New LSM hook to catch outbound packets
From: Paul Moore @ 2007-12-19 22:20 UTC (permalink / raw)
To: netdev
Currently LSMs need to use a netfilter post routing hook to catch outbound
packets and subject them to access control. This works reasonably well but
has always been a bit awkward when IPsec or similar mechanisms were used
because the same packet would end up going through the same LSM hook multiple
times. For obvious reasons this often resulted in unnecessary overhead and
additional headaches when trying to determining the correct LSM security
policy.
This patch attempts to fix this problem by adding a new hook into both the
IPv4 and IPv6 output path. The motiviation behind this new hook is a request
from users to provide packet level ingress/egress access control for all
packets on the system, not just packets that are locally consumed or generated.
I know new networking LSM hooks are frowned upon but there has been a lot of
thought and discussion put into this and we haven't been able to find a better
solution. I've trimmed the rest of the patchset from this posting as it isn't
really relevant for this discussion (the full patchset has been under
discussion on the SELinux and LSM lists), but those who are curious can find
the patches online here (this will see another update later today):
* git://git.infradead.org/users/pcmoore/lblnet-2.6_testing
Thanks.
--
paul moore
linux security @ hp
^ permalink raw reply
* Re: [PATCH 00/29] Swap over NFS -v15
From: Bill Davidsen @ 2007-12-19 22:22 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Linus Torvalds, Andrew Morton, linux-kernel, linux-mm, netdev,
trond.myklebust
In-Reply-To: <20071214153907.770251000@chello.nl>
Peter Zijlstra wrote:
> Hi,
>
> Another posting of the full swap over NFS series.
>
> Andrew/Linus, could we start thinking of sticking this in -mm?
>
Two questions:
1 - what is the memory use impact on the system which don't do swap over
NFS, such as embedded systems, and
2 - what is the advantage of this code over the two existing network
swap approaches, swapping to NFS mounted file and swap to NBD device?
I've used the NFS file when a program was running out of memory and that
seemed to work, people in UNYUUG have reported that the nbd swap works,
so what's better here?
--
Bill Davidsen <davidsen@tmr.com>
"We have more to fear from the bungling of the incompetent than from
the machinations of the wicked." - from Slashdot
^ permalink raw reply
* Re: TSO trimming question
From: Ilpo Järvinen @ 2007-12-19 22:02 UTC (permalink / raw)
To: David Miller; +Cc: Netdev
In-Reply-To: <Pine.LNX.4.64.0712192336130.27945@kivilampi-30.cs.helsinki.fi>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1717 bytes --]
On Wed, 19 Dec 2007, Ilpo Järvinen wrote:
> I'm not fully sure what's purpose of this code in tcp_write_xmit:
>
> if (skb->len < limit) {
> unsigned int trim = skb->len % mss_now;
>
> if (trim)
> limit = skb->len - trim;
> }
>
> Is it used to make sure we send only multiples of mss_now here and leave
> the left-over into another skb? Or does it try to make sure that
> tso_fragment result honors multiple of mss_now boundaries when snd_wnd
> is the limitting factor? For latter IMHO this would be necessary:
>
> if (skb->len > limit)
> limit -= limit % mss_now;
...Anyway, here's an untested patch to just do it :-):
--
i.
[PATCH] [TCP]: Force tso skb split to mss_now boundary (if snd_wnd limits)
If snd_wnd was limitting factor, the tso_fragment might not
create full-sized skbs but would include the window left-overs
as well.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
net/ipv4/tcp_output.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 1c7ef17..8dafda9 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1453,6 +1453,8 @@ static int tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle)
if (trim)
limit = skb->len - trim;
+ } else if (skb->len > limit) {
+ limit -= limit % mss_now;
}
}
@@ -1525,6 +1527,8 @@ void tcp_push_one(struct sock *sk, unsigned int mss_now)
if (trim)
limit = skb->len - trim;
+ } else if (skb->len > limit) {
+ limit -= limit % mss_now;
}
}
--
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