[PATCH 1/2] mac80211_hwsim: Make sure NEW_RADIO contains final name

[PATCH 1/2] mac80211_hwsim: Make sure NEW_RADIO contains final name

[Andrew Zaborowski]

ieee80211_alloc_hw_nm will validate the requested name (if any) before
creating the new device and may use a name different from the one
requested rather than fail.  Make sure the HWSIM_CMD_NEW_RADIO
event/response generated has the final name or userspace will receive
the wrong name.  Note that mac80211_hwsim_new_radio may now modify
params.

A check for duplicate radio name could be added separately.

Signed-off-by: Andrew Zaborowski <andrew.zaborowski@intel.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 0150747..ba4978d 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2438,6 +2438,9 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
 		goto failed;
 	}
 
+	/* ieee80211_alloc_hw_nm may have used a default name */
+	param->hwname = wiphy_name(hw->wiphy);
+
 	if (info)
 		net = genl_info_net(info);
 	else
-- 
2.9.3

[PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Andrew Zaborowski]

Add a HWSIM_ATTR_RADIO_ADDR attribute to those to events/response so
that a userspace medium can query the list of addresses in the
simulator.

When a userspace medium needs to handle a broadcast frame it can't
easily implement the equivalent of what the default kernel medium does
because it doesn't know who to forward the frame to.  The
HWSIM_CMD_GET_RADIO command is a little useless in this respect.
Userspace needs to use HWSIM_CMD_GET_RADIO, then cross-reference the
radio names with wiphy names using NL80211_CMD_GET_WIPHY dump, map the
wiphy names to wiphy indexes (those are more useful than wiphy names
because they don't change -- a wiphy name change doesn't generate an
event so this can't be easily tracked), and use the
NL80211_CMD_GET_INTERFACES dump to obtain the wiphy_idx to mac address
mapping.

Additionally there are two addresses used by hwsim radios
(data->addresses[0] and [1]), first used at the network level and the
second used at hwsim radio level and in HWSIM_ATTR_ADDR_TRANSMITTER /
_RECEIVER.  By default they differ by 0x40 in the first byte but I'm not
sure userspace can make this assumption to do the mapping from the
address inside the frame headers to the address of the hwsim receiving
radio.  I suspect the network layers can change address 0 and they will
be out of sync.

Signed-off-by: Andrew Zaborowski <andrew.zaborowski@intel.com>
---
Additionally I tried to add a HWSIM_ATTR_WIPHY to report the wiphy index
directly without users going through wiphy name to index mapping, but
get_wiphy_idx() is internal to cfg80211.  The index is exposed to
userspace and is more useful than the name so I wonder if this function
should be exported from cfg80211.
---
 drivers/net/wireless/mac80211_hwsim.c | 16 ++++++++++++++++
 drivers/net/wireless/mac80211_hwsim.h |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index ba4978d..dcc1df0 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2305,6 +2305,7 @@ struct hwsim_new_radio_params {
 	bool destroy_on_close;
 	const char *hwname;
 	bool no_vif;
+	const struct mac_address *addrs;
 };
 
 static void hwsim_mcast_config_msg(struct sk_buff *mcast_skb,
@@ -2318,10 +2319,15 @@ static void hwsim_mcast_config_msg(struct sk_buff *mcast_skb,
 				  HWSIM_MCGRP_CONFIG, GFP_KERNEL);
 }
 
+#define HWSIM_ADDR_COUNT \
+	ARRAY_SIZE(((struct mac80211_hwsim_data *) NULL)->addresses)
+
 static int append_radio_msg(struct sk_buff *skb, int id,
 			    struct hwsim_new_radio_params *param)
 {
 	int ret;
+	int i;
+	uint8_t addr_buf[HWSIM_ADDR_COUNT * ETH_ALEN];
 
 	ret = nla_put_u32(skb, HWSIM_ATTR_RADIO_ID, id);
 	if (ret < 0)
@@ -2379,6 +2385,13 @@ static int append_radio_msg(struct sk_buff *skb, int id,
 			return ret;
 	}
 
+	for (i = 0; i < HWSIM_ADDR_COUNT; i++)
+		memcpy(addr_buf + i * ETH_ALEN, param->addrs[i].addr, ETH_ALEN);
+
+	ret = nla_put(skb, HWSIM_ATTR_RADIO_ADDR, sizeof(addr_buf), addr_buf);
+	if (ret < 0)
+		return ret;
+
 	return 0;
 }
 
@@ -2682,6 +2695,8 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
 	list_add_tail(&data->list, &hwsim_radios);
 	spin_unlock_bh(&hwsim_radio_lock);
 
+	param->addrs = data->addresses;
+
 	if (idx > 0)
 		hwsim_mcast_new_radio(idx, info, param);
 
@@ -2772,6 +2787,7 @@ static int mac80211_hwsim_get_radio(struct sk_buff *skb,
 	param.regd = data->regd;
 	param.channels = data->channels;
 	param.hwname = wiphy_name(data->hw->wiphy);
+	param.addrs = data->addresses;
 
 	res = append_radio_msg(skb, data->idx, &param);
 	if (res < 0)
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index 39f2246..53b4cd7 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_RADIO_ADDR: Radio hardware addresses as an array
  * @__HWSIM_ATTR_MAX: enum limit
  */
 
@@ -149,6 +150,7 @@ enum {
 	HWSIM_ATTR_NO_VIF,
 	HWSIM_ATTR_FREQ,
 	HWSIM_ATTR_PAD,
+	HWSIM_ATTR_RADIO_ADDR,
 	__HWSIM_ATTR_MAX,
 };
 #define HWSIM_ATTR_MAX (__HWSIM_ATTR_MAX - 1)
-- 
2.9.3

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Benjamin Beichler]

[-- Attachment #1.1: Type: text/plain, Size: 3724 bytes --]

> Add a HWSIM_ATTR_RADIO_ADDR attribute to those to events/response so
> that a userspace medium can query the list of addresses in the
> simulator.
>
> When a userspace medium needs to handle a broadcast frame it can't
> easily implement the equivalent of what the default kernel medium does
> because it doesn't know who to forward the frame to.  The
> HWSIM_CMD_GET_RADIO command is a little useless in this respect.
> Userspace needs to use HWSIM_CMD_GET_RADIO, then cross-reference the
> radio names with wiphy names using NL80211_CMD_GET_WIPHY dump, map the
> wiphy names to wiphy indexes (those are more useful than wiphy names
> because they don't change -- a wiphy name change doesn't generate an
> event so this can't be easily tracked), and use the
> NL80211_CMD_GET_INTERFACES dump to obtain the wiphy_idx to mac address
> mapping.
I'm a bit confused, I think the medium should always know by its
internal knowledge base, which node would be reachable by a broadcast.

We are also working on the MAC-Management of hwsim, which especially got
weird, if you try to manage many nodes within one kernel. But I don't
understand how this array with addresses could help to improve. When
these addresses do not exactly match to the preconfigured MACs of hwsim,
the frames would not be delivered, since they are only checked against
the internal ones (or you may add a check against your list).

I test to use a internal hashmap, which maps a receiver MAC-Address of a
Frame to a corresponding wiphy node to efficiently deliver frames.
Additionally it would allow to add VIF with different MAC-Addresses (see
corresponding Callback for this) and update the hashmap according to
this. I believe you want to do something similar, but in the userspace.

I also would propose a parameter (like you propose here) as internal
permanent MAC-Address, instead of the automatic increasing
MAC-Addresses, since it become complicated, if you want to use multiple
userspace medium in different network namespaces, as the hwsim MACs are
globally assigned. You may need to handle collisions (I don't know
whether different wiphy could use the same permanent MAC), but this
would be more logical for a simulation, than discover every address
after creation.


>
> Additionally there are two addresses used by hwsim radios
> (data->addresses[0] and [1]), first used at the network level and the
> second used at hwsim radio level and in HWSIM_ATTR_ADDR_TRANSMITTER /
> _RECEIVER.  By default they differ by 0x40 in the first byte but I'm not
> sure userspace can make this assumption to do the mapping from the
> address inside the frame headers to the address of the hwsim receiving
> radio.  I suspect the network layers can change address 0 and they will
> be out of sync.
>
> Signed-off-by: Andrew Zaborowski <andrew.zaborowski@intel.com>
> ---
> Additionally I tried to add a HWSIM_ATTR_WIPHY to report the wiphy index
> directly without users going through wiphy name to index mapping, but
> get_wiphy_idx() is internal to cfg80211.  The index is exposed to
> userspace and is more useful than the name so I wonder if this function
> should be exported from cfg80211.
I also think that would be nice to have ;-)

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: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Andrew Zaborowski]

On 23 February 2017 at 20:01, Benjamin Beichler
<Benjamin.Beichler@uni-rostock.de> wrote:
>> Add a HWSIM_ATTR_RADIO_ADDR attribute to those to events/response so
>> that a userspace medium can query the list of addresses in the
>> simulator.
>>
>> When a userspace medium needs to handle a broadcast frame it can't
>> easily implement the equivalent of what the default kernel medium does
>> because it doesn't know who to forward the frame to.  The
>> HWSIM_CMD_GET_RADIO command is a little useless in this respect.
>> Userspace needs to use HWSIM_CMD_GET_RADIO, then cross-reference the
>> radio names with wiphy names using NL80211_CMD_GET_WIPHY dump, map the
>> wiphy names to wiphy indexes (those are more useful than wiphy names
>> because they don't change -- a wiphy name change doesn't generate an
>> event so this can't be easily tracked), and use the
>> NL80211_CMD_GET_INTERFACES dump to obtain the wiphy_idx to mac address
>> mapping.
> I'm a bit confused, I think the medium should always know by its
> internal knowledge base, which node would be reachable by a broadcast.

So it depends on your architecture really, you can have a setup where
one userspace program creates all of the radios and registers as a
medium.  In this case the program will know what nodes exist, but it
still won't have the hardware addresses to use in
HWSIM_ATTR_ADDR_RECEIVER because there's no way to query them through
netlink (?).  Unless you assume that each consecutive radio gets
42:00:00:00:<n>:00 assigned.  That would only work if no radios have
been created before your program started because you <n> values could
be off.

But you might also want to do your testing from a script that first
creates the radios using one utility program and runs the medium as a
separate program.  In that case that program can't do the equivalent
of what the kernel medium does.

>
> We are also working on the MAC-Management of hwsim, which especially got
> weird, if you try to manage many nodes within one kernel. But I don't
> understand how this array with addresses could help to improve. When
> these addresses do not exactly match to the preconfigured MACs of hwsim,
> the frames would not be delivered, since they are only checked against
> the internal ones (or you may add a check against your list).

Those addresses I try to expose are exactly the preconfigured MACs.

Looking at this some more I found you can retrieve those addresses
from /sys/class/ieee80211/<wiphy>/addresses (there seems to be no
nl80211 API for this) since those are always in sync with the hwsim
addresses, the only issue would be mapping radios to wiphys by name.

>
> I test to use a internal hashmap, which maps a receiver MAC-Address of a
> Frame to a corresponding wiphy node to efficiently deliver frames.
> Additionally it would allow to add VIF with different MAC-Addresses (see
> corresponding Callback for this) and update the hashmap according to
> this. I believe you want to do something similar, but in the userspace.

Yes, either use a lookup table for unicast frames, or deliver all
frames (unicast or multicast) to all radios same as the kernel radio
does.

Best regards

Re: [PATCH 1/2] mac80211_hwsim: Make sure NEW_RADIO contains final name

[Andrew Zaborowski]

On 23 February 2017 at 13:02, Andrew Zaborowski
<andrew.zaborowski@intel.com> wrote:
> ieee80211_alloc_hw_nm will validate the requested name (if any) before
> creating the new device and may use a name different from the one
> requested rather than fail.  Make sure the HWSIM_CMD_NEW_RADIO
> event/response generated has the final name or userspace will receive
> the wrong name.  Note that mac80211_hwsim_new_radio may now modify
> params.

Also related to this I find that the HWSIM_ATTR_RADIO_NAME attributes
emitted contain the name string and are exactly of the right length
while the HWSIM_ATTR_RADIO_NAME attributes received by the kernel are
assumed to be NUL-terminated.  Is there a guarantee that a 0-byte
follows an attribute, or should this be changed for consistency?

Best regards

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Ben Greear]

On 02/23/2017 04:02 PM, Andrew Zaborowski wrote:
> On 23 February 2017 at 20:01, Benjamin Beichler
> <Benjamin.Beichler@uni-rostock.de> wrote:
>>> Add a HWSIM_ATTR_RADIO_ADDR attribute to those to events/response so
>>> that a userspace medium can query the list of addresses in the
>>> simulator.
>>>
>>> When a userspace medium needs to handle a broadcast frame it can't
>>> easily implement the equivalent of what the default kernel medium does
>>> because it doesn't know who to forward the frame to.  The
>>> HWSIM_CMD_GET_RADIO command is a little useless in this respect.
>>> Userspace needs to use HWSIM_CMD_GET_RADIO, then cross-reference the
>>> radio names with wiphy names using NL80211_CMD_GET_WIPHY dump, map the
>>> wiphy names to wiphy indexes (those are more useful than wiphy names
>>> because they don't change -- a wiphy name change doesn't generate an
>>> event so this can't be easily tracked), and use the
>>> NL80211_CMD_GET_INTERFACES dump to obtain the wiphy_idx to mac address
>>> mapping.
>> I'm a bit confused, I think the medium should always know by its
>> internal knowledge base, which node would be reachable by a broadcast.
>
> So it depends on your architecture really, you can have a setup where
> one userspace program creates all of the radios and registers as a
> medium.  In this case the program will know what nodes exist, but it
> still won't have the hardware addresses to use in
> HWSIM_ATTR_ADDR_RECEIVER because there's no way to query them through
> netlink (?).  Unless you assume that each consecutive radio gets
> 42:00:00:00:<n>:00 assigned.  That would only work if no radios have
> been created before your program started because you <n> values could
> be off.
>
> But you might also want to do your testing from a script that first
> creates the radios using one utility program and runs the medium as a
> separate program.  In that case that program can't do the equivalent
> of what the kernel medium does.
>
>>
>> We are also working on the MAC-Management of hwsim, which especially got
>> weird, if you try to manage many nodes within one kernel. But I don't
>> understand how this array with addresses could help to improve. When
>> these addresses do not exactly match to the preconfigured MACs of hwsim,
>> the frames would not be delivered, since they are only checked against
>> the internal ones (or you may add a check against your list).
>
> Those addresses I try to expose are exactly the preconfigured MACs.
>
> Looking at this some more I found you can retrieve those addresses
> from /sys/class/ieee80211/<wiphy>/addresses (there seems to be no
> nl80211 API for this) since those are always in sync with the hwsim
> addresses, the only issue would be mapping radios to wiphys by name.

I ended up using this approach, and I created the wiphys with specific
names so that I could know where to find them in debugfs.
I think all of those patches made it upstream....

I'll post my hwsim related patch set in case someone wants to either apply it
as is or clean it up, etc...

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 1/2] mac80211_hwsim: Make sure NEW_RADIO contains final name

[Johannes Berg]

On Fri, 2017-02-24 at 01:08 +0100, Andrew Zaborowski wrote:
> On 23 February 2017 at 13:02, Andrew Zaborowski
> <andrew.zaborowski@intel.com> wrote:
> > ieee80211_alloc_hw_nm will validate the requested name (if any)
> > before
> > creating the new device and may use a name different from the one
> > requested rather than fail.  Make sure the HWSIM_CMD_NEW_RADIO
> > event/response generated has the final name or userspace will
> > receive
> > the wrong name.  Note that mac80211_hwsim_new_radio may now modify
> > params.
> 
> Also related to this I find that the HWSIM_ATTR_RADIO_NAME attributes
> emitted contain the name string and are exactly of the right length
> while the HWSIM_ATTR_RADIO_NAME attributes received by the kernel are
> assumed to be NUL-terminated.  

I'll agree this is a bit strange - I guess it's too late to fix now
though since userspace might assume "length/data" is the string, rather
than "0-terminated" (especially if there's something like python
userspace where strings can trivially contain NUL bytes).

nla_put_string() would have added the NUL byte.

> Is there a guarantee that a 0-byte
> follows an attribute, or should this be changed for consistency?

        [HWSIM_ATTR_RADIO_NAME] = { .type = NLA_STRING },

enforces that a NUL byte *must* be present when userspace gives us the
information, so we're save - just asymmetric.

Anyway, patch applied.

johannes

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Johannes Berg]

> Additionally I tried to add a HWSIM_ATTR_WIPHY to report the wiphy
> index directly without users going through wiphy name to index
> mapping, but get_wiphy_idx() is internal to cfg80211.  The index is
> exposed to userspace and is more useful than the name so I wonder if
> this function should be exported from cfg80211.

Do you really need the address?

I'd actually prefer to *only* have the wiphy index, and I don't really
see a problem with moving the wiphy_idx from struct
cfg80211_registered_device to struct wiphy.

johannes

Re: [PATCH 1/2] mac80211_hwsim: Make sure NEW_RADIO contains final name

[Andrew Zaborowski]

Hi,

On 27 February 2017 at 14:24, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2017-02-24 at 01:08 +0100, Andrew Zaborowski wrote:
>> Also related to this I find that the HWSIM_ATTR_RADIO_NAME attributes
>> emitted contain the name string and are exactly of the right length
>> while the HWSIM_ATTR_RADIO_NAME attributes received by the kernel are
>> assumed to be NUL-terminated.
>
> I'll agree this is a bit strange - I guess it's too late to fix now
> though since userspace might assume "length/data" is the string, rather
> than "0-terminated" (especially if there's something like python
> userspace where strings can trivially contain NUL bytes).
>
> nla_put_string() would have added the NUL byte.
>
>> Is there a guarantee that a 0-byte
>> follows an attribute, or should this be changed for consistency?
>
>         [HWSIM_ATTR_RADIO_NAME] = { .type = NLA_STRING },
>
> enforces that a NUL byte *must* be present when userspace gives us the
> information, so we're save - just asymmetric.

It seems that would be NLA_NUL_STRING, whlie for NLA_STRING it's
optional, I know because I didn't add the NUL in my userspace and
things still worked.  We can copy the received nla_data into a buffer
and add the 0-byte there to support this as an option.  I don't have a
problem with the asymmetric usage though.

>
> Anyway, patch applied.

Thanks
Best regards

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Andrew Zaborowski]

Hi,

On 27 February 2017 at 14:27, Johannes Berg <johannes@sipsolutions.net> wrote:
>> Additionally I tried to add a HWSIM_ATTR_WIPHY to report the wiphy
>> index directly without users going through wiphy name to index
>> mapping, but get_wiphy_idx() is internal to cfg80211.  The index is
>> exposed to userspace and is more useful than the name so I wonder if
>> this function should be exported from cfg80211.
>
> Do you really need the address?

As it turns out it can be read from /sys, but I do need it so I can
know what to put in HWSIM_ATTR_ADDR_RECEIVER based on the destination
addr in the frame or if I want to forward the frame to all radios.  Or
is there another way to know that?

>
> I'd actually prefer to *only* have the wiphy index, and I don't really
> see a problem with moving the wiphy_idx from struct
> cfg80211_registered_device to struct wiphy.

Ok, I'll try that.  get_wiphy_idx can stay in place, not sure if I
should just drop it.

By having *only* the wiphy index you don't mean dropping the radio
names altogether?  The don't seem useful but userspace may expect
them.

Best regards

Re: [PATCH 1/2] mac80211_hwsim: Make sure NEW_RADIO contains final name

[Johannes Berg]

> >         [HWSIM_ATTR_RADIO_NAME] = { .type = NLA_STRING },
> > 
> > enforces that a NUL byte *must* be present when userspace gives us
> > the
> > information, so we're save - just asymmetric.
> 
> It seems that would be NLA_NUL_STRING, whlie for NLA_STRING it's
> optional, 

Oops. I indeed misread the code there.

> I know because I didn't add the NUL in my userspace and
> things still worked.

:)
Then you probably had some 0 padding or so.

> We can copy the received nla_data into a buffer
> and add the 0-byte there to support this as an option.

I'll do that.

johannes

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Ben Greear]

On 02/27/2017 07:26 AM, Andrew Zaborowski wrote:
> Hi,
>
> On 27 February 2017 at 14:27, Johannes Berg <johannes@sipsolutions.net> wrote:
>>> Additionally I tried to add a HWSIM_ATTR_WIPHY to report the wiphy
>>> index directly without users going through wiphy name to index
>>> mapping, but get_wiphy_idx() is internal to cfg80211.  The index is
>>> exposed to userspace and is more useful than the name so I wonder if
>>> this function should be exported from cfg80211.
>>
>> Do you really need the address?
>
> As it turns out it can be read from /sys, but I do need it so I can
> know what to put in HWSIM_ATTR_ADDR_RECEIVER based on the destination
> addr in the frame or if I want to forward the frame to all radios.  Or
> is there another way to know that?
>
>>
>> I'd actually prefer to *only* have the wiphy index, and I don't really
>> see a problem with moving the wiphy_idx from struct
>> cfg80211_registered_device to struct wiphy.
>
> Ok, I'll try that.  get_wiphy_idx can stay in place, not sure if I
> should just drop it.
>
> By having *only* the wiphy index you don't mean dropping the radio
> names altogether?  The don't seem useful but userspace may expect
> them.

I find the name and addr more useful than an 'index', because if you remove/add
a virtual phy, then the index will probably change, even if name and MAC addr may stay
the same (and so probably be the same logical entitity).

Since phys can be renamed, you cannot assume that the phy will be called
phyX where X is the device-id.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Andrew Zaborowski]

Hi,

On 27 February 2017 at 18:10, Ben Greear <greearb@candelatech.com> wrote:
> On 02/27/2017 07:26 AM, Andrew Zaborowski wrote:
>> As it turns out it can be read from /sys, but I do need it so I can
>> know what to put in HWSIM_ATTR_ADDR_RECEIVER based on the destination
>> addr in the frame or if I want to forward the frame to all radios.  Or
>> is there another way to know that?
>>
>>>
>>> I'd actually prefer to *only* have the wiphy index, and I don't really
>>> see a problem with moving the wiphy_idx from struct
>>> cfg80211_registered_device to struct wiphy.
>>
>>
>> Ok, I'll try that.  get_wiphy_idx can stay in place, not sure if I
>> should just drop it.
>>
>> By having *only* the wiphy index you don't mean dropping the radio
>> names altogether?  The don't seem useful but userspace may expect
>> them.
>
>
> I find the name and addr more useful than an 'index', because if you
> remove/add
> a virtual phy, then the index will probably change, even if name and MAC

I don't think you can remove/add a phy without removing the hwsim
radio.  A hwsim radio is tied to some wiphy at all times.

> addr may stay
> the same (and so probably be the same logical entitity).

They may stay the same or not, you can't assume either.  Sorry I don't
understand your point.

Best regards

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Ben Greear]

On 02/27/2017 05:23 PM, Andrew Zaborowski wrote:
> Hi,
>
> On 27 February 2017 at 18:10, Ben Greear <greearb@candelatech.com> wrote:
>> On 02/27/2017 07:26 AM, Andrew Zaborowski wrote:
>>> As it turns out it can be read from /sys, but I do need it so I can
>>> know what to put in HWSIM_ATTR_ADDR_RECEIVER based on the destination
>>> addr in the frame or if I want to forward the frame to all radios.  Or
>>> is there another way to know that?
>>>
>>>>
>>>> I'd actually prefer to *only* have the wiphy index, and I don't really
>>>> see a problem with moving the wiphy_idx from struct
>>>> cfg80211_registered_device to struct wiphy.
>>>
>>>
>>> Ok, I'll try that.  get_wiphy_idx can stay in place, not sure if I
>>> should just drop it.
>>>
>>> By having *only* the wiphy index you don't mean dropping the radio
>>> names altogether?  The don't seem useful but userspace may expect
>>> them.
>>
>>
>> I find the name and addr more useful than an 'index', because if you
>> remove/add
>> a virtual phy, then the index will probably change, even if name and MAC
>
> I don't think you can remove/add a phy without removing the hwsim
> radio.  A hwsim radio is tied to some wiphy at all times.

You can specify the MAC address and name (ie, wiphy0) when creating a hwsim radio.
You cannot specify the index.  So, from a user-space perspective,
I think the name and MAC is more important than the index.

Anyway, I think you are correct to pay attention to the MAC address and use
that as the identifying key.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Benjamin Beichler]

> You can specify the MAC address and name (ie, wiphy0) when creating a
> hwsim radio.
> You cannot specify the index.  So, from a user-space perspective,
> I think the name and MAC is more important than the index.
>
That is actually not possible. The permanent MAC-Addresses of hwsim
radios are directly created by there internal ID, which is only
increased for new radios and definitely not reassigned. 

But I would be happy, if this were possible, since it would make some
simulation related stuff easier (currently we need to unload and load
the hwsim module over and over again for every run).



-- 
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/

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Ben Greear]

On 03/01/2017 06:33 AM, Benjamin Beichler wrote:
>> You can specify the MAC address and name (ie, wiphy0) when creating a
>> hwsim radio.
>> You cannot specify the index.  So, from a user-space perspective,
>> I think the name and MAC is more important than the index.
>>
> That is actually not possible. The permanent MAC-Addresses of hwsim
> radios are directly created by there internal ID, which is only
> increased for new radios and definitely not reassigned.
>
> But I would be happy, if this were possible, since it would make some
> simulation related stuff easier (currently we need to unload and load
> the hwsim module over and over again for every run).

Ahh, you are right.  But you can specify the wiphy name, and then you
can use that to query the debugfs to find the MAC addr, and then sync
your user-space with that info.

It should also be easy enough to specify the MAC address when creating
the radio.

And, you should be able to create virtual STA, AP, etc, with any specified
MAC address, so the MAC of the radio really should not matter much?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Benjamin Beichler]

Am 01.03.2017 um 16:35 schrieb Ben Greear:
> That is actually not possible. The permanent MAC-Addresses of hwsim
>
>> radios are directly created by there internal ID, which is only
>> increased for new radios and definitely not reassigned.
>>
>> But I would be happy, if this were possible, since it would make some
>> simulation related stuff easier (currently we need to unload and load
>> the hwsim module over and over again for every run).
>
> Ahh, you are right.  But you can specify the wiphy name, and then you
> can use that to query the debugfs to find the MAC addr, and then sync
> your user-space with that info.
>
> It should also be easy enough to specify the MAC address when creating
> the radio.
>
> And, you should be able to create virtual STA, AP, etc, with any
> specified
> MAC address, so the MAC of the radio really should not matter much?
>
That is another problem ... this won't work currently, since the frame
delivery compares the receiver address with the permanent MAC addresses
of all hwsim phys. I work as an idle task on patch, which makes this
lookup (currently implemented as linear search) as a hashmap mapping an
MAC to a wiphy struct , including scenarios with differing MAC addresses
of vifs.

> Thanks,
> Ben
>

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/

Re: [PATCH 2/2][RFC] mac80211_hwsim: Report radio addresses in NEW_RADIO/GET_RADIO

[Johannes Berg]

On Wed, 2017-03-01 at 18:04 +0100, Benjamin Beichler wrote:
> Am 01.03.2017 um 16:35 schrieb Ben Greear:
> > That is actually not possible. The permanent MAC-Addresses of hwsim
> > 
> > > radios are directly created by there internal ID, which is only
> > > increased for new radios and definitely not reassigned.
> > > 
> > > But I would be happy, if this were possible, since it would make
> > > some
> > > simulation related stuff easier (currently we need to unload and
> > > load
> > > the hwsim module over and over again for every run).
> > 
> > Ahh, you are right.  But you can specify the wiphy name, and then
> > you
> > can use that to query the debugfs to find the MAC addr, and then
> > sync
> > your user-space with that info.
> > 
> > It should also be easy enough to specify the MAC address when
> > creating
> > the radio.
> > 
> > And, you should be able to create virtual STA, AP, etc, with any
> > specified
> > MAC address, so the MAC of the radio really should not matter much?
> > 
> 
> That is another problem ... this won't work currently, since the
> frame
> delivery compares the receiver address with the permanent MAC
> addresses
> of all hwsim phys. I work as an idle task on patch, which makes this
> lookup (currently implemented as linear search) as a hashmap mapping
> an
> MAC to a wiphy struct , including scenarios with differing MAC
> addresses
> of vifs.

That's probably something we really ought to fix :)

johannes