| Message ID | 20211222155743.256280-18-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | IEEE 802.15.4 passive scan support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next |
Hi, On Wed, 22 Dec 2021 at 10:58, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > So far only a pure software procedure for sending beacons was possible. > Let's create a couple of driver's hooks in order to allow the device > drivers to provide their own implementation. If not provided, fallback > to the pure software logic. > Can you name a SoftMAC transceiver which provides such an "offload" feature? - Alex
Hi Alexander, At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API. To be short, beacon sending is controller by our driver to be synchronized chip clock or delayed until other operation in progress (a ranging for example). Regards, David Girault
Hi, On Thu, 30 Dec 2021 at 12:00, David Girault <David.Girault@qorvo.com> wrote: > > Hi Alexander, > > At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API. > Do you want to bring this driver upstream as well? Currently those callbacks will be introduced but no user is there. > To be short, beacon sending is controller by our driver to be synchronized chip clock or delayed until > other operation in progress (a ranging for example). > ok. - Alex
Hi Alexander, alex.aring@gmail.com wrote on Thu, 30 Dec 2021 14:48:41 -0500: > Hi, > > On Thu, 30 Dec 2021 at 12:00, David Girault <David.Girault@qorvo.com> wrote: > > > > Hi Alexander, > > > > At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API. > > > Do you want to bring this driver upstream as well? Currently those > callbacks will be introduced but no user is there. I think so far the upstream fate of the DW3000 driver has not been ruled out so let's assume it won't be upstreamed (at least not fully), that's also why we decided to begin with the hwsim driver. However, when designing this series, it appeared quite clear that any hardMAC driver would need this type of interface. The content of the interface, I agree, could be further discussed and even edited, but the main idea of giving the information to the phy driver about what is happening regarding eg. scan operations or beacon frames, might make sense regardless of the current users, no? This being said, if other people decide to upstream a hardMAC driver and need these hooks to behave a little bit differently, it's their right to tweak them and that would also be part of the game. Although we might not need these hooks in a near future at all if we move to the filtering modes, because the promiscuous call with the specific level might indicate to the device how it should configure itself already. > > To be short, beacon sending is controller by our driver to be synchronized chip clock or delayed until > > other operation in progress (a ranging for example). > > > > ok. > > - Alex Thanks, Miquèl
Hi, On Wed, 5 Jan 2022 at 03:48, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > alex.aring@gmail.com wrote on Thu, 30 Dec 2021 14:48:41 -0500: > > > Hi, > > > > On Thu, 30 Dec 2021 at 12:00, David Girault <David.Girault@qorvo.com> wrote: > > > > > > Hi Alexander, > > > > > > At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API. > > > > > Do you want to bring this driver upstream as well? Currently those > > callbacks will be introduced but no user is there. > > I think so far the upstream fate of the DW3000 driver has not been ruled > out so let's assume it won't be upstreamed (at least not fully), that's > also why we decided to begin with the hwsim driver. > ok. > However, when designing this series, it appeared quite clear that any > hardMAC driver would need this type of interface. The content of the > interface, I agree, could be further discussed and even edited, but the > main idea of giving the information to the phy driver about what is > happening regarding eg. scan operations or beacon frames, might make > sense regardless of the current users, no? > A HardMAC driver does not use this driver interface... but there exists a SoftMAC driver for a HardMAC transceiver. This driver currently works because we use dataframes only... It will not support scanning currently and somehow we should make iit not available for drivers like that and for drivers which don't set symbol duration. They need to be fixed. > This being said, if other people decide to upstream a hardMAC driver > and need these hooks to behave a little bit differently, it's their > right to tweak them and that would also be part of the game. > > Although we might not need these hooks in a near future at all if we > move to the filtering modes, because the promiscuous call with the > specific level might indicate to the device how it should configure > itself already. > My concern is that somebody else might want to remove those callbacks because they are not used. - Alex
Hi Alexander, alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:23:04 -0500: > Hi, > > On Wed, 5 Jan 2022 at 03:48, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > alex.aring@gmail.com wrote on Thu, 30 Dec 2021 14:48:41 -0500: > > > > > Hi, > > > > > > On Thu, 30 Dec 2021 at 12:00, David Girault <David.Girault@qorvo.com> wrote: > > > > > > > > Hi Alexander, > > > > > > > > At Qorvo, we have developped a SoftMAC driver for our DW3000 chip that will benefit such API. > > > > > > > Do you want to bring this driver upstream as well? Currently those > > > callbacks will be introduced but no user is there. > > > > I think so far the upstream fate of the DW3000 driver has not been ruled > > out so let's assume it won't be upstreamed (at least not fully), that's > > also why we decided to begin with the hwsim driver. > > > > ok. > > > However, when designing this series, it appeared quite clear that any > > hardMAC driver would need this type of interface. The content of the > > interface, I agree, could be further discussed and even edited, but the > > main idea of giving the information to the phy driver about what is > > happening regarding eg. scan operations or beacon frames, might make > > sense regardless of the current users, no? > > > > A HardMAC driver does not use this driver interface... but there > exists a SoftMAC driver for a HardMAC transceiver. This driver > currently works because we use dataframes only... It will not support > scanning currently and somehow we should make iit not available for > drivers like that and for drivers which don't set symbol duration. > They need to be fixed. My bad. I did not look at it correctly. I made a mistake when talking about a hardMAC. Instead, it is a "custom" low level MAC layer. I believe we can compare the current mac802154 layer mostly to the MLME that is mentioned in the spec. Well here the additional layer that needs these hooks would be the MCPS. I don't know if this will be upstreamed or not, but the need for these hooks is real if such an intermediate low level MAC layer gets introduced. In v2 I will get rid of the two patches adding "driver access" to scans and beacons in order to facilitate the merge of the big part. Then we will have plenty of time to discuss how we can create such an interface. Perhaps I'll be able to propose more code as well to make use of these hooks, we will see. > > This being said, if other people decide to upstream a hardMAC driver > > and need these hooks to behave a little bit differently, it's their > > right to tweak them and that would also be part of the game. > > > > Although we might not need these hooks in a near future at all if we > > move to the filtering modes, because the promiscuous call with the > > specific level might indicate to the device how it should configure > > itself already. > > > > My concern is that somebody else might want to remove those callbacks > because they are not used. Yes, this is likely to happen quickly because of robots :) Thanks, Miquèl
Hi, On Thu, 6 Jan 2022 at 14:15, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:23:04 -0500: > ... > > > > A HardMAC driver does not use this driver interface... but there > > exists a SoftMAC driver for a HardMAC transceiver. This driver > > currently works because we use dataframes only... It will not support > > scanning currently and somehow we should make iit not available for > > drivers like that and for drivers which don't set symbol duration. > > They need to be fixed. > > My bad. I did not look at it correctly. I made a mistake when talking > about a hardMAC. > > Instead, it is a "custom" low level MAC layer. I believe we can compare > the current mac802154 layer mostly to the MLME that is mentioned in the > spec. Well here the additional layer that needs these hooks would be > the MCPS. I don't know if this will be upstreamed or not, but the need > for these hooks is real if such an intermediate low level MAC layer > gets introduced. > > In v2 I will get rid of the two patches adding "driver access" to scans > and beacons in order to facilitate the merge of the big part. Then we > will have plenty of time to discuss how we can create such an interface. > Perhaps I'll be able to propose more code as well to make use of these > hooks, we will see. > That the we have a standardised interface between Ieee802154 and (HardMAC or SoftMAC(mac802154)) (see cfg802154_ops) which is defined according the spec would make it more "stable" that it will work with different HardMAC transceivers (which follows that interface) and mac802154 stack (which also follows that interface). If I understood you correctly. I think this is one reason why we are not having any HardMAC transceivers driver supported in a proper way yet. I can also imagine about a hwsim HardMAC transceiver which redirects cfg802154 to mac802154 SoftMAC instance again (something like that), to have a virtual HardMAC transceiver for testing purpose, etc. In theory that should work... - Alex
Hi Alexander, alex.aring@gmail.com wrote on Thu, 6 Jan 2022 23:21:45 -0500: > Hi, > > On Thu, 6 Jan 2022 at 14:15, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Hi Alexander, > > > > alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:23:04 -0500: > > > ... > > > > > > A HardMAC driver does not use this driver interface... but there > > > exists a SoftMAC driver for a HardMAC transceiver. This driver > > > currently works because we use dataframes only... It will not support > > > scanning currently and somehow we should make iit not available for > > > drivers like that and for drivers which don't set symbol duration. > > > They need to be fixed. > > > > My bad. I did not look at it correctly. I made a mistake when talking > > about a hardMAC. > > > > Instead, it is a "custom" low level MAC layer. I believe we can compare > > the current mac802154 layer mostly to the MLME that is mentioned in the > > spec. Well here the additional layer that needs these hooks would be > > the MCPS. I don't know if this will be upstreamed or not, but the need > > for these hooks is real if such an intermediate low level MAC layer > > gets introduced. > > > > In v2 I will get rid of the two patches adding "driver access" to scans > > and beacons in order to facilitate the merge of the big part. Then we > > will have plenty of time to discuss how we can create such an interface. > > Perhaps I'll be able to propose more code as well to make use of these > > hooks, we will see. > > > > That the we have a standardised interface between Ieee802154 and > (HardMAC or SoftMAC(mac802154)) (see cfg802154_ops) which is defined > according the spec would make it more "stable" that it will work with > different HardMAC transceivers (which follows that interface) and > mac802154 stack (which also follows that interface). If I understood > you correctly. I am not sure. I am really talking about a softMAC. I am not sure where to put that layer "vertically" but according to the spec the MCPS (MAC Common Part Sublayer) is the layer that contains the data primitives, while MLME has been designed for management and configuration. > I think this is one reason why we are not having any HardMAC > transceivers driver supported in a proper way yet. > > I can also imagine about a hwsim HardMAC transceiver which redirects > cfg802154 to mac802154 SoftMAC instance again (something like that), > to have a virtual HardMAC transceiver for testing purpose, etc. In > theory that should work... Yeah I see what you mean, but IMHO that's basically duplicating the softMAC layer, we already have hwsim wired to cfg802154 through mac802154. In a certain way we could argue that this is a hardMAC =) Thanks, Miquèl
Hi, On Fri, 7 Jan 2022 at 02:40, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Hi Alexander, > > alex.aring@gmail.com wrote on Thu, 6 Jan 2022 23:21:45 -0500: > > > Hi, > > > > On Thu, 6 Jan 2022 at 14:15, Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > > > Hi Alexander, > > > > > > alex.aring@gmail.com wrote on Wed, 5 Jan 2022 19:23:04 -0500: > > > > > ... > > > > > > > > A HardMAC driver does not use this driver interface... but there > > > > exists a SoftMAC driver for a HardMAC transceiver. This driver > > > > currently works because we use dataframes only... It will not support > > > > scanning currently and somehow we should make iit not available for > > > > drivers like that and for drivers which don't set symbol duration. > > > > They need to be fixed. > > > > > > My bad. I did not look at it correctly. I made a mistake when talking > > > about a hardMAC. > > > > > > Instead, it is a "custom" low level MAC layer. I believe we can compare > > > the current mac802154 layer mostly to the MLME that is mentioned in the > > > spec. Well here the additional layer that needs these hooks would be > > > the MCPS. I don't know if this will be upstreamed or not, but the need > > > for these hooks is real if such an intermediate low level MAC layer > > > gets introduced. > > > > > > In v2 I will get rid of the two patches adding "driver access" to scans > > > and beacons in order to facilitate the merge of the big part. Then we > > > will have plenty of time to discuss how we can create such an interface. > > > Perhaps I'll be able to propose more code as well to make use of these > > > hooks, we will see. > > > > > > > That the we have a standardised interface between Ieee802154 and > > (HardMAC or SoftMAC(mac802154)) (see cfg802154_ops) which is defined > > according the spec would make it more "stable" that it will work with > > different HardMAC transceivers (which follows that interface) and > > mac802154 stack (which also follows that interface). If I understood > > you correctly. > > > I am not sure. I am really talking about a softMAC. I am not sure > where to put that layer "vertically" but according to the spec the MCPS > (MAC Common Part Sublayer) is the layer that contains the data > primitives, while MLME has been designed for management and > configuration. > ok. > > I think this is one reason why we are not having any HardMAC > > transceivers driver supported in a proper way yet. > > > > I can also imagine about a hwsim HardMAC transceiver which redirects > > cfg802154 to mac802154 SoftMAC instance again (something like that), > > to have a virtual HardMAC transceiver for testing purpose, etc. In > > theory that should work... > > Yeah I see what you mean, but IMHO that's basically duplicating the > softMAC layer, we already have hwsim wired to cfg802154 through > mac802154. In a certain way we could argue that this is a hardMAC =) Would be good to show people "here is how to write a HardMAC driver..." if this is even possible without any change yet. - Alex
diff --git a/include/net/mac802154.h b/include/net/mac802154.h index 97aefba7bf96..72978fb72a3a 100644 --- a/include/net/mac802154.h +++ b/include/net/mac802154.h @@ -214,6 +214,16 @@ enum ieee802154_hw_flags { * Exits the scan mode and returns to a fully functioning state. * Should only be provided if ->enter_scan_mode() is populated. * Returns either zero, or negative errno. + * + * send_beacons + * Send beacons at a fixed rate over the current channel. + * Can be NULL, if the driver doesn't support sending beacons by itself. + * Returns either zero, or negative errno. + * + * stop_beacons + * Stops sending beacons. + * Should only be provided if ->send_beacons() is populated. + * Returns either zero, or negative errno. */ struct ieee802154_ops { struct module *owner; @@ -243,6 +253,9 @@ struct ieee802154_ops { int (*enter_scan_mode)(struct ieee802154_hw *hw, struct cfg802154_scan_request *request); int (*exit_scan_mode)(struct ieee802154_hw *hw); + int (*send_beacons)(struct ieee802154_hw *hw, + struct cfg802154_beacons_request *request); + int (*stop_beacons)(struct ieee802154_hw *hw); }; /** diff --git a/net/mac802154/driver-ops.h b/net/mac802154/driver-ops.h index 2f5650f7bf91..003e6edee049 100644 --- a/net/mac802154/driver-ops.h +++ b/net/mac802154/driver-ops.h @@ -315,4 +315,37 @@ static inline int drv_exit_scan_mode(struct ieee802154_local *local) return ret; } +static inline int drv_send_beacons(struct ieee802154_local *local, + struct cfg802154_beacons_request *request) +{ + int ret; + + might_sleep(); + + if (!local->ops->send_beacons || !local->ops->stop_beacons) + return -EOPNOTSUPP; + + trace_802154_drv_send_beacons(local, request); + ret = local->ops->send_beacons(&local->hw, request); + trace_802154_drv_return_int(local, ret); + + return ret; +} + +static inline int drv_stop_beacons(struct ieee802154_local *local) +{ + int ret; + + might_sleep(); + + if (!local->ops->send_beacons || !local->ops->stop_beacons) + return -EOPNOTSUPP; + + trace_802154_drv_stop_beacons(local); + ret = local->ops->stop_beacons(&local->hw); + trace_802154_drv_return_int(local, ret); + + return ret; +} + #endif /* __MAC802154_DRIVER_OPS */ diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c index b334fa856c00..55af9d16744a 100644 --- a/net/mac802154/scan.c +++ b/net/mac802154/scan.c @@ -109,6 +109,7 @@ int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata, { struct ieee802154_local *local = sdata->local; unsigned int interval; + int ret; lockdep_assert_held(&local->beacons_lock); @@ -117,6 +118,14 @@ int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata, local->ongoing_beacons_request = true; + /* Either let the hardware handle the beacons or handle them manually */ + ret = drv_send_beacons(local, request); + if (ret != -EOPNOTSUPP) { + if (ret) + local->ongoing_beacons_request = false; + return ret; + } + interval = mac802154_scan_get_channel_time(request->interval, request->wpan_phy->symbol_duration); @@ -151,6 +160,8 @@ int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata, int mac802154_stop_beacons_locked(struct ieee802154_local *local) { + int ret; + lockdep_assert_held(&local->beacons_lock); if (!local->ongoing_beacons_request) @@ -159,6 +170,12 @@ int mac802154_stop_beacons_locked(struct ieee802154_local *local) local->ongoing_beacons_request = false; cancel_delayed_work(&local->beacons_work); + ret = drv_stop_beacons(local); + if (ret != -EOPNOTSUPP) + return ret; + + cancel_delayed_work(&local->beacons_work); + return 0; } diff --git a/net/mac802154/trace.h b/net/mac802154/trace.h index 9c0a4f07ced1..b487523f83c3 100644 --- a/net/mac802154/trace.h +++ b/net/mac802154/trace.h @@ -292,6 +292,27 @@ DEFINE_EVENT(local_only_evt4, 802154_drv_exit_scan_mode, TP_ARGS(local) ); +TRACE_EVENT(802154_drv_send_beacons, + TP_PROTO(struct ieee802154_local *local, + struct cfg802154_beacons_request *request), + TP_ARGS(local, request), + TP_STRUCT__entry( + LOCAL_ENTRY + __field(u8, interval) + ), + TP_fast_assign( + LOCAL_ASSIGN; + __entry->interval = request->interval; + ), + TP_printk(LOCAL_PR_FMT ", send beacons at interval: %d", + LOCAL_PR_ARG, __entry->interval) +); + +DEFINE_EVENT(local_only_evt4, 802154_drv_stop_beacons, + TP_PROTO(struct ieee802154_local *local), + TP_ARGS(local) +); + #endif /* !__MAC802154_DRIVER_TRACE || TRACE_HEADER_MULTI_READ */ #undef TRACE_INCLUDE_PATH