Hi,
patch 1 renames HWSIM_CMD_CREATE_RADIO to HWSIM_CMD_NEW_RADIO and
HWSIM_CMD_DESTROY_RADIO to HWSIM_CMD_DEL_RADIO. They are more
fitting on how other pieces of the wireless system work. A define
to old name is provided for backward compatibility.
Patches 2 and 3 use the newly named enums and provide a way to inform
the listeners on the multicast group "config" when new radio is created
and existing radio is deleted.
Cheers,
Jukka
Jukka Rissanen (3):
mac80211-hwsim: Rename CREATE and DESTROY radio to NEW and DEL radio
mac80211-hwsim: Provide multicast event for HWSIM_CMD_NEW_RADIO
mac80211-hwsim: Provide multicast event for HWSIM_CMD_DEL_RADIO
drivers/net/wireless/mac80211_hwsim.c | 184 +++++++++++++++++++++++++++++-----
drivers/net/wireless/mac80211_hwsim.h | 14 ++-
2 files changed, 169 insertions(+), 29 deletions(-)
--
1.8.3.1
On ma, 2014-10-27 at 12:31 +0100, Johannes Berg wrote:
> On Mon, 2014-10-27 at 13:29 +0200, Jukka Rissanen wrote:
>
> > Yes, but from generic nl80211 event you cannot know if event is from
> > hwsim radio or not. With this patch, the listener can know that the
> > radio new/del event is specifically related to hwsim radio.
>
> That's not particularly hard to figure out, for example by looking at
> sysfs.
>
> Is this really so time-constrained/important/... that you can't do that?
It does not seem very practical to dig this information from sysfs as
the same information can be easily get via netlink as this patch shows.
Cheers,
Jukka
On Mon, 2014-10-27 at 12:44 +0200, Jukka Rissanen wrote:
> +static void mcast_new_radio(int id, struct genl_info *info,
> + int channels, const char *reg_alpha2,
> + const struct ieee80211_regdomain *regd,
> + bool reg_strict, bool p2p_device,
> + bool use_chanctx)
Since you're adding yet another function with all these arguments, maybe
you could split them out into some kind of radio config struct that you
can pass around?
johannes
Using the name HWSIM_CMD_NEW_RADIO and HWSIM_CMD_DEL_RADIO is more
fitting on how other pieces of the wireless system work.
Signed-off-by: Jukka Rissanen <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.h | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h
index c9d0315..1f4f790 100644
--- a/drivers/net/wireless/mac80211_hwsim.h
+++ b/drivers/net/wireless/mac80211_hwsim.h
@@ -65,9 +65,10 @@ enum hwsim_tx_control_flags {
* kernel, uses:
* %HWSIM_ATTR_ADDR_TRANSMITTER, %HWSIM_ATTR_FLAGS,
* %HWSIM_ATTR_TX_INFO, %HWSIM_ATTR_SIGNAL, %HWSIM_ATTR_COOKIE
- * @HWSIM_CMD_CREATE_RADIO: create a new radio with the given parameters,
- * returns the radio ID (>= 0) or negative on errors
- * @HWSIM_CMD_DESTROY_RADIO: destroy a radio
+ * @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
+ * @HWSIM_CMD_DEL_RADIO: destroy a radio, reply is multicasted
* @__HWSIM_CMD_MAX: enum limit
*/
enum {
@@ -75,12 +76,15 @@ enum {
HWSIM_CMD_REGISTER,
HWSIM_CMD_FRAME,
HWSIM_CMD_TX_INFO_FRAME,
- HWSIM_CMD_CREATE_RADIO,
- HWSIM_CMD_DESTROY_RADIO,
+ HWSIM_CMD_NEW_RADIO,
+ HWSIM_CMD_DEL_RADIO,
__HWSIM_CMD_MAX,
};
#define HWSIM_CMD_MAX (_HWSIM_CMD_MAX - 1)
+#define HWSIM_CMD_CREATE_RADIO HWSIM_CMD_NEW_RADIO
+#define HWSIM_CMD_DESTROY_RADIO HWSIM_CMD_DEL_RADIO
+
/**
* enum hwsim_attrs - hwsim netlink attributes
*
--
1.8.3.1
Hi Johannes,
On ke, 2014-10-29 at 16:53 +0100, Johannes Berg wrote:
> On Wed, 2014-10-29 at 16:50 +0100, Johannes Berg wrote:
> > On Mon, 2014-10-27 at 12:44 +0200, Jukka Rissanen wrote:
> >
> > > +static void mcast_msg(struct sk_buff *mcast_skb, struct genl_info *info)
> > > +{
> > > + if (info)
> > > + genl_notify(&hwsim_genl_family, mcast_skb,
> > > + genl_info_net(info), info->snd_portid,
> > > + HWSIM_MCGRP_CONFIG, info->nlhdr, GFP_KERNEL);
> > > + else
> > > + genlmsg_multicast(&hwsim_genl_family, mcast_skb, 0,
> > > + HWSIM_MCGRP_CONFIG, GFP_KERNEL);
> > > +}
> >
> > Also - given the parameters and what this does, that's a bad name for
> > the function. Never mind that it doesn't have any sort of identifier
> > (say hwsim_ prefix), it doesn't even do what it says it does.
>
> Or maybe it does? I'm unsure what genl_notify() does...
Yes, genl_notify() will eventually call nlmsg_notify() which will
multicast the message because we have set the group id in the call.
http://lxr.free-electrons.com/source/net/netlink/af_netlink.c#L2856
Cheers,
Jukka
On Mon, 2014-10-27 at 12:44 +0200, Jukka Rissanen wrote:
> Using the name HWSIM_CMD_NEW_RADIO and HWSIM_CMD_DEL_RADIO is more
> fitting on how other pieces of the wireless system work.
Applied.
johannes
On Mon, 2014-10-27 at 16:14 -0700, Marcel Holtmann wrote:
> Hi Luca,
Hi Marcel,
> >>>>> That's not particularly hard to figure out, for example by looking at
> >>>>> sysfs.
> >>>>>
> >>>>> Is this really so time-constrained/important/... that you can't do that?
> >>>>
> >>>> It does not seem very practical to dig this information from sysfs as
> >>>> the same information can be easily get via netlink as this patch shows.
> >>>
> >>> Well, that's a slippery slope. I'd consider it more practical to use
> >>> existing APIs instead of (gratuitously) inventing new ones. It'll even
> >>> work on older kernels as an added benefit.
> >>
> >> I see that different. The component that handles the emulation of the new wireless device should be independent from the component driving it. I prefer to have a race free way of obtaining the needed information without having to monitor nl80211 and sysfs for this. Especially with the use cases that we have in mind it has no business with these other interfaces.
> >>
> >> We have been down this route with the bridge interface where people had to dig out information from sysfs and it did not work out nicely. So now everything moves to netlink.
> >
> > Why does hwsim have to be treated differently from any other device?
> > Unlike bridging, HW emulation doesn't seem to be a real life use case.
> >
> > But I'm probably missing something. ;)
>
> this is the controlling side. The thing that I call emulator. It is the component that creates/destroys the hwsim wiphy. It can also be the one that handles the packet processing similar to wmediumd.
>
> The nl80211/cfg80211 is not treated differently here. This is purely for the MAC80211_HWSIM netlink family side of things. Makes sense?
Ah, okay, I was confused. Makes total sense to me then, I don't see why
we shouldn't have this API to provide the needed information totally
independently from nl80211.
--
Luca.
On 10/27/2014 04:14 PM, Marcel Holtmann wrote:
> Hi Luca,
>
>>>>>> That's not particularly hard to figure out, for example by looking at
>>>>>> sysfs.
>>>>>>
>>>>>> Is this really so time-constrained/important/... that you can't do that?
>>>>>
>>>>> It does not seem very practical to dig this information from sysfs as
>>>>> the same information can be easily get via netlink as this patch shows.
>>>>
>>>> Well, that's a slippery slope. I'd consider it more practical to use
>>>> existing APIs instead of (gratuitously) inventing new ones. It'll even
>>>> work on older kernels as an added benefit.
>>>
>>> I see that different. The component that handles the emulation of the new wireless device should be independent from the component driving it. I prefer to have a race free way of obtaining the needed information without having to monitor nl80211 and sysfs for this. Especially with the use cases that we have in mind it has no business with these other interfaces.
>>>
>>> We have been down this route with the bridge interface where people had to dig out information from sysfs and it did not work out nicely. So now everything moves to netlink.
>>
>> Why does hwsim have to be treated differently from any other device?
>> Unlike bridging, HW emulation doesn't seem to be a real life use case.
>>
>> But I'm probably missing something. ;)
>
> this is the controlling side. The thing that I call emulator. It is the component that creates/destroys the hwsim wiphy. It can also be the one that handles the packet processing similar to wmediumd.
>
> The nl80211/cfg80211 is not treated differently here. This is purely for the MAC80211_HWSIM netlink family side of things. Makes sense?
I just got some patches accepted upstream that allow you to name the phy upon creation
(and to suppress wlanX creation in case that is desired).
If your control tool is creating the phys, then it can know ahead of time the names
and match the events that way.
I'm not taking sides on your particular patch, but those features made my
tool a lot easier to write and more efficient.
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
Hi Luca,
>>>>> That's not particularly hard to figure out, for example by looking at
>>>>> sysfs.
>>>>>
>>>>> Is this really so time-constrained/important/... that you can't do that?
>>>>
>>>> It does not seem very practical to dig this information from sysfs as
>>>> the same information can be easily get via netlink as this patch shows.
>>>
>>> Well, that's a slippery slope. I'd consider it more practical to use
>>> existing APIs instead of (gratuitously) inventing new ones. It'll even
>>> work on older kernels as an added benefit.
>>
>> I see that different. The component that handles the emulation of the new wireless device should be independent from the component driving it. I prefer to have a race free way of obtaining the needed information without having to monitor nl80211 and sysfs for this. Especially with the use cases that we have in mind it has no business with these other interfaces.
>>
>> We have been down this route with the bridge interface where people had to dig out information from sysfs and it did not work out nicely. So now everything moves to netlink.
>
> Why does hwsim have to be treated differently from any other device?
> Unlike bridging, HW emulation doesn't seem to be a real life use case.
>
> But I'm probably missing something. ;)
this is the controlling side. The thing that I call emulator. It is the component that creates/destroys the hwsim wiphy. It can also be the one that handles the packet processing similar to wmediumd.
The nl80211/cfg80211 is not treated differently here. This is purely for the MAC80211_HWSIM netlink family side of things. Makes sense?
Regards
Marcel
On Mon, 2014-10-27 at 13:55 +0200, Jukka Rissanen wrote:
> > That's not particularly hard to figure out, for example by looking at
> > sysfs.
> >
> > Is this really so time-constrained/important/... that you can't do that?
>
> It does not seem very practical to dig this information from sysfs as
> the same information can be easily get via netlink as this patch shows.
Well, that's a slippery slope. I'd consider it more practical to use
existing APIs instead of (gratuitously) inventing new ones. It'll even
work on older kernels as an added benefit.
johannes
When deleting old radio via HWSIM_CMD_DEL_RADIO then listeners on the
multicast group "config" are informed.
Signed-off-by: Jukka Rissanen <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 43 ++++++++++++++++++++++++++++++-----
1 file changed, 37 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 74bc1db..b50a739 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2293,8 +2293,39 @@ failed:
return err;
}
-static void mac80211_hwsim_destroy_radio(struct mac80211_hwsim_data *data)
+static void mcast_del_radio(int id, struct genl_info *info)
{
+ struct sk_buff *mcast_skb;
+ void *data;
+ int ret;
+
+ mcast_skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!mcast_skb)
+ return;
+
+ data = genlmsg_put(mcast_skb, 0, 0, &hwsim_genl_family, 0,
+ HWSIM_CMD_DEL_RADIO);
+ if (!data)
+ goto error;
+
+ ret = nla_put_u32(mcast_skb, HWSIM_ATTR_RADIO_ID, id);
+ if (ret < 0)
+ goto error;
+
+ genlmsg_end(mcast_skb, data);
+
+ mcast_msg(mcast_skb, info);
+
+ return;
+
+error:
+ nlmsg_free(mcast_skb);
+}
+
+static void mac80211_hwsim_del_radio(struct mac80211_hwsim_data *data,
+ struct genl_info *info)
+{
+ mcast_del_radio(data->idx, info);
debugfs_remove_recursive(data->debugfs);
ieee80211_unregister_hw(data->hw);
device_release_driver(data->dev);
@@ -2312,7 +2343,7 @@ static void mac80211_hwsim_free(void)
list))) {
list_del(&data->list);
spin_unlock_bh(&hwsim_radio_lock);
- mac80211_hwsim_destroy_radio(data);
+ mac80211_hwsim_del_radio(data, NULL);
spin_lock_bh(&hwsim_radio_lock);
}
spin_unlock_bh(&hwsim_radio_lock);
@@ -2562,7 +2593,7 @@ static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info)
p2p_device, use_chanctx);
}
-static int hwsim_destroy_radio_nl(struct sk_buff *msg, struct genl_info *info)
+static int hwsim_del_radio_nl(struct sk_buff *msg, struct genl_info *info)
{
struct mac80211_hwsim_data *data;
int idx;
@@ -2577,7 +2608,7 @@ static int hwsim_destroy_radio_nl(struct sk_buff *msg, struct genl_info *info)
continue;
list_del(&data->list);
spin_unlock_bh(&hwsim_radio_lock);
- mac80211_hwsim_destroy_radio(data);
+ mac80211_hwsim_del_radio(data, info);
return 0;
}
spin_unlock_bh(&hwsim_radio_lock);
@@ -2610,9 +2641,9 @@ static const struct genl_ops hwsim_ops[] = {
.flags = GENL_ADMIN_PERM,
},
{
- .cmd = HWSIM_CMD_DESTROY_RADIO,
+ .cmd = HWSIM_CMD_DEL_RADIO,
.policy = hwsim_genl_policy,
- .doit = hwsim_destroy_radio_nl,
+ .doit = hwsim_del_radio_nl,
.flags = GENL_ADMIN_PERM,
},
};
--
1.8.3.1
Hi Johannes,
>>> That's not particularly hard to figure out, for example by looking at
>>> sysfs.
>>>
>>> Is this really so time-constrained/important/... that you can't do that?
>>
>> It does not seem very practical to dig this information from sysfs as
>> the same information can be easily get via netlink as this patch shows.
>
> Well, that's a slippery slope. I'd consider it more practical to use
> existing APIs instead of (gratuitously) inventing new ones. It'll even
> work on older kernels as an added benefit.
I see that different. The component that handles the emulation of the new wireless device should be independent from the component driving it. I prefer to have a race free way of obtaining the needed information without having to monitor nl80211 and sysfs for this. Especially with the use cases that we have in mind it has no business with these other interfaces.
We have been down this route with the bridge interface where people had to dig out information from sysfs and it did not work out nicely. So now everything moves to netlink.
Regards
Marcel
On Mon, 2014-10-27 at 07:19 -0700, Marcel Holtmann wrote:
> Hi Johannes,
>
> >>> That's not particularly hard to figure out, for example by looking at
> >>> sysfs.
> >>>
> >>> Is this really so time-constrained/important/... that you can't do that?
> >>
> >> It does not seem very practical to dig this information from sysfs as
> >> the same information can be easily get via netlink as this patch shows.
> >
> > Well, that's a slippery slope. I'd consider it more practical to use
> > existing APIs instead of (gratuitously) inventing new ones. It'll even
> > work on older kernels as an added benefit.
>
> I see that different. The component that handles the emulation of the new wireless device should be independent from the component driving it. I prefer to have a race free way of obtaining the needed information without having to monitor nl80211 and sysfs for this. Especially with the use cases that we have in mind it has no business with these other interfaces.
>
> We have been down this route with the bridge interface where people had to dig out information from sysfs and it did not work out nicely. So now everything moves to netlink.
Why does hwsim have to be treated differently from any other device?
Unlike bridging, HW emulation doesn't seem to be a real life use case.
But I'm probably missing something. ;)
--
Luca.
When adding new radio via HWSIM_CMD_NEW_RADIO then listeners on the
multicast group "config" are informed.
Signed-off-by: Jukka Rissanen <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 141 +++++++++++++++++++++++++++++-----
1 file changed, 123 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index babbdc1..74bc1db 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -476,6 +476,14 @@ static struct genl_family hwsim_genl_family = {
.maxattr = HWSIM_ATTR_MAX,
};
+enum hwsim_multicast_groups {
+ HWSIM_MCGRP_CONFIG,
+};
+
+static const struct genl_multicast_group hwsim_mcgrps[] = {
+ [HWSIM_MCGRP_CONFIG] = { .name = "config", },
+};
+
/* MAC80211_HWSIM netlink policy */
static const struct nla_policy hwsim_genl_policy[HWSIM_ATTR_MAX + 1] = {
@@ -1943,10 +1951,101 @@ static const struct ieee80211_ops mac80211_hwsim_ops = {
static struct ieee80211_ops mac80211_hwsim_mchan_ops;
-static int mac80211_hwsim_create_radio(int channels, const char *reg_alpha2,
- const struct ieee80211_regdomain *regd,
- bool reg_strict, bool p2p_device,
- bool use_chanctx)
+static void mcast_msg(struct sk_buff *mcast_skb, struct genl_info *info)
+{
+ if (info)
+ genl_notify(&hwsim_genl_family, mcast_skb,
+ genl_info_net(info), info->snd_portid,
+ HWSIM_MCGRP_CONFIG, info->nlhdr, GFP_KERNEL);
+ else
+ genlmsg_multicast(&hwsim_genl_family, mcast_skb, 0,
+ HWSIM_MCGRP_CONFIG, GFP_KERNEL);
+}
+
+static void mcast_new_radio(int id, struct genl_info *info,
+ int channels, const char *reg_alpha2,
+ const struct ieee80211_regdomain *regd,
+ bool reg_strict, bool p2p_device,
+ bool use_chanctx)
+{
+ struct sk_buff *mcast_skb;
+ void *data;
+ int ret;
+
+ mcast_skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!mcast_skb)
+ return;
+
+ data = genlmsg_put(mcast_skb, 0, 0, &hwsim_genl_family, 0,
+ HWSIM_CMD_NEW_RADIO);
+ if (!data)
+ goto error;
+
+ ret = nla_put_u32(mcast_skb, HWSIM_ATTR_RADIO_ID, id);
+ if (ret < 0)
+ goto error;
+
+ if (channels) {
+ ret = nla_put_u32(mcast_skb, HWSIM_ATTR_CHANNELS, channels);
+ if (ret < 0)
+ goto error;
+ }
+
+ if (reg_alpha2) {
+ ret = nla_put(mcast_skb, HWSIM_ATTR_REG_HINT_ALPHA2, 2,
+ reg_alpha2);
+ if (ret < 0)
+ goto error;
+ }
+
+ if (regd) {
+ int i;
+
+ for (i = 0; hwsim_world_regdom_custom[i] != regd &&
+ i < ARRAY_SIZE(hwsim_world_regdom_custom); i++)
+ ;
+
+ if (i < ARRAY_SIZE(hwsim_world_regdom_custom)) {
+ ret = nla_put_u32(mcast_skb, HWSIM_ATTR_REG_CUSTOM_REG,
+ i);
+ if (ret < 0)
+ goto error;
+ }
+ }
+
+ if (reg_strict) {
+ ret = nla_put_flag(mcast_skb, HWSIM_ATTR_REG_STRICT_REG);
+ if (ret < 0)
+ goto error;
+ }
+
+ if (p2p_device) {
+ ret = nla_put_flag(mcast_skb, HWSIM_ATTR_SUPPORT_P2P_DEVICE);
+ if (ret < 0)
+ goto error;
+ }
+
+ if (use_chanctx) {
+ ret = nla_put_flag(mcast_skb, HWSIM_ATTR_USE_CHANCTX);
+ if (ret < 0)
+ goto error;
+ }
+
+ genlmsg_end(mcast_skb, data);
+
+ mcast_msg(mcast_skb, info);
+
+ return;
+
+error:
+ nlmsg_free(mcast_skb);
+}
+
+static int mac80211_hwsim_new_radio(struct genl_info *info,
+ int channels, const char *reg_alpha2,
+ const struct ieee80211_regdomain *regd,
+ bool reg_strict, bool p2p_device,
+ bool use_chanctx)
{
int err;
u8 addr[ETH_ALEN];
@@ -2180,6 +2279,10 @@ static int mac80211_hwsim_create_radio(int channels, const char *reg_alpha2,
list_add_tail(&data->list, &hwsim_radios);
spin_unlock_bh(&hwsim_radio_lock);
+ if (idx > 0)
+ mcast_new_radio(idx, info, channels, reg_alpha2,
+ regd, reg_strict, p2p_device, use_chanctx);
+
return idx;
failed_hw:
@@ -2427,7 +2530,7 @@ static int hwsim_register_received_nl(struct sk_buff *skb_2,
return 0;
}
-static int hwsim_create_radio_nl(struct sk_buff *msg, struct genl_info *info)
+static int hwsim_new_radio_nl(struct sk_buff *msg, struct genl_info *info)
{
unsigned int chans = channels;
const char *alpha2 = NULL;
@@ -2455,8 +2558,8 @@ static int hwsim_create_radio_nl(struct sk_buff *msg, struct genl_info *info)
regd = hwsim_world_regdom_custom[idx];
}
- return mac80211_hwsim_create_radio(chans, alpha2, regd, reg_strict,
- p2p_device, use_chanctx);
+ return mac80211_hwsim_new_radio(info, chans, alpha2, regd, reg_strict,
+ p2p_device, use_chanctx);
}
static int hwsim_destroy_radio_nl(struct sk_buff *msg, struct genl_info *info)
@@ -2501,9 +2604,9 @@ static const struct genl_ops hwsim_ops[] = {
.doit = hwsim_tx_info_frame_received_nl,
},
{
- .cmd = HWSIM_CMD_CREATE_RADIO,
+ .cmd = HWSIM_CMD_NEW_RADIO,
.policy = hwsim_genl_policy,
- .doit = hwsim_create_radio_nl,
+ .doit = hwsim_new_radio_nl,
.flags = GENL_ADMIN_PERM,
},
{
@@ -2542,7 +2645,9 @@ static int hwsim_init_netlink(void)
printk(KERN_INFO "mac80211_hwsim: initializing netlink\n");
- rc = genl_register_family_with_ops(&hwsim_genl_family, hwsim_ops);
+ rc = genl_register_family_with_ops_groups(&hwsim_genl_family,
+ hwsim_ops,
+ hwsim_mcgrps);
if (rc)
goto failure;
@@ -2603,6 +2708,10 @@ static int __init init_mac80211_hwsim(void)
goto out_unregister_driver;
}
+ err = hwsim_init_netlink();
+ if (err < 0)
+ goto out_unregister_driver;
+
for (i = 0; i < radios; i++) {
const char *reg_alpha2 = NULL;
const struct ieee80211_regdomain *regd = NULL;
@@ -2673,10 +2782,10 @@ static int __init init_mac80211_hwsim(void)
break;
}
- err = mac80211_hwsim_create_radio(channels, reg_alpha2,
- regd, reg_strict,
- support_p2p_device,
- channels > 1);
+ err = mac80211_hwsim_new_radio(NULL, channels, reg_alpha2,
+ regd, reg_strict,
+ support_p2p_device,
+ channels > 1);
if (err < 0)
goto out_free_radios;
}
@@ -2702,10 +2811,6 @@ static int __init init_mac80211_hwsim(void)
}
rtnl_unlock();
- err = hwsim_init_netlink();
- if (err < 0)
- goto out_free_mon;
-
return 0;
out_free_mon:
--
1.8.3.1
On Mon, 2014-10-27 at 12:44 +0200, Jukka Rissanen wrote:
> Hi,
>
> patch 1 renames HWSIM_CMD_CREATE_RADIO to HWSIM_CMD_NEW_RADIO and
> HWSIM_CMD_DESTROY_RADIO to HWSIM_CMD_DEL_RADIO. They are more
> fitting on how other pieces of the wireless system work. A define
> to old name is provided for backward compatibility.
That seems reasonable, if a little gratuitous.
> Patches 2 and 3 use the newly named enums and provide a way to inform
> the listeners on the multicast group "config" when new radio is created
> and existing radio is deleted.
Why bother? There are nl80211 events for this already, no?
johannes
Hi Johannes,
On ma, 2014-10-27 at 12:20 +0100, Johannes Berg wrote:
> On Mon, 2014-10-27 at 12:44 +0200, Jukka Rissanen wrote:
> > Hi,
> >
> > patch 1 renames HWSIM_CMD_CREATE_RADIO to HWSIM_CMD_NEW_RADIO and
> > HWSIM_CMD_DESTROY_RADIO to HWSIM_CMD_DEL_RADIO. They are more
> > fitting on how other pieces of the wireless system work. A define
> > to old name is provided for backward compatibility.
>
> That seems reasonable, if a little gratuitous.
>
> > Patches 2 and 3 use the newly named enums and provide a way to inform
> > the listeners on the multicast group "config" when new radio is created
> > and existing radio is deleted.
>
> Why bother? There are nl80211 events for this already, no?
Yes, but from generic nl80211 event you cannot know if event is from
hwsim radio or not. With this patch, the listener can know that the
radio new/del event is specifically related to hwsim radio.
Cheers,
Jukka
On Mon, 2014-10-27 at 13:29 +0200, Jukka Rissanen wrote:
> Yes, but from generic nl80211 event you cannot know if event is from
> hwsim radio or not. With this patch, the listener can know that the
> radio new/del event is specifically related to hwsim radio.
That's not particularly hard to figure out, for example by looking at
sysfs.
Is this really so time-constrained/important/... that you can't do that?
johannes
On Wed, 2014-10-29 at 16:50 +0100, Johannes Berg wrote:
> On Mon, 2014-10-27 at 12:44 +0200, Jukka Rissanen wrote:
>
> > +static void mcast_msg(struct sk_buff *mcast_skb, struct genl_info *info)
> > +{
> > + if (info)
> > + genl_notify(&hwsim_genl_family, mcast_skb,
> > + genl_info_net(info), info->snd_portid,
> > + HWSIM_MCGRP_CONFIG, info->nlhdr, GFP_KERNEL);
> > + else
> > + genlmsg_multicast(&hwsim_genl_family, mcast_skb, 0,
> > + HWSIM_MCGRP_CONFIG, GFP_KERNEL);
> > +}
>
> Also - given the parameters and what this does, that's a bad name for
> the function. Never mind that it doesn't have any sort of identifier
> (say hwsim_ prefix), it doesn't even do what it says it does.
Or maybe it does? I'm unsure what genl_notify() does...
johannes
On Mon, 2014-10-27 at 12:44 +0200, Jukka Rissanen wrote:
> +static void mcast_msg(struct sk_buff *mcast_skb, struct genl_info *info)
> +{
> + if (info)
> + genl_notify(&hwsim_genl_family, mcast_skb,
> + genl_info_net(info), info->snd_portid,
> + HWSIM_MCGRP_CONFIG, info->nlhdr, GFP_KERNEL);
> + else
> + genlmsg_multicast(&hwsim_genl_family, mcast_skb, 0,
> + HWSIM_MCGRP_CONFIG, GFP_KERNEL);
> +}
Also - given the parameters and what this does, that's a bad name for
the function. Never mind that it doesn't have any sort of identifier
(say hwsim_ prefix), it doesn't even do what it says it does.
johannes