* Re: [PATCH] Add IPv6 support to TCP SYN cookies
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2008-02-13 7:31 UTC (permalink / raw)
To: dada1
Cc: johnpol, ggriffin.kernel, alan, andi, netdev, linux-kernel, davem,
yoshfuji
In-Reply-To: <47AAD203.4030706@cosmosbay.com>
In article <47AAD203.4030706@cosmosbay.com> (at Thu, 07 Feb 2008 10:40:19 +0100), Eric Dumazet <dada1@cosmosbay.com> says:
> [NET] IPV4: lower stack usage in cookie_hash() function
>
> 400 bytes allocated on stack might be a litle bit too much. Using a
> per_cpu var is more friendly.
>
> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
Applied to my inet6-2.6.26 tree. Thanks.
--yoshfuji
^ permalink raw reply
* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: Jarek Poplawski @ 2008-02-13 7:29 UTC (permalink / raw)
To: David Miller; +Cc: jchapman, netdev
In-Reply-To: <20080212.220003.108906105.davem@davemloft.net>
On Tue, Feb 12, 2008 at 10:00:03PM -0800, David Miller wrote:
> From: James Chapman <jchapman@katalix.com>
> Date: Tue, 12 Feb 2008 10:58:21 +0000
>
> > Here is a trace from when we had _bh locks.
>
> The problem is that the pppol2tp code calls sk_dst_get() in software
> interrupt context and that is not allowed.
Actually, this lockdep report probably warns of something else:
sk_dst_lock which is seen in process context with softirqs enabled
implicitly endangers some other (ppp_generic) locks, which depend
on ppp_generic pch->upl read_lock, which is taken in softirq context.
I can't see on this report any hardirqs dependancies, so it seems even
blocking hard interrupts, just like in this patch, shouldn't solve
this problem: if BHs were really disabled in exactly the same places,
then it seems this warning should trigger in both variants.
I studied long ago some similar problem with pppoe, and it looked like
ppp_generic needed some fix, but now I've to recall or re-learn almost
all and it needs more time... Anyway, there is a possibility, this
warning isn't so dangerous.
> sk_dst_get() grabs sk->sk_dst_lock without any BH enabling or
> disabling.
>
> It can do that because the usage is to make all the lock
> taking calls in user context, and in the packet processing
> paths use __sk_dst_get().
BTW, does it mean that ip4_datagram_connect() can be used only in user
context?
> Probably what the pppol2tp code should do is use __sk_dst_check()
> instead of sk_dst_get(). You then have to be able to handle
> NULL returns, just like UDP sendmsg() does, which means you'll
> need to cook up a routing lookup if __sk_dst_check() gives you
> NULL because the route became obsolete.
I think that without changing ppp_generic or changing ip_queue_xmit()
to something else in pppol2tp this would be really hard to avoid such
a warning (and maybe not very necessary).
Regards,
Jarek P.
^ permalink raw reply
* Re: [PATCH net-2.6.25] net: Improve cache line coherency of ingress qdisc
From: David Miller @ 2008-02-13 7:14 UTC (permalink / raw)
To: nturton; +Cc: netdev, linux-net-drivers
In-Reply-To: <478F364C.1000301@solarflare.com>
From: Neil Turton <nturton@solarflare.com>
Date: Thu, 17 Jan 2008 11:04:44 +0000
> Move the ingress qdisc members of struct net_device from the transmit
> cache line to the receive cache line to avoid cache line ping-pong.
> These members are only used on the receive path.
>
> Signed-off-by: Neil Turton <nturton@solarflare.com>
Looks reasonable, applied, thanks!
^ permalink raw reply
* Re: [Bug 9750] [patch 2.6.25 v2] dev: avoid a race that triggers assertion failure
From: David Miller @ 2008-02-13 7:10 UTC (permalink / raw)
To: mattilinnanvuori; +Cc: bugme-daemon, netdev
In-Reply-To: <127401.85852.qm@web52006.mail.re2.yahoo.com>
From: Matti Linnanvuori <mattilinnanvuori@yahoo.com>
Date: Tue, 12 Feb 2008 23:06:41 -0800 (PST)
> There is a race in Linux kernel file net/core/dev.c, function dev_close.
> The function calls function dev_deactivate, which calls function
> dev_watchdog_down that deletes the watchdog timer. However, after that, a
> driver can call netif_carrier_ok, which calls function
> __netdev_watchdog_up that can add the watchdog timer again. Function
> unregister_netdevice calls function dev_shutdown that traps the bug
> !timer_pending(&dev->watchdog_timer). Moving dev_deactivate after
> netif_running() has been cleared prevents function netif_carrier_on
> from calling __netdev_watchdog_up and adding the watchdog timer again.
>
> Signed-off-by: Matti Linnanvuori <mattilinnanvuori@yahoo.com>
Patch applied, thanks.
^ permalink raw reply
* Re: [PATCH] Reorder ACK/RST checking in LISTEN state
From: David Miller @ 2008-02-13 7:08 UTC (permalink / raw)
To: katterjohn; +Cc: netdev
In-Reply-To: <47B29055.8060409@gmail.com>
From: Kris Katterjohn <katterjohn@gmail.com>
Date: Wed, 13 Feb 2008 00:38:13 -0600
> I've attached a patch that changes the order of the ACK and RST checking
> in the LISTEN state in tcp_rcv_state_process() in tcp_input.c
>
> Before: If an ACK/RST packet is received, then tcp_rcv_state_process()
> would return 1 because of the ACK. Then (following the function calls
> in tcp_ipv4.c and tcp_minisocks.c), tcp_v4_send_reset() is called--but
> since there is a RST in the packet it just returns. After this, the
> kfree_skb() is called. The same goes in tcp_ipv6.c as well.
>
> But if the order of the ACK and RST checking is reversed, __kfree_skb()
> is called in tcp_rcv_state_process() because of the RST and the function
> returns 0, which skips that other useless stuff.
>
> This is the order specified on page 65 of RFC 793 anyway.
>
> Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
This code has been like this for I don't know how many years,
the end result is the same both before and after your patch,
and the added expense of the existing code is frankly trivial.
I really don't want to apply this, it doesn't buy us anything,
sorry.
^ permalink raw reply
* [Bug 9750] [patch 2.6.25 v2] dev: avoid a race that triggers assertion failure
From: Matti Linnanvuori @ 2008-02-13 7:06 UTC (permalink / raw)
To: bugme-daemon, davem, netdev
[-- Attachment #1: Type: text/plain, Size: 901 bytes --]
From: Matti Linnanvuori <mattilinnanvuori@yahoo.com>
There is a race in Linux kernel file net/core/dev.c, function dev_close.
The function calls function dev_deactivate, which calls function
dev_watchdog_down that deletes the watchdog timer. However, after that, a
driver can call netif_carrier_ok, which calls function
__netdev_watchdog_up that can add the watchdog timer again. Function
unregister_netdevice calls function dev_shutdown that traps the bug
!timer_pending(&dev->watchdog_timer). Moving dev_deactivate after
netif_running() has been cleared prevents function netif_carrier_on
from calling __netdev_watchdog_up and adding the watchdog timer again.
Signed-off-by: Matti Linnanvuori <mattilinnanvuori@yahoo.com>
---
____________________________________________________________________________________
Never miss a thing. Make Yahoo your home page.
http://www.yahoo.com/r/hs
[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 1351 bytes --]
From: Matti Linnanvuori <mattilinnanvuori@yahoo.com>
There is a race in Linux kernel file net/core/dev.c, function dev_close.
The function calls function dev_deactivate, which calls function
dev_watchdog_down that deletes the watchdog timer. However, after that, a
driver can call netif_carrier_ok, which calls function
__netdev_watchdog_up that can add the watchdog timer again. Function
unregister_netdevice calls function dev_shutdown that traps the bug
!timer_pending(&dev->watchdog_timer). Moving dev_deactivate after
netif_running() has been cleared prevents function netif_carrier_on
from calling __netdev_watchdog_up and adding the watchdog timer again.
Signed-off-by: Matti Linnanvuori <mattilinnanvuori@yahoo.com>
---
--- linux-2.6.25/net/core/dev.c 2008-02-12 12:37:51.206833000 +0200
+++ b/net/core/dev.c 2008-02-13 09:01:25.697592900 +0200
@@ -1071,8 +1071,6 @@ int dev_close(struct net_device *dev)
*/
call_netdevice_notifiers(NETDEV_GOING_DOWN, dev);
- dev_deactivate(dev);
-
clear_bit(__LINK_STATE_START, &dev->state);
/* Synchronize to scheduled poll. We cannot touch poll list,
@@ -1083,6 +1081,8 @@ int dev_close(struct net_device *dev)
*/
smp_mb__after_clear_bit(); /* Commit netif_running(). */
+ dev_deactivate(dev);
+
/*
* Call the device specific close. This cannot fail.
* Only if device is UP
^ permalink raw reply
* Re: [PATCH RESEND] [XFRM]: Speed up xfrm_policy and xfrm_state walking
From: David Miller @ 2008-02-13 6:59 UTC (permalink / raw)
To: timo.teras; +Cc: netdev
In-Reply-To: <47A3337C.1090004@iki.fi>
From: Timo_Teräs <timo.teras@iki.fi>
Date: Fri, 01 Feb 2008 16:58:04 +0200
> @@ -1780,6 +1786,7 @@ static int check_reqid(struct xfrm_policy *xp, int dir, int count, void *ptr)
>
> static u32 gen_reqid(void)
> {
> + struct xfrm_policy_walk walk;
> u32 start;
> static u32 reqid = IPSEC_MANUAL_REQID_MAX;
>
> @@ -1788,9 +1795,10 @@ static u32 gen_reqid(void)
> ++reqid;
> if (reqid == 0)
> reqid = IPSEC_MANUAL_REQID_MAX+1;
> - if (xfrm_policy_walk(XFRM_POLICY_TYPE_MAIN, check_reqid,
> - (void*)&reqid) != -EEXIST)
> + xfrm_policy_walk_init(&walk, XFRM_POLICY_TYPE_MAIN);
> + if (xfrm_policy_walk(&walk, check_reqid, (void*)&reqid) != -EEXIST)
> return reqid;
> + xfrm_policy_walk_done(&walk);
> } while (reqid != start);
> return 0;
> }
This will potentially leak walk->policy in the != -EEXIST case.
I think what needs to happen is we invoke xfrm_policy_walk_done()
unconditionally, then we'll potentially return reqid.
^ permalink raw reply
* Re: xfrm_input() and ->seq oddities
From: David Miller @ 2008-02-13 6:54 UTC (permalink / raw)
To: herbert; +Cc: viro, netdev
In-Reply-To: <20080203220027.GA11641@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 4 Feb 2008 09:00:27 +1100
> On Sun, Feb 03, 2008 at 11:04:44AM +0000, Al Viro wrote:
> > IMO that at least deserves a comment near xfrm_input()...
>
> Sure. There is already a comment about encap_type < 0 in there, but
> I think you'll probably be able to explain it much better than I can
> looking in from the outside so if you have a patch... :)
Agreed on all counts ;-)
^ permalink raw reply
* Re: xfrm_input() and ->seq oddities
From: David Miller @ 2008-02-13 6:54 UTC (permalink / raw)
To: herbert; +Cc: viro, netdev
In-Reply-To: <20080203002019.GA32295@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sun, 3 Feb 2008 11:20:19 +1100
> [IPSEC]: Fix bogus usage of u64 on input sequence number
>
> Al Viro spotted a bogus use of u64 on the input sequence number which
> is big-endian. This patch fixes it by giving the input sequence number
> its own member in the xfrm_skb_cb structure.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Applied, thanks Herbert.
^ permalink raw reply
* Re: [PATCH] rtnetlink.c: send a single notification on device state changes
From: David Miller @ 2008-02-13 6:42 UTC (permalink / raw)
To: panther; +Cc: netdev
In-Reply-To: <1202385470416-git-send-email-panther@balabit.hu>
From: Laszlo Attila Toth <panther@balabit.hu>
Date: Thu, 7 Feb 2008 12:57:50 +0100
> In do_setlink() a single notification is sent at the end of the function
> if any modification occured. If the address has been changed, another
> notification is sent.
>
> Both of them is required because originally only the NETDEV_CHANGEADDR
> notification was sent and although device state change implies address
> change, some programs may expect the original notification. It remains
> for compatibity.
>
> If set_operstate() is called from do_setlink(), it doesn't send
> a notification, only if it is called from rtnl_create_link() as earlier.
>
> Signed-off-by: Laszlo Attila Toth <panther@balabit.hu>
Looks good, applied.
Let's hope this doesn't break some stupid application
unintentionally.
^ permalink raw reply
* [PATCH] Reorder ACK/RST checking in LISTEN state
From: Kris Katterjohn @ 2008-02-13 6:38 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 890 bytes --]
Hey everyone,
[I'm not subscribed, so please CC me on any replies]
I've attached a patch that changes the order of the ACK and RST checking
in the LISTEN state in tcp_rcv_state_process() in tcp_input.c
Before: If an ACK/RST packet is received, then tcp_rcv_state_process()
would return 1 because of the ACK. Then (following the function calls
in tcp_ipv4.c and tcp_minisocks.c), tcp_v4_send_reset() is called--but
since there is a RST in the packet it just returns. After this, the
kfree_skb() is called. The same goes in tcp_ipv6.c as well.
But if the order of the ACK and RST checking is reversed, __kfree_skb()
is called in tcp_rcv_state_process() because of the RST and the function
returns 0, which skips that other useless stuff.
This is the order specified on page 65 of RFC 793 anyway.
Signed-off-by: Kris Katterjohn <katterjohn@gmail.com>
Thanks,
Kris Katterjohn
[-- Attachment #2: ackrst.patch --]
[-- Type: text/x-patch, Size: 412 bytes --]
--- net/ipv4/tcp_input.c 2008-02-13 00:05:59.000000000 -0600
+++ net/ipv4/tcp_input.c 2008-02-13 00:10:40.000000000 -0600
@@ -4962,12 +4962,12 @@ int tcp_rcv_state_process(struct sock *s
goto discard;
case TCP_LISTEN:
- if (th->ack)
- return 1;
-
if (th->rst)
goto discard;
+ if (th->ack)
+ return 1;
+
if (th->syn) {
if (icsk->icsk_af_ops->conn_request(sk, skb) < 0)
return 1;
^ permalink raw reply
* Re: [PATCH 3/3][NETLABLE]: Hide netlbl_unlabel_audit_addr6 under ifdef CONFIG_IPV6.
From: David Miller @ 2008-02-13 6:38 UTC (permalink / raw)
To: xemul; +Cc: netdev
In-Reply-To: <47AB3F20.2030809@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Thu, 07 Feb 2008 20:25:52 +0300
> This one is called from under this config only, so move
> it in the same place.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH 2/3][NETLABEL]: Don't produce unused variables when IPv6 is off.
From: David Miller @ 2008-02-13 6:37 UTC (permalink / raw)
To: xemul; +Cc: netdev
In-Reply-To: <47AB3EBF.2090300@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Thu, 07 Feb 2008 20:24:15 +0300
> Some code declares variables on the stack, but uses them
> under #ifdef CONFIG_IPV6, so thay become unused when ipv6
> is off. Fortunately, they are used in a switch's case
> branches, so the fix is rather simple.
>
> Is it OK from coding style POV to add braces inside "cases",
> or should I better avoid such style and rework the patch?
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 1/3][NETLABEL]: Compilation for CONFIG_AUDIT=n case.
From: David Miller @ 2008-02-13 6:35 UTC (permalink / raw)
To: xemul; +Cc: netdev
In-Reply-To: <47AB3DDE.8050109@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Thu, 07 Feb 2008 20:20:30 +0300
> The audit_log_start() will expand into an empty do { } while (0)
> construction and the audit_ctx becomes unused.
>
> The solution: push current->audit_context into audit_log_start()
> directly, since it is not required in any other place in the
> calling function.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Applied.
^ permalink raw reply
* Re: [PATCH][GENETLINK]: Relax dances with genl_lock.
From: David Miller @ 2008-02-13 6:16 UTC (permalink / raw)
To: xemul; +Cc: netdev
In-Reply-To: <47B1D45D.3020409@openvz.org>
From: Pavel Emelyanov <xemul@openvz.org>
Date: Tue, 12 Feb 2008 20:16:13 +0300
> The genl_unregister_family() calls the genl_unregister_mc_groups(),
> which takes and releases the genl_lock and then locks and releases
> this lock itself.
>
> Relax this behavior, all the more so the genl_unregister_mc_groups()
> is called from genl_unregister_family() only.
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Ok, applied, thanks.
^ permalink raw reply
* Re: [PATCH][NETLABEL]: Fix lookup logic of netlbl_domhsh_search_def.
From: David Miller @ 2008-02-13 6:15 UTC (permalink / raw)
To: paul.moore; +Cc: xemul, netdev
In-Reply-To: <200802121134.09965.paul.moore@hp.com>
From: Paul Moore <paul.moore@hp.com>
Date: Tue, 12 Feb 2008 11:34:09 -0500
> On Tuesday 12 February 2008 11:27:16 am Pavel Emelyanov wrote:
> > Currently, if the call to netlbl_domhsh_search succeeds the
> > return result will still be NULL.
> >
> > Fix that, by returning the found entry (if any).
> >
> > Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
>
> Good catch, thanks.
>
> Acked-by: Paul Moore <paul.moore@hp.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH] [IPV6] Minor cleanup: remove unused method declaration (net/ndisc.h).
From: David Miller @ 2008-02-13 6:07 UTC (permalink / raw)
To: ramirose; +Cc: netdev
In-Reply-To: <eb3ff54b0802100840g52a5eb68y8140d3e2e2d94c26@mail.gmail.com>
From: "Rami Rosen" <ramirose@gmail.com>
Date: Sun, 10 Feb 2008 18:40:13 +0200
> This patch removes unused declaration of dflt_rt_lookup() method in
> include/net/ndisc.h
>
> Signed-off-by: Rami Rosen <ramirose@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [RFC, PATCH]: Pass link level header from/to PPP interface
From: David Miller @ 2008-02-13 6:06 UTC (permalink / raw)
To: urs; +Cc: netdev
In-Reply-To: <ygfprv55egs.fsf@janus.isnogud.escape.de>
From: Urs Thuermann <urs@isnogud.escape.de>
Date: 10 Feb 2008 10:48:51 +0100
> So what is your opinion about this change?
No general objections from me.
But if libpcap and tcpdump can already identify PPP packets
then, besides "consistency", what does this buy us other
than potential breakage?
^ permalink raw reply
* Re: [PATCH] Fix comment for skb_pull_rcsum
From: David Miller @ 2008-02-13 6:03 UTC (permalink / raw)
To: urs; +Cc: netdev
In-Reply-To: <ygftzkh5ei0.fsf@janus.isnogud.escape.de>
From: Urs Thuermann <urs@isnogud.escape.de>
Date: 10 Feb 2008 10:48:07 +0100
> Fix comment for skb_pull_rcsum
>
> Signed-off-by: Urs Thuermann <urs@isnogud.escape.de>
Applied, thanks.
^ permalink raw reply
* Re: An ioctl to delete an ipv6 tunnel leads to a kernel panic
From: Natalie Protasevich @ 2008-02-13 6:00 UTC (permalink / raw)
To: David Miller; +Cc: netdev, akpm
In-Reply-To: <20080212.214946.168597748.davem@davemloft.net>
On Feb 12, 2008 9:49 PM, David Miller <davem@davemloft.net> wrote:
> From: "Natalie Protasevich" <protasnb@gmail.com>
> Date: Mon, 11 Feb 2008 12:49:12 -0800
>
> > Possible reason for this failure was identified and tested by the
> > submitter and several other reporters that ran into the same problem.
> > Can the patch be reviewed and pushed upstream if accepted (if the
> > problem hasn't been addressed already)?
>
> There are a lot of bogus patches in there, using funny
> long variable names, and mainly they were meant for testing
> and verification of the problem.
>
> I see no real serious patch submissions in that bug and furthermore
> the patch, if ready, should be submitted formally here to netdev not
> rot in bugzilla.
>
> Finally, what appears to be the proposal cannot be correct. If the
> fib6_add_rt2node() finds that the new route is a duplicate, we should
> disconnect it from the fn->leaf and do a dst_release(). The bug
> appears to be rather that we leave the route attached to the fn, not
> that we drop the refrence to it.
>
> Thank you.
Thanks David for looking in this. I will give this thought to the
diligent reporters, unless someone on the net team can produce a patch
for them to test.
Sometimes reporters come up with patches and I always try to make sure
the patches end up on appropriate mailing list, and I will continue
doing so :)
Regards,
--Natalie
>
^ permalink raw reply
* Re: [PATCH][PPPOL2TP]: Fix SMP oops in pppol2tp driver
From: David Miller @ 2008-02-13 6:00 UTC (permalink / raw)
To: jchapman; +Cc: jarkao2, netdev
In-Reply-To: <47B17BCD.2070903@katalix.com>
From: James Chapman <jchapman@katalix.com>
Date: Tue, 12 Feb 2008 10:58:21 +0000
> Here is a trace from when we had _bh locks.
The problem is that the pppol2tp code calls sk_dst_get() in software
interrupt context and that is not allowed.
sk_dst_get() grabs sk->sk_dst_lock without any BH enabling or
disabling.
It can do that because the usage is to make all the lock
taking calls in user context, and in the packet processing
paths use __sk_dst_get().
Probably what the pppol2tp code should do is use __sk_dst_check()
instead of sk_dst_get(). You then have to be able to handle
NULL returns, just like UDP sendmsg() does, which means you'll
need to cook up a routing lookup if __sk_dst_check() gives you
NULL because the route became obsolete.
^ permalink raw reply
* Re: An ioctl to delete an ipv6 tunnel leads to a kernel panic
From: David Miller @ 2008-02-13 5:49 UTC (permalink / raw)
To: protasnb; +Cc: netdev, akpm
In-Reply-To: <32209efe0802111249x16a6bbe0l4c152b3cf3aa0470@mail.gmail.com>
From: "Natalie Protasevich" <protasnb@gmail.com>
Date: Mon, 11 Feb 2008 12:49:12 -0800
> Possible reason for this failure was identified and tested by the
> submitter and several other reporters that ran into the same problem.
> Can the patch be reviewed and pushed upstream if accepted (if the
> problem hasn't been addressed already)?
There are a lot of bogus patches in there, using funny
long variable names, and mainly they were meant for testing
and verification of the problem.
I see no real serious patch submissions in that bug and furthermore
the patch, if ready, should be submitted formally here to netdev not
rot in bugzilla.
Finally, what appears to be the proposal cannot be correct. If the
fib6_add_rt2node() finds that the new route is a duplicate, we should
disconnect it from the fn->leaf and do a dst_release(). The bug
appears to be rather that we leave the route attached to the fn, not
that we drop the refrence to it.
Thank you.
^ permalink raw reply
* Re: [PATCH] [NET]: Add per-connection option to set max TSO frame size
From: Stephen Hemminger @ 2008-02-13 5:22 UTC (permalink / raw)
To: netdev
In-Reply-To: <20080212220132.8446.88493.stgit@localhost.localdomain>
On Tue, 12 Feb 2008 14:01:32 -0800
PJ Waskiewicz <peter.p.waskiewicz.jr@intel.com> wrote:
> This patch adds the ability for device drivers to control the size of the
> TSO frames being sent to them, per TCP connection. By setting the
> netdevice's max_gso_frame_size value, the socket layer will set the GSO
> frame size based on that value. This will propogate into the TCP layer,
> and send TSO's of that size to the hardware.
>
> This can be desirable to help tune the bursty nature of TSO on a
> per-adapter basis, where one may have 1 GbE and 10 GbE devices coexisting
> in a system, one running multiqueue and the other not, etc.
>
> This can also be desirable for devices that cannot support full 64 KB
> TSO's, but still want to benefit from some level of segmentation
> offloading.
>
> Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> ---
>
> include/linux/netdevice.h | 6 ++++++
> include/net/sock.h | 2 ++
> net/core/dev.c | 1 +
> net/core/sock.c | 6 ++++--
> net/ipv4/tcp_output.c | 4 ++--
> 5 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 047d432..ed1cc32 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -616,6 +616,7 @@ struct net_device
>
> /* Partially transmitted GSO packet. */
> struct sk_buff *gso_skb;
> + int max_gso_frame_size;
should use unsigned rather than int (yes the older code is sloppy).
Also what about IPV6?
--
Stephen Hemminger <stephen.hemminger@vyatta.com>
^ permalink raw reply
* [PATCH] [NET]: Add per-connection option to set max TSO frame size
From: PJ Waskiewicz @ 2008-02-12 22:01 UTC (permalink / raw)
To: davem; +Cc: netdev
This patch adds the ability for device drivers to control the size of the
TSO frames being sent to them, per TCP connection. By setting the
netdevice's max_gso_frame_size value, the socket layer will set the GSO
frame size based on that value. This will propogate into the TCP layer,
and send TSO's of that size to the hardware.
This can be desirable to help tune the bursty nature of TSO on a
per-adapter basis, where one may have 1 GbE and 10 GbE devices coexisting
in a system, one running multiqueue and the other not, etc.
This can also be desirable for devices that cannot support full 64 KB
TSO's, but still want to benefit from some level of segmentation
offloading.
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
include/linux/netdevice.h | 6 ++++++
include/net/sock.h | 2 ++
net/core/dev.c | 1 +
net/core/sock.c | 6 ++++--
net/ipv4/tcp_output.c | 4 ++--
5 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 047d432..ed1cc32 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -616,6 +616,7 @@ struct net_device
/* Partially transmitted GSO packet. */
struct sk_buff *gso_skb;
+ int max_gso_frame_size;
/* ingress path synchronizer */
spinlock_t ingress_lock;
@@ -1475,6 +1476,11 @@ static inline int netif_needs_gso(struct net_device *dev, struct sk_buff *skb)
unlikely(skb->ip_summed != CHECKSUM_PARTIAL));
}
+static inline void netif_set_max_gso_size(struct net_device *dev, int size)
+{
+ dev->max_gso_frame_size = size;
+}
+
/* On bonding slaves other than the currently active slave, suppress
* duplicates except for 802.3ad ETH_P_SLOW, alb non-mcast/bcast, and
* ARP on active-backup slaves with arp_validate enabled.
diff --git a/include/net/sock.h b/include/net/sock.h
index 8a7889b..1977c05 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -151,6 +151,7 @@ struct sock_common {
* @sk_no_check: %SO_NO_CHECK setting, wether or not checkup packets
* @sk_route_caps: route capabilities (e.g. %NETIF_F_TSO)
* @sk_gso_type: GSO type (e.g. %SKB_GSO_TCPV4)
+ * @sk_gso_max_size: Maximum GSO segment size to build
* @sk_lingertime: %SO_LINGER l_linger setting
* @sk_backlog: always used with the per-socket spinlock held
* @sk_callback_lock: used with the callbacks in the end of this struct
@@ -236,6 +237,7 @@ struct sock {
gfp_t sk_allocation;
int sk_route_caps;
int sk_gso_type;
+ int sk_gso_max_size;
int sk_rcvlowat;
unsigned long sk_flags;
unsigned long sk_lingertime;
diff --git a/net/core/dev.c b/net/core/dev.c
index 9549417..f635b29 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4022,6 +4022,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
}
dev->egress_subqueue_count = queue_count;
+ dev->max_gso_frame_size = 65536;
dev->get_stats = internal_stats;
netpoll_netdev_init(dev);
diff --git a/net/core/sock.c b/net/core/sock.c
index 433715f..a8b0ae5 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1076,10 +1076,12 @@ void sk_setup_caps(struct sock *sk, struct dst_entry *dst)
if (sk->sk_route_caps & NETIF_F_GSO)
sk->sk_route_caps |= NETIF_F_GSO_SOFTWARE;
if (sk_can_gso(sk)) {
- if (dst->header_len)
+ if (dst->header_len) {
sk->sk_route_caps &= ~NETIF_F_GSO_MASK;
- else
+ } else {
sk->sk_route_caps |= NETIF_F_SG | NETIF_F_HW_CSUM;
+ sk->sk_gso_max_size = dst->dev->max_gso_frame_size;
+ }
}
}
EXPORT_SYMBOL_GPL(sk_setup_caps);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index ed750f9..8cd128d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -998,7 +998,7 @@ unsigned int tcp_current_mss(struct sock *sk, int large_allowed)
xmit_size_goal = mss_now;
if (doing_tso) {
- xmit_size_goal = (65535 -
+ xmit_size_goal = ((sk->sk_gso_max_size - 1) -
inet_csk(sk)->icsk_af_ops->net_header_len -
inet_csk(sk)->icsk_ext_hdr_len -
tp->tcp_header_len);
@@ -1274,7 +1274,7 @@ static int tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb)
limit = min(send_win, cong_win);
/* If a full-sized TSO skb can be sent, do it. */
- if (limit >= 65536)
+ if (limit >= sk->sk_gso_max_size)
goto send_now;
if (sysctl_tcp_tso_win_divisor) {
^ permalink raw reply related
* [PATCH] [IPROUTE2] Update various classifiers' help output for expected CLASSID syntax
From: PJ Waskiewicz @ 2008-02-12 21:51 UTC (permalink / raw)
To: stephen.hemminger; +Cc: netdev
This updates the help output to specify that CLASSID should be hexidecimal.
This makes sure that a user entering "flowid 1:10" gets his flow put into
band 15 (0x10) and knows why.
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
doc/actions/actions-general | 3 +++
tc/f_basic.c | 1 +
tc/f_fw.c | 1 +
tc/f_route.c | 1 +
tc/f_rsvp.c | 1 +
tc/f_u32.c | 1 +
6 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/doc/actions/actions-general b/doc/actions/actions-general
index 6561eda..70f7cd6 100644
--- a/doc/actions/actions-general
+++ b/doc/actions/actions-general
@@ -88,6 +88,9 @@ tc filter add dev lo parent ffff: protocol ip prio 8 u32 \
match ip dst 127.0.0.8/32 flowid 1:12 \
action ipt -j mark --set-mark 2
+NOTE: flowid 1:12 is parsed flowid 0x1:0x12. Make sure if you want flowid
+decimal 12, then use flowid 1:c.
+
3) A feature i call pipe
The motivation is derived from Unix pipe mechanism but applied to packets.
Essentially take a matching packet and pass it through
diff --git a/tc/f_basic.c b/tc/f_basic.c
index 19a7edf..d6d7767 100644
--- a/tc/f_basic.c
+++ b/tc/f_basic.c
@@ -32,6 +32,7 @@ static void explain(void)
fprintf(stderr, "\n");
fprintf(stderr, "Where: SELECTOR := SAMPLE SAMPLE ...\n");
fprintf(stderr, " FILTERID := X:Y:Z\n");
+ fprintf(stderr, "\nNOTE: CLASSID is parsed as hexidecimal input.\n");
}
static int basic_parse_opt(struct filter_util *qu, char *handle,
diff --git a/tc/f_fw.c b/tc/f_fw.c
index 6d1490b..9f4ef6e 100644
--- a/tc/f_fw.c
+++ b/tc/f_fw.c
@@ -28,6 +28,7 @@ static void explain(void)
fprintf(stderr, "Usage: ... fw [ classid CLASSID ] [ police POLICE_SPEC ]\n");
fprintf(stderr, " POLICE_SPEC := ... look at TBF\n");
fprintf(stderr, " CLASSID := X:Y\n");
+ fprintf(stderr, "\nNOTE: CLASSID is parsed as hexidecimal input.\n");
}
#define usage() return(-1)
diff --git a/tc/f_route.c b/tc/f_route.c
index a41b9d5..3bb963c 100644
--- a/tc/f_route.c
+++ b/tc/f_route.c
@@ -31,6 +31,7 @@ static void explain(void)
fprintf(stderr, " [ flowid CLASSID ] [ police POLICE_SPEC ]\n");
fprintf(stderr, " POLICE_SPEC := ... look at TBF\n");
fprintf(stderr, " CLASSID := X:Y\n");
+ fprintf(stderr, "\nNOTE: CLASSID is parsed as hexidecimal input.\n");
}
#define usage() return(-1)
diff --git a/tc/f_rsvp.c b/tc/f_rsvp.c
index 13fcf97..9019ee2 100644
--- a/tc/f_rsvp.c
+++ b/tc/f_rsvp.c
@@ -34,6 +34,7 @@ static void explain(void)
fprintf(stderr, " u{8|16|32} NUMBER mask MASK at OFFSET}\n");
fprintf(stderr, " POLICE_SPEC := ... look at TBF\n");
fprintf(stderr, " FILTERID := X:Y\n");
+ fprintf(stderr, "\nNOTE: CLASSID is parsed as hexidecimal input.\n");
}
#define usage() return(-1)
diff --git a/tc/f_u32.c b/tc/f_u32.c
index 91f2838..d38c536 100644
--- a/tc/f_u32.c
+++ b/tc/f_u32.c
@@ -36,6 +36,7 @@ static void explain(void)
fprintf(stderr, "Where: SELECTOR := SAMPLE SAMPLE ...\n");
fprintf(stderr, " SAMPLE := { ip | ip6 | udp | tcp | icmp | u{32|16|8} | mark } SAMPLE_ARGS [divisor DIVISOR]\n");
fprintf(stderr, " FILTERID := X:Y:Z\n");
+ fprintf(stderr, "\nNOTE: CLASSID is parsed at hexidecimal input.\n");
}
#define usage() return(-1)
^ 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