[RFC 2/4] mac80211_hwsim: add hwsim_tx_rate_flags to Netlink Attributes

[RFC 2/4] mac80211_hwsim: add hwsim_tx_rate_flags to Netlink Attributes

[Benjamin Beichler]

For correct interpretation of a tx rate, the corresponding rate flags are needed (e.g. whether a HT-MCS rate or a legacy rate) and
moreover for more correct simulation the other infos of the flags are important (like short-GI). Keeping compability, the flags
are not integrated into the existing hwsim_tx_rate, but transmitted as an additional netlink attribute.

Signed-off-by: Benjamin Beichler <benjamin.beichler@uni-rostock.de>
---
 drivers/net/wireless/mac80211_hwsim.c | 93 ++++++++++++++++++++++++++++++++++-
 drivers/net/wireless/mac80211_hwsim.h | 67 +++++++++++++++++++++++++
 2 files changed, 159 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index aeeea7a35404..62c5a00a76c3 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -1014,6 +1014,66 @@ static int hwsim_unicast_netgroup(struct mac80211_hwsim_data *data,
 	return res;
 }
 
+static inline u16 transl_tx_rate_flags_ieee2hwsim(struct ieee80211_tx_rate *rate)
+{
+	u16 result=0;
+	if(rate->flags & IEEE80211_TX_RC_USE_RTS_CTS)
+			result |= MAC80211_HWSIM_TX_RC_USE_RTS_CTS;
+	if(rate->flags & IEEE80211_TX_RC_USE_CTS_PROTECT)
+			result |= MAC80211_HWSIM_TX_RC_USE_CTS_PROTECT;
+	if(rate->flags & IEEE80211_TX_RC_USE_SHORT_PREAMBLE)
+			result |= MAC80211_HWSIM_TX_RC_USE_SHORT_PREAMBLE;
+	if(rate->flags & IEEE80211_TX_RC_MCS)
+			result |= MAC80211_HWSIM_TX_RC_MCS;
+	if(rate->flags & IEEE80211_TX_RC_GREEN_FIELD)
+			result |= MAC80211_HWSIM_TX_RC_GREEN_FIELD;
+	if(rate->flags & IEEE80211_TX_RC_40_MHZ_WIDTH)
+			result |= MAC80211_HWSIM_TX_RC_40_MHZ_WIDTH;
+	if(rate->flags & IEEE80211_TX_RC_DUP_DATA)
+			result |= MAC80211_HWSIM_TX_RC_DUP_DATA;
+	if(rate->flags & IEEE80211_TX_RC_SHORT_GI)
+			result |= MAC80211_HWSIM_TX_RC_SHORT_GI;
+	if(rate->flags & IEEE80211_TX_RC_VHT_MCS)
+			result |= MAC80211_HWSIM_TX_RC_VHT_MCS;
+	if(rate->flags & IEEE80211_TX_RC_80_MHZ_WIDTH)
+			result |= MAC80211_HWSIM_TX_RC_80_MHZ_WIDTH;
+	if(rate->flags & IEEE80211_TX_RC_160_MHZ_WIDTH)
+			result |= MAC80211_HWSIM_TX_RC_160_MHZ_WIDTH;
+
+	return result;
+}
+
+static inline u16 transl_rate_flags_hwsim2ieee(struct hwsim_tx_rate_flag *rate)
+{
+	u16 result=0;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_USE_RTS_CTS)
+			result |= IEEE80211_TX_RC_USE_RTS_CTS;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_USE_CTS_PROTECT)
+			result |= IEEE80211_TX_RC_USE_CTS_PROTECT;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_USE_SHORT_PREAMBLE)
+			result |= IEEE80211_TX_RC_USE_SHORT_PREAMBLE;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_MCS)
+			result |= IEEE80211_TX_RC_MCS;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_GREEN_FIELD)
+			result |= IEEE80211_TX_RC_GREEN_FIELD;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_40_MHZ_WIDTH)
+			result |= IEEE80211_TX_RC_40_MHZ_WIDTH;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_DUP_DATA)
+			result |= IEEE80211_TX_RC_DUP_DATA;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_SHORT_GI)
+			result |= IEEE80211_TX_RC_SHORT_GI;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_VHT_MCS)
+			result |= IEEE80211_TX_RC_VHT_MCS;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_80_MHZ_WIDTH)
+			result |= IEEE80211_TX_RC_80_MHZ_WIDTH;
+	if(rate->flags & MAC80211_HWSIM_TX_RC_160_MHZ_WIDTH)
+			result |= IEEE80211_TX_RC_160_MHZ_WIDTH;
+
+	return result;
+}
+
+
+
 static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
 				       struct sk_buff *my_skb,
 				       int dst_portid)
@@ -1026,6 +1086,8 @@ static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
 	unsigned int hwsim_flags = 0;
 	int i;
 	struct hwsim_tx_rate tx_attempts[IEEE80211_TX_MAX_RATES];
+	struct hwsim_tx_rate_flag tx_attempts_flags[IEEE80211_TX_MAX_RATES];
+
 	uintptr_t cookie;
 
 	if (data->ps != PS_DISABLED)
@@ -1077,7 +1139,10 @@ static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
 
 	for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
 		tx_attempts[i].idx = info->status.rates[i].idx;
+		tx_attempts_flags[i].idx= info->status.rates[i].idx;
 		tx_attempts[i].count = info->status.rates[i].count;
+		tx_attempts_flags[i].flags =
+				transl_tx_rate_flags_ieee2hwsim(&info->status.rates[i]);
 	}
 
 	if (nla_put(skb, HWSIM_ATTR_TX_INFO,
@@ -1085,6 +1150,11 @@ static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw,
 		    tx_attempts))
 		goto nla_put_failure;
 
+	if (nla_put(skb, HWSIM_ATTR_TX_INFO_FLAGS,
+		    sizeof(struct hwsim_tx_rate_flag)*IEEE80211_TX_MAX_RATES,
+		    tx_attempts_flags))
+		goto nla_put_failure;
+
 	/* We create a cookie to identify this skb */
 	data->pending_cookie++;
 	cookie = data->pending_cookie;
@@ -2973,6 +3043,8 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2,
 	struct mac80211_hwsim_data *data2;
 	struct ieee80211_tx_info *txi;
 	struct hwsim_tx_rate *tx_attempts;
+	struct hwsim_tx_rate_flag *tx_attempts_flags;
+
 	u64 ret_skb_cookie;
 	struct sk_buff *skb, *tmp;
 	const u8 *src;
@@ -3033,9 +3105,28 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2,
 	for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
 		txi->status.rates[i].idx = tx_attempts[i].idx;
 		txi->status.rates[i].count = tx_attempts[i].count;
-		/*txi->status.rates[i].flags = 0;*/
+		/* txi->status.rates[i].flags = 0; */
 	}
 
+	if(info->attrs[HWSIM_ATTR_TX_INFO_FLAGS]){
+			tx_attempts_flags = (struct hwsim_tx_rate_flag *)nla_data(
+					       info->attrs[HWSIM_ATTR_TX_INFO_FLAGS]);
+			for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
+
+				WARN(txi->status.rates[i].idx != tx_attempts_flags[i].idx,
+						"rate idx of tx_info received via netlink "
+							"does not match to rate idx of tx_info_flags");
+
+				txi->status.rates[i].flags =
+						transl_rate_flags_hwsim2ieee(&tx_attempts_flags[i]);
+						}
+		}
+		else{
+			WARN_ONCE(1,"received tx_info via netlink does not contain"
+				"rate flags, may cause statistic or rate control problems");
+		}
+
+
 	txi->status.ack_signal = nla_get_u32(info->attrs[HWSIM_ATTR_SIGNAL]);
 
 	if (!(hwsim_flags & HWSIM_TX_CTL_NO_ACK) &&
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index 3f5eda591dba..5f831fffcb13 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -123,6 +123,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_TX_INFO_FLAGS: additional flags for corresponding rates of %HWSIM_ATTR_TX_INFO
  * @__HWSIM_ATTR_MAX: enum limit
  */
 
@@ -149,10 +150,76 @@ enum {
 	HWSIM_ATTR_NO_VIF,
 	HWSIM_ATTR_FREQ,
 	HWSIM_ATTR_PAD,
+	HWSIM_ATTR_TX_INFO_FLAGS,
 	__HWSIM_ATTR_MAX,
 };
 #define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1)
 
+
+/**
+ * enum hwsim_tx_rate_flags - per-rate flags set by the
+ *	Rate Control algorithm. Inspired by structure mac80211_rate_control_flags.
+ *	New flags may be appended, but old flags not deleted, to keep compatibility
+ *	for userspace.
+ *
+ * These flags are set by the Rate control algorithm for each rate during tx,
+ * in the @flags member of struct ieee80211_tx_rate.
+ *
+ * @MAC80211_HWSIM_TX_RC_USE_RTS_CTS: Use RTS/CTS exchange for this rate.
+ * @MAC80211_HWSIM_TX_RC_USE_CTS_PROTECT: CTS-to-self protection is required.
+ *	This is set if the current BSS requires ERP protection.
+ * @MAC80211_HWSIM_TX_RC_USE_SHORT_PREAMBLE: Use short preamble.
+ * @MAC80211_HWSIM_TX_RC_MCS: HT rate.
+ * @MAC80211_HWSIM_TX_RC_VHT_MCS: VHT MCS rate, in this case the idx field is split
+ *	into a higher 4 bits (Nss) and lower 4 bits (MCS number)
+ * @MAC80211_HWSIM_TX_RC_GREEN_FIELD: Indicates whether this rate should be used in
+ *	Greenfield mode.
+ * @MAC80211_HWSIM_TX_RC_40_MHZ_WIDTH: Indicates if the Channel Width should be 40 MHz.
+ * @MAC80211_HWSIM_TX_RC_80_MHZ_WIDTH: Indicates 80 MHz transmission
+ * @MAC80211_HWSIM_TX_RC_160_MHZ_WIDTH: Indicates 160 MHz transmission
+ *	(80+80 isn't supported yet)
+ * @MAC80211_HWSIM_TX_RC_DUP_DATA: The frame should be transmitted on both of the
+ *	adjacent 20 MHz channels, if the current channel type is
+ *	NL80211_CHAN_HT40MINUS or NL80211_CHAN_HT40PLUS.
+ * @MAC80211_HWSIM_TX_RC_SHORT_GI: Short Guard interval should be used for this rate.
+ */
+enum hwsim_tx_rate_flags {
+	MAC80211_HWSIM_TX_RC_USE_RTS_CTS		= BIT(0),
+	MAC80211_HWSIM_TX_RC_USE_CTS_PROTECT		= BIT(1),
+	MAC80211_HWSIM_TX_RC_USE_SHORT_PREAMBLE	= BIT(2),
+
+	/* rate index is an HT/VHT MCS instead of an index */
+	MAC80211_HWSIM_TX_RC_MCS			= BIT(3),
+	MAC80211_HWSIM_TX_RC_GREEN_FIELD		= BIT(4),
+	MAC80211_HWSIM_TX_RC_40_MHZ_WIDTH		= BIT(5),
+	MAC80211_HWSIM_TX_RC_DUP_DATA		= BIT(6),
+	MAC80211_HWSIM_TX_RC_SHORT_GI		= BIT(7),
+	MAC80211_HWSIM_TX_RC_VHT_MCS			= BIT(8),
+	MAC80211_HWSIM_TX_RC_80_MHZ_WIDTH		= BIT(9),
+	MAC80211_HWSIM_TX_RC_160_MHZ_WIDTH		= BIT(10),
+};
+
+/**
+ * struct hwsim_tx_rate - rate selection/status
+ *
+ * @idx: rate index to attempt to send with
+ * @count: number of tries in this rate before going to the next rate
+ *
+ * A value of -1 for @idx indicates an invalid rate and, if used
+ * in an array of retry rates, that no more rates should be tried.
+ *
+ * When used for transmit status reporting, the driver should
+ * always report the rate and number of retries used.
+ *
+ */
+struct hwsim_tx_rate_flag {
+	s8 idx;
+	u16 flags;
+} __packed;
+
+
+
+
 /**
  * struct hwsim_tx_rate - rate selection/status
  *
-- 
2.14.1

Re: [RFC 2/4] mac80211_hwsim: add hwsim_tx_rate_flags to Netlink Attributes

[Johannes Berg]

On Fri, 2017-09-08 at 16:11 +0200, Benjamin Beichler wrote:
> For correct interpretation of a tx rate, the corresponding rate flags
> are needed (e.g. whether a HT-MCS rate or a legacy rate) and
> moreover for more correct simulation the other infos of the flags are
> important (like short-GI). Keeping compability, the flags
> are not integrated into the existing hwsim_tx_rate, but transmitted
> as an additional netlink attribute.

This still exposes a lot of internal detail - perhaps it'd be better to
convert to some sort of stable rate representation, like in cfg80211?
Not sure that's sufficient though, it's a bit less detailed.

johannes

Re: [RFC 2/4] mac80211_hwsim: add hwsim_tx_rate_flags to Netlink Attributes

[Benjamin Beichler]

[-- Attachment #1: Type: text/plain, Size: 2391 bytes --]

Am 08.09.2017 um 16:26 schrieb Johannes Berg:
> On Fri, 2017-09-08 at 16:11 +0200, Benjamin Beichler wrote:
>> For correct interpretation of a tx rate, the corresponding rate flags
>> are needed (e.g. whether a HT-MCS rate or a legacy rate) and
>> moreover for more correct simulation the other infos of the flags are
>> important (like short-GI). Keeping compability, the flags
>> are not integrated into the existing hwsim_tx_rate, but transmitted
>> as an additional netlink attribute.
> This still exposes a lot of internal detail - perhaps it'd be better to
> convert to some sort of stable rate representation, like in cfg80211?
> Not sure that's sufficient though, it's a bit less detailed.
I don't know what is the problem with the details. The only flag, which
is a bit to verbose is  MAC80211_HWSIM_TX_RC_DUP_DATA, which we may
omit. All others describe directly terms used in the IEEE 802.11
standard. Also the representation, that a rate is an MCS-index is quite
good. If you take look here http://mcsindex.com/ , the bitrate would be
not sufficient to get the exact coding and fec rate, therefore you would
also need additional flags. You are right regarding legacy rates, which
are in an encoded table. I tried to decouple internal and external API,
but currently there is no big difference.

Nonetheless the whole hwsim API is highly specialized and only usable
with the linux kernel. Of course the Userland API should be more or less
stable, but the backward compatibility is not touched by this change. As
I already said, this is nearly a fix for hwsim, since currently it's
impossible to differentiate between legacy and MCS-rates, although they
could appear in a single tx_rates array. I think currently minstrel does
not mix HT and legacy rates for data frames, but AFAIK Management/Action
frames are always sent with legacy rates, so there are mixed already.


> johannes

kind regards

Benjamin

-- 
M.Sc. Benjamin Beichler

Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: Benjamin.Beichler@uni-rostock.de
www: http://www.imd.uni-rostock.de/



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5151 bytes --]

Re: [RFC 2/4] mac80211_hwsim: add hwsim_tx_rate_flags to Netlink Attributes

[Johannes Berg]

On Mon, 2017-09-11 at 11:49 +0200, Benjamin Beichler wrote:

> I don't know what is the problem with the details. The only flag,
> which is a bit to verbose is  MAC80211_HWSIM_TX_RC_DUP_DATA, which we
> may omit. All others describe directly terms used in the IEEE 802.11
> standard. Also the representation, that a rate is an MCS-index is
> quite good. If you take look here http://mcsindex.com/ , the bitrate
> would be not sufficient to get the exact coding and fec rate,
> therefore you would also need additional flags. You are right
> regarding legacy rates, which are in an encoded table. I tried to
> decouple internal and external API, but currently there is no big
> difference.

Yeah, I was just concerned that maybe this API was too tightly coupled
to mac80211, but I guess it should be fine.

> Nonetheless the whole hwsim API is highly specialized and only usable
> with the linux kernel. Of course the Userland API should be more or
> less stable, but the backward compatibility is not touched by this
> change. As I already said, this is nearly a fix for hwsim, since
> currently it's impossible to differentiate between legacy and MCS-
> rates, although they could appear in a single tx_rates array. I think
> currently minstrel does not mix HT and legacy rates for data frames,
> but AFAIK Management/Action frames are always sent with legacy rates,
> so there are mixed already.

Ok.

johannes

[RFC 3/4] mac80211_hwsim: explicitly set netlink parallel ops to false

[Benjamin Beichler]

The ops field is zero initialized, therefore parallel ops is already false. This implicates that the netlink callbacks are not
processed in parallel. Maybe this could be utilized to reduce locking overhead or maybe also parallel ops could be implemented.

Signed-off-by: Benjamin Beichler <benjamin.beichler@uni-rostock.de>
---
 drivers/net/wireless/mac80211_hwsim.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 62c5a00a76c3..5dd4be2a8487 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3499,6 +3499,7 @@ static struct genl_family hwsim_genl_family __ro_after_init = {
 	.n_ops = ARRAY_SIZE(hwsim_ops),
 	.mcgrps = hwsim_mcgrps,
 	.n_mcgrps = ARRAY_SIZE(hwsim_mcgrps),
+	.parallel_ops = false
 };
 
 static void destroy_radio(struct work_struct *work)
-- 
2.14.1

Re: [RFC 3/4] mac80211_hwsim: explicitly set netlink parallel ops to false

[Johannes Berg]

On Fri, 2017-09-08 at 16:11 +0200, Benjamin Beichler wrote:
> The ops field is zero initialized, therefore parallel ops is already
> false.

Therefore this patch is completely pointless?

johannes

Re: [RFC 3/4] mac80211_hwsim: explicitly set netlink parallel ops to false

[Benjamin Beichler]

Am 8=2E September 2017 16:19:20 MESZ schrieb Johannes Berg <johannes@sipso=
lutions=2Enet>:
>On Fri, 2017-09-08 at 16:11 +0200, Benjamin Beichler wrote:
>> The ops field is zero initialized, therefore parallel ops is already
>> false=2E
>
>Therefore this patch is completely pointless?
Sorry my first message was missing regarding this=2E My question is, wheth=
er this is intentionally, and if it is parallel, whether we need extensive =
locking here=2E

>
>johannes

--=20
M=2ESc=2E Benjamin Beichler

Universit=C3=A4t Rostock, Fakult=C3=A4t f=C3=BCr Informatik und Elektrotec=
hnik
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: Benjamin=2EBeichler@uni-rostock=2Ede
www: http://www=2Eimd=2Euni-rostock=2Ede/

Re: [RFC 3/4] mac80211_hwsim: explicitly set netlink parallel ops to false

[Johannes Berg]

On Fri, 2017-09-08 at 17:07 +0200, Benjamin Beichler wrote:
> 
> Am 8. September 2017 16:19:20 MESZ schrieb Johannes Berg <johannes@si
> psolutions.net>:
> > On Fri, 2017-09-08 at 16:11 +0200, Benjamin Beichler wrote:
> > > The ops field is zero initialized, therefore parallel ops is
> > > already
> > > false.
> > 
> > Therefore this patch is completely pointless?
> 
> Sorry my first message was missing regarding this. My question is,
> whether this is intentionally, and if it is parallel, whether we need
> extensive locking here.

It's basically intentional - not sure parallel_ops even existed when
this was first written, but we can probably use parallel_ops if we want
to.

johannes

[RFC 4/4] mac80211_hwsim: add radio idx param to netlink callback of radio creation

[Benjamin Beichler]

Since the radio index is already a valid NL attribute, this patch simply
try to read it and create the radio specific to this index by the
previously used scheme.

Since this allows to create radios out of row, we need to search for a
free index for a new radio, when the index is not present as parameter.

Signed-off-by: Benjamin Beichler <benjamin.beichler@uni-rostock.de>
---
 drivers/net/wireless/mac80211_hwsim.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 5dd4be2a8487..faa0e39523aa 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2436,6 +2436,7 @@ struct hwsim_new_radio_params {
 	bool destroy_on_close;
 	const char *hwname;
 	bool no_vif;
+	int idx;
 };
 
 static void hwsim_mcast_config_msg(struct sk_buff *mcast_skb,
@@ -2645,13 +2646,9 @@ 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;
+	memcpy(data->addresses[0].addr, &mac.addr, ETH_ALEN);
+	memcpy(data->addresses[1].addr, &mac.addr, ETH_ALEN);
+	data->addresses[0].addr[0] = 0x02;
 	hw->wiphy->n_addresses = 2;
 	hw->wiphy->addresses = data->addresses;
 
@@ -3304,6 +3301,11 @@ 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_RADIO_ID])
+		param.idx = nla_get_u32(info->attrs[HWSIM_ATTR_RADIO_ID]);
+	else
+		param.idx = -1;
+
 	return mac80211_hwsim_new_radio(info, &param);
 }
 
-- 
2.14.1

Re: [RFC 4/4] mac80211_hwsim: add radio idx param to netlink callback of radio creation

[Johannes Berg]

On Fri, 2017-09-08 at 16:11 +0200, Benjamin Beichler wrote:
> Since the radio index is already a valid NL attribute, this patch
> simply
> try to read it and create the radio specific to this index by the
> previously used scheme.
> 
> Since this allows to create radios out of row, we need to search for
> a
> free index for a new radio, when the index is not present as
> parameter.

This seems like a bad idea, what's the point? You can always change the
MAC address later if you want.

Also, you don't seem to actually do what you said, with the -1.

johannes

Re: [RFC 4/4] mac80211_hwsim: add radio idx param to netlink callback of radio creation

[Benjamin Beichler]

[-- Attachment #1: Type: text/plain, Size: 1414 bytes --]


>> Since the radio index is already a valid NL attribute, this patch >> simply try to read it and create the radio specific to this index >>
by the previously used scheme. >> >> Since this allows to create radios
out of row, we need to search >> for a free index for a new radio, when
the index is not present as >> parameter. > > This seems like a bad
idea, what's the point? You can always change > the MAC address later if
you want. AFAIK, you can change the MAC-Address of an interface on the
phy, but not the perm address of the phy. But for frames sent to a
wmediumd always the perm address is used to identify the sender and
receiver. It is really annoying to additionally keep a translation table
in userspace to manage between random ids/mac-addresses to them you use
in a simulation for this.


> > > Also, you don't seem to actually do what you said, with the -1.
OK, I see this is badly mixed up with patch 1, which introduces the
hashlist stuff.

-- 
M.Sc. Benjamin Beichler

Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: Benjamin.Beichler@uni-rostock.de
www: http://www.imd.uni-rostock.de/



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5151 bytes --]

Re: [RFC 4/4] mac80211_hwsim: add radio idx param to netlink callback of radio creation

[Johannes Berg]

Uh. Is there anything you can do not to mangle your replies? I don't
even know what you're replying to etc.

> AFAIK, you can change the MAC-Address of an interface on
> the phy, but not the perm address of the phy. But for frames sent to
> a wmediumd always the perm address is used to identify the sender and
> receiver. It is really annoying to additionally keep a translation
> table in userspace to manage between random ids/mac-addresses to them
> you use in a simulation for this.

I think we tried to fix that once, but that caused more problems ...

I'd rather allow you to then specify the MAC address though - the
index/address mapping doesn't need to be the identity, after all.

johannes