All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nl80211: allow adding new station to devices in mesh mode
@ 2010-02-05  1:00 Thadeu Lima de Souza Cascardo
  2010-02-05  9:39 ` Johannes Berg
  2010-02-05 16:57 ` Thadeu Lima de Souza Cascardo
  0 siblings, 2 replies; 16+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2010-02-05  1:00 UTC (permalink / raw)
  To: linux-wireless; +Cc: Thadeu Lima de Souza Cascardo

When trying to use the NEW_STATION command, it fails with -EINVAL,
because there's no STA_SUPPORTED_RATES parameter. However, for mesh
mode, this parameter should be NULL. Besides, it requires
LISTEN_INTERVAL to be negative. We allow it to be ommited in the STA
case too, in which it will be -1.

---

Hello.

I was trying to use iw station set in mesh mode and found out I wanted
to call NEW_STATION. I've quickly added a station new command to iw. I
am experimenting with mesh mode with a mac80211 driver. Then, I
realized nl80211_new_station requires STA_SUPPORTED_RATES to be set and,
then, requires it not to be set for mesh mode. I've patched it to
require STA_SUPPORTED_RATES only for AP mode. But now, I get a warning
on rate_lowest_index (from include/net/mac80211.h) and kernel panics
right after that. I've done it on top of latest rc and I am currently
building a version on top of wireless-testing/master.

I've not got any kdump yet, but here is the oops and the patch for iw
too. I've not signed-off the nl80211 patch yet because I want some
feedback and remove this panic.

static int handle_plink_new(struct nl80211_state *state,
                             struct nl_cb *cb,
                             struct nl_msg *msg,
                             int argc, char **argv)
{
       unsigned char mac_addr[ETH_ALEN];

       if (argc < 1)
               return 1;

       if (mac_addr_a2n(mac_addr, argv[0])) {
               fprintf(stderr, "invalid mac address\n");
               return 2;
       }
       NLA_PUT(msg, NL80211_ATTR_MAC, ETH_ALEN, mac_addr);
       return 0;

 nla_put_failure:
       return -ENOBUFS;
}
COMMAND(station, new, "<MAC address>",
       NL80211_CMD_NEW_STATION, 0, CIB_NETDEV, handle_plink_new,
       "Add new plink.");

Feb  4 20:30:56 vespa kernel: [  368.489565] ------------[ cut here ]------------
Feb  4 20:30:56 vespa kernel: [  368.489835] WARNING: at /home/cascardo/linux-2.6/include/net/mac80211.h:2317 minstrel_rate_init+0x63/0x2be [mac80211]()
Feb  4 20:30:56 vespa kernel: [  368.490886] Hardware name: 2658J3U
Feb  4 20:30:56 vespa kernel: [  368.491255] Modules linked in: ath5k mac80211 ath cfg80211 radeon ttm drm_kms_helper drm i2c_algo_bit sco bridge stp llc bnep rfcomm l2cap crc16 bluetooth nsc_ircc ipv6 arc4 ecb snd_intel8x0 snd_intel8x0m snd_ac97_codec thinkpad_acpi ac97_bus rfkill snd_pcm snd_seq snd_timer snd_seq_device pcmcia mousedev snd led_class soundcore video 8250_pnp irda rtc_cmos processor yenta_socket psmouse crc_ccitt snd_page_alloc rtc_core 8250 button battery ac nvram parport_pc rsrc_nonstatic serio_raw i2c_i801 rtc_lib serial_core parport pcspkr pcmcia_core output evdev i2c_core ext3 jbd mbcache dm_mod sg sr_mod cdrom sd_mod crc_t10dif ata_generic uhci_hcd ata_piix libata ehci_hcd e100 intel_agp mii scsi_mod usbcore nls_base agpgart thermal fan thermal_sys hwmon unix [last unloaded: cfg80211]
Feb  4 20:30:56 vespa kernel: [  368.504638] Pid: 1493, comm: iw Not tainted 2.6.33-rc6-00007-g1c67e56a #11
Feb  4 20:30:56 vespa kernel: [  368.504874] Call Trace:
Feb  4 20:30:56 vespa kernel: [  368.505676]  [<c102d882>] warn_slowpath_common+0x6a/0x81
Feb  4 20:30:56 vespa kernel: [  368.514861]  [<f86f1182>] ? minstrel_rate_init+0x63/0x2be [mac80211]
Feb  4 20:30:56 vespa kernel: [  368.524272]  [<c102d8ab>] warn_slowpath_null+0x12/0x15
Feb  4 20:30:56 vespa kernel: [  368.533846]  [<f86f1182>] minstrel_rate_init+0x63/0x2be [mac80211]
Feb  4 20:30:56 vespa kernel: [  368.543567]  [<c10333f7>] ? local_bh_enable+0x10/0x12
Feb  4 20:30:56 vespa kernel: [  368.553209]  [<f86ccd69>] ? sta_info_alloc+0x150/0x15d [mac80211]
Feb  4 20:30:56 vespa kernel: [  368.562871]  [<f86d8ef5>] ieee80211_add_station+0xe3/0x125 [mac80211]
Feb  4 20:30:56 vespa kernel: [  368.572431]  [<f86d8e12>] ? ieee80211_add_station+0x0/0x125 [mac80211]
Feb  4 20:30:56 vespa kernel: [  368.582076]  [<f814230f>] nl80211_new_station+0x164/0x1a2 [cfg80211]
Feb  4 20:30:56 vespa kernel: [  368.591780]  [<c11eab3c>] genl_rcv_msg+0x1a7/0x1c4
Feb  4 20:30:56 vespa kernel: [  368.601294]  [<c11ea995>] ? genl_rcv_msg+0x0/0x1c4
Feb  4 20:30:56 vespa kernel: [  368.610600]  [<c11e9c8d>] netlink_rcv_skb+0x35/0x7b
Feb  4 20:30:56 vespa kernel: [  368.619710]  [<c11ea98e>] genl_rcv+0x20/0x27
Feb  4 20:30:56 vespa kernel: [  368.628538]  [<c11e9abc>] netlink_unicast+0xeb/0x148
Feb  4 20:30:56 vespa kernel: [  368.637372]  [<c11ea1b8>] netlink_sendmsg+0x216/0x223
Feb  4 20:30:56 vespa kernel: [  368.646265]  [<c10c9392>] ? path_to_nameidata+0x16/0x2f
Feb  4 20:30:56 vespa kernel: [  368.655292]  [<c11c4a85>] __sock_sendmsg+0x4a/0x53
Feb  4 20:30:56 vespa kernel: [  368.664485]  [<c11c4d42>] sock_sendmsg+0x97/0xab
Feb  4 20:30:56 vespa kernel: [  368.673372]  [<c10f85e3>] ? proc_alloc_inode+0x1b/0x6a
Feb  4 20:30:56 vespa kernel: [  368.682160]  [<c1113d8b>] ? security_inode_alloc+0x1e/0x20
Feb  4 20:30:56 vespa kernel: [  368.690878]  [<c12368d7>] ? add_preempt_count+0x9f/0xa2
Feb  4 20:30:56 vespa kernel: [  368.699426]  [<c123682b>] ? sub_preempt_count+0x84/0x91
Feb  4 20:30:56 vespa kernel: [  368.707818]  [<c101e0d4>] ? kunmap_atomic+0x86/0x9d
Feb  4 20:30:56 vespa kernel: [  368.716232]  [<c11cd165>] ? copy_from_user+0xd/0xf
Feb  4 20:30:56 vespa kernel: [  368.724662]  [<c11cd478>] ? verify_iovec+0x43/0x6f
Feb  4 20:30:56 vespa kernel: [  368.732805]  [<c11c4f79>] sys_sendmsg+0x18c/0x1f0
Feb  4 20:30:56 vespa kernel: [  368.740813]  [<c11d4bf6>] ? dev_ioctl+0x524/0x577
Feb  4 20:30:56 vespa kernel: [  368.748870]  [<c12367b0>] ? sub_preempt_count+0x9/0x91
Feb  4 20:30:56 vespa kernel: [  368.756850]  [<c123682b>] ? sub_preempt_count+0x84/0x91
Feb  4 20:30:56 vespa kernel: [  368.764613]  [<c123682b>] ? sub_preempt_count+0x84/0x91
Feb  4 20:30:56 vespa kernel: [  368.772215]  [<c1094678>] ? unlock_page+0x42/0x45
Feb  4 20:30:56 vespa kernel: [  368.779676]  [<c10a6d93>] ? __do_fault+0x31d/0x34d
Feb  4 20:30:56 vespa kernel: [  368.787145]  [<c10a869d>] ? handle_mm_fault+0x380/0x7d7
Feb  4 20:30:56 vespa kernel: [  368.794377]  [<c10d5731>] ? mntput_no_expire+0x1e/0x8c
Feb  4 20:30:56 vespa kernel: [  368.801584]  [<c11c6a87>] sys_socketcall+0x163/0x1aa
Feb  4 20:30:56 vespa kernel: [  368.808628]  [<c113d5f4>] ? trace_hardirqs_on_thunk+0xc/0x10
Feb  4 20:30:56 vespa kernel: [  368.815669]  [<c100329f>] sysenter_do_call+0x12/0x2d
Feb  4 20:30:56 vespa kernel: [  368.822608] ---[ end trace 5ab7f913a209c320 ]---

---
 net/wireless/nl80211.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5b79ecf..43e0137 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2000,23 +2000,23 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	u8 *mac_addr = NULL;
 
 	memset(&params, 0, sizeof(params));
+	params.listen_interval = -1;
 
 	if (!info->attrs[NL80211_ATTR_MAC])
 		return -EINVAL;
 
-	if (!info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL])
-		return -EINVAL;
+	mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
 
-	if (!info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES])
-		return -EINVAL;
+	if (info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES]) {
+		params.supported_rates =
+			nla_data(info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES]);
+		params.supported_rates_len =
+			nla_len(info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES]);
+	}
 
-	mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
-	params.supported_rates =
-		nla_data(info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES]);
-	params.supported_rates_len =
-		nla_len(info->attrs[NL80211_ATTR_STA_SUPPORTED_RATES]);
-	params.listen_interval =
-		nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
+	if (info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL])
+		params.listen_interval =
+		    nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
 
 	if (info->attrs[NL80211_ATTR_STA_AID]) {
 		params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]);
@@ -2050,6 +2050,8 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 		/* all ok but must have AID */
 		if (!params.aid)
 			err = -EINVAL;
+		if (!params.supported_rates)
+			err = -EINVAL;
 		break;
 	case NL80211_IFTYPE_MESH_POINT:
 		/* disallow things mesh doesn't support */
-- 
1.6.6.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-05  1:00 [PATCH] nl80211: allow adding new station to devices in mesh mode Thadeu Lima de Souza Cascardo
@ 2010-02-05  9:39 ` Johannes Berg
  2010-02-05  9:59   ` Johannes Berg
  2010-02-05 16:57 ` Thadeu Lima de Souza Cascardo
  1 sibling, 1 reply; 16+ messages in thread
From: Johannes Berg @ 2010-02-05  9:39 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo; +Cc: linux-wireless

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

On Thu, 2010-02-04 at 23:00 -0200, Thadeu Lima de Souza Cascardo wrote:
> When trying to use the NEW_STATION command, it fails with -EINVAL,
> because there's no STA_SUPPORTED_RATES parameter. However, for mesh
> mode, this parameter should be NULL. Besides, it requires
> LISTEN_INTERVAL to be negative. We allow it to be ommited in the STA
> case too, in which it will be -1.
> 
> ---
> 
> Hello.
> 
> I was trying to use iw station set in mesh mode and found out I wanted
> to call NEW_STATION.

Hmm. Can you explain why you want to do that? Mesh usually discovers its
peers, and you can add paths, but peers doesn't make a whole lot of
sense to me?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-05  9:39 ` Johannes Berg
@ 2010-02-05  9:59   ` Johannes Berg
  0 siblings, 0 replies; 16+ messages in thread
From: Johannes Berg @ 2010-02-05  9:59 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo; +Cc: linux-wireless, Andrey Yurovsky

On Fri, 2010-02-05 at 10:39 +0100, Johannes Berg wrote:
> On Thu, 2010-02-04 at 23:00 -0200, Thadeu Lima de Souza Cascardo wrote:
> > When trying to use the NEW_STATION command, it fails with -EINVAL,
> > because there's no STA_SUPPORTED_RATES parameter. However, for mesh
> > mode, this parameter should be NULL. Besides, it requires
> > LISTEN_INTERVAL to be negative. We allow it to be ommited in the STA
> > case too, in which it will be -1.
> > 
> > ---
> > 
> > Hello.
> > 
> > I was trying to use iw station set in mesh mode and found out I wanted
> > to call NEW_STATION.
> 
> Hmm. Can you explain why you want to do that? Mesh usually discovers its
> peers, and you can add paths, but peers doesn't make a whole lot of
> sense to me?

So it's allowed due to commit 155cc9e4b1d60161ee53ffaf2c15b9411f086fa7,
but I don't understand why. Andrey?

johannes


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-05  1:00 [PATCH] nl80211: allow adding new station to devices in mesh mode Thadeu Lima de Souza Cascardo
  2010-02-05  9:39 ` Johannes Berg
@ 2010-02-05 16:57 ` Thadeu Lima de Souza Cascardo
  2010-02-05 17:47   ` Andrey Yurovsky
  1 sibling, 1 reply; 16+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2010-02-05 16:57 UTC (permalink / raw)
  To: linux-wireless; +Cc: johannes, andrey

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

On Thu, Feb 04, 2010 at 11:00:42PM -0200, Thadeu Lima de Souza Cascardo wrote:
> Hello.
> 
> I was trying to use iw station set in mesh mode and found out I wanted
> to call NEW_STATION. I've quickly added a station new command to iw. I
> am experimenting with mesh mode with a mac80211 driver. Then, I
> realized nl80211_new_station requires STA_SUPPORTED_RATES to be set and,
> then, requires it not to be set for mesh mode. I've patched it to
> require STA_SUPPORTED_RATES only for AP mode. But now, I get a warning
> on rate_lowest_index (from include/net/mac80211.h) and kernel panics
> right after that. I've done it on top of latest rc and I am currently
> building a version on top of wireless-testing/master.
> 

It warns and panics in wireless-testing/master too.

However, I did found out that it does not panic when using PID rate
control instead of Minstreal. The same warning still occurs, since both
minstreal and PID rate_init functions call rate_lowest_index, which is
the responsible inline function for the WARNING.

After doing the "station new" command, I can do a "station dump" and get
my newly added station. So, I think this is ready for commit. And then,
we need to sort out both the WARNING and the panic for the Minstrel in
other patches too.

I'll resend a signed-off patch soon. Just wait a little for Andrey's
feedback.

Regards,
Cascardo.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-05 16:57 ` Thadeu Lima de Souza Cascardo
@ 2010-02-05 17:47   ` Andrey Yurovsky
  2010-02-05 17:55     ` Thadeu Lima de Souza Cascardo
  2010-02-06  8:59     ` Johannes Berg
  0 siblings, 2 replies; 16+ messages in thread
From: Andrey Yurovsky @ 2010-02-05 17:47 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo; +Cc: linux-wireless, johannes

Hi,

On Fri, Feb 5, 2010 at 8:57 AM, Thadeu Lima de Souza Cascardo
<cascardo@holoscopio.com> wrote:
> After doing the "station new" command, I can do a "station dump" and get
> my newly added station. So, I think this is ready for commit. And then,
> we need to sort out both the WARNING and the panic for the Minstrel in
> other patches too.

Just to be clear, what are you trying to do here?  Are you manually
populating the peer links (stations) list?  If so, why?  Thanks,

  -Andrey

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-05 17:47   ` Andrey Yurovsky
@ 2010-02-05 17:55     ` Thadeu Lima de Souza Cascardo
  2010-02-05 19:04       ` Simon Raffeiner
  2010-02-06  8:59     ` Johannes Berg
  1 sibling, 1 reply; 16+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2010-02-05 17:55 UTC (permalink / raw)
  To: Andrey Yurovsky; +Cc: linux-wireless, johannes

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

On Fri, Feb 05, 2010 at 09:47:44AM -0800, Andrey Yurovsky wrote:
> Hi,
> 
> On Fri, Feb 5, 2010 at 8:57 AM, Thadeu Lima de Souza Cascardo
> <cascardo@holoscopio.com> wrote:
> > After doing the "station new" command, I can do a "station dump" and get
> > my newly added station. So, I think this is ready for commit. And then,
> > we need to sort out both the WARNING and the panic for the Minstrel in
> > other patches too.
> 
> Just to be clear, what are you trying to do here?  Are you manually
> populating the peer links (stations) list?  If so, why?  Thanks,
> 
>   -Andrey

I did not get automatic plinks for a peer. So, I decided to look at the
code and see when they got added to the sta list. It does when it
receives some management frames and also when it's added manually with
the NEW_STATION nl80211 command.

I want automatic plinks to work. But since I've started to play with it,
why not make the command just work. It's there. If adding plinks
manually to mesh nodes is not permitted, I may just send another patch,
disallowing it entirely and making it explicit in code, comment and git
log.

Did you have anything in mind for the use of this command to mesh mode?
One use case I can think of is to set the block action before the node
appears nearby (since station set will not work without the plink).

Regards,
Cascardo.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-05 17:55     ` Thadeu Lima de Souza Cascardo
@ 2010-02-05 19:04       ` Simon Raffeiner
  2010-02-05 23:16         ` Andrey Yurovsky
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Raffeiner @ 2010-02-05 19:04 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo; +Cc: Andrey Yurovsky, linux-wireless, johannes

Am Freitag, 5. Februar 2010 18:55:56 schrieb Thadeu Lima de Souza Cascardo:

> I did not get automatic plinks for a peer.

The last time that happened I was using different checkouts of wireless-testing 
on both nodes and somebody had changed the code to match a new draft version 
in between.

And the one time before that there was a bug in the ath5k code, effectively 
preventing the device from beaconing.


> I want automatic plinks to work. But since I've started to play with it,
> why not make the command just work. It's there. If adding plinks
> manually to mesh nodes is not permitted, I may just send another patch,
> disallowing it entirely and making it explicit in code, comment and git
> log.

If I'm not mistaken the draft standard says that every node has to actively or 
passively scan for neighbors and automatically select candidate peers. A 
candidate peer has to announce a profile (Mesh Information Element) in its 
beacons that matches the local profile.

If manual addition of plinks is allowed you could theoretically force your 
node to peer with a neighbor running a different profile, e.g. using a different 
path selection or authentication algorithm, or an implementation based on a 
different draft version.

I can't really think of an use for it.

Just my 0.01$.

regards
Simon Raffeiner

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-05 19:04       ` Simon Raffeiner
@ 2010-02-05 23:16         ` Andrey Yurovsky
  0 siblings, 0 replies; 16+ messages in thread
From: Andrey Yurovsky @ 2010-02-05 23:16 UTC (permalink / raw)
  To: Simon Raffeiner; +Cc: Thadeu Lima de Souza Cascardo, linux-wireless, johannes

On Fri, Feb 5, 2010 at 11:04 AM, Simon Raffeiner
<sturmflut@lieberbiber.de> wrote:
> If manual addition of plinks is allowed you could theoretically force your
> node to peer with a neighbor running a different profile, e.g. using a different
> path selection or authentication algorithm, or an implementation based on a
> different draft version.
>
> I can't really think of an use for it.

Right, in your case please ensure that your devices are beaconing
(that's the most common problem) before proceeding.  plinks are
established automatically and as far as I know that's the only thing
we need to support at this time.  If your device isn't beaconing, then
that's a bug and it should be fixed.  That said, ath5k beaconed
correctly last time I tried it.

  -Andrey

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-05 17:47   ` Andrey Yurovsky
  2010-02-05 17:55     ` Thadeu Lima de Souza Cascardo
@ 2010-02-06  8:59     ` Johannes Berg
  2010-02-07 19:51       ` Andrey Yurovsky
  1 sibling, 1 reply; 16+ messages in thread
From: Johannes Berg @ 2010-02-06  8:59 UTC (permalink / raw)
  To: Andrey Yurovsky; +Cc: Thadeu Lima de Souza Cascardo, linux-wireless

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

On Fri, 2010-02-05 at 09:47 -0800, Andrey Yurovsky wrote:
> Hi,
> 
> On Fri, Feb 5, 2010 at 8:57 AM, Thadeu Lima de Souza Cascardo
> <cascardo@holoscopio.com> wrote:
> > After doing the "station new" command, I can do a "station dump" and get
> > my newly added station. So, I think this is ready for commit. And then,
> > we need to sort out both the WARNING and the panic for the Minstrel in
> > other patches too.
> 
> Just to be clear, what are you trying to do here?  Are you manually
> populating the peer links (stations) list?  If so, why?  Thanks,

Andrey, thanks for chiming in!

I do wonder though, the commit (155cc9e4b1d60161ee53) I referenced was
yours, and now you're asking the same question as I am -- "why even
allow adding stations?"

Should we just refuse it (again)? Clearly it would be safer than the
current code that's prone to crashes.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-06  8:59     ` Johannes Berg
@ 2010-02-07 19:51       ` Andrey Yurovsky
  2010-02-08  9:08         ` Johannes Berg
  2010-02-08 13:54         ` Simon Raffeiner
  0 siblings, 2 replies; 16+ messages in thread
From: Andrey Yurovsky @ 2010-02-07 19:51 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Thadeu Lima de Souza Cascardo, linux-wireless, Javier Cardona

On Sat, Feb 6, 2010 at 12:59 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> I do wonder though, the commit (155cc9e4b1d60161ee53) I referenced was
> yours, and now you're asking the same question as I am -- "why even
> allow adding stations?"

It was supported for automated testing and experimentation (you could
then force certain topologies).  I can't think of any other reason to
do it.  I suppose that the same thing goes for mpaths and the mesh
portal tables.

  -Andrey

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-07 19:51       ` Andrey Yurovsky
@ 2010-02-08  9:08         ` Johannes Berg
  2010-02-08 19:25           ` Javier Cardona
  2010-02-08 13:54         ` Simon Raffeiner
  1 sibling, 1 reply; 16+ messages in thread
From: Johannes Berg @ 2010-02-08  9:08 UTC (permalink / raw)
  To: Andrey Yurovsky
  Cc: Thadeu Lima de Souza Cascardo, linux-wireless, Javier Cardona

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

On Sun, 2010-02-07 at 11:51 -0800, Andrey Yurovsky wrote:
> On Sat, Feb 6, 2010 at 12:59 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > I do wonder though, the commit (155cc9e4b1d60161ee53) I referenced was
> > yours, and now you're asking the same question as I am -- "why even
> > allow adding stations?"
> 
> It was supported for automated testing and experimentation (you could
> then force certain topologies).  I can't think of any other reason to
> do it.  I suppose that the same thing goes for mpaths and the mesh
> portal tables.

So would you prefer to fix it (however we would do that, I don't think
that the patch here is correct since it leaves the station struct with
no usable rates afaict), or just disallow it?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-07 19:51       ` Andrey Yurovsky
  2010-02-08  9:08         ` Johannes Berg
@ 2010-02-08 13:54         ` Simon Raffeiner
  2010-02-08 17:26           ` Javier Cardona
  1 sibling, 1 reply; 16+ messages in thread
From: Simon Raffeiner @ 2010-02-08 13:54 UTC (permalink / raw)
  To: Andrey Yurovsky
  Cc: Johannes Berg, Thadeu Lima de Souza Cascardo, linux-wireless,
	Javier Cardona

Am Sonntag 07 Februar 2010 08:51:03 pm schrieben Sie:

> It was supported for automated testing and experimentation (you could
> then force certain topologies). 

Wouldn't one have to disable beaconing then, or at least automatic peering? IMO you can't force a topology if the nodes are still beaconing and establishing paths at the same time.

regards
Simon Raffeiner

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-08 13:54         ` Simon Raffeiner
@ 2010-02-08 17:26           ` Javier Cardona
  0 siblings, 0 replies; 16+ messages in thread
From: Javier Cardona @ 2010-02-08 17:26 UTC (permalink / raw)
  To: Simon Raffeiner
  Cc: Andrey Yurovsky, Johannes Berg, Thadeu Lima de Souza Cascardo,
	linux-wireless

Simon,

On Mon, Feb 8, 2010 at 5:54 AM, Simon Raffeiner
<sturmflut@lieberbiber.de> wrote:
> Am Sonntag 07 Februar 2010 08:51:03 pm schrieben Sie:
>
>> It was supported for automated testing and experimentation (you could
>> then force certain topologies).
>
> Wouldn't one have to disable beaconing then, or at least automatic peering? IMO you can't force a topology if the nodes are still beaconing and establishing paths at the same time.

Automatic peering can be disabled via NL80211_MESHCONF_AUTO_OPEN_PLINKS.

Javier


-- 
Javier Cardona
cozybit Inc.
http://www.cozybit.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-08  9:08         ` Johannes Berg
@ 2010-02-08 19:25           ` Javier Cardona
  2010-02-08 20:26             ` Thadeu Lima de Souza Cascardo
  0 siblings, 1 reply; 16+ messages in thread
From: Javier Cardona @ 2010-02-08 19:25 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Thadeu Lima de Souza Cascardo, linux-wireless, Andrey Yurovsky

Thadeu, Johannes,

On Mon, Feb 8, 2010 at 1:08 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Sun, 2010-02-07 at 11:51 -0800, Andrey Yurovsky wrote:
>> On Sat, Feb 6, 2010 at 12:59 AM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>> > I do wonder though, the commit (155cc9e4b1d60161ee53) I referenced was
>> > yours, and now you're asking the same question as I am -- "why even
>> > allow adding stations?"
>>
>> It was supported for automated testing and experimentation (you could
>> then force certain topologies).  I can't think of any other reason to
>> do it.  I suppose that the same thing goes for mpaths and the mesh
>> portal tables.
>
> So would you prefer to fix it (however we would do that, I don't think
> that the patch here is correct since it leaves the station struct with
> no usable rates afaict), or just disallow it?

After reviewing our mesh test suite, I would vote for disallowing mesh
point addition via iw.  We currently force topologies by 1. turning
auto peering off, 2. waiting for neighbors to be discovered and 3.
controlling peering with 'iw ... station set <MAC address>
plink_action <open|block>'

Thadeu, would this approach work for you?

Javier



-- 
Javier Cardona
cozybit Inc.
http://www.cozybit.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-08 19:25           ` Javier Cardona
@ 2010-02-08 20:26             ` Thadeu Lima de Souza Cascardo
  2010-02-09  7:58               ` Johannes Berg
  0 siblings, 1 reply; 16+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2010-02-08 20:26 UTC (permalink / raw)
  To: Javier Cardona; +Cc: Johannes Berg, linux-wireless, Andrey Yurovsky

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

On Mon, Feb 08, 2010 at 11:25:37AM -0800, Javier Cardona wrote:
> Thadeu, Johannes,
> 
> On Mon, Feb 8, 2010 at 1:08 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Sun, 2010-02-07 at 11:51 -0800, Andrey Yurovsky wrote:
> >> On Sat, Feb 6, 2010 at 12:59 AM, Johannes Berg
> >> <johannes@sipsolutions.net> wrote:
> >> > I do wonder though, the commit (155cc9e4b1d60161ee53) I referenced was
> >> > yours, and now you're asking the same question as I am -- "why even
> >> > allow adding stations?"
> >>
> >> It was supported for automated testing and experimentation (you could
> >> then force certain topologies).  I can't think of any other reason to
> >> do it.  I suppose that the same thing goes for mpaths and the mesh
> >> portal tables.
> >
> > So would you prefer to fix it (however we would do that, I don't think
> > that the patch here is correct since it leaves the station struct with
> > no usable rates afaict), or just disallow it?
> 
> After reviewing our mesh test suite, I would vote for disallowing mesh
> point addition via iw.  We currently force topologies by 1. turning
> auto peering off, 2. waiting for neighbors to be discovered and 3.
> controlling peering with 'iw ... station set <MAC address>
> plink_action <open|block>'
> 
> Thadeu, would this approach work for you?
> 

That's OK for me. Do you want a patch?

> Javier
> 
> 
> 
> -- 
> Javier Cardona
> cozybit Inc.
> http://www.cozybit.com

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] nl80211: allow adding new station to devices in mesh mode
  2010-02-08 20:26             ` Thadeu Lima de Souza Cascardo
@ 2010-02-09  7:58               ` Johannes Berg
  0 siblings, 0 replies; 16+ messages in thread
From: Johannes Berg @ 2010-02-09  7:58 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo
  Cc: Javier Cardona, linux-wireless, Andrey Yurovsky

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

On Mon, 2010-02-08 at 18:26 -0200, Thadeu Lima de Souza Cascardo wrote:

> > After reviewing our mesh test suite, I would vote for disallowing mesh
> > point addition via iw.  We currently force topologies by 1. turning
> > auto peering off, 2. waiting for neighbors to be discovered and 3.
> > controlling peering with 'iw ... station set <MAC address>
> > plink_action <open|block>'
> > 
> > Thadeu, would this approach work for you?

Javier, thanks for taking a look.

> That's OK for me. Do you want a patch?

Please. Just remove the relevant cases from net/wireless/nl80211.c.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2010-02-09  7:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-02-05  1:00 [PATCH] nl80211: allow adding new station to devices in mesh mode Thadeu Lima de Souza Cascardo
2010-02-05  9:39 ` Johannes Berg
2010-02-05  9:59   ` Johannes Berg
2010-02-05 16:57 ` Thadeu Lima de Souza Cascardo
2010-02-05 17:47   ` Andrey Yurovsky
2010-02-05 17:55     ` Thadeu Lima de Souza Cascardo
2010-02-05 19:04       ` Simon Raffeiner
2010-02-05 23:16         ` Andrey Yurovsky
2010-02-06  8:59     ` Johannes Berg
2010-02-07 19:51       ` Andrey Yurovsky
2010-02-08  9:08         ` Johannes Berg
2010-02-08 19:25           ` Javier Cardona
2010-02-08 20:26             ` Thadeu Lima de Souza Cascardo
2010-02-09  7:58               ` Johannes Berg
2010-02-08 13:54         ` Simon Raffeiner
2010-02-08 17:26           ` Javier Cardona

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.