* [PATCH] ppp: fix BUG on non-linear SKB (multilink receive)
From: Ben McKeegan @ 2009-11-12 13:09 UTC (permalink / raw)
To: davem, paulus, netdev, linux-ppp
PPP does not correctly call pskb_may_pull() on all necessary receive paths
before reading the PPP protocol, thus causing PPP to report seemingly
random 'unsupported protocols' and eventually trigger BUG_ON(skb->len <
skb->data_len) in skb_pull_rcsum() when receiving multilink protocol in
non-linear skbs.
ppp_receive_nonmp_frame() does not call pskb_may_pull() before reading the
protocol number. For the non-mp receive path this is not a problem, as
this check is done in ppp_receive_frame(). For the mp receive path,
ppp_mp_reconstruct() usually copies the data into a new linear skb.
However, in the case where the frame is made up of a single mp fragment,
the mp header is pulled and the existing skb used. This skb was then
passed to ppp_receive_nonmp_frame() without checking if the encapsulated
protocol header can safely be read.
Signed-off-by: Ben McKeegan <ben@netservers.co.uk>
---
This a long standing bug we have observed with PPP over L2TP received on
an e1000e interface, although it may arise with other PPP encapsulations
or devices.
diff -uprN linux-2.6.31.6/drivers/net/ppp_generic.c linux-2.6.31.6-ppp-non-linear-skb/drivers/net/ppp_generic.c
--- linux-2.6.31.6/drivers/net/ppp_generic.c 2009-11-10 00:32:31.000000000 +0000
+++ linux-2.6.31.6-ppp-non-linear-skb/drivers/net/ppp_generic.c 2009-11-12 11:58:40.000000000 +0000
@@ -1944,7 +1944,13 @@ ppp_receive_mp_frame(struct ppp *ppp, st
/* Pull completed packets off the queue and receive them. */
while ((skb = ppp_mp_reconstruct(ppp)))
- ppp_receive_nonmp_frame(ppp, skb);
+ if (pskb_may_pull(skb, 2))
+ ppp_receive_nonmp_frame(ppp, skb);
+ else {
+ ++ppp->dev->stats.rx_length_errors;
+ kfree_skb(skb);
+ ppp_receive_error(ppp);
+ }
return;
^ permalink raw reply
* Re: large packet loss take2 2.6.31.x
From: Caleb Cushing @ 2009-11-12 13:46 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: Frans Pop, Andi Kleen, linux-kernel, netdev
In-Reply-To: <20091112113836.GA7963@ff.dom.local>
[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]
> On 11-11-2009 22:47, Andi Kleen wrote:
> ...
>> It might be also useful if you could describe what kind
>> of network devices you use and how you determine
>> the packet loss.
>
> Btw, you didn't send the stats you compared, and your wireshark dump
> doesn't show anything wrong either.
>
> Jarek P.
>
I didn't see that sorry. I wasn't sure if the dump would or not (I'm
not a networking expert, just know more than the average joe).
from dmesg (networking device)
e1000e: Intel(R) PRO/1000 Network Driver - 1.0.2-k2
from lspci
00:19.0 Ethernet controller: Intel Corporation 82562V-2 10/100 Network
Connection (rev 02)
the attached png's show mtr with bad being when I have the problem.
for those not familiar mtr sends an icmp packet to each hop in 1
second then loops. when I'm having this kind of packet loss (and
sometimes it's higher) all services including dhcp, dns, and http (web
browsing) get flaky, or don't work at all (I really can't browse the
web).
--
Caleb Cushing
http://xenoterracide.blogspot.com
[-- Attachment #2: mtr_good.png --]
[-- Type: image/png, Size: 69328 bytes --]
^ permalink raw reply
* Re: [PATCH] [next-next-2.6] net: configurable device name hash
From: Eric Dumazet @ 2009-11-12 14:09 UTC (permalink / raw)
To: Mark Smith; +Cc: David Miller, opurdila, shemminger, netdev
In-Reply-To: <20091112231620.0b56c0b0@opy.nosense.org>
Mark Smith a écrit :
> On Wed, 11 Nov 2009 18:36:26 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
>
>> From: Octavian Purdila <opurdila@ixiacom.com>
>> Date: Wed, 11 Nov 2009 23:47:41 +0200
>>
>>> We could use a similar function that will work in the per namespace
>>> initialization context, but this might upset net namespace folks
>>> since we will get a large hash for every namespace.
>> Use kzalloc(), that's sufficient for a 64K or so hash table which is
>> way more than you ever will need.
>>
>> Use the GFP_* flags that will silently (ie. without a log message)
>> fail, and divide by two until you successfully allocate the table if
>> you're worried about memory fragmentation at allocation time.
>>
>> This is so straightforward, I can't believe we're talking so much
>> about how to implement this, it's a 15 minute hack :-)
>
> Yes, but sadly, sometimes there is too much history(!) to be able to be
> fully aware of it. "suck-it-and-see" type patches are possibly a
> quicker way to find out what people are thinking right now!
>
Before extending hash tables, we should make sure existing algos are going to
scale with millions of netdevices, and they dont scale that much for the moment.
We still have many for_each_netdev() loops...
It's easy to change a constant somewhere in an include file, its less easy to make
real scalability changes :(
^ permalink raw reply
* [PATCH net-next-2.6] ipv4: speedup inet_dump_ifaddr()
From: Eric Dumazet @ 2009-11-12 14:11 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List, Stephen Hemminger
When handling large number of netdevices, inet_dump_ifaddr()
is very slow because it has O(N^2) complexity.
Instead of scanning one single list, we can use the NETDEV_HASHENTRIES
sub lists of the dev_index hash table, and RCU lookups.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv4/devinet.c | 57 ++++++++++++++++++++++++++-----------------
1 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index c2045f9..a74cb4e 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1174,39 +1174,52 @@ nla_put_failure:
static int inet_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
{
struct net *net = sock_net(skb->sk);
- int idx, ip_idx;
+ int h, s_h;
+ int idx, s_idx;
+ int ip_idx, s_ip_idx;
struct net_device *dev;
struct in_device *in_dev;
struct in_ifaddr *ifa;
- int s_ip_idx, s_idx = cb->args[0];
+ struct hlist_head *head;
+ struct hlist_node *node;
- s_ip_idx = ip_idx = cb->args[1];
- idx = 0;
- for_each_netdev(net, dev) {
- if (idx < s_idx)
- goto cont;
- if (idx > s_idx)
- s_ip_idx = 0;
- in_dev = __in_dev_get_rtnl(dev);
- if (!in_dev)
- goto cont;
+ s_h = cb->args[0];
+ s_idx = idx = cb->args[1];
+ s_ip_idx = ip_idx = cb->args[2];
- for (ifa = in_dev->ifa_list, ip_idx = 0; ifa;
- ifa = ifa->ifa_next, ip_idx++) {
- if (ip_idx < s_ip_idx)
- continue;
- if (inet_fill_ifaddr(skb, ifa, NETLINK_CB(cb->skb).pid,
+ rcu_read_lock();
+ for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
+ idx = 0;
+ head = &net->dev_index_head[h];
+ hlist_for_each_entry_rcu(dev, node, head, index_hlist) {
+ if (idx < s_idx)
+ goto cont;
+ if (idx > s_idx)
+ s_ip_idx = 0;
+ in_dev = __in_dev_get_rcu(dev);
+ if (!in_dev)
+ goto cont;
+
+ for (ifa = in_dev->ifa_list, ip_idx = 0; ifa;
+ ifa = ifa->ifa_next, ip_idx++) {
+ if (ip_idx < s_ip_idx)
+ continue;
+ if (inet_fill_ifaddr(skb, ifa,
+ NETLINK_CB(cb->skb).pid,
cb->nlh->nlmsg_seq,
RTM_NEWADDR, NLM_F_MULTI) <= 0)
- goto done;
- }
+ goto done;
+ }
cont:
- idx++;
+ idx++;
+ }
}
done:
- cb->args[0] = idx;
- cb->args[1] = ip_idx;
+ rcu_read_unlock();
+ cb->args[0] = h;
+ cb->args[1] = idx;
+ cb->args[2] = ip_idx;
return skb->len;
}
^ permalink raw reply related
* [PATCH net-next-2.6] ipv6: speedup inet6_dump_addr()
From: Eric Dumazet @ 2009-11-12 14:11 UTC (permalink / raw)
To: David S. Miller; +Cc: Linux Netdev List, Stephen Hemminger
When handling large number of netdevices, inet6_dump_addr()
is very slow because it has O(N^2) complexity.
Instead of scanning one single list, we can use the NETDEV_HASHENTRIES
sub lists of the dev_index hash table, and RCU lookups.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv6/addrconf.c | 171 +++++++++++++++++++++++-------------------
1 files changed, 97 insertions(+), 74 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 9ff8ab9..522bdc7 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3481,91 +3481,114 @@ enum addr_type_t
ANYCAST_ADDR,
};
+/* called with rcu_read_lock() */
+static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
+ struct netlink_callback *cb, enum addr_type_t type,
+ int s_ip_idx, int *p_ip_idx)
+{
+ struct inet6_ifaddr *ifa;
+ struct ifmcaddr6 *ifmca;
+ struct ifacaddr6 *ifaca;
+ int err = 1;
+ int ip_idx = *p_ip_idx;
+
+ read_lock_bh(&idev->lock);
+ switch (type) {
+ case UNICAST_ADDR:
+ /* unicast address incl. temp addr */
+ for (ifa = idev->addr_list; ifa;
+ ifa = ifa->if_next, ip_idx++) {
+ if (ip_idx < s_ip_idx)
+ continue;
+ err = inet6_fill_ifaddr(skb, ifa,
+ NETLINK_CB(cb->skb).pid,
+ cb->nlh->nlmsg_seq,
+ RTM_NEWADDR,
+ NLM_F_MULTI);
+ if (err <= 0)
+ break;
+ }
+ break;
+ case MULTICAST_ADDR:
+ /* multicast address */
+ for (ifmca = idev->mc_list; ifmca;
+ ifmca = ifmca->next, ip_idx++) {
+ if (ip_idx < s_ip_idx)
+ continue;
+ err = inet6_fill_ifmcaddr(skb, ifmca,
+ NETLINK_CB(cb->skb).pid,
+ cb->nlh->nlmsg_seq,
+ RTM_GETMULTICAST,
+ NLM_F_MULTI);
+ if (err <= 0)
+ break;
+ }
+ break;
+ case ANYCAST_ADDR:
+ /* anycast address */
+ for (ifaca = idev->ac_list; ifaca;
+ ifaca = ifaca->aca_next, ip_idx++) {
+ if (ip_idx < s_ip_idx)
+ continue;
+ err = inet6_fill_ifacaddr(skb, ifaca,
+ NETLINK_CB(cb->skb).pid,
+ cb->nlh->nlmsg_seq,
+ RTM_GETANYCAST,
+ NLM_F_MULTI);
+ if (err <= 0)
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+ read_unlock_bh(&idev->lock);
+ *p_ip_idx = ip_idx;
+ return err;
+}
+
static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
enum addr_type_t type)
{
+ struct net *net = sock_net(skb->sk);
+ int h, s_h;
int idx, ip_idx;
int s_idx, s_ip_idx;
- int err = 1;
struct net_device *dev;
- struct inet6_dev *idev = NULL;
- struct inet6_ifaddr *ifa;
- struct ifmcaddr6 *ifmca;
- struct ifacaddr6 *ifaca;
- struct net *net = sock_net(skb->sk);
+ struct inet6_dev *idev;
+ struct hlist_head *head;
+ struct hlist_node *node;
- s_idx = cb->args[0];
- s_ip_idx = ip_idx = cb->args[1];
+ s_h = cb->args[0];
+ s_idx = idx = cb->args[1];
+ s_ip_idx = ip_idx = cb->args[2];
- idx = 0;
- for_each_netdev(net, dev) {
- if (idx < s_idx)
- goto cont;
- if (idx > s_idx)
- s_ip_idx = 0;
- ip_idx = 0;
- if ((idev = in6_dev_get(dev)) == NULL)
- goto cont;
- read_lock_bh(&idev->lock);
- switch (type) {
- case UNICAST_ADDR:
- /* unicast address incl. temp addr */
- for (ifa = idev->addr_list; ifa;
- ifa = ifa->if_next, ip_idx++) {
- if (ip_idx < s_ip_idx)
- continue;
- err = inet6_fill_ifaddr(skb, ifa,
- NETLINK_CB(cb->skb).pid,
- cb->nlh->nlmsg_seq,
- RTM_NEWADDR,
- NLM_F_MULTI);
- if (err <= 0)
- break;
- }
- break;
- case MULTICAST_ADDR:
- /* multicast address */
- for (ifmca = idev->mc_list; ifmca;
- ifmca = ifmca->next, ip_idx++) {
- if (ip_idx < s_ip_idx)
- continue;
- err = inet6_fill_ifmcaddr(skb, ifmca,
- NETLINK_CB(cb->skb).pid,
- cb->nlh->nlmsg_seq,
- RTM_GETMULTICAST,
- NLM_F_MULTI);
- if (err <= 0)
- break;
- }
- break;
- case ANYCAST_ADDR:
- /* anycast address */
- for (ifaca = idev->ac_list; ifaca;
- ifaca = ifaca->aca_next, ip_idx++) {
- if (ip_idx < s_ip_idx)
- continue;
- err = inet6_fill_ifacaddr(skb, ifaca,
- NETLINK_CB(cb->skb).pid,
- cb->nlh->nlmsg_seq,
- RTM_GETANYCAST,
- NLM_F_MULTI);
- if (err <= 0)
- break;
- }
- break;
- default:
- break;
- }
- read_unlock_bh(&idev->lock);
- in6_dev_put(idev);
+ rcu_read_lock();
+ for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) {
+ idx = 0;
+ head = &net->dev_index_head[h];
+ hlist_for_each_entry_rcu(dev, node, head, index_hlist) {
+ if (idx < s_idx)
+ goto cont;
+ if (idx > s_idx)
+ s_ip_idx = 0;
+ ip_idx = 0;
+ if ((idev = __in6_dev_get(dev)) == NULL)
+ goto cont;
- if (err <= 0)
- break;
+ if (in6_dump_addrs(idev, skb, cb, type,
+ s_ip_idx, &ip_idx) <= 0)
+ goto done;
cont:
- idx++;
+ idx++;
+ }
}
- cb->args[0] = idx;
- cb->args[1] = ip_idx;
+done:
+ rcu_read_unlock();
+ cb->args[0] = h;
+ cb->args[1] = idx;
+ cb->args[2] = ip_idx;
+
return skb->len;
}
^ permalink raw reply related
* Re: [PATCH 04/10] AOE: use rcu to find network device
From: Ed Cashin @ 2009-11-12 14:33 UTC (permalink / raw)
To: shemminger, karaluh, ecashin, roel.kluin, harvey.harrison,
bzolnier, netdev
In-Reply-To: <20091110155316.2c3d7b6e@nehalam>
Thanks again for providing this patch to help get things started.
It's very helpful. I appreciate the way it reflects and fits into the
rest of the driver, too.
In the patch, there's a loop around getif, ejectif inside the new
aoecmd_flushnet function:
void aoecmd_flushnet(struct aoedev *d, struct net_device *nd)
{
struct aoetgt **tt, **te;
tt = d->targets;
te = tt + NTARGETS;
for (; tt < te && *tt; tt++) {
struct aoetgt *t = *tt;
struct aoeif *ifp;
while ( (ifp = getif(t, nd)) )
ejectif(t, ifp);
}
}
... but an "if" seems appropriate, since duplicates are avoided when
network devices are added in aoecmd_cfg_rsp:
ifp = getif(t, skb->dev);
if (!ifp) {
ifp = addif(t, skb->dev);
If there's some other reason for it to be "while" in aoecmd_flushnet
that I'm missing, please let me know.
Regarding the new aoe_device_event function,
/* Callback on change of state of network device. */
static int aoe_device_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
struct net_device *nd = ptr;
if (is_aoe_netif(nd) && event == NETDEV_UNREGISTER)
aoedev_ejectnet(nd);
return NOTIFY_DONE;
}
... the is_aoe_netif function really answers the question, "Are we
allowed by the user to do AoE on this local network interface?" The
user can modify the list of allowable interfaces at runtime via sysfs,
as it's a module parameter. So I don't think we can use is_aoe_netif
in the new aoe_device_event function. We should eject a net_device in
this handler even if the user has decided not to use it for AoE.
Please let me know if I'm missing the point.
You said that,
> It needs to:
>
> 1. Get a device ref count when it remembers a device: (ie addif)
> 2. Install a notifier that looks for device removal events
> 3. In notifier, remove interface, including flushing all pending
> skb's for that device.
For 3, does the starter patch flush all pending skbs? Perhaps you
could elaborate on what you had in mind?
I am trying to find the best way for the aoe driver to handle the
situation where ther are no usable local network interfaces. It does
seem to me that the user would benefit from a notice letting them know
that they're trying to do AoE without any usable ethernet. I'm
thinking that doing something like a printk_once would be appropriate.
(But probably not printk_once itself, because if they add a local
interface and then it goes away later, they should be told again that
something's wrong.)
--
Ed Cashin <ecashin@coraid.com>
http://www.coraid.com/
http://noserose.net/e/
^ permalink raw reply
* [PATCH]NET:PCNET32: poll method return 0 when done
From: Figo.zhang @ 2009-11-11 15:11 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev
Poll method return 0 when it is done.
Signed-off-by: Figo.zhang <figo1802@gmail.com>
---
drivers/net/pcnet32.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index c1b3f09..58894a3 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c
@@ -1396,6 +1396,7 @@ static int pcnet32_poll(struct napi_struct *napi, int budget)
spin_unlock_irqrestore(&lp->lock, flags);
if (work_done < budget) {
+ work_done = 0;
spin_lock_irqsave(&lp->lock, flags);
__napi_complete(napi);
^ permalink raw reply related
* Re: [PATCH] ifb: add multi-queue support
From: Patrick McHardy @ 2009-11-12 15:10 UTC (permalink / raw)
To: Changli Gao
Cc: David S. Miller, Stephen Hemminger, Eric Dumazet, Tom Herbert,
netdev
In-Reply-To: <412e6f7f0911111912q27f2b0aate56c637349292c3f@mail.gmail.com>
Changli Gao wrote:
> On Wed, Nov 11, 2009 at 11:59 PM, Patrick McHardy <kaber@trash.net> wrote:
>>> +static int numtxqs = 1;
>>> +module_param(numtxqs, int, 0444);
>>> +MODULE_PARM_DESC(numtxqs, "Number of TX queues per ifb");
>> unsigned?
>
> Yea, unsigned is better, and I need to check whether its value is
> smaller than 1 somewhere. The same will be done with IFLA_NTXQ.
Good point.
>>> + rcu_read_lock();
>>> + skb->dev = dev_get_by_index_rcu(&init_net, skb->iif);
>>> + if (!skb->dev) {
>>> + rcu_read_unlock();
>>> + dev_kfree_skb(skb);
>>> + txq->tx_dropped++;
>>> + break;
>>> + }
>>> + rcu_read_unlock();
>>> + skb->iif = dev->ifindex;
>> What protects the device from disappearing here and below during
>> dev_queue_xmit() and netif_rx_ni()?
>
> For dev_queue_xmit(), dev is holded by skb->_dst, so there is no
> problem. But for netif_rx_ni(), I don't know how to prevent the device
> disappearing, and it seems that all the NIC drivers have this problem.
> Maybe there was the assumption about the execution context of
> netif_rx() before. Now softirq can't be executed by softirqd, so the
> packet receiving path maybe interleaved. I don't know how to prevent
> it happening.
You can either take a reference of move the rcu_read_unlock()
after the skb submission.
>>> + break;
>>> + case __constant_htons(ETH_P_IPV6):
>>> + ip_proto = ipv6_hdr(skb)->nexthdr;
>>> + addr1 = ipv6_hdr(skb)->saddr.s6_addr32[3];
>>> + addr2 = ipv6_hdr(skb)->daddr.s6_addr32[3];
>>> + ihl = 10;
>> Where does 10 come from?
>
> It should be 40, after reviewing IPV6, I found that I need to loop
> until finding the right protocol value.
There is a helper for this (ipv6_skip_exthdr).
>>> +static int ifb_get_tx_queues(struct net *net, struct nlattr *tb[],
>>> + unsigned int *num_tx_queues,
>>> + unsigned int *real_num_tx_queues)
>>> +{
>>> + if (tb[IFLA_NTXQ]) {
>>> + *num_tx_queues = nla_get_u16(tb[IFLA_NTXQ]);
>> We currently use unsigned ints for the queue number, so please
>> use an u32 for the attribute as well.
>>
>>> + *real_num_tx_queues = *num_tx_queues;
>>> + } else {
>>> + *num_tx_queues = numtxqs;
>>> + *real_num_tx_queues = numtxqs;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
> u16 (*ndo_select_queue)(struct net_device *dev,
> struct sk_buff *skb);
> use u16 as the return value so ..., and I think u16 is big enough. If
> you insist on this, I'll use u32 instead.
Well, get_tx_queues() uses unsigned int, as does struct net_device.
I agree that it probably won't ever be needed, but there's no harm
in using the correct type, the attribute encoding doesn't even need
more room.
^ permalink raw reply
* [PATCH] inet: fix inet_bind_bucket_for_each
From: Lucian Adrian Grijincu @ 2009-11-12 15:07 UTC (permalink / raw)
To: netdev, Octavian Purdila
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
The first "node" is supposed to be the cursor used in the for_each.
The second "node" is ment literally and should not be macro expanded:
it's the name of the hlist_node field from the inet_bind_bucket.
This currently works because when inet_bind_bucket_for_each is called
it's argument is still "node".
Signed-off-by: Lucian Adrian Grijincu <lgrijincu@ixiacom.com>
---
include/net/inet_hashtables.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
[-- Attachment #2: 0001-inet-fix-inet_bind_bucket_for_each.patch --]
[-- Type: text/plain, Size: 542 bytes --]
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index 5b698b3..41cbddd 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -92,8 +92,8 @@ static inline struct net *ib_net(struct inet_bind_bucket *ib)
return read_pnet(&ib->ib_net);
}
-#define inet_bind_bucket_for_each(tb, node, head) \
- hlist_for_each_entry(tb, node, head, node)
+#define inet_bind_bucket_for_each(tb, pos, head) \
+ hlist_for_each_entry(tb, pos, head, node)
struct inet_bind_hashbucket {
spinlock_t lock;
^ permalink raw reply related
* Re: [PATCH] ifb: add multi-queue support
From: Patrick McHardy @ 2009-11-12 15:11 UTC (permalink / raw)
To: Changli Gao
Cc: David S. Miller, Stephen Hemminger, Eric Dumazet, Tom Herbert,
netdev
In-Reply-To: <412e6f7f0911120148r48fa553bt610e013d96522a3d@mail.gmail.com>
Changli Gao wrote:
> The corresponding iprout2 patch is attached.
Printing the value during dumps is missing from this patch.
^ permalink raw reply
* Re: [patch] Fix: 'return -ENOMEM' instead of 'return ENOMEM'
From: Mike Christie @ 2009-11-12 15:10 UTC (permalink / raw)
To: roel kluin
Cc: Randy Dunlap, Stephen M. Cameron, David Airlie, Jens Axboe,
Evgeniy Polyakov, iss_storagedev, Eric Dumazet, Joel Becker,
Jeff Liu, Andy Whitcroft, Dave Airlie, Ingo Molnar, dri-devel,
Alexey Dobriyan, Mike Miller, Mark Fasheh, Karsten Keil, rostedt,
Karen Xie, James Bottomley, James E.J. Bottomley, Hannes Reinecke,
Andreas Eversberg, Hannes Eder, Keith
In-Reply-To: <25e057c00911120131q577f18c5v479b9d6f5f8616a6@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 927 bytes --]
roel kluin wrote:
>> * Andrew Morton <akpm@linux-foundation.org> wrote:
>
>>> Andy, can we have a checkpatch rule please?
>> Note, that will upset creative uses of error codes i guess, such as
>> fs/xfs/.
>>
>> But yeah, +1 from me too.
>>
>> Ob'post'mortem - looked for similar patterns in the kernel and there's
>> quite a few bugs there:
>>
>> include/net/inet_hashtables.h: return ENOMEM; # bug
>> drivers/scsi/aic7xxx/aic7xxx_osm.c: return ENOMEM; # works but weird
>> drivers/scsi/cxgb3i/cxgb3i_offload.c: return ENOMEM; # works but weird
I think cxgb3i is actually in the buggy category. cxgb3i_c3cn_send_pdus
can propagate the positive EXYZ error value to other functions which
assume that errors are negative.
Karen, I made the attached patch over James's scsi-rc-fixes tree while
reviewing the code. Could you test, finish up and send upstream?
[-- Attachment #2: cxgb3i-use-negative-errno.patch --]
[-- Type: text/x-patch, Size: 2523 bytes --]
diff --git a/drivers/scsi/cxgb3i/cxgb3i_offload.c b/drivers/scsi/cxgb3i/cxgb3i_offload.c
index c1d5be4..f6a1fb9 100644
--- a/drivers/scsi/cxgb3i/cxgb3i_offload.c
+++ b/drivers/scsi/cxgb3i/cxgb3i_offload.c
@@ -291,7 +291,7 @@ static void act_open_req_arp_failure(struct t3cdev *dev, struct sk_buff *skb)
c3cn_hold(c3cn);
spin_lock_bh(&c3cn->lock);
if (c3cn->state == C3CN_STATE_CONNECTING)
- fail_act_open(c3cn, EHOSTUNREACH);
+ fail_act_open(c3cn, -EHOSTUNREACH);
spin_unlock_bh(&c3cn->lock);
c3cn_put(c3cn);
__kfree_skb(skb);
@@ -792,18 +792,18 @@ static int act_open_rpl_status_to_errno(int status)
{
switch (status) {
case CPL_ERR_CONN_RESET:
- return ECONNREFUSED;
+ return -ECONNREFUSED;
case CPL_ERR_ARP_MISS:
- return EHOSTUNREACH;
+ return -EHOSTUNREACH;
case CPL_ERR_CONN_TIMEDOUT:
- return ETIMEDOUT;
+ return -ETIMEDOUT;
case CPL_ERR_TCAM_FULL:
- return ENOMEM;
+ return -ENOMEM;
case CPL_ERR_CONN_EXIST:
cxgb3i_log_error("ACTIVE_OPEN_RPL: 4-tuple in use\n");
- return EADDRINUSE;
+ return -EADDRINUSE;
default:
- return EIO;
+ return -EIO;
}
}
@@ -817,7 +817,7 @@ static void act_open_retry_timer(unsigned long data)
spin_lock_bh(&c3cn->lock);
skb = alloc_skb(sizeof(struct cpl_act_open_req), GFP_ATOMIC);
if (!skb)
- fail_act_open(c3cn, ENOMEM);
+ fail_act_open(c3cn, -ENOMEM);
else {
skb->sk = (struct sock *)c3cn;
set_arp_failure_handler(skb, act_open_req_arp_failure);
@@ -966,14 +966,14 @@ static int abort_status_to_errno(struct s3_conn *c3cn, int abort_reason,
case CPL_ERR_BAD_SYN: /* fall through */
case CPL_ERR_CONN_RESET:
return c3cn->state > C3CN_STATE_ESTABLISHED ?
- EPIPE : ECONNRESET;
+ -EPIPE : -ECONNRESET;
case CPL_ERR_XMIT_TIMEDOUT:
case CPL_ERR_PERSIST_TIMEDOUT:
case CPL_ERR_FINWAIT2_TIMEDOUT:
case CPL_ERR_KEEPALIVE_TIMEDOUT:
- return ETIMEDOUT;
+ return -ETIMEDOUT;
default:
- return EIO;
+ return -EIO;
}
}
diff --git a/drivers/scsi/cxgb3i/cxgb3i_pdu.c b/drivers/scsi/cxgb3i/cxgb3i_pdu.c
index 7091050..1fe3b0f 100644
--- a/drivers/scsi/cxgb3i/cxgb3i_pdu.c
+++ b/drivers/scsi/cxgb3i/cxgb3i_pdu.c
@@ -388,8 +388,8 @@ int cxgb3i_conn_xmit_pdu(struct iscsi_task *task)
if (err > 0) {
int pdulen = err;
- cxgb3i_tx_debug("task 0x%p, skb 0x%p, len %u/%u, rv %d.\n",
- task, skb, skb->len, skb->data_len, err);
+ cxgb3i_tx_debug("task 0x%p, skb 0x%p, len %u/%u, rv %d.\n",
+ task, skb, skb->len, skb->data_len, err);
if (task->conn->hdrdgst_en)
pdulen += ISCSI_DIGEST_SIZE;
[-- Attachment #3: Type: text/plain, Size: 354 bytes --]
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
[-- Attachment #4: Type: text/plain, Size: 161 bytes --]
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
^ permalink raw reply related
* [PATCH] can: add the missing netlink get_xstats_size callback
From: Wolfgang Grandegger @ 2009-11-12 15:34 UTC (permalink / raw)
To: Linux Netdev List; +Cc: SocketCAN Core Mailing List
This patch adds the missing "get_xstats_size" callback for the
netlink interface, which is required if "fill_xstats" is used,
as pointed out by Patrick McHardy.
Signed-off-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
---
drivers/net/can/dev.c | 6 ++++++
1 file changed, 6 insertions(+)
Index: net-next-2.6/drivers/net/can/dev.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/dev.c
+++ net-next-2.6/drivers/net/can/dev.c
@@ -677,6 +677,11 @@ nla_put_failure:
return -EMSGSIZE;
}
+static size_t can_get_xstats_size(const struct net_device *dev)
+{
+ return sizeof(struct can_device_stats);
+}
+
static int can_fill_xstats(struct sk_buff *skb, const struct net_device *dev)
{
struct can_priv *priv = netdev_priv(dev);
@@ -705,6 +710,7 @@ static struct rtnl_link_ops can_link_ops
.changelink = can_changelink,
.get_size = can_get_size,
.fill_info = can_fill_info,
+ .get_xstats_size = can_get_xstats_size,
.fill_xstats = can_fill_xstats,
};
^ permalink raw reply
* [PATCH] can: Add missing debug flag for making USB CAN devices
From: Wolfgang Grandegger @ 2009-11-12 15:36 UTC (permalink / raw)
To: Linux Netdev List; +Cc: SocketCAN Core Mailing List
Signed-off-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
---
drivers/net/can/usb/Makefile | 2 ++
1 file changed, 2 insertions(+)
Index: net-next-2.6/drivers/net/can/usb/Makefile
===================================================================
--- net-next-2.6.orig/drivers/net/can/usb/Makefile
+++ net-next-2.6/drivers/net/can/usb/Makefile
@@ -3,3 +3,5 @@
#
obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
+
+ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
^ permalink raw reply
* 3% Offer
From: David Rosenblatt @ 2009-11-12 15:30 UTC (permalink / raw)
Are you in need of a loan? We offer loan as low as 3%, If interested contact us via E-mail.
^ permalink raw reply
* Re: [PATCH v2] Documentation: rw_lock lessons learned
From: Linus Torvalds @ 2009-11-12 15:40 UTC (permalink / raw)
To: William Allen Simpson
Cc: Stephen Hemminger, Paul E. McKenney, Linux Kernel Developers,
Linux Kernel Network Developers, Eric Dumazet
In-Reply-To: <4AFBEC44.9030409@gmail.com>
On Thu, 12 Nov 2009, William Allen Simpson wrote:
>
> In recent weeks, two different network projects erroneously
> strayed down the rw_lock path. Update the Documentation
> based upon comments by Eric Dumazet and Paul E. McKenney in
> those threads.
This still retains some pretty old and bogus language. For example, that
file still talks about spinlocks being "faster than a global interrupt
lock", which is kind of amusing - because we've not done that global
interrupt lock thing for the last ten years or so.
So I certainly agree with discouraging rwlocks - I don't think we've ever
really found a situation where they are useful except for some really
special cases - but I suspect the thing needs a bigger overhaul.
I also suspect somebody should actually take a look at our _users_ of
rwlocks. We have a few fairly central ones like 'tasklist_lock', and it
may be an example of a _good_ case of rwlocks, but for a very non-obvious
reason.
In the case of 'tasklist_lock', the magic subtle reason that makes it a
good idea is that that lock is commonly taken for reading in _interrupts_.
And the way rwlocks are defined, that means that you can take it for
reading without doing the *_irq() or *_irqsave() versions, because rwlocks
are not fair, so an active reader will never block a new reader even if
there is a blocked writer pending.
So for tasklist_lock, raw rwlocks are still slower than raw spinlocks, but
because of the rwlock semantics the common case doesn't need to disable
and enable interrupts, so for the common case the comparison is not "raw
rwlock vs raw spinlock", but "raw rwlock vs interrupt-disabling spinlock",
and then it turns out rwlocks tend to win again.
(Of course, lock_write() needs to disable interrupts for tasklist_lock,
but that tends to be the uncommon case).
Ho humm..
Linus
^ permalink raw reply
* Re: [PATCH 0/4] vbus: venet macvlan support
From: Gregory Haskins @ 2009-11-12 15:44 UTC (permalink / raw)
To: Patrick Mullaney
Cc: alacrityvm-devel, linux-kernel, kaber, arnd, bridge, evb, netdev
In-Reply-To: <20091110222632.24100.14884.stgit@mimic.site>
[-- Attachment #1: Type: text/plain, Size: 340 bytes --]
Patrick Mullaney wrote:
> (Applies to alacrityvm.git/master:34534534)
>
> This patchset implements a vbus venet device with a
> macvlan backend.
Thanks Pat, applied.
If possible, please submit a patch for the userspace side "-net
venet-macvlan[,macaddr][,lower-devname]" feature ASAP and I will merge
that as well.
-Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] vbus: venet macvlan support
From: Patrick McHardy @ 2009-11-12 15:53 UTC (permalink / raw)
To: Gregory Haskins
Cc: Patrick Mullaney, alacrityvm-devel, linux-kernel, arnd, bridge,
evb, netdev
In-Reply-To: <4AFC2D65.9040903@gmail.com>
Gregory Haskins wrote:
> Patrick Mullaney wrote:
>> (Applies to alacrityvm.git/master:34534534)
>>
>> This patchset implements a vbus venet device with a
>> macvlan backend.
>
> Thanks Pat, applied.
As I mentioned in my response to these patches, the macvlan part
need more work.
^ permalink raw reply
* Re: [PATCH 0/4] vbus: venet macvlan support
From: Gregory Haskins @ 2009-11-12 15:54 UTC (permalink / raw)
To: Patrick McHardy
Cc: Patrick Mullaney, alacrityvm-devel, linux-kernel, arnd, bridge,
evb, netdev
In-Reply-To: <4AFC2F71.1060408@trash.net>
[-- Attachment #1: Type: text/plain, Size: 787 bytes --]
Patrick McHardy wrote:
> Gregory Haskins wrote:
>> Patrick Mullaney wrote:
>>> (Applies to alacrityvm.git/master:34534534)
>>>
>>> This patchset implements a vbus venet device with a
>>> macvlan backend.
>> Thanks Pat, applied.
>
> As I mentioned in my response to these patches, the macvlan part
> need more work.
Yeah, I talked to Pat offline. He is going to patch it incrementally as
its not a trivial problem to cleanup. For the time being, they are
sitting in the alacrityvm tree, but they are not going upstream until
the macvlan stuff is resolved. All but the last patch are not really my
business to push up, anyway.
In the meantime, Pat or others can enhance the work that he's done to
date, so I will carry them out-of-tree.
Kind Regards,
-Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]
^ permalink raw reply
* Re: [PATCH 0/4] vbus: venet macvlan support
From: Patrick McHardy @ 2009-11-12 15:59 UTC (permalink / raw)
To: Gregory Haskins
Cc: Patrick Mullaney, alacrityvm-devel, linux-kernel, arnd, bridge,
evb, netdev
In-Reply-To: <4AFC2FCC.5090103@gmail.com>
Gregory Haskins wrote:
> Patrick McHardy wrote:
>> Gregory Haskins wrote:
>>> Patrick Mullaney wrote:
>>>> (Applies to alacrityvm.git/master:34534534)
>>>>
>>>> This patchset implements a vbus venet device with a
>>>> macvlan backend.
>>> Thanks Pat, applied.
>> As I mentioned in my response to these patches, the macvlan part
>> need more work.
>
> Yeah, I talked to Pat offline. He is going to patch it incrementally as
> its not a trivial problem to cleanup. For the time being, they are
> sitting in the alacrityvm tree, but they are not going upstream until
> the macvlan stuff is resolved.
I see, thanks.
^ permalink raw reply
* re
From: David Rosenblatt @ 2009-11-12 16:08 UTC (permalink / raw)
Are you in need of a loan? We offer loan as low as 3%, If interested contact us via E-mail.
^ permalink raw reply
* Re: [PATCH]NET:PCNET32: poll method return 0 when done
From: Don Fry @ 2009-11-12 16:13 UTC (permalink / raw)
To: Figo.zhang; +Cc: David S. Miller, netdev
In-Reply-To: <1257952296.1920.3.camel@myhost>
I do not understand why you are suggesting this change. With NAPI
the amount of work done is returned on exit, not zero. All other
drivers I have looked at do not force a zero on exit.
NAK
Don
-----Original Message-----
From: Figo.zhang <figo1802@gmail.com>
To: David S. Miller <davem@davemloft.net>
Cc: netdev <netdev@vger.kernel.org>
Subject: [PATCH]NET:PCNET32: poll method return 0 when done
Date: Wed, 11 Nov 2009 23:11:36 +0800
Poll method return 0 when it is done.
Signed-off-by: Figo.zhang <figo1802@gmail.com>
---
drivers/net/pcnet32.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/pcnet32.c b/drivers/net/pcnet32.c
index c1b3f09..58894a3 100644
--- a/drivers/net/pcnet32.c
+++ b/drivers/net/pcnet32.c
@@ -1396,6 +1396,7 @@ static int pcnet32_poll(struct napi_struct *napi, int budget)
spin_unlock_irqrestore(&lp->lock, flags);
if (work_done < budget) {
+ work_done = 0;
spin_lock_irqsave(&lp->lock, flags);
__napi_complete(napi);
--
To unsubscribe from this list: send the line "unsubscribe netdev" 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 related
* Re: [PATCH 1/2] alchemy: add au1000-eth platform device
From: Ralf Baechle @ 2009-11-12 16:41 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, David Miller, linux-mips
In-Reply-To: <200911100113.31471.florian@openwrt.org>
On Tue, Nov 10, 2009 at 01:13:30AM +0100, Florian Fainelli wrote:
> (resending per Ralf's request as the patch had some checkpatch errors)
You missed the trailing whitespace ... Anyway, fixed that up and queued
it for 2.6.33.
Ralf
^ permalink raw reply
* Re: [PATCH 2/2] au1000-eth: convert to platform_driver model
From: Ralf Baechle @ 2009-11-12 16:42 UTC (permalink / raw)
To: Florian Fainelli; +Cc: linux-mips, netdev, David Miller
In-Reply-To: <200911100113.38685.florian@openwrt.org>
On Tue, Nov 10, 2009 at 01:13:38AM +0100, Florian Fainelli wrote:
>
> This patch converts the au1000-eth driver to become a full
> platform-driver as it ought to be. We now pass PHY-speficic
> configurations through platform_data but for compatibility
> the driver still assumes the default settings (search for PHY1 on
> MAC0) when no platform_data is passed. Tested on my MTX-1 board.
>
> Acked-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
Queued for 2.6.33. Thanks everybody!
Ralf
^ permalink raw reply
* Re: [RACE] net: in process_backlog
From: Stephen Hemminger @ 2009-11-12 16:57 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, Patrick McHardy, netdev
In-Reply-To: <412e6f7f0911120050w740377c7j2cdf24ef9fd2ca59@mail.gmail.com>
On Thu, 12 Nov 2009 16:50:53 +0800
Changli Gao <xiaosuo@gmail.com> wrote:
> Dear Stephen:
>
> I don't think this change
> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commitdiff;h=6e583ce5242f32e925dcb198f7123256d0798370
> is correct.
>
> local_irq_enable();
> break;
> }
> -
> local_irq_enable();
>
> - dev = skb->dev;
> -
> on MP system, flush_backlog() will be called here, and after that
> skb->dev will be invalid, if we access it, sth. unexpected may
> happens.
>
> netif_receive_skb(skb);
> -
> - dev_put(dev);
> } while (++work < quota && jiffies == start_time);
>
> return work;
There is are a couple of issues here, but it is not what you thought
you saw.
The receive process is always done in soft IRQ context. The backlog queue's
are per-cpu. When a device is deleted an IPI is sent to all cpu's to
scan there backlog queue. What should protect the skb is the fact that
the network device destruction process waits for an RCU grace period.
So skb->dev points to valid data.
BUT the flush_backlog is run too late in the device destruction process.
It should be moved out of netdev_run_todo, to right after dev_shutdown().
Also adding a check for skb->dev->reg_state in netif_receive_skb would
be wise to drop packets.
--
^ permalink raw reply
* Re: sysfs question
From: Stephen Hemminger @ 2009-11-12 17:00 UTC (permalink / raw)
To: Kurt Van Dijck; +Cc: Wolfgang Grandegger, netdev
In-Reply-To: <20091112103940.GC322@e-circ.dyndns.org>
On Thu, 12 Nov 2009 11:39:41 +0100
Kurt Van Dijck <kurt.van.dijck@eia.be> wrote:
> Hi,
>
> Within the socketcan project, we came into a situation that
> might benefit with input from the netdev mailing list.
>
> The main issue is the policy to add sysfs properties in
> /sys/class/net/canX.
>
> The reason for this is that cards (devices) with multiple network
> interfaces may require properties per network.
>
> An obvious property would be the 'channel number of the card'. Other
> properties could be things like type of output circuitry, ..., rather
> hardware specific.
>
> I see currently 3 options:
> 1) such properties in /sys/class/net/canX would be allowed.
> 2) such properties would belong in /sys/class/net/canX/<subdirectory tbd>/
> 3) such properties would go somewhere else.
>
> Some input with regard to sysfs policies would be helpfull.
>
> Kurt
It sounds like the property you are proposing is a property of the
upper network layer not the hardware. Putting more properties in sysfs
is good if is hardware related, but awkward if it is really a protocol
attribute.
^ 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