From: Denis Kenzior <denkenz@gmail.com>
To: iwd@lists.01.org
Subject: Re: [PATCH 06/17] p2p: Add device enable/disable logic
Date: Fri, 17 Apr 2020 16:41:39 +0000 [thread overview]
Message-ID: <e2eebf0a-21ef-fe32-0f59-0fdeea4b47b2@gmail.com> (raw)
In-Reply-To: <20200403161430.14168-6-andrew.zaborowski@intel.com>
[-- Attachment #1: Type: text/plain, Size: 6656 bytes --]
Hi Andrew,
On 4/3/20 11:14 AM, Andrew Zaborowski wrote:
> Implement the Enabled property on device interface. The P2P device is
> currently disabled on startup but automatically enabling the P2P device
> can be considered.
> ---
> src/p2p.c | 136 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 136 insertions(+)
>
> diff --git a/src/p2p.c b/src/p2p.c
> index ab21239d..fa8e0e55 100644
> --- a/src/p2p.c
> +++ b/src/p2p.c
> @@ -58,13 +58,19 @@
> struct p2p_device {
> uint64_t wdev_id;
> uint8_t addr[6];
> + struct l_genl_family *nl80211;
> struct wiphy *wiphy;
> unsigned int connections_left;
> struct p2p_capability_attr capability;
> struct p2p_device_info_attr device_info;
> + uint32_t start_stop_cmd_id;
> + l_dbus_property_complete_cb_t pending_complete;
> + struct l_dbus_message *pending_message;
>
> uint8_t country[3];
> struct l_queue *peer_list;
> +
> + bool enabled : 1;
> };
>
> struct p2p_peer {
> @@ -269,6 +275,78 @@ static void p2p_peer_put(void *user_data)
> p2p_peer_free(peer);
> }
>
> +static void p2p_device_enable_cb(struct l_genl_msg *msg, void *user_data)
> +{
> + struct p2p_device *dev = user_data;
> + int error = l_genl_msg_get_error(msg);
> + struct l_dbus_message *message = dev->pending_message;
> +
> + l_debug("START/STOP_P2P_DEVICE: %s (%i)", strerror(-error), -error);
> +
> + if (error)
> + goto done;
> +
> + dev->enabled = !dev->enabled;
> +
> + if (!message)
> + l_dbus_property_changed(dbus_get_bus(),
> + p2p_device_get_path(dev),
> + IWD_P2P_INTERFACE, "Enabled");
Whats this doing? Shouldn't the property always be emitted?
> +
> +done:
> + if (message) {
> + dev->pending_complete(dbus_get_bus(), message,
> + error ? dbus_error_failed(message) :
> + NULL);
> + dev->pending_message = NULL;
> + dev->pending_complete = NULL;
> + }
What this is doing is also not quite clear? Are you going to be
invoking this outside the D-Bus path?
> +}
> +
> +static void p2p_device_enable_destroy(void *user_data)
> +{
> + struct p2p_device *dev = user_data;
> +
> + dev->start_stop_cmd_id = 0;
> +}
> +
> +static bool p2p_device_start_stop(struct p2p_device *dev,
> + l_dbus_property_complete_cb_t complete,
> + struct l_dbus_message *message)
> +{
> + struct l_genl_msg *cmd;
> +
> + if (!dev->enabled)
> + cmd = l_genl_msg_new_sized(NL80211_CMD_START_P2P_DEVICE, 16);
> + else
> + cmd = l_genl_msg_new_sized(NL80211_CMD_STOP_P2P_DEVICE, 16);
> +
> + l_genl_msg_append_attr(cmd, NL80211_ATTR_WDEV, 8, &dev->wdev_id);
> +
> + dev->start_stop_cmd_id = l_genl_family_send(dev->nl80211, cmd,
> + p2p_device_enable_cb, dev,
> + p2p_device_enable_destroy);
> + if (!dev->start_stop_cmd_id) {
> + l_genl_msg_unref(cmd);
> + complete(dbus_get_bus(), message, dbus_error_failed(message));
> + return true;
> + }
> +
> + dev->pending_message = message;
> + dev->pending_complete = complete;
> +
> + if (dev->enabled) {
> + /*
> + * Stopping the P2P device, drop all peers as we can't start
> + * new connections from now on.
> + */
> + l_queue_destroy(dev->peer_list, p2p_peer_put);
> + dev->peer_list = NULL;
> + }
> +
> + return true;
> +}
Note that this function always returns true...
> +
> struct p2p_device *p2p_device_update_from_genl(struct l_genl_msg *msg,
> bool create)
> {
> @@ -350,6 +428,7 @@ struct p2p_device *p2p_device_update_from_genl(struct l_genl_msg *msg,
> dev = l_new(struct p2p_device, 1);
> dev->wdev_id = *wdev_id;
> memcpy(dev->addr, ifaddr, ETH_ALEN);
> + dev->nl80211 = l_genl_family_new(iwd_get_genl(), NL80211_GENL_NAME);
> dev->wiphy = wiphy;
> gethostname(hostname, sizeof(hostname));
> dev->connections_left = 1;
> @@ -510,8 +589,23 @@ static void p2p_device_free(void *user_data)
> {
> struct p2p_device *dev = user_data;
>
> + if (dev->pending_message) {
> + struct l_dbus_message *reply =
> + dbus_error_aborted(dev->pending_message);
> +
> + if (dev->pending_complete)
Can this ever be NULL? I checked and at least in this series it doesn't
seem to be the case...
> + dev->pending_complete(dbus_get_bus(),
> + dev->pending_message, reply);
> + else
> + dbus_pending_reply(&dev->pending_message, reply);
> +
> + dev->pending_message = NULL;
> + dev->pending_complete = NULL;
> + }
> +
> l_dbus_unregister_object(dbus_get_bus(), p2p_device_get_path(dev));
> l_queue_destroy(dev->peer_list, p2p_peer_put);
> + l_genl_family_free(dev->nl80211); /* Cancels dev->start_stop_cmd_id */
> l_free(dev);
> }
>
> @@ -524,6 +618,45 @@ bool p2p_device_destroy(struct p2p_device *dev)
> return true;
> }
>
> +static bool p2p_device_get_enabled(struct l_dbus *dbus,
> + struct l_dbus_message *message,
> + struct l_dbus_message_builder *builder,
> + void *user_data)
> +{
> + struct p2p_device *dev = user_data;
> + bool enabled = dev->enabled;
> +
> + l_dbus_message_builder_append_basic(builder, 'b', &enabled);
> +
> + return true;
> +}
> +
> +static struct l_dbus_message *p2p_device_set_enabled(struct l_dbus *dbus,
> + struct l_dbus_message *message,
> + struct l_dbus_message_iter *new_value,
> + l_dbus_property_complete_cb_t complete,
> + void *user_data)
> +{
> + struct p2p_device *dev = user_data;
> + bool new_enabled;
> +
> + if (!l_dbus_message_iter_get_variant(new_value, "b", &new_enabled))
> + return dbus_error_invalid_args(message);
> +
> + if (dev->start_stop_cmd_id || dev->pending_message)
> + return dbus_error_busy(message);
> +
> + if (dev->enabled == new_enabled) {
> + complete(dbus, message, NULL);
> + return NULL;
> + }
> +
> + if (!p2p_device_start_stop(dev, complete, message))
> + return dbus_error_failed(message);
This can never happen? p2p_device_start_stop always returns true...
> +
> + return NULL;
> +}
> +
> static bool p2p_device_get_name(struct l_dbus *dbus,
> struct l_dbus_message *message,
> struct l_dbus_message_builder *builder,
> @@ -616,6 +749,9 @@ static struct l_dbus_message *p2p_device_get_peers(struct l_dbus *dbus,
>
> static void p2p_interface_setup(struct l_dbus_interface *interface)
> {
> + l_dbus_interface_property(interface, "Enabled", 0, "b",
> + p2p_device_get_enabled,
> + p2p_device_set_enabled);
> l_dbus_interface_property(interface, "Name", 0, "s",
> p2p_device_get_name,
> p2p_device_set_name);
>
Regards,
-Denis
next prev parent reply other threads:[~2020-04-17 16:41 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-03 16:14 [PATCH 01/17] dbus: Add P2P interface name defines Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 02/17] Add minimal p2p.c and p2p.h Andrew Zaborowski
2020-04-17 15:19 ` Denis Kenzior
2020-04-17 21:01 ` Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 03/17] manager: Create/destroy P2P devices Andrew Zaborowski
2020-04-17 15:30 ` Denis Kenzior
2020-04-03 16:14 ` [PATCH 04/17] p2p: Add peer WSC device type properties Andrew Zaborowski
2020-04-17 17:33 ` Denis Kenzior
2020-04-17 20:55 ` Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 05/17] p2p: Add main device settings Andrew Zaborowski
2020-04-17 16:26 ` Denis Kenzior
2020-04-17 23:02 ` Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 06/17] p2p: Add device enable/disable logic Andrew Zaborowski
2020-04-17 16:41 ` Denis Kenzior [this message]
2020-04-25 8:45 ` Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 07/17] p2p: Add the Scan Phase Andrew Zaborowski
2020-04-17 17:27 ` Denis Kenzior
2020-04-03 16:14 ` [PATCH 08/17] p2p: Add the Listen State Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 09/17] p2p: Add the WSC interface on peer DBus objects Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 10/17] p2p: Build and send the GO Negotiation Request Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 11/17] p2p: Handle GO Negotiation Response, send Confirmation Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 12/17] p2p: Handle the Information Not Available response code Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 13/17] p2p: Respond to Probe Reqs when waiting for GO negotiation Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 14/17] p2p: Add the Provision Discovery frame sequence Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 15/17] p2p: Scan for the provision BSS Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 16/17] p2p: Create the P2P-Client interface Andrew Zaborowski
2020-04-03 16:14 ` [PATCH 17/17] p2p: WSC client provisioning and connection Andrew Zaborowski
2020-04-17 15:16 ` [PATCH 01/17] dbus: Add P2P interface name defines Denis Kenzior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e2eebf0a-21ef-fe32-0f59-0fdeea4b47b2@gmail.com \
--to=denkenz@gmail.com \
--cc=iwd@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox