[PATCH wpan-next v6 0/3] IEEE 802.15.4 filtering series followup

[PATCH wpan-next v6 0/3] IEEE 802.15.4 filtering series followup

[Miquel Raynal]

Hello,

The filtering series v4 [1] lead to a discussion about the use of a PIB
attribute to save the required PHY filtering level instead of accessing
the MAC internals, which is bein addressed in patch 1/3 and 2/3. The
last patch has been sent alone as a v5 because of a debug message
needing rewording. Actually Stefan wanted me to rebase on top of
wpan-next without keeping a patch sent as a fix which conflicts with it,
so here it is.

Once these three patches will be merged (I don't expect much discussions
on it to be honest?) I will send the next small series bringing support
for COORD interfaces.

Cheers, Miquèl

Miquel Raynal (3):
  ieee802154: hwsim: Introduce a helper to update all the PIB attributes
  ieee802154: hwsim: Save the current filtering level and use it
  mac802154: Ensure proper scan-level filtering

 drivers/net/ieee802154/mac802154_hwsim.c | 88 +++++++++++++++---------
 net/mac802154/rx.c                       | 16 +++--
 2 files changed, 67 insertions(+), 37 deletions(-)

-- 
2.34.1

[PATCH wpan-next v6 1/3] ieee802154: hwsim: Introduce a helper to update all the PIB attributes

[Miquel Raynal]

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 <miquel.raynal@bootlin.com>
---
 drivers/net/ieee802154/mac802154_hwsim.c | 66 +++++++++++++-----------
 1 file changed, 35 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 1db7da3ccc1a..44dbd5f27dc5 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -90,46 +90,20 @@ 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_old = rtnl_dereference(phy->pib);
+
 	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;
-
-	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;
@@ -140,6 +114,36 @@ static int hwsim_hw_addr_filt(struct ieee802154_hw *hw,
 	return 0;
 }
 
+static int hwsim_hw_channel(struct ieee802154_hw *hw, u8 page, u8 channel)
+{
+	struct hwsim_phy *phy = hw->priv;
+	struct hwsim_pib *pib;
+	int ret;
+
+	rcu_read_lock();
+	pib = rcu_dereference(phy->pib);
+	ret = hwsim_update_pib(hw, page, channel, &pib->filt);
+	rcu_read_unlock();
+
+	return ret;
+}
+
+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;
+
+	rcu_read_lock();
+	pib = rcu_dereference(phy->pib);
+	ret = hwsim_update_pib(hw, pib->page, pib->channel, filt);
+	rcu_read_unlock();
+
+	return ret;
+}
+
 static void hwsim_hw_receive(struct ieee802154_hw *hw, struct sk_buff *skb,
 			     u8 lqi)
 {
-- 
2.34.1

[PATCH wpan-next v6 2/3] ieee802154: hwsim: Save the current filtering level and use it

[Miquel Raynal]

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 <alex.aring@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 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 44dbd5f27dc5..9034706d4a53 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 = {
-- 
2.34.1

[PATCH wpan-next v6 3/3] mac802154: Ensure proper scan-level filtering

[Miquel Raynal]

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 <miquel.raynal@bootlin.com>
---
 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 d1f7b8df41fe..dc9ca2c0ea84 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);
-- 
2.34.1

Re: [PATCH wpan-next v6 0/3] IEEE 802.15.4 filtering series followup

[Alexander Aring]

Hi,

On Wed, Oct 19, 2022 at 10:14 AM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
>
> Hello,
>
> The filtering series v4 [1] lead to a discussion about the use of a PIB
> attribute to save the required PHY filtering level instead of accessing
> the MAC internals, which is bein addressed in patch 1/3 and 2/3. The
> last patch has been sent alone as a v5 because of a debug message
> needing rewording. Actually Stefan wanted me to rebase on top of
> wpan-next without keeping a patch sent as a fix which conflicts with it,
> so here it is.
>
> Once these three patches will be merged (I don't expect much discussions
> on it to be honest?) I will send the next small series bringing support
> for COORD interfaces.
>
> Cheers, Miquèl
>

Acked-by: Alexander Aring <aahringo@redhat.com>

- Alex

Re: [PATCH wpan-next v6 0/3] IEEE 802.15.4 filtering series followup

[Stefan Schmidt]

Hello.

On 19.10.22 15:44, Miquel Raynal wrote:
> Hello,
> 
> The filtering series v4 [1] lead to a discussion about the use of a PIB
> attribute to save the required PHY filtering level instead of accessing
> the MAC internals, which is bein addressed in patch 1/3 and 2/3. The
> last patch has been sent alone as a v5 because of a debug message
> needing rewording. Actually Stefan wanted me to rebase on top of
> wpan-next without keeping a patch sent as a fix which conflicts with it,
> so here it is.
> 
> Once these three patches will be merged (I don't expect much discussions
> on it to be honest?) I will send the next small series bringing support
> for COORD interfaces.
> 
> Cheers, Miquèl
> 
> Miquel Raynal (3):
>    ieee802154: hwsim: Introduce a helper to update all the PIB attributes
>    ieee802154: hwsim: Save the current filtering level and use it
>    mac802154: Ensure proper scan-level filtering
> 
>   drivers/net/ieee802154/mac802154_hwsim.c | 88 +++++++++++++++---------
>   net/mac802154/rx.c                       | 16 +++--
>   2 files changed, 67 insertions(+), 37 deletions(-)


These patches have been applied to the wpan-next tree and will be
part of the next pull request to net-next. Thanks!

regards
Stefan Schmidt