Re: [PATCH 2/3] mac80211-hwsim: Provide multicast event for HWSIM_CMD_NEW_RADIO

2014-10-30 Thread Jukka Rissanen 
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


--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] mac80211-hwsim: Provide multicast event for HWSIM_CMD_NEW_RADIO

2014-10-29 Thread Johannes Berg 
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

--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] mac80211-hwsim: Provide multicast event for HWSIM_CMD_NEW_RADIO

2014-10-29 Thread Johannes Berg 
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

--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] mac80211-hwsim: Provide multicast event for HWSIM_CMD_NEW_RADIO

2014-10-29 Thread Johannes Berg 
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


--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html