Patchwork [V2,21/23] nl80211: verify P2P-GO/CLIENT address with all interface addresses

login
register
mail settings
Submitter Arend van Spriel
Date May 29, 2013, 9:08 a.m.
Message ID <1369818491-7274-22-git-send-email-arend@broadcom.com>
Download mbox | patch
Permalink /patch/247196/
State Accepted
Headers show

Comments

Arend van Spriel - May 29, 2013, 9:08 a.m.
With P2P Device support there will be two interfaces with their
own mac address. The P2P interface address must be unique so verify
it is.

Signed-hostap: Arend van Spriel <arend@broadcom.com>
---
 src/drivers/driver_nl80211.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Jouni Malinen - June 30, 2013, 7:43 a.m.
On Wed, May 29, 2013 at 11:08:08AM +0200, Arend van Spriel wrote:
> With P2P Device support there will be two interfaces with their
> own mac address. The P2P interface address must be unique so verify
> it is.

Could you please clarify the thinking behind this one? The P2P Interface
Address shall be unique for each group and it shall not match any
non-P2P station/AP interface address. However, a P2P Interface Address
can be same as the P2P Device Address and for many cases, it is more
resource friendly (e.g., firmware memory and RX filtering) to use the
same address when possible.
Arend van Spriel - June 30, 2013, 8:43 p.m.
On 06/30/13 09:43, Jouni Malinen wrote:
> On Wed, May 29, 2013 at 11:08:08AM +0200, Arend van Spriel wrote:
>> With P2P Device support there will be two interfaces with their
>> own mac address. The P2P interface address must be unique so verify
>> it is.
>
> Could you please clarify the thinking behind this one? The P2P Interface
> Address shall be unique for each group and it shall not match any
> non-P2P station/AP interface address. However, a P2P Interface Address
> can be same as the P2P Device Address and for many cases, it is more
> resource friendly (e.g., firmware memory and RX filtering) to use the
> same address when possible.

Hi Jouni,

Good point. The P2P interface address and P2P device address only need 
to be different if one would want to do P2P management and have a group 
connection concurrently (correct me if I am wrong here, maybe it is a 
restriction for brcm firmware only) so the check seems a bit too 
restrictive.

Regards,
Arend
Jouni Malinen - June 30, 2013, 10:03 p.m.
On Sun, Jun 30, 2013 at 10:43:46PM +0200, Arend van Spriel wrote:
> Good point. The P2P interface address and P2P device address only
> need to be different if one would want to do P2P management and have
> a group connection concurrently (correct me if I am wrong here,
> maybe it is a restriction for brcm firmware only) so the check seems
> a bit too restrictive.

There is no such restriction in the P2P protocol specification. I've
been trying to figure out why Broadcom went with such design, but
anyway, if that is indeed a restriction in the firmware, it could be
useful to provide a driver capability flag to allow wpa_supplicant to
determine which rule to use.. I would use either the non-P2P station
interface MAC address or alternatively, the first P2P Interface Address
(which should be that non-P2P station interface MAC address with the
locally administered bit set to 1 or another globally unique MAC
address) as the P2P Device Address.

I would really like to avoid confusion and unjustifiable requests in
this area where that existing design could be used to claim that other
drivers need to behave similarly when there is no real technical reason
for doing that. In other words, drivers should be able to select
whichever P2P spec compliant mechanism they need. wpa_supplicant should
hide these internal details from anything above it (and that can now
finally be the case with the global control interface for P2P
operations).

An earlier thread  seemed to already imply that some people may believe
that this new dedicated P2P_DEVICE is needed for some concurrent P2P
operations which is simply not the case; it just adds another option for
internal driver/firmware/hardware design which should not add or remove
any functionality and the only externally observable difference should
be in MAC addresses potentially being different (but even those could be
assigned using the same style between P2P_DEVICE and p2p-mgmt-netdev).
Chengyi Zhao - July 2, 2013, 12:09 p.m.
Hi Jouni,

I think Google, Boardcom and Intel use this solution for the Wi-Fi P2P
cross connection, but it can not run fine in Android 4.2.2 yet. Please
refer to the following information of android P2P, it is a P2P-GO role in
mobile,
if wlan0 connect the internet, and system uses iptable(maybe more another
techonology) to create a special relation between p2p0  and wlan0, P2P-Client
will very conveniently surf internet via  the Wi-Fi P2P cross connection.
In fact this functionality is simalerly tetheing

May be described as:
Cross Connection is the optional capability of a P2P Group Owner to provide
WLAN access to P2P Clients within its P2P Group.

In fact, this is an integrated technology, concurrently enable Wi-Fi
tethering and P2P.


Hi Arend,

Could you please consult with the firmware or Android network system
engineer in Boardcom,
and please clarify the thinking behind this one?

Thanks.

---------------------------------------------------------------------------------------------------------
Reference information:
root@android:/ # ifconfig
lo        Link encap:Local Loopback
          inet addr:127.0.0.1  Mask:255.0.0.0
          inet6 addr: ::1/128 Scope:Host
          UP LOOPBACK RUNNING  MTU:16436  Metric:1
          RX packets:18401 errors:0 dropped:0 overruns:0 frame:0
          TX packets:18401 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:0
          RX bytes:149934880 (142.9 MiB)  TX bytes:149934880 (142.9 MiB)

p2p0      Link encap:Ethernet  HWaddr 12:68:3F:38:6E:2C
          inet addr:192.168.49.1  Bcast:192.168.49.255  Mask:255.255.255.0
          inet6 addr: fe80::1068:3fff:fe38:6e2c/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:9 errors:0 dropped:0 overruns:0 frame:0
          TX packets:21 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:100
          RX bytes:1285 (1.2 KiB)  TX bytes:2677 (2.6 KiB)

wlan0     Link encap:Ethernet  HWaddr 10:68:3F:38:6E:2C
          inet addr:192.168.1.100  Bcast:255.255.255.255  Mask:255.255.255.0
          inet6 addr: fe80::1268:3fff:fe38:6e2c/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
          RX packets:11553 errors:0 dropped:0 overruns:0 frame:0
          TX packets:8693 errors:0 dropped:0 overruns:0 carrier:0
          collisions:0 txqueuelen:100
          RX bytes:8538960 (8.1 MiB)  TX bytes:1071653 (1.0 MiB)

Cheers,

Chengyi



2013/7/1 Jouni Malinen <j@w1.fi>

> On Sun, Jun 30, 2013 at 10:43:46PM +0200, Arend van Spriel wrote:
> > Good point. The P2P interface address and P2P device address only
> > need to be different if one would want to do P2P management and have
> > a group connection concurrently (correct me if I am wrong here,
> > maybe it is a restriction for brcm firmware only) so the check seems
> > a bit too restrictive.
>
> There is no such restriction in the P2P protocol specification. I've
> been trying to figure out why Broadcom went with such design, but
> anyway, if that is indeed a restriction in the firmware, it could be
> useful to provide a driver capability flag to allow wpa_supplicant to
> determine which rule to use.. I would use either the non-P2P station
> interface MAC address or alternatively, the first P2P Interface Address
> (which should be that non-P2P station interface MAC address with the
> locally administered bit set to 1 or another globally unique MAC
> address) as the P2P Device Address.
>
> I would really like to avoid confusion and unjustifiable requests in
> this area where that existing design could be used to claim that other
> drivers need to behave similarly when there is no real technical reason
> for doing that. In other words, drivers should be able to select
> whichever P2P spec compliant mechanism they need. wpa_supplicant should
> hide these internal details from anything above it (and that can now
> finally be the case with the global control interface for P2P
> operations).
>
> An earlier thread  seemed to already imply that some people may believe
> that this new dedicated P2P_DEVICE is needed for some concurrent P2P
> operations which is simply not the case; it just adds another option for
> internal driver/firmware/hardware design which should not add or remove
> any functionality and the only externally observable difference should
> be in MAC addresses potentially being different (but even those could be
> assigned using the same style between P2P_DEVICE and p2p-mgmt-netdev).
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap
>
Arend van Spriel - July 2, 2013, 1:46 p.m.
On 07/01/2013 12:03 AM, Jouni Malinen wrote:
> On Sun, Jun 30, 2013 at 10:43:46PM +0200, Arend van Spriel wrote:
>> Good point. The P2P interface address and P2P device address only
>> need to be different if one would want to do P2P management and have
>> a group connection concurrently (correct me if I am wrong here,
>> maybe it is a restriction for brcm firmware only) so the check seems
>> a bit too restrictive.
>
> There is no such restriction in the P2P protocol specification. I've
> been trying to figure out why Broadcom went with such design, but
> anyway, if that is indeed a restriction in the firmware, it could be
> useful to provide a driver capability flag to allow wpa_supplicant to
> determine which rule to use.. I would use either the non-P2P station
> interface MAC address or alternatively, the first P2P Interface Address
> (which should be that non-P2P station interface MAC address with the
> locally administered bit set to 1 or another globally unique MAC
> address) as the P2P Device Address.

Hi Jouni,

The address scheme that we used with the dummy netdev aka p2p0 was:
p2p-dev-mac = non-p2p-mac with local administered bit set.
p2p-ifc-mac = p2p-dev-mac with byte 3 (or 4) modified.

> I would really like to avoid confusion and unjustifiable requests in
> this area where that existing design could be used to claim that other
> drivers need to behave similarly when there is no real technical reason
> for doing that. In other words, drivers should be able to select
> whichever P2P spec compliant mechanism they need. wpa_supplicant should
> hide these internal details from anything above it (and that can now
> finally be the case with the global control interface for P2P
> operations).

Agree.

> An earlier thread  seemed to already imply that some people may believe
> that this new dedicated P2P_DEVICE is needed for some concurrent P2P
> operations which is simply not the case; it just adds another option for
> internal driver/firmware/hardware design which should not add or remove
> any functionality and the only externally observable difference should
> be in MAC addresses potentially being different (but even those could be
> assigned using the same style between P2P_DEVICE and p2p-mgmt-netdev).

The last sentence is confusing to me. To me the P2P_DEVICE is a 
replacing solution to the p2p-mgmt-netdev, right?

Regards,
Arend
Johannes Berg - July 2, 2013, 1:56 p.m.
On Tue, 2013-07-02 at 15:46 +0200, Arend van Spriel wrote:

> > An earlier thread  seemed to already imply that some people may believe
> > that this new dedicated P2P_DEVICE is needed for some concurrent P2P
> > operations which is simply not the case; it just adds another option for
> > internal driver/firmware/hardware design which should not add or remove
> > any functionality and the only externally observable difference should
> > be in MAC addresses potentially being different (but even those could be
> > assigned using the same style between P2P_DEVICE and p2p-mgmt-netdev).
> 
> The last sentence is confusing to me. To me the P2P_DEVICE is a 
> replacing solution to the p2p-mgmt-netdev, right?

That was never needed either though -- you can do P2P management on
wlan0 (i.e. the BSS client netdev)

johannes
Arend van Spriel - July 2, 2013, 2:11 p.m.
On 07/02/2013 03:56 PM, Johannes Berg wrote:
> On Tue, 2013-07-02 at 15:46 +0200, Arend van Spriel wrote:
>
>>> An earlier thread  seemed to already imply that some people may believe
>>> that this new dedicated P2P_DEVICE is needed for some concurrent P2P
>>> operations which is simply not the case; it just adds another option for
>>> internal driver/firmware/hardware design which should not add or remove
>>> any functionality and the only externally observable difference should
>>> be in MAC addresses potentially being different (but even those could be
>>> assigned using the same style between P2P_DEVICE and p2p-mgmt-netdev).
>>
>> The last sentence is confusing to me. To me the P2P_DEVICE is a
>> replacing solution to the p2p-mgmt-netdev, right?
>
> That was never needed either though -- you can do P2P management on
> wlan0 (i.e. the BSS client netdev)

You mean to say there is no real usage scenario for the P2P Device 
interface?

Gr. AvS
Johannes Berg - July 2, 2013, 2:18 p.m.
On Tue, 2013-07-02 at 16:11 +0200, Arend van Spriel wrote:
> On 07/02/2013 03:56 PM, Johannes Berg wrote:
> > On Tue, 2013-07-02 at 15:46 +0200, Arend van Spriel wrote:
> >
> >>> An earlier thread  seemed to already imply that some people may believe
> >>> that this new dedicated P2P_DEVICE is needed for some concurrent P2P
> >>> operations which is simply not the case; it just adds another option for
> >>> internal driver/firmware/hardware design which should not add or remove
> >>> any functionality and the only externally observable difference should
> >>> be in MAC addresses potentially being different (but even those could be
> >>> assigned using the same style between P2P_DEVICE and p2p-mgmt-netdev).
> >>
> >> The last sentence is confusing to me. To me the P2P_DEVICE is a
> >> replacing solution to the p2p-mgmt-netdev, right?
> >
> > That was never needed either though -- you can do P2P management on
> > wlan0 (i.e. the BSS client netdev)
> 
> You mean to say there is no real usage scenario for the P2P Device 
> interface?

No, not really, just that it's not fundamentally necessary if the driver
works without one.

johannes
Arend van Spriel - July 2, 2013, 2:20 p.m.
On 07/02/2013 04:18 PM, Johannes Berg wrote:
> On Tue, 2013-07-02 at 16:11 +0200, Arend van Spriel wrote:
>> On 07/02/2013 03:56 PM, Johannes Berg wrote:
>>> On Tue, 2013-07-02 at 15:46 +0200, Arend van Spriel wrote:
>>>
>>>>> An earlier thread  seemed to already imply that some people may believe
>>>>> that this new dedicated P2P_DEVICE is needed for some concurrent P2P
>>>>> operations which is simply not the case; it just adds another option for
>>>>> internal driver/firmware/hardware design which should not add or remove
>>>>> any functionality and the only externally observable difference should
>>>>> be in MAC addresses potentially being different (but even those could be
>>>>> assigned using the same style between P2P_DEVICE and p2p-mgmt-netdev).
>>>>
>>>> The last sentence is confusing to me. To me the P2P_DEVICE is a
>>>> replacing solution to the p2p-mgmt-netdev, right?
>>>
>>> That was never needed either though -- you can do P2P management on
>>> wlan0 (i.e. the BSS client netdev)
>>
>> You mean to say there is no real usage scenario for the P2P Device
>> interface?
>
> No, not really, just that it's not fundamentally necessary if the driver
> works without one.

Phew :-)

Gr. AvS
Jouni Malinen - July 2, 2013, 4:28 p.m.
On Tue, Jul 02, 2013 at 08:09:39PM +0800, Chengyi Zhao wrote:
> I think Google, Boardcom and Intel use this solution for the Wi-Fi P2P
> cross connection, but it can not run fine in Android 4.2.2 yet. Please
> refer to the following information of android P2P, it is a P2P-GO role in
> mobile,

I'm not sure what "this solution" here is referring to. Use of three
different MAC addresses? Or the new dedicated non-netdev P2P_DEVICE?
Neither of those is needed for P2P cross connection.

> if wlan0 connect the internet, and system uses iptable(maybe more another
> techonology) to create a special relation between p2p0  and wlan0, P2P-Client
> will very conveniently surf internet via  the Wi-Fi P2P cross connection.
> In fact this functionality is simalerly tetheing

Sure, and that works just fine with wlan0 being used for managing P2P
operations and for the infrastructure AP connection while another netdev
is used for the P2P group (i.e., just two MAC addresses and no need for
P2P_DEVICE concept).
Jouni Malinen - July 2, 2013, 4:47 p.m.
On Tue, Jul 02, 2013 at 03:46:28PM +0200, Arend van Spriel wrote:
> The address scheme that we used with the dummy netdev aka p2p0 was:
> p2p-dev-mac = non-p2p-mac with local administered bit set.
> p2p-ifc-mac = p2p-dev-mac with byte 3 (or 4) modified.

I know this is used, but I've just never understood why, i.e., it looks
like wasting an extra MAC address without any real benefit and with the
added drawback of increased likelihood of hitting duplicated addresses.

> >An earlier thread  seemed to already imply that some people may believe
> >that this new dedicated P2P_DEVICE is needed for some concurrent P2P
> >operations which is simply not the case; it just adds another option for
> >internal driver/firmware/hardware design which should not add or remove
> >any functionality and the only externally observable difference should
> >be in MAC addresses potentially being different (but even those could be
> >assigned using the same style between P2P_DEVICE and p2p-mgmt-netdev).
> 
> The last sentence is confusing to me. To me the P2P_DEVICE is a
> replacing solution to the p2p-mgmt-netdev, right?

It is a cleaner replacement for the _dedicated_ p2p-mgmt-netdev case,
but not for the case where Android uses p2p0 for both p2p-mgmt and group
operations (both of these two alternatives show up in deployed devices).
In other words, I see this more as a mechanism to allow that IMHO odd
three address design to be used in a way that is more acceptable for
upstream Linux. I'd assume there are existing firmware designs that only
support a case where P2P Device Address is not used for anything else
than P2P Device operations, so there is justification for supporting
this, but I hope this is not taken as an encouragement for such MAC
address assignment scheme for new designs.

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 71d20c0..ca1ffb8 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -8892,7 +8892,7 @@  static int wpa_driver_nl80211_if_add(void *priv, enum wpa_driver_if_type type,
 			nl80211_remove_iface(drv, ifidx);
 			return -1;
 		}
-		if (os_memcmp(if_addr, new_addr, ETH_ALEN) == 0) {
+		if (nl80211_addr_in_use(drv->global, new_addr)) {
 			wpa_printf(MSG_DEBUG, "nl80211: Allocate new address "
 				   "for P2P group interface");
 			if (nl80211_p2p_interface_addr(drv, new_addr) < 0) {