netdev - Re: [PATCH] mac802154_hwsim: define perm_extended

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAK8U23Ydy8jC+7ZUfHaa+sQzECo3MYiDFwFY=MjP9z3GBjtQQw@mail.gmail.com>
Date: Wed, 26 Mar 2025 20:24:38 -0300
From: Ramon Fontes <ramonreisfontes@...il.com>
To: Alexander Aring <aahringo@...hat.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, 
	linux-wpan@...r.kernel.org, alex.aring@...il.com, miquel.raynal@...tlin.com, 
	netdev@...r.kernel.org
Subject: Re: [PATCH] mac802154_hwsim: define perm_extended_addr initialization

On Wed, Mar 26, 2025 at 7:49 PM, Alexander Aring <aahringo@...hat.com> wrote:
>
> Hi,
>
> On Wed, Mar 26, 2025 at 2:09 PM Ramon Fontes <ramonreisfontes@...il.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@...il.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
>