[RFC 23/40] wpas P2P: add group started notification

[Johannes Berg]

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
network reference.

> 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.

johannes