* Re: Virtual device and ARP table
From: Eric Dumazet @ 2010-06-07 12:22 UTC (permalink / raw)
To: Christophe Jelger; +Cc: linux-kernel, netdev
In-Reply-To: <4C0CC810.7030501@unibas.ch>
Le lundi 07 juin 2010 à 12:21 +0200, Christophe Jelger a écrit :
> Hello,
>
> I am currently "resurrecting" a Linux module (called LUNAR) which I
> co-developed in 2007 and I'm having a weird kernel crash. This code
> basically used to work fine up to 2.6.18 which was the latest version
> before we stopped our development. I quickly ported it to 2.6.{31,32}:
> it compiles fine and loads fine, but it crashes/hangs the kernel when
> it's really being used.
>
> The module is a virtual device used for MANET routing: with the current
> version, it basically "captures" DNS requests sent to the virtual
> interface --> this triggers the sending of a fake DNS reply (see below)
> and the creation of an ARP table entry for the destination (the MANET
> route is built at the same time). Packets can then be sent to the
> destination.
>
> The problem I'm having is that the kernel quickly hangs after I create a
> new ARP entry (actually only if it's being used). If the entry I create
> is set to NUD_PERMANENT, then everything works fine! I use
> __neigh_lookup_errno to lookup/create the entry and neigh_lookup to
> set/update the MAC address. Note that the ARP entry is created without
> problem, but typically even just doing a userspace "arp -a" command can
> crash the kernel (it also hangs the userspace command!). Doing "arp -na"
> usually does NOT crash the kernel!
>
> I guess the problem comes from a combination of ARP + DNS
> lookups/replies. Note that my kernel module has its own internal fake
> DNS server which captures lookups and sends replies directly back to the
> stack. What is amazing: if the ARP entry I create is set to
> NUD_PERMANENT, then I don't get any crash (however I cannot develop my
> module with permanent ARP entries).
>
> I'm wondering if there were any major changes to the neighbor and arp
> code (between 2.6.18 and 2.6.31) that are somehow causing this problem ?...
>
> Any hint is very welcome.
>
> thanks in advance,
> Christophe
>
> PS: I can easily reproduce the problem, and was trying to debug with
> qemu and gdb server but so fra no success to clearly identify the
> problem. Last point: it seems the kernel does not really "crash" but
> rather ends up in some unstable state and maybe in a loop.
> --
Hi Christophe
You should ask these kind of questions on netdev instead of lkml.
And of course, post your patch, or send us a crystal ball ;)
Yes, many things changed between 2.6.18 and 2.6.34
^ permalink raw reply
* Re: [PATCH net-next-2.6] macvlan: use call_rcu for port free
From: Eric Dumazet @ 2010-06-07 12:15 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, kaber
In-Reply-To: <20100607113629.GA2730@psychotron.lab.eng.brq.redhat.com>
Le lundi 07 juin 2010 à 13:36 +0200, Jiri Pirko a écrit :
> Use call_rcu rather than synchronize_rcu.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> drivers/net/macvlan.c | 12 ++++++++++--
> 1 files changed, 10 insertions(+), 2 deletions(-)
>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes
From: Johannes Berg @ 2010-06-07 11:41 UTC (permalink / raw)
To: Kay Sievers; +Cc: Eric W. Biederman, Greg KH, netdev
In-Reply-To: <AANLkTikMDvpEzrtFVue8gStudKONxuQmVx9B2w1JM1vO@mail.gmail.com>
On Mon, 2010-06-07 at 13:05 +0200, Kay Sievers wrote:
> > + data->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
> > + if (!data->dev) {
> > err = -ENOMEM;
> > goto failed_drvdata;
> > }
> > +
> > + dev_set_name(data->dev, "hwsim%d", i);
> > + data->dev->bus = &hwsim_bus;
> > data->dev->driver = &mac80211_hwsim_driver;
> >
> > + err = device_register(data->dev);
> > + if (err) {
> > + printk(KERN_DEBUG
> > + "mac80211_hwsim: device_register failed (%d)\n",
> > + err);
> > + goto failed_drvdata;
> > + }
> >
> > (ignore the pluses, snipped from a patch) but it ran into a null ptr
> > deref?
>
> Oh, I see. It's probably something nobody ever did before. You try to
> register a bus device which has no parent. Seems that's something
> nobody ever expected to happen. :)
>
> Your driver/subsystem is completely virtual, does not depend on any
> hardware, right? If we create a virtual parent, like:
> parent = kzalloc(sizeof(struct device), GFP_KERNEL);
> dev_set_name(parent, "mac80211_hwsim");
> device_register(parent);
>
> An in your code:
> data->dev->parent = parent;
>
> That should give you a /sys/devices/mac80211_hwsim/ directory where
> all the devices you create should show up.
So that seemed equivalent to my code except for the .bus/.driver
assignments and the two-level hierarchy of course.
(mind you, I think we probably need to have the bus/driver assignment,
but I wanted to try out your suggestion first)
So I removed bus/driver assignment from the above code just to try it
out, and got
Device 'hwsim0' does not have a release() function, it is broken and
must be fixed.
This has evolved far too much for me right now. Can we just apply the
initial patch from Eric and be happier for a while? I can't justify
spending this much time on it right now. Alternatively, you could look
at hwsim too, since it's all virtual, nothing special is required, I do
testing in a virtual machine ...
current patch is at
http://johannes.sipsolutions.net/patches/kernel/all/2010-06-07-11:41/hwsim-bus.patch
johannes
^ permalink raw reply
* BUG: double spinlock in "drivers/net/3c505.c"
From: Alexander Strakh @ 2010-06-07 11:17 UTC (permalink / raw)
To: Philip Blundell
Cc: Craig Southeren, Andrew Tridgell, Alan Cox, netdev, linux-kernel
KERNEL_VERSION: 2.6.35-rc1
SUBJECT: duble spinlock in function elp_start_xmit
SUBSCRIBE:
In driver drivers/net/3c505.c in function elp_start_xmit:
1. In line 1075 we have first spinlock. In the next line we called
check_3c505_dma:
1070 static netdev_tx_t elp_start_xmit(struct sk_buff *skb, struct net_device
*dev)
1071 {
1072 unsigned long flags;
1073 elp_device *adapter = netdev_priv(dev);
1074
1075 spin_lock_irqsave(&adapter->lock, flags);
1076 check_3c505_dma(dev);
2. In function check_3c505_dma we have second spinloock at line 301.
293 static inline void check_3c505_dma(struct net_device *dev)
294 {
295 elp_device *adapter = netdev_priv(dev);
296 if (adapter->dmaing && time_after(jiffies, adapter-
>current_dma.start_time + 10)) {
297 unsigned long flags, f;
298 pr_err("%s: DMA %s timed out, %d bytes left\n", dev-
>name,
299 adapter->current_dma.direction ? "download" :
"upload",
300 get_dma_residue(dev->dma));
301 spin_lock_irqsave(&adapter->lock, flags);
Found by Linux Device Drivers Verification Project (Svace Detector)
^ permalink raw reply
* [PATCH net-next-2.6] macvlan: use call_rcu for port free
From: Jiri Pirko @ 2010-06-07 11:36 UTC (permalink / raw)
To: netdev; +Cc: davem, kaber
Use call_rcu rather than synchronize_rcu.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
drivers/net/macvlan.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 53422ce..59c3155 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -37,6 +37,7 @@ struct macvlan_port {
struct net_device *dev;
struct hlist_head vlan_hash[MACVLAN_HASH_SIZE];
struct list_head vlans;
+ struct rcu_head rcu;
};
static struct macvlan_dev *macvlan_hash_lookup(const struct macvlan_port *port,
@@ -540,14 +541,21 @@ static int macvlan_port_create(struct net_device *dev)
return err;
}
+static void macvlan_port_rcu_free(struct rcu_head *head)
+{
+ struct macvlan_port *port;
+
+ port = container_of(head, struct macvlan_port, rcu);
+ kfree(port);
+}
+
static void macvlan_port_destroy(struct net_device *dev)
{
struct macvlan_port *port = dev->macvlan_port;
netdev_rx_handler_unregister(dev);
rcu_assign_pointer(dev->macvlan_port, NULL);
- synchronize_rcu();
- kfree(port);
+ call_rcu(&port->rcu, macvlan_port_rcu_free);
}
static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
--
1.7.0.1
^ permalink raw reply related
* Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes
From: Kay Sievers @ 2010-06-07 11:05 UTC (permalink / raw)
To: Johannes Berg; +Cc: Eric W. Biederman, Greg KH, netdev
In-Reply-To: <1275905686.29978.3.camel@jlt3.sipsolutions.net>
On Mon, Jun 7, 2010 at 12:14, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2010-06-07 at 11:53 +0200, Kay Sievers wrote:
>
>> > Can you please tell me then how to device_create() without a class? I
>> > cannot seem to create devices without a class at all, even using manual
>> > allocation (yuck) and device_register crashes the kernel.
>>
>> Right, this "convenience API" does not exist for buses. It's not doing
>> much, just allocates a "struct device" and fills in the few values and
>> calls device_register().
>>
>> Does your device create a device node? If not, device_create() should
>> not be used anyway, because the corresponding device_destroy() will
>> not do anything.
>
> No, it doesn't need a dev node. I tried this:
>
> + data->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
> + if (!data->dev) {
> err = -ENOMEM;
> goto failed_drvdata;
> }
> +
> + dev_set_name(data->dev, "hwsim%d", i);
> + data->dev->bus = &hwsim_bus;
> data->dev->driver = &mac80211_hwsim_driver;
>
> + err = device_register(data->dev);
> + if (err) {
> + printk(KERN_DEBUG
> + "mac80211_hwsim: device_register failed (%d)\n",
> + err);
> + goto failed_drvdata;
> + }
>
> (ignore the pluses, snipped from a patch) but it ran into a null ptr
> deref?
Oh, I see. It's probably something nobody ever did before. You try to
register a bus device which has no parent. Seems that's something
nobody ever expected to happen. :)
Your driver/subsystem is completely virtual, does not depend on any
hardware, right? If we create a virtual parent, like:
parent = kzalloc(sizeof(struct device), GFP_KERNEL);
dev_set_name(parent, "mac80211_hwsim");
device_register(parent);
An in your code:
data->dev->parent = parent;
That should give you a /sys/devices/mac80211_hwsim/ directory where
all the devices you create should show up.
If that works as expected, we should probably add something like:
struct device *device_virtual_parent(const char *name);
which will allow you to create such a parent device in the
/sys/device/virtual/ directory.
Let me know if the above hack with the virtual parent works, then we
can check what do add to the core.
Thanks,
Kay
^ permalink raw reply
* Re: [Patch 2/2] mlx4: add dynamic LRO disable support
From: Stanislaw Gruszka @ 2010-06-07 11:00 UTC (permalink / raw)
To: Cong Wang; +Cc: Ben Hutchings, netdev, herbert.xu, nhorman, davem
In-Reply-To: <4C0CB325.2040704@redhat.com>
On Mon, 07 Jun 2010 16:51:49 +0800
Cong Wang <amwang@redhat.com> wrote:
> > Now that I look at the patch again, I see you're using a static (i.e.
> > global) variable to 'back up' the non-zero (enabled) value of num_lro.
> > This is introducing a bug! The correct value is apparently set in
> > mlx4_en_get_profile(); you would need to replicate that.
> >
>
> Oh, probably, but unfortunately 'num_lro' is static so only visible
> in en_main.c.
So just remove "static" and make it global :-)
Stanislaw
^ permalink raw reply
* Re: [PATCH] tcp: Fix slowness in read /proc/net/tcp
From: Eric Dumazet @ 2010-06-07 10:32 UTC (permalink / raw)
To: Andi Kleen; +Cc: Tom Herbert, davem, netdev, Yakov Lerner
In-Reply-To: <87eigjgrcw.fsf@basil.nowhere.org>
Le lundi 07 juin 2010 à 10:49 +0200, Andi Kleen a écrit :
> Eric Dumazet <eric.dumazet@gmail.com> writes:
> >
> > BTW, another problem of /proc/net/tcp is the buffer size used by netstat
> > utility : 1024 bytes instead of PAGE_SIZE, making O(N^2) behavior even
> > more palpable.
>
> That would be easily fixable in netstat.
>
> But I'm not sure net-tools is still maintained as a separate package,
> afaik the standard procedure is to submit a patch to a big distribution
> and the others take it from there.
Well, it seems this is fixed in latest netstat, sorry for the noise (I
still have machines with RHEL 4)
^ permalink raw reply
* [PATCH v3] netdev:bfin_mac: reclaim and free tx skb as soon as possible after transfer
From: sonic zhang @ 2010-06-07 10:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev, uclinux-dist-devel
>From 40560ae9e8db42e2d2259b791ace160534c9a0f2 Mon Sep 17 00:00:00 2001
From: Sonic Zhang <sonic.zhang@analog.com>
Date: Thu, 3 Jun 2010 11:44:33 +0800
Subject: [PATCH v3] netdev:bfin_mac: reclaim and free tx skb as soon as possible after transfer
SKBs hold onto resources that can't be held indefinitely, such as TCP
socket references and netfilter conntrack state. So if a packet is left
in TX ring for a long time, there might be a TCP socket that cannot be
closed and freed up.
Current blackfin EMAC driver always reclaim and free used tx skbs in future
transfers. The problem is that future transfer may not come as soon as
possible. This patch start a timer after transfer to reclaim and free skb.
There is nearly no performance drop with this patch.
TX interrupt is not enabled because of a strange behavior of the Blackfin EMAC.
If EMAC TX transfer control is turned on, endless TX interrupts are triggered
no matter if TX DMA is enabled or not. Since DMA walks down the ring automatically,
TX transfer control can't be turned off in the middle. The only way is to disable
TX interrupt completely.
Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
---
drivers/net/bfin_mac.c | 114 ++++++++++++++++++++++++++++++------------------
drivers/net/bfin_mac.h | 5 ++
2 files changed, 77 insertions(+), 42 deletions(-)
diff --git a/drivers/net/bfin_mac.c b/drivers/net/bfin_mac.c
index 368f333..6f0755f 100644
--- a/drivers/net/bfin_mac.c
+++ b/drivers/net/bfin_mac.c
@@ -922,61 +922,73 @@ static void bfin_mac_hwtstamp_init(struct net_device *netdev)
# define bfin_tx_hwtstamp(dev, skb)
#endif
-static void adjust_tx_list(void)
+static inline void _tx_reclaim_skb(void)
+{
+ do {
+ tx_list_head->desc_a.config &= ~DMAEN;
+ tx_list_head->status.status_word = 0;
+ if (tx_list_head->skb) {
+ dev_kfree_skb(tx_list_head->skb);
+ tx_list_head->skb = NULL;
+ }
+ tx_list_head = tx_list_head->next;
+
+ } while (tx_list_head->status.status_word != 0);
+}
+
+static void tx_reclaim_skb(struct bfin_mac_local *lp)
{
int timeout_cnt = MAX_TIMEOUT_CNT;
- if (tx_list_head->status.status_word != 0 &&
- current_tx_ptr != tx_list_head) {
- goto adjust_head; /* released something, just return; */
- }
+ if (tx_list_head->status.status_word != 0)
+ _tx_reclaim_skb();
- /*
- * if nothing released, check wait condition
- * current's next can not be the head,
- * otherwise the dma will not stop as we want
- */
- if (current_tx_ptr->next->next == tx_list_head) {
+ if (current_tx_ptr->next == tx_list_head) {
while (tx_list_head->status.status_word == 0) {
+ /* slow down polling to avoid too many queue stop. */
udelay(10);
- if (tx_list_head->status.status_word != 0 ||
- !(bfin_read_DMA2_IRQ_STATUS() & DMA_RUN)) {
- goto adjust_head;
- }
- if (timeout_cnt-- < 0) {
- printk(KERN_ERR DRV_NAME
- ": wait for adjust tx list head timeout\n");
+ /* reclaim skb if DMA is not running. */
+ if (!(bfin_read_DMA2_IRQ_STATUS() & DMA_RUN))
+ break;
+ if (timeout_cnt-- < 0)
break;
- }
- }
- if (tx_list_head->status.status_word != 0) {
- goto adjust_head;
}
+
+ if (timeout_cnt >= 0)
+ _tx_reclaim_skb();
+ else
+ netif_stop_queue(lp->ndev);
}
- return;
+ if (current_tx_ptr->next != tx_list_head &&
+ netif_queue_stopped(lp->ndev))
+ netif_wake_queue(lp->ndev);
+
+ if (tx_list_head != current_tx_ptr) {
+ /* shorten the timer interval if tx queue is stopped */
+ if (netif_queue_stopped(lp->ndev))
+ lp->tx_reclaim_timer.expires =
+ jiffies + (TX_RECLAIM_JIFFIES >> 4);
+ else
+ lp->tx_reclaim_timer.expires =
+ jiffies + TX_RECLAIM_JIFFIES;
+
+ mod_timer(&lp->tx_reclaim_timer,
+ lp->tx_reclaim_timer.expires);
+ }
-adjust_head:
- do {
- tx_list_head->desc_a.config &= ~DMAEN;
- tx_list_head->status.status_word = 0;
- if (tx_list_head->skb) {
- dev_kfree_skb(tx_list_head->skb);
- tx_list_head->skb = NULL;
- } else {
- printk(KERN_ERR DRV_NAME
- ": no sk_buff in a transmitted frame!\n");
- }
- tx_list_head = tx_list_head->next;
- } while (tx_list_head->status.status_word != 0 &&
- current_tx_ptr != tx_list_head);
return;
+}
+static void tx_reclaim_skb_timeout(unsigned long lp)
+{
+ tx_reclaim_skb((struct bfin_mac_local *)lp);
}
static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
+ struct bfin_mac_local *lp = netdev_priv(dev);
u16 *data;
u32 data_align = (unsigned long)(skb->data) & 0x3;
union skb_shared_tx *shtx = skb_tx(skb);
@@ -1009,8 +1021,6 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
skb->len);
current_tx_ptr->desc_a.start_addr =
(u32)current_tx_ptr->packet;
- if (current_tx_ptr->status.status_word != 0)
- current_tx_ptr->status.status_word = 0;
blackfin_dcache_flush_range(
(u32)current_tx_ptr->packet,
(u32)(current_tx_ptr->packet + skb->len + 2));
@@ -1022,6 +1032,9 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
*/
SSYNC();
+ /* always clear status buffer before start tx dma */
+ current_tx_ptr->status.status_word = 0;
+
/* enable this packet's dma */
current_tx_ptr->desc_a.config |= DMAEN;
@@ -1037,13 +1050,14 @@ static int bfin_mac_hard_start_xmit(struct sk_buff *skb,
bfin_write_EMAC_OPMODE(bfin_read_EMAC_OPMODE() | TE);
out:
- adjust_tx_list();
-
bfin_tx_hwtstamp(dev, skb);
current_tx_ptr = current_tx_ptr->next;
dev->stats.tx_packets++;
dev->stats.tx_bytes += (skb->len);
+
+ tx_reclaim_skb(lp);
+
return NETDEV_TX_OK;
}
@@ -1167,8 +1181,11 @@ real_rx:
#ifdef CONFIG_NET_POLL_CONTROLLER
static void bfin_mac_poll(struct net_device *dev)
{
+ struct bfin_mac_local *lp = netdev_priv(dev);
+
disable_irq(IRQ_MAC_RX);
bfin_mac_interrupt(IRQ_MAC_RX, dev);
+ tx_reclaim_skb(lp);
enable_irq(IRQ_MAC_RX);
}
#endif /* CONFIG_NET_POLL_CONTROLLER */
@@ -1232,12 +1249,20 @@ static int bfin_mac_enable(void)
/* Our watchdog timed out. Called by the networking layer */
static void bfin_mac_timeout(struct net_device *dev)
{
+ struct bfin_mac_local *lp = netdev_priv(dev);
+
pr_debug("%s: %s\n", dev->name, __func__);
bfin_mac_disable();
+ if (timer_pending(&lp->tx_reclaim_timer))
+ del_timer(&(lp->tx_reclaim_timer));
+
/* reset tx queue */
- tx_list_tail = tx_list_head->next;
+ current_tx_ptr = tx_list_head;
+
+ if (netif_queue_stopped(lp->ndev))
+ netif_wake_queue(lp->ndev);
bfin_mac_enable();
@@ -1430,6 +1455,7 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
SET_NETDEV_DEV(ndev, &pdev->dev);
platform_set_drvdata(pdev, ndev);
lp = netdev_priv(ndev);
+ lp->ndev = ndev;
/* Grab the MAC address in the MAC */
*(__le32 *) (&(ndev->dev_addr[0])) = cpu_to_le32(bfin_read_EMAC_ADDRLO());
@@ -1485,6 +1511,10 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
ndev->netdev_ops = &bfin_mac_netdev_ops;
ndev->ethtool_ops = &bfin_mac_ethtool_ops;
+ init_timer(&lp->tx_reclaim_timer);
+ lp->tx_reclaim_timer.data = (unsigned long)lp;
+ lp->tx_reclaim_timer.function = tx_reclaim_skb_timeout;
+
spin_lock_init(&lp->lock);
/* now, enable interrupts */
diff --git a/drivers/net/bfin_mac.h b/drivers/net/bfin_mac.h
index 1ae7b82..04e4050 100644
--- a/drivers/net/bfin_mac.h
+++ b/drivers/net/bfin_mac.h
@@ -13,9 +13,12 @@
#include <linux/net_tstamp.h>
#include <linux/clocksource.h>
#include <linux/timecompare.h>
+#include <linux/timer.h>
#define BFIN_MAC_CSUM_OFFLOAD
+#define TX_RECLAIM_JIFFIES (HZ / 5)
+
struct dma_descriptor {
struct dma_descriptor *next_dma_desc;
unsigned long start_addr;
@@ -68,6 +71,8 @@ struct bfin_mac_local {
int wol; /* Wake On Lan */
int irq_wake_requested;
+ struct timer_list tx_reclaim_timer;
+ struct net_device *ndev;
/* MII and PHY stuffs */
int old_link; /* used by bf537_adjust_link */
--
1.6.0
^ permalink raw reply related
* Re: [PATCH v2] netdev:bfin_mac: reclaim and free tx skb as soon as possible after transfer
From: Sonic Zhang @ 2010-06-07 10:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, uclinux-dist-devel
In-Reply-To: <1275904680.2545.44.camel@edumazet-laptop>
On Mon, Jun 7, 2010 at 5:58 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le vendredi 04 juin 2010 à 12:44 +0800, Sonic Zhang a écrit :
>
>>
>> Yes, you are right. dev_kfree_skb_irq() queues used skb to the
>> complete queue. But, it is actually freed in the other soft irq
>> NET_TX_SOFTIRQ.
>
> I guess you didnt understood my mail, so I'll re-explain :
>
> dev_kfree_skb_irq() queues skb only if packet is not already orphaned.
>
> As most packets are now orphaned (in recent kernels where your patch
> applies), dev_kfree_skb_irq() can free packet immediately, with no
> NET_TX_SOFTIRQ overhead.
>
>
OK. I didn't notice the recent change in kernel to orphan most
packets. You are right, my second reason is out of date.
Thanks
Sonic
>
>
^ permalink raw reply
* Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes
From: Johannes Berg @ 2010-06-07 10:14 UTC (permalink / raw)
To: Kay Sievers; +Cc: Eric W. Biederman, Greg KH, netdev
In-Reply-To: <AANLkTin_qjU0-xXfMDYeYWRd0BmvT4OIdK4S_GQ3ZYUi@mail.gmail.com>
On Mon, 2010-06-07 at 11:53 +0200, Kay Sievers wrote:
> > Can you please tell me then how to device_create() without a class? I
> > cannot seem to create devices without a class at all, even using manual
> > allocation (yuck) and device_register crashes the kernel.
>
> Right, this "convenience API" does not exist for buses. It's not doing
> much, just allocates a "struct device" and fills in the few values and
> calls device_register().
>
> Does your device create a device node? If not, device_create() should
> not be used anyway, because the corresponding device_destroy() will
> not do anything.
No, it doesn't need a dev node. I tried this:
+ data->dev = kzalloc(sizeof(struct device), GFP_KERNEL);
+ if (!data->dev) {
err = -ENOMEM;
goto failed_drvdata;
}
+
+ dev_set_name(data->dev, "hwsim%d", i);
+ data->dev->bus = &hwsim_bus;
data->dev->driver = &mac80211_hwsim_driver;
+ err = device_register(data->dev);
+ if (err) {
+ printk(KERN_DEBUG
+ "mac80211_hwsim: device_register failed (%d)\n",
+ err);
+ goto failed_drvdata;
+ }
(ignore the pluses, snipped from a patch) but it ran into a null ptr
deref?
johannes
^ permalink raw reply
* Re: RX/close vcc race with solos/atmtcp/usbatm/he
From: David Woodhouse @ 2010-06-07 10:02 UTC (permalink / raw)
To: linux-atm-general; +Cc: netdev, Nathan Williams
In-Reply-To: <1274872584.20576.13579.camel@macbook.infradead.org>
On Wed, 2010-05-26 at 12:16 +0100, David Woodhouse wrote:
> I've had this crash reported to me...
>
> [18842.727906] EIP: [<e082f490>] br2684_push+0x19/0x234 [br2684]
> SS:ESP 0068:dfb89d14
Nathan, did you manage to get your customer to confirm that this fixes
the problem? It'd be useful to get this into 2.6.35 and -stable.
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation
^ permalink raw reply
* Re: [v5 Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: David Miller @ 2010-06-07 10:01 UTC (permalink / raw)
To: amwang
Cc: andy, fubar, fbl, linux-kernel, mpm, netdev, bridge, gospo,
nhorman, jmoyer, shemminger, bonding-devel
In-Reply-To: <4C0CC29D.9070507@redhat.com>
From: Cong Wang <amwang@redhat.com>
Date: Mon, 07 Jun 2010 17:57:49 +0800
> Hmm, I still feel like this way is ugly, although it may work.
> I guess David doesn't like it either.
Of course I don't like it. :-)
I suspect the locking scheme will need to be changed.
Besides, if we're going to hack this up and do write lock attempts in
the read locking paths, there is no point in using a rwlock any more.
And I'm personally in disfavor of all rwlock usage anyways (it dirties
the cacheline for readers just as equally for writers, and if the
critically protected code path is short enough, that shared cache
line atomic operation will be the predominant cost).
So I'd say, 1) make this a spinlock and 2) try to use RCU for the
read path.
That would fix everything.
^ permalink raw reply
* Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes
From: Kay Sievers @ 2010-06-07 9:53 UTC (permalink / raw)
To: Johannes Berg; +Cc: Eric W. Biederman, Greg KH, netdev
In-Reply-To: <1275903773.29978.1.camel@jlt3.sipsolutions.net>
On Mon, Jun 7, 2010 at 11:42, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Sun, 2010-06-06 at 19:17 +0200, Kay Sievers wrote:
>
>> There is no real difference between classes and buses. Actually we're
>> working on merging them completely inside the kernel. Just declare a
>> "struct bus_type" instead of a "struct class".
>
> Can you please tell me then how to device_create() without a class? I
> cannot seem to create devices without a class at all, even using manual
> allocation (yuck) and device_register crashes the kernel.
Right, this "convenience API" does not exist for buses. It's not doing
much, just allocates a "struct device" and fills in the few values and
calls device_register().
Does your device create a device node? If not, device_create() should
not be used anyway, because the corresponding device_destroy() will
not do anything.
Kay
^ permalink raw reply
* Re: [Bugme-new] [Bug 16120] New: Oops: 0000 [#1] SMP, unable to handle kernel NULL pointer dereference at (null)
From: David Miller @ 2010-06-07 9:58 UTC (permalink / raw)
To: eric.dumazet
Cc: akpm, netdev, bugzilla-daemon, bugme-daemon, alex.vizor, kaber
In-Reply-To: <1275904120.2545.40.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 07 Jun 2010 11:48:40 +0200
> [PATCH net-2.6] ipmr: dont corrupt lists
>
> ipmr_rules_exit() and ip6mr_rules_exit() free a list of items, but
> forget to properly remove these items from list. List head is not
> changed and still points to freed memory.
>
> This can trigger a fault later when icmpv6_sk_exit() is called.
>
> Fix is to either reinit list, or use list_del() to properly remove items
> from list before freeing them.
>
> bugzilla report : https://bugzilla.kernel.org/show_bug.cgi?id=16120
>
> Introduced by commit d1db275dd3f6e4 (ipv6: ip6mr: support multiple
> tables) and commit f0ad0860d01e (ipv4: ipmr: support multiple tables)
>
> Reported-by: Alex Zhavnerchik <alex.vizor@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Patrick McHardy <kaber@trash.net>
Applied, thanks a lot Eric.
^ permalink raw reply
* Re: [PATCH v2] netdev:bfin_mac: reclaim and free tx skb as soon as possible after transfer
From: Eric Dumazet @ 2010-06-07 9:58 UTC (permalink / raw)
To: Sonic Zhang; +Cc: David Miller, netdev, uclinux-dist-devel
In-Reply-To: <AANLkTimO47rtvIwYAwpaMYtK5SEnhM0m3AxL5q-8lvA7@mail.gmail.com>
Le vendredi 04 juin 2010 à 12:44 +0800, Sonic Zhang a écrit :
>
> Yes, you are right. dev_kfree_skb_irq() queues used skb to the
> complete queue. But, it is actually freed in the other soft irq
> NET_TX_SOFTIRQ.
I guess you didnt understood my mail, so I'll re-explain :
dev_kfree_skb_irq() queues skb only if packet is not already orphaned.
As most packets are now orphaned (in recent kernels where your patch
applies), dev_kfree_skb_irq() can free packet immediately, with no
NET_TX_SOFTIRQ overhead.
^ permalink raw reply
* Re: [v5 Patch 1/3] netpoll: add generic support for bridge and bonding devices
From: Cong Wang @ 2010-06-07 9:57 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jay Vosburgh, Flavio Leitner, linux-kernel, Matt Mackall, netdev,
bridge, Andy Gospodarek, Neil Horman, Jeff Moyer,
Stephen Hemminger, bonding-devel, David Miller
In-Reply-To: <20100604191841.GM7497@gospo.rdu.redhat.com>
On 06/05/10 03:18, Andy Gospodarek wrote:
> On Wed, Jun 02, 2010 at 06:04:45PM +0800, Cong Wang wrote:
>> On 06/02/10 02:42, Jay Vosburgh wrote:
>>> Cong Wang<amwang@redhat.com> wrote:
>>>
>>>> On 06/01/10 03:08, Flavio Leitner wrote:
>>>>> On Mon, May 31, 2010 at 01:56:52PM +0800, Cong Wang wrote:
>>>>>> Hi, Flavio,
>>>>>>
>>>>>> Please use the attached patch instead, try to see if it solves
>>>>>> all your problems.
>>>>>
>>>>> I tried and it hangs. No backtraces this time.
>>>>> The bond_change_active_slave() prints before NETDEV_BONDING_FAILOVER
>>>>> notification, so I think it won't work.
>>>>
>>>> Ah, I thought the same.
>>>>
>>>>>
>>>>> Please, correct if I'm wrong, but when a failover happens with your
>>>>> patch applied, the netconsole would be disabled forever even with
>>>>> another healthy slave, right?
>>>>>
>>>>
>>>> Yes, this is an easy solution, because bonding has several modes,
>>>> it is complex to make netpoll work in different modes.
>>>
>>> If I understand correctly, the root cause of the problem with
>>> netconsole and bonding is that bonding is, ultimately, performing
>>> printks with a write lock held, and when netconsole recursively calls
>>> into bonding to send the printk over the netconsole, there is a deadlock
>>> (when the bonding xmit function attempts to acquire the same lock for
>>> read).
>>
>>
>> Yes.
>>
>>>
>>> You're trying to avoid the deadlock by shutting off netconsole
>>> (permanently, it looks like) for one problem case: a failover, which
>>> does some printks with a write lock held.
>>>
>>> This doesn't look to me like a complete solution, there are
>>> other cases in bonding that will do printk with write locks held. I
>>> suspect those will also hang netconsole as things exist today, and won't
>>> be affected by your patch below.
>>
>>
>> I can expect that, bonding modes are complex.
>>
>>>
>>> For example:
>>>
>>> The sysfs functions to set the primary (bonding_store_primary)
>>> or active (bonding_store_active_slave) options: a pr_info is called to
>>> provide a log message of the results. These could be tested by setting
>>> the primary or active options via sysfs, e.g.,
>>>
>>> echo eth0> /sys/class/net/bond0/bonding/primary
>>> echo eth0> /sys/class/net/bond0/bonding/active
>>>
>>> If the kernel is defined with DEBUG, there are a few pr_debug
>>> calls within write_locks (bond_del_vlan, for example).
>>>
>>> If the slave's underlying device driver's ndo_vlan_rx_register
>>> or ndo_vlan_rx_kill_vid functions call printk (and it looks like some do
>>> for error cases, e.g., igbvf, ehea, enic), those would also presumably
>>> deadlock (because bonding holds its write_lock when calling the ndo_
>>> vlan functions).
>>>
>>> It also appears that (with the patch below) some nominally
>>> normal usage patterns will immediately disable netconsole. The one that
>>> comes to mind is if the primary= option is set (to "eth1" for this
>>> example), but that slave not enslaved first (the slaves are added, say,
>>> eth0 then eth1). In that situation, when the primary slave (eth1 here)
>>> is added, the first thing that will happen is a failover, and that will
>>> disable netconsole.
>>>
>>
>> Thanks for your detailed explanation!
>>
>> This is why I said bonding is complex. I guess we would have to adjust
>> netpoll code for different bonding cases, one solution seems not fix all.
>> I am not sure how much work to do, since I am not familiar with bonding
>> code. Maybe Andy can help?
>>
>
> Sorry I've been silent until now. This does seem quite similar to a
> problem I've previously encountered when dealing with bonding+netpoll on
> some old 2.6.9-based kernels. There is no guarantee the methods used
> there will apply here, but I'll talk about them anyway.
>
> As Flavio noticed, recursive calls into the bond transmit routines were
> not a good idea. I discovered the same and worked around this issue by
> checking to see if we could take the bond->lock for writing before
> continuing. If we could not get, I wanted to signal that this should be
> queued for transmission later. Based on the flow of netpoll_send_skb
> (or possibly for another reason that is escaping me right now) I added
> one of these checks in bond_poll_controller too. These aren't the
> prettiest fixes, but seemed to work well for me when I did this work in
> the past. I realize the differences are not that great compared to some
> of the patches posted by Flavio, but I think they are worth trying.
Hmm, I still feel like this way is ugly, although it may work.
I guess David doesn't like it either.
Anyway, Flavio, could you try the following patch as well?
Thanks a lot!
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index ef60244..d7b9b99 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1290,6 +1290,12 @@ static bool slaves_support_netpoll(struct net_device *bond_dev)
> static void bond_poll_controller(struct net_device *bond_dev)
> {
> struct net_device *dev = bond_dev->npinfo->netpoll->real_dev;
> + struct bonding *bond = netdev_priv(bond_dev);
> +
> + if (!write_trylock(&bond->lock))
> + return;
> + write_unlock(&bond->lock);
> +
> if (dev != bond_dev)
> netpoll_poll_dev(dev);
> }
> @@ -4418,7 +4424,11 @@ static void bond_set_xmit_hash_policy(struct bonding *bond)
>
> static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> - const struct bonding *bond = netdev_priv(dev);
> + struct bonding *bond = netdev_priv(dev);
> +
> + if (!write_trylock(&bond->lock))
> + return NETDEV_TX_BUSY;
> + write_unlock(&bond->lock);
>
> switch (bond->params.mode) {
> case BOND_MODE_ROUNDROBIN:
>
> The other key to all of this is to make sure that queuing is done
> correctly now that we expect to queue these frames and have them sent at
> some point when there is a member of the bond that is actually capable
> of sending them out.
>
> The new style of sending queued skbs in a workqueue is much better than
> what was done in the 2.6.9 timeframe, but careful attention should still
> be paid to txq lock and which processor is the owner. Returning
> something other than NETDEV_TX_OK from bond_start_xmit and checking for
> locks being held there should also help with any deadlocks that show up
> while running in queue_process (though they would not be recursive).
>
> I'm not in a good spot to test this right now, but I can take a look at
> next week and we can try and track down any of the other deadlocks that
> currently exist as I suspect this will not resolve all of the issues.
^ permalink raw reply
* Re: [Bugme-new] [Bug 16120] New: Oops: 0000 [#1] SMP, unable to handle kernel NULL pointer dereference at (null)
From: Eric Dumazet @ 2010-06-07 9:48 UTC (permalink / raw)
To: Andrew Morton, David Miller
Cc: netdev, bugzilla-daemon, bugme-daemon, alex.vizor,
Patrick McHardy
In-Reply-To: <1275730457.5238.14.camel@edumazet-laptop>
Le samedi 05 juin 2010 à 11:34 +0200, Eric Dumazet a écrit :
> Le samedi 05 juin 2010 à 11:17 +0200, Eric Dumazet a écrit :
> > Le vendredi 04 juin 2010 à 16:17 -0700, Andrew Morton a écrit :
> > > (switched to email. Please respond via emailed reply-to-all, not via the
> > > bugzilla web interface).
> > >
> > > On Fri, 4 Jun 2010 09:25:58 GMT
> > > bugzilla-daemon@bugzilla.kernel.org wrote:
> > >
> > > > https://bugzilla.kernel.org/show_bug.cgi?id=16120
> > > >
> > > > Summary: Oops: 0000 [#1] SMP, unable to handle kernel NULL
> > > > pointer dereference at (null)
> > > > Product: Platform Specific/Hardware
> > > > Version: 2.5
> > > > Kernel Version: 2.6.35-rc1
> > > > Platform: All
> > > > OS/Version: Linux
> > > > Tree: Mainline
> > > > Status: NEW
> > > > Severity: high
> > > > Priority: P1
> > > > Component: x86-64
> > > > AssignedTo: platform_x86_64@kernel-bugs.osdl.org
> > > > ReportedBy: alex.vizor@gmail.com
> > > > Regression: Yes
> > > >
> > > >
> > > > Created an attachment (id=26647)
> > > > --> (https://bugzilla.kernel.org/attachment.cgi?id=26647) id)
> >
> > > > 2.6.35-rc1 kernel log
> > > >
> > > > It happens randomly, almost a week I used 2.6.35-rc1 and don't have any
> > > > problems. But since last day it happened twice.
> > > >
> > > > I attached kernel log, please inform me if I can help in investigation.
> > > >
> > >
> > > ip6mr_sk_done() oopsed.
> >
> > Only thing I found a first glance is a typo but this should not be the
> > root of the problem.
> >
>
I was able to reproduce the problem here, and following patch solves it.
(I see David already committed first patch about macro typo)
Thanks !
[PATCH net-2.6] ipmr: dont corrupt lists
ipmr_rules_exit() and ip6mr_rules_exit() free a list of items, but
forget to properly remove these items from list. List head is not
changed and still points to freed memory.
This can trigger a fault later when icmpv6_sk_exit() is called.
Fix is to either reinit list, or use list_del() to properly remove items
from list before freeing them.
bugzilla report : https://bugzilla.kernel.org/show_bug.cgi?id=16120
Introduced by commit d1db275dd3f6e4 (ipv6: ip6mr: support multiple
tables) and commit f0ad0860d01e (ipv4: ipmr: support multiple tables)
Reported-by: Alex Zhavnerchik <alex.vizor@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Patrick McHardy <kaber@trash.net>
---
net/ipv4/ipmr.c | 4 +++-
net/ipv6/ip6mr.c | 4 +++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 856123f..757f25e 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -267,8 +267,10 @@ static void __net_exit ipmr_rules_exit(struct net *net)
{
struct mr_table *mrt, *next;
- list_for_each_entry_safe(mrt, next, &net->ipv4.mr_tables, list)
+ list_for_each_entry_safe(mrt, next, &net->ipv4.mr_tables, list) {
+ list_del(&mrt->list);
kfree(mrt);
+ }
fib_rules_unregister(net->ipv4.mr_rules_ops);
}
#else
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index 89c0b07..66078da 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -254,8 +254,10 @@ static void __net_exit ip6mr_rules_exit(struct net *net)
{
struct mr6_table *mrt, *next;
- list_for_each_entry_safe(mrt, next, &net->ipv6.mr6_tables, list)
+ list_for_each_entry_safe(mrt, next, &net->ipv6.mr6_tables, list) {
+ list_del(&mrt->list);
ip6mr_free_table(mrt);
+ }
fib_rules_unregister(net->ipv6.mr6_rules_ops);
}
#else
^ permalink raw reply related
* Re: [RFC][PATCH] Fix another namespace issue with devices assigned to classes
From: Johannes Berg @ 2010-06-07 9:42 UTC (permalink / raw)
To: Kay Sievers; +Cc: Eric W. Biederman, Greg KH, netdev
In-Reply-To: <AANLkTinUAR2gkY7ipgXKy-pMHcmLuouEeyXxnP97U_7N@mail.gmail.com>
On Sun, 2010-06-06 at 19:17 +0200, Kay Sievers wrote:
> There is no real difference between classes and buses. Actually we're
> working on merging them completely inside the kernel. Just declare a
> "struct bus_type" instead of a "struct class".
Can you please tell me then how to device_create() without a class? I
cannot seem to create devices without a class at all, even using manual
allocation (yuck) and device_register crashes the kernel.
johannes
^ permalink raw reply
* Re: [Patch 2/2] mlx4: add dynamic LRO disable support
From: Cong Wang @ 2010-06-07 8:51 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, herbert.xu, nhorman, sgruszka, davem
In-Reply-To: <1275661552.2095.13.camel@achroite.uk.solarflarecom.com>
On 06/04/10 22:25, Ben Hutchings wrote:
> On Fri, 2010-06-04 at 09:56 +0800, Cong Wang wrote:
>> On 06/03/10 20:37, Ben Hutchings wrote:
>>> On Wed, 2010-06-02 at 23:39 -0400, Amerigo Wang wrote:
>>>> This patch adds dynamic LRO diable support for mlx4 net driver.
>>>> It also fixes a bug of mlx4, which checks NETIF_F_LRO flag in rx
>>>> path without rtnl lock.
>>> [...]
>>>
>>> Is that flag test actually unsafe - and if so, how is testing num_lro
>>> any better? Perhaps access to net_device::features should be wrapped
>>> with ACCESS_ONCE() to ensure that reads and writes are atomic.
>>>
>>
>> At least, I don't find there is any race with 'num_lro', thus
>> no lock is needed.
>
> In both cases there is a race condition but it is harmless so long as
> the read and the write are atomic. There is a general assumption in
> networking code that this is the case for int and long. Personally I
> would prefer to see this made explicit using ACCESS_ONCE(), but I don't
> see any specific problem in mlx4 (not that I'm familiar with this driver
> either).
Hmm, right, it seems mlx4_en_add() is async too.
I will pick your suggestion.
>
> Now that I look at the patch again, I see you're using a static (i.e.
> global) variable to 'back up' the non-zero (enabled) value of num_lro.
> This is introducing a bug! The correct value is apparently set in
> mlx4_en_get_profile(); you would need to replicate that.
>
Oh, probably, but unfortunately 'num_lro' is static so only visible
in en_main.c.
Thanks!
^ permalink raw reply
* RE: [PATCH] r8169: fix random mdio_write failures
From: hayeswang @ 2010-06-07 9:26 UTC (permalink / raw)
To: 'Francois Romieu', 'Timo Teräs'; +Cc: netdev, davem
In-Reply-To: <20100605124103.GA3213@electric-eye.fr.zoreil.com>
Our hardware engineer suggests that check the completed indication
per 100 micro seconds. And it needs 20 micro seconds delay after the
completed indication for the next command.
Best Regards,
Hayes
-----Original Message-----
From: Francois Romieu [mailto:romieu@fr.zoreil.com]
Sent: Saturday, June 05, 2010 8:41 PM
To: Timo Teräs
Cc: netdev@vger.kernel.org; Edward Hsu; Hayeswang; davem@davemloft.net
Subject: Re: [PATCH] r8169: fix random mdio_write failures
Timo Teräs <timo.teras@iki.fi> :
[...]
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c index
> 217e709..03a8318 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
> @@ -559,6 +559,11 @@ static void mdio_write(void __iomem *ioaddr, int
reg_addr, int value)
> break;
> udelay(25);
> }
> + /*
> + * Some configurations require a small delay even after the write
> + * completed indication or the next write might fail.
> + */
> + udelay(25);
Acked-off-by: Francois Romieu <romieu@fr.zoreil.com>
Good work.
I wonder if increasing the in-loop delay as well would help the write
succeed faster (or slower ?).
--
Ueimor
------Please consider the environment before printing this e-mail.
^ permalink raw reply
* Re: in_dev->refcnt and dev->refcnt
From: Eric Dumazet @ 2010-06-07 9:30 UTC (permalink / raw)
To: ratheesh k; +Cc: netdev, linux-net
In-Reply-To: <AANLkTinijGlJRnqhcEbXhc84MXc62gbshxvTa6JeXo2K@mail.gmail.com>
Le lundi 07 juin 2010 à 14:50 +0530, ratheesh k a écrit :
> Is there any relation between in_dev->refcnt and dev->refcnt ? . My
> question is : if put or get on one variable affects other ?
>
No, there is no relation between them.
You can increase / decrease one refcount or the other, using separate
API.
^ permalink raw reply
* in_dev->refcnt and dev->refcnt
From: ratheesh k @ 2010-06-07 9:20 UTC (permalink / raw)
To: netdev, linux-net
Is there any relation between in_dev->refcnt and dev->refcnt ? . My
question is : if put or get on one variable affects other ?
-Ratheesh
^ permalink raw reply
* Re: [PATCH] virtio_net: indicate oom when addbuf returns failure
From: Michael S. Tsirkin @ 2010-06-07 9:15 UTC (permalink / raw)
To: Herbert Xu; +Cc: Bruce Rogers, netdev, Shirley Ma, virtualization, stable
In-Reply-To: <20100606222441.GA5992@gondor.apana.org.au>
On Mon, Jun 07, 2010 at 08:24:41AM +1000, Herbert Xu wrote:
> On Sun, Jun 06, 2010 at 11:13:00PM +0300, Michael S. Tsirkin wrote:
> >
> > Actually this code looks strange:
> > Note that add_buf inicates out of memory
> > condition with a positive return value, and ring full
> > (which is not an error!) with -ENOSPC.
>
> Indeed, this ultimately came from
>
> commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399
> Author: Shirley Ma <mashirle@us.ibm.com>
> Date: Fri Jan 29 03:20:04 2010 +0000
>
> virtio_net: Defer skb allocation in receive path Date: Wed, 13 Jan 2010 12:53:38 -0800
>
> (Greg, please don't apply this even though I've just given you
> the upstream commit ID that you were asking for :)
>
> where it confuses a memory allocation error with an add_buf failure.
>
> > Possibly the right thing to do is to
> > 1. handle ENOMEM specially
> > 2. fix add_buf to return ENOMEM on error
>
> I think we should make it so that only a memory allocation error
> is returned as before. There is no need for returning the add_buf
> error unless add_buf is now doing an allocation itself that needs
> to be retried.
That's what my patch did, right? Ack it?
> Thanks,
> --
> 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: [Patch] infiniband: check local reserved ports
From: Cong Wang @ 2010-06-07 9:04 UTC (permalink / raw)
To: Roland Dreier
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, Tetsuo Handa,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <ada1vcmpyvu.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>
On 06/05/10 00:04, Roland Dreier wrote:
> > > Should this inet_is_reserved_local_port() test apply to all the "port
> > > spaces" that this code is handling? I honestly am ignorant of the
> > > intended semantics of the new local_reserved_ports stuff, hence my question.
>
> > Yes, but I only found this case, is there any else?
>
> My question was more in the other direction: should this test apply to
> all the "port spaces" handled here? From looking at the code, it
> appears the answer is yes -- it seems that putting a port in
> local_reserved_ports reserves that port for IPv4 and IPv6, UDP, TCP,
> SCTP, DCCP, everything, so we should probably reserve all RDMA CM ports too.
Yes.
So this patch looks good for you? :)
Thanks.
--
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
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