[PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

Users may want to send a frame on the current channel
without specifying it.

Make mgmt_tx pass a NULL channel to mac80211 if none has
been specified by the user.

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 net/wireless/nl80211.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 31d265f..c2376c7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7142,6 +7142,9 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
 		return -EOPNOTSUPP;
 
 	switch (wdev->iftype) {
+	case NL80211_IFTYPE_P2P_DEVICE:
+		if (!info->attrs[NL80211_ATTR_WIPHY_FREQ])
+			return -EINVAL;
 	case NL80211_IFTYPE_STATION:
 	case NL80211_IFTYPE_ADHOC:
 	case NL80211_IFTYPE_P2P_CLIENT:
@@ -7149,7 +7152,6 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
 	case NL80211_IFTYPE_AP_VLAN:
 	case NL80211_IFTYPE_MESH_POINT:
 	case NL80211_IFTYPE_P2P_GO:
-	case NL80211_IFTYPE_P2P_DEVICE:
 		break;
 	default:
 		return -EOPNOTSUPP;
@@ -7177,9 +7179,16 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
 
 	no_cck = nla_get_flag(info->attrs[NL80211_ATTR_TX_NO_CCK_RATE]);
 
-	err = nl80211_parse_chandef(rdev, info, &chandef);
-	if (err)
-		return err;
+	/*
+	 * get the channel if any has been specified, otherwise pass NULL to
+	 * mac80211. The latter will use the current one
+	 */
+	chandef.chan = NULL;
+	if (info->attrs[NL80211_ATTR_WIPHY_FREQ]) {
+		err = nl80211_parse_chandef(rdev, info, &chandef);
+		if (err)
+			return err;
+	}
 
 	if (!dont_wait_for_ack) {
 		msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-- 
1.8.1.5

[PATCH 2/2] mac80211: in mgmt_tx use the current channel if none has been specified

[Antonio Quartulli]

From: Antonio Quartulli <antonio@open-mesh.com>

In mgmt_tx, if no channel has been specified via cfg80211,
use the current one.

If the interface requires offchan (because disconnected or
for other reasons) then fail.

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 net/mac80211/cfg.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 9034da1..e6e41c7 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2836,6 +2836,13 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		return -EOPNOTSUPP;
 	}
 
+	/*
+	 * configurations requiring offchan cannot work if no channel has been
+	 * specified
+	 */
+	if (need_offchan && !chan)
+		return -EINVAL;
+
 	mutex_lock(&local->mtx);
 
 	/* Check if the operating channel is the requested channel */
@@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		rcu_read_lock();
 		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
 
+		/* if no channel was specified, use the current one */
+		if (chanctx_conf && !chan)
+			chan = chanctx_conf->def.chan;
+
 		if (chanctx_conf)
 			need_offchan = chan != chanctx_conf->def.chan;
 		else
@@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		rcu_read_unlock();
 	}
 
+	/* at this point a channel should have been chosen */
+	if (!chan) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
 	if (need_offchan && !offchan) {
 		ret = -EBUSY;
 		goto out_unlock;
-- 
1.8.1.5

Re: [PATCH 2/2] mac80211: in mgmt_tx use the current channel if none has been specified

[Johannes Berg]

On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote:

> @@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
>  		rcu_read_lock();
>  		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>  
> +		/* if no channel was specified, use the current one */
> +		if (chanctx_conf && !chan)
> +			chan = chanctx_conf->def.chan;
> +
>  		if (chanctx_conf)
>  			need_offchan = chan != chanctx_conf->def.chan;
>  		else
> @@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
>  		rcu_read_unlock();
>  	}
>  
> +	/* at this point a channel should have been chosen */
> +	if (!chan) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +

These two changes make no sense at all. If you look at the function
you'll see that "chan" isn't used at all after the check, and modifying
the "check if ..." part to use the channel also doesn't make sense.

johannes

Re: [PATCH 2/2] mac80211: in mgmt_tx use the current channel if none has been specified

[Antonio Quartulli]

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

On Mon, Jun 03, 2013 at 05:01:30PM +0200, Johannes Berg wrote:
> On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote:
> 
> > @@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> >  		rcu_read_lock();
> >  		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> >  
> > +		/* if no channel was specified, use the current one */
> > +		if (chanctx_conf && !chan)
> > +			chan = chanctx_conf->def.chan;
> > +
> >  		if (chanctx_conf)
> >  			need_offchan = chan != chanctx_conf->def.chan;
> >  		else
> > @@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> >  		rcu_read_unlock();
> >  	}
> >  
> > +	/* at this point a channel should have been chosen */
> > +	if (!chan) {
> > +		ret = -EINVAL;
> > +		goto out_unlock;
> > +	}
> > +
> 
> These two changes make no sense at all. If you look at the function
> you'll see that "chan" isn't used at all after the check, 

uhm? it is passed to ieee80211_start_roc_work() right after (this part has not
been changed).

2904         ret = ieee80211_start_roc_work(local, sdata, chan,


> and modifying
> the "check if ..." part to use the channel also doesn't make sense.
> 

to which part do you exactly refer? I'm just ensuring that the chan variable
gets assigned before something accesses it.

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/2] mac80211: in mgmt_tx use the current channel if none has been specified

[Johannes Berg]

On Mon, 2013-06-03 at 20:03 +0200, Antonio Quartulli wrote:
> On Mon, Jun 03, 2013 at 05:01:30PM +0200, Johannes Berg wrote:
> > On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote:
> > 
> > > @@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> > >  		rcu_read_lock();
> > >  		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> > >  
> > > +		/* if no channel was specified, use the current one */
> > > +		if (chanctx_conf && !chan)
> > > +			chan = chanctx_conf->def.chan;
> > > +
> > >  		if (chanctx_conf)
> > >  			need_offchan = chan != chanctx_conf->def.chan;
> > >  		else
> > > @@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> > >  		rcu_read_unlock();
> > >  	}
> > >  
> > > +	/* at this point a channel should have been chosen */
> > > +	if (!chan) {
> > > +		ret = -EINVAL;
> > > +		goto out_unlock;
> > > +	}
> > > +
> > 
> > These two changes make no sense at all. If you look at the function
> > you'll see that "chan" isn't used at all after the check, 
> 
> uhm? it is passed to ieee80211_start_roc_work() right after (this part has not
> been changed).

> 2904         ret = ieee80211_start_roc_work(local, sdata, chan,

No, you can't get there without need_offchan.

johannes

Re: [PATCH 2/2] mac80211: in mgmt_tx use the current channel if none has been specified

[Antonio Quartulli]

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

On Mon, Jun 03, 2013 at 08:16:03PM +0200, Johannes Berg wrote:
> On Mon, 2013-06-03 at 20:03 +0200, Antonio Quartulli wrote:
> > On Mon, Jun 03, 2013 at 05:01:30PM +0200, Johannes Berg wrote:
> > > On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote:
> > > 
> > > > @@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> > > >  		rcu_read_lock();
> > > >  		chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> > > >  
> > > > +		/* if no channel was specified, use the current one */
> > > > +		if (chanctx_conf && !chan)
> > > > +			chan = chanctx_conf->def.chan;
> > > > +
> > > >  		if (chanctx_conf)
> > > >  			need_offchan = chan != chanctx_conf->def.chan;
> > > >  		else
> > > > @@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> > > >  		rcu_read_unlock();
> > > >  	}
> > > >  
> > > > +	/* at this point a channel should have been chosen */
> > > > +	if (!chan) {
> > > > +		ret = -EINVAL;
> > > > +		goto out_unlock;
> > > > +	}
> > > > +
> > > 
> > > These two changes make no sense at all. If you look at the function
> > > you'll see that "chan" isn't used at all after the check, 
> > 
> > uhm? it is passed to ieee80211_start_roc_work() right after (this part has not
> > been changed).
> 
> > 2904         ret = ieee80211_start_roc_work(local, sdata, chan,
> 
> No, you can't get there without need_offchan.

ah, you are right. Now I see why these two changes do not make sense at all :-)

Thanks!

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[Nicolas Cavallari]

On 03/06/2013 08:39, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> Users may want to send a frame on the current channel
> without specifying it.
> 
> Make mgmt_tx pass a NULL channel to mac80211 if none has
> been specified by the user.
> 
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> ---
>  net/wireless/nl80211.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 

Have you tested tracing or ath6kl ?

Re: [PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[Antonio Quartulli]

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

On Mon, Jun 03, 2013 at 04:07:05PM +0200, Nicolas Cavallari wrote:
> On 03/06/2013 08:39, Antonio Quartulli wrote:
> > From: Antonio Quartulli <antonio@open-mesh.com>
> > 
> > Users may want to send a frame on the current channel
> > without specifying it.
> > 
> > Make mgmt_tx pass a NULL channel to mac80211 if none has
> > been specified by the user.
> > 
> > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> > ---
> >  net/wireless/nl80211.c | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> > 
> 
> Have you tested tracing or ath6kl ?

no.
For tracing I think I totally forgot to update the "prototype". For ath6kl I
have no possibility since I have no hw using that driver :/

The concern about ath6kl comes from the fact that it is a fullMAC driver?


Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[Johannes Berg]

On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> Users may want to send a frame on the current channel
> without specifying it.
> 
> Make mgmt_tx pass a NULL channel to mac80211 if none has
> been specified by the user.

cfg80211 isn't just a mac80211 frontend ... ;-)

Also, as Nicolas said, ath6kl_mgmt_tx() will crash after this patch if
it's called in AP mode w/o a channel, so you need to think about that.

Tracing looks fine though.

johannes

Re: [PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[Nicolas Cavallari]

On 03/06/2013 16:59, Johannes Berg wrote:
> On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote:
>> From: Antonio Quartulli <antonio@open-mesh.com>
>>
>> Users may want to send a frame on the current channel
>> without specifying it.
>>
>> Make mgmt_tx pass a NULL channel to mac80211 if none has
>> been specified by the user.
> 
> cfg80211 isn't just a mac80211 frontend ... ;-)
> 
> Also, as Nicolas said, ath6kl_mgmt_tx() will crash after this patch if
> it's called in AP mode w/o a channel, so you need to think about that.

It will crash unconditionally. All ath6kl_mgmt_tx()'s code paths access
chan->center_freq at some point.

ath6kl_mgmt_tx with NULL chan (was Re: [PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency)

[Antonio Quartulli]

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

On Mon, Jun 03, 2013 at 05:04:08PM +0200, Nicolas Cavallari wrote:
> On 03/06/2013 16:59, Johannes Berg wrote:
> > On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote:
> >> From: Antonio Quartulli <antonio@open-mesh.com>
> >>
> >> Users may want to send a frame on the current channel
> >> without specifying it.
> >>
> >> Make mgmt_tx pass a NULL channel to mac80211 if none has
> >> been specified by the user.
> > 
> > cfg80211 isn't just a mac80211 frontend ... ;-)
> > 
> > Also, as Nicolas said, ath6kl_mgmt_tx() will crash after this patch if
> > it's called in AP mode w/o a channel, so you need to think about that.
> 
> It will crash unconditionally. All ath6kl_mgmt_tx()'s code paths access
> chan->center_freq at some point.

Hello Nicolas,
I'm also CCing Kalle Valo since get_maintainer.pl told me he is the guy for
these kind of questions :-)

I'm looking at ath6kl_mgmt_tx() in ath6kl/cfg80211.c and I've seen that the
currently "configured" frequency can be obtained by reading the
ath6kl_vif->ch_hint field.

But, is this correct? I couldn't see any real relation between the ch_hint field
and the real frequency (probably because a lot of logic is hidden to the
driver).
I could only understand that the ch_hint field stores the frequency passed as
parameter during the connection, but I have found no guarantee that this is the
really used one.

Can someone please clarify on this?

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: ath6kl_mgmt_tx with NULL chan

[Kalle Valo]

Antonio Quartulli <ordex@autistici.org> writes:

> On Mon, Jun 03, 2013 at 05:04:08PM +0200, Nicolas Cavallari wrote:
>> On 03/06/2013 16:59, Johannes Berg wrote:
>> > On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote:
>> >> From: Antonio Quartulli <antonio@open-mesh.com>
>> >>
>> >> Users may want to send a frame on the current channel
>> >> without specifying it.
>> >>
>> >> Make mgmt_tx pass a NULL channel to mac80211 if none has
>> >> been specified by the user.
>> > 
>> > cfg80211 isn't just a mac80211 frontend ... ;-)
>> > 
>> > Also, as Nicolas said, ath6kl_mgmt_tx() will crash after this patch if
>> > it's called in AP mode w/o a channel, so you need to think about that.
>> 
>> It will crash unconditionally. All ath6kl_mgmt_tx()'s code paths access
>> chan->center_freq at some point.
>
> Hello Nicolas,
> I'm also CCing Kalle Valo since get_maintainer.pl told me he is the guy for
> these kind of questions :-)
>
> I'm looking at ath6kl_mgmt_tx() in ath6kl/cfg80211.c and I've seen that the
> currently "configured" frequency can be obtained by reading the
> ath6kl_vif->ch_hint field.
>
> But, is this correct?

I did a quick look. To me using ch_hint looks correct.

> I couldn't see any real relation between the ch_hint field and the
> real frequency (probably because a lot of logic is hidden to the
> driver). I could only understand that the ch_hint field stores the
> frequency passed as parameter during the connection, but I have found
> no guarantee that this is the really used one.

Can you be more specific, please?

To me it looks that ch_hint is used both with ath6kl_wmi_reconnect_cmd()
and ath6kl_wmi_connect_cmd() commands, which both are used to connect to
a network. I don't see any other variables used for specifying the
frequency to the firmware. But I could just be blind...

-- 
Kalle Valo

Re: ath6kl_mgmt_tx with NULL chan

[Antonio Quartulli]

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

On Wed, Jun 05, 2013 at 02:57:07AM -0700, Kalle Valo wrote:
> Antonio Quartulli <ordex@autistici.org> writes:
> 
> > On Mon, Jun 03, 2013 at 05:04:08PM +0200, Nicolas Cavallari wrote:
> >> On 03/06/2013 16:59, Johannes Berg wrote:
> >> > On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote:
> >> >> From: Antonio Quartulli <antonio@open-mesh.com>
> >> >>
> >> >> Users may want to send a frame on the current channel
> >> >> without specifying it.
> >> >>
> >> >> Make mgmt_tx pass a NULL channel to mac80211 if none has
> >> >> been specified by the user.
> >> > 
> >> > cfg80211 isn't just a mac80211 frontend ... ;-)
> >> > 
> >> > Also, as Nicolas said, ath6kl_mgmt_tx() will crash after this patch if
> >> > it's called in AP mode w/o a channel, so you need to think about that.
> >> 
> >> It will crash unconditionally. All ath6kl_mgmt_tx()'s code paths access
> >> chan->center_freq at some point.
> >
> > Hello Nicolas,
> > I'm also CCing Kalle Valo since get_maintainer.pl told me he is the guy for
> > these kind of questions :-)
> >
> > I'm looking at ath6kl_mgmt_tx() in ath6kl/cfg80211.c and I've seen that the
> > currently "configured" frequency can be obtained by reading the
> > ath6kl_vif->ch_hint field.
> >
> > But, is this correct?
> 
> I did a quick look. To me using ch_hint looks correct.
> 
> > I couldn't see any real relation between the ch_hint field and the
> > real frequency (probably because a lot of logic is hidden to the
> > driver). I could only understand that the ch_hint field stores the
> > frequency passed as parameter during the connection, but I have found
> > no guarantee that this is the really used one.
> 
> Can you be more specific, please?
> 
> To me it looks that ch_hint is used both with ath6kl_wmi_reconnect_cmd()
> and ath6kl_wmi_connect_cmd() commands, which both are used to connect to
> a network. I don't see any other variables used for specifying the
> frequency to the firmware. But I could just be blind...

I agree with your analysis. My doubt came from the fact that I don't know what
the firmware does and I was wondering whether it could ignore the channel passed
as argument on connect for some reason.

Actually the doubt was raised due to the variable name "ch_HINT".
But you are the ath6k expert :-) Therefore I guess this can work.

Thanks a lot!

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: ath6kl_mgmt_tx with NULL chan

[Kalle Valo]

Antonio Quartulli <antonio@open-mesh.com> writes:

> On Wed, Jun 05, 2013 at 02:57:07AM -0700, Kalle Valo wrote:
>> Antonio Quartulli <ordex@autistici.org> writes:
>> 
>> > I'm looking at ath6kl_mgmt_tx() in ath6kl/cfg80211.c and I've seen that the
>> > currently "configured" frequency can be obtained by reading the
>> > ath6kl_vif->ch_hint field.
>> >
>> > But, is this correct?
>> 
>> I did a quick look. To me using ch_hint looks correct.
>> 
>> > I couldn't see any real relation between the ch_hint field and the
>> > real frequency (probably because a lot of logic is hidden to the
>> > driver). I could only understand that the ch_hint field stores the
>> > frequency passed as parameter during the connection, but I have found
>> > no guarantee that this is the really used one.
>> 
>> Can you be more specific, please?
>> 
>> To me it looks that ch_hint is used both with ath6kl_wmi_reconnect_cmd()
>> and ath6kl_wmi_connect_cmd() commands, which both are used to connect to
>> a network. I don't see any other variables used for specifying the
>> frequency to the firmware. But I could just be blind...
>
> I agree with your analysis. My doubt came from the fact that I don't know what
> the firmware does and I was wondering whether it could ignore the channel passed
> as argument on connect for some reason.

It might do that, I'm not involved with the firmware development.

> Actually the doubt was raised due to the variable name "ch_HINT".

Yeah, the name is really misleading. But that's still legacy from the
pre-cleanup driver, so I wouldn't worry about that too much.

> But you are the ath6k expert :-) Therefore I guess this can work.

It should but you never know :)

But when modifying that code, please add a check to make sure that
channel 0 is not used by accident.

-- 
Kalle Valo

Re: ath6kl_mgmt_tx with NULL chan

[Antonio Quartulli]

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

On Wed, Jun 05, 2013 at 03:15:26AM -0700, Kalle Valo wrote:
> But when modifying that code, please add a check to make sure that
> channel 0 is not used by accident.

Ok, maybe I'll send another patch for this. I don't want to mix a new behaviour
with a fix.


Thanks a lot!


Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[Kalle Valo]

Antonio Quartulli <ordex@autistici.org> writes:

> From: Antonio Quartulli <antonio@open-mesh.com>
>
> Users may want to send a frame on the current channel
> without specifying it.
>
> Make mgmt_tx pass a NULL channel to mac80211 if none has
> been specified by the user.
>
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>

Why? And what users are we talking about here? It would be nice if the
commit log would give some context here for use who nothing about this
patch. "Users may want" is not very informative :)

-- 
Kalle Valo

Re: [PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[Antonio Quartulli]

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

On Wed, Jun 05, 2013 at 02:47:01AM -0700, Kalle Valo wrote:
> Antonio Quartulli <ordex@autistici.org> writes:
> 
> > From: Antonio Quartulli <antonio@open-mesh.com>
> >
> > Users may want to send a frame on the current channel
> > without specifying it.
> >
> > Make mgmt_tx pass a NULL channel to mac80211 if none has
> > been specified by the user.
> >
> > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> 
> Why? And what users are we talking about here? It would be nice if the
> commit log would give some context here for use who nothing about this
> patch. "Users may want" is not very informative :)

Hello Kalle,
thanks for your feedback.

Sure, I can change the commit log.

However, I already wrote a couple of (userspace) applications which wanted to
send a message on the current channel and the only way to do it was to first get
the current channel and then pass it when sending a CMD_FRAME via nl80211. Of
course this approach is just a bit racy :-)

Moreover, I'm currently working on improving IBSS/RSN support in wpa_supplicant
and sending frames on the current channel is needed.

Do you think it is worth mentioning userspace applications like wpa_s in the
kernel commit message?

Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[Kalle Valo]

Antonio Quartulli <antonio@open-mesh.com> writes:

> On Wed, Jun 05, 2013 at 02:47:01AM -0700, Kalle Valo wrote:
>> Antonio Quartulli <ordex@autistici.org> writes:
>> 
>> > From: Antonio Quartulli <antonio@open-mesh.com>
>> >
>> > Users may want to send a frame on the current channel
>> > without specifying it.
>> >
>> > Make mgmt_tx pass a NULL channel to mac80211 if none has
>> > been specified by the user.
>> >
>> > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
>> 
>> Why? And what users are we talking about here? It would be nice if the
>> commit log would give some context here for use who nothing about this
>> patch. "Users may want" is not very informative :)
>
> Hello Kalle,
> thanks for your feedback.
>
> Sure, I can change the commit log.
>
> However, I already wrote a couple of (userspace) applications which wanted to
> send a message on the current channel and the only way to do it was to first get
> the current channel and then pass it when sending a CMD_FRAME via nl80211. Of
> course this approach is just a bit racy :-)
>
> Moreover, I'm currently working on improving IBSS/RSN support in wpa_supplicant
> and sending frames on the current channel is needed.
>
> Do you think it is worth mentioning userspace applications like wpa_s in the
> kernel commit message?

I don't know what others think, but to me it is. It makes it easier to
understand why the change is made. And also do we really need the change
or not.

-- 
Kalle Valo

Re: [PATCH 1/2] nl80211: allow sending CMD_FRAME without specifying any frequency

[Antonio Quartulli]

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

On Wed, Jun 05, 2013 at 03:05:21AM -0700, Kalle Valo wrote:
> Antonio Quartulli <antonio@open-mesh.com> writes:
> 
> > On Wed, Jun 05, 2013 at 02:47:01AM -0700, Kalle Valo wrote:
> >> Antonio Quartulli <ordex@autistici.org> writes:
> >> 
> >> > From: Antonio Quartulli <antonio@open-mesh.com>
> >> >
> >> > Users may want to send a frame on the current channel
> >> > without specifying it.
> >> >
> >> > Make mgmt_tx pass a NULL channel to mac80211 if none has
> >> > been specified by the user.
> >> >
> >> > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> >> 
> >> Why? And what users are we talking about here? It would be nice if the
> >> commit log would give some context here for use who nothing about this
> >> patch. "Users may want" is not very informative :)
> >
> > Hello Kalle,
> > thanks for your feedback.
> >
> > Sure, I can change the commit log.
> >
> > However, I already wrote a couple of (userspace) applications which wanted to
> > send a message on the current channel and the only way to do it was to first get
> > the current channel and then pass it when sending a CMD_FRAME via nl80211. Of
> > course this approach is just a bit racy :-)
> >
> > Moreover, I'm currently working on improving IBSS/RSN support in wpa_supplicant
> > and sending frames on the current channel is needed.
> >
> > Do you think it is worth mentioning userspace applications like wpa_s in the
> > kernel commit message?
> 
> I don't know what others think, but to me it is. It makes it easier to
> understand why the change is made. And also do we really need the change
> or not.

Oky. Then I'll change the commit message and add these details in the next
version.

Thanks!


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]