[PATCH] mac80211: No echo TX pkt to Monitor interfaces for injection

[PATCH] mac80211: No echo TX pkt to Monitor interfaces for injection

[warmcat]

Block of code at the end of ieee80211_tx_status() seems to exist in order
to echo tx packets down Monitor mode interfaces with a synthesized
radiotap preamble describing the outcome of the transmission.

The effect it has with injected packets is that two copies of each
injected packet appears on the soft Monitor mode interface it was injected
down, one with a correct radiotap header reflecting what was injected,
and one with a bogus 0x0b length radiotap header.  This behaviour
was seen on iwl3945 and rt73usb.

This patch adds a new attribute to ieee80211_tx_control's flags field,
IEEE80211_TXCTL_NO_SOFT_MONITOR_ECHO, which is set for injected
packets.  The code to add a new radiotap header and echo the packet
down monitor mode interfaces is now conditional on this attribute not
being set.

Signed-off-by: Andy Green <andy@warmcat.com>
---

 include/net/mac80211.h   |    1 +
 net/mac80211/ieee80211.c |    5 +++++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 17a4dd7..6bf8f7b 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -192,6 +192,7 @@ struct ieee80211_tx_control {
 #define IEEE80211_TXCTL_FIRST_FRAGMENT	(1<<8) /* this is a first fragment of
 						* the frame */
 #define IEEE80211_TXCTL_TKIP_NEW_PHASE1_KEY (1<<9)
+#define IEEE80211_TXCTL_NO_SOFT_MONITOR_ECHO (1<<10)
 	u32 flags;			       /* tx control flags defined
 						* above */
 	u8 retry_limit;		/* 1 = only first attempt, 2 = one retry, .. */
diff --git a/net/mac80211/ieee80211.c b/net/mac80211/ieee80211.c
index 7dda339..b094274 100644
--- a/net/mac80211/ieee80211.c
+++ b/net/mac80211/ieee80211.c
@@ -1309,6 +1309,7 @@ __ieee80211_tx_prepare(struct ieee80211_txrx_data *tx,
 								TXRX_DROP) {
 			return TXRX_DROP;
 		}
+		control->flags |= IEEE80211_TXCTL_NO_SOFT_MONITOR_ECHO;
 		/*
 		 * we removed the radiotap header after this point,
 		 * we filled control with what we could use
@@ -4850,6 +4851,9 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb,
 
 	rthdr->data_retries = status->retry_count;
 
+	if (status->control.flags & IEEE80211_TXCTL_NO_SOFT_MONITOR_ECHO)
+		goto out_no_echo;
+
 	read_lock(&local->sub_if_lock);
 	monitors = local->monitors;
 	list_for_each_entry(sdata, &local->sub_if_list, list) {
@@ -4885,6 +4889,7 @@ void ieee80211_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb,
 	}
  out:
 	read_unlock(&local->sub_if_lock);
+ out_no_echo:
 	if (skb)
 		dev_kfree_skb(skb);
 }

Re: [PATCH] mac80211: No echo TX pkt to Monitor interfaces for injection

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1419 bytes --]

On Thu, 2007-08-02 at 23:38 +0100, warmcat wrote:
> Block of code at the end of ieee80211_tx_status() seems to exist in order
> to echo tx packets down Monitor mode interfaces with a synthesized
> radiotap preamble describing the outcome of the transmission.
> 
> The effect it has with injected packets is that two copies of each
> injected packet appears on the soft Monitor mode interface it was injected
> down, one with a correct radiotap header reflecting what was injected,
> and one with a bogus 0x0b length radiotap header.  This behaviour
> was seen on iwl3945 and rt73usb.
> 
> This patch adds a new attribute to ieee80211_tx_control's flags field,
> IEEE80211_TXCTL_NO_SOFT_MONITOR_ECHO, which is set for injected
> packets.  The code to add a new radiotap header and echo the packet
> down monitor mode interfaces is now conditional on this attribute not
> being set.

Ah. Now I see why this is happening. It's because any packet you send
into the monitor interface from userspace is already redirected back to
userspace by upper levels... Hmm. It *seems* this patch is correct, but
it changes the semantics slightly: previously, you would see transmitted
packets with their transmit indication (whether it was acked etc) and
now you do not get that for injected packets. Could we instead somehow
get the upper layers to not redirect that and fix the radiotap header?


johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH] mac80211: No echo TX pkt to Monitor interfaces for injection

[Andy Green]

Somebody in the thread at some point said:

> Ah. Now I see why this is happening. It's because any packet you send
> into the monitor interface from userspace is already redirected back to
> userspace by upper levels... Hmm. It *seems* this patch is correct, but
> it changes the semantics slightly: previously, you would see transmitted
> packets with their transmit indication (whether it was acked etc) and
> now you do not get that for injected packets. Could we instead somehow
> get the upper layers to not redirect that and fix the radiotap header?

With the patch, injected packets have the same Monitor mode footprint
with hard or soft monitoring (you see the injected radiotap stuff)
because the synthesized "tx report" packet doesn't exist in the real
world shown by hard monitoring.

Acutually I think where it might all be heading is Michael Buesch's
method of selecting between hard and soft Monitor mode based on
IFF_PROMISC.  Maybe stuff like the user MLE plan wants to hear about TX
reports, it's only interested in a context it is trying to associate to,
maybe it would be happy with the filtered soft Monitor (this would deal
with the objection about increased power consumption for unfiltered user
MLE hardware promisc too)... apps that truly want hard Monitor don't
want the TX reports...

If that is where we are headed then it deprecates this patch, you
actually want the TX report in soft monitor (only) which is the current
situation... we just need to get to the place that tcpdump (which sets
IFF_PROMISC for the duration) never ever sees soft Monitor...

-Andy

Re: [PATCH] mac80211: No echo TX pkt to Monitor interfaces for injection

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1169 bytes --]

On Fri, 2007-08-03 at 11:15 +0100, Andy Green wrote:

> With the patch, injected packets have the same Monitor mode footprint
> with hard or soft monitoring (you see the injected radiotap stuff)
> because the synthesized "tx report" packet doesn't exist in the real
> world shown by hard monitoring.

I don't see why there's a distinction between hard and soft monitor
mode. To me, hard monitor mode is basically "show me everything you can"
while soft is "best effort". Soft monitor mode also makes no sense while
no other interface is operating.

The synthesized 'tx report packet' isn't that; the only synthesized
thing is the radiotap header, the rest of the packet is what we actually
sent and were thus unable to capture ourselves.

> Acutually I think where it might all be heading is Michael Buesch's
> method of selecting between hard and soft Monitor mode based on
> IFF_PROMISC.

Ah. I see. You're thinking of hard/soft monitor as something totally
different than it is. Soft monitor really is only a hack for
pseudo-softmac hardware that doesn't allow you to capture control
packets while another interface is operating.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH] mac80211: No echo TX pkt to Monitor interfaces for injection

[Andy Green]

Somebody in the thread at some point said:
> On Fri, 2007-08-03 at 11:15 +0100, Andy Green wrote:
> 
>> With the patch, injected packets have the same Monitor mode footprint
>> with hard or soft monitoring (you see the injected radiotap stuff)
>> because the synthesized "tx report" packet doesn't exist in the real
>> world shown by hard monitoring.
> 
> I don't see why there's a distinction between hard and soft monitor
> mode. To me, hard monitor mode is basically "show me everything you can"
> while soft is "best effort". Soft monitor mode also makes no sense while
> no other interface is operating.

I also think it makes no sense to have a dependency on what Monitor mode
provides based on the other virtual interfaces, unless the hardware
cannot live up to the various demands at the same time.  (What hardware
has this problem that it can't run a Managed interface with hardware
promisc Rx?)

> The synthesized 'tx report packet' isn't that; the only synthesized
> thing is the radiotap header, the rest of the packet is what we actually
> sent and were thus unable to capture ourselves.

Yes, that is true.

>> Acutually I think where it might all be heading is Michael Buesch's
>> method of selecting between hard and soft Monitor mode based on
>> IFF_PROMISC.
> 
> Ah. I see. You're thinking of hard/soft monitor as something totally
> different than it is. Soft monitor really is only a hack for
> pseudo-softmac hardware that doesn't allow you to capture control
> packets while another interface is operating.

Well it has leaked out of that raison d'etre and oozed into being what
you get on any secondary virtual interface in Monitor mode.

Still I was suggesting maybe it does have another reason for existing,
the application of the usermode MLME might fit well with the filtered
aspect of it and the existing "TX report echos".

-Andy

Re: [PATCH] mac80211: No echo TX pkt to Monitor interfaces for injection

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

On Fri, 2007-08-03 at 11:34 +0100, Andy Green wrote:

> I also think it makes no sense to have a dependency on what Monitor mode
> provides based on the other virtual interfaces, unless the hardware
> cannot live up to the various demands at the same time.  (What hardware
> has this problem that it can't run a Managed interface with hardware
> promisc Rx?)

That's exactly the point though. Some hardware cannot do it and that's
why it exists. Not sure what hardware, probably Intel stuff with their
great firmware...

> Well it has leaked out of that raison d'etre and oozed into being what
> you get on any secondary virtual interface in Monitor mode.

I don't think you get that with bcm43xx since that allows
monitor_during_oper. I'm guessing that zd1211 should also support it but
just doesn't set the flag.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH] mac80211: No echo TX pkt to Monitor interfaces for injection

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On Thu, 2007-08-02 at 23:38 +0100, warmcat wrote:

> This patch adds a new attribute to ieee80211_tx_control's flags field,
> IEEE80211_TXCTL_NO_SOFT_MONITOR_ECHO, which is set for injected
> packets.  The code to add a new radiotap header and echo the packet
> down monitor mode interfaces is now conditional on this attribute not
> being set.

Regardless of whether my approach (just posted) will be accepted, this
patch cannot go in: it stops injected packets from showing up on all
monitor interfaces but the one they were injected into. If my
IFF_NO_MIRROR approach is rejected then we'll have to make this a bit
more elaborate by storing the injection device ifindex in the info
instead of just a flag.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH] mac80211: No echo TX pkt to Monitor interfaces for injection

[Andy Green]

Somebody in the thread at some point said:
> On Thu, 2007-08-02 at 23:38 +0100, warmcat wrote:
> 
>> This patch adds a new attribute to ieee80211_tx_control's flags field,
>> IEEE80211_TXCTL_NO_SOFT_MONITOR_ECHO, which is set for injected
>> packets.  The code to add a new radiotap header and echo the packet
>> down monitor mode interfaces is now conditional on this attribute not
>> being set.
> 
> Regardless of whether my approach (just posted) will be accepted, this
> patch cannot go in: it stops injected packets from showing up on all
> monitor interfaces but the one they were injected into. If my
> IFF_NO_MIRROR approach is rejected then we'll have to make this a bit
> more elaborate by storing the injection device ifindex in the info
> instead of just a flag.

Yep, fair enough.

-Andy