* [RFCv3] Document P2P dbus interfaces @ 2019-10-10 20:51 Andrew Zaborowski 2019-11-14 4:40 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Andrew Zaborowski @ 2019-10-10 20:51 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 7829 bytes --] From: Andrew Zaborowski <andrew.zaborowski@intel.com> Proposed minimum P2P interfaces for establishing basic connections. The device discovery results in creation of P2PPeer objects. In the Wi-Fi Display API we are passing raw IE data because there's a relatively big set of different values that may be encoded in them. We could reduce them to 2-3 bools and integers but this might limit the client implementations feature set. --- doc/p2p-api.txt | 82 ++++++++++++++++++++++++++++++++++ doc/p2p-peer-api.txt | 66 +++++++++++++++++++++++++++ doc/p2p-wfd-agent-api.txt | 34 ++++++++++++++ doc/signal-level-agent-api.txt | 4 ++ 4 files changed, 186 insertions(+) create mode 100644 doc/p2p-api.txt create mode 100644 doc/p2p-peer-api.txt create mode 100644 doc/p2p-wfd-agent-api.txt diff --git a/doc/p2p-api.txt b/doc/p2p-api.txt new file mode 100644 index 00000000..84f862e5 --- /dev/null +++ b/doc/p2p-api.txt @@ -0,0 +1,82 @@ +P2P hierarchy +============= + +Service net.connman.iwd +Interface net.connman.iwd.P2P [Experimental] +Object path /{phy0,phy1,...} + +Methods array(on) GetPeers() + + Returns a list (possibly empty) of detected P2P peers. + Each record returned contains a tuple of the following + values. + + object Object + + net.connman.iwd.P2PPeer object representing + the peer device. + + int16 SignalStrength + + Peer's maximum signal strength expressed + in 100 * dBm. The value is the range of 0 + (strongest signal) to -10000 (weakest signal) + + void RegisterSignalLevelAgent(object path, + array(int16) levels) + + Register the agent object to receive signal strength + level change notifications on the + net.connman.iwd.SignalLevelAgent interface, see + signal-level-agent-api.txt. The "levels" + parameters decides the thresholds in dBm that will + generate a call to the agent's Changed + method whenever current RSSI crosses any of the + values. The values must be passed in descending + order. The number and distance between requested + threshold values is a compromise between resolution + and the frequency of system wakeups and + context-switches that are going to be occuring to + update the client's signal meter. Only one agent + can be registered at any time. + + Possible Errors: [service].Error.InvalidArguments + [service].Error.Failed + [service].Error.AlreadyExists + [service].Error.NotSupported + + void RegisterWFDService(object path, array(byte) payload) + + Register the Wi-Fi Display service running on local + host with this P2P device so that device discovery + and connection setup phases include necessary WFD + information for the peers and receive peer's WFD + service information. An object may be provided to + receive relevant WFD information about peers and + connections through method calls on the object's + net.connman.iwd.P2PWFDAgent interface. See + p2p-wfd-agent-api.txt. Only one agent can be + registered at any time. The byte array provided + will be included in WFD Information Elements + present in relevant frames sent to peers. + + Possible Errors: [service].Error.InvalidArguments + [service].Error.AlreadyExists + [service].Error.NotSupported + +Properties boolean Enabled [readwrite] + + Whether local P2P device is started, i.e. is scanning + for peers, is discoverable by peers and/or connected + to peer(s). + + string Name [readwrite] + + Sets local P2P device name as it is going to be + presented on other devices that we will connect to + or ones that discover us in scanning. + + uint16 MaxConnections [readonly] + + Maximum number of concurrent P2P peers that local + hardware is capable of connecting to. Often 1. diff --git a/doc/p2p-peer-api.txt b/doc/p2p-peer-api.txt new file mode 100644 index 00000000..898f4fb9 --- /dev/null +++ b/doc/p2p-peer-api.txt @@ -0,0 +1,66 @@ +P2PPeer hierarchy +================= + +Service net.connman.iwd +Interface net.connman.iwd.P2PPeer [Experimental] +Object path /{phy0,phy1,...}/p2p_peers/{aa_bb_cc_dd_ee_ff} + +Methods ConnectPushButton() + + Connect to the P2P peer in the Push Button mode + using the device pointed to by the .Device property. + Returns when connection is complete. + + Possible errors: net.connman.iwd.Aborted + net.connman.iwd.Busy + net.connman.iwd.Failed + net.connman.iwd.NotSupported + net.connman.iwd.Timeout + net.connman.iwd.InProgress + + ConnectPIN(string pin) + + Connect to the P2P peer in PIN mode. Returns when + connection is complete. + + See net.connman.iwd.WiFiSimpleConfiguration.StartPIN() + for how the pin argument is used. + + Possible errors: net.connman.iwd.Aborted + net.connman.iwd.Busy + net.connman.iwd.Failed + net.connman.iwd.NotSupported + net.connman.iwd.Timeout + net.connman.iwd.InProgress + + Disconnect() [optional] + + If connected, disconnect from this peer. + + Possible errors: net.connman.iwd.Failed + +Properties string Name [readonly] + + P2P Peer's display name + + string DeviceCategory [readonly] + + The category part of the peer's declared + Primary Device Type. + + string DeviceSubcategory [readonly, optional] + + The Sub Category part of the peer's declared + Primary Device Type. + + object Device [readonly] + + The object with a net.connman.iwd.P2P interface + that discovered this peer. + + boolean Connected [readonly] + + Whether there's currently an active connection + to this peer. The property is read-only and + changes as a result of the Connect and + Disconnect methods calls. diff --git a/doc/p2p-wfd-agent-api.txt b/doc/p2p-wfd-agent-api.txt new file mode 100644 index 00000000..be0ee01b --- /dev/null +++ b/doc/p2p-wfd-agent-api.txt @@ -0,0 +1,34 @@ +P2PWFDAgent hierarchy +===================== + +Service unique name +Interface net.connman.iwd.P2PWFDAgent +Object path freely definable + +Methods void Release(object device) + + This method gets called when IWD unregisters the + WFD service on a specific P2P device. An agent can + use it to do cleanup tasks. There is no need to + unregister the agent, because when this method + gets called it has already been unregistered. + + void WFDPeersChanged(object device, + array(object, array(byte)) peers) + + Called when the set of discovered WFD-capable + peers has changed during device discovery. + + The device object has the net.connman.iwd.P2P + interface while the objects in the peers array + have net.connman.iwd.P2PPeer interfaces. The byte + array included contains the reassembled payload of + the WFD Information Elements presented by the peer. + + void NewConnection(object peer, array(byte) payload) + + Called when a new P2P connection has been established + to a WFD-capable peer. The peer object has the + net.connman.iwd.P2PPerr interface. The byte array + contains the reassembled payload of the WFD + Information Elements presented by the peer. diff --git a/doc/signal-level-agent-api.txt b/doc/signal-level-agent-api.txt index 847e7ca3..1c786d96 100644 --- a/doc/signal-level-agent-api.txt +++ b/doc/signal-level-agent-api.txt @@ -31,3 +31,7 @@ Methods void Release(object device) 0 would mean signal is received at -40 or more dBm and 3 would mean below -60 dBm and might correspond to 1 out of 4 bars on a UI signal meter. + + The device parameter may be either a + net.connman.iwd.Station object or a + net.connman.iwd.P2PPeer object. -- 2.20.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-10-10 20:51 [RFCv3] Document P2P dbus interfaces Andrew Zaborowski @ 2019-11-14 4:40 ` Denis Kenzior 2019-11-14 16:59 ` Andrew Zaborowski 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2019-11-14 4:40 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 11262 bytes --] Hi Andrew, On 10/10/19 3:51 PM, Andrew Zaborowski wrote: > From: Andrew Zaborowski <andrew.zaborowski@intel.com> > > Proposed minimum P2P interfaces for establishing basic connections. The > device discovery results in creation of P2PPeer objects. > > In the Wi-Fi Display API we are passing raw IE data because there's a > relatively big set of different values that may be encoded in them. We > could reduce them to 2-3 bools and integers but this might limit the > client implementations feature set. So I'm not really a huge fan of this. We'd still need to be able to understand what is inside the IE in order to verify it properly. I'd err on the side of providing some attributes instead of full on binary IEs. Also, some of the subelements involved should really be filled in by iwd itself and not the external application. Otherwise you'll end up with some sort of Frankenstein where the app provides some info but we'd need to rewrite or modify it anyway. > --- > doc/p2p-api.txt | 82 ++++++++++++++++++++++++++++++++++ > doc/p2p-peer-api.txt | 66 +++++++++++++++++++++++++++ > doc/p2p-wfd-agent-api.txt | 34 ++++++++++++++ > doc/signal-level-agent-api.txt | 4 ++ > 4 files changed, 186 insertions(+) > create mode 100644 doc/p2p-api.txt > create mode 100644 doc/p2p-peer-api.txt > create mode 100644 doc/p2p-wfd-agent-api.txt > > diff --git a/doc/p2p-api.txt b/doc/p2p-api.txt > new file mode 100644 > index 00000000..84f862e5 > --- /dev/null > +++ b/doc/p2p-api.txt > @@ -0,0 +1,82 @@ > +P2P hierarchy > +============= > + > +Service net.connman.iwd > +Interface net.connman.iwd.P2P [Experimental] > +Object path /{phy0,phy1,...} So we changed the paths for 1.0 and this might need to be updated. > + > +Methods array(on) GetPeers() > + > + Returns a list (possibly empty) of detected P2P peers. > + Each record returned contains a tuple of the following > + values. > + > + object Object > + > + net.connman.iwd.P2PPeer object representing Name this iwd.Peer? > + the peer device. > + > + int16 SignalStrength > + > + Peer's maximum signal strength expressed > + in 100 * dBm. The value is the range of 0 > + (strongest signal) to -10000 (weakest signal) > + > + void RegisterSignalLevelAgent(object path, > + array(int16) levels) > + > + Register the agent object to receive signal strength > + level change notifications on the > + net.connman.iwd.SignalLevelAgent interface, see > + signal-level-agent-api.txt. The "levels" We moved this into the station api. So maybe just refer to that here... Though really, I wonder if you even want to bother with this for the first iteration? I'd maybe leave this out? > + parameters decides the thresholds in dBm that will > + generate a call to the agent's Changed > + method whenever current RSSI crosses any of the > + values. The values must be passed in descending > + order. The number and distance between requested > + threshold values is a compromise between resolution > + and the frequency of system wakeups and > + context-switches that are going to be occuring to > + update the client's signal meter. Only one agent > + can be registered at any time. > + > + Possible Errors: [service].Error.InvalidArguments > + [service].Error.Failed > + [service].Error.AlreadyExists > + [service].Error.NotSupported > + > + void RegisterWFDService(object path, array(byte) payload) I would maybe make this a bit more generic in case other P2P services are supported in the future, like file sharing, etc. But, I'm not sure this belongs here? P2P is a per-phy interface; do you really want the clients to register WiFi Display per phy? Or just once and have it apply to all devices in the system, current or ones yet to be plugged in? Perhaps we need a ServiceManager or add this to AgentManager API? I suppose it might be in theory possible to support both a Sink and a Source role on different phys, but then shouldn't you be distinguishing between the roles somehow? > + > + Register the Wi-Fi Display service running on local > + host with this P2P device so that device discovery > + and connection setup phases include necessary WFD okay... > + information for the peers and receive peer's WFD > + service information. An object may be provided to > + receive relevant WFD information about peers and > + connections through method calls on the object's > + net.connman.iwd.P2PWFDAgent interface. See > + p2p-wfd-agent-api.txt. Only one agent can be > + registered at any time. The byte array provided > + will be included in WFD Information Elements > + present in relevant frames sent to peers. So besides the IE information (which I don't really want transported in binary), what real use is the WFD agent? > + > + Possible Errors: [service].Error.InvalidArguments > + [service].Error.AlreadyExists > + [service].Error.NotSupported > + > +Properties boolean Enabled [readwrite] > + > + Whether local P2P device is started, i.e. is scanning > + for peers, is discoverable by peers and/or connected > + to peer(s). > + > + string Name [readwrite] > + > + Sets local P2P device name as it is going to be > + presented on other devices that we will connect to > + or ones that discover us in scanning. > + > + uint16 MaxConnections [readonly] > + > + Maximum number of concurrent P2P peers that local > + hardware is capable of connecting to. Often 1. I think we've talked about this before. Given what we now know, I would drop this for now. I think the likelihood of this becoming > 1 is astronomically small and we shouldn't even worry about this. This means we can also move some attributes / methods from the Peer interface onto this one. > diff --git a/doc/p2p-peer-api.txt b/doc/p2p-peer-api.txt > new file mode 100644 > index 00000000..898f4fb9 > --- /dev/null > +++ b/doc/p2p-peer-api.txt > @@ -0,0 +1,66 @@ > +P2PPeer hierarchy > +================= > + > +Service net.connman.iwd > +Interface net.connman.iwd.P2PPeer [Experimental] .iwd.Peer? > +Object path /{phy0,phy1,...}/p2p_peers/{aa_bb_cc_dd_ee_ff} Object path needs to be updated post 1.0 > + > +Methods ConnectPushButton() > + > + Connect to the P2P peer in the Push Button mode > + using the device pointed to by the .Device property. > + Returns when connection is complete. Why are you method names different from SimpleConfiguration? in fact, why not just add .SimpleConfiguration interface to the same object path and just have clients use that? > + > + Possible errors: net.connman.iwd.Aborted > + net.connman.iwd.Busy > + net.connman.iwd.Failed > + net.connman.iwd.NotSupported > + net.connman.iwd.Timeout > + net.connman.iwd.InProgress > + > + ConnectPIN(string pin) > + > + Connect to the P2P peer in PIN mode. Returns when > + connection is complete. > + > + See net.connman.iwd.WiFiSimpleConfiguration.StartPIN() > + for how the pin argument is used. > + > + Possible errors: net.connman.iwd.Aborted > + net.connman.iwd.Busy > + net.connman.iwd.Failed > + net.connman.iwd.NotSupported > + net.connman.iwd.Timeout > + net.connman.iwd.InProgress > + > + Disconnect() [optional] A method can't be optional? Anyway, I'd put this on the P2P interface instead. > + > + If connected, disconnect from this peer. > + > + Possible errors: net.connman.iwd.Failed > + > +Properties string Name [readonly] > + > + P2P Peer's display name > + > + string DeviceCategory [readonly] > + > + The category part of the peer's declared > + Primary Device Type. > + > + string DeviceSubcategory [readonly, optional] > + > + The Sub Category part of the peer's declared > + Primary Device Type. > + > + object Device [readonly] > + > + The object with a net.connman.iwd.P2P interface > + that discovered this peer. Okay > + > + boolean Connected [readonly] > + > + Whether there's currently an active connection > + to this peer. The property is read-only and > + changes as a result of the Connect and > + Disconnect methods calls. This wording might need to be updated.... > diff --git a/doc/p2p-wfd-agent-api.txt b/doc/p2p-wfd-agent-api.txt > new file mode 100644 > index 00000000..be0ee01b > --- /dev/null > +++ b/doc/p2p-wfd-agent-api.txt > @@ -0,0 +1,34 @@ > +P2PWFDAgent hierarchy > +===================== > + > +Service unique name > +Interface net.connman.iwd.P2PWFDAgent > +Object path freely definable > + > +Methods void Release(object device) > + > + This method gets called when IWD unregisters the > + WFD service on a specific P2P device. An agent can > + use it to do cleanup tasks. There is no need to > + unregister the agent, because when this method > + gets called it has already been unregistered. > + > + void WFDPeersChanged(object device, > + array(object, array(byte)) peers) > + > + Called when the set of discovered WFD-capable > + peers has changed during device discovery. > + > + The device object has the net.connman.iwd.P2P > + interface while the objects in the peers array > + have net.connman.iwd.P2PPeer interfaces. The byte > + array included contains the reassembled payload of > + the WFD Information Elements presented by the peer. Okay, so you want to notify the agent instead of adding a WFD specific interface to the Peer object. I still wonder if we can avoid transporting raw IEs here... Also, how are you making sure that the peers are actually ones that the agent cares about? There's no sense in sending Source role peer info if we're also a Source... Can we be a source and a sink at the same time? > + > + void NewConnection(object peer, array(byte) payload) > + > + Called when a new P2P connection has been established > + to a WFD-capable peer. The peer object has the > + net.connman.iwd.P2PPerr interface. The byte array typo > + contains the reassembled payload of the WFD > + Information Elements presented by the peer. So I'm not entirely sure how this is useful? The fact that we made a connection to a peer is already available via the Peer.Connected property. Is the payload somehow different from what was sent via WFDPeersChanged? > diff --git a/doc/signal-level-agent-api.txt b/doc/signal-level-agent-api.txt > index 847e7ca3..1c786d96 100644 > --- a/doc/signal-level-agent-api.txt > +++ b/doc/signal-level-agent-api.txt > @@ -31,3 +31,7 @@ Methods void Release(object device) > 0 would mean signal is received at -40 or more dBm > and 3 would mean below -60 dBm and might correspond > to 1 out of 4 bars on a UI signal meter. > + > + The device parameter may be either a > + net.connman.iwd.Station object or a > + net.connman.iwd.P2PPeer object. > This file no longer exists FYI. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-14 4:40 ` Denis Kenzior @ 2019-11-14 16:59 ` Andrew Zaborowski 2019-11-14 18:19 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Andrew Zaborowski @ 2019-11-14 16:59 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 18252 bytes --] Hi Denis, On Thu, 14 Nov 2019 at 05:42, Denis Kenzior <denkenz@gmail.com> wrote: > On 10/10/19 3:51 PM, Andrew Zaborowski wrote: > > Proposed minimum P2P interfaces for establishing basic connections. The > > device discovery results in creation of P2PPeer objects. > > > > In the Wi-Fi Display API we are passing raw IE data because there's a > > relatively big set of different values that may be encoded in them. We > > could reduce them to 2-3 bools and integers but this might limit the > > client implementations feature set. > > So I'm not really a huge fan of this. We'd still need to be able to > understand what is inside the IE in order to verify it properly. I'd > err on the side of providing some attributes instead of full on binary > IEs. > > Also, some of the subelements involved should really be filled in by iwd > itself and not the external application. Otherwise you'll end up with > some sort of Frankenstein where the app provides some info but we'd need > to rewrite or modify it anyway. I didn't plan to interpret the contents of the IE in any way. The idea would be to have little service-specific code, and in case of WFD there's no need to do anything more than passing the ies. So I see only two options, either full on binary IE passed in both directions between iwd and the service implementation, or full on individual values from the WFD subelements. There's a lot of them though, for example for a probe response you have at least: * 12 capability bits in a bitmap * RTSP tcp port * throughput * optional coupled sink status * optional coupled sink MAC * R2 support (true/false) + 10 or more optional values... I just had a look at the how gnome-screencast is passing those to NM / wpa_supplicant and they basically hardcode the IE in one place in the code and never touch the contents... On receival they only check if the WFD IEs were present and don't really look at the contents either. This is very new code though and I haven't looked at openwfd or miraclecast in detail. > > > --- > > doc/p2p-api.txt | 82 ++++++++++++++++++++++++++++++++++ > > doc/p2p-peer-api.txt | 66 +++++++++++++++++++++++++++ > > doc/p2p-wfd-agent-api.txt | 34 ++++++++++++++ > > doc/signal-level-agent-api.txt | 4 ++ > > 4 files changed, 186 insertions(+) > > create mode 100644 doc/p2p-api.txt > > create mode 100644 doc/p2p-peer-api.txt > > create mode 100644 doc/p2p-wfd-agent-api.txt > > > > diff --git a/doc/p2p-api.txt b/doc/p2p-api.txt > > new file mode 100644 > > index 00000000..84f862e5 > > --- /dev/null > > +++ b/doc/p2p-api.txt > > @@ -0,0 +1,82 @@ > > +P2P hierarchy > > +============= > > + > > +Service net.connman.iwd > > +Interface net.connman.iwd.P2P [Experimental] > > +Object path /{phy0,phy1,...} > > So we changed the paths for 1.0 and this might need to be updated. Right. > > > + > > +Methods array(on) GetPeers() > > + > > + Returns a list (possibly empty) of detected P2P peers. > > + Each record returned contains a tuple of the following > > + values. > > + > > + object Object > > + > > + net.connman.iwd.P2PPeer object representing > > Name this iwd.Peer? Sounds very generic, are we sure we won't have another spec that uses "peers" too? > > > + the peer device. > > + > > + int16 SignalStrength > > + > > + Peer's maximum signal strength expressed > > + in 100 * dBm. The value is the range of 0 > > + (strongest signal) to -10000 (weakest signal) > > + > > + void RegisterSignalLevelAgent(object path, > > + array(int16) levels) > > + > > + Register the agent object to receive signal strength > > + level change notifications on the > > + net.connman.iwd.SignalLevelAgent interface, see > > + signal-level-agent-api.txt. The "levels" > > We moved this into the station api. So maybe just refer to that here... > > Though really, I wonder if you even want to bother with this for the > first iteration? I'd maybe leave this out? Yeah, I wasn't going to implement it at this time. But since you'll have RSSI in every mode of wireless connections, AP, adhoc, ... wouldn't it make sense to have a common interface and share code in iwd and in the clients? > > > + parameters decides the thresholds in dBm that will > > + generate a call to the agent's Changed > > + method whenever current RSSI crosses any of the > > + values. The values must be passed in descending > > + order. The number and distance between requested > > + threshold values is a compromise between resolution > > + and the frequency of system wakeups and > > + context-switches that are going to be occuring to > > + update the client's signal meter. Only one agent > > + can be registered at any time. > > + > > + Possible Errors: [service].Error.InvalidArguments > > + [service].Error.Failed > > + [service].Error.AlreadyExists > > + [service].Error.NotSupported > > + > > + void RegisterWFDService(object path, array(byte) payload) > > I would maybe make this a bit more generic in case other P2P services > are supported in the future, like file sharing, etc. I was thinking of having a separate agent interface and separate register method for each service, the parameters are going to be different... but we could also have a common register method and another method on the service's agent to query the actual values of the parameters (at the cost of additional round trips) > > But, I'm not sure this belongs here? P2P is a per-phy interface; do you > really want the clients to register WiFi Display per phy? Or just once > and have it apply to all devices in the system, current or ones yet to > be plugged in? Perhaps we need a ServiceManager or add this to > AgentManager API? Yep, perhaps a ServiceManager (P2PServiceManager?) is a better place... > > I suppose it might be in theory possible to support both a Sink and a > Source role on different phys, but then shouldn't you be distinguishing > between the roles somehow? It is possible but luckily it's all up to the wfd implementation. We'd only see this information if we wanted to build the WFD IEs ourselves like you suggest but we wouldn't be doing anything more than encoding / decoding it from the IEs. > > > + > > + Register the Wi-Fi Display service running on local > > + host with this P2P device so that device discovery > > + and connection setup phases include necessary WFD > > okay... > > > + information for the peers and receive peer's WFD > > + service information. An object may be provided to > > + receive relevant WFD information about peers and > > + connections through method calls on the object's > > + net.connman.iwd.P2PWFDAgent interface. See > > + p2p-wfd-agent-api.txt. Only one agent can be > > + registered at any time. The byte array provided > > + will be included in WFD Information Elements > > + present in relevant frames sent to peers. > > So besides the IE information (which I don't really want transported in > binary), what real use is the WFD agent? So it would be receiving events on a new WFD connection and during discovery it would be receiving the capabilities etc. of the discovered peers. > > > + > > + Possible Errors: [service].Error.InvalidArguments > > + [service].Error.AlreadyExists > > + [service].Error.NotSupported > > + > > +Properties boolean Enabled [readwrite] > > + > > + Whether local P2P device is started, i.e. is scanning > > + for peers, is discoverable by peers and/or connected > > + to peer(s). > > + > > + string Name [readwrite] > > + > > + Sets local P2P device name as it is going to be > > + presented on other devices that we will connect to > > + or ones that discover us in scanning. > > + > > + uint16 MaxConnections [readonly] > > + > > + Maximum number of concurrent P2P peers that local > > + hardware is capable of connecting to. Often 1. > > I think we've talked about this before. Given what we now know, I would > drop this for now. I think the likelihood of this becoming > 1 is > astronomically small and we shouldn't even worry about this. So I don't really have enough information to say that, the 2 P2P capable adapters that I worked with only support one client or one GO interface but those are only 2 adapters. I have actually changed this since this proposal to be named AvailableConnections and switch between 1 and 0 as the device becomes busy. So it has an additional use of indicating whether a connection is in progress... the Connected property on the Peer only becomes 1 when the connection is operative. > This means > we can also move some attributes / methods from the Peer interface onto > this one. So this is a slightly different topic and I wouldn't do it. Apparently some device will make sure they become the GO during GO Negotiation (or as soon as P2P is enabled) and then use the invitation procedure instead of the group formation procedure, so then they can be connected to multiple peripherals and this seems to be an actual use case. MaxConnections still doesn't sound right because in theory there's no maximum, but we still need some way to indicate if we're busy. > > > diff --git a/doc/p2p-peer-api.txt b/doc/p2p-peer-api.txt > > new file mode 100644 > > index 00000000..898f4fb9 > > --- /dev/null > > +++ b/doc/p2p-peer-api.txt > > @@ -0,0 +1,66 @@ > > +P2PPeer hierarchy > > +================= > > + > > +Service net.connman.iwd > > +Interface net.connman.iwd.P2PPeer [Experimental] > .iwd.Peer? > > > +Object path /{phy0,phy1,...}/p2p_peers/{aa_bb_cc_dd_ee_ff} > > Object path needs to be updated post 1.0 > > > + > > +Methods ConnectPushButton() > > + > > + Connect to the P2P peer in the Push Button mode > > + using the device pointed to by the .Device property. > > + Returns when connection is complete. > > Why are you method names different from SimpleConfiguration? in fact, > why not just add .SimpleConfiguration interface to the same object path > and just have clients use that? Could do this, yes, although it would have been nice to have "Connect" in the method names. And I'm not sure how to resolve this in the code since the implementation of the methods will be very different from the one in wsc.c, preferably I wouldn't duplicate setup_wsc_interface. > > > + > > + Possible errors: net.connman.iwd.Aborted > > + net.connman.iwd.Busy > > + net.connman.iwd.Failed > > + net.connman.iwd.NotSupported > > + net.connman.iwd.Timeout > > + net.connman.iwd.InProgress > > + > > + ConnectPIN(string pin) > > + > > + Connect to the P2P peer in PIN mode. Returns when > > + connection is complete. > > + > > + See net.connman.iwd.WiFiSimpleConfiguration.StartPIN() > > + for how the pin argument is used. > > + > > + Possible errors: net.connman.iwd.Aborted > > + net.connman.iwd.Busy > > + net.connman.iwd.Failed > > + net.connman.iwd.NotSupported > > + net.connman.iwd.Timeout > > + net.connman.iwd.InProgress > > + > > + Disconnect() [optional] > > A method can't be optional? Well since the optional concept is kind of made up it just means returning an error when the method or property doesn't apply. > Anyway, I'd put this on the P2P interface > instead. > > > + > > + If connected, disconnect from this peer. > > + > > + Possible errors: net.connman.iwd.Failed > > + > > +Properties string Name [readonly] > > + > > + P2P Peer's display name > > + > > + string DeviceCategory [readonly] > > + > > + The category part of the peer's declared > > + Primary Device Type. > > + > > + string DeviceSubcategory [readonly, optional] > > + > > + The Sub Category part of the peer's declared > > + Primary Device Type. > > + > > + object Device [readonly] > > + > > + The object with a net.connman.iwd.P2P interface > > + that discovered this peer. > > Okay > > > + > > + boolean Connected [readonly] > > + > > + Whether there's currently an active connection > > + to this peer. The property is read-only and > > + changes as a result of the Connect and > > + Disconnect methods calls. > > This wording might need to be updated.... You mean because we'd only have one connection? > > > diff --git a/doc/p2p-wfd-agent-api.txt b/doc/p2p-wfd-agent-api.txt > > new file mode 100644 > > index 00000000..be0ee01b > > --- /dev/null > > +++ b/doc/p2p-wfd-agent-api.txt > > @@ -0,0 +1,34 @@ > > +P2PWFDAgent hierarchy > > +===================== > > + > > +Service unique name > > +Interface net.connman.iwd.P2PWFDAgent > > +Object path freely definable > > + > > +Methods void Release(object device) > > + > > + This method gets called when IWD unregisters the > > + WFD service on a specific P2P device. An agent can > > + use it to do cleanup tasks. There is no need to > > + unregister the agent, because when this method > > + gets called it has already been unregistered. > > + > > + void WFDPeersChanged(object device, > > + array(object, array(byte)) peers) > > + > > + Called when the set of discovered WFD-capable > > + peers has changed during device discovery. > > + > > + The device object has the net.connman.iwd.P2P > > + interface while the objects in the peers array > > + have net.connman.iwd.P2PPeer interfaces. The byte > > + array included contains the reassembled payload of > > + the WFD Information Elements presented by the peer. > > Okay, so you want to notify the agent instead of adding a WFD specific > interface to the Peer object. I still wonder if we can avoid > transporting raw IEs here... Also, how are you making sure that the > peers are actually ones that the agent cares about? There's no sense in > sending Source role peer info if we're also a Source... > > Can we be a source and a sink at the same time? Yeah, but I don't really see benefit in hiding the WFD sources if we're the source, or WFD sinks if we're a sink. It'll be simpler for everyone if the WFD service takes care of this. > > > + > > + void NewConnection(object peer, array(byte) payload) > > + > > + Called when a new P2P connection has been established > > + to a WFD-capable peer. The peer object has the > > + net.connman.iwd.P2PPerr interface. The byte array > > typo > > > + contains the reassembled payload of the WFD > > + Information Elements presented by the peer. > > So I'm not entirely sure how this is useful? The fact that we made a > connection to a peer is already available via the Peer.Connected > property. Is the payload somehow different from what was sent via > WFDPeersChanged? Hmm, no, it should be the same in practice, so yeah, we can maybe drop this. The spec actually has a different subset of subelements allowed in each frame (Associate vs. Beacon or Probe Response) but in wpa_supplicant for example, you set one WFD IE for all possible frames. The bit that might change is "Available for WFD Session" now that I think of it... but I assume when a device is not available it simply won't be discovering or entering group formation. But it could be a candidate for modifying inside iwd rather than taking a value from the DBus client. By the way, one thing that needs to change in this proposal is how peer discovery (scanning) is triggered and how the current state is signalled. Discovery is more expensive than we previously thought and can't be automatically enabled whenever the P2P interface is enabled. Best regards ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-14 16:59 ` Andrew Zaborowski @ 2019-11-14 18:19 ` Denis Kenzior 2019-11-14 19:37 ` Andrew Zaborowski 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2019-11-14 18:19 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 12789 bytes --] Hi Andrew, On 11/14/19 10:59 AM, Andrew Zaborowski wrote: > Hi Denis, > > On Thu, 14 Nov 2019 at 05:42, Denis Kenzior <denkenz@gmail.com> wrote: >> On 10/10/19 3:51 PM, Andrew Zaborowski wrote: >>> Proposed minimum P2P interfaces for establishing basic connections. The >>> device discovery results in creation of P2PPeer objects. >>> >>> In the Wi-Fi Display API we are passing raw IE data because there's a >>> relatively big set of different values that may be encoded in them. We >>> could reduce them to 2-3 bools and integers but this might limit the >>> client implementations feature set. >> >> So I'm not really a huge fan of this. We'd still need to be able to >> understand what is inside the IE in order to verify it properly. I'd >> err on the side of providing some attributes instead of full on binary >> IEs. >> >> Also, some of the subelements involved should really be filled in by iwd >> itself and not the external application. Otherwise you'll end up with >> some sort of Frankenstein where the app provides some info but we'd need >> to rewrite or modify it anyway. > > I didn't plan to interpret the contents of the IE in any way. The > idea would be to have little service-specific code, and in case of WFD > there's no need to do anything more than passing the ies. So two issues: - Passing in unchecked binary info from an unprivileged process is just not going to happen though. We have to look inside anyway to sanitize it. - Some of the sub elements are dynamic (connected bssid for example). So you will end up having to interpret it anyway. > > So I see only two options, either full on binary IE passed in both > directions between iwd and the service implementation, or full on > individual values from the WFD subelements. There's a lot of them > though, for example for a probe response you have at least: > > * 12 capability bits in a bitmap > * RTSP tcp port > * throughput > * optional coupled sink status > * optional coupled sink MAC > * R2 support (true/false) > > + 10 or more optional values... So lets start with the bare minimum and see. In the end this API is experimental, so we can still change things. > > I just had a look at the how gnome-screencast is passing those to NM / > wpa_supplicant and they basically hardcode the IE in one place in the > code and never touch the contents... On receival they only check if > the WFD IEs were present and don't really look at the contents either. > This is very new code though and I haven't looked at openwfd or > miraclecast in detail. Even more reason not to bother with passing IEs around. <snip> >> >>> + >>> +Methods array(on) GetPeers() >>> + >>> + Returns a list (possibly empty) of detected P2P peers. >>> + Each record returned contains a tuple of the following >>> + values. >>> + >>> + object Object >>> + >>> + net.connman.iwd.P2PPeer object representing >> >> Name this iwd.Peer? > > Sounds very generic, are we sure we won't have another spec that uses > "peers" too? I'm not aware of any. But P2PPeer does not CamelCase well regardless. I'm happy to hear other ideas. <snip> >>> + void RegisterSignalLevelAgent(object path, >>> + array(int16) levels) >>> + >>> + Register the agent object to receive signal strength >>> + level change notifications on the >>> + net.connman.iwd.SignalLevelAgent interface, see >>> + signal-level-agent-api.txt. The "levels" >> >> We moved this into the station api. So maybe just refer to that here... >> >> Though really, I wonder if you even want to bother with this for the >> first iteration? I'd maybe leave this out? > > Yeah, I wasn't going to implement it at this time. But since you'll > have RSSI in every mode of wireless connections, AP, adhoc, ... > wouldn't it make sense to have a common interface and share code in > iwd and in the clients? You don't though. For AP you have multiple clients, which rssi are you going to show? Same goes for p2p-go really. With adhoc you have an even weirder problem. >>> + void RegisterWFDService(object path, array(byte) payload) >> >> I would maybe make this a bit more generic in case other P2P services >> are supported in the future, like file sharing, etc. > > I was thinking of having a separate agent interface and separate > register method for each service, the parameters are going to be > different... but we could also have a common register method and > another method on the service's agent to query the actual values of > the parameters (at the cost of additional round trips) > I'd just make this into a dictionary of parameters and have it depend on the service type. >> >> But, I'm not sure this belongs here? P2P is a per-phy interface; do you >> really want the clients to register WiFi Display per phy? Or just once >> and have it apply to all devices in the system, current or ones yet to >> be plugged in? Perhaps we need a ServiceManager or add this to >> AgentManager API? > > Yep, perhaps a ServiceManager (P2PServiceManager?) is a better place... > >> >> I suppose it might be in theory possible to support both a Sink and a >> Source role on different phys, but then shouldn't you be distinguishing >> between the roles somehow? > > It is possible but luckily it's all up to the wfd implementation. > We'd only see this information if we wanted to build the WFD IEs > ourselves like you suggest but we wouldn't be doing anything more than > encoding / decoding it from the IEs. Then we can't do this as a global registration. Because some sub-elements are dynamic it would be up to the application to update the IE properly. I'm really not sure we can avoid building the WFD IE ourselves. <snip. >>> + uint16 MaxConnections [readonly] >>> + >>> + Maximum number of concurrent P2P peers that local >>> + hardware is capable of connecting to. Often 1. >> >> I think we've talked about this before. Given what we now know, I would >> drop this for now. I think the likelihood of this becoming > 1 is >> astronomically small and we shouldn't even worry about this. > > So I don't really have enough information to say that, the 2 P2P > capable adapters that I worked with only support one client or one GO > interface but those are only 2 adapters. > > I have actually changed this since this proposal to be named > AvailableConnections and switch between 1 and 0 as the device becomes > busy. So it has an additional use of indicating whether a connection > is in progress... the Connected property on the Peer only becomes 1 > when the connection is operative. Yeah, but that's the problem. What do you actually mean by AvailableConnections? Do you mean: 1. connections on a given P2P_CLIENT or P2P_GO interface? or 2. number of P2P_CLIENT & P2P_GO interfaces? If 1, then this makes no sense anyway. For P2P_CLIENT it is always 1. For P2P_GO it is essentially unlimited. For 2, I doubt you will ever find any hardware capable of doing more than 1 anyway. And it is a complication I wouldn't worry about now regardless. So if the v1 API is limited to a single connection and we later find out it is actually possible to do more, then we worry about it then. > >> This means >> we can also move some attributes / methods from the Peer interface onto >> this one. > > So this is a slightly different topic and I wouldn't do it. > Apparently some device will make sure they become the GO during GO > Negotiation (or as soon as P2P is enabled) and then use the invitation > procedure instead of the group formation procedure, so then they can > be connected to multiple peripherals and this seems to be an actual > use case. Yeah, see above. You're talking about 1 here. And I'd argue that AvailableConnections or MaxConnections still doesn't make sense in this context. > > MaxConnections still doesn't sound right because in theory there's no > maximum, but we still need some way to indicate if we're busy. Exactly. So you need to find another way. >>> + >>> +Methods ConnectPushButton() >>> + >>> + Connect to the P2P peer in the Push Button mode >>> + using the device pointed to by the .Device property. >>> + Returns when connection is complete. >> >> Why are you method names different from SimpleConfiguration? in fact, >> why not just add .SimpleConfiguration interface to the same object path >> and just have clients use that? > > Could do this, yes, although it would have been nice to have "Connect" > in the method names. And I'm not sure how to resolve this in the code > since the implementation of the methods will be very different from > the one in wsc.c, preferably I wouldn't duplicate setup_wsc_interface. Pretty easy I would think? I mean you do have the netdev associated with it, so you can simply check the interface type. >>> + >>> + Disconnect() [optional] >> >> A method can't be optional? > > Well since the optional concept is kind of made up it just means > returning an error when the method or property doesn't apply. Made up? I don't think so. Properties are explicitly allowed to be optional. Methods are not. Check the D-Bus spec ;) We never had any optional methods and never will :) >>> + boolean Connected [readonly] >>> + >>> + Whether there's currently an active connection >>> + to this peer. The property is read-only and >>> + changes as a result of the Connect and >>> + Disconnect methods calls. >> >> This wording might need to be updated.... > > You mean because we'd only have one connection? Sorry, should have been more clear. The Connect and maybe even Disconnect method calls will be named differently. So the wording needs to be updated accordingly. >> >> Okay, so you want to notify the agent instead of adding a WFD specific >> interface to the Peer object. I still wonder if we can avoid >> transporting raw IEs here... Also, how are you making sure that the >> peers are actually ones that the agent cares about? There's no sense in >> sending Source role peer info if we're also a Source... >> >> Can we be a source and a sink at the same time? > > Yeah, but I don't really see benefit in hiding the WFD sources if > we're the source, or WFD sinks if we're a sink. It'll be simpler for > everyone if the WFD service takes care of this. That goes against our core design principles and you know it :) > >> >>> + >>> + void NewConnection(object peer, array(byte) payload) >>> + >>> + Called when a new P2P connection has been established >>> + to a WFD-capable peer. The peer object has the >>> + net.connman.iwd.P2PPerr interface. The byte array >> >> typo >> >>> + contains the reassembled payload of the WFD >>> + Information Elements presented by the peer. >> >> So I'm not entirely sure how this is useful? The fact that we made a >> connection to a peer is already available via the Peer.Connected >> property. Is the payload somehow different from what was sent via >> WFDPeersChanged? > > Hmm, no, it should be the same in practice, so yeah, we can maybe drop this. okay > > The spec actually has a different subset of subelements allowed in > each frame (Associate vs. Beacon or Probe Response) but in > wpa_supplicant for example, you set one WFD IE for all possible > frames. > > The bit that might change is "Available for WFD Session" now that I > think of it... but I assume when a device is not available it simply > won't be discovering or entering group formation. But it could be a > candidate for modifying inside iwd rather than taking a value from the > DBus client. > > By the way, one thing that needs to change in this proposal is how > peer discovery (scanning) is triggered and how the current state is > signalled. Discovery is more expensive than we previously thought and > can't be automatically enabled whenever the P2P interface is enabled. > Funny, okay I'm curious to learn more now. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-14 18:19 ` Denis Kenzior @ 2019-11-14 19:37 ` Andrew Zaborowski 2019-11-14 20:33 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Andrew Zaborowski @ 2019-11-14 19:37 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 13823 bytes --] On Thu, 14 Nov 2019 at 19:20, Denis Kenzior <denkenz@gmail.com> wrote: > On 11/14/19 10:59 AM, Andrew Zaborowski wrote: > > I didn't plan to interpret the contents of the IE in any way. The > > idea would be to have little service-specific code, and in case of WFD > > there's no need to do anything more than passing the ies. > > So two issues: > - Passing in unchecked binary info from an unprivileged process is just > not going to happen though. We have to look inside anyway to sanitize it. Fair enough, although I think the level of sanitization would only need to be the same as when we send out an SSID provided by an unprivileged process... we only check it's utf-8, not too long and such. > - Some of the sub elements are dynamic (connected bssid for example). > So you will end up having to interpret it anyway. AFAIK we don't strictly need to do anything and things will work just like with wpa_supplicant... but we could. > > > > > So I see only two options, either full on binary IE passed in both > > directions between iwd and the service implementation, or full on > > individual values from the WFD subelements. There's a lot of them > > though, for example for a probe response you have at least: > > > > * 12 capability bits in a bitmap > > * RTSP tcp port > > * throughput > > * optional coupled sink status > > * optional coupled sink MAC > > * R2 support (true/false) > > > > + 10 or more optional values... > > So lets start with the bare minimum and see. In the end this API is > experimental, so we can still change things. Ok, let's do it. I don't like that because we're again creating barriers for client software to start supporting iwd, and adding more work for iwd to do while doing this. > > > > > I just had a look at the how gnome-screencast is passing those to NM / > > wpa_supplicant and they basically hardcode the IE in one place in the > > code and never touch the contents... On receival they only check if > > the WFD IEs were present and don't really look at the contents either. > > This is very new code though and I haven't looked at openwfd or > > miraclecast in detail. > > Even more reason not to bother with passing IEs around. > > <snip> > > >> > >>> + > >>> +Methods array(on) GetPeers() > >>> + > >>> + Returns a list (possibly empty) of detected P2P peers. > >>> + Each record returned contains a tuple of the following > >>> + values. > >>> + > >>> + object Object > >>> + > >>> + net.connman.iwd.P2PPeer object representing > >> > >> Name this iwd.Peer? > > > > Sounds very generic, are we sure we won't have another spec that uses > > "peers" too? > > I'm not aware of any. But P2PPeer does not CamelCase well regardless. > I'm happy to hear other ideas. > > <snip> > > >>> + void RegisterSignalLevelAgent(object path, > >>> + array(int16) levels) > >>> + > >>> + Register the agent object to receive signal strength > >>> + level change notifications on the > >>> + net.connman.iwd.SignalLevelAgent interface, see > >>> + signal-level-agent-api.txt. The "levels" > >> > >> We moved this into the station api. So maybe just refer to that here... > >> > >> Though really, I wonder if you even want to bother with this for the > >> first iteration? I'd maybe leave this out? > > > > Yeah, I wasn't going to implement it at this time. But since you'll > > have RSSI in every mode of wireless connections, AP, adhoc, ... > > wouldn't it make sense to have a common interface and share code in > > iwd and in the clients? > > You don't though. For AP you have multiple clients, which rssi are you > going to show? That would be per client obviously, when/if information about the clients is added. > Same goes for p2p-go really. With P2P we are initiating the connections, so we have to expose information about each peer anyway. > With adhoc you have an > even weirder problem. > > >>> + void RegisterWFDService(object path, array(byte) payload) > >> > >> I would maybe make this a bit more generic in case other P2P services > >> are supported in the future, like file sharing, etc. > > > > I was thinking of having a separate agent interface and separate > > register method for each service, the parameters are going to be > > different... but we could also have a common register method and > > another method on the service's agent to query the actual values of > > the parameters (at the cost of additional round trips) > > > > I'd just make this into a dictionary of parameters and have it depend on > the service type. Ok, sounds good. > > >> > >> But, I'm not sure this belongs here? P2P is a per-phy interface; do you > >> really want the clients to register WiFi Display per phy? Or just once > >> and have it apply to all devices in the system, current or ones yet to > >> be plugged in? Perhaps we need a ServiceManager or add this to > >> AgentManager API? > > > > Yep, perhaps a ServiceManager (P2PServiceManager?) is a better place... > > > >> > >> I suppose it might be in theory possible to support both a Sink and a > >> Source role on different phys, but then shouldn't you be distinguishing > >> between the roles somehow? > > > > It is possible but luckily it's all up to the wfd implementation. > > We'd only see this information if we wanted to build the WFD IEs > > ourselves like you suggest but we wouldn't be doing anything more than > > encoding / decoding it from the IEs. > > Then we can't do this as a global registration. Because some > sub-elements are dynamic it would be up to the application to update the > IE properly. I'm really not sure we can avoid building the WFD IE > ourselves. I think it should still be global actually, but maybe we need to expand the API somehow. I think it should be global because an application like gnome-screencast is not going to know which device to use. But we may need to add a way for the service to say it's connected now so that its not considered for any simultaneous connections, until the first connection is closed. So since our assumption is that we're always initiating our connections (or in the future responing to an invitation, but the response is again our initiative), we could also register services per-connection and per-scan? This would work for gnome-screencast, since I believe it has its own dialog that shows you available devices to connect to... not sure about other services. > > <snip. > > >>> + uint16 MaxConnections [readonly] > >>> + > >>> + Maximum number of concurrent P2P peers that local > >>> + hardware is capable of connecting to. Often 1. > >> > >> I think we've talked about this before. Given what we now know, I would > >> drop this for now. I think the likelihood of this becoming > 1 is > >> astronomically small and we shouldn't even worry about this. > > > > So I don't really have enough information to say that, the 2 P2P > > capable adapters that I worked with only support one client or one GO > > interface but those are only 2 adapters. > > > > I have actually changed this since this proposal to be named > > AvailableConnections and switch between 1 and 0 as the device becomes > > busy. So it has an additional use of indicating whether a connection > > is in progress... the Connected property on the Peer only becomes 1 > > when the connection is operative. > > Yeah, but that's the problem. What do you actually mean by > AvailableConnections? Do you mean: > > 1. connections on a given P2P_CLIENT or P2P_GO interface? or > 2. number of P2P_CLIENT & P2P_GO interfaces? So this should not bleed into the API. As long as we allow the user to make another connection, it should be signalled to the user somehow. Whether we're using a separate interface or just make another connection from our GO interface. > > If 1, then this makes no sense anyway. For P2P_CLIENT it is always 1. > For P2P_GO it is essentially unlimited. Right, but we need to inform the user somehow whether we can make another connection with whatever role we have currently negotiated or because we're idle. (and for P2P_GO we may want to set an artificial limit, it's only unlimited in theory) > > For 2, I doubt you will ever find any hardware capable of doing more > than 1 anyway. And it is a complication I wouldn't worry about now > regardless. So if the v1 API is limited to a single connection and we > later find out it is actually possible to do more, then we worry about > it then. So from what I've seen, it is possible, but not until we support the GO role. However our main usecase right now (I believe...) is a PC, phone, tablet, rather than a peripheral device, so I think this is definitely going to be expected from iwd, so are you sure you want to limit the API to a single connection? AFAIK the android API for apps doesn't have this limit for example. > > > > >> This means > >> we can also move some attributes / methods from the Peer interface onto > >> this one. > > > > So this is a slightly different topic and I wouldn't do it. > > Apparently some device will make sure they become the GO during GO > > Negotiation (or as soon as P2P is enabled) and then use the invitation > > procedure instead of the group formation procedure, so then they can > > be connected to multiple peripherals and this seems to be an actual > > use case. > > Yeah, see above. You're talking about 1 here. And I'd argue that > AvailableConnections or MaxConnections still doesn't make sense in this > context. > > > > > MaxConnections still doesn't sound right because in theory there's no > > maximum, but we still need some way to indicate if we're busy. > > Exactly. So you need to find another way. Hmm, maybe a 'State' property but I'm not sure how to call the different states to make this clear... > > >>> + > >>> +Methods ConnectPushButton() > >>> + > >>> + Connect to the P2P peer in the Push Button mode > >>> + using the device pointed to by the .Device property. > >>> + Returns when connection is complete. > >> > >> Why are you method names different from SimpleConfiguration? in fact, > >> why not just add .SimpleConfiguration interface to the same object path > >> and just have clients use that? > > > > Could do this, yes, although it would have been nice to have "Connect" > > in the method names. And I'm not sure how to resolve this in the code > > since the implementation of the methods will be very different from > > the one in wsc.c, preferably I wouldn't duplicate setup_wsc_interface. > > Pretty easy I would think? I mean you do have the netdev associated > with it, so you can simply check the interface type. We only have a netdev after the GO Negotiation. Esentially I'd need to have a new struct wsc, which would encapsulate either: * a current struct wsc for a station interface, or * a P2P peer that corresponds to a scan result. Then the wsc.c PushButton method, for instance, would need to either do its current work or delegate it to a p2p.c function and exit. > > >>> + > >>> + Disconnect() [optional] > >> > >> A method can't be optional? > > > > Well since the optional concept is kind of made up it just means > > returning an error when the method or property doesn't apply. > > Made up? I don't think so. Properties are explicitly allowed to be > optional. Methods are not. Check the D-Bus spec ;) You mean https://dbus.freedesktop.org/doc/dbus-specification.html ? I think we've talked about optional properties not being considered in the specs before... > We never had any > optional methods and never will :) > > >>> + boolean Connected [readonly] > >>> + > >>> + Whether there's currently an active connection > >>> + to this peer. The property is read-only and > >>> + changes as a result of the Connect and > >>> + Disconnect methods calls. > >> > >> This wording might need to be updated.... > > > > You mean because we'd only have one connection? > > Sorry, should have been more clear. The Connect and maybe even > Disconnect method calls will be named differently. So the wording needs > to be updated accordingly. > > >> > >> Okay, so you want to notify the agent instead of adding a WFD specific > >> interface to the Peer object. I still wonder if we can avoid > >> transporting raw IEs here... Also, how are you making sure that the > >> peers are actually ones that the agent cares about? There's no sense in > >> sending Source role peer info if we're also a Source... > >> > >> Can we be a source and a sink at the same time? > > > > Yeah, but I don't really see benefit in hiding the WFD sources if > > we're the source, or WFD sinks if we're a sink. It'll be simpler for > > everyone if the WFD service takes care of this. > > That goes against our core design principles and you know it :) I think it depends on the point of view. You might as well want the caller to specify what service or device class or WSC method it's interested in and only show them matching peers. You could even do things like this for normal infrastructure-mode networks. Best regards ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-14 19:37 ` Andrew Zaborowski @ 2019-11-14 20:33 ` Denis Kenzior 2019-11-15 0:38 ` Andrew Zaborowski 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2019-11-14 20:33 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 8170 bytes --] Hi Andrew, On 11/14/19 1:37 PM, Andrew Zaborowski wrote: > On Thu, 14 Nov 2019 at 19:20, Denis Kenzior <denkenz@gmail.com> wrote: >> On 11/14/19 10:59 AM, Andrew Zaborowski wrote: >>> I didn't plan to interpret the contents of the IE in any way. The >>> idea would be to have little service-specific code, and in case of WFD >>> there's no need to do anything more than passing the ies. >> >> So two issues: >> - Passing in unchecked binary info from an unprivileged process is just >> not going to happen though. We have to look inside anyway to sanitize it. > > Fair enough, although I think the level of sanitization would only > need to be the same as when we send out an SSID provided by an > unprivileged process... we only check it's utf-8, not too long and > such. Well, about the only place we do this is in Adhoc and AP code. And the SSID gets sanitized by the kernel (and maybe even the firmware) as well. So there are multiple layers of protection here. But in general, we do not allow arbitrary binary blobs to be passed around. That's just not our design philosophy. > >> - Some of the sub elements are dynamic (connected bssid for example). >> So you will end up having to interpret it anyway. > > AFAIK we don't strictly need to do anything and things will work just > like with wpa_supplicant... but we could. > Fair enough >>> Yeah, I wasn't going to implement it at this time. But since you'll >>> have RSSI in every mode of wireless connections, AP, adhoc, ... >>> wouldn't it make sense to have a common interface and share code in >>> iwd and in the clients? >> >> You don't though. For AP you have multiple clients, which rssi are you >> going to show? > > That would be per client obviously, when/if information about the > clients is added. And the kernel has APIs for this? AFAIK you don't even get this info without polling. So trying to use our agent for this is not going to work. > >> Same goes for p2p-go really. > > With P2P we are initiating the connections, so we have to expose > information about each peer anyway. > So lets see how that works out given the above. I think for now I would leave all of this out. >> Then we can't do this as a global registration. Because some >> sub-elements are dynamic it would be up to the application to update the >> IE properly. I'm really not sure we can avoid building the WFD IE >> ourselves. > > I think it should still be global actually, but maybe we need to > expand the API somehow. I think it should be global because an > application like gnome-screencast is not going to know which device to > use. But we may need to add a way for the service to say it's > connected now so that its not considered for any simultaneous > connections, until the first connection is closed. Well, it is explicitly allowed to run multiple WFD sessions if the infrastructure is there, even without a P2P link. So I generally agree this should be global, just pointing out that certain elements would need to be dynamic. This also (probably?) precludes us from advertising a Sink role and a Source role for example. And also makes things like doing simultaneous P2P_GO + P2P_CLIENT sort of pointless. So overall I'd say we should ignore the possibility of multiple P2P iftypes. It will be either a P2P_GO with multiple clients or a P2P_CLIENT and we're limited to 1. > > So since our assumption is that we're always initiating our > connections (or in the future responing to an invitation, but the > response is again our initiative), we could also register services > per-connection and per-scan? This would work for gnome-screencast, > since I believe it has its own dialog that shows you available devices > to connect to... not sure about other services. > We can always introduce a per phy agent that would override the global one if we really encounter a use case that required this. Or allow multiple agents to be registered and if an application triggers a connection, then the agent registered on that application's dbus unique name (if any) gets preferred for any IEs broadcast. For now I'd ignore this and assume we can only do one role at a time. >> >> Yeah, but that's the problem. What do you actually mean by >> AvailableConnections? Do you mean: >> >> 1. connections on a given P2P_CLIENT or P2P_GO interface? or >> 2. number of P2P_CLIENT & P2P_GO interfaces? > > So this should not bleed into the API. As long as we allow the user > to make another connection, it should be signalled to the user > somehow. Whether we're using a separate interface or just make > another connection from our GO interface. Then just indicate what role we have and let the application make that choice. So if we're in P2P_GO mode, it can try to establish another connection. And if we're in P2P_CLIENT mode, tough luck. > >> >> If 1, then this makes no sense anyway. For P2P_CLIENT it is always 1. >> For P2P_GO it is essentially unlimited. > > Right, but we need to inform the user somehow whether we can make > another connection with whatever role we have currently negotiated or > because we're idle. > > (and for P2P_GO we may want to set an artificial limit, it's only > unlimited in theory) Why? The hardware is the arbiter of this in the end. > >> >> For 2, I doubt you will ever find any hardware capable of doing more >> than 1 anyway. And it is a complication I wouldn't worry about now >> regardless. So if the v1 API is limited to a single connection and we >> later find out it is actually possible to do more, then we worry about >> it then. > > So from what I've seen, it is possible, but not until we support the > GO role. However our main usecase right now (I believe...) is a PC, > phone, tablet, rather than a peripheral device, so I think this is > definitely going to be expected from iwd, so are you sure you want to > limit the API to a single connection? Ah, imprecise wording here. By single connection I mean a single P2P_GO/P2P_CLIENT interface active at a time. So if we're in P2P_GO mode, we can do multiple 'connections'. But since we're not doing P2P_GO yet, then effectively we would only support a single active peer today. > > AFAIK the android API for apps doesn't have this limit for example. > Right, but they still expect apps to setup the topology appropriately. And this is maybe something you need to consider for the future: being able to force one mode vs the other. > >> >>>>> + >>>>> +Methods ConnectPushButton() >>>>> + >>>>> + Connect to the P2P peer in the Push Button mode >>>>> + using the device pointed to by the .Device property. >>>>> + Returns when connection is complete. >>>> >>>> Why are you method names different from SimpleConfiguration? in fact, >>>> why not just add .SimpleConfiguration interface to the same object path >>>> and just have clients use that? >>> >>> Could do this, yes, although it would have been nice to have "Connect" >>> in the method names. And I'm not sure how to resolve this in the code >>> since the implementation of the methods will be very different from >>> the one in wsc.c, preferably I wouldn't duplicate setup_wsc_interface. >> >> Pretty easy I would think? I mean you do have the netdev associated >> with it, so you can simply check the interface type. > > We only have a netdev after the GO Negotiation. > > Esentially I'd need to have a new struct wsc, which would encapsulate either: > > * a current struct wsc for a station interface, or > * a P2P peer that corresponds to a scan result. > > Then the wsc.c PushButton method, for instance, would need to either > do its current work or delegate it to a p2p.c function and exit. Then make wsc work more like struct network. And have it register the interface when a station is available or on request for a p2p peer. We may have to be creative. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-14 20:33 ` Denis Kenzior @ 2019-11-15 0:38 ` Andrew Zaborowski 2019-11-15 3:25 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Andrew Zaborowski @ 2019-11-15 0:38 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 9305 bytes --] On Thu, 14 Nov 2019 at 21:36, Denis Kenzior <denkenz@gmail.com> wrote: > On 11/14/19 1:37 PM, Andrew Zaborowski wrote: > >>> Yeah, I wasn't going to implement it at this time. But since you'll > >>> have RSSI in every mode of wireless connections, AP, adhoc, ... > >>> wouldn't it make sense to have a common interface and share code in > >>> iwd and in the clients? > >> > >> You don't though. For AP you have multiple clients, which rssi are you > >> going to show? > > > > That would be per client obviously, when/if information about the > > clients is added. > > And the kernel has APIs for this? AFAIK you don't even get this info > without polling. So trying to use our agent for this is not going to work. I think there's a patchset that's being rebased occasionally. But what I mean is that maybe that interface doc shouldn't be buried in station-api.txt. > > > > >> Same goes for p2p-go really. > > > > With P2P we are initiating the connections, so we have to expose > > information about each peer anyway. > > > > So lets see how that works out given the above. I think for now I would > leave all of this out. Ok. > > >> Then we can't do this as a global registration. Because some > >> sub-elements are dynamic it would be up to the application to update the > >> IE properly. I'm really not sure we can avoid building the WFD IE > >> ourselves. > > > > I think it should still be global actually, but maybe we need to > > expand the API somehow. I think it should be global because an > > application like gnome-screencast is not going to know which device to > > use. But we may need to add a way for the service to say it's > > connected now so that its not considered for any simultaneous > > connections, until the first connection is closed. > > Well, it is explicitly allowed to run multiple WFD sessions if the > infrastructure is there, even without a P2P link. > > So I generally agree this should be global, just pointing out that > certain elements would need to be dynamic. This also (probably?) > precludes us from advertising a Sink role and a Source role for example. > And also makes things like doing simultaneous P2P_GO + P2P_CLIENT sort > of pointless. When talking about the API it's only a question of whether the parameters dictionary should have to booleans, one for sink, one for source, or one "role" variable. We're not acting on that information anyway, it's only so we can encode it in the IE. > > So overall I'd say we should ignore the possibility of multiple P2P > iftypes. It will be either a P2P_GO with multiple clients or a > P2P_CLIENT and we're limited to 1. In the code I'm currently ignoring that possibilty, but talking about the API it wouldn't make much difference. > > > > > So since our assumption is that we're always initiating our > > connections (or in the future responing to an invitation, but the > > response is again our initiative), we could also register services > > per-connection and per-scan? This would work for gnome-screencast, > > since I believe it has its own dialog that shows you available devices > > to connect to... not sure about other services. > > > > We can always introduce a per phy agent that would override the global > one if we really encounter a use case that required this. Or allow > multiple agents to be registered and if an application triggers a > connection, then the agent registered on that application's dbus unique > name (if any) gets preferred for any IEs broadcast. > > For now I'd ignore this and assume we can only do one role at a time. Right. But I'm thinking if there is a whole better way of doing the service registration, i.e. per connection instead of globally. Say your use case is you're connecting first to a TV and then to a keyboard, or simultaneously (as a GO). There's no point advertising the WFD capabilities to the keyboard device or to anyone other than the TV, once your WFD session is running and functional. Maybe the ServiceManager is the wrong way to go and the Connect / PushButton method should receive the relevant information. > > >> > >> Yeah, but that's the problem. What do you actually mean by > >> AvailableConnections? Do you mean: > >> > >> 1. connections on a given P2P_CLIENT or P2P_GO interface? or > >> 2. number of P2P_CLIENT & P2P_GO interfaces? > > > > So this should not bleed into the API. As long as we allow the user > > to make another connection, it should be signalled to the user > > somehow. Whether we're using a separate interface or just make > > another connection from our GO interface. > > Then just indicate what role we have and let the application make that > choice. So if we're in P2P_GO mode, it can try to establish another > connection. And if we're in P2P_CLIENT mode, tough luck. I think it would be nicer to just prefer GO automatically (maybe without forcing it), for example by having the GO Intent default to 14 out of 15 and be configurable in main.conf. > > > > >> > >> If 1, then this makes no sense anyway. For P2P_CLIENT it is always 1. > >> For P2P_GO it is essentially unlimited. > > > > Right, but we need to inform the user somehow whether we can make > > another connection with whatever role we have currently negotiated or > > because we're idle. > > > > (and for P2P_GO we may want to set an artificial limit, it's only > > unlimited in theory) > > Why? The hardware is the arbiter of this in the end. Some dedicated access points have trouble serving 10 connections simultaneously on one radio channel so for our off the shelf hardware and minimal ap.c code it's going to be very difficult. > > > > >> > >> For 2, I doubt you will ever find any hardware capable of doing more > >> than 1 anyway. And it is a complication I wouldn't worry about now > >> regardless. So if the v1 API is limited to a single connection and we > >> later find out it is actually possible to do more, then we worry about > >> it then. > > > > So from what I've seen, it is possible, but not until we support the > > GO role. However our main usecase right now (I believe...) is a PC, > > phone, tablet, rather than a peripheral device, so I think this is > > definitely going to be expected from iwd, so are you sure you want to > > limit the API to a single connection? > > Ah, imprecise wording here. By single connection I mean a single > P2P_GO/P2P_CLIENT interface active at a time. So if we're in P2P_GO > mode, we can do multiple 'connections'. But since we're not doing > P2P_GO yet, then effectively we would only support a single active peer > today. So I assume you're Ok having the Disconnect method on the Peer interface, not on P2PDevice? So that our DBus clients are prepared to support multiple connections instead of 1? Or I wonder if we should repurpose WSC.Cancel to also disconnect a fully established connection. There's no need for separate Cancel and Disconnect methods, but Disconnect sounds more correct. > > > > > AFAIK the android API for apps doesn't have this limit for example. > > > > Right, but they still expect apps to setup the topology appropriately. > And this is maybe something you need to consider for the future: being > able to force one mode vs the other. > > > > >> > >>>>> + > >>>>> +Methods ConnectPushButton() > >>>>> + > >>>>> + Connect to the P2P peer in the Push Button mode > >>>>> + using the device pointed to by the .Device property. > >>>>> + Returns when connection is complete. > >>>> > >>>> Why are you method names different from SimpleConfiguration? in fact, > >>>> why not just add .SimpleConfiguration interface to the same object path > >>>> and just have clients use that? > >>> > >>> Could do this, yes, although it would have been nice to have "Connect" > >>> in the method names. And I'm not sure how to resolve this in the code > >>> since the implementation of the methods will be very different from > >>> the one in wsc.c, preferably I wouldn't duplicate setup_wsc_interface. > >> > >> Pretty easy I would think? I mean you do have the netdev associated > >> with it, so you can simply check the interface type. > > > > We only have a netdev after the GO Negotiation. > > > > Esentially I'd need to have a new struct wsc, which would encapsulate either: > > > > * a current struct wsc for a station interface, or > > * a P2P peer that corresponds to a scan result. > > > > Then the wsc.c PushButton method, for instance, would need to either > > do its current work or delegate it to a p2p.c function and exit. > > Then make wsc work more like struct network. And have it register the > interface when a station is available or on request for a p2p peer. We > may have to be creative. Right, I was just thinking that there's going to be little common code between the two usages of the WSC interface. For the first case it's going to be the same wsc.c code there is today, but for the latter the 3 methods will directly call into p2p.c and exit. (specifically PushButton, StartPin and Cancel) Best regards ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-15 0:38 ` Andrew Zaborowski @ 2019-11-15 3:25 ` Denis Kenzior 2019-11-15 4:02 ` Andrew Zaborowski 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2019-11-15 3:25 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 4886 bytes --] Hi Andrew, >> And the kernel has APIs for this? AFAIK you don't even get this info >> without polling. So trying to use our agent for this is not going to work. > > I think there's a patchset that's being rebased occasionally. But > what I mean is that maybe that interface doc shouldn't be buried in > station-api.txt. Sure. But until we know that this is possible this step is premature. Right now the signal agent logically belongs in station since that is the only thing using it. >> >> So I generally agree this should be global, just pointing out that >> certain elements would need to be dynamic. This also (probably?) >> precludes us from advertising a Sink role and a Source role for example. >> And also makes things like doing simultaneous P2P_GO + P2P_CLIENT sort >> of pointless. > > When talking about the API it's only a question of whether the > parameters dictionary should have to booleans, one for sink, one for > source, or one "role" variable. We're not acting on that information > anyway, it's only so we can encode it in the IE. > By the way, to answer my earlier question. It is possible to be both a Sink and a Source at the same time: "0b11: dual-role possible, i.e., either a WFD Source or a Primary Sink" >> For now I'd ignore this and assume we can only do one role at a time. > > Right. But I'm thinking if there is a whole better way of doing the > service registration, i.e. per connection instead of globally. Say > your use case is you're connecting first to a TV and then to a > keyboard, or simultaneously (as a GO). There's no point advertising > the WFD capabilities to the keyboard device or to anyone other than I actually wouldn't expect this to be a good idea. Once you start advertising a set of IEs as a P2P GO, I'm not sure changing these is really something that is expected, particularly when you already have a peer connection established. Changing the contents to signal some state is probably OK (this is done in WSC for example). But taking out IEs entirely is a bit of a gray area. See for example: WFD Session Availability bits: 0b00: Not available for WFD Session Or Section 5.2.1: "If a WFD Device acts as a P2P Group Owner, the WFD Device shall insert one or more WFD IEs after the other information elements in the Beacon frames it transmits. " I assume that at least the WFD spec wants you to twiddle the contents of the IEs and not remove IEs entirely. > the TV, once your WFD session is running and functional. Maybe the > ServiceManager is the wrong way to go and the Connect / PushButton > method should receive the relevant information. And don't you need the advertising data before you even get to the point where you can invoke operations on the Peer interface? >> Then just indicate what role we have and let the application make that >> choice. So if we're in P2P_GO mode, it can try to establish another >> connection. And if we're in P2P_CLIENT mode, tough luck. > > I think it would be nicer to just prefer GO automatically (maybe > without forcing it), for example by having the GO Intent default to 14 > out of 15 and be configurable in main.conf. > Yes, sure. What I'm saying though is that once we're connected, we should indicate the topology somehow. >> Why? The hardware is the arbiter of this in the end. > > Some dedicated access points have trouble serving 10 connections > simultaneously on one radio channel so for our off the shelf hardware > and minimal ap.c code it's going to be very difficult. And? Any limit you pick will end up being wrong for someone. >> >> Ah, imprecise wording here. By single connection I mean a single >> P2P_GO/P2P_CLIENT interface active at a time. So if we're in P2P_GO >> mode, we can do multiple 'connections'. But since we're not doing >> P2P_GO yet, then effectively we would only support a single active peer >> today. > > So I assume you're Ok having the Disconnect method on the Peer > interface, not on P2PDevice? So that our DBus clients are prepared to > support multiple connections instead of 1? I think these two things are somewhat orthogonal. We can still have Disconnect on the Device and not the peer. All that would do is preclude us from disconnecting individual links in P2P GO mode. But sure, I suppose it can remain on the Peer object. > > Or I wonder if we should repurpose WSC.Cancel to also disconnect a > fully established connection. There's no need for separate Cancel and > Disconnect methods, but Disconnect sounds more correct. > Cancel only applies to the discovery and configuration procedure. The WSC is a separate handshake. So having a separate disconnect method is still needed once you get into the actual connection process. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-15 3:25 ` Denis Kenzior @ 2019-11-15 4:02 ` Andrew Zaborowski 2019-11-15 5:08 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Andrew Zaborowski @ 2019-11-15 4:02 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 4972 bytes --] On Fri, 15 Nov 2019 at 04:27, Denis Kenzior <denkenz@gmail.com> wrote: > >> For now I'd ignore this and assume we can only do one role at a time. > > > > Right. But I'm thinking if there is a whole better way of doing the > > service registration, i.e. per connection instead of globally. Say > > your use case is you're connecting first to a TV and then to a > > keyboard, or simultaneously (as a GO). There's no point advertising > > the WFD capabilities to the keyboard device or to anyone other than > > I actually wouldn't expect this to be a good idea. Once you start > advertising a set of IEs as a P2P GO, I'm not sure changing these is > really something that is expected, particularly when you already have a > peer connection established. Changing the contents to signal some state > is probably OK (this is done in WSC for example). But taking out IEs > entirely is a bit of a gray area. Right, but passing the WFD IE in the Association Request kind of signals that you want to establish a WFD connection and you don't want it for your second connection. You can keep signalling it in your beacons, although once the user has closed the screencasting app, I wouldn't worry about just removing those IEs from the beacons even though you may still be connected to the hypothetical P2P keyboard. > > See for example: > > WFD Session Availability bits: > 0b00: Not available for WFD Session > > Or Section 5.2.1: > "If a WFD Device acts as a P2P Group Owner, the WFD Device shall insert > one or more WFD IEs after the other information elements in the Beacon > frames it transmits. " > > I assume that at least the WFD spec wants you to twiddle the contents of > the IEs and not remove IEs entirely. > > > the TV, once your WFD session is running and functional. Maybe the > > ServiceManager is the wrong way to go and the Connect / PushButton > > method should receive the relevant information. > > And don't you need the advertising data before you even get to the point > where you can invoke operations on the Peer interface? In theory you need to advertise it in your scan. Like I mentioned in another email, I think the scans should also be explicitly triggered by the apps because they're expensive, so those would also need to know the service parameters. Maybe that's too complicated for now and we better try a global P2PServiceManager thing first. > > >> Then just indicate what role we have and let the application make that > >> choice. So if we're in P2P_GO mode, it can try to establish another > >> connection. And if we're in P2P_CLIENT mode, tough luck. > > > > I think it would be nicer to just prefer GO automatically (maybe > > without forcing it), for example by having the GO Intent default to 14 > > out of 15 and be configurable in main.conf. > > > > Yes, sure. What I'm saying though is that once we're connected, we > should indicate the topology somehow. I think we should just indicate that we can still attempt more connections, optimally without exposing the P2P roles. > > >> Why? The hardware is the arbiter of this in the end. > > > > Some dedicated access points have trouble serving 10 connections > > simultaneously on one radio channel so for our off the shelf hardware > > and minimal ap.c code it's going to be very difficult. > > And? Any limit you pick will end up being wrong for someone. > > >> > >> Ah, imprecise wording here. By single connection I mean a single > >> P2P_GO/P2P_CLIENT interface active at a time. So if we're in P2P_GO > >> mode, we can do multiple 'connections'. But since we're not doing > >> P2P_GO yet, then effectively we would only support a single active peer > >> today. > > > > So I assume you're Ok having the Disconnect method on the Peer > > interface, not on P2PDevice? So that our DBus clients are prepared to > > support multiple connections instead of 1? > > I think these two things are somewhat orthogonal. We can still have > Disconnect on the Device and not the peer. All that would do is > preclude us from disconnecting individual links in P2P GO mode. But > sure, I suppose it can remain on the Peer object. > > > > > Or I wonder if we should repurpose WSC.Cancel to also disconnect a > > fully established connection. There's no need for separate Cancel and > > Disconnect methods, but Disconnect sounds more correct. > > > > Cancel only applies to the discovery and configuration procedure. The > WSC is a separate handshake. So having a separate disconnect method is > still needed once you get into the actual connection process. In any case on of the two is redundant ;) Disconnect already aborts a connection attempt (device discovery, group owner negotiation / provision discovery, provisioning or the PSK connection). Of these only the provisioning uses WSC so conceptually the WSC interface is not a great match. Best regards ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-15 4:02 ` Andrew Zaborowski @ 2019-11-15 5:08 ` Denis Kenzior 2019-11-15 12:33 ` Andrew Zaborowski 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2019-11-15 5:08 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 5282 bytes --] Hi Andrew, On 11/14/19 10:02 PM, Andrew Zaborowski wrote: > On Fri, 15 Nov 2019 at 04:27, Denis Kenzior <denkenz@gmail.com> wrote: >>>> For now I'd ignore this and assume we can only do one role at a time. >>> >>> Right. But I'm thinking if there is a whole better way of doing the >>> service registration, i.e. per connection instead of globally. Say >>> your use case is you're connecting first to a TV and then to a >>> keyboard, or simultaneously (as a GO). There's no point advertising >>> the WFD capabilities to the keyboard device or to anyone other than >> >> I actually wouldn't expect this to be a good idea. Once you start >> advertising a set of IEs as a P2P GO, I'm not sure changing these is >> really something that is expected, particularly when you already have a >> peer connection established. Changing the contents to signal some state >> is probably OK (this is done in WSC for example). But taking out IEs >> entirely is a bit of a gray area. > > Right, but passing the WFD IE in the Association Request kind of > signals that you want to establish a WFD connection and you don't want > it for your second connection. You can keep signalling it in your > beacons, although once the user has closed the screencasting app, I > wouldn't worry about just removing those IEs from the beacons even > though you may still be connected to the hypothetical P2P keyboard. > So you want two separate registrations? A global capability and a per-connection "I want to use this profile" type of thing. It sounded like you only wanted the latter above. So really, I don't see the point. If iwd understands about WFD, then it simply wouldn't send any WFD IEs to a WFD-incapable device. One can argue that the capability you propose would still be wanted in some weird situations. For example, WFD allows 'Dual mode Sink & Source' only in the Beacon and Probe Response frames. For Association, it needs to pick one role. Or maybe the Keyboard in your example is actually also WFD capable.. But then the spec mysteriously says: "Depending on the availability of the WFD Device for a WFD Session, the value for WFD Session Availability bits (B5B4) of the WFD Device Information field of WFD Device Information subelement within the WFD IE may change" in Section 4.5.2.1 "Usage of a WFD IE to establish P2P connection". So who knows what they actually mean by that? Maybe we can always send WFD IEs and just twiddle those bits off? I really wouldn't worry about these bizarre cases right now until you have something that actually works. >> >> See for example: >> >> WFD Session Availability bits: >> 0b00: Not available for WFD Session >> >> Or Section 5.2.1: >> "If a WFD Device acts as a P2P Group Owner, the WFD Device shall insert >> one or more WFD IEs after the other information elements in the Beacon >> frames it transmits. " >> >> I assume that at least the WFD spec wants you to twiddle the contents of >> the IEs and not remove IEs entirely. >> >>> the TV, once your WFD session is running and functional. Maybe the >>> ServiceManager is the wrong way to go and the Connect / PushButton >>> method should receive the relevant information. >> >> And don't you need the advertising data before you even get to the point >> where you can invoke operations on the Peer interface? > > In theory you need to advertise it in your scan. Like I mentioned in > another email, I think the scans should also be explicitly triggered > by the apps because they're expensive, so those would also need to > know the service parameters. Okay. So you need to present those details then. >> Yes, sure. What I'm saying though is that once we're connected, we >> should indicate the topology somehow. > > I think we should just indicate that we can still attempt more > connections, optimally without exposing the P2P roles. I don't know why you'd want to hide that, but I'm fine either way. >> Cancel only applies to the discovery and configuration procedure. The >> WSC is a separate handshake. So having a separate disconnect method is >> still needed once you get into the actual connection process. > > In any case on of the two is redundant ;) Disconnect already aborts a > connection attempt (device discovery, group owner negotiation / > provision discovery, provisioning or the PSK connection). I think about this differently. You have 2 phases: 1. Simple Configuration (WiFi Direct flavor) which involves the group formation and wsc negotiation 2. Actual connection establishment I don't see why Disconnect can't just affect 2 without affecting 1. The operations are on different interfaces after all. > > Of these only the provisioning uses WSC so conceptually the WSC > interface is not a great match. Honestly, I'm not convinced that reinventing the wheel and duplicating the SimpleConfiguration API on the P2P Peer is any better. Even in the classic station case WSC involves device discovery as well. So, in P2P, arguably everything up to the PSK connection establishment can be viewed as part of WSC. It sounds like you need to send a v4 before we can go further. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-15 5:08 ` Denis Kenzior @ 2019-11-15 12:33 ` Andrew Zaborowski 2019-11-15 14:51 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Andrew Zaborowski @ 2019-11-15 12:33 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 4877 bytes --] On Fri, 15 Nov 2019 at 06:10, Denis Kenzior <denkenz@gmail.com> wrote: > So you want two separate registrations? A global capability and a > per-connection "I want to use this profile" type of thing. It sounded > like you only wanted the latter above. I was rather thinking of borth Scan() and PushButton() taking the information about services to advertise but it's a complication. > > So really, I don't see the point. If iwd understands about WFD, then it > simply wouldn't send any WFD IEs to a WFD-incapable device. > > One can argue that the capability you propose would still be wanted in > some weird situations. > > For example, WFD allows 'Dual mode Sink & Source' only in the Beacon and > Probe Response frames. For Association, it needs to pick one role. > > Or maybe the Keyboard in your example is actually also WFD capable.. > But then the spec mysteriously says: > > "Depending on the availability of the WFD Device for a WFD Session, the > value for WFD Session Availability bits (B5B4) of the WFD Device > Information field of WFD Device Information subelement within the WFD IE > may change" in Section 4.5.2.1 "Usage of a WFD IE to establish P2P > connection". So who knows what they actually mean by that? Maybe we > can always send WFD IEs and just twiddle those bits off? > > I really wouldn't worry about these bizarre cases right now until you > have something that actually works. > > >> > >> See for example: > >> > >> WFD Session Availability bits: > >> 0b00: Not available for WFD Session > >> > >> Or Section 5.2.1: > >> "If a WFD Device acts as a P2P Group Owner, the WFD Device shall insert > >> one or more WFD IEs after the other information elements in the Beacon > >> frames it transmits. " > >> > >> I assume that at least the WFD spec wants you to twiddle the contents of > >> the IEs and not remove IEs entirely. > >> > >>> the TV, once your WFD session is running and functional. Maybe the > >>> ServiceManager is the wrong way to go and the Connect / PushButton > >>> method should receive the relevant information. > >> > >> And don't you need the advertising data before you even get to the point > >> where you can invoke operations on the Peer interface? > > > > In theory you need to advertise it in your scan. Like I mentioned in > > another email, I think the scans should also be explicitly triggered > > by the apps because they're expensive, so those would also need to > > know the service parameters. > > Okay. So you need to present those details then. > > >> Yes, sure. What I'm saying though is that once we're connected, we > >> should indicate the topology somehow. > > > > I think we should just indicate that we can still attempt more > > connections, optimally without exposing the P2P roles. > > I don't know why you'd want to hide that, but I'm fine either way. It sounds more like the typo of information you want on a diagnostics interface because it is not supposed to affect your connection, like your AP's BSSID. > > >> Cancel only applies to the discovery and configuration procedure. The > >> WSC is a separate handshake. So having a separate disconnect method is > >> still needed once you get into the actual connection process. > > > > In any case on of the two is redundant ;) Disconnect already aborts a > > connection attempt (device discovery, group owner negotiation / > > provision discovery, provisioning or the PSK connection). > > I think about this differently. You have 2 phases: > 1. Simple Configuration (WiFi Direct flavor) which involves the group > formation and wsc negotiation Well, it simply isn't part of WSC, but ok. > 2. Actual connection establishment > > I don't see why Disconnect can't just affect 2 without affecting 1. The > operations are on different interfaces after all. We just don't need 2 methods and I already have Disconnect working this way (consistent with Station.Disconnect) so it'd mean just artificially crippling it, also adding a race where your Cancel call may work or not. (Tecnically provisioning and the connection are both on the P2P_CLIENT interface) > > > > > Of these only the provisioning uses WSC so conceptually the WSC > > interface is not a great match. > > Honestly, I'm not convinced that reinventing the wheel and duplicating > the SimpleConfiguration API on the P2P Peer is any better. > > Even in the classic station case WSC involves device discovery as well. > So, in P2P, arguably everything up to the PSK connection establishment > can be viewed as part of WSC. > > It sounds like you need to send a v4 before we can go further. Right, I'm still not sure about how discovery should be triggered but I can go on with the code with the answers and ideas I got from you already. Best regards ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-15 12:33 ` Andrew Zaborowski @ 2019-11-15 14:51 ` Denis Kenzior 2019-11-15 15:10 ` Andrew Zaborowski 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2019-11-15 14:51 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 1191 bytes --] Hi Andrew, >> I think about this differently. You have 2 phases: >> 1. Simple Configuration (WiFi Direct flavor) which involves the group >> formation and wsc negotiation > > Well, it simply isn't part of WSC, but ok. > P2P in the end just took WSC and changed the discovery bits a bit. Fundamentally its still WSC in the end. But I guess depends on how you think about it... >> 2. Actual connection establishment >> >> I don't see why Disconnect can't just affect 2 without affecting 1. The >> operations are on different interfaces after all. > > We just don't need 2 methods and I already have Disconnect working > this way (consistent with Station.Disconnect) so it'd mean just > artificially crippling it, also adding a race where your Cancel call > may work or not. Station.Disconnect does not affect WSC state FYI. It can't since Station doesn't even know about the ongoing WSC process until after station_connect_network is invoked. So in that respect we'd be completely consistent. > > (Tecnically provisioning and the connection are both on the P2P_CLIENT > interface) > Same as Station/WSC today... Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-15 14:51 ` Denis Kenzior @ 2019-11-15 15:10 ` Andrew Zaborowski 2019-11-15 15:25 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Andrew Zaborowski @ 2019-11-15 15:10 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 1544 bytes --] On Fri, 15 Nov 2019 at 15:53, Denis Kenzior <denkenz@gmail.com> wrote: > >> I think about this differently. You have 2 phases: > >> 1. Simple Configuration (WiFi Direct flavor) which involves the group > >> formation and wsc negotiation > > > > Well, it simply isn't part of WSC, but ok. > > > > P2P in the end just took WSC and changed the discovery bits a bit. > Fundamentally its still WSC in the end. But I guess depends on how you > think about it... > > >> 2. Actual connection establishment > >> > >> I don't see why Disconnect can't just affect 2 without affecting 1. The > >> operations are on different interfaces after all. > > > > We just don't need 2 methods and I already have Disconnect working > > this way (consistent with Station.Disconnect) so it'd mean just > > artificially crippling it, also adding a race where your Cancel call > > may work or not. > > Station.Disconnect does not affect WSC state FYI. It can't since > Station doesn't even know about the ongoing WSC process until after > station_connect_network is invoked. So in that respect we'd be > completely consistent. What I meant is that Disconnect() can cancel your station connection attempt at any point, while with Peer.Disconnect() you're going to have to be more careful for no reason. > > > > > (Tecnically provisioning and the connection are both on the P2P_CLIENT > > interface) > > > > Same as Station/WSC today... I was just noting they're not happening on different interfaces. Best regards ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-15 15:10 ` Andrew Zaborowski @ 2019-11-15 15:25 ` Denis Kenzior 2019-11-15 22:35 ` Andrew Zaborowski 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2019-11-15 15:25 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 1381 bytes --] Hi Andrew, >> Station.Disconnect does not affect WSC state FYI. It can't since >> Station doesn't even know about the ongoing WSC process until after >> station_connect_network is invoked. So in that respect we'd be >> completely consistent. > > What I meant is that Disconnect() can cancel your station connection > attempt at any point, while with Peer.Disconnect() you're going to > have to be more careful for no reason. You will have to explain that then, because I don't get it? How is Peer.Disconnect any different? Your 'Peer' state doesn't change to connecting until after WSC runs. And if we become a GO, then things become even more funny since we can only send invites, not actually initiate a connection. So all these methods become somewhat irrelevant. In fact, I wonder if we should be removing the WSC interface when we become the GO and adding another one for performing invitations. > I was just noting they're not happening on different interfaces. Of course. That's because P2P_CLIENT is just a station iftype under the hood in the kernel with some extra stuff bolted on. But we knew that ;) Maybe what we should do is just implement the bare minimum client side API, learn from it and assume that whatever API we come up with now will likely not survive when we start implementing GO side. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-15 15:25 ` Denis Kenzior @ 2019-11-15 22:35 ` Andrew Zaborowski 2019-11-16 2:27 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Andrew Zaborowski @ 2019-11-15 22:35 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 3202 bytes --] Hi Denis, On Fri, 15 Nov 2019 at 16:27, Denis Kenzior <denkenz@gmail.com> wrote: > >> Station.Disconnect does not affect WSC state FYI. It can't since > >> Station doesn't even know about the ongoing WSC process until after > >> station_connect_network is invoked. So in that respect we'd be > >> completely consistent. > > > > What I meant is that Disconnect() can cancel your station connection > > attempt at any point, while with Peer.Disconnect() you're going to > > have to be more careful for no reason. > > You will have to explain that then, because I don't get it? How is > Peer.Disconnect any different? Your 'Peer' state doesn't change to > connecting until after WSC runs. That's needlessly complicating things for the users. From the user's point of view there's a device they want to connect to and use it. They call a single method and get a connection established, and if they need to interrupt the process then it's still single process. You could split it in any number of ways, as before WSC there's a bunch of other things happening, but from the user's point of view those are details they don't want to see. WSC is one of 4 or 5 steps (you can make up more) in the P2P connection but there's no benefit from making the user aware of them. > And if we become a GO, then things > become even more funny since we can only send invites, not actually > initiate a connection. Invitation is just a technical name for a frame sequence, group formation is another, but it's just ways to connect to a device. Note how Android shows the same Accept/Decline dialog whether you're using group formation or invitation. (same or similar... I thought the wording was the same but don't have screenshots at hand) On a similar note: The P2P devices I've seen don't have a physical WSC button (not surprising), but they don't even have a virtual button or the word Push Button anywhere in the user interfaces. Take for example Android connecting to a wifi-display TV, neither mentions any WSC term like Push Button in their UIs. The verbs are connect and accept/decline connection (and similar). WSC is really just a protocol used at the network level and an implementation detail, as are the P2P roles. > So all these methods become somewhat irrelevant. > In fact, I wonder if we should be removing the WSC interface when we > become the GO and adding another one for performing invitations. > > > I was just noting they're not happening on different interfaces. > > Of course. Oh so this was just a typo I was replying to I guess. > That's because P2P_CLIENT is just a station iftype under the > hood in the kernel with some extra stuff bolted on. But we knew that ;) > > Maybe what we should do is just implement the bare minimum client side > API, learn from it and assume that whatever API we come up with now will > likely not survive when we start implementing GO side. There really shouldn't be any difference to the user. > > Regards, > -Denis > _______________________________________________ > iwd mailing list -- iwd(a)lists.01.org > To unsubscribe send an email to iwd-leave(a)lists.01.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-15 22:35 ` Andrew Zaborowski @ 2019-11-16 2:27 ` Denis Kenzior 2019-11-16 3:02 ` Andrew Zaborowski 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2019-11-16 2:27 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 1480 bytes --] Hi Andrew, >> And if we become a GO, then things >> become even more funny since we can only send invites, not actually >> initiate a connection. > > Invitation is just a technical name for a frame sequence, group > formation is another, but it's just ways to connect to a device. Note > how Android shows the same Accept/Decline dialog whether you're using > group formation or invitation. (same or similar... I thought the > wording was the same but don't have screenshots at hand) - How are you handling the case where the Peer wants us to display our PIN? And provision discovery in general? - Invitations? - invitation by a P2P Device (not group owner) of another P2P Device? - The fact that GO has a bunch of stats about its clients and you probably want to expose that similarly to AP mode? - The fact that GO has to handle connections from legacy clients and thus needs to provide legacy WPS methods? - persistent groups, with custom or saved credentials? To me it seems quite impractical to mix GO and client and to try and hide these details. I have very strong doubts you will succeed here. And I also think you're not avoiding exposing topology details. For example, is Peer.Disconnect resulting in a disconnection of a single peer or group tear down? I think you really need to send a v4. I just don't see how you're handling many of the P2P concepts, much less P2P-GO specific ones in this one. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-16 2:27 ` Denis Kenzior @ 2019-11-16 3:02 ` Andrew Zaborowski 2019-11-16 4:10 ` Denis Kenzior 0 siblings, 1 reply; 19+ messages in thread From: Andrew Zaborowski @ 2019-11-16 3:02 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 3239 bytes --] On Sat, 16 Nov 2019 at 03:28, Denis Kenzior <denkenz@gmail.com> wrote: > >> And if we become a GO, then things > >> become even more funny since we can only send invites, not actually > >> initiate a connection. > > > > Invitation is just a technical name for a frame sequence, group > > formation is another, but it's just ways to connect to a device. Note > > how Android shows the same Accept/Decline dialog whether you're using > > group formation or invitation. (same or similar... I thought the > > wording was the same but don't have screenshots at hand) > > - How are you handling the case where the Peer wants us to display our > PIN? And provision discovery in general? We can add a signal for this, but this is independent of the role and not a high priority. Do we have a use case for this? > - Invitations? > - invitation by a P2P Device (not group owner) of another P2P Device? We don't need to unless we're also going to be running on peripheral devices, but that's not our current use case. > - The fact that GO has a bunch of stats about its clients and you > probably want to expose that similarly to AP mode? We don't expose anything in AP mode? Our main use case is to provide bluetooth-like connections to devices. > - The fact that GO has to handle connections from legacy clients and > thus needs to provide legacy WPS methods? Is there a use case for this? > - persistent groups, with custom or saved credentials? We will need to start saving credentials because that's something that's actually in use, and we need to do this both on the GO and client. > > To me it seems quite impractical to mix GO and client and to try and > hide these details. I have very strong doubts you will succeed here. > > And I also think you're not avoiding exposing topology details. For > example, is Peer.Disconnect resulting in a disconnection of a single > peer or group tear down? Assuming that is the last peer in that group then either will work, I don't see the difference here. If there are more peers then obviously you don't want to break things by disconnecting them? > > I think you really need to send a v4. I just don't see how you're Yes, we agreed on that, although I'm still missing any reasoning for the Cancel method for example. If you have ideas on how to handle the following I'd also welcome them: * how to best signal whether the device is available to start a new connection, assuming you didn't like the AvailableConnections (int) property. * how to trigger device discovery, optimally in a way that forces the client to stop it after a while, basically to make sure it's not left running when the UI isn't active. For example for our "agents" we automatically detect if they're disconnected, here we probably also want the client to "own" the scan in some way. > handling many of the P2P concepts, much less P2P-GO specific ones in > this one. What are some P2P-GO specific concepts that would affect the DBus API? As long as we provide for having multiple active connections, and we allow for new properties on the Peer interface in the future, we should be fine. Best regards ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-16 3:02 ` Andrew Zaborowski @ 2019-11-16 4:10 ` Denis Kenzior 2019-11-16 23:57 ` Andrew Zaborowski 0 siblings, 1 reply; 19+ messages in thread From: Denis Kenzior @ 2019-11-16 4:10 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 5371 bytes --] Hi Andrew, On 11/15/19 9:02 PM, Andrew Zaborowski wrote: > On Sat, 16 Nov 2019 at 03:28, Denis Kenzior <denkenz@gmail.com> wrote: >>>> And if we become a GO, then things >>>> become even more funny since we can only send invites, not actually >>>> initiate a connection. >>> >>> Invitation is just a technical name for a frame sequence, group >>> formation is another, but it's just ways to connect to a device. Note >>> how Android shows the same Accept/Decline dialog whether you're using >>> group formation or invitation. (same or similar... I thought the >>> wording was the same but don't have screenshots at hand) >> >> - How are you handling the case where the Peer wants us to display our >> PIN? And provision discovery in general? > > We can add a signal for this, but this is independent of the role and > not a high priority. It sort of is. How is the user notified that some other device is trying to connect to us? How do they know its time to enter the password or just display it on the screen? And a signal is not enough as the UI might want to perform an authorization step here as well. > > Do we have a use case for this? Absolutely. > >> - Invitations? >> - invitation by a P2P Device (not group owner) of another P2P Device? > > We don't need to unless we're also going to be running on peripheral > devices, but that's not our current use case. The latter is optional and of dubious value. The former might be kind of important. > >> - The fact that GO has a bunch of stats about its clients and you >> probably want to expose that similarly to AP mode? > > We don't expose anything in AP mode? > AP mode isn't really finished. We need to add this some day. > Our main use case is to provide bluetooth-like connections to devices. > >> - The fact that GO has to handle connections from legacy clients and >> thus needs to provide legacy WPS methods? > > Is there a use case for this? > If you want to pass conformance tests it is. See Table 7: GOUT Protocol Conformance Tests in the Wi-Fi Direct Test plan. >> - persistent groups, with custom or saved credentials? > > We will need to start saving credentials because that's something > that's actually in use, and we need to do this both on the GO and > client. Fair enough. > >> >> To me it seems quite impractical to mix GO and client and to try and >> hide these details. I have very strong doubts you will succeed here. >> >> And I also think you're not avoiding exposing topology details. For >> example, is Peer.Disconnect resulting in a disconnection of a single >> peer or group tear down? > > Assuming that is the last peer in that group then either will work, I > don't see the difference here. But you may want to keep the group around. Anyway, this is not a biggie but I think you're making an assumption this would never be wanted. > > If there are more peers then obviously you don't want to break things > by disconnecting them? Sure, this one is obvious. > >> >> I think you really need to send a v4. I just don't see how you're > > Yes, we agreed on that, although I'm still missing any reasoning for > the Cancel method for example. If you have ideas on how to handle the That's just how Station & WSC work today, and actually work pretty well compared to any other WPS API I saw. I don't see why following the same pattern is so bad? In the end the user still invokes a single method and gets connected. Really, all your criticisms apply to station/wsc as well. But then aborts are sort of an uncommon case anyway. > following I'd also welcome them: > > * how to best signal whether the device is available to start a new connection, > assuming you didn't like the AvailableConnections (int) property. At the moment I think exposing the role is the way to go. But feel free to convince me otherwise. > > * how to trigger device discovery, optimally in a way that forces the > client to stop it after > a while, basically to make sure it's not left running when the UI > isn't active. For > example for our "agents" we automatically detect if they're > disconnected, here we > probably also want the client to "own" the scan in some way. One idea would be to add a StartDiscovery / StopDiscovery methods and track client state. So if a client drops off the bus, then we automatically stop discovery sessions initiated by it. This could also be used to send parameters for the discovery. E.g. only WFD devices. Each distinct dbus client (i.e. dbus unique service name) can set its own parameters for peers it is interested in. However, these commands might need to be limited if we're in some existing connection or not. You're the expert on the details here. > >> handling many of the P2P concepts, much less P2P-GO specific ones in >> this one. > > What are some P2P-GO specific concepts that would affect the DBus API? > As long as we provide for having multiple active connections, and we > allow for new properties on the Peer interface in the future, we > should be fine. > I think this was already covered. If you can unify the API, great. I think there are some nasty stumbling blocks though. Regards, -Denis ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFCv3] Document P2P dbus interfaces 2019-11-16 4:10 ` Denis Kenzior @ 2019-11-16 23:57 ` Andrew Zaborowski 0 siblings, 0 replies; 19+ messages in thread From: Andrew Zaborowski @ 2019-11-16 23:57 UTC (permalink / raw) To: iwd [-- Attachment #1: Type: text/plain, Size: 6489 bytes --] Hi Denis, On Sat, 16 Nov 2019 at 05:12, Denis Kenzior <denkenz@gmail.com> wrote: > On 11/15/19 9:02 PM, Andrew Zaborowski wrote: > > On Sat, 16 Nov 2019 at 03:28, Denis Kenzior <denkenz@gmail.com> wrote: > >>>> And if we become a GO, then things > >>>> become even more funny since we can only send invites, not actually > >>>> initiate a connection. > >>> > >>> Invitation is just a technical name for a frame sequence, group > >>> formation is another, but it's just ways to connect to a device. Note > >>> how Android shows the same Accept/Decline dialog whether you're using > >>> group formation or invitation. (same or similar... I thought the > >>> wording was the same but don't have screenshots at hand) > >> > >> - How are you handling the case where the Peer wants us to display our > >> PIN? And provision discovery in general? > > > > We can add a signal for this, but this is independent of the role and > > not a high priority. > > It sort of is. How is the user notified that some other device is > trying to connect to us? How do they know its time to enter the > password or just display it on the screen? Device to device connections weren't really my concern for this iteration of the API, but I think we may have even talked about having an optional agent to receive the invitations / new connections. I guess your use case is something like file sharing? Note that there's no standard service for this, you need to choose one of a hundred apps incompatible between each other, and you need to have a compatible app on the other end. > > And a signal is not enough as the UI might want to perform an > authorization step here as well. > > > > > Do we have a use case for this? > > Absolutely. > > > > >> - Invitations? > >> - invitation by a P2P Device (not group owner) of another P2P Device? > > > > We don't need to unless we're also going to be running on peripheral > > devices, but that's not our current use case. > > The latter is optional and of dubious value. The former might be kind > of important. > > > > >> - The fact that GO has a bunch of stats about its clients and you > >> probably want to expose that similarly to AP mode? > > > > We don't expose anything in AP mode? > > > > AP mode isn't really finished. We need to add this some day. Things like bandwidth usage, IP? They should probably use the same property names for both AP and P2P. > > > Our main use case is to provide bluetooth-like connections to devices. > > > >> - The fact that GO has to handle connections from legacy clients and > >> thus needs to provide legacy WPS methods? > > > > Is there a use case for this? > > > > If you want to pass conformance tests it is. See Table 7: GOUT Protocol > Conformance Tests in the Wi-Fi Direct Test plan. I can't find this document right now but I assume there's a few more things that may be not worth the effort implementing and are required for certification. > > >> - persistent groups, with custom or saved credentials? > > > > We will need to start saving credentials because that's something > > that's actually in use, and we need to do this both on the GO and > > client. > > Fair enough. > > > > >> > >> To me it seems quite impractical to mix GO and client and to try and > >> hide these details. I have very strong doubts you will succeed here. > >> > >> And I also think you're not avoiding exposing topology details. For > >> example, is Peer.Disconnect resulting in a disconnection of a single > >> peer or group tear down? > > > > Assuming that is the last peer in that group then either will work, I > > don't see the difference here. > > But you may want to keep the group around. Anyway, this is not a > biggie but I think you're making an assumption this would never be wanted. It may be wanted or it may be the default if we manage to become the GO (the peer may in theory require the GO role). But it may be a config setting. > > > > > If there are more peers then obviously you don't want to break things > > by disconnecting them? > > Sure, this one is obvious. > > > > >> > >> I think you really need to send a v4. I just don't see how you're > > > > Yes, we agreed on that, although I'm still missing any reasoning for > > the Cancel method for example. If you have ideas on how to handle the > > That's just how Station & WSC work today, and actually work pretty well > compared to any other WPS API I saw. I don't see why following the same > pattern is so bad? In the end the user still invokes a single method > and gets connected. > > Really, all your criticisms apply to station/wsc as well. But then > aborts are sort of an uncommon case anyway. > > > following I'd also welcome them: > > > > * how to best signal whether the device is available to start a new connection, > > assuming you didn't like the AvailableConnections (int) property. > > At the moment I think exposing the role is the way to go. But feel free > to convince me otherwise. So I was rather thinking of one simple property to indicate if we can start a new connection and/or how many. This would combine at least: * our P2P role, * whether we're already connected (in other words whether we have as many active connections as max allowed P2P interfaces...) * whether we're currently connecting/disconnecting as a P2P client (during those operations the Connected property will be false in my current design) * whether we're currently in Group Formation in which case we're not allowed do anything as either client or GO. > > > > > * how to trigger device discovery, optimally in a way that forces the > > client to stop it after > > a while, basically to make sure it's not left running when the UI > > isn't active. For > > example for our "agents" we automatically detect if they're > > disconnected, here we > > probably also want the client to "own" the scan in some way. > > One idea would be to add a StartDiscovery / StopDiscovery methods and > track client state. So if a client drops off the bus, then we > automatically stop discovery sessions initiated by it. This could also > be used to send parameters for the discovery. E.g. only WFD devices. > Each distinct dbus client (i.e. dbus unique service name) can set its > own parameters for peers it is interested in. Ok. Best regards ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-11-16 23:57 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-10 20:51 [RFCv3] Document P2P dbus interfaces Andrew Zaborowski 2019-11-14 4:40 ` Denis Kenzior 2019-11-14 16:59 ` Andrew Zaborowski 2019-11-14 18:19 ` Denis Kenzior 2019-11-14 19:37 ` Andrew Zaborowski 2019-11-14 20:33 ` Denis Kenzior 2019-11-15 0:38 ` Andrew Zaborowski 2019-11-15 3:25 ` Denis Kenzior 2019-11-15 4:02 ` Andrew Zaborowski 2019-11-15 5:08 ` Denis Kenzior 2019-11-15 12:33 ` Andrew Zaborowski 2019-11-15 14:51 ` Denis Kenzior 2019-11-15 15:10 ` Andrew Zaborowski 2019-11-15 15:25 ` Denis Kenzior 2019-11-15 22:35 ` Andrew Zaborowski 2019-11-16 2:27 ` Denis Kenzior 2019-11-16 3:02 ` Andrew Zaborowski 2019-11-16 4:10 ` Denis Kenzior 2019-11-16 23:57 ` Andrew Zaborowski
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.