| Message ID | 20190226094104.35192-1-julius.n@gmx.net (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames | expand |
Hi Julius, On 26.02.19 10:40, Julius Niedworok wrote: > In order to force delivery of TX status frames for research and debugging > purposes, implement the IFF_ECHO flag for ieee80211 devices. When this flag > is set for a specific interface, IEEE80211_TX_CTL_REQ_TX_STATUS is enabled > in all packets sent from that interface. IFF_ECHO can be set via > /sys/class/net/<dev>/flags. The default is disabled. (..) > + /* > + * Force TX status frames on ieee80211 devices. > + * Since IFF_ECHO is used by CAN devices for a different > + * purpose, we must check dev->ieee80211_ptr. > + */ The reason for IFF_ECHO was, that the data frame which is sent onto the wire (by one application) is not visible to all the other applications on the same (local) host. Therefore a successful transmission on the wire triggers the 'echo' of the sent content into the local host. So what are you getting back after you enabled IFF_ECHO on your mac80211 device? Is it just a 'status' about a sent packet, or is it the packet ('full content') itself? Regards, Oliver
Hi Oliver, > On 26.02.2019 12:04, Oliver Hartkopp wrote: > > Hi Julius, > (..) > > The reason for IFF_ECHO was, that the data frame which is sent onto the wire (by one application) is not visible to all the other applications on the same (local) host. Therefore a successful transmission on the wire triggers the 'echo' of the sent content into the local host. > Thank you for the explanation - I can adjust the comment, if you like to. > So what are you getting back after you enabled IFF_ECHO on your mac80211 device? > > Is it just a 'status' about a sent packet, or is it the packet ('full content') itself? We are actually getting back the full content of the packet. So it matches the behaviour of the 'echo' in CAN. > > Regards, > Oliver > Many thanks, Julius
On Tue, 2019-02-26 at 14:13 +0100, Julius Niedworok wrote: > > Thank you for the explanation - I can adjust the comment, if you like to. > > > So what are you getting back after you enabled IFF_ECHO on your mac80211 device? > > > > Is it just a 'status' about a sent packet, or is it the packet ('full content') itself? > > We are actually getting back the full content of the packet. So it > matches the behaviour of the 'echo' in CAN. I don't think it does, really. In CAN, if I understand correctly, this is used for regular operation interfaces, where you might want to run 'tcpdump', on wifi the equivalent would be 'tcpdump -i wlan0'. This *already* implements full visibility of outgoing and incoming frames. Not sure how CAN even manages *not to*, but I don't really need to care :-) You're proposing to add this to the *monitor* interfaces and you really should have made the flag conditional on that to make that clear. However, even on monitor interfaces, you typically *already* see the frames you transmitted there (as raw frames, which is the only thing you can do). What you're proposing is to use IFF_ECHO to show frames transmitted through *other* interfaces on the monitor interface. I don't think the IFF_ECHO semantics really match this. Additionally, drivers are sort of free to ignore the REQ_TX_STATUS, or we could in the future add ways of using the _noskb to feed back TX status to the state machines where needed, so I'm not really sure I even _want_ this to be set in stone in such an API. Now, I can also see how this can be useful for debugging, but it feels to me like this should be a driver (debug) option? johannes
> On 26.02.2019 14:33 Johannes Berg wrote > > You're proposing to add this to the *monitor* interfaces and you really > should have made the flag conditional on that to make that clear. > > However, even on monitor interfaces, you typically *already* see the > frames you transmitted there (as raw frames, which is the only thing you > can do). > Thanks for your prompt reply and thoughts on our patch. Let us briefly describe our test setup to ensure everyone on this mailing list is one the same page. Our general setup looks like this: 1 $ iw wlp1s0 info Interface wlp1s0 ifindex 5 wdev 0x1 addr 4c:5e:0c:11:43:ac type managed wiphy 0 txpower 30.00 dBm 1 $ iw phy phy0 interface add mon0 type monitor 1 $ iw phy phy0 interface add mon1 type monitor When we send (raw) packets on mon0 using packetspammer [1] and listen on the _other_ monitor mode interface mon1, we receive frames that were sent on the first one: 1 $ packetspammer mon0 2 $ tcpdump -i mon1 'wlan addr2 13:22:33:44:55:66' This is due to the fact that frames sent on mon0 are echoed back as TX status frames, because REQ_TX_STATUS is always set for frames sent from monitor mode interfaces. But when we replace mon0 with an interface in managed mode (wlp1s0), the receipt of frames stops, because in managed mode REQ_TX_STATUS is cleared in most frames: 1 $ ifup wlp1s0 1 $ ping -I wlp1s0 192.168.254.1 # this address is not assigned to any host 2 $ tcpdump -i mon1 ‚wlan addr2 4c:5e:0c:11:43:ac‘ > What you're proposing is to use IFF_ECHO to show frames transmitted > through *other* interfaces on the monitor interface. > > I don’t think the IFF_ECHO semantics really match this. > What we propose is to use IFF_ECHO to force REQ_TX_STATUS being set for all frames sent on the interface. But you are right: The goal is that frames transmitted through the other interface show up on the monitor interface (but only after passing the driver). However, this is exactly how we understand the semantics of IFF_ECHO in the kernel documentation. > > Additionally, drivers are sort of free to ignore the REQ_TX_STATUS, or > we could in the future add ways of using the _noskb to feed back TX > status to the state machines where needed, so I'm not really sure I even > _want_ this to be set in stone in such an API. > As far as we know, drivers must return a TX status frame, if REQ_TX_STATUS is set, but can do whatever they want, if it is clear. This is no problem for our functionality, because we force the delivery of TX status frames by permanently setting REQ_TX_STATUS. As long as the semantics of REQ_TX_STATUS remains like it is now, the functionality will always be as expected from our API. > Now, I can also see how this can be useful for debugging, but it feels > to me like this should be a driver (debug) option? > We could also achieve the functionality by modifying the drivers but this would mean that we had to add this functionality to every driver. Moreover, the feature of TX status frames, how it is implemented currently for monitor mode interfaces, is part of the mac80211 implementation. The decision to force TX status frames for monitor mode interfaces is made in the common mac80211 implementation. > johannes > Once again, thanks for your comments. We appreciate your response. Julius and Charlie [1] https://warmcat.com/git/packetspammer
> Let us briefly describe our test setup to ensure everyone on this mailing > list is one the same page. > > Our general setup looks like this: > 1 $ iw wlp1s0 info > Interface wlp1s0 > ifindex 5 > wdev 0x1 > addr 4c:5e:0c:11:43:ac > type managed > wiphy 0 > txpower 30.00 dBm > 1 $ iw phy phy0 interface add mon0 type monitor > 1 $ iw phy phy0 interface add mon1 type monitor > > When we send (raw) packets on mon0 using packetspammer [1] and listen on > the _other_ monitor mode interface mon1, we receive frames that were sent > on the first one: > 1 $ packetspammer mon0 > 2 $ tcpdump -i mon1 'wlan addr2 13:22:33:44:55:66' > > This is due to the fact that frames sent on mon0 are echoed back as TX > status frames, because REQ_TX_STATUS is always set for frames sent from > monitor mode interfaces. Yes, I understand :-) > But when we replace mon0 with an interface in managed mode (wlp1s0), the > receipt of frames stops, because in managed mode REQ_TX_STATUS is cleared > in most frames: > 1 $ ifup wlp1s0 > 1 $ ping -I wlp1s0 192.168.254.1 # this address is not assigned to any host > 2 $ tcpdump -i mon1 ‚wlan addr2 4c:5e:0c:11:43:ac‘ Yes, also understand. > > What you're proposing is to use IFF_ECHO to show frames transmitted > > through *other* interfaces on the monitor interface. > > > > I don’t think the IFF_ECHO semantics really match this. > > What we propose is to use IFF_ECHO to force REQ_TX_STATUS being set for all > frames sent on the interface. But you are right: The goal is that frames > transmitted through the other interface show up on the monitor interface > (but only after passing the driver). However, this is exactly how we > understand the semantics of IFF_ECHO in the kernel documentation. I disagree. First of all, IFF_ECHO is only documented/used *inside* the kernel, and cannot be set by userspace today. It's documented by CAN as such: Documentation/networking/can.rst: Local Loopback of Sent Frames ----------------------------- As described in :ref:`socketcan-local-loopback1` the CAN network device driver should support a local loopback functionality similar to the local echo e.g. of tty devices. In this case the driver flag IFF_ECHO has to be set to prevent the PF_CAN core from locally echoing sent frames (aka loopback) as fallback solution:: dev->flags = (IFF_NOARP | IFF_ECHO); Note that everything here is specific to a single interface. Also note that it's a signal from the *driver* to the *stack* to not do the loopback itself, because the driver will do it. I think in the case of all other sockets/interfaces, the stack will do the echo anyway, for tcpdump etc. purposes. The documentation in the uapi just states: @IFF_ECHO: echo sent packets. Volatile. and makes no representation about which interface, but I'd argue that all the flags are specific to a single interface and thus you'd expect this to also be. Thus, I don't think this was ever intended for any cross-interface behaviour, even if it may be on the same physical NIC. > As far as we know, drivers must return a TX status frame, if REQ_TX_STATUS > is set, but can do whatever they want, if it is clear. Not all drivers can and do this, I believe. Some things don't work very well if they don't do it, but I _think_ you've just been lucky and used hardware that does in fact support it. Also note that for some hardware that does support this, there's sometimes significant overhead - not just the performance overhead of actually reporting the frames, but sometimes also overhead in how the hardware is programmed and used, and how TX status is extracted. > This is no problem for our > functionality, because we force the delivery of TX status frames by > permanently setting REQ_TX_STATUS. As long as the semantics of > REQ_TX_STATUS remains like it is now, the functionality will always be > as expected from our API. Sure, for now, for your specific case of ath9k :-) > We could also achieve the functionality by modifying the drivers but this > would mean that we had to add this functionality to every driver. > Moreover, the feature of TX status frames, how it is implemented currently > for monitor mode interfaces, is part of the mac80211 implementation. The > decision to force TX status frames for monitor mode interfaces is made in > the common mac80211 implementation. I suppose it could be in mac80211 (perhaps debugfs?) too. I just really don't think IFF_ECHO is the right approach. johannes
> On 01.03.2019 09:32, Johannes Berg wrote: > > Thus, I don't think this was ever intended for any cross-interface > behaviour, even if it may be on the same physical NIC. Now we got your point. We are sorry for the confusion - it seems we understood that wrong. > Not all drivers can and do this, I believe. Some things don't work very > well if they don't do it, but I _think_ you've just been lucky and used > hardware that does in fact support it. If the drivers do not adhere to the API, this is a problem of the drivers, right? Because, how can we rely on *any* functionality of the drivers when we assume that they do not adhere to the documented interfaces? > Also note that for some hardware that does support this, there's > sometimes significant overhead - not just the performance overhead of > actually reporting the frames, but sometimes also overhead in how the > hardware is programmed and used, and how TX status is extracted. The default of this option will be disabled. If it is turned on manually for debugging, the performance impact is probably acceptable. > I suppose it could be in mac80211 (perhaps debugfs?) too. I just really > don’t think IFF_ECHO is the right approach. We see your point. So you think we should use debugfs to enable/disable our functionality? If so, we are happy to see that we find a way to force REQ_TX_STATUS to one from there. Thank you, Julius and Charlie
diff --git a/net/core/dev.c b/net/core/dev.c index 8e276e0..076b556 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -7543,6 +7543,14 @@ int __dev_change_flags(struct net_device *dev, unsigned int flags, IFF_AUTOMEDIA)) | (dev->flags & (IFF_UP | IFF_VOLATILE | IFF_PROMISC | IFF_ALLMULTI)); + /* + * Force TX status frames on ieee80211 devices. + * Since IFF_ECHO is used by CAN devices for a different + * purpose, we must check dev->ieee80211_ptr. + */ + + if (dev->ieee80211_ptr) + dev->flags = (dev->flags & ~IFF_ECHO) | (flags & IFF_ECHO); /* * Load in the correct multicast list now the flags have changed. diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 928f13a..2a02f66 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -2463,6 +2463,9 @@ static struct sk_buff *ieee80211_build_hdr(struct ieee80211_sub_if_data *sdata, if (IS_ERR(sta)) sta = NULL; + if (sdata->dev->flags & IFF_ECHO) + info_flags |= IEEE80211_TX_CTL_REQ_TX_STATUS; + /* convert Ethernet header to proper 802.11 header (based on * operation mode) */ ethertype = (skb->data[12] << 8) | skb->data[13]; @@ -3468,6 +3471,9 @@ static bool ieee80211_xmit_fast(struct ieee80211_sub_if_data *sdata, (tid_tx ? IEEE80211_TX_CTL_AMPDU : 0); info->control.flags = IEEE80211_TX_CTRL_FAST_XMIT; + if (sdata->dev->flags & IFF_ECHO) + info->flags |= IEEE80211_TX_CTL_REQ_TX_STATUS; + if (hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_QOS_DATA)) { tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK; *ieee80211_get_qos_ctl(hdr) = tid;
At Technical University of Munich we use MAC 802.11 TX status frames to perform several measurements in MAC 802.11 setups. With ath based drivers this was possible until commit d94a461d7a7df6 ("ath9k: use ieee80211_tx_status_noskb where possible") as the driver ignored the IEEE80211_TX_CTL_REQ_TX_STATUS flag and always delivered TX status frames. Since this commit, this behavior was changed and the driver now adheres to IEEE80211_TX_CTL_REQ_TX_STATUS. Due to performance reasons, IEEE80211_TX_CTL_REQ_TX_STATUS is not set for data frames from interfaces in managed mode. Hence, frames that are sent from a managed mode interface do never deliver TX status frames. This remains true even if a monitor mode interface (e.g. a measurement interface) is added to the same wireless hardware device. Thus, there is no possibility for receiving TX status frames for frames sent on an interface in managed mode if the driver adheres to IEEE80211_TX_CTL_REQ_TX_STATUS. In order to force delivery of TX status frames for research and debugging purposes, implement the IFF_ECHO flag for ieee80211 devices. When this flag is set for a specific interface, IEEE80211_TX_CTL_REQ_TX_STATUS is enabled in all packets sent from that interface. IFF_ECHO can be set via /sys/class/net/<dev>/flags. The default is disabled. Co-developed-by: Charlie Groh <ga58taw@mytum.de> Signed-off-by: Charlie Groh <ga58taw@mytum.de> Signed-off-by: Julius Niedworok <julius.n@gmx.net> --- net/core/dev.c | 8 ++++++++ net/mac80211/tx.c | 6 ++++++ 2 files changed, 14 insertions(+)