From be8c6d86d558860bbc493ad3fbc51fc96ab0c2e4 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:06 +0200 Subject: [PATCH 01/28] net: mac802154: Rename the synchronous xmit worker There are currently two driver hooks: one is synchronous, the other is not. We cannot rely on driver implementations to provide a synchronous API (which is related to the bus medium more than a wish to have a synchronized implementation) so we are going to introduce a sync API above any kind of driver transmit function. In order to clarify what this worker is for (synchronous driver implementation), let's rename it so that people don't get bothered by the fact that their driver does not make use of the "xmit worker" which is a too generic name. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-2-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/ieee802154_i.h | 2 +- net/mac802154/main.c | 2 +- net/mac802154/tx.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index 1381e6a5e1800..d7632c6d225f1 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -123,7 +123,7 @@ ieee802154_sdata_running(struct ieee802154_sub_if_data *sdata) extern struct ieee802154_mlme_ops mac802154_mlme_wpan; void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb); -void ieee802154_xmit_worker(struct work_struct *work); +void ieee802154_xmit_sync_worker(struct work_struct *work); netdev_tx_t ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev); netdev_tx_t diff --git a/net/mac802154/main.c b/net/mac802154/main.c index bd7bdb1219dd8..392771bba9dd7 100644 --- a/net/mac802154/main.c +++ b/net/mac802154/main.c @@ -95,7 +95,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops) skb_queue_head_init(&local->skb_queue); - INIT_WORK(&local->tx_work, ieee802154_xmit_worker); + INIT_WORK(&local->tx_work, ieee802154_xmit_sync_worker); /* init supported flags with 802.15.4 default ranges */ phy->supported.max_minbe = 8; diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index c829e4a753256..97df5985b8307 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -22,7 +22,7 @@ #include "ieee802154_i.h" #include "driver-ops.h" -void ieee802154_xmit_worker(struct work_struct *work) +void ieee802154_xmit_sync_worker(struct work_struct *work) { struct ieee802154_local *local = container_of(work, struct ieee802154_local, tx_work); From 983a974b40f66d202b075ab8ac584c698a3fc141 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:07 +0200 Subject: [PATCH 02/28] net: mac802154: Rename the main tx_work struct This entry is dedicated to synchronous transmissions done by drivers without async hook. Make this clearer that this is not a work that any driver can use by at least prefixing it with "sync_". While at it, let's enhance the comment explaining why we choose one or the other. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-3-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/ieee802154_i.h | 2 +- net/mac802154/main.c | 2 +- net/mac802154/tx.c | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index d7632c6d225f1..a8b7b9049f14e 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -55,7 +55,7 @@ struct ieee802154_local { struct sk_buff_head skb_queue; struct sk_buff *tx_skb; - struct work_struct tx_work; + struct work_struct sync_tx_work; /* A negative Linux error code or a null/positive MLME error status */ int tx_result; }; diff --git a/net/mac802154/main.c b/net/mac802154/main.c index 392771bba9dd7..40fab08df24ba 100644 --- a/net/mac802154/main.c +++ b/net/mac802154/main.c @@ -95,7 +95,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops) skb_queue_head_init(&local->skb_queue); - INIT_WORK(&local->tx_work, ieee802154_xmit_sync_worker); + INIT_WORK(&local->sync_tx_work, ieee802154_xmit_sync_worker); /* init supported flags with 802.15.4 default ranges */ phy->supported.max_minbe = 8; diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 97df5985b8307..a01689ddd5470 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -25,7 +25,7 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) { struct ieee802154_local *local = - container_of(work, struct ieee802154_local, tx_work); + container_of(work, struct ieee802154_local, sync_tx_work); struct sk_buff *skb = local->tx_skb; struct net_device *dev = skb->dev; int res; @@ -76,7 +76,10 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) /* Stop the netif queue on each sub_if_data object. */ ieee802154_stop_queue(&local->hw); - /* async is priority, otherwise sync is fallback */ + /* Drivers should preferably implement the async callback. In some rare + * cases they only provide a sync callback which we will use as a + * fallback. + */ if (local->ops->xmit_async) { unsigned int len = skb->len; @@ -90,7 +93,7 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) dev->stats.tx_bytes += len; } else { local->tx_skb = skb; - queue_work(local->workqueue, &local->tx_work); + queue_work(local->workqueue, &local->sync_tx_work); } return NETDEV_TX_OK; From d08d951a9ae7e30fc0421a1af2b4193ce4aa6b3e Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:08 +0200 Subject: [PATCH 03/28] net: mac802154: Enhance the error path in the main tx helper Before adding more logic in the error path, let's move the wake queue call there, rename the default label and create an additional one. There is no functional change. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-4-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/tx.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index a01689ddd5470..4a46ce8d2ac83 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -65,7 +65,7 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) consume_skb(skb); skb = nskb; } else { - goto err_tx; + goto err_free_skb; } } @@ -84,10 +84,8 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) unsigned int len = skb->len; ret = drv_xmit_async(local, skb); - if (ret) { - ieee802154_wake_queue(&local->hw); - goto err_tx; - } + if (ret) + goto err_wake_netif_queue; dev->stats.tx_packets++; dev->stats.tx_bytes += len; @@ -98,7 +96,9 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) return NETDEV_TX_OK; -err_tx: +err_wake_netif_queue: + ieee802154_wake_queue(&local->hw); +err_free_skb: kfree_skb(skb); return NETDEV_TX_OK; } From bde000ae459f2829ed88e967f7fa7665b4e3afaf Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:09 +0200 Subject: [PATCH 04/28] net: mac802154: Follow the count of ongoing transmissions In order to create a synchronous API for MLME command purposes, we need to be able to track the end of the ongoing transmissions. Let's introduce an atomic variable which is incremented when a transmission starts and decremented when relevant so that we know at any moment whether there is an ongoing transmission. The counter gets decremented in the following situations: - The operation is asynchronous and there was a failure during the offloading process. - The operation is synchronous and the synchronous operation failed. - The operation finished, either successfully or not. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-5-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- include/net/cfg802154.h | 3 +++ net/mac802154/tx.c | 3 +++ net/mac802154/util.c | 2 ++ 3 files changed, 8 insertions(+) diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index d8d8719315fd8..678ff00c7d701 100644 --- a/include/net/cfg802154.h +++ b/include/net/cfg802154.h @@ -214,6 +214,9 @@ struct wpan_phy { /* the network namespace this phy lives in currently */ possible_net_t _net; + /* Transmission monitoring */ + atomic_t ongoing_txs; + char priv[] __aligned(NETDEV_ALIGN); }; diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 4a46ce8d2ac83..33f64ecd96c7b 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -44,6 +44,7 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) err_tx: /* Restart the netif queue on each sub_if_data object. */ ieee802154_wake_queue(&local->hw); + atomic_dec(&local->phy->ongoing_txs); kfree_skb(skb); netdev_dbg(dev, "transmission failed\n"); } @@ -75,6 +76,7 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) /* Stop the netif queue on each sub_if_data object. */ ieee802154_stop_queue(&local->hw); + atomic_inc(&local->phy->ongoing_txs); /* Drivers should preferably implement the async callback. In some rare * cases they only provide a sync callback which we will use as a @@ -98,6 +100,7 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) err_wake_netif_queue: ieee802154_wake_queue(&local->hw); + atomic_dec(&local->phy->ongoing_txs); err_free_skb: kfree_skb(skb); return NETDEV_TX_OK; diff --git a/net/mac802154/util.c b/net/mac802154/util.c index 9f024d85563b4..76dc663e2af43 100644 --- a/net/mac802154/util.c +++ b/net/mac802154/util.c @@ -88,6 +88,7 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb, } dev_consume_skb_any(skb); + atomic_dec(&hw->phy->ongoing_txs); } EXPORT_SYMBOL(ieee802154_xmit_complete); @@ -99,6 +100,7 @@ void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb, local->tx_result = reason; ieee802154_wake_queue(hw); dev_kfree_skb_any(skb); + atomic_dec(&hw->phy->ongoing_txs); } EXPORT_SYMBOL(ieee802154_xmit_error); From 20a19d1df3e4079cbaa045ec89bbefb831d4705d Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:10 +0200 Subject: [PATCH 05/28] net: mac802154: Bring the ability to hold the transmit queue Create a hold_txs atomic variable and increment/decrement it when relevant, ie. when we want to hold the queue or release it: currently all the "stopped" situations are suitable, but very soon we will more extensively use this feature for MLME purposes. Upon release, the atomic counter is decremented and checked. If it is back to 0, then the netif queue gets woken up. This makes the whole process fully transparent, provided that all the users of ieee802154_wake/stop_queue() now call ieee802154_hold/release_queue() instead. In no situation individual drivers should call any of these helpers manually in order to avoid messing with the counters. There are other functions more suited for this purpose which have been introduced, such as the _xmit_complete() and _xmit_error() helpers which will handle all that for them. One advantage is that, as no more drivers call the stop/wake helpers directly, we can safely stop exporting them and only declare the hold/release ones in a header only accessible to the core. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-6-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- include/net/cfg802154.h | 6 +++-- include/net/mac802154.h | 27 ------------------- net/ieee802154/core.c | 2 ++ net/mac802154/cfg.c | 4 +-- net/mac802154/ieee802154_i.h | 19 +++++++++++++ net/mac802154/tx.c | 6 ++--- net/mac802154/util.c | 52 +++++++++++++++++++++++++++++++----- 7 files changed, 75 insertions(+), 41 deletions(-) diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index 678ff00c7d701..e87f1b07f20f6 100644 --- a/include/net/cfg802154.h +++ b/include/net/cfg802154.h @@ -11,7 +11,7 @@ #include #include -#include +#include #include #include @@ -214,8 +214,10 @@ struct wpan_phy { /* the network namespace this phy lives in currently */ possible_net_t _net; - /* Transmission monitoring */ + /* Transmission monitoring and control */ + spinlock_t queue_lock; atomic_t ongoing_txs; + atomic_t hold_txs; char priv[] __aligned(NETDEV_ALIGN); }; diff --git a/include/net/mac802154.h b/include/net/mac802154.h index bdac0ddbdcdb5..357d25ef627a3 100644 --- a/include/net/mac802154.h +++ b/include/net/mac802154.h @@ -460,33 +460,6 @@ void ieee802154_unregister_hw(struct ieee802154_hw *hw); */ void ieee802154_rx_irqsafe(struct ieee802154_hw *hw, struct sk_buff *skb, u8 lqi); -/** - * ieee802154_wake_queue - wake ieee802154 queue - * @hw: pointer as obtained from ieee802154_alloc_hw(). - * - * Tranceivers usually have either one transmit framebuffer or one framebuffer - * for both transmitting and receiving. Hence, the core currently 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); - -/** - * ieee802154_stop_queue - stop ieee802154 queue - * @hw: pointer as obtained from ieee802154_alloc_hw(). - * - * Tranceivers usually have either one transmit framebuffer or one framebuffer - * for both transmitting and receiving. Hence, the core currently 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); /** * ieee802154_xmit_complete - frame transmission complete diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c index de259b5170ab1..47a4de6df88bc 100644 --- a/net/ieee802154/core.c +++ b/net/ieee802154/core.c @@ -130,6 +130,8 @@ wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size) init_waitqueue_head(&rdev->dev_wait); + spin_lock_init(&rdev->wpan_phy.queue_lock); + return &rdev->wpan_phy; } EXPORT_SYMBOL(wpan_phy_new); diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c index 1e4a9f74ed438..b51100fd9e3f2 100644 --- a/net/mac802154/cfg.c +++ b/net/mac802154/cfg.c @@ -46,7 +46,7 @@ static int ieee802154_suspend(struct wpan_phy *wpan_phy) if (!local->open_count) goto suspend; - ieee802154_stop_queue(&local->hw); + ieee802154_hold_queue(local); synchronize_net(); /* stop hardware - this must stop RX */ @@ -72,7 +72,7 @@ static int ieee802154_resume(struct wpan_phy *wpan_phy) return ret; wake_up: - ieee802154_wake_queue(&local->hw); + ieee802154_release_queue(local); local->suspended = false; return 0; } diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index a8b7b9049f14e..0c7ff9e0b6321 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -130,6 +130,25 @@ netdev_tx_t ieee802154_subif_start_xmit(struct sk_buff *skb, struct net_device *dev); enum hrtimer_restart ieee802154_xmit_ifs_timer(struct hrtimer *timer); +/** + * ieee802154_hold_queue - hold ieee802154 queue + * @local: main mac object + * + * Hold a queue by incrementing an atomic counter and requesting the netif + * queues to be stopped. The queues cannot be woken up while the counter has not + * been reset with as any ieee802154_release_queue() calls as needed. + */ +void ieee802154_hold_queue(struct ieee802154_local *local); + +/** + * ieee802154_release_queue - release ieee802154 queue + * @local: main mac object + * + * Release a queue which is held by decrementing an atomic counter and wake it + * up only if the counter reaches 0. + */ +void ieee802154_release_queue(struct ieee802154_local *local); + /* MIB callbacks */ void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan); diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 33f64ecd96c7b..6a53c83cf0399 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -43,7 +43,7 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) err_tx: /* Restart the netif queue on each sub_if_data object. */ - ieee802154_wake_queue(&local->hw); + ieee802154_release_queue(local); atomic_dec(&local->phy->ongoing_txs); kfree_skb(skb); netdev_dbg(dev, "transmission failed\n"); @@ -75,7 +75,7 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) } /* Stop the netif queue on each sub_if_data object. */ - ieee802154_stop_queue(&local->hw); + ieee802154_hold_queue(local); atomic_inc(&local->phy->ongoing_txs); /* Drivers should preferably implement the async callback. In some rare @@ -99,7 +99,7 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) return NETDEV_TX_OK; err_wake_netif_queue: - ieee802154_wake_queue(&local->hw); + ieee802154_release_queue(local); atomic_dec(&local->phy->ongoing_txs); err_free_skb: kfree_skb(skb); diff --git a/net/mac802154/util.c b/net/mac802154/util.c index 76dc663e2af43..0ed8b5bcbe8a7 100644 --- a/net/mac802154/util.c +++ b/net/mac802154/util.c @@ -13,7 +13,17 @@ /* privid for wpan_phys to determine whether they belong to us or not */ const void *const mac802154_wpan_phy_privid = &mac802154_wpan_phy_privid; -void ieee802154_wake_queue(struct ieee802154_hw *hw) +/** + * ieee802154_wake_queue - wake ieee802154 queue + * @local: main mac object + * + * Tranceivers usually have either one transmit framebuffer or one framebuffer + * for both transmitting and receiving. Hence, the core currently 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. + */ +static void ieee802154_wake_queue(struct ieee802154_hw *hw) { struct ieee802154_local *local = hw_to_local(hw); struct ieee802154_sub_if_data *sdata; @@ -27,9 +37,18 @@ void ieee802154_wake_queue(struct ieee802154_hw *hw) } rcu_read_unlock(); } -EXPORT_SYMBOL(ieee802154_wake_queue); -void ieee802154_stop_queue(struct ieee802154_hw *hw) +/** + * ieee802154_stop_queue - stop ieee802154 queue + * @local: main mac object + * + * Tranceivers usually have either one transmit framebuffer or one framebuffer + * for both transmitting and receiving. Hence, the core currently 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. + */ +static void ieee802154_stop_queue(struct ieee802154_hw *hw) { struct ieee802154_local *local = hw_to_local(hw); struct ieee802154_sub_if_data *sdata; @@ -43,14 +62,33 @@ void ieee802154_stop_queue(struct ieee802154_hw *hw) } rcu_read_unlock(); } -EXPORT_SYMBOL(ieee802154_stop_queue); + +void ieee802154_hold_queue(struct ieee802154_local *local) +{ + unsigned long flags; + + spin_lock_irqsave(&local->phy->queue_lock, flags); + if (!atomic_fetch_inc(&local->phy->hold_txs)) + ieee802154_stop_queue(&local->hw); + spin_unlock_irqrestore(&local->phy->queue_lock, flags); +} + +void ieee802154_release_queue(struct ieee802154_local *local) +{ + unsigned long flags; + + spin_lock_irqsave(&local->phy->queue_lock, flags); + if (!atomic_dec_and_test(&local->phy->hold_txs)) + ieee802154_wake_queue(&local->hw); + spin_unlock_irqrestore(&local->phy->queue_lock, flags); +} enum hrtimer_restart ieee802154_xmit_ifs_timer(struct hrtimer *timer) { struct ieee802154_local *local = container_of(timer, struct ieee802154_local, ifs_timer); - ieee802154_wake_queue(&local->hw); + ieee802154_release_queue(local); return HRTIMER_NORESTART; } @@ -84,7 +122,7 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb, hw->phy->sifs_period * NSEC_PER_USEC, HRTIMER_MODE_REL); } else { - ieee802154_wake_queue(hw); + ieee802154_release_queue(local); } dev_consume_skb_any(skb); @@ -98,7 +136,7 @@ void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb, struct ieee802154_local *local = hw_to_local(hw); local->tx_result = reason; - ieee802154_wake_queue(hw); + ieee802154_release_queue(local); dev_kfree_skb_any(skb); atomic_dec(&hw->phy->ongoing_txs); } From 226730e1aa2844b6e5499a6fe6bea4db17a894a8 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:11 +0200 Subject: [PATCH 06/28] net: mac802154: Create a hot tx path Let's rename the current Tx path to show that this is the "hot" Tx path. We will soon introduce a slower Tx path for MLME commands. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-7-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/tx.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 6a53c83cf0399..607019b8f8ab5 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -106,6 +106,12 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) return NETDEV_TX_OK; } +static netdev_tx_t +ieee802154_hot_tx(struct ieee802154_local *local, struct sk_buff *skb) +{ + return ieee802154_tx(local, skb); +} + netdev_tx_t ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev) { @@ -113,7 +119,7 @@ ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev) skb->skb_iif = dev->ifindex; - return ieee802154_tx(sdata->local, skb); + return ieee802154_hot_tx(sdata->local, skb); } netdev_tx_t @@ -135,5 +141,5 @@ ieee802154_subif_start_xmit(struct sk_buff *skb, struct net_device *dev) skb->skb_iif = dev->ifindex; - return ieee802154_tx(sdata->local, skb); + return ieee802154_hot_tx(sdata->local, skb); } From a40612f399eabe23424acf5985643a35d4f2832e Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:12 +0200 Subject: [PATCH 07/28] net: mac802154: Introduce a helper to disable the queue Sometimes calling the stop queue helper is not enough because it does not hold any lock. In order to be safe and avoid racy situations when trying to (soon) sync the Tx queue, for instance before sending an MLME frame, let's now introduce an helper which actually hold the necessary locks when doing so. Suggested-by: Alexander Aring Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-8-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/ieee802154_i.h | 12 ++++++++++++ net/mac802154/util.c | 14 ++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index 0c7ff9e0b6321..e34db1d49ef43 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -149,6 +149,18 @@ void ieee802154_hold_queue(struct ieee802154_local *local); */ void ieee802154_release_queue(struct ieee802154_local *local); +/** + * ieee802154_disable_queue - disable ieee802154 queue + * @local: main mac object + * + * When trying to sync the Tx queue, we cannot just stop the queue + * (which is basically a bit being set without proper lock handling) + * because it would be racy. We actually need to call netif_tx_disable() + * instead, which is done by this helper. Restarting the queue can + * however still be done with a regular wake call. + */ +void ieee802154_disable_queue(struct ieee802154_local *local); + /* MIB callbacks */ void mac802154_dev_set_page_channel(struct net_device *dev, u8 page, u8 chan); diff --git a/net/mac802154/util.c b/net/mac802154/util.c index 0ed8b5bcbe8a7..999534f644854 100644 --- a/net/mac802154/util.c +++ b/net/mac802154/util.c @@ -83,6 +83,20 @@ void ieee802154_release_queue(struct ieee802154_local *local) spin_unlock_irqrestore(&local->phy->queue_lock, flags); } +void ieee802154_disable_queue(struct ieee802154_local *local) +{ + struct ieee802154_sub_if_data *sdata; + + rcu_read_lock(); + list_for_each_entry_rcu(sdata, &local->interfaces, list) { + if (!sdata->dev) + continue; + + netif_tx_disable(sdata->dev); + } + rcu_read_unlock(); +} + enum hrtimer_restart ieee802154_xmit_ifs_timer(struct hrtimer *timer) { struct ieee802154_local *local = From f0feb34904735ffa21fe7b0c50f9f9527ec74b7a Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:13 +0200 Subject: [PATCH 08/28] net: mac802154: Introduce a tx queue flushing mechanism Right now we are able to stop a queue but we have no indication if a transmission is ongoing or not. Thanks to recent additions, we can track the number of ongoing transmissions so we know if the last transmission is over. Adding on top of it an internal wait queue also allows to be woken up asynchronously when this happens. If, beforehands, we marked the queue to be held and stopped it, we end up flushing and stopping the tx queue. Thanks to this feature, we will soon be able to introduce a synchronous transmit API. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-9-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- include/net/cfg802154.h | 1 + net/ieee802154/core.c | 1 + net/mac802154/cfg.c | 2 +- net/mac802154/ieee802154_i.h | 1 + net/mac802154/tx.c | 26 ++++++++++++++++++++++++-- net/mac802154/util.c | 6 ++++-- 6 files changed, 32 insertions(+), 5 deletions(-) diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index e87f1b07f20f6..0804d79669a43 100644 --- a/include/net/cfg802154.h +++ b/include/net/cfg802154.h @@ -218,6 +218,7 @@ struct wpan_phy { spinlock_t queue_lock; atomic_t ongoing_txs; atomic_t hold_txs; + wait_queue_head_t sync_txq; char priv[] __aligned(NETDEV_ALIGN); }; diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c index 47a4de6df88bc..57546e07e06ad 100644 --- a/net/ieee802154/core.c +++ b/net/ieee802154/core.c @@ -129,6 +129,7 @@ wpan_phy_new(const struct cfg802154_ops *ops, size_t priv_size) wpan_phy_net_set(&rdev->wpan_phy, &init_net); init_waitqueue_head(&rdev->dev_wait); + init_waitqueue_head(&rdev->wpan_phy.sync_txq); spin_lock_init(&rdev->wpan_phy.queue_lock); diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c index b51100fd9e3f2..93df24f755724 100644 --- a/net/mac802154/cfg.c +++ b/net/mac802154/cfg.c @@ -46,7 +46,7 @@ static int ieee802154_suspend(struct wpan_phy *wpan_phy) if (!local->open_count) goto suspend; - ieee802154_hold_queue(local); + ieee802154_sync_and_hold_queue(local); synchronize_net(); /* stop hardware - this must stop RX */ diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index e34db1d49ef43..a057827fc48a5 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -124,6 +124,7 @@ extern struct ieee802154_mlme_ops mac802154_mlme_wpan; void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb); void ieee802154_xmit_sync_worker(struct work_struct *work); +int ieee802154_sync_and_hold_queue(struct ieee802154_local *local); netdev_tx_t ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev); netdev_tx_t diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 607019b8f8ab5..38f74b8b67402 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -44,7 +44,8 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) err_tx: /* Restart the netif queue on each sub_if_data object. */ ieee802154_release_queue(local); - atomic_dec(&local->phy->ongoing_txs); + if (!atomic_dec_and_test(&local->phy->ongoing_txs)) + wake_up(&local->phy->sync_txq); kfree_skb(skb); netdev_dbg(dev, "transmission failed\n"); } @@ -100,12 +101,33 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) err_wake_netif_queue: ieee802154_release_queue(local); - atomic_dec(&local->phy->ongoing_txs); + if (!atomic_dec_and_test(&local->phy->ongoing_txs)) + wake_up(&local->phy->sync_txq); err_free_skb: kfree_skb(skb); return NETDEV_TX_OK; } +static int ieee802154_sync_queue(struct ieee802154_local *local) +{ + int ret; + + ieee802154_hold_queue(local); + ieee802154_disable_queue(local); + wait_event(local->phy->sync_txq, !atomic_read(&local->phy->ongoing_txs)); + ret = local->tx_result; + ieee802154_release_queue(local); + + return ret; +} + +int ieee802154_sync_and_hold_queue(struct ieee802154_local *local) +{ + ieee802154_hold_queue(local); + + return ieee802154_sync_queue(local); +} + static netdev_tx_t ieee802154_hot_tx(struct ieee802154_local *local, struct sk_buff *skb) { diff --git a/net/mac802154/util.c b/net/mac802154/util.c index 999534f644854..5e1fcc7b0123b 100644 --- a/net/mac802154/util.c +++ b/net/mac802154/util.c @@ -140,7 +140,8 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb, } dev_consume_skb_any(skb); - atomic_dec(&hw->phy->ongoing_txs); + if (!atomic_dec_and_test(&hw->phy->ongoing_txs)) + wake_up(&hw->phy->sync_txq); } EXPORT_SYMBOL(ieee802154_xmit_complete); @@ -152,7 +153,8 @@ void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb, local->tx_result = reason; ieee802154_release_queue(local); dev_kfree_skb_any(skb); - atomic_dec(&hw->phy->ongoing_txs); + if (!atomic_dec_and_test(&hw->phy->ongoing_txs)) + wake_up(&hw->phy->sync_txq); } EXPORT_SYMBOL(ieee802154_xmit_error); From ddd9ee7cda122ac55571b8b11b51ef1e71918b63 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:14 +0200 Subject: [PATCH 09/28] net: mac802154: Introduce a synchronous API for MLME commands This is the slow path, we need to wait for each command to be processed before continuing so let's introduce an helper which does the transmission and blocks until it gets notified of its asynchronous completion. This helper is going to be used when introducing scan support. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-10-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/ieee802154_i.h | 4 ++++ net/mac802154/tx.c | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index a057827fc48a5..8a4816ae71e7b 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -125,6 +125,10 @@ extern struct ieee802154_mlme_ops mac802154_mlme_wpan; void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb); void ieee802154_xmit_sync_worker(struct work_struct *work); int ieee802154_sync_and_hold_queue(struct ieee802154_local *local); +int ieee802154_mlme_op_pre(struct ieee802154_local *local); +int ieee802154_mlme_tx(struct ieee802154_local *local, struct sk_buff *skb); +void ieee802154_mlme_op_post(struct ieee802154_local *local); +int ieee802154_mlme_tx_one(struct ieee802154_local *local, struct sk_buff *skb); netdev_tx_t ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev); netdev_tx_t diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 38f74b8b67402..4827391600f6a 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -128,6 +128,50 @@ int ieee802154_sync_and_hold_queue(struct ieee802154_local *local) return ieee802154_sync_queue(local); } +int ieee802154_mlme_op_pre(struct ieee802154_local *local) +{ + return ieee802154_sync_and_hold_queue(local); +} + +int ieee802154_mlme_tx(struct ieee802154_local *local, struct sk_buff *skb) +{ + int ret; + + /* Avoid possible calls to ->ndo_stop() when we asynchronously perform + * MLME transmissions. + */ + rtnl_lock(); + + /* Ensure the device was not stopped, otherwise error out */ + if (!local->open_count) { + rtnl_unlock(); + return -ENETDOWN; + } + + ieee802154_tx(local, skb); + ret = ieee802154_sync_queue(local); + + rtnl_unlock(); + + return ret; +} + +void ieee802154_mlme_op_post(struct ieee802154_local *local) +{ + ieee802154_release_queue(local); +} + +int ieee802154_mlme_tx_one(struct ieee802154_local *local, struct sk_buff *skb) +{ + int ret; + + ieee802154_mlme_op_pre(local); + ret = ieee802154_mlme_tx(local, skb); + ieee802154_mlme_op_post(local); + + return ret; +} + static netdev_tx_t ieee802154_hot_tx(struct ieee802154_local *local, struct sk_buff *skb) { From 2b13db13af50a5dcdb944723c828915a50f0c3b2 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:15 +0200 Subject: [PATCH 10/28] net: mac802154: Add a warning in the hot path We should never start a transmission after the queue has been stopped. But because it might work we don't kill the function here but rather warn loudly the user that something is wrong. Set a flag when the queue should remain stopped. Reset this flag when the queue actually gets restarded. Just check this value to know if a transmission is legitimate, warn if it is not. Turn the flags variable into an unsigned long to allow the use of atomic helpers on it. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-11-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- include/net/cfg802154.h | 5 ++++- net/mac802154/tx.c | 16 +++++++++++++++- net/mac802154/util.c | 1 + 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index 0804d79669a43..428cece222059 100644 --- a/include/net/cfg802154.h +++ b/include/net/cfg802154.h @@ -166,11 +166,14 @@ wpan_phy_cca_cmp(const struct wpan_phy_cca *a, const struct wpan_phy_cca *b) * level setting. * @WPAN_PHY_FLAG_CCA_MODE: Indicates that transceiver will support cca mode * setting. + * @WPAN_PHY_FLAG_STATE_QUEUE_STOPPED: Indicates that the transmit queue was + * temporarily stopped. */ enum wpan_phy_flags { WPAN_PHY_FLAG_TXPOWER = BIT(1), WPAN_PHY_FLAG_CCA_ED_LEVEL = BIT(2), WPAN_PHY_FLAG_CCA_MODE = BIT(3), + WPAN_PHY_FLAG_STATE_QUEUE_STOPPED = BIT(4), }; struct wpan_phy { @@ -182,7 +185,7 @@ struct wpan_phy { */ const void *privid; - u32 flags; + unsigned long flags; /* * This is a PIB according to 802.15.4-2011. diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 4827391600f6a..6188f42276e78 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -123,9 +123,13 @@ static int ieee802154_sync_queue(struct ieee802154_local *local) int ieee802154_sync_and_hold_queue(struct ieee802154_local *local) { + int ret; + ieee802154_hold_queue(local); + ret = ieee802154_sync_queue(local); + set_bit(WPAN_PHY_FLAG_STATE_QUEUE_STOPPED, &local->phy->flags); - return ieee802154_sync_queue(local); + return ret; } int ieee802154_mlme_op_pre(struct ieee802154_local *local) @@ -172,9 +176,19 @@ int ieee802154_mlme_tx_one(struct ieee802154_local *local, struct sk_buff *skb) return ret; } +static bool ieee802154_queue_is_stopped(struct ieee802154_local *local) +{ + return test_bit(WPAN_PHY_FLAG_STATE_QUEUE_STOPPED, &local->phy->flags); +} + static netdev_tx_t ieee802154_hot_tx(struct ieee802154_local *local, struct sk_buff *skb) { + /* Warn if the net interface tries to transmit frames while the + * ieee802154 core assumes the queue is stopped. + */ + WARN_ON_ONCE(ieee802154_queue_is_stopped(local)); + return ieee802154_tx(local, skb); } diff --git a/net/mac802154/util.c b/net/mac802154/util.c index 5e1fcc7b0123b..60eb7bd3bfc14 100644 --- a/net/mac802154/util.c +++ b/net/mac802154/util.c @@ -29,6 +29,7 @@ static void ieee802154_wake_queue(struct ieee802154_hw *hw) struct ieee802154_sub_if_data *sdata; rcu_read_lock(); + clear_bit(WPAN_PHY_FLAG_STATE_QUEUE_STOPPED, &local->phy->flags); list_for_each_entry_rcu(sdata, &local->interfaces, list) { if (!sdata->dev) continue; From 4f790184139beb8b35a1f4f9a4b6731c6eef1763 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Thu, 19 May 2022 17:05:16 +0200 Subject: [PATCH 11/28] net: mac802154: Add a warning in the slow path In order to be able to detect possible conflicts between the net interface core and the ieee802154 core, let's add a warning in the slow path: we want to be sure that whenever we start an asynchronous MLME transmission (which can be fully asynchronous) the net core somehow agrees that this transmission is possible, ie. the device was not stopped. Warning in this case would allow us to track down more easily possible issues with the MLME logic if we ever get reports. Unlike in the hot path, such a situation cannot be handled. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220519150516.443078-12-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/tx.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 6188f42276e78..5b471e9322713 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -132,6 +132,25 @@ int ieee802154_sync_and_hold_queue(struct ieee802154_local *local) return ret; } +static bool ieee802154_netif_is_down(struct ieee802154_local *local) +{ + struct ieee802154_sub_if_data *sdata; + bool is_down = true; + + rcu_read_lock(); + list_for_each_entry_rcu(sdata, &local->interfaces, list) { + if (!sdata->dev) + continue; + + is_down = !netif_running(sdata->dev); + if (is_down) + break; + } + rcu_read_unlock(); + + return is_down; +} + int ieee802154_mlme_op_pre(struct ieee802154_local *local) { return ieee802154_sync_and_hold_queue(local); @@ -152,6 +171,14 @@ int ieee802154_mlme_tx(struct ieee802154_local *local, struct sk_buff *skb) return -ENETDOWN; } + /* Warn if the ieee802154 core thinks MLME frames can be sent while the + * net interface expects this cannot happen. + */ + if (WARN_ON_ONCE(ieee802154_netif_is_down(local))) { + rtnl_unlock(); + return -ENETDOWN; + } + ieee802154_tx(local, skb); ret = ieee802154_sync_queue(local); From 2ec2f6bed4d12a7ed74ed179a5d2879b117105d1 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Mon, 13 Jun 2022 00:37:34 -0400 Subject: [PATCH 12/28] mac802154: util: fix release queue handling The semantic of atomic_dec_and_test() is to return true if zero is reached and we need call ieee802154_wake_queue() when zero is reached. Fixes: 20a19d1df3e4 ("net: mac802154: Bring the ability to hold the transmit queue") Signed-off-by: Alexander Aring Reviewed-by: Miquel Raynal Link: https://lore.kernel.org/r/20220613043735.1039895-2-aahringo@redhat.com Signed-off-by: Stefan Schmidt --- net/mac802154/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/mac802154/util.c b/net/mac802154/util.c index 60eb7bd3bfc14..60f6c0f10641b 100644 --- a/net/mac802154/util.c +++ b/net/mac802154/util.c @@ -79,7 +79,7 @@ void ieee802154_release_queue(struct ieee802154_local *local) unsigned long flags; spin_lock_irqsave(&local->phy->queue_lock, flags); - if (!atomic_dec_and_test(&local->phy->hold_txs)) + if (atomic_dec_and_test(&local->phy->hold_txs)) ieee802154_wake_queue(&local->hw); spin_unlock_irqrestore(&local->phy->queue_lock, flags); } From 6c1c78d0182fcbfe953bf5a0c3e71204d176b887 Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Mon, 13 Jun 2022 00:37:35 -0400 Subject: [PATCH 13/28] mac802154: fix atomic_dec_and_test checks We need to call wake_up() when hold_txs reaches zero. The semantic of atomic_dec_and_test() is that it returns true when it's zero. Fixes: f0feb3490473 ("net: mac802154: Introduce a tx queue flushing mechanism") Signed-off-by: Alexander Aring Reviewed-by: Miquel Raynal Link: https://lore.kernel.org/r/20220613043735.1039895-3-aahringo@redhat.com Signed-off-by: Stefan Schmidt --- net/mac802154/tx.c | 4 ++-- net/mac802154/util.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 5b471e9322713..8ddcd2e841cab 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -44,7 +44,7 @@ void ieee802154_xmit_sync_worker(struct work_struct *work) err_tx: /* Restart the netif queue on each sub_if_data object. */ ieee802154_release_queue(local); - if (!atomic_dec_and_test(&local->phy->ongoing_txs)) + if (atomic_dec_and_test(&local->phy->ongoing_txs)) wake_up(&local->phy->sync_txq); kfree_skb(skb); netdev_dbg(dev, "transmission failed\n"); @@ -101,7 +101,7 @@ ieee802154_tx(struct ieee802154_local *local, struct sk_buff *skb) err_wake_netif_queue: ieee802154_release_queue(local); - if (!atomic_dec_and_test(&local->phy->ongoing_txs)) + if (atomic_dec_and_test(&local->phy->ongoing_txs)) wake_up(&local->phy->sync_txq); err_free_skb: kfree_skb(skb); diff --git a/net/mac802154/util.c b/net/mac802154/util.c index 60f6c0f10641b..f08605f59b609 100644 --- a/net/mac802154/util.c +++ b/net/mac802154/util.c @@ -141,7 +141,7 @@ void ieee802154_xmit_complete(struct ieee802154_hw *hw, struct sk_buff *skb, } dev_consume_skb_any(skb); - if (!atomic_dec_and_test(&hw->phy->ongoing_txs)) + if (atomic_dec_and_test(&hw->phy->ongoing_txs)) wake_up(&hw->phy->sync_txq); } EXPORT_SYMBOL(ieee802154_xmit_complete); @@ -154,7 +154,7 @@ void ieee802154_xmit_error(struct ieee802154_hw *hw, struct sk_buff *skb, local->tx_result = reason; ieee802154_release_queue(local); dev_kfree_skb_any(skb); - if (!atomic_dec_and_test(&hw->phy->ongoing_txs)) + if (atomic_dec_and_test(&hw->phy->ongoing_txs)) wake_up(&hw->phy->sync_txq); } EXPORT_SYMBOL(ieee802154_xmit_error); From fbdaa5ba6bd6955f7e7f9228e4d815cc5e43fe5b Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Fri, 17 Jun 2022 21:29:14 +0200 Subject: [PATCH 14/28] net: mac802154: Fix a Tx warning check The purpose of the netif_is_down() helper was to ensure that the network interface used was still up when performing the transmission. What it actually did was to check if _all_ interfaces were up. This was not noticed at that time because I did not use interfaces at all before discussing with Alexander Aring about how to handle coordinators properly. Drop the helper and call netif_running() on the right sub interface object directly. Fixes: 4f790184139b ("net: mac802154: Add a warning in the slow path") Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20220617192914.1275611-1-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/ieee802154_i.h | 8 ++++++-- net/mac802154/tx.c | 31 ++++++++----------------------- 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index 8a4816ae71e7b..010365a6364e0 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -126,9 +126,13 @@ void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb); void ieee802154_xmit_sync_worker(struct work_struct *work); int ieee802154_sync_and_hold_queue(struct ieee802154_local *local); int ieee802154_mlme_op_pre(struct ieee802154_local *local); -int ieee802154_mlme_tx(struct ieee802154_local *local, struct sk_buff *skb); +int ieee802154_mlme_tx(struct ieee802154_local *local, + struct ieee802154_sub_if_data *sdata, + struct sk_buff *skb); void ieee802154_mlme_op_post(struct ieee802154_local *local); -int ieee802154_mlme_tx_one(struct ieee802154_local *local, struct sk_buff *skb); +int ieee802154_mlme_tx_one(struct ieee802154_local *local, + struct ieee802154_sub_if_data *sdata, + struct sk_buff *skb); netdev_tx_t ieee802154_monitor_start_xmit(struct sk_buff *skb, struct net_device *dev); netdev_tx_t diff --git a/net/mac802154/tx.c b/net/mac802154/tx.c index 8ddcd2e841cab..9d8d43cf1e64c 100644 --- a/net/mac802154/tx.c +++ b/net/mac802154/tx.c @@ -132,31 +132,14 @@ int ieee802154_sync_and_hold_queue(struct ieee802154_local *local) return ret; } -static bool ieee802154_netif_is_down(struct ieee802154_local *local) -{ - struct ieee802154_sub_if_data *sdata; - bool is_down = true; - - rcu_read_lock(); - list_for_each_entry_rcu(sdata, &local->interfaces, list) { - if (!sdata->dev) - continue; - - is_down = !netif_running(sdata->dev); - if (is_down) - break; - } - rcu_read_unlock(); - - return is_down; -} - int ieee802154_mlme_op_pre(struct ieee802154_local *local) { return ieee802154_sync_and_hold_queue(local); } -int ieee802154_mlme_tx(struct ieee802154_local *local, struct sk_buff *skb) +int ieee802154_mlme_tx(struct ieee802154_local *local, + struct ieee802154_sub_if_data *sdata, + struct sk_buff *skb) { int ret; @@ -174,7 +157,7 @@ int ieee802154_mlme_tx(struct ieee802154_local *local, struct sk_buff *skb) /* Warn if the ieee802154 core thinks MLME frames can be sent while the * net interface expects this cannot happen. */ - if (WARN_ON_ONCE(ieee802154_netif_is_down(local))) { + if (WARN_ON_ONCE(!netif_running(sdata->dev))) { rtnl_unlock(); return -ENETDOWN; } @@ -192,12 +175,14 @@ void ieee802154_mlme_op_post(struct ieee802154_local *local) ieee802154_release_queue(local); } -int ieee802154_mlme_tx_one(struct ieee802154_local *local, struct sk_buff *skb) +int ieee802154_mlme_tx_one(struct ieee802154_local *local, + struct ieee802154_sub_if_data *sdata, + struct sk_buff *skb) { int ret; ieee802154_mlme_op_pre(local); - ret = ieee802154_mlme_tx(local, skb); + ret = ieee802154_mlme_tx(local, sdata, skb); ieee802154_mlme_op_post(local); return ret; From d90fdb9138262e2c68e4305965348dc065f5c7a3 Mon Sep 17 00:00:00 2001 From: Jilin Yuan Date: Fri, 8 Jul 2022 23:15:38 +0800 Subject: [PATCH 15/28] net/ieee802154: fix repeated words in comments Delete the redundant word 'was'. Signed-off-by: Jilin Yuan Link: https://lore.kernel.org/r/20220708151538.51483-1-yuanjilin@cdjrlc.com Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/ca8210.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c index 42c0b451088dc..450b16ad40a41 100644 --- a/drivers/net/ieee802154/ca8210.c +++ b/drivers/net/ieee802154/ca8210.c @@ -2293,7 +2293,7 @@ static int ca8210_set_csma_params( * @retries: Number of retries * * Sets the number of times to retry a transmission if no acknowledgment was - * was received from the other end when one was requested. + * received from the other end when one was requested. * * Return: 0 or linux error code */ From dfc3082da9c97112007b6d585e28c1aeb8618acb Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Thu, 15 Sep 2022 15:12:58 +0800 Subject: [PATCH 16/28] net: ieee802154: mcr20a: Switch to use dev_err_probe() helper dev_err() can be replace with dev_err_probe() which will check if error code is -EPROBE_DEFER. Signed-off-by: Yang Yingliang Link: https://lore.kernel.org/r/20220915071258.678536-1-yangyingliang@huawei.com Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/mcr20a.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/net/ieee802154/mcr20a.c b/drivers/net/ieee802154/mcr20a.c index 2fe0e4a0a0c4c..f53d185e0568d 100644 --- a/drivers/net/ieee802154/mcr20a.c +++ b/drivers/net/ieee802154/mcr20a.c @@ -1233,12 +1233,9 @@ mcr20a_probe(struct spi_device *spi) } rst_b = devm_gpiod_get(&spi->dev, "rst_b", GPIOD_OUT_HIGH); - if (IS_ERR(rst_b)) { - ret = PTR_ERR(rst_b); - if (ret != -EPROBE_DEFER) - dev_err(&spi->dev, "Failed to get 'rst_b' gpio: %d", ret); - return ret; - } + if (IS_ERR(rst_b)) + return dev_err_probe(&spi->dev, PTR_ERR(rst_b), + "Failed to get 'rst_b' gpio"); /* reset mcr20a */ usleep_range(10, 20); From a6a6163b9a934df5965792d60af1f53aa51fdd39 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Fri, 7 Oct 2022 10:53:03 +0200 Subject: [PATCH 17/28] mac802154: Introduce filtering levels The 802154 specification details several filtering levels in which the PHY and the MAC could be. The amount of filtering will vary if they are in promiscuous mode or in scanning mode. Otherwise they are expected to do some very basic checks, such as enforcing the frame validity. Either the PHY is able to do so, and the MAC has nothing to do, or the PHY has a lower filtering level than expected and the MAC should take over. For now we just define these levels in an enumeration. In a second time, we will add a per-PHY parameter showing the expected filtering level as well as a per device current filtering level, and will initialize all these fields. In a third time, we will use them to apply more filtering by software when the PHY is limited. Indeed, if the drivers know they cannot reach the requested level of filtering, they will overwrite the "current filtering" parameter so that it reflects what they do. Then, in the core, the expected filtering level will be used to decide whether some additional software processing is needed or not. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20221007085310.503366-2-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- include/linux/ieee802154.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h index f1f9412b6ac6a..0303eb84d5961 100644 --- a/include/linux/ieee802154.h +++ b/include/linux/ieee802154.h @@ -276,6 +276,30 @@ enum { IEEE802154_SYSTEM_ERROR = 0xff, }; +/** + * enum ieee802154_filtering_level - Filtering levels applicable to a PHY + * + * @IEEE802154_FILTERING_NONE: No filtering at all, what is received is + * forwarded to the softMAC + * @IEEE802154_FILTERING_1_FCS: First filtering level, frames with an invalid + * FCS should be dropped + * @IEEE802154_FILTERING_2_PROMISCUOUS: Second filtering level, promiscuous + * mode as described in the spec, identical in terms of filtering to the + * level one on PHY side, but at the MAC level the frame should be + * forwarded to the upper layer directly + * @IEEE802154_FILTERING_3_SCAN: Third filtering level, scan related, where + * only beacons must be processed, all remaining traffic gets dropped + * @IEEE802154_FILTERING_4_FRAME_FIELDS: Fourth filtering level actually + * enforcing the validity of the content of the frame with various checks + */ +enum ieee802154_filtering_level { + IEEE802154_FILTERING_NONE, + IEEE802154_FILTERING_1_FCS, + IEEE802154_FILTERING_2_PROMISCUOUS, + IEEE802154_FILTERING_3_SCAN, + IEEE802154_FILTERING_4_FRAME_FIELDS, +}; + /* frame control handling */ #define IEEE802154_FCTL_FTYPE 0x0003 #define IEEE802154_FCTL_ACKREQ 0x0020 From e9d8d9c4084dadfa5a68a2e6e4e4dda7a0a728ba Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Fri, 7 Oct 2022 10:53:04 +0200 Subject: [PATCH 18/28] mac802154: move receive parameters above start This patch moves all receive parameters above the drv_start() functionality to make it accessibile in the drv_start() function. Signed-off-by: Alexander Aring Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/r/20221007085310.503366-3-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/driver-ops.h | 190 ++++++++++++++++++------------------- 1 file changed, 95 insertions(+), 95 deletions(-) diff --git a/net/mac802154/driver-ops.h b/net/mac802154/driver-ops.h index d23f0db98015a..c9d54088a5670 100644 --- a/net/mac802154/driver-ops.h +++ b/net/mac802154/driver-ops.h @@ -24,203 +24,221 @@ drv_xmit_sync(struct ieee802154_local *local, struct sk_buff *skb) return local->ops->xmit_sync(&local->hw, skb); } -static inline int drv_start(struct ieee802154_local *local) +static inline int drv_set_pan_id(struct ieee802154_local *local, __le16 pan_id) { + struct ieee802154_hw_addr_filt filt; int ret; might_sleep(); - trace_802154_drv_start(local); - local->started = true; - smp_mb(); - ret = local->ops->start(&local->hw); + if (!local->ops->set_hw_addr_filt) { + WARN_ON(1); + return -EOPNOTSUPP; + } + + filt.pan_id = pan_id; + + trace_802154_drv_set_pan_id(local, pan_id); + ret = local->ops->set_hw_addr_filt(&local->hw, &filt, + IEEE802154_AFILT_PANID_CHANGED); trace_802154_drv_return_int(local, ret); return ret; } -static inline void drv_stop(struct ieee802154_local *local) +static inline int +drv_set_extended_addr(struct ieee802154_local *local, __le64 extended_addr) { - might_sleep(); + struct ieee802154_hw_addr_filt filt; + int ret; - trace_802154_drv_stop(local); - local->ops->stop(&local->hw); - trace_802154_drv_return_void(local); + might_sleep(); - /* sync away all work on the tasklet before clearing started */ - tasklet_disable(&local->tasklet); - tasklet_enable(&local->tasklet); + if (!local->ops->set_hw_addr_filt) { + WARN_ON(1); + return -EOPNOTSUPP; + } - barrier(); + filt.ieee_addr = extended_addr; - local->started = false; + trace_802154_drv_set_extended_addr(local, extended_addr); + ret = local->ops->set_hw_addr_filt(&local->hw, &filt, + IEEE802154_AFILT_IEEEADDR_CHANGED); + trace_802154_drv_return_int(local, ret); + return ret; } static inline int -drv_set_channel(struct ieee802154_local *local, u8 page, u8 channel) +drv_set_short_addr(struct ieee802154_local *local, __le16 short_addr) { + struct ieee802154_hw_addr_filt filt; int ret; might_sleep(); - trace_802154_drv_set_channel(local, page, channel); - ret = local->ops->set_channel(&local->hw, page, channel); + if (!local->ops->set_hw_addr_filt) { + WARN_ON(1); + return -EOPNOTSUPP; + } + + filt.short_addr = short_addr; + + trace_802154_drv_set_short_addr(local, short_addr); + ret = local->ops->set_hw_addr_filt(&local->hw, &filt, + IEEE802154_AFILT_SADDR_CHANGED); trace_802154_drv_return_int(local, ret); return ret; } -static inline int drv_set_tx_power(struct ieee802154_local *local, s32 mbm) +static inline int +drv_set_pan_coord(struct ieee802154_local *local, bool is_coord) { + struct ieee802154_hw_addr_filt filt; int ret; might_sleep(); - if (!local->ops->set_txpower) { + if (!local->ops->set_hw_addr_filt) { WARN_ON(1); return -EOPNOTSUPP; } - trace_802154_drv_set_tx_power(local, mbm); - ret = local->ops->set_txpower(&local->hw, mbm); + filt.pan_coord = is_coord; + + trace_802154_drv_set_pan_coord(local, is_coord); + ret = local->ops->set_hw_addr_filt(&local->hw, &filt, + IEEE802154_AFILT_PANC_CHANGED); trace_802154_drv_return_int(local, ret); return ret; } -static inline int drv_set_cca_mode(struct ieee802154_local *local, - const struct wpan_phy_cca *cca) +static inline int +drv_set_promiscuous_mode(struct ieee802154_local *local, bool on) { int ret; might_sleep(); - if (!local->ops->set_cca_mode) { + if (!local->ops->set_promiscuous_mode) { WARN_ON(1); return -EOPNOTSUPP; } - trace_802154_drv_set_cca_mode(local, cca); - ret = local->ops->set_cca_mode(&local->hw, cca); + trace_802154_drv_set_promiscuous_mode(local, on); + ret = local->ops->set_promiscuous_mode(&local->hw, on); trace_802154_drv_return_int(local, ret); return ret; } -static inline int drv_set_lbt_mode(struct ieee802154_local *local, bool mode) +static inline int drv_start(struct ieee802154_local *local) { int ret; might_sleep(); - if (!local->ops->set_lbt) { - WARN_ON(1); - return -EOPNOTSUPP; - } - - trace_802154_drv_set_lbt_mode(local, mode); - ret = local->ops->set_lbt(&local->hw, mode); + trace_802154_drv_start(local); + local->started = true; + smp_mb(); + ret = local->ops->start(&local->hw); trace_802154_drv_return_int(local, ret); return ret; } +static inline void drv_stop(struct ieee802154_local *local) +{ + might_sleep(); + + trace_802154_drv_stop(local); + local->ops->stop(&local->hw); + trace_802154_drv_return_void(local); + + /* sync away all work on the tasklet before clearing started */ + tasklet_disable(&local->tasklet); + tasklet_enable(&local->tasklet); + + barrier(); + + local->started = false; +} + static inline int -drv_set_cca_ed_level(struct ieee802154_local *local, s32 mbm) +drv_set_channel(struct ieee802154_local *local, u8 page, u8 channel) { int ret; might_sleep(); - if (!local->ops->set_cca_ed_level) { - WARN_ON(1); - return -EOPNOTSUPP; - } - - trace_802154_drv_set_cca_ed_level(local, mbm); - ret = local->ops->set_cca_ed_level(&local->hw, mbm); + trace_802154_drv_set_channel(local, page, channel); + ret = local->ops->set_channel(&local->hw, page, channel); trace_802154_drv_return_int(local, ret); return ret; } -static inline int drv_set_pan_id(struct ieee802154_local *local, __le16 pan_id) +static inline int drv_set_tx_power(struct ieee802154_local *local, s32 mbm) { - struct ieee802154_hw_addr_filt filt; int ret; might_sleep(); - if (!local->ops->set_hw_addr_filt) { + if (!local->ops->set_txpower) { WARN_ON(1); return -EOPNOTSUPP; } - filt.pan_id = pan_id; - - trace_802154_drv_set_pan_id(local, pan_id); - ret = local->ops->set_hw_addr_filt(&local->hw, &filt, - IEEE802154_AFILT_PANID_CHANGED); + trace_802154_drv_set_tx_power(local, mbm); + ret = local->ops->set_txpower(&local->hw, mbm); trace_802154_drv_return_int(local, ret); return ret; } -static inline int -drv_set_extended_addr(struct ieee802154_local *local, __le64 extended_addr) +static inline int drv_set_cca_mode(struct ieee802154_local *local, + const struct wpan_phy_cca *cca) { - struct ieee802154_hw_addr_filt filt; int ret; might_sleep(); - if (!local->ops->set_hw_addr_filt) { + if (!local->ops->set_cca_mode) { WARN_ON(1); return -EOPNOTSUPP; } - filt.ieee_addr = extended_addr; - - trace_802154_drv_set_extended_addr(local, extended_addr); - ret = local->ops->set_hw_addr_filt(&local->hw, &filt, - IEEE802154_AFILT_IEEEADDR_CHANGED); + trace_802154_drv_set_cca_mode(local, cca); + ret = local->ops->set_cca_mode(&local->hw, cca); trace_802154_drv_return_int(local, ret); return ret; } -static inline int -drv_set_short_addr(struct ieee802154_local *local, __le16 short_addr) +static inline int drv_set_lbt_mode(struct ieee802154_local *local, bool mode) { - struct ieee802154_hw_addr_filt filt; int ret; might_sleep(); - if (!local->ops->set_hw_addr_filt) { + if (!local->ops->set_lbt) { WARN_ON(1); return -EOPNOTSUPP; } - filt.short_addr = short_addr; - - trace_802154_drv_set_short_addr(local, short_addr); - ret = local->ops->set_hw_addr_filt(&local->hw, &filt, - IEEE802154_AFILT_SADDR_CHANGED); + trace_802154_drv_set_lbt_mode(local, mode); + ret = local->ops->set_lbt(&local->hw, mode); trace_802154_drv_return_int(local, ret); return ret; } static inline int -drv_set_pan_coord(struct ieee802154_local *local, bool is_coord) +drv_set_cca_ed_level(struct ieee802154_local *local, s32 mbm) { - struct ieee802154_hw_addr_filt filt; int ret; might_sleep(); - if (!local->ops->set_hw_addr_filt) { + if (!local->ops->set_cca_ed_level) { WARN_ON(1); return -EOPNOTSUPP; } - filt.pan_coord = is_coord; - - trace_802154_drv_set_pan_coord(local, is_coord); - ret = local->ops->set_hw_addr_filt(&local->hw, &filt, - IEEE802154_AFILT_PANC_CHANGED); + trace_802154_drv_set_cca_ed_level(local, mbm); + ret = local->ops->set_cca_ed_level(&local->hw, mbm); trace_802154_drv_return_int(local, ret); return ret; } @@ -264,22 +282,4 @@ drv_set_max_frame_retries(struct ieee802154_local *local, s8 max_frame_retries) return ret; } -static inline int -drv_set_promiscuous_mode(struct ieee802154_local *local, bool on) -{ - int ret; - - might_sleep(); - - if (!local->ops->set_promiscuous_mode) { - WARN_ON(1); - return -EOPNOTSUPP; - } - - trace_802154_drv_set_promiscuous_mode(local, on); - ret = local->ops->set_promiscuous_mode(&local->hw, on); - trace_802154_drv_return_int(local, ret); - return ret; -} - #endif /* __MAC802154_DRIVER_OPS */ From ac8037c35bd1fb1799d39f52205f813e6585b58b Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Fri, 7 Oct 2022 10:53:05 +0200 Subject: [PATCH 19/28] mac802154: set filter at drv_start() The current filtering level is set on the first interface up on a wpan phy. If we support scan functionality we need to change the filtering level on the fly on an operational phy and switching back again. This patch will move the receive mode parameter e.g. address filter and promiscuous mode to the drv_start() functionality to allow changing the receive mode on an operational phy not on first ifup only. In future this should be handled on driver layer because each hardware has it's own way to enter a specific filtering level. However this should offer to switch to mode IEEE802154_FILTERING_NONE and back to IEEE802154_FILTERING_4_FRAME_FIELDS. Only IEEE802154_FILTERING_4_FRAME_FIELDS and IEEE802154_FILTERING_NONE are somewhat supported by current hardware. All other filtering levels can be supported in future but will end in IEEE802154_FILTERING_NONE as the receive part can kind of "emulate" those receive paths by doing additional filtering routines. There are in total three filtering levels in the code: - the per-interface default level (should not be changed) - the required per-interface level (mac commands may play with it) - the actual per-PHY (hw) level that is currently in use Signed-off-by: Alexander Aring [ Link: https://lore.kernel.org/r/20221007085310.503366-4-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- include/net/cfg802154.h | 7 +++- net/mac802154/cfg.c | 2 +- net/mac802154/driver-ops.h | 71 +++++++++++++++++++++++++++++++++++- net/mac802154/ieee802154_i.h | 12 ++++++ net/mac802154/iface.c | 44 ++++++++-------------- net/mac802154/rx.c | 12 +++++- 6 files changed, 115 insertions(+), 33 deletions(-) diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index 428cece222059..e1481f9cf049c 100644 --- a/include/net/cfg802154.h +++ b/include/net/cfg802154.h @@ -223,6 +223,11 @@ struct wpan_phy { atomic_t hold_txs; wait_queue_head_t sync_txq; + /* Current filtering level on reception. + * Only allowed to be changed if phy is not operational. + */ + enum ieee802154_filtering_level filtering; + char priv[] __aligned(NETDEV_ALIGN); }; @@ -374,8 +379,6 @@ struct wpan_dev { bool lbt; - bool promiscuous_mode; - /* fallback for acknowledgment bit setting */ bool ackreq; }; diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c index 93df24f755724..dc2d918fac68e 100644 --- a/net/mac802154/cfg.c +++ b/net/mac802154/cfg.c @@ -67,7 +67,7 @@ static int ieee802154_resume(struct wpan_phy *wpan_phy) goto wake_up; /* restart hardware */ - ret = drv_start(local); + ret = drv_start(local, local->phy->filtering, &local->addr_filt); if (ret) return ret; diff --git a/net/mac802154/driver-ops.h b/net/mac802154/driver-ops.h index c9d54088a5670..a7af3f0ddb3ef 100644 --- a/net/mac802154/driver-ops.h +++ b/net/mac802154/driver-ops.h @@ -129,12 +129,81 @@ drv_set_promiscuous_mode(struct ieee802154_local *local, bool on) return ret; } -static inline int drv_start(struct ieee802154_local *local) +static inline int drv_start(struct ieee802154_local *local, + enum ieee802154_filtering_level level, + const struct ieee802154_hw_addr_filt *addr_filt) { int ret; might_sleep(); + /* setup receive mode parameters e.g. address mode */ + if (local->hw.flags & IEEE802154_HW_AFILT) { + ret = drv_set_pan_id(local, addr_filt->pan_id); + if (ret < 0) + return ret; + + ret = drv_set_short_addr(local, addr_filt->short_addr); + if (ret < 0) + return ret; + + ret = drv_set_extended_addr(local, addr_filt->ieee_addr); + if (ret < 0) + return ret; + } + + switch (level) { + case IEEE802154_FILTERING_NONE: + fallthrough; + case IEEE802154_FILTERING_1_FCS: + fallthrough; + case IEEE802154_FILTERING_2_PROMISCUOUS: + /* TODO: Requires a different receive mode setup e.g. + * at86rf233 hardware. + */ + fallthrough; + case IEEE802154_FILTERING_3_SCAN: + if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) { + ret = drv_set_promiscuous_mode(local, true); + if (ret < 0) + return ret; + } else { + return -EOPNOTSUPP; + } + + /* In practice other filtering levels can be requested, but as + * for now most hardware/drivers only support + * IEEE802154_FILTERING_NONE, we fallback to this actual + * filtering level in hardware and make our own additional + * filtering in mac802154 receive path. + * + * TODO: Move this logic to the device drivers as hardware may + * support more higher level filters. Hardware may also require + * a different order how register are set, which could currently + * be buggy, so all received parameters need to be moved to the + * start() callback and let the driver go into the mode before + * it will turn on receive handling. + */ + local->phy->filtering = IEEE802154_FILTERING_NONE; + break; + case IEEE802154_FILTERING_4_FRAME_FIELDS: + /* Do not error out if IEEE802154_HW_PROMISCUOUS because we + * expect the hardware to operate at the level + * IEEE802154_FILTERING_4_FRAME_FIELDS anyway. + */ + if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) { + ret = drv_set_promiscuous_mode(local, false); + if (ret < 0) + return ret; + } + + local->phy->filtering = IEEE802154_FILTERING_4_FRAME_FIELDS; + break; + default: + WARN_ON(1); + return -EINVAL; + } + trace_802154_drv_start(local); local->started = true; smp_mb(); diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index 010365a6364e0..509e0172fe821 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -26,6 +26,8 @@ struct ieee802154_local { struct ieee802154_hw hw; const struct ieee802154_ops *ops; + /* hardware address filter */ + struct ieee802154_hw_addr_filt addr_filt; /* ieee802154 phy */ struct wpan_phy *phy; @@ -82,6 +84,16 @@ struct ieee802154_sub_if_data { struct ieee802154_local *local; struct net_device *dev; + /* Each interface starts and works in nominal state at a given filtering + * level given by iface_default_filtering, which is set once for all at + * the interface creation and should not evolve over time. For some MAC + * operations however, the filtering level may change temporarily, as + * reflected in the required_filtering field. The actual filtering at + * the PHY level may be different and is shown in struct wpan_phy. + */ + enum ieee802154_filtering_level iface_default_filtering; + enum ieee802154_filtering_level required_filtering; + unsigned long state; char name[IFNAMSIZ]; diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c index 500ed1b812503..d9b50884d34e7 100644 --- a/net/mac802154/iface.c +++ b/net/mac802154/iface.c @@ -147,25 +147,12 @@ static int ieee802154_setup_hw(struct ieee802154_sub_if_data *sdata) struct wpan_dev *wpan_dev = &sdata->wpan_dev; int ret; - if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) { - ret = drv_set_promiscuous_mode(local, - wpan_dev->promiscuous_mode); - if (ret < 0) - return ret; - } + sdata->required_filtering = sdata->iface_default_filtering; if (local->hw.flags & IEEE802154_HW_AFILT) { - ret = drv_set_pan_id(local, wpan_dev->pan_id); - if (ret < 0) - return ret; - - ret = drv_set_extended_addr(local, wpan_dev->extended_addr); - if (ret < 0) - return ret; - - ret = drv_set_short_addr(local, wpan_dev->short_addr); - if (ret < 0) - return ret; + local->addr_filt.pan_id = wpan_dev->pan_id; + local->addr_filt.ieee_addr = wpan_dev->extended_addr; + local->addr_filt.short_addr = wpan_dev->short_addr; } if (local->hw.flags & IEEE802154_HW_LBT) { @@ -206,7 +193,8 @@ static int mac802154_slave_open(struct net_device *dev) if (res) goto err; - res = drv_start(local); + res = drv_start(local, sdata->required_filtering, + &local->addr_filt); if (res) goto err; } @@ -223,15 +211,16 @@ static int mac802154_slave_open(struct net_device *dev) static int ieee802154_check_mac_settings(struct ieee802154_local *local, - struct wpan_dev *wpan_dev, - struct wpan_dev *nwpan_dev) + struct ieee802154_sub_if_data *sdata, + struct ieee802154_sub_if_data *nsdata) { + struct wpan_dev *nwpan_dev = &nsdata->wpan_dev; + struct wpan_dev *wpan_dev = &sdata->wpan_dev; + ASSERT_RTNL(); - if (local->hw.flags & IEEE802154_HW_PROMISCUOUS) { - if (wpan_dev->promiscuous_mode != nwpan_dev->promiscuous_mode) - return -EBUSY; - } + if (sdata->iface_default_filtering != nsdata->iface_default_filtering) + return -EBUSY; if (local->hw.flags & IEEE802154_HW_AFILT) { if (wpan_dev->pan_id != nwpan_dev->pan_id || @@ -285,8 +274,7 @@ ieee802154_check_concurrent_iface(struct ieee802154_sub_if_data *sdata, /* check all phy mac sublayer settings are the same. * We have only one phy, different values makes trouble. */ - ret = ieee802154_check_mac_settings(local, wpan_dev, - &nsdata->wpan_dev); + ret = ieee802154_check_mac_settings(local, sdata, nsdata); if (ret < 0) return ret; } @@ -586,7 +574,7 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata, sdata->dev->priv_destructor = mac802154_wpan_free; sdata->dev->netdev_ops = &mac802154_wpan_ops; sdata->dev->ml_priv = &mac802154_mlme_wpan; - wpan_dev->promiscuous_mode = false; + sdata->iface_default_filtering = IEEE802154_FILTERING_4_FRAME_FIELDS; wpan_dev->header_ops = &ieee802154_header_ops; mutex_init(&sdata->sec_mtx); @@ -600,7 +588,7 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata, case NL802154_IFTYPE_MONITOR: sdata->dev->needs_free_netdev = true; sdata->dev->netdev_ops = &mac802154_monitor_ops; - wpan_dev->promiscuous_mode = true; + sdata->iface_default_filtering = IEEE802154_FILTERING_NONE; break; default: BUG(); diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c index b8ce84618a55b..8543c28948a06 100644 --- a/net/mac802154/rx.c +++ b/net/mac802154/rx.c @@ -268,10 +268,20 @@ void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb) ieee802154_monitors_rx(local, skb); + /* TODO: Avoid delivering frames received at the level + * IEEE802154_FILTERING_NONE on interfaces not expecting it because of + * the missing auto ACK handling feature. + */ + + /* TODO: Handle upcomming receive path where the PHY is at the + * IEEE802154_FILTERING_NONE level during a scan. + */ + /* Check if transceiver doesn't validate the checksum. * If not we validate the checksum here. */ - if (local->hw.flags & IEEE802154_HW_RX_DROP_BAD_CKSUM) { + if (local->hw.flags & IEEE802154_HW_RX_DROP_BAD_CKSUM || + local->phy->filtering == IEEE802154_FILTERING_NONE) { crc = crc_ccitt(0, skb->data, skb->len); if (crc) { rcu_read_unlock(); From a87815b7bbcd10d7e113c366eb6310c5898ff952 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Fri, 7 Oct 2022 10:53:06 +0200 Subject: [PATCH 20/28] ieee802154: hwsim: Record the address filter values As a first step, introduce a basic implementation for the ->set_hw_addr_filt() hook. In a second step, the values recorded here will be used to perform proper filtering during reception. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20221007085310.503366-5-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/mac802154_hwsim.c | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c index 38c217bd7c822..458be66b51959 100644 --- a/drivers/net/ieee802154/mac802154_hwsim.c +++ b/drivers/net/ieee802154/mac802154_hwsim.c @@ -47,6 +47,7 @@ static const struct genl_multicast_group hwsim_mcgrps[] = { struct hwsim_pib { u8 page; u8 channel; + struct ieee802154_hw_addr_filt filt; struct rcu_head rcu; }; @@ -101,6 +102,38 @@ static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel) pib->channel = channel; pib_old = rtnl_dereference(phy->pib); + + pib->filt.short_addr = pib_old->filt.short_addr; + pib->filt.pan_id = pib_old->filt.pan_id; + pib->filt.ieee_addr = pib_old->filt.ieee_addr; + pib->filt.pan_coord = pib_old->filt.pan_coord; + + rcu_assign_pointer(phy->pib, pib); + kfree_rcu(pib_old, rcu); + return 0; +} + +static int hwsim_hw_addr_filt(struct ieee802154_hw *hw, + struct ieee802154_hw_addr_filt *filt, + unsigned long changed) +{ + struct hwsim_phy *phy = hw->priv; + struct hwsim_pib *pib, *pib_old; + + pib = kzalloc(sizeof(*pib), GFP_KERNEL); + if (!pib) + return -ENOMEM; + + pib_old = rtnl_dereference(phy->pib); + + pib->page = pib_old->page; + pib->channel = pib_old->channel; + + pib->filt.short_addr = filt->short_addr; + pib->filt.pan_id = filt->pan_id; + pib->filt.ieee_addr = filt->ieee_addr; + pib->filt.pan_coord = filt->pan_coord; + rcu_assign_pointer(phy->pib, pib); kfree_rcu(pib_old, rcu); return 0; @@ -172,6 +205,7 @@ static const struct ieee802154_ops hwsim_ops = { .start = hwsim_hw_start, .stop = hwsim_hw_stop, .set_promiscuous_mode = hwsim_set_promiscuous_mode, + .set_hw_addr_filt = hwsim_hw_addr_filt, }; static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info) @@ -787,6 +821,8 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev, } pib->channel = 13; + pib->filt.short_addr = cpu_to_le16(IEEE802154_ADDR_BROADCAST); + pib->filt.pan_id = cpu_to_le16(IEEE802154_PANID_BROADCAST); rcu_assign_pointer(phy->pib, pib); phy->idx = idx; INIT_LIST_HEAD(&phy->edges); From ea562d8c486eebd2707bcd193974078a2a47affc Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Fri, 7 Oct 2022 10:53:07 +0200 Subject: [PATCH 21/28] ieee802154: hwsim: Implement address filtering We have access to the address filters being theoretically applied, we also have access to the actual filtering level applied, so let's add a proper frame validation sequence in hwsim. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20221007085310.503366-6-miquel.raynal@bootlin.com [stefan@datenfreihafen.org: fixup some checkpatch warnings] Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/mac802154_hwsim.c | 110 ++++++++++++++++++++++- include/net/ieee802154_netdev.h | 8 ++ 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c index 458be66b51959..75d802e0b6853 100644 --- a/drivers/net/ieee802154/mac802154_hwsim.c +++ b/drivers/net/ieee802154/mac802154_hwsim.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -139,6 +140,112 @@ static int hwsim_hw_addr_filt(struct ieee802154_hw *hw, return 0; } +static void hwsim_hw_receive(struct ieee802154_hw *hw, struct sk_buff *skb, + u8 lqi) +{ + struct ieee802154_hdr hdr; + struct hwsim_phy *phy = hw->priv; + struct hwsim_pib *pib; + + rcu_read_lock(); + pib = rcu_dereference(phy->pib); + + if (!pskb_may_pull(skb, 3)) { + dev_dbg(hw->parent, "invalid frame\n"); + goto drop; + } + + memcpy(&hdr, skb->data, 3); + + /* Level 4 filtering: Frame fields validity */ + if (hw->phy->filtering == IEEE802154_FILTERING_4_FRAME_FIELDS) { + /* a) Drop reserved frame types */ + switch (mac_cb(skb)->type) { + case IEEE802154_FC_TYPE_BEACON: + case IEEE802154_FC_TYPE_DATA: + case IEEE802154_FC_TYPE_ACK: + case IEEE802154_FC_TYPE_MAC_CMD: + break; + default: + dev_dbg(hw->parent, "unrecognized frame type 0x%x\n", + mac_cb(skb)->type); + goto drop; + } + + /* b) Drop reserved frame versions */ + switch (hdr.fc.version) { + case IEEE802154_2003_STD: + case IEEE802154_2006_STD: + case IEEE802154_STD: + break; + default: + dev_dbg(hw->parent, + "unrecognized frame version 0x%x\n", + hdr.fc.version); + goto drop; + } + + /* c) PAN ID constraints */ + if ((mac_cb(skb)->dest.mode == IEEE802154_ADDR_LONG || + mac_cb(skb)->dest.mode == IEEE802154_ADDR_SHORT) && + mac_cb(skb)->dest.pan_id != pib->filt.pan_id && + mac_cb(skb)->dest.pan_id != cpu_to_le16(IEEE802154_PANID_BROADCAST)) { + dev_dbg(hw->parent, + "unrecognized PAN ID %04x\n", + le16_to_cpu(mac_cb(skb)->dest.pan_id)); + goto drop; + } + + /* d1) Short address constraints */ + if (mac_cb(skb)->dest.mode == IEEE802154_ADDR_SHORT && + mac_cb(skb)->dest.short_addr != pib->filt.short_addr && + mac_cb(skb)->dest.short_addr != cpu_to_le16(IEEE802154_ADDR_BROADCAST)) { + dev_dbg(hw->parent, + "unrecognized short address %04x\n", + le16_to_cpu(mac_cb(skb)->dest.short_addr)); + goto drop; + } + + /* d2) Extended address constraints */ + if (mac_cb(skb)->dest.mode == IEEE802154_ADDR_LONG && + mac_cb(skb)->dest.extended_addr != pib->filt.ieee_addr) { + dev_dbg(hw->parent, + "unrecognized long address 0x%016llx\n", + mac_cb(skb)->dest.extended_addr); + goto drop; + } + + /* d4) Specific PAN coordinator case (no parent) */ + if ((mac_cb(skb)->type == IEEE802154_FC_TYPE_DATA || + mac_cb(skb)->type == IEEE802154_FC_TYPE_MAC_CMD) && + mac_cb(skb)->dest.mode == IEEE802154_ADDR_NONE) { + dev_dbg(hw->parent, + "relaying is not supported\n"); + goto drop; + } + + /* e) Beacon frames follow specific PAN ID rules */ + if (mac_cb(skb)->type == IEEE802154_FC_TYPE_BEACON && + pib->filt.pan_id != cpu_to_le16(IEEE802154_PANID_BROADCAST) && + mac_cb(skb)->dest.pan_id != pib->filt.pan_id) { + dev_dbg(hw->parent, + "invalid beacon PAN ID %04x\n", + le16_to_cpu(mac_cb(skb)->dest.pan_id)); + goto drop; + } + } + + rcu_read_unlock(); + + ieee802154_rx_irqsafe(hw, skb, lqi); + + return; + +drop: + rcu_read_unlock(); + kfree_skb(skb); +} + static int hwsim_hw_xmit(struct ieee802154_hw *hw, struct sk_buff *skb) { struct hwsim_phy *current_phy = hw->priv; @@ -166,8 +273,7 @@ static int hwsim_hw_xmit(struct ieee802154_hw *hw, struct sk_buff *skb) einfo = rcu_dereference(e->info); if (newskb) - ieee802154_rx_irqsafe(e->endpoint->hw, newskb, - einfo->lqi); + hwsim_hw_receive(e->endpoint->hw, newskb, einfo->lqi); } } rcu_read_unlock(); diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h index d0d188c3294bd..1b82bbafe8c7a 100644 --- a/include/net/ieee802154_netdev.h +++ b/include/net/ieee802154_netdev.h @@ -69,6 +69,14 @@ struct ieee802154_hdr_fc { #endif }; +enum ieee802154_frame_version { + IEEE802154_2003_STD, + IEEE802154_2006_STD, + IEEE802154_STD, + IEEE802154_RESERVED_STD, + IEEE802154_MULTIPURPOSE_STD = IEEE802154_2003_STD, +}; + struct ieee802154_hdr { struct ieee802154_hdr_fc fc; u8 seq; From a4b5b4c56dd8b1dd46b2f13cb09f5f8031978f86 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Fri, 7 Oct 2022 10:53:08 +0200 Subject: [PATCH 22/28] mac802154: Drop IEEE802154_HW_RX_DROP_BAD_CKSUM This IEEE802154_HW_RX_DROP_BAD_CKSUM flag was only used by hwsim to reflect the fact that it would not validate the checksum (FCS). So this was only useful while the only filtering level hwsim was capable of was "NONE". Now that the driver has been improved we no longer need this flag. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20221007085310.503366-7-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/mac802154_hwsim.c | 3 ++- include/net/mac802154.h | 4 ---- net/mac802154/rx.c | 7 ++----- 3 files changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c index 75d802e0b6853..1db7da3ccc1a0 100644 --- a/drivers/net/ieee802154/mac802154_hwsim.c +++ b/drivers/net/ieee802154/mac802154_hwsim.c @@ -287,6 +287,7 @@ static int hwsim_hw_start(struct ieee802154_hw *hw) struct hwsim_phy *phy = hw->priv; phy->suspended = false; + return 0; } @@ -933,7 +934,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 | IEEE802154_HW_RX_DROP_BAD_CKSUM; + hw->flags = IEEE802154_HW_PROMISCUOUS; hw->parent = dev; err = ieee802154_register_hw(hw); diff --git a/include/net/mac802154.h b/include/net/mac802154.h index 357d25ef627a3..4a3a9de9da733 100644 --- a/include/net/mac802154.h +++ b/include/net/mac802154.h @@ -111,9 +111,6 @@ struct ieee802154_hw { * promiscuous mode setting. * * @IEEE802154_HW_RX_OMIT_CKSUM: Indicates that receiver omits FCS. - * - * @IEEE802154_HW_RX_DROP_BAD_CKSUM: Indicates that receiver will not filter - * frames with bad checksum. */ enum ieee802154_hw_flags { IEEE802154_HW_TX_OMIT_CKSUM = BIT(0), @@ -123,7 +120,6 @@ enum ieee802154_hw_flags { IEEE802154_HW_AFILT = BIT(4), IEEE802154_HW_PROMISCUOUS = BIT(5), IEEE802154_HW_RX_OMIT_CKSUM = BIT(6), - IEEE802154_HW_RX_DROP_BAD_CKSUM = BIT(7), }; /* Indicates that receiver omits FCS and xmitter will add FCS on it's own. */ diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c index 8543c28948a06..80dd52bc6bf13 100644 --- a/net/mac802154/rx.c +++ b/net/mac802154/rx.c @@ -277,11 +277,8 @@ void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb) * IEEE802154_FILTERING_NONE level during a scan. */ - /* Check if transceiver doesn't validate the checksum. - * If not we validate the checksum here. - */ - if (local->hw.flags & IEEE802154_HW_RX_DROP_BAD_CKSUM || - local->phy->filtering == IEEE802154_FILTERING_NONE) { + /* Level 1 filtering: Check the FCS by software when relevant */ + if (local->hw.phy->filtering == IEEE802154_FILTERING_NONE) { crc = crc_ccitt(0, skb->data, skb->len); if (crc) { rcu_read_unlock(); From 0218277df5a527c9e9474aa0e4e4e2f4bbc6f5cc Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Fri, 7 Oct 2022 10:53:09 +0200 Subject: [PATCH 23/28] mac802154: Avoid delivering frames received in a non satisfying filtering mode We must avoid the situation where one interface disables address filtering and AACK on the PHY while another interface expects to run with AACK and address filtering enabled. Just ignore the frames on the concerned interface if this happens. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20221007085310.503366-8-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/rx.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c index 80dd52bc6bf13..6640bd2373523 100644 --- a/net/mac802154/rx.c +++ b/net/mac802154/rx.c @@ -209,6 +209,13 @@ __ieee802154_rx_handle_packet(struct ieee802154_local *local, if (!ieee802154_sdata_running(sdata)) continue; + /* Do not deliver packets received on interfaces expecting + * AACK=1 if the address filters where disabled. + */ + if (local->hw.phy->filtering < IEEE802154_FILTERING_4_FRAME_FIELDS && + sdata->required_filtering == IEEE802154_FILTERING_4_FRAME_FIELDS) + continue; + ieee802154_subif_frame(sdata, skb, &hdr); skb = NULL; break; @@ -268,11 +275,6 @@ void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb) ieee802154_monitors_rx(local, skb); - /* TODO: Avoid delivering frames received at the level - * IEEE802154_FILTERING_NONE on interfaces not expecting it because of - * the missing auto ACK handling feature. - */ - /* TODO: Handle upcomming receive path where the PHY is at the * IEEE802154_FILTERING_NONE level during a scan. */ From 3a22550ab50a2bed1779c7d03c9f0239d33cc514 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Mon, 5 Sep 2022 22:27:24 +0200 Subject: [PATCH 24/28] net: mac802154: Avoid displaying misleading debug information With DEBUG defined, any frame received will see its MHR fields (fc and addresses, mainly) being printed in the kernel log buffer, unconditionally. In most cases this is fine, but in some specific cases (like Acknowledgment frames, where both the source and destination addressing fields are omitted), it displays garbage which is misleading. Only print the addressing fields when they are present, which clarifies the logs. Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/r/20220905202724.1322046-1-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/rx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c index 6640bd2373523..d1f7b8df41fe3 100644 --- a/net/mac802154/rx.c +++ b/net/mac802154/rx.c @@ -114,8 +114,10 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata, static void ieee802154_print_addr(const char *name, const struct ieee802154_addr *addr) { - if (addr->mode == IEEE802154_ADDR_NONE) + if (addr->mode == IEEE802154_ADDR_NONE) { pr_debug("%s not present\n", name); + return; + } pr_debug("%s PAN ID: %04x\n", name, le16_to_cpu(addr->pan_id)); if (addr->mode == IEEE802154_ADDR_SHORT) { From f8be91fbfc7dd2d69762d510cc29c3b52d3ef4db Mon Sep 17 00:00:00 2001 From: Alexander Aring Date: Mon, 5 Sep 2022 22:34:12 +0200 Subject: [PATCH 25/28] ieee802154: atusb: add support for trac feature This patch adds support for reading the trac register if atusb firmware reports tx done. There is currently a feature to compare a sequence number, if the payload is 1 it tells the driver only the sequence number is available if it's two there is additional the trac status register as payload. Currently the atusb_in_good() function determines if it's a tx done or rx done if according the payload length. This patch is doing the same and assumes this behaviour. Signed-off-by: Alexander Aring Signed-off-by: Miquel Raynal Link: https://lore.kernel.org/r/20220905203412.1322947-10-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/atusb.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/drivers/net/ieee802154/atusb.c b/drivers/net/ieee802154/atusb.c index 2c338783893d9..95a4a3cdc8a4c 100644 --- a/drivers/net/ieee802154/atusb.c +++ b/drivers/net/ieee802154/atusb.c @@ -191,7 +191,7 @@ static void atusb_work_urbs(struct work_struct *work) /* ----- Asynchronous USB -------------------------------------------------- */ -static void atusb_tx_done(struct atusb *atusb, u8 seq) +static void atusb_tx_done(struct atusb *atusb, u8 seq, int reason) { struct usb_device *usb_dev = atusb->usb_dev; u8 expect = atusb->tx_ack_seq; @@ -199,7 +199,10 @@ static void atusb_tx_done(struct atusb *atusb, u8 seq) dev_dbg(&usb_dev->dev, "%s (0x%02x/0x%02x)\n", __func__, seq, expect); if (seq == expect) { /* TODO check for ifs handling in firmware */ - ieee802154_xmit_complete(atusb->hw, atusb->tx_skb, false); + if (reason == IEEE802154_SUCCESS) + ieee802154_xmit_complete(atusb->hw, atusb->tx_skb, false); + else + ieee802154_xmit_error(atusb->hw, atusb->tx_skb, reason); } else { /* TODO I experience this case when atusb has a tx complete * irq before probing, we should fix the firmware it's an @@ -215,7 +218,8 @@ static void atusb_in_good(struct urb *urb) struct usb_device *usb_dev = urb->dev; struct sk_buff *skb = urb->context; struct atusb *atusb = SKB_ATUSB(skb); - u8 len, lqi; + int result = IEEE802154_SUCCESS; + u8 len, lqi, trac; if (!urb->actual_length) { dev_dbg(&usb_dev->dev, "atusb_in: zero-sized URB ?\n"); @@ -224,8 +228,27 @@ static void atusb_in_good(struct urb *urb) len = *skb->data; - if (urb->actual_length == 1) { - atusb_tx_done(atusb, len); + switch (urb->actual_length) { + case 2: + trac = TRAC_MASK(*(skb->data + 1)); + switch (trac) { + case TRAC_SUCCESS: + case TRAC_SUCCESS_DATA_PENDING: + /* already IEEE802154_SUCCESS */ + break; + case TRAC_CHANNEL_ACCESS_FAILURE: + result = IEEE802154_CHANNEL_ACCESS_FAILURE; + break; + case TRAC_NO_ACK: + result = IEEE802154_NO_ACK; + break; + default: + result = IEEE802154_SYSTEM_ERROR; + } + + fallthrough; + case 1: + atusb_tx_done(atusb, len, result); return; } From 9a60850e8cd9160beba79aa6d529332eb18c9bc7 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Wed, 19 Oct 2022 15:44:21 +0200 Subject: [PATCH 26/28] ieee802154: hwsim: Introduce a helper to update all the PIB attributes Perform the update of the PIB structure only in a single place. This way we can have much simpler functions when updating the page, channel or address filters. This helper will become even more useful when we will update the ->set_promiscuous() callback to actually save the filtering level in the PIB structure. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20221019134423.877169-2-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/mac802154_hwsim.c | 56 +++++++++++++----------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c index 1db7da3ccc1a0..44dbd5f27dc5c 100644 --- a/drivers/net/ieee802154/mac802154_hwsim.c +++ b/drivers/net/ieee802154/mac802154_hwsim.c @@ -90,54 +90,58 @@ static int hwsim_hw_ed(struct ieee802154_hw *hw, u8 *level) return 0; } -static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel) +static int hwsim_update_pib(struct ieee802154_hw *hw, u8 page, u8 channel, + struct ieee802154_hw_addr_filt *filt) { struct hwsim_phy *phy = hw->priv; struct hwsim_pib *pib, *pib_old; - pib = kzalloc(sizeof(*pib), GFP_KERNEL); + pib = kzalloc(sizeof(*pib), GFP_ATOMIC); if (!pib) return -ENOMEM; - pib->page = page; - pib->channel = channel; - pib_old = rtnl_dereference(phy->pib); - pib->filt.short_addr = pib_old->filt.short_addr; - pib->filt.pan_id = pib_old->filt.pan_id; - pib->filt.ieee_addr = pib_old->filt.ieee_addr; - pib->filt.pan_coord = pib_old->filt.pan_coord; + pib->page = page; + pib->channel = channel; + pib->filt.short_addr = filt->short_addr; + pib->filt.pan_id = filt->pan_id; + pib->filt.ieee_addr = filt->ieee_addr; + pib->filt.pan_coord = filt->pan_coord; rcu_assign_pointer(phy->pib, pib); kfree_rcu(pib_old, rcu); return 0; } -static int hwsim_hw_addr_filt(struct ieee802154_hw *hw, - struct ieee802154_hw_addr_filt *filt, - unsigned long changed) +static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel) { struct hwsim_phy *phy = hw->priv; - struct hwsim_pib *pib, *pib_old; + struct hwsim_pib *pib; + int ret; - pib = kzalloc(sizeof(*pib), GFP_KERNEL); - if (!pib) - return -ENOMEM; + rcu_read_lock(); + pib = rcu_dereference(phy->pib); + ret = hwsim_update_pib(hw, page, channel, &pib->filt); + rcu_read_unlock(); - pib_old = rtnl_dereference(phy->pib); + return ret; +} - pib->page = pib_old->page; - pib->channel = pib_old->channel; +static int hwsim_hw_addr_filt(struct ieee802154_hw *hw, + struct ieee802154_hw_addr_filt *filt, + unsigned long changed) +{ + struct hwsim_phy *phy = hw->priv; + struct hwsim_pib *pib; + int ret; - pib->filt.short_addr = filt->short_addr; - pib->filt.pan_id = filt->pan_id; - pib->filt.ieee_addr = filt->ieee_addr; - pib->filt.pan_coord = filt->pan_coord; + rcu_read_lock(); + pib = rcu_dereference(phy->pib); + ret = hwsim_update_pib(hw, pib->page, pib->channel, filt); + rcu_read_unlock(); - rcu_assign_pointer(phy->pib, pib); - kfree_rcu(pib_old, rcu); - return 0; + return ret; } static void hwsim_hw_receive(struct ieee802154_hw *hw, struct sk_buff *skb, From 19177eedcf4412a90ec1ebaffac3514d0a3b4ff2 Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Wed, 19 Oct 2022 15:44:22 +0200 Subject: [PATCH 27/28] ieee802154: hwsim: Save the current filtering level and use it Save the requested filtering level in the ->set_promiscuous() helper. The logic is: either we want to enable promiscuous mode and we want to disable filters entirely, or we want to use the highest filtering level by default. This is of course an assumption that only works today, but if in the future intermediate levels (such as scan filtering level) are implemented in the core, this logic will need to be updated. This would imply replacing ->set_promiscuous() by something more fine grained anyway, so we are probably safe with this assumption. Once saved in the PIB structure, we can use this value instead of trying to access the PHY structure to know what hardware filtering level has been advertised. Suggested-by: Alexander Aring Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20221019134423.877169-3-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- drivers/net/ieee802154/mac802154_hwsim.c | 28 +++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c index 44dbd5f27dc5c..9034706d4a53a 100644 --- a/drivers/net/ieee802154/mac802154_hwsim.c +++ b/drivers/net/ieee802154/mac802154_hwsim.c @@ -49,6 +49,7 @@ struct hwsim_pib { u8 page; u8 channel; struct ieee802154_hw_addr_filt filt; + enum ieee802154_filtering_level filt_level; struct rcu_head rcu; }; @@ -91,7 +92,8 @@ static int hwsim_hw_ed(struct ieee802154_hw *hw, u8 *level) } static int hwsim_update_pib(struct ieee802154_hw *hw, u8 page, u8 channel, - struct ieee802154_hw_addr_filt *filt) + struct ieee802154_hw_addr_filt *filt, + enum ieee802154_filtering_level filt_level) { struct hwsim_phy *phy = hw->priv; struct hwsim_pib *pib, *pib_old; @@ -108,6 +110,7 @@ static int hwsim_update_pib(struct ieee802154_hw *hw, u8 page, u8 channel, pib->filt.pan_id = filt->pan_id; pib->filt.ieee_addr = filt->ieee_addr; pib->filt.pan_coord = filt->pan_coord; + pib->filt_level = filt_level; rcu_assign_pointer(phy->pib, pib); kfree_rcu(pib_old, rcu); @@ -122,7 +125,7 @@ static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel) rcu_read_lock(); pib = rcu_dereference(phy->pib); - ret = hwsim_update_pib(hw, page, channel, &pib->filt); + ret = hwsim_update_pib(hw, page, channel, &pib->filt, pib->filt_level); rcu_read_unlock(); return ret; @@ -138,7 +141,7 @@ static int hwsim_hw_addr_filt(struct ieee802154_hw *hw, rcu_read_lock(); pib = rcu_dereference(phy->pib); - ret = hwsim_update_pib(hw, pib->page, pib->channel, filt); + ret = hwsim_update_pib(hw, pib->page, pib->channel, filt, pib->filt_level); rcu_read_unlock(); return ret; @@ -162,7 +165,7 @@ static void hwsim_hw_receive(struct ieee802154_hw *hw, struct sk_buff *skb, memcpy(&hdr, skb->data, 3); /* Level 4 filtering: Frame fields validity */ - if (hw->phy->filtering == IEEE802154_FILTERING_4_FRAME_FIELDS) { + if (pib->filt_level == IEEE802154_FILTERING_4_FRAME_FIELDS) { /* a) Drop reserved frame types */ switch (mac_cb(skb)->type) { case IEEE802154_FC_TYPE_BEACON: @@ -305,7 +308,22 @@ static void hwsim_hw_stop(struct ieee802154_hw *hw) static int hwsim_set_promiscuous_mode(struct ieee802154_hw *hw, const bool on) { - return 0; + enum ieee802154_filtering_level filt_level; + struct hwsim_phy *phy = hw->priv; + struct hwsim_pib *pib; + int ret; + + if (on) + filt_level = IEEE802154_FILTERING_NONE; + else + filt_level = IEEE802154_FILTERING_4_FRAME_FIELDS; + + rcu_read_lock(); + pib = rcu_dereference(phy->pib); + ret = hwsim_update_pib(hw, pib->page, pib->channel, &pib->filt, filt_level); + rcu_read_unlock(); + + return ret; } static const struct ieee802154_ops hwsim_ops = { From 4161634bce9537ed173b3c8fd0bf9f0218bcf41c Mon Sep 17 00:00:00 2001 From: Miquel Raynal Date: Wed, 19 Oct 2022 15:44:23 +0200 Subject: [PATCH 28/28] mac802154: Ensure proper scan-level filtering We now have a fine grained filtering information so let's ensure proper filtering in scan mode, which means that only beacons are processed. Signed-off-by: Miquel Raynal Acked-by: Alexander Aring Link: https://lore.kernel.org/r/20221019134423.877169-4-miquel.raynal@bootlin.com Signed-off-by: Stefan Schmidt --- net/mac802154/rx.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c index d1f7b8df41fe3..dc9ca2c0ea84f 100644 --- a/net/mac802154/rx.c +++ b/net/mac802154/rx.c @@ -34,6 +34,7 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata, struct sk_buff *skb, const struct ieee802154_hdr *hdr) { struct wpan_dev *wpan_dev = &sdata->wpan_dev; + struct wpan_phy *wpan_phy = sdata->local->hw.phy; __le16 span, sshort; int rc; @@ -42,6 +43,17 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata, span = wpan_dev->pan_id; sshort = wpan_dev->short_addr; + /* Level 3 filtering: Only beacons are accepted during scans */ + if (sdata->required_filtering == IEEE802154_FILTERING_3_SCAN && + sdata->required_filtering > wpan_phy->filtering) { + if (mac_cb(skb)->type != IEEE802154_FC_TYPE_BEACON) { + dev_dbg(&sdata->dev->dev, + "drop non-beacon frame (0x%x) during scan\n", + mac_cb(skb)->type); + goto fail; + } + } + switch (mac_cb(skb)->dest.mode) { case IEEE802154_ADDR_NONE: if (mac_cb(skb)->dest.mode != IEEE802154_ADDR_NONE) @@ -277,10 +289,6 @@ void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb) ieee802154_monitors_rx(local, skb); - /* TODO: Handle upcomming receive path where the PHY is at the - * IEEE802154_FILTERING_NONE level during a scan. - */ - /* Level 1 filtering: Check the FCS by software when relevant */ if (local->hw.phy->filtering == IEEE802154_FILTERING_NONE) { crc = crc_ccitt(0, skb->data, skb->len);