* Re: Forward Shared Patch
From: Julian Anastasov @ 2011-09-25 8:39 UTC (permalink / raw)
To: Simon Horman; +Cc: lvs-devel, netdev
In-Reply-To: <20110925011347.GB31019@verge.net.au>
Hello,
On Sun, 25 Sep 2011, Simon Horman wrote:
> Hi Julian,
>
> I am wondering if you have any interest in/objections to trying
> to merge your forward shared patch which allows an IPVS director
> to act as a router for realservers using the direct routing or tunnelling
> forwarding mechanisms.
>
> http://www.ssi.bg/~ja/#lvsgw
Such feature is already added to 2.6.33:
/proc/sys/net/ipv4/conf/*/accept_local
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: bridge carrier issue
From: Marc Haber @ 2011-09-25 8:51 UTC (permalink / raw)
To: netdev
In-Reply-To: <c1e52584-2b98-4e95-bfb1-fe4cd64c1c47@tahiti.vyatta.com>
On Tue, Sep 20, 2011 at 04:00:18PM -0700, Stephen Hemminger wrote:
> Looked at dnsmasq and it needs some more work to handle IPv6 address
> correctly. It sleeps if it gets a tentative address.
>
> radvd probably has similar issues.
And bind. And libvirtd. And apache. And probably a truckload of other
applications.
The ONLY application that I am aware of and that handles this case
properly is ntpd.
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
^ permalink raw reply
* Re: [PATCH 1/2] bridge: leave carrier on for empty bridge
From: Marc Haber @ 2011-09-25 8:55 UTC (permalink / raw)
To: netdev
In-Reply-To: <4E618DCD.5070901@sfc.wide.ad.jp>
On Sat, Sep 03, 2011 at 11:15:41AM +0900, Ang Way Chuang wrote:
> A more ideal solution in this case is to add the option to enabling
> carrier on sysfs and modify libvirtd to turn on the carrier if IPv6 is
> enabled.
I would even do it manually from an init script if that saves me from
having an extra dummy interface on a bridge just to force it up.
> Since there is no other
> complain from other user with this setup, I guess nobody actually
> assigns IPv6 to libvirtd network device at this moment
I do. Daily. And get bitten every time I reboot. A _major_ nuisance.
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
^ permalink raw reply
* Re: [PATCH 1/2] bridge: leave carrier on for empty bridge
From: Marc Haber @ 2011-09-25 9:05 UTC (permalink / raw)
To: netdev
In-Reply-To: <4E632A2E.5040805@gmail.com>
On Sun, Sep 04, 2011 at 09:35:10AM +0200, Nicolas de Pesloüan wrote:
> - for those using bridge without any port, manually setting the IP
> will assert carrier on. (By the way, why don't they use a dummy
> device instead?)
Because there is no documentation or recommendation how to use things.
Being a networking guy, I have always treated a bridge as kind of a
switch, where it is natural to say "vlan 120, ip address a.b.c.d". I
just did the same with br0 on all my virtualization hosts, which used
to work until just recently.
Additionally, most distributions' init mechanisms are not very good in
handling dependencies between network interfaces, so writing stuff
like "configure br0, configure dummy0, add dummy0 to br0, wait for
things to settle, start daemons, start VMs" requires tweaking.
Is it recommended to actually have radvd and/or dhcpd listen on the
dummy interface wich is member of a bridge instead of the bridge itself?
> - for those using bridge with ports:
> -- Using any kind of autoconfig will work as expected. Carrier will
> only be asserted at the time first port get carrier.
This only holds when the host with the bridge is the network client,
and only if rdnssd is not used. If the host with the bridge is
supposed to run radvd and/or dhcpd, it is desired to have those
services running before the first port gets attached to the bridge
(which may be hours after the system boot in the desktop
virtualization case).
> -- Using static IP confifiguration, carrier will possibly be
> erroneously reported as on during the small time gap between IP
> address configuration and first port is added to the bridge. This
> time gap may be removed by simply configuring the IP after the first
> port is added. This is probably already true for most distribs. And
> anyway, this time gap is probably not a problem.
You're assuming that the first port gets added immediately after the
port was created. This is not the case, Virtualbox only creates the
vboxnet interface that gets added to the bridge when the VM is
started, which may be a manual operation done by the user at the time
she actually needs the VM.
> -- Carrier will also be erroneously reported as on after removing
> the last port, if the bridge still has an IP.
For my use cases, this is the desired behavior.
> (But we can arrange for this not to happen).
Please not.
> And in order to ensure user really understand why carrier is on of
> off, we can simply issue an INFO message for the non-natural case
> (bridge_has_no_port & bridga_has_at_least_one_ip).
I fail to understand why this is the non-natural case.
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
^ permalink raw reply
* Re: [PATCH 1/2] bridge: leave carrier on for empty bridge
From: Marc Haber @ 2011-09-25 9:16 UTC (permalink / raw)
To: netdev
In-Reply-To: <4E65C31F.1010809@gmail.com>
On Tue, Sep 06, 2011 at 08:52:15AM +0200, Nicolas de Pesloüan wrote:
> I really support the idea to keep the current behavior (assert
> carrier on br0 when at least one port have carrier) and to fix the
> applications to wait for the IPv6 address to be checked (DAD)
> instead of dying on bind() failure.
A really brilliant idea. Break things and then expect hundreds of apps
or millions of installations to adapt.
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
^ permalink raw reply
* cannot assign IPv6 address to dummy interface
From: Marc Haber @ 2011-09-25 10:13 UTC (permalink / raw)
To: netdev
Hi,
this is a locally compiled vanilla 3.0.4 kernel on Debian sid.
$ uname -a
Linux swivel 3.0.4-zgws1 #2 SMP PREEMPT Sat Sep 24 12:34:50 UTC 2011 x86_64 GNU/Linux
$ ip addr show dev dummy0
7: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master br0 state UNKNOWN
link/ether 06:b8:41:e5:aa:aa brd ff:ff:ff:ff:ff:ff
$ sudo ip addr add dev dummy0 192.168.18.254/24 brd +
$ sudo ip addr add dev dummy0 2a01:238:4071:321f::100/64
RTNETLINK answers: Permission denied
$ sudo ip addr add dev eth0 2a01:238:4071:321f::100/64
$ ip addr show dev eth0
2: eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast state DOWN qlen 1000
link/ether f0:de:f1:79:90:1c brd ff:ff:ff:ff:ff:ff
inet6 2a01:238:4071:321f::100/64 scope global tentative
valid_lft forever preferred_lft forever
$ ip addr show dev dummy0
7: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master br0 state UNKNOWN
link/ether 06:b8:41:e5:aa:aa brd ff:ff:ff:ff:ff:ff
inet 192.168.18.254/24 brd 192.168.18.255 scope global dummy0
$
Why is root not allowed to assign an IPv6 address to a dummy interface?
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
^ permalink raw reply
* [PATCH] ipv6: nullify ipv6_ac_list and ipv6_fl_list when creating new socket
From: Yan, Zheng @ 2011-09-25 12:21 UTC (permalink / raw)
To: netdev@vger.kernel.org, davem@davemloft.net
ipv6_ac_list and ipv6_fl_list from listening socket are inadvertently
shared with new socket created for connection.
Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>
---
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3c9fa61..79cc646 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1383,6 +1383,8 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
newtp->af_specific = &tcp_sock_ipv6_mapped_specific;
#endif
+ newnp->ipv6_ac_list = NULL;
+ newnp->ipv6_fl_list = NULL;
newnp->pktoptions = NULL;
newnp->opt = NULL;
newnp->mcast_oif = inet6_iif(skb);
@@ -1447,6 +1449,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
First: no IPv4 options.
*/
newinet->inet_opt = NULL;
+ newnp->ipv6_ac_list = NULL;
newnp->ipv6_fl_list = NULL;
/* Clone RX bits */
^ permalink raw reply related
* Re: [net-next 05/10] if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
From: Stephen Hemminger @ 2011-09-25 17:22 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Greg Rose, netdev, gospo
In-Reply-To: <1316855863-6091-6-git-send-email-jeffrey.t.kirsher@intel.com>
On Sat, 24 Sep 2011 02:17:38 -0700
Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> From: Greg Rose <gregory.v.rose@intel.com>
>
> Add configuration setting for drivers to turn spoof checking on or off
> for discrete VFs.
>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> include/linux/if_link.h | 7 +++++++
> include/linux/netdevice.h | 3 +++
> net/core/rtnetlink.c | 25 ++++++++++++++++++++++---
> 3 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> index 0ee969a..8bd6d6d 100644
> --- a/include/linux/if_link.h
> +++ b/include/linux/if_link.h
> @@ -279,6 +279,7 @@ enum {
> IFLA_VF_MAC, /* Hardware queue specific attributes */
> IFLA_VF_VLAN,
> IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */
> + IFLA_VF_SPOOFCHK, /* Spoof Checking on/off switch */
> __IFLA_VF_MAX,
> };
>
> @@ -300,12 +301,18 @@ struct ifla_vf_tx_rate {
> __u32 rate; /* Max TX bandwidth in Mbps, 0 disables throttling */
> };
>
> +struct ifla_vf_spoofchk {
> + __u32 vf;
> + __u32 setting;
> +};
> +
> struct ifla_vf_info {
> __u32 vf;
> __u8 mac[32];
> __u32 vlan;
> __u32 qos;
> __u32 tx_rate;
> + __u32 spoofchk;
> };
This breaks ABI compatibility, unless you add some special case code
to handle the case of tools sending the old ifla_vf_info. Users may have older version
of ip utilities that send smaller size structure.
^ permalink raw reply
* Re: [net-next] iproute2: Add new command to IP link to enable/disable VF spoof check
From: Stephen Hemminger @ 2011-09-25 17:23 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: Greg Rose, davem, netdev, gospo
In-Reply-To: <1316938998-29855-1-git-send-email-jeffrey.t.kirsher@intel.com>
On Sun, 25 Sep 2011 01:23:18 -0700
Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> From: Greg Rose <gregory.v.rose@intel.com>
>
> Add IP link command parsing for VF spoof checking enable/disable
>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
See my comments about ABI compatibility. Can't just grow size of structure.
Need to have new version of iproute work with older kernels.
^ permalink raw reply
* Re: [PATCH] tcp: ECN blackhole should not force quickack mode
From: Eric Dumazet @ 2011-09-25 19:24 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, netdev, Jerry Chu, Ilpo Järvinen, Jim Gettys,
Dave Taht
In-Reply-To: <1316825745.1865.24.camel@mojatatu>
Le vendredi 23 septembre 2011 à 20:55 -0400, Jamal Hadi Salim a écrit :
> So small confusion for me, Eric:
> Is theres some middlebox clearing the ECT but allowing the TCP header
> ECN flags to be set during SYN echange? If yes, why would not allowing
> quickack help?
As you may know, the SYN and SYNACK packets dont have ECT bits, as
mandated by RFC. So there is no way to detect a TOS blackhole in TCP
session init.
On following trace taken from my laptop connecting to a ssh server
(192.168.2.1), you can see that my laptop sends ECT messages, but never
receive ECT bits. Each sides believe the session is ECN enabled, as the
SYN and SYNACK messages have the appropriate ECN markers.
So we have somewhere a box that mask the TOS bits. This is a well known
effect on some networks, unfortunately.
The 'linux funny extension' is wrong in this case, since I have no
retransmits, no losses, yet my laptop is in quickack mode (it sends an
ACK for every incoming data packet), because you assumed receiving a
packet with no ECT bits means this packet was a retransmit.
21:14:44.665405 IP (tos 0x0, ttl 64, id 63689, offset 0, flags [DF], proto TCP (6), length 60)
192.168.2.32.39529 > 192.168.2.1.22: Flags [SEW], cksum 0xf2ea (incorrect -> 0x2151), seq 1589453915, win 14600, options [mss 1460,sackOK,TS val 56466 ecr 0,nop,wscale 7], length 0
21:14:44.773643 IP (tos 0x0, ttl 46, id 0, offset 0, flags [DF], proto TCP (6), length 60)
192.168.2.1.22 > 192.168.2.32.39529: Flags [S.E], cksum 0xbce1 (correct), seq 1422101564, ack 1589453916, win 14480, options [mss 1452,sackOK,TS val 176781781 ecr 56466,nop,wscale 7], length 0
21:14:44.773691 IP (tos 0x0, ttl 64, id 63690, offset 0, flags [DF], proto TCP (6), length 52)
192.168.2.32.39529 > 192.168.2.1.22: Flags [.], cksum 0xf2e2 (incorrect -> 0x23f8), ack 1, win 115, options [nop,nop,TS val 56477 ecr 176781781], length 0
21:14:44.888804 IP (tos 0x0, ttl 46, id 62941, offset 0, flags [DF], proto TCP (6), length 91)
192.168.2.1.22 > 192.168.2.32.39529: Flags [P.], cksum 0x64d1 (correct), seq 1:40, ack 1, win 114, options [nop,nop,TS val 176781792 ecr 56477], length 39
21:14:44.888842 IP (tos 0x0, ttl 64, id 63691, offset 0, flags [DF], proto TCP (6), length 52)
192.168.2.32.39529 > 192.168.2.1.22: Flags [.], cksum 0xf2e2 (incorrect -> 0x23ba), ack 40, win 115, options [nop,nop,TS val 56489 ecr 176781792], length 0
21:14:44.888929 IP (tos 0x2,ECT(0), ttl 64, id 63692, offset 0, flags [DF], proto TCP (6), length 91)
192.168.2.32.39529 > 192.168.2.1.22: Flags [P.], cksum 0x649d (correct), seq 1:40, ack 40, win 115, options [nop,nop,TS val 56489 ecr 176781792], length 39
21:14:44.993714 IP (tos 0x0, ttl 46, id 62942, offset 0, flags [DF], proto TCP (6), length 52)
192.168.2.1.22 > 192.168.2.32.39529: Flags [.], cksum 0x2389 (correct), ack 40, win 114, options [nop,nop,TS val 176781803 ecr 56489], length 0
21:14:44.993758 IP (tos 0x2,ECT(0), ttl 64, id 63693, offset 0, flags [DF], proto TCP (6), length 1196)
192.168.2.32.39529 > 192.168.2.1.22: Flags [P.], cksum 0x1e1b (correct), seq 40:1184, ack 40, win 115, options [nop,nop,TS val 56499 ecr 176781803], length 1144
21:14:44.994447 IP (tos 0x0, ttl 46, id 62943, offset 0, flags [DF], proto TCP (6), length 900)
192.168.2.1.22 > 192.168.2.32.39529: Flags [P.], cksum 0xd3d4 (correct), seq 40:888, ack 40, win 114, options [nop,nop,TS val 176781803 ecr 56489], length 848
21:14:44.994463 IP (tos 0x0, ttl 64, id 63694, offset 0, flags [DF], proto TCP (6), length 52)
192.168.2.32.39529 > 192.168.2.1.22: Flags [.], cksum 0xf2e2 (incorrect -> 0x1ba9), ack 888, win 128, options [nop,nop,TS val 56499 ecr 176781803], length 0
21:14:45.546332 IP (tos 0x0, ttl 64, id 63695, offset 0, flags [DF], proto TCP (6), length 1196)
192.168.2.32.39529 > 192.168.2.1.22: Flags [P.], cksum 0x1a8c (correct), seq 40:1184, ack 888, win 128, options [nop,nop,TS val 56549 ecr 176781803], length 1144
^ permalink raw reply
* Re: [net-next 05/10] if_link: Add additional parameter to IFLA_VF_INFO for spoof checking
From: Ben Hutchings @ 2011-09-25 20:06 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Jeff Kirsher, davem, Greg Rose, netdev, gospo
In-Reply-To: <20110925102238.55ba72eb@s6510.linuxnetplumber.net>
On Sun, 2011-09-25 at 10:22 -0700, Stephen Hemminger wrote:
> On Sat, 24 Sep 2011 02:17:38 -0700
> Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
>
> > From: Greg Rose <gregory.v.rose@intel.com>
> >
> > Add configuration setting for drivers to turn spoof checking on or off
> > for discrete VFs.
> >
> > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> > include/linux/if_link.h | 7 +++++++
> > include/linux/netdevice.h | 3 +++
> > net/core/rtnetlink.c | 25 ++++++++++++++++++++++---
> > 3 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> > index 0ee969a..8bd6d6d 100644
> > --- a/include/linux/if_link.h
> > +++ b/include/linux/if_link.h
> > @@ -279,6 +279,7 @@ enum {
> > IFLA_VF_MAC, /* Hardware queue specific attributes */
> > IFLA_VF_VLAN,
> > IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */
> > + IFLA_VF_SPOOFCHK, /* Spoof Checking on/off switch */
> > __IFLA_VF_MAX,
> > };
> >
> > @@ -300,12 +301,18 @@ struct ifla_vf_tx_rate {
> > __u32 rate; /* Max TX bandwidth in Mbps, 0 disables throttling */
> > };
> >
> > +struct ifla_vf_spoofchk {
> > + __u32 vf;
> > + __u32 setting;
> > +};
> > +
> > struct ifla_vf_info {
> > __u32 vf;
> > __u8 mac[32];
> > __u32 vlan;
> > __u32 qos;
> > __u32 tx_rate;
> > + __u32 spoofchk;
> > };
>
> This breaks ABI compatibility, unless you add some special case code
> to handle the case of tools sending the old ifla_vf_info. Users may have older version
> of ip utilities that send smaller size structure.
Unless I'm missing something, that structure is not sent or received by
userland; that's why I thought it should be #ifdef __KERNEL__.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 1/2] bridge: leave carrier on for empty bridge
From: Nicolas de Pesloüan @ 2011-09-25 20:10 UTC (permalink / raw)
To: Marc Haber; +Cc: netdev
In-Reply-To: <20110925091606.GE32712@torres.zugschlus.de>
Le 25/09/2011 11:16, Marc Haber a écrit :
> On Tue, Sep 06, 2011 at 08:52:15AM +0200, Nicolas de Pesloüan wrote:
>> I really support the idea to keep the current behavior (assert
>> carrier on br0 when at least one port have carrier) and to fix the
>> applications to wait for the IPv6 address to be checked (DAD)
>> instead of dying on bind() failure.
>
> A really brilliant idea. Break things and then expect hundreds of apps
> or millions of installations to adapt.
Well... Considering the noise you made recently about the new bridge behavior, I'm sure we all
understand this is a problem for you.
We try and find a reasonable solution between two somewhat incompatible expectations and this is not
easy. If you have had a real reading of my proposal, you would have noticed that I really tried to
find a solution that would be acceptable for both situations. Stephen, which is far more experienced
than me on this topic explained that my proposal was (and still is) not applicable.
Instead of annoying us with comments like "brilliant idea" or "nice idea", feel free to propose
something that would contribute to the solution.
Nicolas.
^ permalink raw reply
* Re: Forward Shared Patch
From: Simon Horman @ 2011-09-25 23:00 UTC (permalink / raw)
To: Julian Anastasov; +Cc: lvs-devel, netdev
In-Reply-To: <alpine.LFD.2.00.1109251129190.1474@ja.ssi.bg>
On Sun, Sep 25, 2011 at 11:39:21AM +0300, Julian Anastasov wrote:
>
> Hello,
>
> On Sun, 25 Sep 2011, Simon Horman wrote:
>
> > Hi Julian,
> >
> > I am wondering if you have any interest in/objections to trying
> > to merge your forward shared patch which allows an IPVS director
> > to act as a router for realservers using the direct routing or tunnelling
> > forwarding mechanisms.
> >
> > http://www.ssi.bg/~ja/#lvsgw
>
> Such feature is already added to 2.6.33:
>
> /proc/sys/net/ipv4/conf/*/accept_local
Oh, great. Sorry for the noise.
^ permalink raw reply
* Re: [PATCH] tcp: ECN blackhole should not force quickack mode
From: jamal @ 2011-09-26 1:07 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Jerry Chu, Ilpo Järvinen, Jim Gettys,
Dave Taht
In-Reply-To: <1316978684.2531.7.camel@edumazet-laptop>
On Sun, 2011-09-25 at 21:24 +0200, Eric Dumazet wrote:
>
> So we have somewhere a box that mask the TOS bits. This is a well known
> effect on some networks, unfortunately.
ok
> The 'linux funny extension' is wrong in this case, since I have no
> retransmits, no losses, yet my laptop is in quickack mode (it sends an
> ACK for every incoming data packet), because you assumed receiving a
> packet with no ECT bits means this packet was a retransmit.
No objection from me - only one corner case i can think of is if
expected behavior happens and the first data packet was a retransmit
(i.e you wouldnt have seen an ECT which you need for your heuristic),
but that is probably not a big a deal.
In regards to the 'linux funny extension' - this was a brilliant idea
in my opinion back then from Alexey; lots of discussions happened but
I cant remember if it made it in some RFC or not (I will try to search
some old archives).
cheers,
jamal
^ permalink raw reply
* [PATCH] staging/octeon: Software should check the checksum of no tcp/udp packets
From: rongqing.li @ 2011-09-26 1:08 UTC (permalink / raw)
To: linux-mips, netdev; +Cc: ralf, david.daney
From: Roy.Li <rongqing.li@windriver.com>
Icmp packets with wrong checksum are never dropped since
skb->ip_summed is set to CHECKSUM_UNNECESSARY.
When icmp packets with wrong checksum pass through the octeon
net driver, the not_IP, IP_exc, L4_error hardware indicators
show no error. so the driver sets CHECKSUM_UNNECESSARY on
skb->ip_summed.
L4_error only works for TCP/UDP, not for ICMP.
Signed-off-by: Roy.Li <rongqing.li@windriver.com>
---
drivers/staging/octeon/ethernet-rx.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/octeon/ethernet-rx.c b/drivers/staging/octeon/ethernet-rx.c
index 1a7c19a..1747024 100644
--- a/drivers/staging/octeon/ethernet-rx.c
+++ b/drivers/staging/octeon/ethernet-rx.c
@@ -411,7 +411,8 @@ static int cvm_oct_napi_poll(struct napi_struct *napi, int budget)
skb->protocol = eth_type_trans(skb, dev);
skb->dev = dev;
- if (unlikely(work->word2.s.not_IP || work->word2.s.IP_exc || work->word2.s.L4_error))
+ if (unlikely(work->word2.s.not_IP || work->word2.s.IP_exc ||
+ work->word2.s.L4_error || !work->word2.s.tcp_or_udp))
skb->ip_summed = CHECKSUM_NONE;
else
skb->ip_summed = CHECKSUM_UNNECESSARY;
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] tcp: ECN blackhole should not force quickack mode
From: jamal @ 2011-09-26 1:13 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Jerry Chu, Ilpo Järvinen, Jim Gettys,
Dave Taht
In-Reply-To: <1316999266.1865.35.camel@mojatatu>
On Sun, 2011-09-25 at 21:07 -0400, jamal wrote:
> In regards to the 'linux funny extension' - this was a brilliant idea
> in my opinion back then from Alexey; lots of discussions happened but
> I cant remember if it made it in some RFC or not (I will try to search
> some old archives).
Wasnt hard:
https://tools.ietf.org/id/draft-ietf-tsvwg-tcp-ecn-00.txt
It doesnt seem to have made it to RFC and i cant remember why.
[We dont wanna bring Sally out of retirement but we can ask KK if
it bugs you;->]
cheers,
jamal
^ permalink raw reply
* [PATCH 0/4] IPVS: sysctl Documentation and Various Clean-Ups
From: Simon Horman @ 2011-09-26 1:23 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy
Hi,
patch one of this series adds some missing entries to the sysctl
documentation. Julian, I would appreciate a review of this change.
The remaining three patches are minor code clean-ups flagged
by gcc 4.6.1.
My intention for this post is to solicit review.
I intend to post a pull request separately at a later date.
^ permalink raw reply
* [PATCH 1/4] IPVS: Add documentation for new sysctl entries
From: Simon Horman @ 2011-09-26 1:23 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy, Simon Horman
In-Reply-To: <1317000233-11169-1-git-send-email-horms@verge.net.au>
Add missing documentation for conntrack, snat_reroute and sync_version.
Also fix up a typo, IPVS_DEBUG should be IP_VS_DEBUG.
Signed-off-by: Simon Horman <horms@verge.net.au>
conntrack
---
Documentation/networking/ipvs-sysctl.txt | 53 +++++++++++++++++++++++++++++-
1 files changed, 52 insertions(+), 1 deletions(-)
diff --git a/Documentation/networking/ipvs-sysctl.txt b/Documentation/networking/ipvs-sysctl.txt
index 4ccdbca..5214339 100644
--- a/Documentation/networking/ipvs-sysctl.txt
+++ b/Documentation/networking/ipvs-sysctl.txt
@@ -15,6 +15,23 @@ amemthresh - INTEGER
enabled and the variable is automatically set to 2, otherwise
the strategy is disabled and the variable is set to 1.
+conntrack - BOOLEAN
+ 0 - disabled (default)
+ not 0 - enabled
+
+ If set, maintain connection tracking entries for
+ connections handled by IPVS.
+
+ This should be enabled if connections handled by IPVS are to be
+ also handled by stateful firewall rules. That is, iptables rules
+ that make use of connection tracking. It is a performance
+ optimisation to disable this setting otherwise.
+
+ Connections handled by the IPVS FTP application module
+ will have connection tracking entries regardless of this setting.
+
+ Only available when IPVS is compiled with the CONFIG_IP_VS_NFCT
+
cache_bypass - BOOLEAN
0 - disabled (default)
not 0 - enabled
@@ -39,7 +56,7 @@ debug_level - INTEGER
11 - IPVS packet handling (ip_vs_in/ip_vs_out)
12 or more - packet traversal
- Only available when IPVS is compiled with the CONFIG_IPVS_DEBUG
+ Only available when IPVS is compiled with the CONFIG_IP_VS_DEBUG
Higher debugging levels include the messages for lower debugging
levels, so setting debug level 2, includes level 0, 1 and 2
@@ -141,3 +158,37 @@ sync_threshold - INTEGER
synchronized, every time the number of its incoming packets
modulus 50 equals the threshold. The range of the threshold is
from 0 to 49.
+
+snat_reroute - BOOLEAN
+ 0 - disabled
+ not 0 - enabled (default)
+
+ If enabled, recalculate the route of SNATed packets from
+ realservers so that they are routed as if they originate from the
+ director. Otherwise they are routed as if they are forwarded by the
+ director.
+
+ If policy routing is in effect then it is possible that the route
+ of a packet originating from a director is routed differently to a
+ packet being forwarded by the director.
+
+ If policy routing is not in effect then the recalculated route will
+ always be the same as the original route so it is an optimisation
+ to disable snat_reroute and avoid the recalculation.
+
+sync_version - INTEGER
+ default 1
+
+ The version of the synchronisation protocol used when sending
+ synchronisation messages.
+
+ 0 selects the original synchronisation protocol (version 0). This
+ shuold be used Use this when sending synchronisation messages to a
+ legacy system that only understands the original synchronisation
+ protocol.
+
+ 1 selects the current synchronisation protocol (version 1). This
+ should be used where possible.
+
+ Kernels with this sync_version entry are able to receive messages
+ of both version 1 and version 2 of the synchronisation protocol.
--
1.7.5.4
^ permalink raw reply related
* [PATCH 2/4] IPVS: Remove unused parameter from ip_vs_confirm_conntrack()
From: Simon Horman @ 2011-09-26 1:23 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy, Simon Horman
In-Reply-To: <1317000233-11169-1-git-send-email-horms@verge.net.au>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/net/ip_vs.h | 5 ++---
net/netfilter/ipvs/ip_vs_nfct.c | 2 +-
net/netfilter/ipvs/ip_vs_xmit.c | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 30b49ed..5207248 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1371,7 +1371,7 @@ static inline int ip_vs_conntrack_enabled(struct netns_ipvs *ipvs)
extern void ip_vs_update_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp,
int outin);
-extern int ip_vs_confirm_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp);
+extern int ip_vs_confirm_conntrack(struct sk_buff *skb);
extern void ip_vs_nfct_expect_related(struct sk_buff *skb, struct nf_conn *ct,
struct ip_vs_conn *cp, u_int8_t proto,
const __be16 port, int from_rs);
@@ -1389,8 +1389,7 @@ static inline void ip_vs_update_conntrack(struct sk_buff *skb,
{
}
-static inline int ip_vs_confirm_conntrack(struct sk_buff *skb,
- struct ip_vs_conn *cp)
+static inline int ip_vs_confirm_conntrack(struct sk_buff *skb);
{
return NF_ACCEPT;
}
diff --git a/net/netfilter/ipvs/ip_vs_nfct.c b/net/netfilter/ipvs/ip_vs_nfct.c
index f454c80..022e77e 100644
--- a/net/netfilter/ipvs/ip_vs_nfct.c
+++ b/net/netfilter/ipvs/ip_vs_nfct.c
@@ -127,7 +127,7 @@ ip_vs_update_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp, int outin)
nf_conntrack_alter_reply(ct, &new_tuple);
}
-int ip_vs_confirm_conntrack(struct sk_buff *skb, struct ip_vs_conn *cp)
+int ip_vs_confirm_conntrack(struct sk_buff *skb)
{
return nf_conntrack_confirm(skb);
}
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 6132b21..2820eb7 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -323,7 +323,7 @@ ip_vs_dst_reset(struct ip_vs_dest *dest)
\
(skb)->ipvs_property = 1; \
if (unlikely((cp)->flags & IP_VS_CONN_F_NFCT)) \
- __ret = ip_vs_confirm_conntrack(skb, cp); \
+ __ret = ip_vs_confirm_conntrack(skb); \
if (__ret == NF_ACCEPT) { \
nf_reset(skb); \
skb_forward_csum(skb); \
--
1.7.5.4
^ permalink raw reply related
* [PATCH 3/4] IPVS: Remove unused return value of protocol state transitions
From: Simon Horman @ 2011-09-26 1:23 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy, Simon Horman
In-Reply-To: <1317000233-11169-1-git-send-email-horms@verge.net.au>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
include/net/ip_vs.h | 6 +++---
net/netfilter/ipvs/ip_vs_core.c | 13 ++++++-------
net/netfilter/ipvs/ip_vs_proto_sctp.c | 14 ++++----------
net/netfilter/ipvs/ip_vs_proto_tcp.c | 6 ++----
net/netfilter/ipvs/ip_vs_proto_udp.c | 5 ++---
5 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 5207248..ec70a0d 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -428,9 +428,9 @@ struct ip_vs_protocol {
const char *(*state_name)(int state);
- int (*state_transition)(struct ip_vs_conn *cp, int direction,
- const struct sk_buff *skb,
- struct ip_vs_proto_data *pd);
+ void (*state_transition)(struct ip_vs_conn *cp, int direction,
+ const struct sk_buff *skb,
+ struct ip_vs_proto_data *pd);
int (*register_app)(struct net *net, struct ip_vs_app *inc);
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 0e4422b..c1ec85d 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -188,14 +188,13 @@ ip_vs_conn_stats(struct ip_vs_conn *cp, struct ip_vs_service *svc)
}
-static inline int
+static inline void
ip_vs_set_state(struct ip_vs_conn *cp, int direction,
const struct sk_buff *skb,
struct ip_vs_proto_data *pd)
{
- if (unlikely(!pd->pp->state_transition))
- return 0;
- return pd->pp->state_transition(cp, direction, skb, pd);
+ if (likely(pd->pp->state_transition))
+ pd->pp->state_transition(cp, direction, skb, pd);
}
static inline int
@@ -557,7 +556,7 @@ int ip_vs_leave(struct ip_vs_service *svc, struct sk_buff *skb,
ip_vs_in_stats(cp, skb);
/* set state */
- cs = ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd);
+ ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd);
/* transmit the first SYN packet */
ret = cp->packet_xmit(skb, cp, pd->pp);
@@ -1507,7 +1506,7 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
struct ip_vs_protocol *pp;
struct ip_vs_proto_data *pd;
struct ip_vs_conn *cp;
- int ret, restart, pkts;
+ int ret, pkts;
struct netns_ipvs *ipvs;
/* Already marked as IPVS request or reply? */
@@ -1605,7 +1604,7 @@ ip_vs_in(unsigned int hooknum, struct sk_buff *skb, int af)
}
ip_vs_in_stats(cp, skb);
- restart = ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd);
+ ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd);
if (cp->packet_xmit)
ret = cp->packet_xmit(skb, cp, pp);
/* do not touch skb anymore */
diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index b027ccc..df9f13b 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -906,7 +906,7 @@ static const char *sctp_state_name(int state)
return "?";
}
-static inline int
+static inline void
set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
int direction, const struct sk_buff *skb)
{
@@ -924,7 +924,7 @@ set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
sch = skb_header_pointer(skb, ihl + sizeof(sctp_sctphdr_t),
sizeof(_sctpch), &_sctpch);
if (sch == NULL)
- return 0;
+ return;
chunk_type = sch->type;
/*
@@ -993,21 +993,15 @@ set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
cp->timeout = pd->timeout_table[cp->state = next_state];
else /* What to do ? */
cp->timeout = sctp_timeouts[cp->state = next_state];
-
- return 1;
}
-static int
+static void
sctp_state_transition(struct ip_vs_conn *cp, int direction,
const struct sk_buff *skb, struct ip_vs_proto_data *pd)
{
- int ret = 0;
-
spin_lock(&cp->lock);
- ret = set_sctp_state(pd, cp, direction, skb);
+ set_sctp_state(pd, cp, direction, skb);
spin_unlock(&cp->lock);
-
- return ret;
}
static inline __u16 sctp_app_hashkey(__be16 port)
diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
index c0cc341..ef8641f 100644
--- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
@@ -546,7 +546,7 @@ set_tcp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
/*
* Handle state transitions
*/
-static int
+static void
tcp_state_transition(struct ip_vs_conn *cp, int direction,
const struct sk_buff *skb,
struct ip_vs_proto_data *pd)
@@ -561,13 +561,11 @@ tcp_state_transition(struct ip_vs_conn *cp, int direction,
th = skb_header_pointer(skb, ihl, sizeof(_tcph), &_tcph);
if (th == NULL)
- return 0;
+ return;
spin_lock(&cp->lock);
set_tcp_state(pd, cp, direction, th);
spin_unlock(&cp->lock);
-
- return 1;
}
static inline __u16 tcp_app_hashkey(__be16 port)
diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c
index f1282cb..f4b7262 100644
--- a/net/netfilter/ipvs/ip_vs_proto_udp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
@@ -454,18 +454,17 @@ static const char * udp_state_name(int state)
return udp_state_name_table[state] ? udp_state_name_table[state] : "?";
}
-static int
+static void
udp_state_transition(struct ip_vs_conn *cp, int direction,
const struct sk_buff *skb,
struct ip_vs_proto_data *pd)
{
if (unlikely(!pd)) {
pr_err("UDP no ns data\n");
- return 0;
+ return;
}
cp->timeout = pd->timeout_table[IP_VS_UDP_S_NORMAL];
- return 1;
}
static void __udp_init(struct net *net, struct ip_vs_proto_data *pd)
--
1.7.5.4
^ permalink raw reply related
* [PATCH 4/4] IPVS: Removed unused variables
From: Simon Horman @ 2011-09-26 1:23 UTC (permalink / raw)
To: lvs-devel, netdev, netfilter-devel, netfilter
Cc: Wensong Zhang, Julian Anastasov, Patrick McHardy, Simon Horman
In-Reply-To: <1317000233-11169-1-git-send-email-horms@verge.net.au>
ipvs is not used in ip_vs_genl_set_cmd() or ip_vs_genl_get_cmd()
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_ctl.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 36f4495..f10a903 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3194,10 +3194,8 @@ static int ip_vs_genl_set_cmd(struct sk_buff *skb, struct genl_info *info)
int ret = 0, cmd;
int need_full_svc = 0, need_full_dest = 0;
struct net *net;
- struct netns_ipvs *ipvs;
net = skb_sknet(skb);
- ipvs = net_ipvs(net);
cmd = info->genlhdr->cmd;
mutex_lock(&__ip_vs_mutex);
@@ -3307,10 +3305,8 @@ static int ip_vs_genl_get_cmd(struct sk_buff *skb, struct genl_info *info)
void *reply;
int ret, cmd, reply_cmd;
struct net *net;
- struct netns_ipvs *ipvs;
net = skb_sknet(skb);
- ipvs = net_ipvs(net);
cmd = info->genlhdr->cmd;
if (cmd == IPVS_CMD_GET_SERVICE)
--
1.7.5.4
^ permalink raw reply related
* [PATCH] tcp: properly release md5sig_pool references
From: Yan, Zheng @ 2011-09-26 2:35 UTC (permalink / raw)
To: netdev@vger.kernel.org, davem@davemloft.net, yoshfuji
tcp_v4_md5_do_add() increases use count of md5sig_pool by one for
each tcp md5sig peer, so we should call tcp_free_md5sig_pool() for
each peer when destroying the socket.
Signed-off-by: Zheng Yan <zheng.z.yan@intel.com>
---
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c34f015..3ca72ae 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1007,10 +1007,11 @@ static void tcp_v4_clear_md5_list(struct sock *sk)
*/
if (tp->md5sig_info->entries4) {
int i;
- for (i = 0; i < tp->md5sig_info->entries4; i++)
+ for (i = 0; i < tp->md5sig_info->entries4; i++) {
kfree(tp->md5sig_info->keys4[i].base.key);
+ tcp_free_md5sig_pool();
+ }
tp->md5sig_info->entries4 = 0;
- tcp_free_md5sig_pool();
}
if (tp->md5sig_info->keys4) {
kfree(tp->md5sig_info->keys4);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3c9fa61..9dcef7c 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -668,10 +668,11 @@ static void tcp_v6_clear_md5_list (struct sock *sk)
int i;
if (tp->md5sig_info->entries6) {
- for (i = 0; i < tp->md5sig_info->entries6; i++)
+ for (i = 0; i < tp->md5sig_info->entries6; i++) {
kfree(tp->md5sig_info->keys6[i].base.key);
+ tcp_free_md5sig_pool();
+ }
tp->md5sig_info->entries6 = 0;
- tcp_free_md5sig_pool();
}
kfree(tp->md5sig_info->keys6);
@@ -679,10 +680,11 @@ static void tcp_v6_clear_md5_list (struct sock *sk)
tp->md5sig_info->alloced6 = 0;
if (tp->md5sig_info->entries4) {
- for (i = 0; i < tp->md5sig_info->entries4; i++)
+ for (i = 0; i < tp->md5sig_info->entries4; i++) {
kfree(tp->md5sig_info->keys4[i].base.key);
+ tcp_free_md5sig_pool();
+ }
tp->md5sig_info->entries4 = 0;
- tcp_free_md5sig_pool();
}
kfree(tp->md5sig_info->keys4);
^ permalink raw reply related
* Re: [PATCH 1/2] net: Disable false positive memory leak report
From: Eric Dumazet @ 2011-09-26 5:29 UTC (permalink / raw)
To: Huajun Li; +Cc: David Miller, netdev
In-Reply-To: <CA+v9cxao_66a1Jb2L4M3fmUvNv8+QzWTFMqu7Dxnh7a5Zyqt0g@mail.gmail.com>
Le samedi 24 septembre 2011 à 23:57 +0800, Huajun Li a écrit :
> Memory leak detector reports following false positive memory leak, the
> patch disables it.
>
> unreferenced object 0xffff880073a70000 (size 8192):
> comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffff8124db64>] create_object+0x144/0x360
> [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
> [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
> [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
> [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
> [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
> [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
> [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
> [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffff880073a74290 (size 8192):
> comm "swapper", pid 1, jiffies 4294937832 (age 445.740s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace:
> [<ffffffff8124db64>] create_object+0x144/0x360
> [<ffffffff8191192e>] kmemleak_alloc+0x7e/0x110
> [<ffffffff81235b26>] __kmalloc_node+0x156/0x3a0
> [<ffffffff81935512>] flow_cache_cpu_prepare.clone.1+0x58/0xc0
> [<ffffffff8214c361>] flow_cache_init_global+0xb6/0x1af
> [<ffffffff8100225d>] do_one_initcall+0x4d/0x260
> [<ffffffff820ec2e9>] kernel_init+0x161/0x23a
> [<ffffffff8194ab04>] kernel_thread_helper+0x4/0x10
> [<ffffffffffffffff>] 0xffffffffffffffff
>
>
> Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
> ---
> net/core/flow.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/net/core/flow.c b/net/core/flow.c
> index 555a456..ba3e617 100644
> --- a/net/core/flow.c
> +++ b/net/core/flow.c
> @@ -365,6 +365,13 @@ static int __cpuinit
> flow_cache_cpu_prepare(struct flow_cache *fc, int cpu)
>
> if (!fcp->hash_table) {
> fcp->hash_table = kzalloc_node(sz, GFP_KERNEL, cpu_to_node(cpu));
> + /*
> + * Avoid a kmemleak false positive. The pointer to this block
> + * is refferenced by per-CPU varaible, here just mark it as not
> + * being a leak.
> + */
> + kmemleak_not_leak(fcp->hash_table);
> +
> if (!fcp->hash_table) {
> pr_err("NET: failed to allocate flow cache sz %zu\n", sz);
> return -ENOMEM;
This makes no sense to me.
per-cpu variables are taken into account by kmemleak.
If not, you should report this problem to kmemleak maintainer.
^ permalink raw reply
* Re: [PATCH 0/4] IPVS: sysctl Documentation and Various Clean-Ups
From: Julian Anastasov @ 2011-09-26 5:39 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, netfilter, Wensong Zhang,
Patrick McHardy
In-Reply-To: <1317000233-11169-1-git-send-email-horms@verge.net.au>
Hello,
On Mon, 26 Sep 2011, Simon Horman wrote:
> Hi,
>
> patch one of this series adds some missing entries to the sysctl
> documentation. Julian, I would appreciate a review of this change.
Acked-by: Julian Anastasov <ja@ssi.bg>
for all changes, with one minor problem in patch 1
for sync_version: 'sh[uo]ld be used [Use this ]when'
> The remaining three patches are minor code clean-ups flagged
> by gcc 4.6.1.
>
> My intention for this post is to solicit review.
> I intend to post a pull request separately at a later date.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH 2/2] net: Fix potential memory leak
From: Eric Dumazet @ 2011-09-26 6:09 UTC (permalink / raw)
To: Huajun Li; +Cc: David Miller, netdev
In-Reply-To: <CA+v9cxYuFArVUh9gc-be-Lmy4TOTxp72VaCC4QeCEE3p1j2Ykg@mail.gmail.com>
Le samedi 24 septembre 2011 à 23:57 +0800, Huajun Li a écrit :
> While preparing flow caches, once fail may cause potential memory leak , fix it.
>
> Signed-off-by: Huajun Li <huajun.li.lee@gmail.com>
> ---
> net/core/flow.c | 19 ++++++++++++++++++-
> 1 files changed, 18 insertions(+), 1 deletions(-)
>
> diff --git a/net/core/flow.c b/net/core/flow.c
> index ba3e617..2dcaa03 100644
> --- a/net/core/flow.c
> +++ b/net/core/flow.c
> @@ -420,7 +420,7 @@ static int __init flow_cache_init(struct flow_cache *fc)
>
> for_each_online_cpu(i) {
> if (flow_cache_cpu_prepare(fc, i))
> - return -ENOMEM;
> + goto err;
> }
> fc->hotcpu_notifier = (struct notifier_block){
> .notifier_call = flow_cache_cpu,
> @@ -433,6 +433,23 @@ static int __init flow_cache_init(struct flow_cache *fc)
> add_timer(&fc->rnd_timer);
>
> return 0;
> +err:
> + if (fc->percpu) {
> + free_percpu(fc->percpu);
> + fc->percpu = NULL;
> + }
> +
> + /*
> + * Check each possible CPUs rather than online ones because they may be
> + * offline before the notifier is registered.
> + */
Please remove this comment.
> + for_each_possible_cpu(i) {
> + struct flow_cache_percpu *fcp = per_cpu_ptr(fc->percpu, i);
> + kfree(fcp->hash_table);
> + fcp->hash_table = NULL;
> + }
You access fc->percpu after freeing it...
> +
> + return -ENOMEM;
> }
>
> static int __init flow_cache_init_global(void)
Previous to 2.6.37 (commit 83b6b1f5d134), a memory allocation at this
stage was panicing the box, so no worry about mem leak :)
Now I wonder if a proper patch would not print a nice message in
flow_cache_init_global() if flow_cache_init() returns an error, instead
of silently panicing or something worse...
Before submitting a new patch, could you test this case (injecting a
memalloc error in flow_cache_cpu_prepare() for example.
^ 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