In preparation to a wider series, here are a number of small and random
fixes across the subsystem.
Miquel Raynal (9):
net: ieee802154: hwsim: Ensure proper channel selection at probe time
net: ieee802154: hwsim: Ensure frame checksum are valid
net: ieee802154: mcr20a: Fix lifs/sifs periods
net: ieee802154: at86rf230: Stop leaking skb's
net: ieee802154: ca8210: Stop leaking skb's
net: ieee802154: Use the IEEE802154_MAX_PAGE define when relevant
net: ieee802154: Return meaningful error codes from the netlink
helpers
net: mac802154: Explain the use of ieee802154_wake/stop_queue()
MAINTAINERS: Remove Harry Morris bouncing address
MAINTAINERS | 3 +--
drivers/net/ieee802154/at86rf230.c | 1 +
drivers/net/ieee802154/ca8210.c | 1 +
drivers/net/ieee802154/mac802154_hwsim.c | 12 ++----------
drivers/net/ieee802154/mcr20a.c | 4 ++--
include/net/mac802154.h | 12 ++++++++++++
net/ieee802154/nl-phy.c | 5 +++--
net/ieee802154/nl802154.c | 8 ++++----
8 files changed, 26 insertions(+), 20 deletions(-)
--
2.27.0
Drivers are expected to set the PHY current_channel and current_page
according to their default state. The hwsim driver is advertising being
configured on channel 13 by default but that is not reflected in its own
internal pib structure. In order to ensure that this driver consider the
current channel as being 13 internally, we can call hwsim_hw_channel()
instead of creating an empty pib structure.
We assume here that kvfree_rcu(NULL) is a valid call.
Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/net/ieee802154/mac802154_hwsim.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 8caa61ec718f..795f8eb5387b 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -732,7 +732,6 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
{
struct ieee802154_hw *hw;
struct hwsim_phy *phy;
- struct hwsim_pib *pib;
int idx;
int err;
@@ -780,13 +779,8 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
/* hwsim phy channel 13 as default */
hw->phy->current_channel = 13;
- pib = kzalloc(sizeof(*pib), GFP_KERNEL);
- if (!pib) {
- err = -ENOMEM;
- goto err_pib;
- }
+ hwsim_hw_channel(hw, hw->phy->current_page, hw->phy->current_channel);
- rcu_assign_pointer(phy->pib, pib);
phy->idx = idx;
INIT_LIST_HEAD(&phy->edges);
@@ -815,8 +809,6 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
err_subscribe:
ieee802154_unregister_hw(phy->hw);
err_reg:
- kfree(pib);
-err_pib:
ieee802154_free_hw(phy->hw);
return err;
}
--
2.27.0
There is no point in accepting frames with a wrong or missing checksum,
at least not outside of a promiscuous setting. Set the right flag by
default in the hwsim driver to ensure checksums are not ignored.
Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/net/ieee802154/mac802154_hwsim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 795f8eb5387b..5324d0eda223 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -784,7 +784,7 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
phy->idx = idx;
INIT_LIST_HEAD(&phy->edges);
- hw->flags = IEEE802154_HW_PROMISCUOUS;
+ hw->flags = IEEE802154_HW_PROMISCUOUS | IEEE802154_HW_RX_DROP_BAD_CKSUM;
hw->parent = dev;
err = ieee802154_register_hw(hw);
--
2.27.0
These periods are expressed in time units (microseconds) while 40 and 12
are the number of symbol durations these periods will last. We need to
multiply them both with phy->symbol_duration in order to get these
values in microseconds.
Fixes: 8c6ad9cc5157 ("ieee802154: Add NXP MCR20A IEEE 802.15.4 transceiver driver")
Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/net/ieee802154/mcr20a.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c
index 8dc04e2590b1..383231b85464 100644
--- a/drivers/net/ieee802154/mcr20a.c
+++ b/drivers/net/ieee802154/mcr20a.c
@@ -976,8 +976,8 @@ static void mcr20a_hw_setup(struct mcr20a_local *lp)
dev_dbg(printdev(lp), "%s\n", __func__);
phy->symbol_duration = 16;
- phy->lifs_period = 40;
- phy->sifs_period = 12;
+ phy->lifs_period = 40 * phy->symbol_duration;
+ phy->sifs_period = 12 * phy->symbol_duration;
hw->flags = IEEE802154_HW_TX_OMIT_CKSUM |
IEEE802154_HW_AFILT |
--
2.27.0
Upon error the ieee802154_xmit_complete() helper is not called. Only
ieee802154_wake_queue() is called manually. We then leak the skb
structure.
Free the skb structure upon error before returning.
There is no Fixes tag applying here, many changes have been made on this
area and the issue kind of always existed.
Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/net/ieee802154/at86rf230.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 7d67f41387f5..0746150f78cf 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -344,6 +344,7 @@ at86rf230_async_error_recover_complete(void *context)
kfree(ctx);
ieee802154_wake_queue(lp->hw);
+ dev_kfree_skb_any(lp->tx_skb);
}
static void
--
2.27.0
This define already exist but is hardcoded in nl-phy.c. Use the
definition when relevant.
Signed-off-by: Miquel Raynal <[email protected]>
---
net/ieee802154/nl-phy.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/ieee802154/nl-phy.c b/net/ieee802154/nl-phy.c
index dd5a45f8a78a..02f6a53d0faa 100644
--- a/net/ieee802154/nl-phy.c
+++ b/net/ieee802154/nl-phy.c
@@ -30,7 +30,8 @@ static int ieee802154_nl_fill_phy(struct sk_buff *msg, u32 portid,
{
void *hdr;
int i, pages = 0;
- uint32_t *buf = kcalloc(32, sizeof(uint32_t), GFP_KERNEL);
+ uint32_t *buf = kcalloc(IEEE802154_MAX_PAGE + 1, sizeof(uint32_t),
+ GFP_KERNEL);
pr_debug("%s\n", __func__);
@@ -47,7 +48,7 @@ static int ieee802154_nl_fill_phy(struct sk_buff *msg, u32 portid,
nla_put_u8(msg, IEEE802154_ATTR_PAGE, phy->current_page) ||
nla_put_u8(msg, IEEE802154_ATTR_CHANNEL, phy->current_channel))
goto nla_put_failure;
- for (i = 0; i < 32; i++) {
+ for (i = 0; i <= IEEE802154_MAX_PAGE; i++) {
if (phy->supported.channels[i])
buf[pages++] = phy->supported.channels[i] | (i << 27);
}
--
2.27.0
Upon error the ieee802154_xmit_complete() helper is not called. Only
ieee802154_wake_queue() is called manually. We then leak the skb
structure.
Free the skb structure upon error before returning.
Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Miquel Raynal <[email protected]>
---
drivers/net/ieee802154/ca8210.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index ece6ff6049f6..5d1b356cb9d3 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -1772,6 +1772,7 @@ static int ca8210_async_xmit_complete(
);
if (status != MAC_TRANSACTION_OVERFLOW) {
ieee802154_wake_queue(priv->hw);
+ dev_kfree_skb_any(atusb->tx_skb);
return 0;
}
}
--
2.27.0
Harry's e-mail address from Cascoda bounces, I have not found any
contributions from him since 2018 so let's drop the Maintainer entry
from the CA8210 driver and mark it Orphan.
Signed-off-by: Miquel Raynal <[email protected]>
---
MAINTAINERS | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4d479b554361..ab2b32080b73 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4099,9 +4099,8 @@ N: csky
K: csky
CA8210 IEEE-802.15.4 RADIO DRIVER
-M: Harry Morris <[email protected]>
L: [email protected]
-S: Maintained
+S: Orphan
W: https://github.com/Cascoda/ca8210-linux.git
F: Documentation/devicetree/bindings/net/ieee802154/ca8210.txt
F: drivers/net/ieee802154/ca8210.c
--
2.27.0
It is not straightforward to the newcomer that a single skb can be sent
at a time and that the internal process is to stop the queue when
processing a frame before re-enabling it. Make this clear by documenting
the ieee802154_wake/stop_queue() helpers.
Signed-off-by: Miquel Raynal <[email protected]>
---
include/net/mac802154.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/net/mac802154.h b/include/net/mac802154.h
index d524ffb9eb25..94b2e3008e77 100644
--- a/include/net/mac802154.h
+++ b/include/net/mac802154.h
@@ -464,6 +464,12 @@ void ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct sk_buff *skb,
* ieee802154_wake_queue - wake ieee802154 queue
* @hw: pointer as obtained from ieee802154_alloc_hw().
*
+ * Tranceivers have either one transmit framebuffer or one framebuffer for both
+ * transmitting and receiving. Hence, the core only handles one frame at a time
+ * for each phy, which means we had to stop the queue to avoid new skb to come
+ * during the transmission. The queue then needs to be woken up after the
+ * operation.
+ *
* Drivers should use this function instead of netif_wake_queue.
*/
void ieee802154_wake_queue(struct ieee802154_hw *hw);
@@ -472,6 +478,12 @@ void ieee802154_wake_queue(struct ieee802154_hw *hw);
* ieee802154_stop_queue - stop ieee802154 queue
* @hw: pointer as obtained from ieee802154_alloc_hw().
*
+ * Tranceivers have either one transmit framebuffer or one framebuffer for both
+ * transmitting and receiving. Hence, the core only handles one frame at a time
+ * for each phy, which means we need to tell upper layers to stop giving us new
+ * skbs while we are busy with the transmitted one. The queue must then be
+ * stopped before transmitting.
+ *
* Drivers should use this function instead of netif_stop_queue.
*/
void ieee802154_stop_queue(struct ieee802154_hw *hw);
--
2.27.0
Returning -1 does not indicate anything useful.
Use a standard and meaningful error code instead.
Signed-off-by: Miquel Raynal <[email protected]>
---
net/ieee802154/nl802154.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 277124f206e0..e0b072aecf0f 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -1441,7 +1441,7 @@ static int nl802154_send_key(struct sk_buff *msg, u32 cmd, u32 portid,
hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
if (!hdr)
- return -1;
+ return -ENOBUFS;
if (nla_put_u32(msg, NL802154_ATTR_IFINDEX, dev->ifindex))
goto nla_put_failure;
@@ -1634,7 +1634,7 @@ static int nl802154_send_device(struct sk_buff *msg, u32 cmd, u32 portid,
hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
if (!hdr)
- return -1;
+ return -ENOBUFS;
if (nla_put_u32(msg, NL802154_ATTR_IFINDEX, dev->ifindex))
goto nla_put_failure;
@@ -1812,7 +1812,7 @@ static int nl802154_send_devkey(struct sk_buff *msg, u32 cmd, u32 portid,
hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
if (!hdr)
- return -1;
+ return -ENOBUFS;
if (nla_put_u32(msg, NL802154_ATTR_IFINDEX, dev->ifindex))
goto nla_put_failure;
@@ -1988,7 +1988,7 @@ static int nl802154_send_seclevel(struct sk_buff *msg, u32 cmd, u32 portid,
hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
if (!hdr)
- return -1;
+ return -ENOBUFS;
if (nla_put_u32(msg, NL802154_ATTR_IFINDEX, dev->ifindex))
goto nla_put_failure;
--
2.27.0
Hi Miquel,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.16 next-20220120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Miquel-Raynal/ieee802154-A-bunch-of-fixes/20220120-083906
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1d1df41c5a33359a00e919d54eaebfb789711fdc
config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220120/[email protected]/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project f7b7138a62648f4019c55e4671682af1f851f295)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/77d3026b30aff560ef269d03aecc09f8c46a9173
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Miquel-Raynal/ieee802154-A-bunch-of-fixes/20220120-083906
git checkout 77d3026b30aff560ef269d03aecc09f8c46a9173
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/ieee802154/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/net/ieee802154/ca8210.c:1775:22: error: use of undeclared identifier 'atusb'
dev_kfree_skb_any(atusb->tx_skb);
^
1 error generated.
vim +/atusb +1775 drivers/net/ieee802154/ca8210.c
1737
1738 /**
1739 * ca8210_async_xmit_complete() - Called to announce that an asynchronous
1740 * transmission has finished
1741 * @hw: ieee802154_hw of ca8210 that has finished exchange
1742 * @msduhandle: Identifier of transmission that has completed
1743 * @status: Returned 802.15.4 status code of the transmission
1744 *
1745 * Return: 0 or linux error code
1746 */
1747 static int ca8210_async_xmit_complete(
1748 struct ieee802154_hw *hw,
1749 u8 msduhandle,
1750 u8 status)
1751 {
1752 struct ca8210_priv *priv = hw->priv;
1753
1754 if (priv->nextmsduhandle != msduhandle) {
1755 dev_err(
1756 &priv->spi->dev,
1757 "Unexpected msdu_handle on data confirm, Expected %d, got %d\n",
1758 priv->nextmsduhandle,
1759 msduhandle
1760 );
1761 return -EIO;
1762 }
1763
1764 priv->async_tx_pending = false;
1765 priv->nextmsduhandle++;
1766
1767 if (status) {
1768 dev_err(
1769 &priv->spi->dev,
1770 "Link transmission unsuccessful, status = %d\n",
1771 status
1772 );
1773 if (status != MAC_TRANSACTION_OVERFLOW) {
1774 ieee802154_wake_queue(priv->hw);
> 1775 dev_kfree_skb_any(atusb->tx_skb);
1776 return 0;
1777 }
1778 }
1779 ieee802154_xmit_complete(priv->hw, priv->tx_skb, true);
1780
1781 return 0;
1782 }
1783
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
[email protected] wrote on Thu, 20 Jan 2022 15:31:39 +0800:
> Hi Miquel,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.16 next-20220120]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Miquel-Raynal/ieee802154-A-bunch-of-fixes/20220120-083906
> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1d1df41c5a33359a00e919d54eaebfb789711fdc
> config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220120/[email protected]/config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project f7b7138a62648f4019c55e4671682af1f851f295)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/0day-ci/linux/commit/77d3026b30aff560ef269d03aecc09f8c46a9173
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Miquel-Raynal/ieee802154-A-bunch-of-fixes/20220120-083906
> git checkout 77d3026b30aff560ef269d03aecc09f8c46a9173
> # save the config file to linux build tree
> mkdir build_dir
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/ieee802154/
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> >> drivers/net/ieee802154/ca8210.c:1775:22: error: use of undeclared identifier 'atusb'
> dev_kfree_skb_any(atusb->tx_skb);
> ^
> 1 error generated.
>
>
> vim +/atusb +1775 drivers/net/ieee802154/ca8210.c
>
> 1737
> 1738 /**
> 1739 * ca8210_async_xmit_complete() - Called to announce that an asynchronous
> 1740 * transmission has finished
> 1741 * @hw: ieee802154_hw of ca8210 that has finished exchange
> 1742 * @msduhandle: Identifier of transmission that has completed
> 1743 * @status: Returned 802.15.4 status code of the transmission
> 1744 *
> 1745 * Return: 0 or linux error code
> 1746 */
> 1747 static int ca8210_async_xmit_complete(
> 1748 struct ieee802154_hw *hw,
> 1749 u8 msduhandle,
> 1750 u8 status)
> 1751 {
> 1752 struct ca8210_priv *priv = hw->priv;
> 1753
> 1754 if (priv->nextmsduhandle != msduhandle) {
> 1755 dev_err(
> 1756 &priv->spi->dev,
> 1757 "Unexpected msdu_handle on data confirm, Expected %d, got %d\n",
> 1758 priv->nextmsduhandle,
> 1759 msduhandle
> 1760 );
> 1761 return -EIO;
> 1762 }
> 1763
> 1764 priv->async_tx_pending = false;
> 1765 priv->nextmsduhandle++;
> 1766
> 1767 if (status) {
> 1768 dev_err(
> 1769 &priv->spi->dev,
> 1770 "Link transmission unsuccessful, status = %d\n",
> 1771 status
> 1772 );
> 1773 if (status != MAC_TRANSACTION_OVERFLOW) {
> 1774 ieee802154_wake_queue(priv->hw);
> > 1775 dev_kfree_skb_any(atusb->tx_skb);
Looks like I messed with the configuration and this driver was not
compile-tested anymore. I'll fix this.
> 1776 return 0;
> 1777 }
> 1778 }
> 1779 ieee802154_xmit_complete(priv->hw, priv->tx_skb, true);
> 1780
> 1781 return 0;
> 1782 }
> 1783
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
Thanks,
Miquèl