If simulation needs predictable permanent mac addresses of hwsim wireless
phy, this patch add the ability to create a new radio with a user defined
permanent mac address. Allowed mac addresses needs to be locally
administrated mac addresses (as also the former fixed 42:* and 02:* were).
To do not break the operation with legacy software using hwsim, the new
address is set twice. The problem here is, the netlink call backs use
wiphy->addresses[1] as identification of a radio and not the proposed
permanent address (wiphy->addresses[0]). This design decision is not
documented in the kernel repo, therefore this patch simply reproduces this,
but with the same address.
Signed-off-by: Benjamin Beichler <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 53 +++++++++++++++++++++++++++++------
drivers/net/wireless/mac80211_hwsim.h | 9 +++++-
2 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 2d4e97b6dc8f..1e0d651c43fc 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2375,6 +2375,7 @@ struct hwsim_new_radio_params {
bool destroy_on_close;
const char *hwname;
bool no_vif;
+ u8 *perm_addr;
};
static void hwsim_mcast_config_msg(struct sk_buff *mcast_skb,
@@ -2539,15 +2540,25 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
skb_queue_head_init(&data->pending);
SET_IEEE80211_DEV(hw, data->dev);
- eth_zero_addr(addr);
- addr[0] = 0x02;
- addr[3] = idx >> 8;
- addr[4] = idx;
- memcpy(data->addresses[0].addr, addr, ETH_ALEN);
- memcpy(data->addresses[1].addr, addr, ETH_ALEN);
- data->addresses[1].addr[0] |= 0x40;
- hw->wiphy->n_addresses = 2;
- hw->wiphy->addresses = data->addresses;
+ if (!param->perm_addr) {
+ eth_zero_addr(addr);
+ addr[0] = 0x02;
+ addr[3] = idx >> 8;
+ addr[4] = idx;
+ memcpy(data->addresses[0].addr, addr, ETH_ALEN);
+ /* Why need here second address ? */
+ data->addresses[1].addr[0] |= 0x40;
+ memcpy(data->addresses[1].addr, addr, ETH_ALEN);
+ hw->wiphy->n_addresses = 2;
+ hw->wiphy->addresses = data->addresses;
+ /* possible address clash is checked at hash table insertion */
+ } else {
+ memcpy(data->addresses[0].addr, param->perm_addr, ETH_ALEN);
+ /* compatibility with automatically generated mac addr */
+ memcpy(data->addresses[1].addr, param->perm_addr, ETH_ALEN);
+ hw->wiphy->n_addresses = 2;
+ hw->wiphy->addresses = data->addresses;
+ }
data->channels = param->channels;
data->use_chanctx = param->use_chanctx;
@@ -3172,6 +3183,30 @@ static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info)
param.regd = hwsim_world_regdom_custom[idx];
}
+ if (info->attrs[HWSIM_ATTR_PERM_ADDR]) {
+ if (nla_len(info->attrs[HWSIM_ATTR_PERM_ADDR]) != ETH_ALEN) {
+ pr_debug("mac80211_hwsim: MAC has wrong size\n");
+ return -EINVAL;
+ }
+ if (!is_valid_ether_addr(
+ nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
+ pr_debug("mac80211_hwsim: MAC is no valid source addr\n");
+ return -EINVAL;
+ }
+ if (!is_local_ether_addr(
+ nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
+ pr_debug("mac80211_hwsim: MAC is not locally administered\n");
+ return -EINVAL;
+ }
+ if (get_hwsim_data_ref_from_addr(
+ nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
+ pr_debug("mac80211_hwsim: perm MAC already in use\n");
+ return -EINVAL;
+ }
+
+ param.perm_addr = nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]);
+ }
+
ret = mac80211_hwsim_new_radio(info, ¶m);
kfree(hwname);
return ret;
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index 3f5eda591dba..e2592b596090 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -67,7 +67,12 @@ enum hwsim_tx_control_flags {
* %HWSIM_ATTR_TX_INFO, %HWSIM_ATTR_SIGNAL, %HWSIM_ATTR_COOKIE
* @HWSIM_CMD_NEW_RADIO: create a new radio with the given parameters,
* returns the radio ID (>= 0) or negative on errors, if successful
- * then multicast the result
+ * then multicast the result, uses optional parameter:
+ * %HWSIM_ATTR_REG_STRICT_REG, %HWSIM_ATTR_SUPPORT_P2P_DEVICE,
+ * %HWSIM_ATTR_DESTROY_RADIO_ON_CLOSE, %HWSIM_ATTR_CHANNELS,
+ * %HWSIM_ATTR_NO_VIF, %HWSIM_ATTR_RADIO_NAME, %HWSIM_ATTR_USE_CHANCTX,
+ * %HWSIM_ATTR_REG_HINT_ALPHA2, %HWSIM_ATTR_REG_CUSTOM_REG,
+ * %HWSIM_ATTR_PERM_ADDR
* @HWSIM_CMD_DEL_RADIO: destroy a radio, reply is multicasted
* @HWSIM_CMD_GET_RADIO: fetch information about existing radios, uses:
* %HWSIM_ATTR_RADIO_ID
@@ -123,6 +128,7 @@ enum {
* @HWSIM_ATTR_RADIO_NAME: Name of radio, e.g. phy666
* @HWSIM_ATTR_NO_VIF: Do not create vif (wlanX) when creating radio.
* @HWSIM_ATTR_FREQ: Frequency at which packet is transmitted or received.
+ * @HWSIM_ATTR_PERM_ADDR: permanent mac address of new radio
* @__HWSIM_ATTR_MAX: enum limit
*/
@@ -149,6 +155,7 @@ enum {
HWSIM_ATTR_NO_VIF,
HWSIM_ATTR_FREQ,
HWSIM_ATTR_PAD,
+ HWSIM_ATTR_PERM_ADDR,
__HWSIM_ATTR_MAX,
};
#define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1)
--
2.15.1
Am 22.01.2018 um 14:23 schrieb Johannes Berg:
> On Mon, 2018-01-15 at 15:48 +0100, Benjamin Beichler wrote:
>
>>>> + if (get=5Fhwsim=5Fdata=5Fref=5Ffrom=5Faddr(
>>>> + nla=5Fdata(info->attrs[HWSIM=5FATTR=5FPERM=5FADDR]))) {
>>>> + pr=5Fdebug("mac80211=5Fhwsim: perm MAC already in use\n");
>>>> + return -EINVAL;
>>>> + }
>>> This is racy afaict - remove it and return a clash later when you fail
>>> to insert the new radio.
>> Ehm yes, actually exactly this test is already in the rhashtable patch.
>> But maybe I should also change there the error print to a NL=5FERR=5FMSG() =
=3F
> Oh, really=3F I just sent that out, but whatever, we have enough time to
> fix it :-)
Should I sent then the complete patch set again, or should I create a
new one based on your current mac80211next branch, excluding the
accepted patches =3F
> And yes, using netlink extended ack messages here as well would be
> good.
>
> johannes
>
Benjamin
--
M.Sc. Benjamin Beichler
Universit=C3=A4t Rostock, Fakult=C3=A4t f=C3=BCr Informatik und Elektrotechnik
Institut f=C3=BCr Angewandte Mikroelektronik und Datentechnik
University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE
Richard-Wagner-Stra=C3=9Fe 31
18119 Rostock
Deutschland/Germany
phone: +49 (0) 381 498 - 7278
email: [email protected]
www: http://www.imd.uni-rostock.de/
On Mon, 2018-01-22 at 15:49 +0100, Benjamin Beichler wrote:
>
> Should I sent then the complete patch set again, or should I create a
> new one based on your current mac80211next branch, excluding the
> accepted patches ?
Please only send the ones I don't have yet, if necessary throwing in a
separate fix for them.
johannes
On Mon, 2018-01-15 at 15:39 +0100, Benjamin Beichler wrote:
>
>
> I really don't know, why there were 2 addresses introduced in the first
> place, and why the netlink operations for receiving only check against
> the second address and not the first address (see
> get_hwsim_data_ref_from_addr). My suggestion would be, only allocate one
> mac address and check against this one. I don't know, whether really any
> user of hwsim rely on the structure, that an adapter needs two mac
> addresses. Since this is part of the initial commit, this decision is
> not documented in git ...
Fair enough, let's leave it then.
johannes
On Mon, 2018-01-15 at 15:48 +0100, Benjamin Beichler wrote:
> > > + if (get_hwsim_data_ref_from_addr(
> > > + nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
> > > + pr_debug("mac80211_hwsim: perm MAC already in use\n");
> > > + return -EINVAL;
> > > + }
> >
> > This is racy afaict - remove it and return a clash later when you fail
> > to insert the new radio.
>
> Ehm yes, actually exactly this test is already in the rhashtable patch.
> But maybe I should also change there the error print to a NL_ERR_MSG() ?
Oh, really? I just sent that out, but whatever, we have enough time to
fix it :-)
And yes, using netlink extended ack messages here as well would be
good.
johannes
On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
> To do not break the operation with legacy software using hwsim, the new
> address is set twice. The problem here is, the netlink call backs use
> wiphy->addresses[1] as identification of a radio and not the proposed
> permanent address (wiphy->addresses[0]). This design decision is not
> documented in the kernel repo, therefore this patch simply reproduces this,
> but with the same address.
It's weird to have the same address twice - perhaps we can XOR 0x40 or
so?
johannes
On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
>
>
> + if (info->attrs[HWSIM_ATTR_PERM_ADDR]) {
> + if (nla_len(info->attrs[HWSIM_ATTR_PERM_ADDR]) != ETH_ALEN) {
> + pr_debug("mac80211_hwsim: MAC has wrong size\n");
> + return -EINVAL;
> + }
Oh, also - don't do this.
1) don't print, use NL_SET_ERR_MSG().
2) use the policy to validate lengths
> + if (!is_local_ether_addr(
> + nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
> + pr_debug("mac80211_hwsim: MAC is not locally administered\n");
> + return -EINVAL;
> + }
This doesn't really matter - it's purely virtual. I think we can avoid
that.
> + if (get_hwsim_data_ref_from_addr(
> + nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
> + pr_debug("mac80211_hwsim: perm MAC already in use\n");
> + return -EINVAL;
> + }
This is racy afaict - remove it and return a clash later when you fail
to insert the new radio.
johannes
Am 15.01.2018 um 13:14 schrieb Johannes Berg:
> On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
>> =20
>> + if (info->attrs[HWSIM_ATTR_PERM_ADDR]) {
>> + if (nla_len(info->attrs[HWSIM_ATTR_PERM_ADDR]) !=3D ETH_ALEN) {
>> + pr_debug("mac80211_hwsim: MAC has wrong size\n");
>> + return -EINVAL;
>> + }
> Oh, also - don't do this.
>
> 1) don't print, use NL_SET_ERR_MSG().
Ok, I only adapted the code already there, but I change this.
>
> 2) use the policy to validate lengths
Yeah, of course I change that, much easier -.-
>
>> + if (!is_local_ether_addr(
>> + nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
>> + pr_debug("mac80211_hwsim: MAC is not locally administered\n");
>> + return -EINVAL;
>> + }
> This doesn't really matter - it's purely virtual. I think we can avoid
> that.
>
>> + if (get_hwsim_data_ref_from_addr(
>> + nla_data(info->attrs[HWSIM_ATTR_PERM_ADDR]))) {
>> + pr_debug("mac80211_hwsim: perm MAC already in use\n");
>> + return -EINVAL;
>> + }
> This is racy afaict - remove it and return a clash later when you fail
> to insert the new radio.
Ehm yes, actually exactly this test is already in the rhashtable patch.
But maybe I should also change there the error print to a NL_ERR_MSG() ?
>
> johannes
>
--=20
M.Sc. Benjamin Beichler
Universit=C3=A4t Rostock, Fakult=C3=A4t f=C3=BCr Informatik und Elektrote=
chnik
Institut f=C3=BCr Angewandte Mikroelektronik und Datentechnik
University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE
Richard-Wagner-Stra=C3=9Fe 31
18119 Rostock
Deutschland/Germany
phone: +49 (0) 381 498 - 7278
email: [email protected]
www: http://www.imd.uni-rostock.de/
Am 15.01.2018 um 13:09 schrieb Johannes Berg:
> On Wed, 2018-01-10 at 17:42 +0100, Benjamin Beichler wrote:
>
>> To do not break the operation with legacy software using hwsim, the ne=
w
>> address is set twice. The problem here is, the netlink call backs use
>> wiphy->addresses[1] as identification of a radio and not the proposed
>> permanent address (wiphy->addresses[0]). This design decision is not
>> documented in the kernel repo, therefore this patch simply reproduces =
this,
>> but with the same address.
> It's weird to have the same address twice - perhaps we can XOR 0x40 or
> so?
I really don't know, why there were 2 addresses introduced in the first
place, and why the netlink operations for receiving only check against
the second address and not the first address (see
get_hwsim_data_ref_from_addr). My suggestion would be, only allocate one
mac address and check against this one. I don't know, whether really any
user of hwsim rely on the structure, that an adapter needs two mac
addresses. Since this is part of the initial commit, this decision is
not documented in git ...
> johannes
>
>
--=20
M.Sc. Benjamin Beichler
Universit=C3=A4t Rostock, Fakult=C3=A4t f=C3=BCr Informatik und Elektrote=
chnik
Institut f=C3=BCr Angewandte Mikroelektronik und Datentechnik
University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE
Richard-Wagner-Stra=C3=9Fe 31
18119 Rostock
Deutschland/Germany
phone: +49 (0) 381 498 - 7278
email: [email protected]
www: http://www.imd.uni-rostock.de/