| Message ID | 1424217588-29558-1-git-send-email-greearb@candelatech.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
On Tue, 2015-02-17 at 15:59 -0800, greearb@candelatech.com wrote: > From: Ben Greear <greearb@candelatech.com> > > The goal is to allow the user-space application to properly > filter packets before sending them down to the kernel. This > should more closely mimic what a real piece of hardware would > do. > + * @HWSIM_CMD_NOTIFY: notify user-space about driver changes. This is > + * designed to help the user-space app better emulate radio hardware. > + * This command uses: > + * %HWSIM_ATTR_FREQ # Notify current operating center frequency. > + * %HWSIM_ATTR_ADDR_TRANSMITTER # ID which radio we are notifying about. > * @__HWSIM_CMD_MAX: enum limit This seems a bit strange - don't we already tag packets with the frequency? Why would you need the channel change separately? What does that even mean? Depending on how you use this it could entirely break off-channel operation, for example. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/23/2015 03:49 AM, Johannes Berg wrote: > On Tue, 2015-02-17 at 15:59 -0800, greearb@candelatech.com wrote: >> From: Ben Greear <greearb@candelatech.com> >> >> The goal is to allow the user-space application to properly >> filter packets before sending them down to the kernel. This >> should more closely mimic what a real piece of hardware would >> do. > >> + * @HWSIM_CMD_NOTIFY: notify user-space about driver changes. This is >> + * designed to help the user-space app better emulate radio hardware. >> + * This command uses: >> + * %HWSIM_ATTR_FREQ # Notify current operating center frequency. >> + * %HWSIM_ATTR_ADDR_TRANSMITTER # ID which radio we are notifying about. >> * @__HWSIM_CMD_MAX: enum limit > > This seems a bit strange - don't we already tag packets with the > frequency? Why would you need the channel change separately? What does > that even mean? Depending on how you use this it could entirely break > off-channel operation, for example. I was thinking about passive scans. In that case, we would not always get a packet transmitted when the channel changes? I was thinking user-space would mimic a real radio that can only listen on one channel at once (can any real radios actually listen on two channels at once?) So, if we are off-channel, and pkt arrives for the 'main' channel, then a real radio should drop it, right? Of course, if user-space does not care, then it can simply ignore the channel-change logic so I think this would be backwards compat with existing hwsim user-space apps. Thanks, Ben > > johannes >
On Mon, 2015-02-23 at 09:43 -0800, Ben Greear wrote: > > This seems a bit strange - don't we already tag packets with the > > frequency? Why would you need the channel change separately? What does > > that even mean? Depending on how you use this it could entirely break > > off-channel operation, for example. > > I was thinking about passive scans. In that case, we would not always get a packet transmitted > when the channel changes? Ah, well, ok. However, hwsim doesn't actually really have a concept of the 'current channel', for example in the offchannel code it just temporarily listens on two channels ... so that's not very good for a more realistic implementation :) > I was thinking user-space would mimic a real radio that can only listen on > one channel at once (can any real radios actually listen on two channels at once?) No, real radios cannot do that (not really anyway - I guess if it was VHT80 it's really already listening on 4 channels but ...) > So, if we are off-channel, and pkt arrives for the 'main' channel, then > a real radio should drop it, right? Yes. > Of course, if user-space does not care, then it can simply ignore the channel-change > logic so I think this would be backwards compat with existing hwsim user-space apps. Sure. But given the murky concept of channel change, and not going to PS for off-channel in hwsim etc. I think this would need a bit more design rather than just exposing the mac80211 channel change. Additionally, with chanctx support that won't even be invoked for example. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/24/2015 02:11 AM, Johannes Berg wrote: > On Mon, 2015-02-23 at 09:43 -0800, Ben Greear wrote: > >>> This seems a bit strange - don't we already tag packets with the >>> frequency? Why would you need the channel change separately? What does >>> that even mean? Depending on how you use this it could entirely break >>> off-channel operation, for example. >> >> I was thinking about passive scans. In that case, we would not always get a packet transmitted >> when the channel changes? > > Ah, well, ok. However, hwsim doesn't actually really have a concept of > the 'current channel', for example in the offchannel code it just > temporarily listens on two channels ... so that's not very good for a > more realistic implementation :) > >> I was thinking user-space would mimic a real radio that can only listen on >> one channel at once (can any real radios actually listen on two channels at once?) > > No, real radios cannot do that (not really anyway - I guess if it was > VHT80 it's really already listening on 4 channels but ...) > >> So, if we are off-channel, and pkt arrives for the 'main' channel, then >> a real radio should drop it, right? > > Yes. > >> Of course, if user-space does not care, then it can simply ignore the channel-change >> logic so I think this would be backwards compat with existing hwsim user-space apps. > > Sure. But given the murky concept of channel change, and not going to PS > for off-channel in hwsim etc. I think this would need a bit more design > rather than just exposing the mac80211 channel change. Additionally, > with chanctx support that won't even be invoked for example. We could push more and more of this to user-space and let it decide whether and how to forward or accept frames for particular radios. But, to do that, we need the low-level settings sent to user-space (such as current channel). Encryption keys could be a future enhancement here, so that we can do 'hardware' encryption in hwsim (and handle encrypt/decrypt logic however we want in user-space). For in-kernel testing (ie, no user-space hwsim tool), then other methods can be used, or we can just ignore the trickier details. My interest is primarily in user-space hwsim tool at the moment. Thanks, Ben
On Tue, 2015-02-24 at 06:36 -0800, Ben Greear wrote: > We could push more and more of this to user-space and let it decide whether and > how to forward or accept frames for particular radios. Sure, no objection to that. However, just arbitrarily adding a "change channel" call, without thinking about the realities of the code already supporting multi-channel concurrency, remain-on-channel and hw-scan operations won't get us very far and just lead to issues with the API. > But, to do that, we need the low-level settings sent to user-space > (such as current channel). Encryption keys could be a future enhancement > here, so that we can do 'hardware' encryption in hwsim (and handle encrypt/decrypt > logic however we want in user-space). Sure. I'd just like to ask that the API is actually useful in more than the default single-channel support BSS-only case :) johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/24/2015 06:40 AM, Johannes Berg wrote: > On Tue, 2015-02-24 at 06:36 -0800, Ben Greear wrote: > >> We could push more and more of this to user-space and let it decide whether and >> how to forward or accept frames for particular radios. > > Sure, no objection to that. However, just arbitrarily adding a "change > channel" call, without thinking about the realities of the code already > supporting multi-channel concurrency, remain-on-channel and hw-scan > operations won't get us very far and just lead to issues with the API. I took a look at the hw-scan code a bit...I guess we might could do additional info calls to user-space as we iterate through the channels while scanning? A real driver would be causing the NIC to change channels at these junctures, either by directly setting registers or sending some message off to the target NIC's cpu, right? I would assume that off-channel work could do similar logic. Is that the sort of thing you had in mind? Thanks, Ben >> But, to do that, we need the low-level settings sent to user-space >> (such as current channel). Encryption keys could be a future enhancement >> here, so that we can do 'hardware' encryption in hwsim (and handle encrypt/decrypt >> logic however we want in user-space). > > Sure. I'd just like to ask that the API is actually useful in more than > the default single-channel support BSS-only case :) > > johannes >
On Wed, 2015-03-11 at 14:05 -0700, Ben Greear wrote: > I took a look at the hw-scan code a bit...I guess we might could do additional > info calls to user-space as we iterate through the channels while scanning? > > A real driver would be causing the NIC to change channels at these junctures, > either by directly setting registers or sending some message off to the > target NIC's cpu, right? Well, depends. Our driver just asks the firmware to do the scan, and it will do all the scheduling by itself, i.e. it'll go through the channels at convenient times etc. > I would assume that off-channel work could do similar logic. Yeah. > Is that the sort of thing you had in mind? To be honest, I'm not really sure myself. It seems to really do this you'd also need powersave handling (tell the AP you're going to sleep when scanning) and potentially P2P-GO NoA handling (tell the clients you're going to sleep). The thing with hwsim right now is that it totally makes use of its ability to be on as many channels at the same time as it wants, so it doesn't have to worry about that, but if you want to actually have the channel "changed" then we need to handle a lot more details. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/31/2015 07:27 AM, Johannes Berg wrote: > On Wed, 2015-03-11 at 14:05 -0700, Ben Greear wrote: > >> I took a look at the hw-scan code a bit...I guess we might could do additional >> info calls to user-space as we iterate through the channels while scanning? >> >> A real driver would be causing the NIC to change channels at these junctures, >> either by directly setting registers or sending some message off to the >> target NIC's cpu, right? > > Well, depends. Our driver just asks the firmware to do the scan, and it > will do all the scheduling by itself, i.e. it'll go through the channels > at convenient times etc. I do not want to offload scanning in user-space, which is the equivalent of what your driver is doing as far as the kernel is concerned? If someone wants to do that in the future, then sure, they can implement such a thing. >> I would assume that off-channel work could do similar logic. > > Yeah. > >> Is that the sort of thing you had in mind? > > To be honest, I'm not really sure myself. It seems to really do this > you'd also need powersave handling (tell the AP you're going to sleep > when scanning) and potentially P2P-GO NoA handling (tell the clients > you're going to sleep). Yes, I have plans to do that sort of thing..but I have not looked into it properly yet. Either way, it seems a bit orthogonal to changing the channel..like first we send the PS packets, then change channel, etc. > The thing with hwsim right now is that it totally makes use of its > ability to be on as many channels at the same time as it wants, so it > doesn't have to worry about that, but if you want to actually have the > channel "changed" then we need to handle a lot more details. One step at a time I think...my patch should not break anything..userspace that doesn't care can simply ignore the new message. Thanks, Ben > > johannes >
On Tue, 2015-03-31 at 08:56 -0700, Ben Greear wrote: > > Well, depends. Our driver just asks the firmware to do the scan, and it > > will do all the scheduling by itself, i.e. it'll go through the channels > > at convenient times etc. > > I do not want to offload scanning in user-space, which is the equivalent > of what your driver is doing as far as the kernel is concerned? If someone wants > to do that in the future, then sure, they can implement such a thing. Well, the question isn't really that offloading, the question is what happens with the hw-scan logic in hwsim? Though I guess now that I think about it, that wouldn't show up in userspace at all with your changes. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/14/2015 01:13 AM, Johannes Berg wrote: > On Tue, 2015-03-31 at 08:56 -0700, Ben Greear wrote: > >>> Well, depends. Our driver just asks the firmware to do the scan, and it >>> will do all the scheduling by itself, i.e. it'll go through the channels >>> at convenient times etc. >> >> I do not want to offload scanning in user-space, which is the equivalent >> of what your driver is doing as far as the kernel is concerned? If someone wants >> to do that in the future, then sure, they can implement such a thing. > > Well, the question isn't really that offloading, the question is what > happens with the hw-scan logic in hwsim? Though I guess now that I think > about it, that wouldn't show up in userspace at all with your changes. I think for HW scan and related matters, you treat user-space like a firmware target, so you send it a message, and then get some response or action, similar to how you would request ath10k or some USB dongle NIC to do things for you. My own interest in hwsim user-space is to make it act more like ath9k than a firmware target driver, but I don't think my changes preclude doing target based emulation in the future. Thanks, Ben
On Tue, 2015-04-14 at 07:56 -0700, Ben Greear wrote: > > Well, the question isn't really that offloading, the question is what > > happens with the hw-scan logic in hwsim? Though I guess now that I think > > about it, that wouldn't show up in userspace at all with your changes. > > I think for HW scan and related matters, you treat user-space like a firmware > target, so you send it a message, and then get some response or action, similar > to how you would request ath10k or some USB dongle NIC to do things for you. That's not really how it works today, but it's one possible (probably the only possible) approach. > My own interest in hwsim user-space is to make it act more like ath9k > than a firmware target driver, but I don't think my changes preclude doing > target based emulation in the future. True, although I'd like to see the multi-channel issue addressed better. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/14/2015 08:06 AM, Johannes Berg wrote: > On Tue, 2015-04-14 at 07:56 -0700, Ben Greear wrote: > >>> Well, the question isn't really that offloading, the question is what >>> happens with the hw-scan logic in hwsim? Though I guess now that I think >>> about it, that wouldn't show up in userspace at all with your changes. >> >> I think for HW scan and related matters, you treat user-space like a firmware >> target, so you send it a message, and then get some response or action, similar >> to how you would request ath10k or some USB dongle NIC to do things for you. > > That's not really how it works today, but it's one possible (probably > the only possible) approach. > >> My own interest in hwsim user-space is to make it act more like ath9k >> than a firmware target driver, but I don't think my changes preclude doing >> target based emulation in the future. > > True, although I'd like to see the multi-channel issue addressed better. I need a hint or two on what exactly you want changed in my patch to address your request, or maybe you or someone else can just address the issue in follow-on patches? Thanks, Ben
On Tue, 2015-04-14 at 08:55 -0700, Ben Greear wrote: > > True, although I'd like to see the multi-channel issue addressed better. > > I need a hint or two on what exactly you want changed in my patch to > address your request, or maybe you or someone else can just address > the issue in follow-on patches? So right now you're basically saying to userspace "switch to channel X". But that's not really how the more generic system works, that's more "start using channel X (ctx=1)" / "stop using channel X (ctx=1)" / "change channel to X (ctx=1)" and similar. It seems to me that such an API should probably be the only API to userspace. The non-chanctx case in hwsim could simply fake it by starting with "start using channel X (ctx=0)" and then changing that channel all the time. The issue is that with your patch in this can't be addressed in follow-on patches as it fixes the userspace API. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/15/2015 02:33 AM, Johannes Berg wrote: > On Tue, 2015-04-14 at 08:55 -0700, Ben Greear wrote: > >>> True, although I'd like to see the multi-channel issue addressed better. >> >> I need a hint or two on what exactly you want changed in my patch to >> address your request, or maybe you or someone else can just address >> the issue in follow-on patches? > > So right now you're basically saying to userspace "switch to channel X". > But that's not really how the more generic system works, that's more > "start using channel X (ctx=1)" / "stop using channel X (ctx=1)" / > "change channel to X (ctx=1)" and similar. > > It seems to me that such an API should probably be the only API to > userspace. The non-chanctx case in hwsim could simply fake it by > starting with "start using channel X (ctx=0)" and then changing that > channel all the time. > > The issue is that with your patch in this can't be addressed in > follow-on patches as it fixes the userspace API. Ok, so adding an additional 'uint16 ctx' to the channel change data, and providing a 'type' field that includes start-using, stop-using would address this problem adequately? Start-using seems it would be the same as change-channel, or do I need a type for that as well? THanks, Ben > > johannes >
On Wed, 2015-04-15 at 08:06 -0700, Ben Greear wrote: [...] > > The issue is that with your patch in this can't be addressed in > > follow-on patches as it fixes the userspace API. > > Ok, so adding an additional 'uint16 ctx' to the channel change data, > and providing a 'type' field that includes start-using, stop-using > would address this problem adequately? > > Start-using seems it would be the same as change-channel, or do I need > a type for that as well? Yeah that seems reasonble. You'd need to store the ctx id in the actual chanctx struct in hwsim I guess (and unconditionally use 0 for the non-chanctx case) I'm not sure we should conflate start-using and change-channel, userspace might have to set up data structures if it really wants to track multiple channels, so having a separate start-using might be easier? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c index a3aaf37..3a45a45 100644 --- a/drivers/net/wireless/mac80211_hwsim.c +++ b/drivers/net/wireless/mac80211_hwsim.c @@ -851,6 +851,52 @@ static bool hwsim_ps_rx_ok(struct mac80211_hwsim_data *data, return true; } +static void mac80211_hwsim_check_nl_notify(struct mac80211_hwsim_data *data) +{ + struct sk_buff *skb; + u32 center_freq = 0; + u32 _portid; + void *msg_head; + + /* wmediumd mode check */ + _portid = ACCESS_ONCE(wmediumd_portid); + + if (!_portid) + return; + + skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_ATOMIC); + if (skb == NULL) + goto err_print; + + msg_head = genlmsg_put(skb, 0, 0, &hwsim_genl_family, 0, + HWSIM_CMD_NOTIFY); + if (msg_head == NULL) { + printk(KERN_DEBUG "mac80211_hwsim: problem with msg_head, notify\n"); + goto nla_put_failure; + } + + if (nla_put(skb, HWSIM_ATTR_ADDR_TRANSMITTER, + ETH_ALEN, data->addresses[1].addr)) + goto nla_put_failure; + + if (data->channel) + center_freq = data->channel->center_freq; + + if (nla_put_u32(skb, HWSIM_ATTR_FREQ, center_freq)) + goto nla_put_failure; + + genlmsg_end(skb, msg_head); + if (genlmsg_unicast(&init_net, skb, _portid)) + goto err_print; + + return; + +nla_put_failure: + nlmsg_free(skb); +err_print: + printk(KERN_DEBUG "mac80211_hwsim: error occurred in %s\n", __func__); +} + static void mac80211_hwsim_tx_frame_nl(struct ieee80211_hw *hw, struct sk_buff *my_skb, int dst_portid) @@ -1399,6 +1445,8 @@ static int mac80211_hwsim_config(struct ieee80211_hw *hw, u32 changed) HRTIMER_MODE_REL); } + mac80211_hwsim_check_nl_notify(data); + return 0; } diff --git a/drivers/net/wireless/mac80211_hwsim.h b/drivers/net/wireless/mac80211_hwsim.h index 85da35a..8f2306a 100644 --- a/drivers/net/wireless/mac80211_hwsim.h +++ b/drivers/net/wireless/mac80211_hwsim.h @@ -68,6 +68,11 @@ enum hwsim_tx_control_flags { * @HWSIM_CMD_CREATE_RADIO: create a new radio with the given parameters, * returns the radio ID (>= 0) or negative on errors * @HWSIM_CMD_DESTROY_RADIO: destroy a radio + * @HWSIM_CMD_NOTIFY: notify user-space about driver changes. This is + * designed to help the user-space app better emulate radio hardware. + * This command uses: + * %HWSIM_ATTR_FREQ # Notify current operating center frequency. + * %HWSIM_ATTR_ADDR_TRANSMITTER # ID which radio we are notifying about. * @__HWSIM_CMD_MAX: enum limit */ enum { @@ -77,6 +82,7 @@ enum { HWSIM_CMD_TX_INFO_FRAME, HWSIM_CMD_CREATE_RADIO, HWSIM_CMD_DESTROY_RADIO, + HWSIM_CMD_NOTIFY, __HWSIM_CMD_MAX, }; #define HWSIM_CMD_MAX (_HWSIM_CMD_MAX - 1)