[RFC 23/40] wpas P2P: add group started notification
johannes at sipsolutions.net
Wed Mar 16 10:49:24 EDT 2011
On Wed, 2011-03-16 at 15:50 +0200, Jouni Malinen wrote:
> On Wed, Jan 05, 2011 at 08:53:19PM +0100, Johannes Berg wrote:
> > +void wpas_notify_p2p_group_started(struct wpa_supplicant *wpa_s,
> > + struct wpa_ssid *ssid, int network_id,
> > + int self_is_go)
> What is the expected value in network_id? What would it be used for?
Hmm, we do use it to create a DBus network path, so it's used as a
> Please note that it may or may not be the same as ssid->id.
Yeah I believe that's why it exists, if
wpas_p2p_store_persistent_group() fails we still want a network ID to
point to in Dbus.
> Why is
> self_is_go needed here? Isn't that clear from ssid?
It probably is. I'm not 100% familiar with all the details.
> > @@ -393,7 +393,8 @@ static void wpas_p2p_store_persistent_group(struct wpa_supplicant *wpa_s,
> > changed = 1;
> > s = wpa_config_add_network(wpa_s->conf);
> > if (s == NULL)
> > - return;
> > + return -1;
> > + wpas_notify_network_added(wpa_s, s);
> Hmm.. Do we really want to handle the persistent P2P parameters in the
> same way as any network block? This cannot be selected or used in the
> same way as the other networks (that's why there are all the exceptions
> in ctrl_iface.c for ssid->disabled == 2).
We already pretty much treat them as networks, and you have to load and
select them that way too to re-invoke persistent groups. Not sure what
we should do instead?
> > @@ -444,6 +447,8 @@ static void wpas_group_formation_completed(struct wpa_supplicant *wpa_s,
> > int client;
> > int persistent;
> > u8 go_dev_addr[ETH_ALEN];
> > + int network_id = -1;
> > + int self_is_go = 0;
> Storing a separate self_is_go is kind of pointless here since !client
> has the same value..
Makes sense. I'll go look.
> > if (persistent)
> > - wpas_p2p_store_persistent_group(wpa_s->parent, ssid,
> > - go_dev_addr);
> > + network_id = wpas_p2p_store_persistent_group(wpa_s->parent,
> > + ssid, go_dev_addr);
> > + if (network_id < 0)
> > + network_id = ssid->id;
> > + if (self_is_go)
> > + wpas_notify_p2p_group_started(wpa_s, ssid, network_id, 1);
> This network_id usage looks a bit odd and the reason for me asking the
> expected meaning of network_id above.
Right -- see above.
More information about the HostAP