* Re: [PATCH] udp: port starting location not random
From: Stephen Hemminger @ 2012-10-04 21:28 UTC (permalink / raw)
To: David Miller; +Cc: dada1, netdev
In-Reply-To: <20121004.171246.1480276635491836767.davem@davemloft.net>
On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Thu, 4 Oct 2012 14:08:28 -0700
>
> > While working on VXLAN, noticed a bug in UDP introduced by:
> >
> > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> > Author: Eric Dumazet <dada1@cosmosbay.com>
> > Date: Wed Oct 8 11:44:17 2008 -0700
> >
> > udp: Improve port randomization
> >
> >
> > The logic for choosing where to start for port randomization incorrectly
> > calculates the starting port number. It is always ends up using
> > the low end of the range independent of the value of random.
> > This causes all UDP port searches to start at the same port.
> >
> > Doing the following fixes it but at the cost of doing a real divide.
> >
> > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >
> > ---
> > Resend, previous send was not going to netdev.
> >
> > Not sure if worth fixing for stable, because only has performance impact
> > and some application might be depending on current broken behaviour.
> >
> >
> >
> > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
> > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
> > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> > remaining = (high - low) + 1;
> >
> > rand = net_random();
> > - first = (((u64)rand * remaining) >> 32) + low;
> > + first = rand % remaining + low;
>
> Try replacing "remaining" with "(remaining << (64 - 16))" in
> the expression instead.
The standalone program gets same result.
^ permalink raw reply
* [GIT] Networking
From: David Miller @ 2012-10-04 21:42 UTC (permalink / raw)
To: torvalds; +Cc: akpm, netdev, linux-kernel
The most important bit in here is the fix for input route caching
from Eric Dumazet, it's a shame we couldn't fully analyze this
in time for 3.6 as it's a 3.6 regression introduced by the routing
cache removal.
Anyways, will send quickly to -stable after you pull this in.
Other changes of note:
1) Fix lockdep splats in team and bonding, from Eric Dumazet.
2) IPV6 adds link local route even when there is no link local address,
from Nicolas Dichtel.
3) Fix ixgbe PTP implementation, from Jacob Keller.
4) Fix excessive stack usage in cxgb4 driver, from Vipul Pandya.
5) MAC length computed improperly in VLAN demux, from Antonio
Quartulli.
Please pull, thanks a lot!
The following changes since commit 7fe0b14b725d6d09a1d9e1409bd465cb88b587f9:
Merge tag 'spi-3.7' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/misc (2012-10-02 17:26:42 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master
for you to fetch changes up to 6825a26c2dc21eb4f8df9c06d3786ddec97cf53b:
ipv6: release reference of ip6_null_entry's dst entry in __ip6_del_rt (2012-10-04 16:00:07 -0400)
----------------------------------------------------------------
Antonio Quartulli (1):
8021q: fix mac_len recomputation in vlan_untag()
Dan Carpenter (1):
bnx2x: use strlcpy() to copy a string
Dave Jones (2):
silence some noisy printks in irda
Remove noisy printks from llcp_sock_connect
David S. Miller (4):
pch_gbe: Fix PTP dependencies.
cxgb4: Fix build error due to missing linux/vmalloc.h include.
Merge branch 'master' of git://git.kernel.org/.../jkirsher/net
Merge branch 'fixes-for-3.7' of git://gitorious.org/linux-can/linux-can
Emil Tantilov (1):
ixgbe: fix poll loop for FDIRCTRL.INIT_DONE bit
Eric Dumazet (3):
ipv4: add a fib_type to fib_info
bonding: set qdisc_tx_busylock to avoid LOCKDEP splat
team: set qdisc_tx_busylock to avoid LOCKDEP splat
Erik Hugne (1):
tipc: prevent dropped connections due to rcvbuf overflow
Gao feng (1):
ipv6: release reference of ip6_null_entry's dst entry in __ip6_del_rt
Jacob Keller (3):
ixgbe: Fix PTP X540 SDP alignment code for PPS signal
ixgbe: (PTP) Fix PPS interrupt code
ixgbe: fix PTP ethtool timestamping function
Marc Kleine-Budde (1):
can: mpc5xxx_can: fix section type conflict
Michael Chan (1):
tg3: Fix sparse warnings.
Nicolas Dichtel (3):
ipv6: don't add link local route when there is no link local address
sctp: fix a typo in prototype of __sctp_rcv_lookup()
sctp: check src addr when processing SACK to update transport state
Peter Senna Tschudin (2):
can: peak_pci: fix error return code
can: peak_pcmcia: fix error return code
Vipul Pandya (1):
cxgb4: Dynamically allocate memory in t4_memory_rw() and get_vpd_params()
Yuval Mintz (1):
bnx2x: fix ring size for 10G functions
htbegin (1):
net: ethernet: davinci_cpdma: decrease the desc count when cleaning up the remaining packets
joshua.a.hay@intel.com (1):
ixgbe: add support for X540-AT1
drivers/net/bonding/bond_main.c | 2 +
drivers/net/can/mscan/mpc5xxx_can.c | 8 +--
drivers/net/can/sja1000/peak_pci.c | 2 +
drivers/net/can/sja1000/peak_pcmcia.c | 4 +-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 17 ++++---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 5 +-
drivers/net/ethernet/broadcom/tg3.c | 11 +++--
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 1 +
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 43 +++++++++++-----
drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 5 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 +++++-
drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 202 ++++++++++++++++++++++++++++++++++++----------------------------------------
drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 1 +
drivers/net/ethernet/oki-semi/pch_gbe/Kconfig | 2 +-
drivers/net/ethernet/ti/davinci_cpdma.c | 1 +
drivers/net/team/team.c | 2 +
include/net/ip_fib.h | 1 +
include/net/sctp/structs.h | 2 +-
net/8021q/vlan_core.c | 3 +-
net/ipv4/fib_semantics.c | 2 +
net/ipv6/addrconf.c | 15 +-----
net/ipv6/route.c | 11 +++--
net/irda/af_irda.c | 2 +-
net/irda/irttp.c | 2 +-
net/nfc/llcp/sock.c | 8 +--
net/sctp/input.c | 2 +-
net/sctp/outqueue.c | 15 ++++--
net/sctp/sm_sideeffect.c | 4 +-
net/sctp/sm_statefuns.c | 2 +-
net/tipc/socket.c | 1 +
32 files changed, 209 insertions(+), 185 deletions(-)
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: Eric Dumazet @ 2012-10-04 21:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, dada1, netdev
In-Reply-To: <20121004142859.08ab0385@nehalam.linuxnetplumber.net>
On Thu, 2012-10-04 at 14:28 -0700, Stephen Hemminger wrote:
> On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
> David Miller <davem@davemloft.net> wrote:
>
> > From: Stephen Hemminger <shemminger@vyatta.com>
> > Date: Thu, 4 Oct 2012 14:08:28 -0700
> >
> > > While working on VXLAN, noticed a bug in UDP introduced by:
> > >
> > > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> > > Author: Eric Dumazet <dada1@cosmosbay.com>
> > > Date: Wed Oct 8 11:44:17 2008 -0700
> > >
> > > udp: Improve port randomization
> > >
> > >
> > > The logic for choosing where to start for port randomization incorrectly
> > > calculates the starting port number. It is always ends up using
> > > the low end of the range independent of the value of random.
> > > This causes all UDP port searches to start at the same port.
> > >
> > > Doing the following fixes it but at the cost of doing a real divide.
> > >
> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> > >
> > > ---
> > > Resend, previous send was not going to netdev.
> > >
> > > Not sure if worth fixing for stable, because only has performance impact
> > > and some application might be depending on current broken behaviour.
> > >
> > >
> > >
> > > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
> > > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
> > > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> > > remaining = (high - low) + 1;
> > >
> > > rand = net_random();
> > > - first = (((u64)rand * remaining) >> 32) + low;
> > > + first = rand % remaining + low;
> >
> > Try replacing "remaining" with "(remaining << (64 - 16))" in
> > the expression instead.
>
> The standalone program gets same result.
Hey, I hope you understand random32() does allocate a 32bit value, not a
15bit one...
If really we had such a bug, I am pretty sure we would have noticed.
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: David Miller @ 2012-10-04 21:50 UTC (permalink / raw)
To: eric.dumazet; +Cc: shemminger, dada1, netdev
In-Reply-To: <1349387153.21172.1.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 04 Oct 2012 23:45:53 +0200
> On Thu, 2012-10-04 at 14:28 -0700, Stephen Hemminger wrote:
>> On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
>> David Miller <davem@davemloft.net> wrote:
>>
>> > From: Stephen Hemminger <shemminger@vyatta.com>
>> > Date: Thu, 4 Oct 2012 14:08:28 -0700
>> >
>> > > While working on VXLAN, noticed a bug in UDP introduced by:
>> > >
>> > > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
>> > > Author: Eric Dumazet <dada1@cosmosbay.com>
>> > > Date: Wed Oct 8 11:44:17 2008 -0700
>> > >
>> > > udp: Improve port randomization
>> > >
>> > >
>> > > The logic for choosing where to start for port randomization incorrectly
>> > > calculates the starting port number. It is always ends up using
>> > > the low end of the range independent of the value of random.
>> > > This causes all UDP port searches to start at the same port.
>> > >
>> > > Doing the following fixes it but at the cost of doing a real divide.
>> > >
>> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
>> > >
>> > > ---
>> > > Resend, previous send was not going to netdev.
>> > >
>> > > Not sure if worth fixing for stable, because only has performance impact
>> > > and some application might be depending on current broken behaviour.
>> > >
>> > >
>> > >
>> > > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
>> > > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
>> > > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
>> > > remaining = (high - low) + 1;
>> > >
>> > > rand = net_random();
>> > > - first = (((u64)rand * remaining) >> 32) + low;
>> > > + first = rand % remaining + low;
>> >
>> > Try replacing "remaining" with "(remaining << (64 - 16))" in
>> > the expression instead.
>>
>> The standalone program gets same result.
>
> Hey, I hope you understand random32() does allocate a 32bit value, not a
> 15bit one...
>
> If really we had such a bug, I am pretty sure we would have noticed.
But the issue is is "remaining" that's of limited range, not rand.
I didn't say to shift up 'rand', but rather 'remaining'.
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: Eric Dumazet @ 2012-10-04 21:57 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, dada1, netdev
In-Reply-To: <20121004.175009.1336502669507423764.davem@davemloft.net>
On Thu, 2012-10-04 at 17:50 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 04 Oct 2012 23:45:53 +0200
>
> > On Thu, 2012-10-04 at 14:28 -0700, Stephen Hemminger wrote:
> >> On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
> >> David Miller <davem@davemloft.net> wrote:
> >>
> >> > From: Stephen Hemminger <shemminger@vyatta.com>
> >> > Date: Thu, 4 Oct 2012 14:08:28 -0700
> >> >
> >> > > While working on VXLAN, noticed a bug in UDP introduced by:
> >> > >
> >> > > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> >> > > Author: Eric Dumazet <dada1@cosmosbay.com>
> >> > > Date: Wed Oct 8 11:44:17 2008 -0700
> >> > >
> >> > > udp: Improve port randomization
> >> > >
> >> > >
> >> > > The logic for choosing where to start for port randomization incorrectly
> >> > > calculates the starting port number. It is always ends up using
> >> > > the low end of the range independent of the value of random.
> >> > > This causes all UDP port searches to start at the same port.
> >> > >
> >> > > Doing the following fixes it but at the cost of doing a real divide.
> >> > >
> >> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >> > >
> >> > > ---
> >> > > Resend, previous send was not going to netdev.
> >> > >
> >> > > Not sure if worth fixing for stable, because only has performance impact
> >> > > and some application might be depending on current broken behaviour.
> >> > >
> >> > >
> >> > >
> >> > > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
> >> > > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
> >> > > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> >> > > remaining = (high - low) + 1;
> >> > >
> >> > > rand = net_random();
> >> > > - first = (((u64)rand * remaining) >> 32) + low;
> >> > > + first = rand % remaining + low;
> >> >
> >> > Try replacing "remaining" with "(remaining << (64 - 16))" in
> >> > the expression instead.
> >>
> >> The standalone program gets same result.
> >
> > Hey, I hope you understand random32() does allocate a 32bit value, not a
> > 15bit one...
> >
> > If really we had such a bug, I am pretty sure we would have noticed.
>
> But the issue is is "remaining" that's of limited range, not rand.
> I didn't say to shift up 'rand', but rather 'remaining'.
Sorry I see no issue.
remaining is the delta between high and low, and its right,
unless your compiler or cpu has a nice bug.
remaining = (high - low) + 1;
And I see no bug at all here.
The only assumption is that rand is a random number between 0 and
0xFFFFFFFF
$RANDOM is between 0 and 0x7FFF
Thats the difference
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: Stephen Hemminger @ 2012-10-04 21:59 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, dada1, netdev
In-Reply-To: <20121004.175009.1336502669507423764.davem@davemloft.net>
On Thu, 04 Oct 2012 17:50:09 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 04 Oct 2012 23:45:53 +0200
>
> > On Thu, 2012-10-04 at 14:28 -0700, Stephen Hemminger wrote:
> >> On Thu, 04 Oct 2012 17:12:46 -0400 (EDT)
> >> David Miller <davem@davemloft.net> wrote:
> >>
> >> > From: Stephen Hemminger <shemminger@vyatta.com>
> >> > Date: Thu, 4 Oct 2012 14:08:28 -0700
> >> >
> >> > > While working on VXLAN, noticed a bug in UDP introduced by:
> >> > >
> >> > > commit 9088c5609584684149f3fb5b065aa7f18dcb03ff
> >> > > Author: Eric Dumazet <dada1@cosmosbay.com>
> >> > > Date: Wed Oct 8 11:44:17 2008 -0700
> >> > >
> >> > > udp: Improve port randomization
> >> > >
> >> > >
> >> > > The logic for choosing where to start for port randomization incorrectly
> >> > > calculates the starting port number. It is always ends up using
> >> > > the low end of the range independent of the value of random.
> >> > > This causes all UDP port searches to start at the same port.
> >> > >
> >> > > Doing the following fixes it but at the cost of doing a real divide.
> >> > >
> >> > > Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> >> > >
> >> > > ---
> >> > > Resend, previous send was not going to netdev.
> >> > >
> >> > > Not sure if worth fixing for stable, because only has performance impact
> >> > > and some application might be depending on current broken behaviour.
> >> > >
> >> > >
> >> > >
> >> > > --- a/net/ipv4/udp.c 2012-10-01 17:06:53.107427436 -0700
> >> > > +++ b/net/ipv4/udp.c 2012-10-04 13:43:21.278960379 -0700
> >> > > @@ -216,7 +216,7 @@ int udp_lib_get_port(struct sock *sk, un
> >> > > remaining = (high - low) + 1;
> >> > >
> >> > > rand = net_random();
> >> > > - first = (((u64)rand * remaining) >> 32) + low;
> >> > > + first = rand % remaining + low;
> >> >
> >> > Try replacing "remaining" with "(remaining << (64 - 16))" in
> >> > the expression instead.
> >>
> >> The standalone program gets same result.
> >
> > Hey, I hope you understand random32() does allocate a 32bit value, not a
> > 15bit one...
> >
> > If really we had such a bug, I am pretty sure we would have noticed.
>
> But the issue is is "remaining" that's of limited range, not rand.
> I didn't say to shift up 'rand', but rather 'remaining'.
I think the issue is the Gcc is deciding to truncate the math.
If it is done as separate steps, then the right answer happens:
Good:
t = ((uint64_t) rand * remaining) >> 32;
first = t + low;
Bad:
first = (((uint64_t)rand * remaining) >> (64-16)) + low;
^ permalink raw reply
* Re: [PATCH] udp: port starting location not random
From: Stephen Hemminger @ 2012-10-04 22:06 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David Miller
In-Reply-To: <1349387874.21172.5.camel@edumazet-glaptop>
Nevermind, it depends on random being big enough. This shows that it
works with large enough random(). The hash I was getting from VXLAN
was too small...
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
int main(int ac, char **av) {
int low, high, remaining;
unsigned int rand;
unsigned short first;
low = atoi(av[1]);
high = atoi(av[2]);
remaining = (high - low) + 1;
srandom(getpid());
rand = (uint32_t) random();
first = (((uint64_t)rand * remaining) >> 32) + low;
printf("%d %d %u => %u\n", low, high, rand, first);
return 0;
}
^ permalink raw reply
* [PATCH] net, bluetooth: don't attempt to free a channel that wasn't created
From: Sasha Levin @ 2012-10-04 23:59 UTC (permalink / raw)
To: marcel, gustavo, johan.hedberg, davem
Cc: levinsasha928, davej, linux-kernel, linux-bluetooth, netdev,
Sasha Levin
We may currently attempt to free a channel which wasn't created due to
an error in the initialization path, this would cause a NULL ptr deref.
Introduced in commit 61d6ef3e ("Bluetooth: Make better use of l2cap_chan
reference counting").
Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
net/bluetooth/l2cap_sock.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 083f2bf..66c295a 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1083,7 +1083,8 @@ static void l2cap_sock_destruct(struct sock *sk)
{
BT_DBG("sk %p", sk);
- l2cap_chan_put(l2cap_pi(sk)->chan);
+ if (l2cap_pi(sk)->chan)
+ l2cap_chan_put(l2cap_pi(sk)->chan);
if (l2cap_pi(sk)->rx_busy_skb) {
kfree_skb(l2cap_pi(sk)->rx_busy_skb);
l2cap_pi(sk)->rx_busy_skb = NULL;
--
1.7.12
^ permalink raw reply related
* [PATCH] net, TTY: initialize tty->driver_data before usage
From: Sasha Levin @ 2012-10-05 0:01 UTC (permalink / raw)
To: samuel, davem, gregkh, jslaby, alan
Cc: levinsasha928, davej, linux-kernel, netdev, Sasha Levin
Commit 9c650ffc ("TTY: ircomm_tty, add tty install") split _open() to
_install() and _open(). It also moved the initialization of driver_data
out of open(), but never added it to install() - causing a NULL ptr
deref whenever the driver was used.
Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
net/irda/ircomm/ircomm_tty.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/irda/ircomm/ircomm_tty.c b/net/irda/ircomm/ircomm_tty.c
index 95a3a7a..496ce2c 100644
--- a/net/irda/ircomm/ircomm_tty.c
+++ b/net/irda/ircomm/ircomm_tty.c
@@ -421,6 +421,8 @@ static int ircomm_tty_install(struct tty_driver *driver, struct tty_struct *tty)
hashbin_insert(ircomm_tty, (irda_queue_t *) self, line, NULL);
}
+ tty->driver_data = self;
+
return tty_port_install(&self->port, driver, tty);
}
--
1.7.12
^ permalink raw reply related
* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Joe Perches @ 2012-10-05 0:09 UTC (permalink / raw)
To: David Miller
Cc: peter.senna, shemminger, mlindner, kernel-janitors, netdev,
linux-kernel
In-Reply-To: <20121004.145419.1859367129463136197.davem@davemloft.net>
On Thu, 2012-10-04 at 14:54 -0400, David Miller wrote:
> From: Peter Senna Tschudin <peter.senna@gmail.com>
> > On Thu, Oct 4, 2012 at 8:23 PM, David Miller <davem@davemloft.net> wrote:
> >> We want to know the implications of the bug being fixed.
> >> Does it potentially cause an OOPS? Bad reference counting and thus
> >> potential leaks or early frees?
> >>
> >> You have to analyze the implications and ramifications of the bug
> >> being fixed. We need that information.
You are asking for deeper level analysis that may not
be reasonably possible from the robotic patch fixed
by a robotic piece of a bit of code correction via a
static analysis.
> >> It's just "bad error code, this is the script that fixed it, kthx,
> >> bye" which is pretty much useless for anaylsis.
Which may be all but impossible but for the handful of
folks that know all the gory/intimate details of the
original bit of code.
> What does it potentially cause the caller to do? Will it potentially
> treat an error as a success and as a result register an object
> illegally?
>
> Real analysis please. The text you provided above is basically still
> robotic and could be used to describe any error code return fix.
Right, it's useful to attempt but may be infeasible in
practice.
^ permalink raw reply
* Re: [patch v3 01/11] netlink: add reference of module in netlink_dump_start
From: Gao feng @ 2012-10-05 0:50 UTC (permalink / raw)
To: Ben Hutchings
Cc: davem, eric.dumazet, steffen.klassert, netfilter-devel,
linux-rdma, netdev, linux-crypto, pablo, stephen.hemminger,
jengelh
In-Reply-To: <1349365302.2817.1.camel@bwh-desktop.uk.solarflarecom.com>
于 2012年10月04日 23:41, Ben Hutchings 写道:
> 'which' not 'witch' :-)
>
> Ben.
thanks Ben,will fix it :)
^ permalink raw reply
* [PATCH] net: Fix skb_under_panic oops in neigh_resolve_output
From: ramesh.nagappa @ 2012-10-05 3:05 UTC (permalink / raw)
To: bnramesh
Cc: davem, edumazet, ebiederm, xemul, netdev, linux-kernel,
Ramesh Nagappa
From: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
The retry loop in neigh_resolve_output() and neigh_connected_output()
call dev_hard_header() with out reseting the skb to network_header.
This causes the retry to fail with skb_under_panic. The fix is to
reset the network_header within the retry loop.
Signed-off-by: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
Reviewed-by: Shawn Lu <shawn.lu@ericsson.com>
Reviewed-by: Robert Coulson <robert.coulson@ericsson.com>
Reviewed-by: Billie Alsup <billie.alsup@ericsson.com>
---
net/core/neighbour.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 117afaf..f50c542 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1313,6 +1313,7 @@ int neigh_resolve_output(struct neighbour *neigh, struct sk_buff *skb)
do {
seq = read_seqbegin(&neigh->ha_lock);
+ __skb_pull(skb, skb_network_offset(skb));
err = dev_hard_header(skb, dev, ntohs(skb->protocol),
neigh->ha, NULL, skb->len);
} while (read_seqretry(&neigh->ha_lock, seq));
@@ -1342,10 +1343,9 @@ int neigh_connected_output(struct neighbour *neigh, struct sk_buff *skb)
unsigned int seq;
int err;
- __skb_pull(skb, skb_network_offset(skb));
-
do {
seq = read_seqbegin(&neigh->ha_lock);
+ __skb_pull(skb, skb_network_offset(skb));
err = dev_hard_header(skb, dev, ntohs(skb->protocol),
neigh->ha, NULL, skb->len);
} while (read_seqretry(&neigh->ha_lock, seq));
--
1.7.11.4
^ permalink raw reply related
* Re: Question: Proper place for port details?
From: Jeff Kirsher @ 2012-10-05 3:12 UTC (permalink / raw)
To: raspl; +Cc: netdev
In-Reply-To: <506D4896.8080900@linux.vnet.ibm.com>
On 10/04/2012 01:28 AM, Stefan Raspl wrote:
> Some network device drivers are capable of collecting further
> information on the link partner, e.g. reporting details on the port
> in the adjacent switch. Is there a good place to make these
> available outside of the kernel? ethtool gives a bit of information
> like the link partner's speed or duplex mode. But is adding further
> information about the link partner to ethtool desired, or is there a
> better place somewhere else?
>
> Ciao,
> Stefan
IMHO, reporting details on a port should be reported by Ethtool.
^ permalink raw reply
* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Julia Lawall @ 2012-10-05 5:22 UTC (permalink / raw)
To: Joe Perches
Cc: David Miller, peter.senna, shemminger, mlindner, kernel-janitors,
netdev, linux-kernel
In-Reply-To: <1349395795.2008.26.camel@joe-AO722>
On Thu, 4 Oct 2012, Joe Perches wrote:
> On Thu, 2012-10-04 at 14:54 -0400, David Miller wrote:
>> From: Peter Senna Tschudin <peter.senna@gmail.com>
>>> On Thu, Oct 4, 2012 at 8:23 PM, David Miller <davem@davemloft.net> wrote:
>>>> We want to know the implications of the bug being fixed.
>>>> Does it potentially cause an OOPS? Bad reference counting and thus
>>>> potential leaks or early frees?
>>>>
>>>> You have to analyze the implications and ramifications of the bug
>>>> being fixed. We need that information.
>
> You are asking for deeper level analysis that may not
> be reasonably possible from the robotic patch fixed
> by a robotic piece of a bit of code correction via a
> static analysis.
As Peter pointed out, it is not even a robotic fix, if robotic means
literally that it was done by a robot. A tool was used to find a
potential problem, and then Peter studied the code to see what fix was
appropriate.
In this case, finding out the impact, requires looking up the call chain
in all directions to find out what the callers do with the returned value.
And understanding the likely impact along the way. Often the call chain
involves function pointers. This is all possible to do. And perhaps it
would be even more helpful to the consumers of the patch than just having
the problem fixed. But the fact remains that at other places in the
function, someone thought it was useful to return an error code, so in the
place where the error code return is missing, it seems like a useful fault
to fix, whether it has an impact now or might have one later. The main
human analysis required to generate the patch is to be convinced that the
given return path really represents an error condition and that the
function normally returns the specified kind of value in that case. If
there is some way in which these points are unclear, then the commit
message should certainly explain the reasoning that was used.
julia
>
>>>> It's just "bad error code, this is the script that fixed it, kthx,
>>>> bye" which is pretty much useless for anaylsis.
>
> Which may be all but impossible but for the handful of
> folks that know all the gory/intimate details of the
> original bit of code.
>
>> What does it potentially cause the caller to do? Will it potentially
>> treat an error as a success and as a result register an object
>> illegally?
>>
>> Real analysis please. The text you provided above is basically still
>> robotic and could be used to describe any error code return fix.
>
> Right, it's useful to attempt but may be infeasible in
> practice.
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH 01/11] net: axienet: Remove NETIF_F_SG dropping for no checksum feature
From: Michal Simek @ 2012-10-05 5:22 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-kernel, anirudh, John.Linn, grant.likely,
rob.herring
In-Reply-To: <20121004.142612.1111166663063631532.davem@davemloft.net>
Hi David,
On 10/04/2012 08:26 PM, David Miller wrote:
>
> Sorry, no.
>
> I've announced on netdev very clearly that net-next submissions are not
> appropriate at this time and that only pure bug fixes should be submitted.
>
> Watch for the announcement on netdev of net-next openning up after the
> merge window closes, that's when you should resubmit this series.
Sorry I should label it as RFC.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: [PATCH 0/3] virtio-net: inline header support
From: Rusty Russell @ 2012-10-05 5:43 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Michael S. Tsirkin, Thomas Lendacky, kvm, netdev, linux-kernel,
virtualization, avi, Sasha Levin
In-Reply-To: <506D8DD5.20904@redhat.com>
Paolo Bonzini <pbonzini@redhat.com> writes:
> Il 04/10/2012 14:51, Rusty Russell ha scritto:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>>> Il 04/10/2012 02:11, Rusty Russell ha scritto:
>>>>>> There's a reason I haven't done this. I really, really dislike "my
>>>>>> implemention isn't broken" feature bits. We could have an infinite
>>>>>> number of them, for each bug in each device.
>>>>>
>>>>> However, this bug affects (almost) all implementations and (almost) all
>>>>> devices. It even makes sense to reserve a transport feature bit for it
>>>>> instead of a device feature bit.
>>>>
>>>> Perhaps, but we have to fix the bugs first!
>>>
>>> Yes. :) Isn't that what mst's patch does?
>>>
>>>> As I said, my torture patch broke qemu immediately. Since noone has
>>>> leapt onto fixing that, I'll take a look now...
>>>
>>> I can look at virtio-scsi.
>>
>> Actually, you can't, see my reply to Anthony...
>>
>> Message-ID: <87lifm1y1n.fsf@rustcorp.com.au>
>
> struct virtio_scsi_req_cmd {
> // Read-only
> u8 lun[8];
> u64 id;
> u8 task_attr;
> u8 prio;
> u8 crn;
> char cdb[cdb_size];
> char dataout[];
> // Write-only part
> u32 sense_len;
> u32 residual;
> u16 status_qualifier;
> u8 status;
> u8 response;
> u8 sense[sense_size];
> char datain[];
> };
>
> where cdb_size and sense_size come from configuration space. The device
> right now expects everything before dataout/datain to be in a single
> descriptor, but that's in no way part of the spec. Am I missing
> something egregious?
Since you wrote it, I hope not :)
That's good. But virtio_blk's scsi command is insoluble AFAICT. As I
said to Anthony, the best rules are "always" and "never", so I'd really
rather not have to grandfather that in.
Cheers,
Rusty.
^ permalink raw reply
* Re: [PATCH 08/11] net: ll_temac: Do not use fixed mtu size
From: Michal Simek @ 2012-10-05 5:58 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev, linux-kernel, Anirudha Sarangi, John Linn, Grant Likely,
Rob Herring, David S. Miller
In-Reply-To: <1349378563.2817.27.camel@bwh-desktop.uk.solarflarecom.com>
On 10/04/2012 09:22 PM, Ben Hutchings wrote:
> On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
>> Use max mtu instead.
> [...]
>
> MTU does not include the Ethernet header so I have no idea how this is
> expected to work...
Right. This is wrong fix. It should be the same as is in axienet.
There is max_frm_size used which is mtu+header+tailer.
I will fix it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* [patch v4 1/2] netlink: add reference of module in netlink_dump_start
From: Gao feng @ 2012-10-05 6:15 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
steffen.klassert-opNxpl+3fjRBDgjK7y7TUQ,
pablo-Cap9r6Oaw4JrovVCs/uTlw, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
stephen.hemminger-ZtmgI6mnKB3QT0dZR+AlfA, jengelh-9+2X+4sQBs8,
Gao feng
I get a panic when I use ss -a and rmmod inet_diag at the
same time.
it's because netlink_dump use inet_diag_dump witch function
belongs to module inet_diag.
I search the codes and find many modules have the same problem.
We need add reference of the module which the cb->dump belongs
to.
Thanks for all help from Stephen,Jan,Eric,Steffen and Pablo.
Change From v3:
change netlink_dump_start to inline,suggestion from Pablo and
Eric.
Change From v2:
delete netlink_dump_done,and call module_put in netlink_dump
and netlink_sock_destruct.
Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
include/linux/netlink.h | 20 ++++++++++++++++----
net/netlink/af_netlink.c | 29 +++++++++++++++++++++--------
2 files changed, 37 insertions(+), 12 deletions(-)
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index f80c56a..6d3af05 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -245,6 +245,8 @@ struct netlink_callback {
struct netlink_callback *cb);
int (*done)(struct netlink_callback *cb);
void *data;
+ /* the module that dump function belong to */
+ struct module *module;
u16 family;
u16 min_dump_alloc;
unsigned int prev_seq, seq;
@@ -262,14 +264,24 @@ __nlmsg_put(struct sk_buff *skb, u32 portid, u32 seq, int type, int len, int fla
struct netlink_dump_control {
int (*dump)(struct sk_buff *skb, struct netlink_callback *);
- int (*done)(struct netlink_callback*);
+ int (*done)(struct netlink_callback *);
void *data;
+ struct module *module;
u16 min_dump_alloc;
};
-extern int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
- struct netlink_dump_control *control);
+extern int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
+ const struct nlmsghdr *nlh,
+ struct netlink_dump_control *control);
+static inline int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
+ const struct nlmsghdr *nlh,
+ struct netlink_dump_control *control)
+{
+ if (!control->module)
+ control->module = THIS_MODULE;
+
+ return __netlink_dump_start(ssk, skb, nlh, control);
+}
#endif /* __KERNEL__ */
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 0f2e3ad..01e944a 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -169,6 +169,8 @@ static void netlink_sock_destruct(struct sock *sk)
if (nlk->cb) {
if (nlk->cb->done)
nlk->cb->done(nlk->cb);
+
+ module_put(nlk->cb->module);
netlink_destroy_callback(nlk->cb);
}
@@ -1758,6 +1760,7 @@ static int netlink_dump(struct sock *sk)
nlk->cb = NULL;
mutex_unlock(nlk->cb_mutex);
+ module_put(cb->module);
netlink_consume_callback(cb);
return 0;
@@ -1767,9 +1770,9 @@ errout_skb:
return err;
}
-int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
- const struct nlmsghdr *nlh,
- struct netlink_dump_control *control)
+int __netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
+ const struct nlmsghdr *nlh,
+ struct netlink_dump_control *control)
{
struct netlink_callback *cb;
struct sock *sk;
@@ -1784,6 +1787,7 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
cb->done = control->done;
cb->nlh = nlh;
cb->data = control->data;
+ cb->module = control->module;
cb->min_dump_alloc = control->min_dump_alloc;
atomic_inc(&skb->users);
cb->skb = skb;
@@ -1794,19 +1798,28 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
return -ECONNREFUSED;
}
nlk = nlk_sk(sk);
- /* A dump is in progress... */
+
mutex_lock(nlk->cb_mutex);
+ /* A dump is in progress... */
if (nlk->cb) {
mutex_unlock(nlk->cb_mutex);
netlink_destroy_callback(cb);
- sock_put(sk);
- return -EBUSY;
+ ret = -EBUSY;
+ goto out;
}
+ /* add reference of module which cb->dump belongs to */
+ if (!try_module_get(cb->module)) {
+ mutex_unlock(nlk->cb_mutex);
+ netlink_destroy_callback(cb);
+ ret = -EPROTONOSUPPORT;
+ goto out;
+ }
+
nlk->cb = cb;
mutex_unlock(nlk->cb_mutex);
ret = netlink_dump(sk);
-
+out:
sock_put(sk);
if (ret)
@@ -1817,7 +1830,7 @@ int netlink_dump_start(struct sock *ssk, struct sk_buff *skb,
*/
return -EINTR;
}
-EXPORT_SYMBOL(netlink_dump_start);
+EXPORT_SYMBOL(__netlink_dump_start);
void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err)
{
--
1.7.7.6
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] net: Fix skb_under_panic oops in neigh_resolve_output
From: Eric Dumazet @ 2012-10-05 6:27 UTC (permalink / raw)
To: ramesh.nagappa
Cc: bnramesh, davem, edumazet, ebiederm, xemul, netdev, linux-kernel,
Ramesh Nagappa
In-Reply-To: <1349406325-3157-1-git-send-email-ramesh.nagappa@gmail.com>
On Thu, 2012-10-04 at 20:05 -0700, ramesh.nagappa@gmail.com wrote:
> From: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
>
> The retry loop in neigh_resolve_output() and neigh_connected_output()
> call dev_hard_header() with out reseting the skb to network_header.
> This causes the retry to fail with skb_under_panic. The fix is to
> reset the network_header within the retry loop.
>
> Signed-off-by: Ramesh Nagappa <ramesh.nagappa@ericsson.com>
> Reviewed-by: Shawn Lu <shawn.lu@ericsson.com>
> Reviewed-by: Robert Coulson <robert.coulson@ericsson.com>
> Reviewed-by: Billie Alsup <billie.alsup@ericsson.com>
> ---
> net/core/neighbour.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 117afaf..f50c542 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -1313,6 +1313,7 @@ int neigh_resolve_output(struct neighbour *neigh, struct sk_buff *skb)
>
> do {
> seq = read_seqbegin(&neigh->ha_lock);
> + __skb_pull(skb, skb_network_offset(skb));
This __skb_pull() could be done before the read_seqbegin() to keep the
protected section short.
> err = dev_hard_header(skb, dev, ntohs(skb->protocol),
> neigh->ha, NULL, skb->len);
> } while (read_seqretry(&neigh->ha_lock, seq));
> @@ -1342,10 +1343,9 @@ int neigh_connected_output(struct neighbour *neigh, struct sk_buff *skb)
> unsigned int seq;
> int err;
>
> - __skb_pull(skb, skb_network_offset(skb));
> -
> do {
> seq = read_seqbegin(&neigh->ha_lock);
> + __skb_pull(skb, skb_network_offset(skb));
This __skb_pull() could be done before the read_seqbegin() to keep the
protected section short.
> err = dev_hard_header(skb, dev, ntohs(skb->protocol),
> neigh->ha, NULL, skb->len);
> } while (read_seqretry(&neigh->ha_lock, seq));
Thanks
^ permalink raw reply
* Re: [PATCH 02/11] net: axienet: Add ioctl support
From: Michal Simek @ 2012-10-05 5:54 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev, linux-kernel, Anirudha Sarangi, John Linn, Grant Likely,
Rob Herring, David S. Miller
In-Reply-To: <1349378262.2817.25.camel@bwh-desktop.uk.solarflarecom.com>
On 10/04/2012 09:17 PM, Ben Hutchings wrote:
> On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
>> Allow user to access the MDIO from userspace.
>>
>> Signed-off-by: Michal Simek <monstr@monstr.eu>
>> CC: Anirudha Sarangi <anirudh@xilinx.com>
>> CC: John Linn <John.Linn@xilinx.com>
>> CC: Grant Likely <grant.likely@secretlab.ca>
>> CC: Rob Herring <rob.herring@calxeda.com>
>> CC: David S. Miller <davem@davemloft.net>
>> ---
>> drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 15 +++++++++++++++
>> 1 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> index 50167ab..a5b41cd 100644
>> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> @@ -1053,6 +1053,20 @@ static void axienet_poll_controller(struct net_device *ndev)
>> }
>> #endif
>>
>> +/* Ioctl MII Interface */
>> +static int axienet_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>> +{
>> + struct axienet_local *priv = netdev_priv(dev);
>> +
>> + if (!netif_running(dev))
>> + return -EINVAL;
>
> Not sure this is the appropriate error code.
>
>> + if (!priv->phy_dev)
>> + return -ENODEV;
>
> Error code should be EOPNOTSUPP - the device is present but just doesn't
> support MDIO.
ok. Thanks will fix it.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* Re: [PATCH 11/11] net: xilinx: Show csum in bootlog
From: Michal Simek @ 2012-10-05 5:48 UTC (permalink / raw)
To: Ben Hutchings
Cc: netdev, linux-kernel, Anirudha Sarangi, John Linn, Grant Likely,
Rob Herring, David S. Miller
In-Reply-To: <1349378144.2817.23.camel@bwh-desktop.uk.solarflarecom.com>
On 10/04/2012 09:15 PM, Ben Hutchings wrote:
> On Thu, 2012-10-04 at 20:14 +0200, Michal Simek wrote:
>> Just show current setting in bootlog.
> [...]
>> --- a/drivers/net/ethernet/xilinx/ll_temac_main.c
>> +++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
>> @@ -1052,12 +1052,14 @@ static int __devinit temac_of_probe(struct platform_device *op)
>> /* Setup checksum offload, but default to off if not specified */
>> lp->temac_features = 0;
>> p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,txcsum", NULL);
>> + dev_info(&op->dev, "TX_CSUM %d\n", be32_to_cpup(p));
>> if (p && be32_to_cpu(*p)) {
>> lp->temac_features |= TEMAC_FEATURE_TX_CSUM;
>> /* Can checksum TCP/UDP over IPv4. */
>> ndev->features |= NETIF_F_IP_CSUM;
>> }
>> p = (__be32 *)of_get_property(op->dev.of_node, "xlnx,rxcsum", NULL);
>> + dev_info(&op->dev, "RX_CSUM %d\n", be32_to_cpup(p));
> [...]
>
> Is there any particular reason you think this needs to be logged by
> default, rather than letting users run ethtool -k? I suggest using
> dev_dbg() instead.
The reason was just to show it in the bootlog.
I will check ethtool support for these drivers.
Thanks for your comments,
Michal
--
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian
^ permalink raw reply
* [patch v4 2/2] infiniband: pass rdma_cm module to netlink_dump_start
From: Gao feng @ 2012-10-05 6:15 UTC (permalink / raw)
To: davem
Cc: eric.dumazet, steffen.klassert, pablo, netdev, linux-rdma,
stephen.hemminger, jengelh, Gao feng, Roland Dreier, Sean Hefty
In-Reply-To: <1349417749-24869-1-git-send-email-gaofeng@cn.fujitsu.com>
set netlink_dump_control.module to avoid panic.
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
---
drivers/infiniband/core/cma.c | 3 ++-
drivers/infiniband/core/netlink.c | 1 +
include/rdma/rdma_netlink.h | 1 +
3 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 26b3760..4fff27a 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -3498,7 +3498,8 @@ out:
}
static const struct ibnl_client_cbs cma_cb_table[] = {
- [RDMA_NL_RDMA_CM_ID_STATS] = { .dump = cma_get_id_stats },
+ [RDMA_NL_RDMA_CM_ID_STATS] = { .dump = cma_get_id_stats,
+ .module = THIS_MODULE },
};
static int __init cma_init(void)
diff --git a/drivers/infiniband/core/netlink.c b/drivers/infiniband/core/netlink.c
index fe10a94..da06abd 100644
--- a/drivers/infiniband/core/netlink.c
+++ b/drivers/infiniband/core/netlink.c
@@ -154,6 +154,7 @@ static int ibnl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct netlink_dump_control c = {
.dump = client->cb_table[op].dump,
+ .module = client->cb_table[op].module,
};
return netlink_dump_start(nls, skb, nlh, &c);
}
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index 3c5363a..bd3d8b2 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -39,6 +39,7 @@ struct rdma_cm_id_stats {
struct ibnl_client_cbs {
int (*dump)(struct sk_buff *skb, struct netlink_callback *nlcb);
+ struct module *module;
};
int ibnl_init(void);
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH 19/20] drivers/net/ethernet/marvell/skge.c: fix error return code
From: Joe Perches @ 2012-10-05 7:36 UTC (permalink / raw)
To: Julia Lawall
Cc: David Miller, peter.senna, shemminger, mlindner, kernel-janitors,
netdev, linux-kernel
In-Reply-To: <alpine.DEB.2.02.1210050712590.1958@localhost6.localdomain6>
On Fri, 2012-10-05 at 07:22 +0200, Julia Lawall wrote:
> A tool was used to find a potential problem, and then Peter
> studied the code to see what fix was appropriate.
Hi Julia.
Was it true that a static analysis tool found the original
potential issue? If so, what tool was it?
But wasn't the scripted fix applied to the rest of the tree
robotically?
^ permalink raw reply
* Re: [RFC PATCH 1/2] sctp: fix a typo in prototype of __sctp_rcv_lookup()
From: Nicolas Dichtel @ 2012-10-05 7:37 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: linux-sctp, netdev
In-Reply-To: <506DC190.6050407@gmail.com>
Le 04/10/2012 19:04, Vlad Yasevich a écrit :
> On 10/03/2012 11:43 AM, Nicolas Dichtel wrote:
>> Just to avoid confusion when people only reads this prototype.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> ---
>> net/sctp/input.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/sctp/input.c b/net/sctp/input.c
>> index 25dfe73..8bd3c27 100644
>> --- a/net/sctp/input.c
>> +++ b/net/sctp/input.c
>> @@ -68,8 +68,8 @@
>> static int sctp_rcv_ootb(struct sk_buff *);
>> static struct sctp_association *__sctp_rcv_lookup(struct net *net,
>> struct sk_buff *skb,
>> - const union sctp_addr *laddr,
>> const union sctp_addr *paddr,
>> + const union sctp_addr *laddr,
>> struct sctp_transport **transportp);
>> static struct sctp_endpoint *__sctp_rcv_lookup_endpoint(struct net *net,
>> const union sctp_addr *laddr);
>>
>
> Wow, this must have been very old...
v2.5.33 ;-)
^ permalink raw reply
* Re: kernel 3.2.27 on arm: WARNING: at mm/page_alloc.c:2109 __alloc_pages_nodemask+0x1d4/0x68c()
From: Eric Dumazet @ 2012-10-05 7:41 UTC (permalink / raw)
To: mbizon; +Cc: David Madore, Francois Romieu, netdev, linux-kernel, Hugh Dickins
In-Reply-To: <1349368171.16011.79.camel@edumazet-glaptop>
On Thu, 2012-10-04 at 18:29 +0200, Eric Dumazet wrote:
> On Thu, 2012-10-04 at 18:02 +0200, Maxime Bizon wrote:
> > On Fri, 2012-08-31 at 19:21 -0700, Hugh Dickins wrote:
> >
> > Hi,
> >
> > > Francois is right that a GFP_ATOMIC allocation from pskb_expand_head()
> > > is failing, which can easily happen, and cause your "failed to reallocate
> > > TX buffer" errors; but it's well worth looking up what's actually on
> > > lines 2108 and 2109 of mm/page_alloc.c in 3.2.27:
> > >
> > > if (order >= MAX_ORDER) {
> > > WARN_ON_ONCE(!(gfp_mask & __GFP_NOWARN));
> > >
> > > That was probably not a sane allocation request, it has gone out of range:
> > > maybe the skb header is even corrupted. If you're lucky, it might be
> > > something that netdev will recognize as already fixed.
> >
> > I have the same problem on the exact same hardware and found the cause:
> >
> > Author: Eric Dumazet <eric.dumazet@gmail.com>
> > Date: Tue Apr 10 20:08:39 2012 +0000
> >
> > net: allow pskb_expand_head() to get maximum tailroom
> >
> > [ Upstream commit 87151b8689d890dfb495081f7be9b9e257f7a2df ]
> >
> >
> > It turns out this change has a bad side effect on drivers that uses
> > skb_recycle(), in that case mv643xx_eth.c
> >
> > Since skb_recycle() resets skb->data using (skb->head + NET_SKB_PAD), a
> > recycled skb going multiple times through a path that needs to expand
> > skb head will get bigger and bigger each time, and you eventually end up
> > with an allocation failure.
> >
> > An idea to fix this would be to pass needed skb size to skb_resize() and
> > set skb->data to MIN(NET_SKB_PAD, (skb->end - skb->head - skb_size) / 2)
> >
> > skb recycling gives a small speed boost, but does not get a lot of test
> > coverage since only 3 drivers uses it
> >
>
> Thanks Maxime
By the way, the commit you pointed has no effect on the reallocation
performed by pskb_expand_head() :
int size = nhead + skb_end_offset(skb) + ntail;
So pskb_expand_head() always assumed the current head is fully used, and
because we have some kmalloc-power-of-two contraints, each time
pskb_expand_head() is called with a non zero (nhead + ntail) we double
the skb->head ksize.
So why are we using skb_end_offset(skb) here is the question.
I guess it could be (skb_tail_pointer(skb) - skb->head) on some uses.
(ie not counting the unused bytes from tail_pointer to end_pointer)
Its probably long to audit all pskb_expand_head() users (about 77 in
tree), but most of them use (nhead = 0, ntail = 0)
It looks like the following patch should be fine.
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index cdc2859..dd42c6a 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1053,11 +1053,22 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
{
int i;
u8 *data;
- int size = nhead + skb_end_offset(skb) + ntail;
+ unsigned int tail_offset = skb_tail_pointer(skb) - skb->head;
+ int size = nhead + ntail;
long off;
BUG_ON(nhead < 0);
+ /* callers using nhead == 0 and ntail == 0 want to get a fresh copy,
+ * so allocate same amount of memory (skb_end_offset)
+ * For others, they want extra headroom or tailroom against the
+ * currently used portion of header (skb->head -> skb_tail_pointer)
+ */
+ if (!size)
+ size = skb_end_offset(skb);
+ else
+ size += tail_offset;
+
if (skb_shared(skb))
BUG();
@@ -1074,7 +1085,7 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
/* Copy only real data... and, alas, header. This should be
* optimized for the cases when header is void.
*/
- memcpy(data + nhead, skb->head, skb_tail_pointer(skb) - skb->head);
+ memcpy(data + nhead, skb->head, tail_offset);
memcpy((struct skb_shared_info *)(data + size),
skb_shinfo(skb),
^ 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