* [PATCH 2/2] hv: handle skb allocation failure
From: Stephen Hemminger @ 2010-03-10 1:42 UTC (permalink / raw)
To: Hank Janssen, Greg KH; +Cc: netdev
In-Reply-To: <20100309173855.7d297c44@nehalam>
Some fixes to receive handling:
* Dieing with assertion failure when running out of memory is not ok
* Use newer alloc function to get aligned skb
* Dropped statistic is supposed to be incremented only by
driver it was responsible for the drop.
Compile tested only.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
---
drivers/staging/hv/netvsc_drv.c | 32 ++++++++++++--------------------
1 file changed, 12 insertions(+), 20 deletions(-)
--- a/drivers/staging/hv/netvsc_drv.c 2010-03-09 17:27:57.263312289 -0800
+++ b/drivers/staging/hv/netvsc_drv.c 2010-03-09 17:34:45.012686682 -0800
@@ -292,7 +292,6 @@ static int netvsc_recv_callback(struct h
struct net_device_context *net_device_ctx;
struct sk_buff *skb;
void *data;
- int ret;
int i;
unsigned long flags;
@@ -306,12 +305,12 @@ static int netvsc_recv_callback(struct h
net_device_ctx = netdev_priv(net);
- /* Allocate a skb - TODO preallocate this */
- /* Pad 2-bytes to align IP header to 16 bytes */
- skb = dev_alloc_skb(packet->TotalDataBufferLength + 2);
- ASSERT(skb);
- skb_reserve(skb, 2);
- skb->dev = net;
+ /* Allocate a skb - TODO direct I/O to pages? */
+ skb = netdev_alloc_skb_ip_align(net, packet->TotalDataBufferLength);
+ if (unlikely(!skb)) {
+ ++net->stats.rx_dropped;
+ return 0;
+ }
/* for kmap_atomic */
local_irq_save(flags);
@@ -336,25 +335,18 @@ static int netvsc_recv_callback(struct h
local_irq_restore(flags);
skb->protocol = eth_type_trans(skb, net);
-
skb->ip_summed = CHECKSUM_NONE;
+ net->stats.rx_packets++;
+ net->stats.rx_bytes += skb->len;
+
/*
* Pass the skb back up. Network stack will deallocate the skb when it
- * is done
+ * is done.
+ * TODO - use NAPI?
*/
- ret = netif_rx(skb);
-
- switch (ret) {
- case NET_RX_DROP:
- net->stats.rx_dropped++;
- break;
- default:
- net->stats.rx_packets++;
- net->stats.rx_bytes += skb->len;
- break;
+ netif_rx(skb);
- }
DPRINT_DBG(NETVSC_DRV, "# of recvs %lu total size %lu",
net->stats.rx_packets, net->stats.rx_bytes);
^ permalink raw reply
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Paul E. McKenney @ 2010-03-10 2:14 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Herbert Xu, David S. Miller, netdev, Stephen Hemminger
In-Reply-To: <201003092212.59627.arnd@arndb.de>
On Tue, Mar 09, 2010 at 10:12:59PM +0100, Arnd Bergmann wrote:
> On Sunday 07 March 2010, Paul E. McKenney wrote:
> > On Sun, Mar 07, 2010 at 10:45:00AM +0800, Herbert Xu wrote:
> > > On Sat, Mar 06, 2010 at 11:00:00AM -0800, Paul E. McKenney wrote:
> >
> > Arnd, would it be reasonable to extend your RCU-sparse changes to have
> > four different pointer namespaces, one for each flavor of RCU? (RCU,
> > RCU-bh, RCU-sched, and SRCU)? Always a fan of making the computer do
> > the auditing where reasonable. ;-)
> >
> > This could potentially catch the mismatched call_rcu()s, at least if the
> > rcu_head could be labeled.
> >
> > Other thoughts?
>
> I've just tried annotating net/ipv4/route.c like this and did not get
> very far, because the same pointers are used for rcu and rcu_bh.
> Could you check if this is a false positive or an actual finding?
Hmmm... I am only seeing a call_rcu_bh() here, so unless I am missing
something, this is a real problem in TREE_PREEMPT_RCU kernels. The
call_rcu_bh() only interacts with the rcu_read_lock_bh() readers, not
the rcu_read_lock() readers.
One approach is to run freed blocks through both types of grace periods,
I suppose.
Thanx, Paul
^ permalink raw reply
* [PATCH net-2.6] ipv6 ip6_tunnel: eliminate unused recursion field from ip6_tnl{}.
From: YOSHIFUJI Hideaki @ 2010-03-10 2:47 UTC (permalink / raw)
To: davem, netdev; +Cc: yoshfuji
Commit a43912ab19... ("tunnel: eliminate recursion field") eliminated
use of recursion field from tunnel structures, but its definition
still exists in ip6_tnl{}.
Let's remove that unused field.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
include/net/ip6_tunnel.h | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/include/net/ip6_tunnel.h b/include/net/ip6_tunnel.h
index 83b4e00..fbf9d1c 100644
--- a/include/net/ip6_tunnel.h
+++ b/include/net/ip6_tunnel.h
@@ -15,7 +15,6 @@
struct ip6_tnl {
struct ip6_tnl *next; /* next tunnel in list */
struct net_device *dev; /* virtual device associated with tunnel */
- int recursion; /* depth of hard_start_xmit recursion */
struct ip6_tnl_parm parms; /* tunnel configuration parameters */
struct flowi fl; /* flowi template for xmit */
struct dst_entry *dst_cache; /* cached dst */
--
1.5.6.5
^ permalink raw reply related
* [PATCH 0/3] pci: fix/cleanup pcix get and set mmrbc functions
From: Dean Nelson @ 2010-03-10 3:26 UTC (permalink / raw)
To: jbarnes; +Cc: netdev, linux-pci
A customer running RHEL4.8 encountered
"e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang"
type errors, which were determined to be the result of a bad return value
from e1000_pcix_get_mmrbc() causing the call to e1000_pcix_set_mmrbc() to
be skipped in the following snippet of code from e1000_init_hw().
switch (hw->mac_type) {
case e1000_82545_rev_3:
case e1000_82546_rev_3:
break;
default:
/* Workaround for PCI-X problem when BIOS sets MMRBC incorrectly. */
if (hw->bus_type == e1000_bus_type_pcix
&& e1000_pcix_get_mmrbc(hw) > 2048)
e1000_pcix_set_mmrbc(hw, 2048);
break;
}
e1000_pcix_get_mmrbc() is basically a wrapper for a call to pcix_get_mmrbc().
e1000_pcix_set_mmrbc() is the same for pcix_set_mmrbc().
The following three patches are a response to the problems that were found to
exist with pcix_get_max_mmrbc(), pcix_get_mmrbc() and pcix_set_mmrbc().
Versions of these patches applicable to RHEL4 were verified by the customer to
solve their problem.
^ permalink raw reply
* [PATCH 1/3] pci: fix return value from pcix_get_max_mmrbc()
From: Dean Nelson @ 2010-03-10 3:26 UTC (permalink / raw)
To: jbarnes; +Cc: netdev, linux-pci
In-Reply-To: <20100310032632.6331.15414.send-patch@aqua>
For the PCI_X_STATUS register, pcix_get_max_mmrbc() is returning an incorrect
value, which is based on:
(stat & PCI_X_STATUS_MAX_READ) >> 12
Valid return values are 512, 1024, 2048, 4096, which correspond to a 'stat'
(masked and right shifted by 21) of 0, 1, 2, 3, respectively.
A right shift by 11 would generate the correct return value when 'stat' (masked
and right shifted by 21) has a value of 1 or 2. But for a value of 0 or 3 it's
not possible to generate the correct return value by only right shifting.
Fix is based on pcix_get_mmrbc()'s similar dealings with the PCI_X_CMD register.
Signed-off-by: Dean Nelson <dnelson@redhat.com>
---
drivers/pci/pci.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 5b548ae..1decd4f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2571,7 +2571,7 @@ int pcix_get_max_mmrbc(struct pci_dev *dev)
if (err)
return -EINVAL;
- return (stat & PCI_X_STATUS_MAX_READ) >> 12;
+ return 512 << ((stat & PCI_X_STATUS_MAX_READ) >> 21);
}
EXPORT_SYMBOL(pcix_get_max_mmrbc);
^ permalink raw reply related
* [PATCH 2/3] pci: fix access of PCI_X_CMD by pcix get and set mmrbc functions
From: Dean Nelson @ 2010-03-10 3:26 UTC (permalink / raw)
To: jbarnes; +Cc: netdev, linux-pci
In-Reply-To: <20100310032632.6331.15414.send-patch@aqua>
An e1000 driver on a system with a PCI-X bus was always being returned
a value of 135 from both pcix_get_mmrbc() and pcix_set_mmrbc(). This
value reflects an error return of PCIBIOS_BAD_REGISTER_NUMBER from
pci_bus_read_config_dword(,, cap + PCI_X_CMD,).
This is because for a dword, the following portion of the PCI_OP_READ()
macro:
if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER;
expands to:
if (pos & 3) return PCIBIOS_BAD_REGISTER_NUMBER;
And is always true for 'cap + PCI_X_CMD', which is 0xe4 + 2 = 0xe6. ('cap' is
the result of calling pci_find_capability(, PCI_CAP_ID_PCIX).)
The same problem exists for pci_bus_write_config_dword(,, cap + PCI_X_CMD,).
In both cases, instead of calling _dword(), _word() should be called.
Signed-off-by: Dean Nelson <dnelson@redhat.com>
---
drivers/pci/pci.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1decd4f..cdf201e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2585,13 +2585,13 @@ EXPORT_SYMBOL(pcix_get_max_mmrbc);
int pcix_get_mmrbc(struct pci_dev *dev)
{
int ret, cap;
- u32 cmd;
+ u16 cmd;
cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
if (!cap)
return -EINVAL;
- ret = pci_read_config_dword(dev, cap + PCI_X_CMD, &cmd);
+ ret = pci_read_config_word(dev, cap + PCI_X_CMD, &cmd);
if (!ret)
ret = 512 << ((cmd & PCI_X_CMD_MAX_READ) >> 2);
@@ -2611,7 +2611,8 @@ EXPORT_SYMBOL(pcix_get_mmrbc);
int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
{
int cap, err = -EINVAL;
- u32 stat, cmd, v, o;
+ u32 stat, v, o;
+ u16 cmd;
if (mmrbc < 512 || mmrbc > 4096 || !is_power_of_2(mmrbc))
goto out;
@@ -2629,7 +2630,7 @@ int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
if (v > (stat & PCI_X_STATUS_MAX_READ) >> 21)
return -E2BIG;
- err = pci_read_config_dword(dev, cap + PCI_X_CMD, &cmd);
+ err = pci_read_config_word(dev, cap + PCI_X_CMD, &cmd);
if (err)
goto out;
@@ -2641,7 +2642,7 @@ int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
cmd &= ~PCI_X_CMD_MAX_READ;
cmd |= v << 2;
- err = pci_write_config_dword(dev, cap + PCI_X_CMD, cmd);
+ err = pci_write_config_word(dev, cap + PCI_X_CMD, cmd);
}
out:
return err;
^ permalink raw reply related
* [PATCH 3/3] pci: cleanup error return for pcix get and set mmrbc functions
From: Dean Nelson @ 2010-03-10 3:26 UTC (permalink / raw)
To: jbarnes; +Cc: netdev, linux-pci
In-Reply-To: <20100310032632.6331.15414.send-patch@aqua>
pcix_get_mmrbc() returns the maximum memory read byte count (mmrbc), if
successful, or an appropriate error value, if not.
Distinguishing errors from correct values and understanding the meaning of an
error can be somewhat confusing in that:
correct values: 512, 1024, 2048, 4096
errors: -EINVAL -22
PCIBIOS_FUNC_NOT_SUPPORTED 0x81
PCIBIOS_BAD_VENDOR_ID 0x83
PCIBIOS_DEVICE_NOT_FOUND 0x86
PCIBIOS_BAD_REGISTER_NUMBER 0x87
PCIBIOS_SET_FAILED 0x88
PCIBIOS_BUFFER_TOO_SMALL 0x89
The PCIBIOS_ errors are returned from the PCI functions generated by the
PCI_OP_READ() and PCI_OP_WRITE() macros.
In a similar manner, pcix_set_mmrbc() also returns the PCIBIOS_ error values
returned from pci_read_config_[word|dword]() and pci_write_config_word().
Following pcix_get_max_mmrbc()'s example, the following patch simply returns
-EINVAL for all PCIBIOS_ errors encountered by pcix_get_mmrbc(), and -EINVAL
or -EIO for those encountered by pcix_set_mmrbc().
This simplification was chosen in light of the fact that none of the current
callers of these functions are interested in the specific type of error
encountered. In the future, should this change, one could simply create a
function that maps each PCIBIOS_ error to a corresponding unique errno value,
which could be called by pcix_get_max_mmrbc(), pcix_get_mmrbc(), and
pcix_set_mmrbc().
Additionally, this patch eliminates some unnecessary variables.
Signed-off-by: Dean Nelson <dnelson@redhat.com>
---
drivers/pci/pci.c | 36 ++++++++++++++++--------------------
1 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index cdf201e..5b6ed6f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2560,15 +2560,14 @@ EXPORT_SYMBOL_GPL(pci_reset_function);
*/
int pcix_get_max_mmrbc(struct pci_dev *dev)
{
- int err, cap;
+ int cap;
u32 stat;
cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
if (!cap)
return -EINVAL;
- err = pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat);
- if (err)
+ if (pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat))
return -EINVAL;
return 512 << ((stat & PCI_X_STATUS_MAX_READ) >> 21);
@@ -2584,18 +2583,17 @@ EXPORT_SYMBOL(pcix_get_max_mmrbc);
*/
int pcix_get_mmrbc(struct pci_dev *dev)
{
- int ret, cap;
+ int cap;
u16 cmd;
cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
if (!cap)
return -EINVAL;
- ret = pci_read_config_word(dev, cap + PCI_X_CMD, &cmd);
- if (!ret)
- ret = 512 << ((cmd & PCI_X_CMD_MAX_READ) >> 2);
+ if (pci_read_config_word(dev, cap + PCI_X_CMD, &cmd))
+ return -EINVAL;
- return ret;
+ return 512 << ((cmd & PCI_X_CMD_MAX_READ) >> 2);
}
EXPORT_SYMBOL(pcix_get_mmrbc);
@@ -2610,29 +2608,27 @@ EXPORT_SYMBOL(pcix_get_mmrbc);
*/
int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
{
- int cap, err = -EINVAL;
+ int cap;
u32 stat, v, o;
u16 cmd;
if (mmrbc < 512 || mmrbc > 4096 || !is_power_of_2(mmrbc))
- goto out;
+ return -EINVAL;
v = ffs(mmrbc) - 10;
cap = pci_find_capability(dev, PCI_CAP_ID_PCIX);
if (!cap)
- goto out;
+ return -EINVAL;
- err = pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat);
- if (err)
- goto out;
+ if (pci_read_config_dword(dev, cap + PCI_X_STATUS, &stat))
+ return -EINVAL;
if (v > (stat & PCI_X_STATUS_MAX_READ) >> 21)
return -E2BIG;
- err = pci_read_config_word(dev, cap + PCI_X_CMD, &cmd);
- if (err)
- goto out;
+ if (pci_read_config_word(dev, cap + PCI_X_CMD, &cmd))
+ return -EINVAL;
o = (cmd & PCI_X_CMD_MAX_READ) >> 2;
if (o != v) {
@@ -2642,10 +2638,10 @@ int pcix_set_mmrbc(struct pci_dev *dev, int mmrbc)
cmd &= ~PCI_X_CMD_MAX_READ;
cmd |= v << 2;
- err = pci_write_config_word(dev, cap + PCI_X_CMD, cmd);
+ if (pci_write_config_word(dev, cap + PCI_X_CMD, cmd))
+ return -EIO;
}
-out:
- return err;
+ return 0;
}
EXPORT_SYMBOL(pcix_set_mmrbc);
^ permalink raw reply related
* When best to submit patches with small changes in multiple files?
From: Frans Pop @ 2010-03-10 3:49 UTC (permalink / raw)
To: netdev
I have patches prepared to remove trailing spaces from kernel messages
for ./net and ./drivers/net (including wireless).
In total 15 patches with
81 files changed, 351 insertions(+), 351 deletions(-)
When would be the best time to submit:
- now, to be included in .34
- now, for -next
- any time
- near beginning of merge window (rebased against netdev-next)
- not at all ;-)
?
Cheers,
FJP
^ permalink raw reply
* Re: [PATCH net-2.6] ipv6 ip6_tunnel: eliminate unused recursion field from ip6_tnl{}.
From: Eric Dumazet @ 2010-03-10 4:48 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: davem, netdev
In-Reply-To: <201003100247.o2A2lqaX001898@94.43.138.210.xn.2iij.net>
Le mercredi 10 mars 2010 à 11:47 +0900, YOSHIFUJI Hideaki a écrit :
> Commit a43912ab19... ("tunnel: eliminate recursion field") eliminated
> use of recursion field from tunnel structures, but its definition
> still exists in ip6_tnl{}.
>
> Let's remove that unused field.
>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* [PATCH] net: Fix dev_mc_add()
From: Eric Dumazet @ 2010-03-10 5:09 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Jiri Pirko
Commit 6e17d45a (net: add addr len check to dev_mc_add)
added a bug in dev_mc_add(), since it can now exit with a lock
imbalance.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Jiri Pirko <jpirko@redhat.com>
---
diff --git a/net/core/dev_mcast.c b/net/core/dev_mcast.c
index fd91569..3dc295b 100644
--- a/net/core/dev_mcast.c
+++ b/net/core/dev_mcast.c
@@ -97,8 +97,9 @@ int dev_mc_add(struct net_device *dev, void *addr, int alen, int glbl)
netif_addr_lock_bh(dev);
if (alen != dev->addr_len)
- return -EINVAL;
- err = __dev_addr_add(&dev->mc_list, &dev->mc_count, addr, alen, glbl);
+ err = -EINVAL;
+ else
+ err = __dev_addr_add(&dev->mc_list, &dev->mc_count, addr, alen, glbl);
if (!err)
__dev_set_rx_mode(dev);
netif_addr_unlock_bh(dev);
^ permalink raw reply related
* [PATCH] net: Annotates neigh_invalidate()
From: Eric Dumazet @ 2010-03-10 5:40 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Annotates neigh_invalidate() with __releases() and __acquires() for
sparse sake.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index d102f6d..6cee643 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -771,6 +771,8 @@ static __inline__ int neigh_max_probes(struct neighbour *n)
}
static void neigh_invalidate(struct neighbour *neigh)
+ __releases(neigh->lock)
+ __acquires(neigh->lock)
{
struct sk_buff *skb;
^ permalink raw reply related
* [PATCH] net: remove rcu locking from fib_rules_event()
From: Eric Dumazet @ 2010-03-10 6:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Paul E. McKenney
We hold RTNL at this point and dont use RCU variants of list traversals,
we dont need rcu_read_lock()/rcu_read_unlock()
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/core/fib_rules.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 9a24377..2ff3489 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -108,7 +108,7 @@ fib_rules_register(struct fib_rules_ops *tmpl, struct net *net)
struct fib_rules_ops *ops;
int err;
- ops = kmemdup(tmpl, sizeof (*ops), GFP_KERNEL);
+ ops = kmemdup(tmpl, sizeof(*ops), GFP_KERNEL);
if (ops == NULL)
return ERR_PTR(-ENOMEM);
@@ -123,7 +123,6 @@ fib_rules_register(struct fib_rules_ops *tmpl, struct net *net)
return ops;
}
-
EXPORT_SYMBOL_GPL(fib_rules_register);
void fib_rules_cleanup_ops(struct fib_rules_ops *ops)
@@ -157,7 +156,6 @@ void fib_rules_unregister(struct fib_rules_ops *ops)
call_rcu(&ops->rcu, fib_rules_put_rcu);
}
-
EXPORT_SYMBOL_GPL(fib_rules_unregister);
static int fib_rule_match(struct fib_rule *rule, struct fib_rules_ops *ops,
@@ -220,7 +218,6 @@ out:
return err;
}
-
EXPORT_SYMBOL_GPL(fib_rules_lookup);
static int validate_rulemsg(struct fib_rule_hdr *frh, struct nlattr **tb,
@@ -613,7 +610,7 @@ static int fib_nl_dumprule(struct sk_buff *skb, struct netlink_callback *cb)
break;
cb->args[1] = 0;
- skip:
+skip:
idx++;
}
rcu_read_unlock();
@@ -685,7 +682,6 @@ static int fib_rules_event(struct notifier_block *this, unsigned long event,
struct fib_rules_ops *ops;
ASSERT_RTNL();
- rcu_read_lock();
switch (event) {
case NETDEV_REGISTER:
@@ -699,8 +695,6 @@ static int fib_rules_event(struct notifier_block *this, unsigned long event,
break;
}
- rcu_read_unlock();
-
return NOTIFY_DONE;
}
^ permalink raw reply related
* Re: [PATCH] can: bfin_can: switch to common Blackfin can header
From: Wolfgang Grandegger @ 2010-03-10 7:41 UTC (permalink / raw)
To: Mike Frysinger
Cc: Urs Thuermann, netdev-u79uwXL29TY76Z2rM5mHXA,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w, Oliver Hartkopp,
uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b,
David S. Miller
In-Reply-To: <1268150589-27123-1-git-send-email-vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Mike Frysinger wrote:
> The MMR bits are being moved to this header, so include it.
>
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Acked-by: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> ---
> note: this can be merged for 2.6.34 or 2.6.35
Is it required to make the driver working under 2.6.34?
Wolfgang.
^ permalink raw reply
* Re: [net-next-2.6 PATCH 0/3] bonding: refuse to change bond type if it's used V2
From: Jiri Pirko @ 2010-03-10 8:37 UTC (permalink / raw)
To: netdev; +Cc: fubar, bonding-devel, davem, shemminger
In-Reply-To: <20100309141724.GB2783@psychotron.lab.eng.brq.redhat.com>
Reposting.
Tue, Mar 09, 2010 at 03:17:24PM CET, jpirko@redhat.com wrote:
>Reworked version of patch/patchset. This time I use netdevice notifier as
>Stephen suggested. Changed the name of the original ones (NETDEV_BONDING_*TYPE)
>to be more general and reused them. Now the change is refused by brigde only.
>I will follow up with vlan soon.
>
>Jirka
^ permalink raw reply
* Re: [net-next PATCH v6 0/3] net: reserve ports for applications using fixed port numbers
From: Cong Wang @ 2010-03-10 9:23 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David Miller, opurdila, netdev, linux-kernel, nhorman,
eric.dumazet
In-Reply-To: <m1vddbdhmo.fsf@fess.ebiederm.org>
Eric W. Biederman wrote:
>
> I would add the restriction that the values in the list of ranges
> always must be increasing, and in general restrict the set of accepted
> values as much as possible. If we don't accept it now we don't have
> to worry about some userspace application relying on some unitended
> side effect a few years into the future.
I don't think this is good.
Suppose that when I just want to add one port into the list and keep the
original ones, I want to do this:
orig=$(cat ip_local_reserved_ports)
new_list="$orig, $new_one"
echo "$new_list" > ip_local_reserved_ports
If we add this restriction, the above could be failed if the new port
is lower than the original ones. This will be not convenient.
>
>
> I think it is a serious bug that you clear the destination bitmap
> in the middle of parsing it. That will either open or close all
> ports in the middle of parsing, and I can't see how that would
> ever be a good thing.
>
Agreed.
By the way, Octavian, any new updates?
^ permalink raw reply
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Arnd Bergmann @ 2010-03-10 9:41 UTC (permalink / raw)
To: paulmck; +Cc: Herbert Xu, David S. Miller, netdev, Stephen Hemminger
In-Reply-To: <20100310021410.GD6203@linux.vnet.ibm.com>
On Wednesday 10 March 2010 03:14:10 Paul E. McKenney wrote:
> On Tue, Mar 09, 2010 at 10:12:59PM +0100, Arnd Bergmann wrote:
>
> > I've just tried annotating net/ipv4/route.c like this and did not get
> > very far, because the same pointers are used for rcu and rcu_bh.
> > Could you check if this is a false positive or an actual finding?
>
> Hmmm... I am only seeing a call_rcu_bh() here, so unless I am missing
> something, this is a real problem in TREE_PREEMPT_RCU kernels. The
> call_rcu_bh() only interacts with the rcu_read_lock_bh() readers, not
> the rcu_read_lock() readers.
>
> One approach is to run freed blocks through both types of grace periods,
> I suppose.
Well, if I introduce different __rcu and __rcu_bh address space annotations,
sparse would still not like that, because then you can only pass the annotated
pointers into either rcu_dereference or rcu_dereference_bh.
What the code seems to be doing here is in some places
local_bh_disable();
...
rcu_read_lock();
rcu_dereference(rt_hash_table[h].chain);
rcu_read_unlock();
...
local_bh_enable();
and in others
rcu_read_lock_bh();
rcu_dereference_bh(rt_hash_table[h].chain);
rcu_read_unlock_bh();
When rt_hash_table[h].chain gets the __rcu_bh annotation, we'd have to
turn first rcu_dereference into rcu_dereference_bh in order to have a clean
build with sparse. Would that change be
a) correct from RCU perspective,
b) desirable for code inspection, and
c) lockdep-clean?
Arnd
^ permalink raw reply
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Eric Dumazet @ 2010-03-10 10:39 UTC (permalink / raw)
To: Arnd Bergmann
Cc: paulmck, Herbert Xu, David S. Miller, netdev, Stephen Hemminger
In-Reply-To: <201003101041.32518.arnd@arndb.de>
Le mercredi 10 mars 2010 à 10:41 +0100, Arnd Bergmann a écrit :
> On Wednesday 10 March 2010 03:14:10 Paul E. McKenney wrote:
> > On Tue, Mar 09, 2010 at 10:12:59PM +0100, Arnd Bergmann wrote:
> >
> > > I've just tried annotating net/ipv4/route.c like this and did not get
> > > very far, because the same pointers are used for rcu and rcu_bh.
> > > Could you check if this is a false positive or an actual finding?
> >
> > Hmmm... I am only seeing a call_rcu_bh() here, so unless I am missing
> > something, this is a real problem in TREE_PREEMPT_RCU kernels. The
> > call_rcu_bh() only interacts with the rcu_read_lock_bh() readers, not
> > the rcu_read_lock() readers.
> >
> > One approach is to run freed blocks through both types of grace periods,
> > I suppose.
>
> Well, if I introduce different __rcu and __rcu_bh address space annotations,
> sparse would still not like that, because then you can only pass the annotated
> pointers into either rcu_dereference or rcu_dereference_bh.
>
> What the code seems to be doing here is in some places
>
> local_bh_disable();
> ...
> rcu_read_lock();
> rcu_dereference(rt_hash_table[h].chain);
> rcu_read_unlock();
> ...
> local_bh_enable();
>
> and in others
>
> rcu_read_lock_bh();
> rcu_dereference_bh(rt_hash_table[h].chain);
> rcu_read_unlock_bh();
>
> When rt_hash_table[h].chain gets the __rcu_bh annotation, we'd have to
> turn first rcu_dereference into rcu_dereference_bh in order to have a clean
> build with sparse. Would that change be
> a) correct from RCU perspective,
> b) desirable for code inspection, and
> c) lockdep-clean?
>
Its really rcu_dereference_bh() that could/should be used:
I see no problem changing
local_bh_disable();
...
rcu_read_lock();
rcu_dereference(rt_hash_table[h].chain);
rcu_read_unlock();
...
local_bh_enable();
to
local_bh_disable();
...
rcu_read_lock();
rcu_dereference_bh(rt_hash_table[h].chain);
rcu_read_unlock();
...
local_bh_enable();
^ permalink raw reply
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Herbert Xu @ 2010-03-10 10:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: Arnd Bergmann, paulmck, David S. Miller, netdev,
Stephen Hemminger
In-Reply-To: <1268217583.2880.2.camel@edumazet-laptop>
On Wed, Mar 10, 2010 at 11:39:43AM +0100, Eric Dumazet wrote:
>
> Its really rcu_dereference_bh() that could/should be used:
> I see no problem changing
>
>
> local_bh_disable();
> ...
> rcu_read_lock();
> rcu_dereference(rt_hash_table[h].chain);
> rcu_read_unlock();
> ...
> local_bh_enable();
Why don't we just ignore the bh part for rcu_dereference?
After all it's call_rcu_bh and the other primitives that we really
care about. For rcu_dereference bh should make no difference
whatsoever.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: bug report: netpoll: allow execution of multiple rx_hooks per interface
From: Daniel Borkmann @ 2010-03-10 11:13 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Daniel Borkmann, netdev, David S. Miller
In-Reply-To: <20100306110659.GE4958@bicker>
[-- Attachment #1: Type: text/plain, Size: 922 bytes --]
Hi Dan,
Dan Carpenter wrote:
> Smatch complains about 508e14b4a4f: netpoll: allow execution of multiple
> rx_hooks per interface
>
> net/core/netpoll.c +840 netpoll_setup(124) error: potential null dereference 'npinfo'. (kmalloc returns null)
> 839 if (!ndev->npinfo) {
> 840 spin_lock_irqsave(&npinfo->rx_lock, flags);
>
> npinfo can be null here if the kmalloc() failed.
>
> 841 list_for_each_entry_safe(npe, tmp, &npinfo->rx_np, rx) {
> 842 npe->dev = NULL;
> 843 }
> 844 spin_unlock_irqrestore(&npinfo->rx_lock, flags);
> 845
> 846 kfree(npinfo);
> 847 }
Sorry for the late reply, I'm currently on holiday and have hardly
access to the Internet. I'll fix the problem as soon as possible.
You'll hear from me.
Thanks,
Daniel
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 261 bytes --]
^ permalink raw reply
* Re: [Uclinux-dist-devel] [PATCH] can: bfin_can: switch to common Blackfin can header
From: Mike Frysinger @ 2010-03-10 12:17 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Urs Thuermann, netdev, socketcan-core, Oliver Hartkopp,
uclinux-dist-devel, David S. Miller
In-Reply-To: <4B974D38.6000501@grandegger.com>
On Wed, Mar 10, 2010 at 02:41, Wolfgang Grandegger wrote:
> Mike Frysinger wrote:
>> The MMR bits are being moved to this header, so include it.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>
> Acked-by: Wolfgang Grandegger <wg@grandegger.com>
>
>> ---
>> note: this can be merged for 2.6.34 or 2.6.35
>
> Is it required to make the driver working under 2.6.34?
not for 2.6.34, but it will be for 2.6.35
-mike
^ permalink raw reply
* can SFQ and perturb break paquet ordering ?
From: Julien Vehent @ 2010-03-10 12:22 UTC (permalink / raw)
To: netdev
Hi netdev folks,
I was digging into the sch_sfq code and I was wondering the following (I
assume it's nothing new, I'm just trying to get it right):
Let's consider one UDP connection that has two packets. Packet#1 goes to
bucket#1, then perturb modifies the hashing algorithms and thus Packet#2
goes to bucket#800.
If the round robin was positionned after bucket#1 but before bucket#800
when perturb occured, then Packet#2 will be sent before Packet#1. And since
UDP can't reordered, it will be processed before on the received end.
Am I getting this right or is there something in the code that I missed ?
Thanks for your help.
Julien Vehent
^ permalink raw reply
* Re: can SFQ and perturb break paquet ordering ?
From: Patrick McHardy @ 2010-03-10 12:30 UTC (permalink / raw)
To: Julien Vehent; +Cc: netdev
In-Reply-To: <ca4f1a12b3ba9c81f3276c091063613f@localhost>
Julien Vehent wrote:
> Hi netdev folks,
>
> I was digging into the sch_sfq code and I was wondering the following (I
> assume it's nothing new, I'm just trying to get it right):
>
> Let's consider one UDP connection that has two packets. Packet#1 goes to
> bucket#1, then perturb modifies the hashing algorithms and thus Packet#2
> goes to bucket#800.
>
> If the round robin was positionned after bucket#1 but before bucket#800
> when perturb occured, then Packet#2 will be sent before Packet#1. And since
> UDP can't reordered, it will be processed before on the received end.
>
> Am I getting this right or is there something in the code that I missed ?
That's correct.
^ permalink raw reply
* Re: [net-next PATCH v6 0/3] net: reserve ports for applications using fixed port numbers
From: Octavian Purdila @ 2010-03-10 12:42 UTC (permalink / raw)
To: Cong Wang
Cc: Eric W. Biederman, David Miller, netdev, linux-kernel, nhorman,
eric.dumazet
In-Reply-To: <4B976503.4050702@redhat.com>
On Wednesday 10 March 2010 11:23:15 you wrote:
> Eric W. Biederman wrote:
> > I would add the restriction that the values in the list of ranges
> > always must be increasing, and in general restrict the set of accepted
> > values as much as possible. If we don't accept it now we don't have
> > to worry about some userspace application relying on some unitended
> > side effect a few years into the future.
>
> I don't think this is good.
>
> Suppose that when I just want to add one port into the list and keep the
> original ones, I want to do this:
>
> orig=$(cat ip_local_reserved_ports)
> new_list="$orig, $new_one"
> echo "$new_list" > ip_local_reserved_ports
>
> If we add this restriction, the above could be failed if the new port
> is lower than the original ones. This will be not convenient.
>
> > I think it is a serious bug that you clear the destination bitmap
> > in the middle of parsing it. That will either open or close all
> > ports in the middle of parsing, and I can't see how that would
> > ever be a good thing.
>
> Agreed.
>
> By the way, Octavian, any new updates?
>
Sorry, didn't got time to work on this lately, but I will submit a new version
I hope end of this week to address Eric's comments.
^ permalink raw reply
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Paul E. McKenney @ 2010-03-10 13:13 UTC (permalink / raw)
To: Herbert Xu
Cc: Eric Dumazet, Arnd Bergmann, David S. Miller, netdev,
Stephen Hemminger
In-Reply-To: <20100310104907.GA23752@gondor.apana.org.au>
On Wed, Mar 10, 2010 at 06:49:07PM +0800, Herbert Xu wrote:
> On Wed, Mar 10, 2010 at 11:39:43AM +0100, Eric Dumazet wrote:
> >
> > Its really rcu_dereference_bh() that could/should be used:
> > I see no problem changing
> >
> >
> > local_bh_disable();
> > ...
> > rcu_read_lock();
> > rcu_dereference(rt_hash_table[h].chain);
> > rcu_read_unlock();
> > ...
> > local_bh_enable();
>
> Why don't we just ignore the bh part for rcu_dereference?
>
> After all it's call_rcu_bh and the other primitives that we really
> care about. For rcu_dereference bh should make no difference
> whatsoever.
If CONFIG_PROVE_RCU is set, rcu_dereference() checks for rcu_read_lock()
and rcu_dereference_bh() checks for either rcu_read_lock_bh() or BH
being disabled. Yes, this is a bit restrictive, but there are a few too
many to check by hand these days.
Thanx, Paul
^ permalink raw reply
* Re: [PATCH 6/13] bridge: Add core IGMP snooping support
From: Paul E. McKenney @ 2010-03-10 13:19 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: Herbert Xu, David S. Miller, netdev, Stephen Hemminger
In-Reply-To: <201003101041.32518.arnd@arndb.de>
On Wed, Mar 10, 2010 at 10:41:32AM +0100, Arnd Bergmann wrote:
> On Wednesday 10 March 2010 03:14:10 Paul E. McKenney wrote:
> > On Tue, Mar 09, 2010 at 10:12:59PM +0100, Arnd Bergmann wrote:
> >
> > > I've just tried annotating net/ipv4/route.c like this and did not get
> > > very far, because the same pointers are used for rcu and rcu_bh.
> > > Could you check if this is a false positive or an actual finding?
> >
> > Hmmm... I am only seeing a call_rcu_bh() here, so unless I am missing
> > something, this is a real problem in TREE_PREEMPT_RCU kernels. The
> > call_rcu_bh() only interacts with the rcu_read_lock_bh() readers, not
> > the rcu_read_lock() readers.
> >
> > One approach is to run freed blocks through both types of grace periods,
> > I suppose.
>
> Well, if I introduce different __rcu and __rcu_bh address space annotations,
> sparse would still not like that, because then you can only pass the annotated
> pointers into either rcu_dereference or rcu_dereference_bh.
>
> What the code seems to be doing here is in some places
>
> local_bh_disable();
> ...
> rcu_read_lock();
> rcu_dereference(rt_hash_table[h].chain);
> rcu_read_unlock();
> ...
> local_bh_enable();
>
> and in others
>
> rcu_read_lock_bh();
> rcu_dereference_bh(rt_hash_table[h].chain);
> rcu_read_unlock_bh();
Hmmm... This is actually legal.
> When rt_hash_table[h].chain gets the __rcu_bh annotation, we'd have to
> turn first rcu_dereference into rcu_dereference_bh in order to have a clean
> build with sparse. Would that change be
> a) correct from RCU perspective,
> b) desirable for code inspection, and
> c) lockdep-clean?
I have a patch queued up that will make rcu_dereference_bh() handle this
correctly -- current -tip and mainline would complain. Please see below
for a sneak preview.
Thoughts?
Thanx, Paul
rcu: make rcu_read_lock_bh_held() allow for disabled BH
Disabling BH can stand in for rcu_read_lock_bh(), and this patch updates
rcu_read_lock_bh_held() to allow for this. In order to avoid
include-file hell, this function is moved out of line to kernel/rcupdate.c.
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
include/linux/rcupdate.h | 19 ++++---------------
kernel/rcupdate.c | 22 ++++++++++++++++++++++
2 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 75921b8..c393acc 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -119,22 +119,11 @@ static inline int rcu_read_lock_held(void)
return lock_is_held(&rcu_lock_map);
}
-/**
- * rcu_read_lock_bh_held - might we be in RCU-bh read-side critical section?
- *
- * If CONFIG_PROVE_LOCKING is selected and enabled, returns nonzero iff in
- * an RCU-bh read-side critical section. In absence of CONFIG_PROVE_LOCKING,
- * this assumes we are in an RCU-bh read-side critical section unless it can
- * prove otherwise.
- *
- * Check rcu_scheduler_active to prevent false positives during boot.
+/*
+ * rcu_read_lock_bh_held() is defined out of line to avoid #include-file
+ * hell.
*/
-static inline int rcu_read_lock_bh_held(void)
-{
- if (!debug_lockdep_rcu_enabled())
- return 1;
- return lock_is_held(&rcu_bh_lock_map);
-}
+extern int rcu_read_lock_bh_held(void);
/**
* rcu_read_lock_sched_held - might we be in RCU-sched read-side critical section?
diff --git a/kernel/rcupdate.c b/kernel/rcupdate.c
index f1125c1..913eccb 100644
--- a/kernel/rcupdate.c
+++ b/kernel/rcupdate.c
@@ -45,6 +45,7 @@
#include <linux/mutex.h>
#include <linux/module.h>
#include <linux/kernel_stat.h>
+#include <linux/hardirq.h>
#ifdef CONFIG_DEBUG_LOCK_ALLOC
static struct lock_class_key rcu_lock_key;
@@ -66,6 +67,27 @@ EXPORT_SYMBOL_GPL(rcu_sched_lock_map);
int rcu_scheduler_active __read_mostly;
EXPORT_SYMBOL_GPL(rcu_scheduler_active);
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+
+/**
+ * rcu_read_lock_bh_held - might we be in RCU-bh read-side critical section?
+ *
+ * Check for bottom half being disabled, which covers both the
+ * CONFIG_PROVE_RCU and not cases. Note that if someone uses
+ * rcu_read_lock_bh(), but then later enables BH, lockdep (if enabled)
+ * will show the situation.
+ *
+ * Check debug_lockdep_rcu_enabled() to prevent false positives during boot.
+ */
+int rcu_read_lock_bh_held(void)
+{
+ if (!debug_lockdep_rcu_enabled())
+ return 1;
+ return in_softirq();
+}
+
+#endif /* #ifdef CONFIG_DEBUG_LOCK_ALLOC */
+
/*
* This function is invoked towards the end of the scheduler's initialization
* process. Before this is called, the idle task might contain
^ 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