[Linux-zigbee-devel] [PATCH] 6lowpan: Sync default hardware address of lowpan links to their wpan
When a lowpan link to a wpan device is created, set the hardware address of the lowpan link to that of the wpan device. Signed-off-by: Alan Ott --- net/ieee802154/6lowpan.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c index c85e71e..fb89133 100644 --- a/net/ieee802154/6lowpan.c +++ b/net/ieee802154/6lowpan.c @@ -1386,6 +1386,9 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev, entry->ldev = dev; + BUG_ON(IEEE802154_ADDR_LEN != dev->addr_len); + memcpy(dev->dev_addr, real_dev->dev_addr, IEEE802154_ADDR_LEN); + mutex_lock(&lowpan_dev_info(dev)->dev_list_mtx); INIT_LIST_HEAD(&entry->list); list_add_tail(&entry->list, &lowpan_devices); -- 1.8.1.2 -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
Re: [Linux-zigbee-devel] [PATCH] 6lowpan: Sync default hardware address of lowpan links to their wpan
Hi Alan, On Sat, Oct 05, 2013 at 05:38:00PM -0400, Alan Ott wrote: > When a lowpan link to a wpan device is created, set the hardware address > of the lowpan link to that of the wpan device. > > Signed-off-by: Alan Ott > --- > net/ieee802154/6lowpan.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c > index c85e71e..fb89133 100644 > --- a/net/ieee802154/6lowpan.c > +++ b/net/ieee802154/6lowpan.c > @@ -1386,6 +1386,9 @@ static int lowpan_newlink(struct net *src_net, struct > net_device *dev, > > entry->ldev = dev; > > + BUG_ON(IEEE802154_ADDR_LEN != dev->addr_len); So if somebody make a: "ip link add link $NOT_8BYTE_HWADDR_DEV name $NAME type lowpan" the kernel creates a BUG_ON? Okay it seems that case is very unusual but better is to return a errno so "maybe(I don't know it)" the userspace software will generate a error and the whole kernel doesn't crash. > + memcpy(dev->dev_addr, real_dev->dev_addr, IEEE802154_ADDR_LEN); Is there one case where we read the dev->dev_addr? I saw a case when we set this [1], but I don't know why we need this. Did you detect some problems because this isn't the "real" hw addr? Other case is I am feeling uneasy when we have two netdev devices with the same hw addr. - Alex [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/ieee802154/6lowpan.c?id=refs/tags/v3.12-rc3#n1102 -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
Re: [Linux-zigbee-devel] [PATCH] 6lowpan: Sync default hardware address of lowpan links to their wpan
On 10/05/2013 08:24 PM, Alexander Aring wrote: > Hi Alan, > > On Sat, Oct 05, 2013 at 05:38:00PM -0400, Alan Ott wrote: >> When a lowpan link to a wpan device is created, set the hardware address >> of the lowpan link to that of the wpan device. >> >> Signed-off-by: Alan Ott >> --- >> net/ieee802154/6lowpan.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c >> index c85e71e..fb89133 100644 >> --- a/net/ieee802154/6lowpan.c >> +++ b/net/ieee802154/6lowpan.c >> @@ -1386,6 +1386,9 @@ static int lowpan_newlink(struct net *src_net, struct >> net_device *dev, >> >> entry->ldev = dev; >> >> +BUG_ON(IEEE802154_ADDR_LEN != dev->addr_len); > So if somebody make a: > "ip link add link $NOT_8BYTE_HWADDR_DEV name $NAME type lowpan" > > the kernel creates a BUG_ON? Okay it seems that case is very unusual but > better is to return a errno so "maybe(I don't know it)" the userspace > software will generate a error and the whole kernel doesn't crash. That line checks the length of the address of the lowpan device, not the real device the lowpan device is attached to (and on second look, I don't like the Yoda code; didn't notice that before). Further, running: ip link add link eth0 name lowpan0 type lowpan like you suggest, crashes the kernel hard, with or without my patch. More stuff to fix >> +memcpy(dev->dev_addr, real_dev->dev_addr, IEEE802154_ADDR_LEN); > Is there one case where we read the dev->dev_addr? I saw a case when we > set this [1], but I don't know why we need this. Did you detect some > problems because this isn't the "real" hw addr? > > Other case is I am feeling uneasy when we have two netdev devices with > the same hw addr. > [1]https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/ieee802154/6lowpan.c?id=refs/tags/v3.12-rc3#n1102 In my testing, lowpan devices need a hardware address which is the same as the wpan, or else stuff like ping doesn't work at all. In all the examples I've ever seen, the lowpan's hardware address has been set manually. What does your test setup look like that you don't have a hardware address for your lowpan device? Are you able to ping other Linux hosts? Alan. -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
Re: [Linux-zigbee-devel] [PATCH] 6lowpan: Sync default hardware address of lowpan links to their wpan
On Sat, Oct 05, 2013 at 09:01:52PM -0400, Alan Ott wrote: > On 10/05/2013 08:24 PM, Alexander Aring wrote: > >Hi Alan, > > > >On Sat, Oct 05, 2013 at 05:38:00PM -0400, Alan Ott wrote: > >>When a lowpan link to a wpan device is created, set the hardware address > >>of the lowpan link to that of the wpan device. > >> > >>Signed-off-by: Alan Ott > >>--- > >> net/ieee802154/6lowpan.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >>diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c > >>index c85e71e..fb89133 100644 > >>--- a/net/ieee802154/6lowpan.c > >>+++ b/net/ieee802154/6lowpan.c > >>@@ -1386,6 +1386,9 @@ static int lowpan_newlink(struct net *src_net, struct > >>net_device *dev, > >>entry->ldev = dev; > >>+ BUG_ON(IEEE802154_ADDR_LEN != dev->addr_len); > >So if somebody make a: > >"ip link add link $NOT_8BYTE_HWADDR_DEV name $NAME type lowpan" > > > >the kernel creates a BUG_ON? Okay it seems that case is very unusual but > >better is to return a errno so "maybe(I don't know it)" the userspace > >software will generate a error and the whole kernel doesn't crash. > > That line checks the length of the address of the lowpan device, not > the real device the lowpan device is attached to (and on second > look, I don't like the Yoda code; didn't notice that before). > > Further, running: > ip link add link eth0 name lowpan0 type lowpan > > like you suggest, crashes the kernel hard, with or without my patch. > More stuff to fix > Okay isn't the dev->addr_len some static value? Maybe we should check than for: if (real_dev->type != ARPHRD_IEEE802154) return -EINVAL; after we set real_dev. It's all ARPHRD_IEEE802154 specific code. > > >>+ memcpy(dev->dev_addr, real_dev->dev_addr, IEEE802154_ADDR_LEN); > >Is there one case where we read the dev->dev_addr? I saw a case when we > >set this [1], but I don't know why we need this. Did you detect some > >problems because this isn't the "real" hw addr? > > > >Other case is I am feeling uneasy when we have two netdev devices with > >the same hw addr. > >[1]https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/ieee802154/6lowpan.c?id=refs/tags/v3.12-rc3#n1102 > > In my testing, lowpan devices need a hardware address which is the > same as the wpan, or else stuff like ping doesn't work at all. In > all the examples I've ever seen, the lowpan's hardware address has > been set manually. > Ah ok. > What does your test setup look like that you don't have a hardware > address for your lowpan device? Are you able to ping other Linux > hosts? > I have a little script for setting up a lowpan device. I set the hardware address with: ip link set $LOWPAN_DEV address $EUI64_ADDR after I add the lowpan link. I am able to ping linux and contiki(without fragmentation). - Alex -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
[Linux-zigbee-devel] [PATCH v2 2/2] 6lowpan: Sync default hardware address of lowpan links to their wpan
When a lowpan link to a wpan device is created, set the hardware address of the lowpan link to that of the wpan device. Signed-off-by: Alan Ott --- net/ieee802154/6lowpan.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c index 8f56b2b..ff41b4d 100644 --- a/net/ieee802154/6lowpan.c +++ b/net/ieee802154/6lowpan.c @@ -1388,6 +1388,9 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev, entry->ldev = dev; + /* Set the lowpan harware address to the wpan hardware address. */ + memcpy(dev->dev_addr, real_dev->dev_addr, IEEE802154_ADDR_LEN); + mutex_lock(&lowpan_dev_info(dev)->dev_list_mtx); INIT_LIST_HEAD(&entry->list); list_add_tail(&entry->list, &lowpan_devices); -- 1.8.1.2 -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
[Linux-zigbee-devel] [PATCH v2 0/2] 6lowpan default hardware address
Alexander Aring suggested that devices desired to be linked to 6lowpan be checked for actually being of type IEEE802154, since IEEE802154 devices are all that are supported by 6lowpan at present. Alan Ott (2): 6lowpan: Only make 6lowpan links to IEEE802154 devices 6lowpan: Sync default hardware address of lowpan links to their wpan net/ieee802154/6lowpan.c | 5 + 1 file changed, 5 insertions(+) -- 1.8.1.2 -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
[Linux-zigbee-devel] [PATCH v2 1/2] 6lowpan: Only make 6lowpan links to IEEE802154 devices
Refuse to create 6lowpan links if the actual hardware interface is of any type other than ARPHRD_IEEE802154. Signed-off-by: Alan Ott Suggested-by: Alexander Aring --- net/ieee802154/6lowpan.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c index c85e71e..8f56b2b 100644 --- a/net/ieee802154/6lowpan.c +++ b/net/ieee802154/6lowpan.c @@ -1372,6 +1372,8 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev, real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK])); if (!real_dev) return -ENODEV; + if (real_dev->type != ARPHRD_IEEE802154) + return -EINVAL; lowpan_dev_info(dev)->real_dev = real_dev; lowpan_dev_info(dev)->fragment_tag = 0; -- 1.8.1.2 -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
[Linux-zigbee-devel] [PATCH v1 0/3] Fix race conditions in mrf24j40 interrupts
After testing with the betas of this patchset, it's been rebased and is ready for inclusion. David Hauweele noticed that the mrf24j40 would hang arbitrarily after some period of heavy traffic. Two race conditions were discovered, and the driver was changed to use threaded interrupts, since the enable/disable of interrupts in the driver has recently been a lighning rod whenever issues arise related to interrupts (costing engineering time), and since threaded interrupts are the right way to do it. Alan Ott (3): mrf24j40: Move INIT_COMPLETION() to before packet transmission mrf24j40: Use threaded IRQ handler mrf24j40: Use level-triggered interrupts drivers/net/ieee802154/mrf24j40.c | 31 +-- 1 file changed, 9 insertions(+), 22 deletions(-) -- 1.8.1.2 -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
[Linux-zigbee-devel] [PATCH v1 2/3] mrf24j40: Use threaded IRQ handler
Eliminate all the workqueue and interrupt enable/disable. Signed-off-by: Alan Ott --- drivers/net/ieee802154/mrf24j40.c | 27 +++ 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c index 66bb4ce..c1bc688 100644 --- a/drivers/net/ieee802154/mrf24j40.c +++ b/drivers/net/ieee802154/mrf24j40.c @@ -82,7 +82,6 @@ struct mrf24j40 { struct mutex buffer_mutex; /* only used to protect buf */ struct completion tx_complete; - struct work_struct irqwork; u8 *buf; /* 3 bytes. Used for SPI single-register transfers. */ }; @@ -590,17 +589,6 @@ static struct ieee802154_ops mrf24j40_ops = { static irqreturn_t mrf24j40_isr(int irq, void *data) { struct mrf24j40 *devrec = data; - - disable_irq_nosync(irq); - - schedule_work(&devrec->irqwork); - - return IRQ_HANDLED; -} - -static void mrf24j40_isrwork(struct work_struct *work) -{ - struct mrf24j40 *devrec = container_of(work, struct mrf24j40, irqwork); u8 intstat; int ret; @@ -618,7 +606,7 @@ static void mrf24j40_isrwork(struct work_struct *work) mrf24j40_handle_rx(devrec); out: - enable_irq(devrec->spi->irq); + return IRQ_HANDLED; } static int mrf24j40_probe(struct spi_device *spi) @@ -642,7 +630,6 @@ static int mrf24j40_probe(struct spi_device *spi) mutex_init(&devrec->buffer_mutex); init_completion(&devrec->tx_complete); - INIT_WORK(&devrec->irqwork, mrf24j40_isrwork); devrec->spi = spi; spi_set_drvdata(spi, devrec); @@ -688,11 +675,12 @@ static int mrf24j40_probe(struct spi_device *spi) val &= ~0x3; /* Clear RX mode (normal) */ write_short_reg(devrec, REG_RXMCR, val); - ret = request_irq(spi->irq, - mrf24j40_isr, - IRQF_TRIGGER_FALLING, - dev_name(&spi->dev), - devrec); + ret = request_threaded_irq(spi->irq, + NULL, + mrf24j40_isr, + IRQF_TRIGGER_FALLING|IRQF_ONESHOT, + dev_name(&spi->dev), + devrec); if (ret) { dev_err(printdev(devrec), "Unable to get IRQ"); @@ -721,7 +709,6 @@ static int mrf24j40_remove(struct spi_device *spi) dev_dbg(printdev(devrec), "remove\n"); free_irq(spi->irq, devrec); - flush_work(&devrec->irqwork); /* TODO: Is this the right call? */ ieee802154_unregister_device(devrec->dev); ieee802154_free_device(devrec->dev); /* TODO: Will ieee802154_free_device() wait until ->xmit() is -- 1.8.1.2 -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
[Linux-zigbee-devel] [PATCH v1 1/3] mrf24j40: Move INIT_COMPLETION() to before packet transmission
This avoids a race condition where complete(tx_complete) could be called before tx_complete is initialized. Signed-off-by: Alan Ott --- drivers/net/ieee802154/mrf24j40.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c index 42e6dee..66bb4ce 100644 --- a/drivers/net/ieee802154/mrf24j40.c +++ b/drivers/net/ieee802154/mrf24j40.c @@ -344,6 +344,8 @@ static int mrf24j40_tx(struct ieee802154_dev *dev, struct sk_buff *skb) if (ret) goto err; + INIT_COMPLETION(devrec->tx_complete); + /* Set TXNTRIG bit of TXNCON to send packet */ ret = read_short_reg(devrec, REG_TXNCON, &val); if (ret) @@ -354,8 +356,6 @@ static int mrf24j40_tx(struct ieee802154_dev *dev, struct sk_buff *skb) val |= 0x4; write_short_reg(devrec, REG_TXNCON, val); - INIT_COMPLETION(devrec->tx_complete); - /* Wait for the device to send the TX complete interrupt. */ ret = wait_for_completion_interruptible_timeout( &devrec->tx_complete, -- 1.8.1.2 -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel
[Linux-zigbee-devel] [PATCH v1 3/3] mrf24j40: Use level-triggered interrupts
The mrf24j40 generates level interrupts. There are rare cases where it appears that the interrupt line never gets de-asserted between interrupts, causing interrupts to be lost, and causing a hung device from the driver's perspective. Switching the driver to interpret these interrupts as level-triggered fixes this issue. Signed-off-by: Alan Ott --- drivers/net/ieee802154/mrf24j40.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ieee802154/mrf24j40.c b/drivers/net/ieee802154/mrf24j40.c index c1bc688..0632d34 100644 --- a/drivers/net/ieee802154/mrf24j40.c +++ b/drivers/net/ieee802154/mrf24j40.c @@ -678,7 +678,7 @@ static int mrf24j40_probe(struct spi_device *spi) ret = request_threaded_irq(spi->irq, NULL, mrf24j40_isr, - IRQF_TRIGGER_FALLING|IRQF_ONESHOT, + IRQF_TRIGGER_LOW|IRQF_ONESHOT, dev_name(&spi->dev), devrec); -- 1.8.1.2 -- October Webinars: Code for Performance Free Intel webinars can help you accelerate application performance. Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from the latest Intel processors and coprocessors. See abstracts and register > http://pubads.g.doubleclick.net/gampad/clk?id=60134791&iu=/4140/ostg.clktrk ___ Linux-zigbee-devel mailing list Linux-zigbee-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel