P2P Device support: how to deal with p2p_no_group_iface option

P2P Device support: how to deal with p2p_no_group_iface option

[Arend van Spriel]

Hi Jouni,

I looked into issues around hwsim p2p tests and got all but one p2p test 
passing now (test_autogo_tdls). I had to remove the p2p_no_group_iface 
option from the p2px.conf files, because otherwise it tries to change 
the P2P management interface into a P2P group interface. When using 
P2P_DEVICE and P2P management interface this is not allowed by 
mac80211_hwsim. Also for brcmfmac the P2P_DEVICE interface is dedicated 
and can only be added/deleted, but not changed. Not sure if that is true 
for iwlmvm as well.

So I would like to discuss how to deal with the p2p_no_group_iface 
option. As P2P_DEVICE is a new concept the name of the option may no 
longer match what it intends. Is the option to force all P2P operations 
to be done on a single interface, ie. wlan0 (or whatever is specified on 
the command line) and no P2P_DEVICE is to be created. Or should it 
change the interface from the command line as P2P group interface.

If we can make a decision here I can finalize the P2P_DEVICE support 
changes.

Regards,
Arend

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Johannes Berg]

Hi,

> I looked into issues around hwsim p2p tests and got all but one p2p test 
> passing now (test_autogo_tdls). I had to remove the p2p_no_group_iface 
> option from the p2px.conf files, because otherwise it tries to change 
> the P2P management interface into a P2P group interface. When using 
> P2P_DEVICE and P2P management interface this is not allowed by 
> mac80211_hwsim. Also for brcmfmac the P2P_DEVICE interface is dedicated 
> and can only be added/deleted, but not changed. Not sure if that is true 
> for iwlmvm as well.
> 
> So I would like to discuss how to deal with the p2p_no_group_iface 
> option. As P2P_DEVICE is a new concept the name of the option may no 
> longer match what it intends. Is the option to force all P2P operations 
> to be done on a single interface, ie. wlan0 (or whatever is specified on 
> the command line) and no P2P_DEVICE is to be created. Or should it 
> change the interface from the command line as P2P group interface.

I don't think I'd do either of those. Not creating P2P_DEVICE will
simply not work with drivers expecting it, and changing iftype to/from
P2P-Device isn't supported since it would delete/create the netdev.

I don't really see much choice but reject (or ignore) this option for
drivers using P2P_DEVICE. Why would anyone *really* want P2P operation
on wlan0 when another interface can be used?

johannes

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Arend van Spriel]

On 05/27/2013 11:03 AM, Johannes Berg wrote:
> Hi,
>
>> I looked into issues around hwsim p2p tests and got all but one p2p test
>> passing now (test_autogo_tdls). I had to remove the p2p_no_group_iface
>> option from the p2px.conf files, because otherwise it tries to change
>> the P2P management interface into a P2P group interface. When using
>> P2P_DEVICE and P2P management interface this is not allowed by
>> mac80211_hwsim. Also for brcmfmac the P2P_DEVICE interface is dedicated
>> and can only be added/deleted, but not changed. Not sure if that is true
>> for iwlmvm as well.
>>
>> So I would like to discuss how to deal with the p2p_no_group_iface
>> option. As P2P_DEVICE is a new concept the name of the option may no
>> longer match what it intends. Is the option to force all P2P operations
>> to be done on a single interface, ie. wlan0 (or whatever is specified on
>> the command line) and no P2P_DEVICE is to be created. Or should it
>> change the interface from the command line as P2P group interface.
>
> I don't think I'd do either of those. Not creating P2P_DEVICE will
> simply not work with drivers expecting it, and changing iftype to/from
> P2P-Device isn't supported since it would delete/create the netdev.

So should we check that in cfg80211 upon wiphy_register().

> I don't really see much choice but reject (or ignore) this option for
> drivers using P2P_DEVICE. Why would anyone *really* want P2P operation
> on wlan0 when another interface can be used?

In this mac80211_hwsim is a special case. We could make P2P_DEVICE 
support in mac80211_hwsim optional using module parameter to allow 
testing both cases.

Regards,
Arend

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Johannes Berg]

On Mon, 2013-05-27 at 15:56 +0200, Arend van Spriel wrote:

> > I don't think I'd do either of those. Not creating P2P_DEVICE will
> > simply not work with drivers expecting it, and changing iftype to/from
> > P2P-Device isn't supported since it would delete/create the netdev.
> 
> So should we check that in cfg80211 upon wiphy_register().

Check what?

> > I don't really see much choice but reject (or ignore) this option for
> > drivers using P2P_DEVICE. Why would anyone *really* want P2P operation
> > on wlan0 when another interface can be used?
> 
> In this mac80211_hwsim is a special case. We could make P2P_DEVICE 
> support in mac80211_hwsim optional using module parameter to allow 
> testing both cases.

Yes, that we could do.

johannes

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Arend van Spriel]

On 05/27/2013 03:59 PM, Johannes Berg wrote:
> On Mon, 2013-05-27 at 15:56 +0200, Arend van Spriel wrote:
>
>>> I don't think I'd do either of those. Not creating P2P_DEVICE will
>>> simply not work with drivers expecting it, and changing iftype to/from
>>> P2P-Device isn't supported since it would delete/create the netdev.
>>
>> So should we check that in cfg80211 upon wiphy_register().
>
> Check what?

Check that the interface combinations contain a iface limit with only 
P2P_DEVICE:

	{
		.max = 1,
		.types = BIT(NL80211_IFTYPE_P2P_DEVICE)
	}

>>> I don't really see much choice but reject (or ignore) this option for
>>> drivers using P2P_DEVICE. Why would anyone *really* want P2P operation
>>> on wlan0 when another interface can be used?
>>
>> In this mac80211_hwsim is a special case. We could make P2P_DEVICE
>> support in mac80211_hwsim optional using module parameter to allow
>> testing both cases.
>
> Yes, that we could do.
>
> johannes
>
>

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Johannes Berg]

On Mon, 2013-05-27 at 16:03 +0200, Arend van Spriel wrote:
> On 05/27/2013 03:59 PM, Johannes Berg wrote:
> > On Mon, 2013-05-27 at 15:56 +0200, Arend van Spriel wrote:
> >
> >>> I don't think I'd do either of those. Not creating P2P_DEVICE will
> >>> simply not work with drivers expecting it, and changing iftype to/from
> >>> P2P-Device isn't supported since it would delete/create the netdev.
> >>
> >> So should we check that in cfg80211 upon wiphy_register().
> >
> > Check what?
> 
> Check that the interface combinations contain a iface limit with only 
> P2P_DEVICE:
> 
> 	{
> 		.max = 1,
> 		.types = BIT(NL80211_IFTYPE_P2P_DEVICE)
> 	}

We have that, right?

wiphy_verify_combinations:
                        /* Only a single P2P_DEVICE can be allowed */
                        if (WARN_ON(types &
BIT(NL80211_IFTYPE_P2P_DEVICE) &&
                                    c->limits[j].max > 1))
                                return -EINVAL;

johannes

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Arend van Spriel]

On 05/27/2013 05:53 PM, Johannes Berg wrote:
> On Mon, 2013-05-27 at 16:03 +0200, Arend van Spriel wrote:
>> On 05/27/2013 03:59 PM, Johannes Berg wrote:
>>> On Mon, 2013-05-27 at 15:56 +0200, Arend van Spriel wrote:
>>>
>>>>> I don't think I'd do either of those. Not creating P2P_DEVICE will
>>>>> simply not work with drivers expecting it, and changing iftype to/from
>>>>> P2P-Device isn't supported since it would delete/create the netdev.
>>>>
>>>> So should we check that in cfg80211 upon wiphy_register().
>>>
>>> Check what?
>>
>> Check that the interface combinations contain a iface limit with only
>> P2P_DEVICE:
>>
>> 	{
>> 		.max = 1,
>> 		.types = BIT(NL80211_IFTYPE_P2P_DEVICE)
>> 	}
>
> We have that, right?
>
> wiphy_verify_combinations:
>                          /* Only a single P2P_DEVICE can be allowed */
>                          if (WARN_ON(types &
> BIT(NL80211_IFTYPE_P2P_DEVICE) &&
>                                      c->limits[j].max > 1))
>                                  return -EINVAL;

I see. I did not search the code. my bad.

Gr. AvS

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Jouni Malinen]

On Mon, May 27, 2013 at 03:56:19PM +0200, Arend van Spriel wrote:
> In this mac80211_hwsim is a special case. We could make P2P_DEVICE
> support in mac80211_hwsim optional using module parameter to allow
> testing both cases.

Yes, that would be helpful. I don't understand why my test setup moves
to using P2P Device by default when mac80211_hwsim is perfectly capable
of operating without that.. I have been thinking of applying the
wpa_supplicant changes in a way that would make them disabled by default
and use a runtime configuration parameter (e.g., driver_param to
driver_nl80211.c) to enable this functionality for now. This would allow
the patches to go in now and make it somewhat easier to test them.
Anyway, if mac80211_hwsim changes its default behavior, this would be of
lesser needed (well, once a new kernel release or compat-drivers gets
released).

In general, I don't think it is a good idea to change existing behavior
with a kernel or wpa_supplicant upgrade. It is obviously fine if this is
needed for the functionality to work in the first place, but that is not
the case with mac80211_hwsim.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Marcel Holtmann]

Hi Arend,

>>> I looked into issues around hwsim p2p tests and got all but one p2p test
>>> passing now (test_autogo_tdls). I had to remove the p2p_no_group_iface
>>> option from the p2px.conf files, because otherwise it tries to change
>>> the P2P management interface into a P2P group interface. When using
>>> P2P_DEVICE and P2P management interface this is not allowed by
>>> mac80211_hwsim. Also for brcmfmac the P2P_DEVICE interface is dedicated
>>> and can only be added/deleted, but not changed. Not sure if that is true
>>> for iwlmvm as well.
>>> 
>>> So I would like to discuss how to deal with the p2p_no_group_iface
>>> option. As P2P_DEVICE is a new concept the name of the option may no
>>> longer match what it intends. Is the option to force all P2P operations
>>> to be done on a single interface, ie. wlan0 (or whatever is specified on
>>> the command line) and no P2P_DEVICE is to be created. Or should it
>>> change the interface from the command line as P2P group interface.
>> 
>> I don't think I'd do either of those. Not creating P2P_DEVICE will
>> simply not work with drivers expecting it, and changing iftype to/from
>> P2P-Device isn't supported since it would delete/create the netdev.
> 
> So should we check that in cfg80211 upon wiphy_register().
> 
>> I don't really see much choice but reject (or ignore) this option for
>> drivers using P2P_DEVICE. Why would anyone *really* want P2P operation
>> on wlan0 when another interface can be used?
> 
> In this mac80211_hwsim is a special case. We could make P2P_DEVICE support in mac80211_hwsim optional using module parameter to allow testing both cases.

or instead of overloading it with pointless module parameters, we allow dynamic device creation via its already existing netlink interface. I do actually have patches for this. I just never cleaned them up enough to submit them upstream. There is a bunch of required refactoring that has to go in first before this would be possible, but it does work in my setup that I have running on my test machine.

I would like to get to a state where mac80211_hwsim can be loaded all the time and starts out with radios=0. And you just add test radios one by one. Which then in fact can all have different features enabled or disabled. To that extend, the test cases for the regulatory subsystem could also be moved into userspace and do not need to be hardcoded in the kernel module anymore.

Regards

Marcel

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Johannes Berg]

On Mon, 2013-05-27 at 21:11 +0300, Jouni Malinen wrote:
> On Mon, May 27, 2013 at 03:56:19PM +0200, Arend van Spriel wrote:
> > In this mac80211_hwsim is a special case. We could make P2P_DEVICE
> > support in mac80211_hwsim optional using module parameter to allow
> > testing both cases.
> 
> Yes, that would be helpful. I don't understand why my test setup moves
> to using P2P Device by default when mac80211_hwsim is perfectly capable
> of operating without that.. [...]
> 
> In general, I don't think it is a good idea to change existing behavior
> with a kernel or wpa_supplicant upgrade. It is obviously fine if this is
> needed for the functionality to work in the first place, but that is not
> the case with mac80211_hwsim.

It's kinda a side effect of having "I support P2P-Device" also mean "I
want P2P-Device to be used", but for everything other than hwsim that
seems like the only reasonable choice, so ... I wouldn't mind changing
the default in hwsim though to not support/desire P2P-Device w/o a
configuration option or so.

johannes

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Arend van Spriel]

On 05/27/2013 09:15 PM, Johannes Berg wrote:
> On Mon, 2013-05-27 at 21:11 +0300, Jouni Malinen wrote:
>> On Mon, May 27, 2013 at 03:56:19PM +0200, Arend van Spriel wrote:
>>> In this mac80211_hwsim is a special case. We could make P2P_DEVICE
>>> support in mac80211_hwsim optional using module parameter to allow
>>> testing both cases.
>>
>> Yes, that would be helpful. I don't understand why my test setup moves
>> to using P2P Device by default when mac80211_hwsim is perfectly capable
>> of operating without that.. [...]
>>
>> In general, I don't think it is a good idea to change existing behavior
>> with a kernel or wpa_supplicant upgrade. It is obviously fine if this is
>> needed for the functionality to work in the first place, but that is not
>> the case with mac80211_hwsim.
>
> It's kinda a side effect of having "I support P2P-Device" also mean "I
> want P2P-Device to be used", but for everything other than hwsim that
> seems like the only reasonable choice, so ... I wouldn't mind changing
> the default in hwsim though to not support/desire P2P-Device w/o a
> configuration option or so.

Hi Johannes,

Marcel suggested a different approach that sounds pretty compelling from 
a test perspective. However, that does change mac80211_hwsim behaviour 
more significantly. No pain, no gain :-)

Regards,
Arend

Re: P2P Device support: how to deal with p2p_no_group_iface option

[Johannes Berg]

On Mon, 2013-05-27 at 21:20 +0200, Arend van Spriel wrote:

> Marcel suggested a different approach that sounds pretty compelling from 
> a test perspective. However, that does change mac80211_hwsim behaviour 
> more significantly. No pain, no gain :-)

Yeah, he's suggested it before, and it'd be a good thing to have.
Luckily this is a test tool so I don't think anyone would mind changing
the module parameters too much. Hopefully.

johannes