P2P Device support: how to deal with p2p_no_group_iface option

DISCUSSION Arend van Spriel 4 participants 12 messages May 25, 2013 – May 27, 2013

Share & tools

Arend van Spriel

author

2013-05-25 11:23 UTC #0 lore raw

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

Johannes Berg

2013-05-27 09:03 UTC #1 lore raw

Hi,

Quoted 9 lines

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.

Quoted 6 lines

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

Arend van Spriel

author

2013-05-27 13:56 UTC #2 lore raw

Quoted 18 lines · Johannes Berg

Hi,
Quoted 9 lines
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.
Quoted 6 lines
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.

Quoted 3 lines

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

Quoted 3 lines

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

Johannes Berg

2013-05-27 13:59 UTC #3 lore raw

Quoted 5 lines · Arend van Spriel

Quoted 3 lines
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?

Quoted 7 lines

Quoted 3 lines
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

Arend van Spriel

author

2013-05-27 14:04 UTC #4 lore raw

Quoted 9 lines · Johannes Berg

Quoted 5 lines · Arend van Spriel
Quoted 3 lines
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)
	}

Quoted 13 lines

Quoted 7 lines
Quoted 3 lines
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

Johannes Berg

2013-05-27 15:53 UTC #5 lore raw

Quoted 11 lines · Arend van Spriel

Quoted 9 lines · Johannes Berg
Quoted 5 lines · Arend van Spriel
Quoted 3 lines
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?

Quoted 7 lines

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

Arend van Spriel

author

2013-05-27 17:40 UTC #6 lore raw

Quoted 20 lines · Johannes Berg

Quoted 11 lines · Arend van Spriel
Quoted 9 lines · Johannes Berg
Quoted 5 lines · Arend van Spriel
Quoted 3 lines
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?
Quoted 7 lines
Check that the interface combinations contain a iface limit with only P2P_DEVICE:
	{
		.max = 1,
		.types = BIT(NL80211_IFTYPE_P2P_DEVICE)
	}

Quoted 8 lines

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

Jouni Malinen

2013-05-27 18:22 UTC #7 lore raw

In reply to Arend van Spriel

Quoted 3 lines · Arend van Spriel

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.

Signature · 1 line

Jouni Malinen                                            PGP id EFC895FA

Johannes Berg

2013-05-27 19:16 UTC #8 lore raw

Quoted 5 lines · Jouni Malinen

Quoted 3 lines · Arend van Spriel
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.

Quoted 8 lines

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

Arend van Spriel

author

2013-05-27 19:20 UTC #9 lore raw

Quoted 15 lines · Johannes Berg

Quoted 5 lines · Jouni Malinen
Quoted 3 lines · Arend van Spriel
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.
Quoted 8 lines
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.

Quoted 5 lines

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

Johannes Berg

2013-05-27 19:27 UTC #10 lore raw

Quoted 3 lines · Arend van Spriel

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

Marcel Holtmann

2013-05-27 18:51 UTC #11 lore raw

In reply to Arend van Spriel

Hi Arend,

Quoted 20 lines

Quoted 16 lines
Quoted 9 lines
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.
Quoted 6 lines
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.
Quoted 3 lines
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.

Quoted 7 lines

So should we check that in cfg80211 upon wiphy_register().
Quoted 3 lines
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