From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.kundenserver.de ([212.227.17.13]:60251 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934451AbbEOPCg (ORCPT ); Fri, 15 May 2015 11:02:36 -0400 Message-ID: <55560A88.1050903@xsilon.com> Date: Fri, 15 May 2015 16:02:32 +0100 From: Simon Vincent MIME-Version: 1.0 Subject: Re: Kernel crash when using multiple interfaces References: <5555EC72.6060302@xsilon.com> <20150515142026.GA11157@omega> In-Reply-To: <20150515142026.GA11157@omega> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-wpan-owner@vger.kernel.org List-ID: To: Alexander Aring Cc: linux-wpan@vger.kernel.org I tried the solution you proposed but it did not work, it resulted in a kernel crash (bad paging request). We require multiple 802.15.4 interfaces for routing between different 802.15.4 networks. We have a 802.15.4 powerline phy (Hanadu [1]) and a 802.15.4 radio phy (MRF24J40). On some boxes we have both phys and route using RPL between the powerline and radio networks. [1] http://www.hanadu.org/ - Simon On 15/05/15 15:20, Alexander Aring wrote: > Hi Simon. > > On Fri, May 15, 2015 at 01:54:10PM +0100, Simon Vincent wrote: >> I have found the Kernel crashes when multiple 802.15.4 interfaces are used >> at the same time. >> I have tracked it down in the kernel to net/mac802154/tx.c >> The problem is the ieee802154_xmit_cb is a global variable so after it has >> been assigned and added to the work queue it can be corrupted/changed by >> another interface transmitting a packet. >> >> I have fixed it by allocating the structure on the heap. If this is a >> satisfactory fix I can submit it as a patch. >> > first, thank you for finding this issue. > > Yes, with multiple interfaces this code have issues. When we declare > ieee802154_xmit_cb per phy instead of one for every phy this should be > fixed. > > Can you try the following? This should be a solution without calling kmalloc > in this callback: > > diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h > index eec668f..c9b38c0 100644 > --- a/net/mac802154/ieee802154_i.h > +++ b/net/mac802154/ieee802154_i.h > @@ -28,11 +28,21 @@ > > #include "llsec.h" > > + > /* mac802154 device private data */ > struct ieee802154_local { > struct ieee802154_hw hw; > const struct ieee802154_ops *ops; > > + /* xmit worker handler > + * TODO remove this xmit_sync is debprecated > + */ > + struct ieee802154_xmit_cb { > + struct sk_buff *skb; > + struct work_struct work; > + struct ieee802154_local *local; > + } xmit_cb; > + > /* ieee802154 phy */ > struct wpan_phy *phy; > > diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c > index c62e956..afbddc0 100644 > --- a/net/mac802154/tx.c > +++ b/net/mac802154/tx.c > @@ -30,17 +30,6 @@ > #include "ieee802154_i.h" > #include "driver-ops.h" > > -/* IEEE 802.15.4 transceivers can sleep during the xmit session, so process > - * packets through the workqueue. > - */ > -struct ieee802154_xmit_cb { > - struct sk_buff *skb; > - struct work_struct work; > - struct ieee802154_local *local; > -}; > - > -static struct ieee802154_xmit_cb ieee802154_xmit_cb; > - > static void ieee802154_xmit_worker(struct work_struct *work) > { > struct ieee802154_xmit_cb *cb = > @@ -106,11 +95,11 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) > dev->stats.tx_packets++; > dev->stats.tx_bytes += skb->len; > } else { > - INIT_WORK(&ieee802154_xmit_cb.work, ieee802154_xmit_worker); > - ieee802154_xmit_cb.skb = skb; > - ieee802154_xmit_cb.local = local; > + INIT_WORK(&local->xmit_cb.work, ieee802154_xmit_worker); > + local->xmit_cb.skb = skb; > + local->xmit_cb.local = local; > > - queue_work(local->workqueue, &ieee802154_xmit_cb.work); > + queue_work(local->workqueue, &local->xmit_cb.work); > } > > return NETDEV_TX_OK; > > > > > I have anoother question? Which is your use case to use multiple > interfaces? I am still searching for one... The most multiple interfaces > can't be running because if the phy sets registers like (address > filters, promiscuous mode, etc) we can't running another interface with > different mac values (because we have only one registers in the phy). > > This means we have a lot of multiple wpan interfaces with the same mac > address which represents one. See [0] which checks on different > settings. > > The only one driver (which don't do any mac functionality) is the fakelb > driver. This driver is compareable with a phy without any mac > functionality. > > I was thinking about to remove the multiple interface support and only > have one interface which can have different types, so you could morph > node to monitor and backwards for example. But maybe there exists a > real use-case for have multiple interfaces on one phy... > > - Alex > > [0] http://lxr.free-electrons.com/source/net/mac802154/iface.c#L165