diff mbox series

mac802154_hwsim: define perm_extended_addr initialization

Message ID 20250326180909.10406-1-ramonreisfontes@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series mac802154_hwsim: define perm_extended_addr initialization | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 3 maintainers not CCed: edumazet@google.com andrew+netdev@lunn.ch stefan@datenfreihafen.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch fail ERROR: code indent should use tabs where possible ERROR: open brace '{' following function definitions go on the next line WARNING: please, no spaces at the start of a line
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Ramon Fontes March 26, 2025, 6:09 p.m. UTC 
This establishes an initialization method for perm_extended_addr, aligning it with the approach used in mac80211_hwsim.

Signed-off-by: Ramon Fontes <ramonreisfontes@gmail.com>
---
 drivers/net/ieee802154/mac802154_hwsim.c | 18 +++++++++++++++++-
 drivers/net/ieee802154/mac802154_hwsim.h |  2 ++
 2 files changed, 19 insertions(+), 1 deletion(-)

Comments

Alexander Aring March 26, 2025, 10:49 p.m. UTC | #1 
Hi,

On Wed, Mar 26, 2025 at 2:09 PM Ramon Fontes <ramonreisfontes@gmail.com> wrote:
>
> This establishes an initialization method for perm_extended_addr, aligning it with the approach used in mac80211_hwsim.
>

that is based on the phy index value instead of a random generated one?

> Signed-off-by: Ramon Fontes <ramonreisfontes@gmail.com>
> ---
>  drivers/net/ieee802154/mac802154_hwsim.c | 18 +++++++++++++++++-
>  drivers/net/ieee802154/mac802154_hwsim.h |  2 ++
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> index 1cab20b5a..400cdac1f 100644
> --- a/drivers/net/ieee802154/mac802154_hwsim.c
> +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> @@ -41,6 +41,17 @@ enum hwsim_multicast_groups {
>         HWSIM_MCGRP_CONFIG,
>  };
>
> +__le64 addr_to_le64(u8 *addr) {
> +    return cpu_to_le64(((u64)addr[0] << 56) |
> +                        ((u64)addr[1] << 48) |
> +                        ((u64)addr[2] << 40) |
> +                        ((u64)addr[3] << 32) |
> +                        ((u64)addr[4] << 24) |
> +                        ((u64)addr[5] << 16) |
> +                        ((u64)addr[6] << 8)  |
> +                        ((u64)addr[7]));
> +}
> +
>  static const struct genl_multicast_group hwsim_mcgrps[] = {
>         [HWSIM_MCGRP_CONFIG] = { .name = "config", },
>  };
> @@ -896,6 +907,7 @@ static int hwsim_subscribe_all_others(struct hwsim_phy *phy)
>  static int hwsim_add_one(struct genl_info *info, struct device *dev,
>                          bool init)
>  {
> +       u8 addr[8];

why not using directly __le64?

>         struct ieee802154_hw *hw;
>         struct hwsim_phy *phy;
>         struct hwsim_pib *pib;
> @@ -942,7 +954,11 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
>         /* 950 MHz GFSK 802.15.4d-2009 */
>         hw->phy->supported.channels[6] |= 0x3ffc00;
>
> -       ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
> +       memset(addr, 0, sizeof(addr));
> +       /* give a specific prefix to the address */
> +       addr[0] = 0x02;
> +       addr[7] = idx;
> +       hw->phy->perm_extended_addr = addr_to_le64(addr);
>
>         /* hwsim phy channel 13 as default */
>         hw->phy->current_channel = 13;
> diff --git a/drivers/net/ieee802154/mac802154_hwsim.h b/drivers/net/ieee802154/mac802154_hwsim.h
> index 6c6e30e38..536d95eb1 100644
> --- a/drivers/net/ieee802154/mac802154_hwsim.h
> +++ b/drivers/net/ieee802154/mac802154_hwsim.h
> @@ -1,6 +1,8 @@
>  #ifndef __MAC802154_HWSIM_H
>  #define __MAC802154_HWSIM_H
>
> +__le64 addr_to_le64(u8 *addr);
> +

This is a uapi header for netlink which is not yet delivered through
kernel-headers installation.

Why do we need this prototype declaration here?

Thanks.

- Alex
Ramon Fontes March 26, 2025, 11:24 p.m. UTC | #2 
On Wed, Mar 26, 2025 at 7:49 PM, Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Wed, Mar 26, 2025 at 2:09 PM Ramon Fontes <ramonreisfontes@gmail.com> wrote:
> >
> > This establishes an initialization method for perm_extended_addr, aligning it with the approach used in mac80211_hwsim.
> >
>
> that is based on the phy index value instead of a random generated one?

Yes, that's based on the phy index value.

>
> > Signed-off-by: Ramon Fontes <ramonreisfontes@gmail.com>
> > ---
> >  drivers/net/ieee802154/mac802154_hwsim.c | 18 +++++++++++++++++-
> >  drivers/net/ieee802154/mac802154_hwsim.h |  2 ++
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> > index 1cab20b5a..400cdac1f 100644
> > --- a/drivers/net/ieee802154/mac802154_hwsim.c
> > +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> > @@ -41,6 +41,17 @@ enum hwsim_multicast_groups {
> >         HWSIM_MCGRP_CONFIG,
> >  };
> >
> > +__le64 addr_to_le64(u8 *addr) {
> > +    return cpu_to_le64(((u64)addr[0] << 56) |
> > +                        ((u64)addr[1] << 48) |
> > +                        ((u64)addr[2] << 40) |
> > +                        ((u64)addr[3] << 32) |
> > +                        ((u64)addr[4] << 24) |
> > +                        ((u64)addr[5] << 16) |
> > +                        ((u64)addr[6] << 8)  |
> > +                        ((u64)addr[7]));
> > +}
> > +
> >  static const struct genl_multicast_group hwsim_mcgrps[] = {
> >         [HWSIM_MCGRP_CONFIG] = { .name = "config", },
> >  };
> > @@ -896,6 +907,7 @@ static int hwsim_subscribe_all_others(struct hwsim_phy *phy)
> >  static int hwsim_add_one(struct genl_info *info, struct device *dev,
> >                          bool init)
> >  {
> > +       u8 addr[8];
>
> why not using directly

Yes, we don't need it.

>
> >         struct ieee802154_hw *hw;
> >         struct hwsim_phy *phy;
> >         struct hwsim_pib *pib;
> > @@ -942,7 +954,11 @@ static int hwsim_add_one(struct genl_info *info, struct device *dev,
> >         /* 950 MHz GFSK 802.15.4d-2009 */
> >         hw->phy->supported.channels[6] |= 0x3ffc00;
> >
> > -       ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
> > +       memset(addr, 0, sizeof(addr));
> > +       /* give a specific prefix to the address */
> > +       addr[0] = 0x02;
> > +       addr[7] = idx;
> > +       hw->phy->perm_extended_addr = addr_to_le64(addr);

I think we can replace everything with only one line of code:

hw->phy->perm_extended_addr = cpu_to_le64(((u64)0x02 << 56) | ((u64)idx));

This does the trick! What do you think?

> >
> >         /* hwsim phy channel 13 as default */
> >         hw->phy->current_channel = 13;
> > diff --git a/drivers/net/ieee802154/mac802154_hwsim.h b/drivers/net/ieee802154/mac802154_hwsim.h
> > index 6c6e30e38..536d95eb1 100644
> > --- a/drivers/net/ieee802154/mac802154_hwsim.h
> > +++ b/drivers/net/ieee802154/mac802154_hwsim.h
> > @@ -1,6 +1,8 @@
> >  #ifndef __MAC802154_HWSIM_H
> >  #define __MAC802154_HWSIM_H
> >
> > +__le64 addr_to_le64(u8 *addr);
> > +
>
> This is a uapi header for netlink which is not yet delivered through
> kernel-headers installation.
>
> Why do we need this prototype declaration here?

We don't need it.

>
> Thanks.
>
> - Alex
>
diff mbox series

Patch

diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 1cab20b5a..400cdac1f 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -41,6 +41,17 @@  enum hwsim_multicast_groups {
 	HWSIM_MCGRP_CONFIG,
 };
 
+__le64 addr_to_le64(u8 *addr) {
+    return cpu_to_le64(((u64)addr[0] << 56) |
+                        ((u64)addr[1] << 48) |
+                        ((u64)addr[2] << 40) |
+                        ((u64)addr[3] << 32) |
+                        ((u64)addr[4] << 24) |
+                        ((u64)addr[5] << 16) |
+                        ((u64)addr[6] << 8)  |
+                        ((u64)addr[7]));
+}
+
 static const struct genl_multicast_group hwsim_mcgrps[] = {
 	[HWSIM_MCGRP_CONFIG] = { .name = "config", },
 };
@@ -896,6 +907,7 @@  static int hwsim_subscribe_all_others(struct hwsim_phy *phy)
 static int hwsim_add_one(struct genl_info *info, struct device *dev,
 			 bool init)
 {
+	u8 addr[8];
 	struct ieee802154_hw *hw;
 	struct hwsim_phy *phy;
 	struct hwsim_pib *pib;
@@ -942,7 +954,11 @@  static int hwsim_add_one(struct genl_info *info, struct device *dev,
 	/* 950 MHz GFSK 802.15.4d-2009 */
 	hw->phy->supported.channels[6] |= 0x3ffc00;
 
-	ieee802154_random_extended_addr(&hw->phy->perm_extended_addr);
+	memset(addr, 0, sizeof(addr));
+	/* give a specific prefix to the address */
+	addr[0] = 0x02;
+	addr[7] = idx;
+	hw->phy->perm_extended_addr = addr_to_le64(addr);
 
 	/* hwsim phy channel 13 as default */
 	hw->phy->current_channel = 13;
diff --git a/drivers/net/ieee802154/mac802154_hwsim.h b/drivers/net/ieee802154/mac802154_hwsim.h
index 6c6e30e38..536d95eb1 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.h
+++ b/drivers/net/ieee802154/mac802154_hwsim.h
@@ -1,6 +1,8 @@ 
 #ifndef __MAC802154_HWSIM_H
 #define __MAC802154_HWSIM_H
 
+__le64 addr_to_le64(u8 *addr);
+
 /* mac802154 hwsim netlink commands
  *
  * @MAC802154_HWSIM_CMD_UNSPEC: unspecified command to catch error