[PATCH] nl80211: Provide TDLS link state

[PATCH] nl80211: Provide TDLS link state

[Paul Stewart]

Provide a method to query TDLS state in drivers that use the
NL80211_CMD_TDLS_OPER method for link setup.

Signed-off-by: Paul Stewart <pstew@chromium.org>
---
 include/uapi/linux/nl80211.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 9922b9b..4f163f9 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3866,6 +3866,7 @@ enum nl80211_pmksa_candidate_attr {
  * @NL80211_TDLS_TEARDOWN: Teardown a TDLS link which is already established
  * @NL80211_TDLS_ENABLE_LINK: Enable TDLS link
  * @NL80211_TDLS_DISABLE_LINK: Disable TDLS link
+ * @NL80211_TDLS_QUERY_LINK: Query TDLS link status
  */
 enum nl80211_tdls_operation {
 	NL80211_TDLS_DISCOVERY_REQ,
@@ -3873,6 +3874,19 @@ enum nl80211_tdls_operation {
 	NL80211_TDLS_TEARDOWN,
 	NL80211_TDLS_ENABLE_LINK,
 	NL80211_TDLS_DISABLE_LINK,
+	NL80211_TDLS_QUERY_LINK,
+};
+
+/**
+ * enum nl80211_tdls_link_state - values returned fo %NL80211_TDLS_QUERY_LINK
+ * @NL80211_TDLS_LINK_STATE_UNKNOWN: Nothing is known about this peer
+ * @NL80211_TDLS_LINK_STATE_UNCONNECTED: TDLS link is not setup to peer
+ * @NL80211_TDLS_LINK_STATE_CONNECTED: TDLS link is setup to peer
+ */
+enum nl80211_tdls_link_state {
+	NL80211_TDLS_LINK_STATE_UNKNOWN,
+	NL80211_TDLS_LINK_STATE_UNCONNECTED,
+	NL80211_TDLS_LINK_STATE_CONNECTED,
 };
 
 /*
-- 
1.9.1.423.g4596e3a

Re: [PATCH] nl80211: Provide TDLS link state

[Luca Coelho]

Hi Paul,

On Tue, 2014-05-20 at 13:27 -0700, Paul Stewart wrote:
> Provide a method to query TDLS state in drivers that use the
> NL80211_CMD_TDLS_OPER method for link setup.
> 
> Signed-off-by: Paul Stewart <pstew@chromium.org>
> ---
>  include/uapi/linux/nl80211.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 9922b9b..4f163f9 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -3866,6 +3866,7 @@ enum nl80211_pmksa_candidate_attr {
>   * @NL80211_TDLS_TEARDOWN: Teardown a TDLS link which is already established
>   * @NL80211_TDLS_ENABLE_LINK: Enable TDLS link
>   * @NL80211_TDLS_DISABLE_LINK: Disable TDLS link
> + * @NL80211_TDLS_QUERY_LINK: Query TDLS link status
>   */
>  enum nl80211_tdls_operation {
>  	NL80211_TDLS_DISCOVERY_REQ,
> @@ -3873,6 +3874,19 @@ enum nl80211_tdls_operation {
>  	NL80211_TDLS_TEARDOWN,
>  	NL80211_TDLS_ENABLE_LINK,
>  	NL80211_TDLS_DISABLE_LINK,
> +	NL80211_TDLS_QUERY_LINK,
> +};
> +
> +/**
> + * enum nl80211_tdls_link_state - values returned fo %NL80211_TDLS_QUERY_LINK

Small typo here.  s/fo/for/ ?

--
Luca.

[PATCHv2] nl80211: Provide TDLS link state

[Paul Stewart]

Provide a method to query TDLS state in drivers that use the
NL80211_CMD_TDLS_OPER method for link setup.

Signed-off-by: Paul Stewart <pstew@chromium.org>
---
 include/uapi/linux/nl80211.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 9922b9b..44aca23 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3866,6 +3866,7 @@ enum nl80211_pmksa_candidate_attr {
  * @NL80211_TDLS_TEARDOWN: Teardown a TDLS link which is already established
  * @NL80211_TDLS_ENABLE_LINK: Enable TDLS link
  * @NL80211_TDLS_DISABLE_LINK: Disable TDLS link
+ * @NL80211_TDLS_QUERY_LINK: Query TDLS link status
  */
 enum nl80211_tdls_operation {
 	NL80211_TDLS_DISCOVERY_REQ,
@@ -3873,6 +3874,19 @@ enum nl80211_tdls_operation {
 	NL80211_TDLS_TEARDOWN,
 	NL80211_TDLS_ENABLE_LINK,
 	NL80211_TDLS_DISABLE_LINK,
+	NL80211_TDLS_QUERY_LINK,
+};
+
+/**
+ * enum nl80211_tdls_link_state - values returned for %NL80211_TDLS_QUERY_LINK
+ * @NL80211_TDLS_LINK_STATE_UNKNOWN: Nothing is known about this peer
+ * @NL80211_TDLS_LINK_STATE_UNCONNECTED: TDLS link is not setup to peer
+ * @NL80211_TDLS_LINK_STATE_CONNECTED: TDLS link is setup to peer
+ */
+enum nl80211_tdls_link_state {
+	NL80211_TDLS_LINK_STATE_UNKNOWN,
+	NL80211_TDLS_LINK_STATE_UNCONNECTED,
+	NL80211_TDLS_LINK_STATE_CONNECTED,
 };
 
 /*
-- 
1.9.1.423.g4596e3a

Re: [PATCHv2] nl80211: Provide TDLS link state

[Johannes Berg]

On Tue, 2014-05-20 at 13:27 -0700, Paul Stewart wrote:
> Provide a method to query TDLS state in drivers that use the
> NL80211_CMD_TDLS_OPER method for link setup.

Apart from the fact that your patch isn't very clear on how this was
intended to be used (some return value?) I don't really see why even
such a driver couldn't implement station operations
(get_station/dump_station) and get the same API for this that mac80211
already implements.

iw wlan0 station dump gives me
Station 02:00:00:00:01:00 (on wlan0)
	inactive time:	450 ms
	rx bytes:	3068
	rx packets:	2
	tx bytes:	3036
	tx packets:	2
	tx retries:	0
	tx failed:	0
	signal:  	-30 dBm
	signal avg:	-30 dBm
	tx bitrate:	24.0 MBit/s
	rx bitrate:	12.0 MBit/s
	authorized:	yes
	authenticated:	yes
	preamble:	long
	WMM/WME:	no
	MFP:		no
	TDLS peer:	yes
[...]


johannes

Re: [PATCHv2] nl80211: Provide TDLS link state

[Paul Stewart]

On Wed, May 21, 2014 at 3:41 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> On Tue, 2014-05-20 at 13:27 -0700, Paul Stewart wrote:
> > Provide a method to query TDLS state in drivers that use the
> > NL80211_CMD_TDLS_OPER method for link setup.
>
> Apart from the fact that your patch isn't very clear on how this was
> intended to be used (some return value?) I don't really see why even
> such a driver couldn't implement station operations
> (get_station/dump_station) and get the same API for this that mac80211
> already implements.


 I still think this might be too heavyweight in situations where the
firmware handles TDLS links. Think about a system where the firmware
is completely responsible for terminating incoming TDLS requests and
offers no facility to notify the kernel as the TDLS link (initiated
remotely) was established.  Unless the driver polls the firmware for
ALL MAC addresses, there's no way for a station entry to magically
appear.  I wan this method so we can directly ask the firmware "hey,
is there a TDLS link to 02:00:00:00:01:00?"  This pre-supposes that
user-space has some knowledge about this remote host (suppose we're
streaming media to it) and we want to know whether in this case, for
diagnostic reasons, whether TDLS was established between them at this
time.
>
>
> iw wlan0 station dump gives me
> Station 02:00:00:00:01:00 (on wlan0)
>         inactive time:  450 ms
>         rx bytes:       3068
>         rx packets:     2
>         tx bytes:       3036
>         tx packets:     2
>         tx retries:     0
>         tx failed:      0
>         signal:         -30 dBm
>         signal avg:     -30 dBm
>         tx bitrate:     24.0 MBit/s
>         rx bitrate:     12.0 MBit/s
>         authorized:     yes
>         authenticated:  yes
>         preamble:       long
>         WMM/WME:        no
>         MFP:            no
>         TDLS peer:      yes
> [...]
>
>
> johannes
>

Re: [PATCHv2] nl80211: Provide TDLS link state

[Johannes Berg]

On Wed, 2014-05-21 at 09:17 -0700, Paul Stewart wrote:

>  I still think this might be too heavyweight in situations where the
> firmware handles TDLS links. Think about a system where the firmware
> is completely responsible for terminating incoming TDLS requests and
> offers no facility to notify the kernel as the TDLS link (initiated
> remotely) was established.

Is it really that bad to demand the firmware notify the host? It's not
like this is a very frequent event, and if querying is supported then
notification can't be all that much more difficult.

We're not in a great position to demand that upstream, I guess, it
should be easier for you :-)

If it's really such a big problem, in theory one could support
get_station() (which gives you a MAC address) and not dump_station(). I
wouldn't necessarily recommend it, but it does seem possible.


> Unless the driver polls the firmware for
> ALL MAC addresses, there's no way for a station entry to magically
> appear.  I wan this method so we can directly ask the firmware "hey,
> is there a TDLS link to 02:00:00:00:01:00?"  This pre-supposes that
> user-space has some knowledge about this remote host (suppose we're
> streaming media to it) and we want to know whether in this case, for
> diagnostic reasons, whether TDLS was established between them at this
> time.

But then the next diagnostic thing will be ... oh, let's see what
 * the signal quality is
 * bitrate is used
 * etc.

all of which is covered by station information (and can optionally be
made available for the station entry.) Right now you're only asking "is
there a link" but I see no reason that (in particular for
diagnostics/debug!) you wouldn't want more :-)

johannes

Re: [PATCHv2] nl80211: Provide TDLS link state

[Paul Stewart]

On Wed, May 21, 2014 at 12:59 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2014-05-21 at 09:17 -0700, Paul Stewart wrote:
>
>>  I still think this might be too heavyweight in situations where the
>> firmware handles TDLS links. Think about a system where the firmware
>> is completely responsible for terminating incoming TDLS requests and
>> offers no facility to notify the kernel as the TDLS link (initiated
>> remotely) was established.
>
> Is it really that bad to demand the firmware notify the host? It's not
> like this is a very frequent event, and if querying is supported then
> notification can't be all that much more difficult.

I think firmware vendors are coming to the point of view that they
must minimize host processor wakeups.  They may argue from that
perspective that the firmware should divulge such information only on
request.

> We're not in a great position to demand that upstream, I guess, it
> should be easier for you :-)
>
> If it's really such a big problem, in theory one could support
> get_station() (which gives you a MAC address) and not dump_station(). I
> wouldn't necessarily recommend it, but it does seem possible.
>

Okay.  I suppose I can shelve this until I can ask for firmware
changes to support this pattern.  I'm not sure I'll get it, though,
and I'm doubtful whether that's a tenable path for others.

Re: [PATCHv2] nl80211: Provide TDLS link state

[Johannes Berg]

On Wed, 2014-05-21 at 13:19 -0700, Paul Stewart wrote:

> > Is it really that bad to demand the firmware notify the host? It's not
> > like this is a very frequent event, and if querying is supported then
> > notification can't be all that much more difficult.
> 
> I think firmware vendors are coming to the point of view that they
> must minimize host processor wakeups.  They may argue from that
> perspective that the firmware should divulge such information only on
> request.

That's reasonable, but the request could also be "list all TDLS peers
please", right? Then you could still implement it very easily. It
doesn't have to be an event.

johannes