diff mbox

Retreive shared frequency when a singly "phy" is shared between multiple interfaces

Message ID 6C370B347C3FE8438C9692873287D2E119560C6B04@SJEXCHCCR01.corp.ad.broadcom.com
State Accepted
Commit 57ebba598ddc38fdb5257de9881ef4395884be5a
Headers show

Commit Message

Jithu Jance Nov. 10, 2011, 10:08 a.m. UTC 
Hi Johannes,

Thanks for your reply. Please correct me, if my understanding is wrong. I am talking about a concurrent scenario where we have a legacy STA on wlan0 interface(primary MAC address) already in connected state. Then we are starting an Autonomous GO by issuing p2p_group_add on wlan1(p2p_device_address) interface.

In wpas_p2p_init_go_params, I could see that the wpa_supplicant invokes wpa_drv_shared_freq to get the shared freq. Since this handler wasn't present, I implemented it to retrieve the freq. Without the below patch, the p2p_group_add was always resulting in a GO on freq 2412. Freq 2412 would be set when there is no known preference as per "wpas_p2p_init_go_params" function. Did I miss something?? 


Sorry for the whitespace issue. I am attaching the corrected patch below. 

Signed-hostap: Jithu Jance <jithu@broadcom.com>

---
 src/drivers/driver_nl80211.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

Comments

Johannes Berg Nov. 10, 2011, 10:15 a.m. UTC | #1 
On Thu, 2011-11-10 at 02:08 -0800, Jithu Jance wrote:
> Hi Johannes,
> 
> Thanks for your reply. Please correct me, if my understanding is
> wrong. I am talking about a concurrent scenario where we have a legacy
> STA on wlan0 interface(primary MAC address) already in connected
> state. Then we are starting an Autonomous GO by issuing p2p_group_add
> on wlan1(p2p_device_address) interface.
> 
> In wpas_p2p_init_go_params, I could see that the wpa_supplicant
> invokes wpa_drv_shared_freq to get the shared freq. Since this handler
> wasn't present, I implemented it to retrieve the freq. Without the
> below patch, the p2p_group_add was always resulting in a GO on freq
> 2412. Freq 2412 would be set when there is no known preference as per
> "wpas_p2p_init_go_params" function. Did I miss something?? 

Probably not, but now I'm totally confused -- how are you managing to
have a p2p device interface "wlan1" with nl80211? That would seem to
require major nl80211 changes and isn't really how it's working there
today.

johannes
Johannes Berg Nov. 10, 2011, 10:26 a.m. UTC | #2 
On Thu, 2011-11-10 at 02:08 -0800, Jithu Jance wrote:

> Thanks for your reply. Please correct me, if my understanding is
> wrong. I am talking about a concurrent scenario where we have a legacy
> STA on wlan0 interface(primary MAC address) already in connected
> state. Then we are starting an Autonomous GO by issuing p2p_group_add
> on wlan1(p2p_device_address) interface.

In fact, and now I'm getting sort of off-topic, I really don't like that
approach. Having "wlan1" as the P2P device interface is pretty useless
-- I think it's a workaround approach. That interface can never really
be used to transfer data, so it being a network interface is not very
natural.

I'm starting to think that there is a real need for having a P2P
abstraction (enable/disable P2P) with P2P device address, but I think
that a separate network interface is almost certainly not the right
answer.

johannes
Jithu Jance Nov. 10, 2011, 11:31 a.m. UTC | #3 
> I'm starting to think that there is a real need for having a P2P
> abstraction (enable/disable P2P) with P2P device address, but I think
> that a separate network interface is almost certainly not the right
> answer.

I agree with you. We had to do use separate interface since we didn't want 
to use STA Interface for p2p device address related operations. And currently there is no way
for the  supplicant to get the P2P device address from the driver (if P2P
address is different from the primary mac address). If supplicant can retrieve 
p2p_dev_addr from driver, then supplicant can be run on wlan0 interface for 
STA operations and can use the retrieved P2P address for building P2P IE's 
and for sending P2P action frames. 

The current patch should address the functionality of retrieving the shared frequency (if we have multiple
interface shared across a single PHY). This i guess won't break anything else. Would it be possible to take-in this
patch, at least till the suggested P2P abstraction is implemented. Kindly share your thoughts on this.


- Jithu Jance.
Johannes Berg Nov. 10, 2011, 12:12 p.m. UTC | #4 
On Thu, 2011-11-10 at 03:31 -0800, Jithu Jance wrote:
> > I'm starting to think that there is a real need for having a P2P
> > abstraction (enable/disable P2P) with P2P device address, but I think
> > that a separate network interface is almost certainly not the right
> > answer.
> 
> I agree with you. We had to do use separate interface since we didn't want 
> to use STA Interface for p2p device address related operations. 

Dare I ask why not?

> And currently there is no way
> for the  supplicant to get the P2P device address from the driver (if P2P
> address is different from the primary mac address). If supplicant can retrieve 
> p2p_dev_addr from driver, then supplicant can be run on wlan0 interface for 
> STA operations and can use the retrieved P2P address for building P2P IE's 
> and for sending P2P action frames. 
> 
> The current patch should address the functionality of retrieving the shared frequency (if we have multiple
> interface shared across a single PHY). This i guess won't break anything else. Would it be possible to take-in this
> patch, at least till the suggested P2P abstraction is implemented. Kindly share your thoughts on this.

I'm not strictly opposed to this patch, but I think you're designing
something that isn't really what will be accepted upstream, the way I
see it you're thus painting yourself into a corner there and it'll
probably be hard to get out of it again.

I was actually thinking about this less from an address point of view
but more from explicitly enabling/disabling p2p etc.

johannes
Jouni Malinen Nov. 19, 2011, 9:33 a.m. UTC | #5 
On Thu, Nov 10, 2011 at 03:31:35AM -0800, Jithu Jance wrote:
> I agree with you. We had to do use separate interface since we didn't want 
> to use STA Interface for p2p device address related operations. And currently there is no way
> for the  supplicant to get the P2P device address from the driver (if P2P
> address is different from the primary mac address). If supplicant can retrieve 
> p2p_dev_addr from driver, then supplicant can be run on wlan0 interface for 
> STA operations and can use the retrieved P2P address for building P2P IE's 
> and for sending P2P action frames. 

This is something that we need to discuss in more detail (on
linux-wireless mailing list, I'd guess). There are other driver (e.g.,
ath6kl) where a mechanism for being able to use a different address or
well, at least some kind of different context for P2P operations may be
desired. I agree with Johannes that adding a separate wlan1 interface
for P2P device is not really the best way of doing this. Though, there
could be some potential issues in being able to multiplex all P2P
management operations on the station interface (but most, if not all, of
those have been resolved with new attributes allowing TX rate control
etc.).

It could be a good starting point to run some experiments with a new
nl80211 command to enable P2P functionality on the initial wlan0
interface with a mechanism that allows the driver to return a P2P
Device Address that may (or may not) differ from the wlan0 MAC address.

> The current patch should address the functionality of retrieving the shared frequency (if we have multiple
> interface shared across a single PHY). This i guess won't break anything else. Would it be possible to take-in this
> patch, at least till the suggested P2P abstraction is implemented. Kindly share your thoughts on this.

While shared_freq() was not originally really meant for nl80211-based
drivers, I applied this now.
Johannes Berg Nov. 19, 2011, 11:09 a.m. UTC | #6 
On Sat, 2011-11-19 at 11:33 +0200, Jouni Malinen wrote:

> This is something that we need to discuss in more detail (on
> linux-wireless mailing list, I'd guess). There are other driver (e.g.,
> ath6kl) where a mechanism for being able to use a different address or
> well, at least some kind of different context for P2P operations may be
> desired. I agree with Johannes that adding a separate wlan1 interface
> for P2P device is not really the best way of doing this. Though, there
> could be some potential issues in being able to multiplex all P2P
> management operations on the station interface (but most, if not all, of
> those have been resolved with new attributes allowing TX rate control
> etc.).
> 
> It could be a good starting point to run some experiments with a new
> nl80211 command to enable P2P functionality on the initial wlan0
> interface with a mechanism that allows the driver to return a P2P
> Device Address that may (or may not) differ from the wlan0 MAC address.

I agree with this, and some sort of context management for that would be
good even for other devices -- e.g. iwlwifi right now will enable P2P
device context on r-o-c and disable it again on a timeout.

This also goes towards having things like being discoverable (extended
listen) offloaded to the device.

johannes
Jithu Jance Nov. 19, 2011, 2:01 p.m. UTC | #7 
Hi Jouni, Johannes,

Thanks for your suggestions.

>It could be a good starting point to run some experiments with a new
>nl80211 command to enable P2P functionality on the initial wlan0
>interface with a mechanism that allows the driver to return a P2P
>Device Address that may (or may not) differ from the wlan0 MAC address.

This approach looks good!! I will try this out.



-- Jithu Jance



On Sat, Nov 19, 2011 at 3:03 PM, Jouni Malinen <j@w1.fi> wrote:
>
> On Thu, Nov 10, 2011 at 03:31:35AM -0800, Jithu Jance wrote:
> > I agree with you. We had to do use separate interface since we didn't want
> > to use STA Interface for p2p device address related operations. And currently there is no way
> > for the  supplicant to get the P2P device address from the driver (if P2P
> > address is different from the primary mac address). If supplicant can retrieve
> > p2p_dev_addr from driver, then supplicant can be run on wlan0 interface for
> > STA operations and can use the retrieved P2P address for building P2P IE's
> > and for sending P2P action frames.
>
> This is something that we need to discuss in more detail (on
> linux-wireless mailing list, I'd guess). There are other driver (e.g.,
> ath6kl) where a mechanism for being able to use a different address or
> well, at least some kind of different context for P2P operations may be
> desired. I agree with Johannes that adding a separate wlan1 interface
> for P2P device is not really the best way of doing this. Though, there
> could be some potential issues in being able to multiplex all P2P
> management operations on the station interface (but most, if not all, of
> those have been resolved with new attributes allowing TX rate control
> etc.).
>
> It could be a good starting point to run some experiments with a new
> nl80211 command to enable P2P functionality on the initial wlan0
> interface with a mechanism that allows the driver to return a P2P
> Device Address that may (or may not) differ from the wlan0 MAC address.
>
> > The current patch should address the functionality of retrieving the shared frequency (if we have multiple
> > interface shared across a single PHY). This i guess won't break anything else. Would it be possible to take-in this
> > patch, at least till the suggested P2P abstraction is implemented. Kindly share your thoughts on this.
>
> While shared_freq() was not originally really meant for nl80211-based
> drivers, I applied this now.
>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP@lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap
diff mbox

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 2ab10ae..e5077e0 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -269,6 +269,7 @@  static int nl80211_send_frame_cmd(struct wpa_driver_nl80211_data *drv,
 				  const u8 *buf, size_t buf_len, u64 *cookie,
 				  int no_cck);
 static int wpa_driver_nl80211_probe_req_report(void *priv, int report);
+static int wpa_driver_nl80211_shared_freq(void *priv);
 
 #ifdef HOSTAPD
 static void add_ifidx(struct wpa_driver_nl80211_data *drv, int ifidx);
@@ -7231,6 +7232,41 @@  static int nl80211_signal_poll(void *priv, struct wpa_signal_info *si)
 	return nl80211_get_link_noise(drv, si);
 }
 
+static int wpa_driver_nl80211_shared_freq(void *priv)
+{
+	struct i802_bss *bss = priv;
+	struct wpa_driver_nl80211_data *drv = bss->drv;
+	struct nl80211_global *global = drv->global;
+	struct wpa_driver_nl80211_data *driver = NULL;
+	int freq = 0;
+
+	/* If the same phy is in connected state with some other interface,
+	 * then retrieve the assoc freq */
+	wpa_printf(MSG_DEBUG, "nl80211: get shared freq for PHY(%s)", drv->phyname);
+
+	dl_list_for_each(driver, &global->interfaces,
+		struct wpa_driver_nl80211_data, list) {
+
+		/* skip, if its the same instance */
+		if(drv == driver)
+			continue;
+
+		if(os_strcmp(drv->phyname, driver->phyname) == 0) {
+			if(driver->associated) {
+				wpa_printf(MSG_DEBUG, "nl80211: Found a match for (%s) "
+					"with macaddr("MACSTR") associated to %s ",
+					driver->phyname, MAC2STR(driver->addr), driver->ssid);
+				freq = nl80211_get_assoc_freq(driver);
+				wpa_printf(MSG_DEBUG, "nl80211: shared freq for PHY(%s):%d ", drv->phyname, freq);
+			}
+		}
+	}
+
+	if(!freq)
+		wpa_printf(MSG_DEBUG, "nl80211: No shared Interface for PHY(%s)", drv->phyname);
+
+	return freq;
+}
 
 static int nl80211_send_frame(void *priv, const u8 *data, size_t data_len,
 			      int encrypt)
@@ -7612,6 +7648,7 @@  const struct wpa_driver_ops wpa_driver_nl80211_ops = {
 	.signal_monitor = nl80211_signal_monitor,
 	.signal_poll = nl80211_signal_poll,
 	.send_frame = nl80211_send_frame,
+	.shared_freq = wpa_driver_nl80211_shared_freq,
 	.set_param = nl80211_set_param,
 	.get_radio_name = nl80211_get_radio_name,
 	.add_pmkid = nl80211_add_pmkid,