[Proposal]TX flags

[Proposal]TX flags

[Gábor Stefanik]

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

I propose the following suggested Radiotap features to be standardised:

* A new, 16-bit wide "TX flags" field (field number 15), containing
the following flags:
   - TX failed due to excessive retries,
   - CTS-to-self protection,
   - RTS/CTS-handshake,
   - Frame shall not be ACKed ("No-ACK"), and
   - Sequence number shall not be recalculated ("No-Seq").

Rationale for these changes:

     1. The "excessive retries" flag can be used by the drivers to
indicate that despite numerous retransmissions, the frame was never
acknowledged. This is useful for drivers that can't return the number
of retransmissions used, e.g. when retransmissions are handled by
hardware, with no feedback about the retries used to the driver. This
should never be set together with the No-ACK flag. It only makes sense
to use this on feedback of transmitted frames. This is currently
implemented in the Linux kernel.
     2. The CTS-to-self protection flag is useful both when frames are
sent and for feedback of transmissions. The userspace can send a frame
with this flag set, to request the driver to transmit the frame with
CTS-to-self protection (this is not implemented anywhere yet). On
feedback, the driver can indicate that it automatically transmitted
the frame using CTS-to-self protection, useful if the driver can't
return the actual CTS frame (this is implemented in Linux).
     3. The RTS/CTS handshake flag can be used the same way as the
CTS-to-self flag, to request/feedback the use of RTS/CTS handshakes
for transmitting frames. The feedback part is implemented in Linux.
     4. The No-ACK flag indicates that the frame being sent shall not
be ACKed, and as such, should only be transmitted once, with no
retransmissions. This only makes sense when sending frames from
userspace, and is useful for sending frames that are not to be ACKed
according to the IEEE 802.11 specification (e.g. multicasts, ACKs,
beacons, malformed/non-standard frames).
     5. The No-Seq flag is used when sending frames from the
userspace, to indicate that the frame already has its sequence number
preconfigured, and should not be renumbered by the low-level. This is
useful for sending fragments of a frame one-by-one (fragments need to
have the same sequence number to allow correct reassembly).

This can be accomplished with the following Radiotap changes:
* Define a new field, TX flags, at number 15, with the following parameters:
   - Field contents: an unsigned 16-bit integer,
   - Field alignment: 2;
   - Define bit 0 (mask 0x0001) of this field as "TX failed due to
excessive retries",
   - Define bit 1 (mask 0x0002) as "CTS-to-self protection",
   - Define bit 2 (mask 0x0004) as "RTS/CTS handshake",
   - Define bit 3 (mask 0x0008) as "Frame shall not be ACKed",
   - Define bit 4 (mask 0x0010) as "Pre-calculated sequence number",
   - Reserve all other bits in this field (mask 0xffe0) for future use.

Two patches have been attached to implement these changes in:
 * the Linux mac80211 wireless stack (already implements the field
with bits 0, 1 and 2 for feedback only). The attached patch implements
bits 3 and 4 for sending.
 * the Wireshark network traffic analyzer, allowing dissection of the
entire field.

If no one objects to this proposal in 7 days, I will resubmit it as a
formal RFA, to allow eventual adoption.

--Gábor Stefanik

[-- Attachment #2: wireshark-radiotap-txflags.patch --]
[-- Type: text/x-patch, Size: 6366 bytes --]

 packet-radiotap.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

Index: epan/dissectors/packet-radiotap.c
===================================================================
--- epan/dissectors/packet-radiotap.c	(revision 28045)
+++ epan/dissectors/packet-radiotap.c	(working copy)
@@ -105,6 +105,7 @@
     IEEE80211_RADIOTAP_DB_ANTSIGNAL = 12,
     IEEE80211_RADIOTAP_DB_ANTNOISE = 13,
     IEEE80211_RADIOTAP_FCS = 14,
+    IEEE80211_RADIOTAP_TX_FLAGS = 15,
     IEEE80211_RADIOTAP_XCHANNEL = 18,
     IEEE80211_RADIOTAP_EXT = 31
 };
@@ -168,6 +169,15 @@
 #define	IEEE80211_RADIOTAP_F_BADFCS	0x40	/* does not pass FCS check */
 #define	IEEE80211_RADIOTAP_F_SHORTGI	0x80	/* HT short GI */
 
+/* For IEEE80211_RADIOTAP_TX_FLAGS */
+#define IEEE80211_RADIOTAP_F_TX_FAIL	0x0001	/* failed due to excessive
+						 * retries */
+#define IEEE80211_RADIOTAP_F_TX_CTS	0x0002	/* used cts 'protection' */
+#define IEEE80211_RADIOTAP_F_TX_RTS	0x0004	/* used rts/cts handshake */
+#define IEEE80211_RADIOTAP_F_TX_NOACK	0x0008	/* frame should not be ACKed */
+#define IEEE80211_RADIOTAP_F_TX_NOSEQ	0x0010	/* sequence number handled
+						 * by userspace */
+
 /* XXX need max array size */
 static const int ieee80211_htrates[16] = {
 	13,		/* IFM_IEEE80211_MCS0 */
@@ -211,6 +221,12 @@
 static int hf_radiotap_channel_flags_sturbo = -1;
 static int hf_radiotap_channel_flags_half = -1;
 static int hf_radiotap_channel_flags_quarter = -1;
+static int hf_radiotap_txflags = -1;
+static int hf_radiotap_txflags_fail = -1;
+static int hf_radiotap_txflags_cts = -1;
+static int hf_radiotap_txflags_rts = -1;
+static int hf_radiotap_txflags_noack = -1;
+static int hf_radiotap_txflags_noseq = -1;
 static int hf_radiotap_xchannel = -1;
 static int hf_radiotap_xchannel_frequency = -1;
 static int hf_radiotap_xchannel_flags = -1;
@@ -260,6 +276,7 @@
 static int hf_radiotap_present_db_antsignal = -1;
 static int hf_radiotap_present_db_antnoise = -1;
 static int hf_radiotap_present_fcs = -1;
+static int hf_radiotap_present_txflags = -1;
 static int hf_radiotap_present_xchannel = -1;
 static int hf_radiotap_present_ext = -1;
 
@@ -282,6 +299,7 @@
 static gint ett_radiotap_present = -1;
 static gint ett_radiotap_flags = -1;
 static gint ett_radiotap_channel_flags = -1;
+static gint ett_radiotap_txflags = -1;
 static gint ett_radiotap_xchannel_flags = -1;
 
 static dissector_handle_t ieee80211_handle;
@@ -451,6 +469,7 @@
 #define RADIOTAP_MASK_DB_ANTSIGNAL          0x00001000
 #define RADIOTAP_MASK_DB_ANTNOISE           0x00002000
 #define RADIOTAP_MASK_FCS                   0x00004000
+#define RADIOTAP_MASK_TXFLAGS               0x00008000
 #define RADIOTAP_MASK_XCHANNEL              0x00040000
 #define RADIOTAP_MASK_EXT                   0x80000000
 
@@ -530,6 +549,11 @@
 	FT_BOOLEAN, 32, NULL, RADIOTAP_MASK_FCS,
 	"Specifies if the FCS field is present", HFILL } },
 
+    { &hf_radiotap_present_txflags,
+      { "TX Flags", "radiotap.present.txflags",
+	FT_BOOLEAN, 32, NULL, RADIOTAP_MASK_TXFLAGS,
+	"Specifies if the TX flags field is present", HFILL } },
+
     { &hf_radiotap_present_xchannel,
       { "Channel+", "radiotap.present.xchannel",
 	FT_BOOLEAN, 32, NULL, RADIOTAP_MASK_XCHANNEL,
@@ -653,6 +677,35 @@
        { "Quarter Rate Channel (5MHz Channel Width)", "radiotap.channel.type.quarter",
 	 FT_BOOLEAN, 16, NULL, 0x8000, "Channel Type Quarter Rate", HFILL } },
 
+    { &hf_radiotap_txflags,
+      { "TX flags", "radiotap.txflags",
+	FT_UINT16, BASE_HEX, NULL, 0x0, "", HFILL } },
+
+    { &hf_radiotap_txflags_fail,
+      { "Excessive retries", "radiotap.txflags.fail",
+	FT_BOOLEAN, 16, NULL, IEEE80211_RADIOTAP_F_TX_FAIL,
+	"Frame TX failed due to excessive retries", HFILL } },
+
+    { &hf_radiotap_txflags_cts,
+      { "CTS-to-self", "radiotap.txflags.cts",
+	FT_BOOLEAN, 16, NULL, IEEE80211_RADIOTAP_F_TX_CTS,
+	"Frame used CTS-to-self protection", HFILL } },
+
+    { &hf_radiotap_txflags_rts,
+      { "RTS/CTS", "radiotap.txflags.rts",
+	FT_BOOLEAN, 16, NULL, IEEE80211_RADIOTAP_F_TX_RTS,
+	"Frame was transmitted with an RTS/CTS-handshake", HFILL } },
+
+    { &hf_radiotap_txflags_noack,
+      { "No ACK expected", "radiotap.txflags.noack",
+	FT_BOOLEAN, 16, NULL, IEEE80211_RADIOTAP_F_TX_NOACK,
+	"Frame was transmitted once with no waiting for an ACK", HFILL } },
+
+    { &hf_radiotap_txflags_noseq,
+      { "No sequence counter", "radiotap.txflags.noseq",
+	FT_BOOLEAN, 16, NULL, IEEE80211_RADIOTAP_F_TX_NOSEQ,
+	"Frame had its sequence number configured by userspace", HFILL } },
+
     { &hf_radiotap_xchannel,
       { "Channel number", "radiotap.xchannel",
 	FT_UINT32, BASE_DEC, NULL, 0x0, "", HFILL } },
@@ -780,6 +833,7 @@
     &ett_radiotap_present,
     &ett_radiotap_flags,
     &ett_radiotap_channel_flags,
+    &ett_radiotap_txflags,
     &ett_radiotap_xchannel_flags
   };
 
@@ -896,6 +950,8 @@
 	    tvb, 4, 4, TRUE);
 	proto_tree_add_item(present_tree, hf_radiotap_present_fcs,
 	    tvb, 4, 4, TRUE);
+	proto_tree_add_item(present_tree, hf_radiotap_present_txflags,
+	    tvb, 4, 4, TRUE);
 	proto_tree_add_item(present_tree, hf_radiotap_present_xchannel,
 	    tvb, 4, 4, TRUE);
 	proto_tree_add_item(present_tree, hf_radiotap_present_ext,
@@ -1242,6 +1298,34 @@
 	    offset+=4;
 	    length_remaining-=4;
 	    break;
+	case IEEE80211_RADIOTAP_TX_FLAGS: {
+	    proto_item *it;
+
+	    align_offset = ALIGN_OFFSET(offset, 2);
+	    offset += align_offset;
+	    length_remaining -= align_offset;
+	    if (length_remaining < 2)
+		break;
+	    if (tree) {
+		flags = tvb_get_letohs(tvb, offset);
+		it = proto_tree_add_uint(radiotap_tree, hf_radiotap_txflags,
+			tvb, offset, 2, flags);
+		flags_tree = proto_item_add_subtree(it, ett_radiotap_txflags);
+		proto_tree_add_boolean(flags_tree, hf_radiotap_txflags_fail,
+			tvb, offset, 1, flags);
+		proto_tree_add_boolean(flags_tree, hf_radiotap_txflags_cts,
+			tvb, offset, 1, flags);
+		proto_tree_add_boolean(flags_tree, hf_radiotap_txflags_rts,
+			tvb, offset, 1, flags);
+		proto_tree_add_boolean(flags_tree, hf_radiotap_txflags_noack,
+			tvb, offset, 1, flags);
+		proto_tree_add_boolean(flags_tree, hf_radiotap_txflags_noseq,
+			tvb, offset, 1, flags);
+		}
+		offset+=2;
+	        length_remaining-=2;
+	    break;
+	}
 	default:
 	    /*
 	     * This indicates a field whose size we do not

[-- Attachment #3: linux-radiotap-txflags.patch --]
[-- Type: text/x-patch, Size: 4488 bytes --]

 Documentation/networking/mac80211-injection.txt |   12 +++++++++++-
 include/net/ieee80211_radiotap.h                |    3 +++
 net/mac80211/tx.c                               |   21 ++++++++++++++++-----
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/Documentation/networking/mac80211-injection.txt b/Documentation/networking/mac80211-injection.txt
index 84906ef..cd80f4a 100644
--- a/Documentation/networking/mac80211-injection.txt
+++ b/Documentation/networking/mac80211-injection.txt
@@ -36,10 +36,20 @@ radiotap headers and used to control injection:
    IEEE80211_RADIOTAP_F_FCS: FCS will be removed and recalculated
    IEEE80211_RADIOTAP_F_WEP: frame will be encrypted if key available
    IEEE80211_RADIOTAP_F_FRAG: frame will be fragmented if longer than the
-			      current fragmentation threshold. Note that
+			      current fragmentation threshold. (Note that
 			      this flag is only reliable when software
 			      fragmentation is enabled)
 
+
+ * IEEE80211_RADIOTAP_TX_FLAGS
+
+   IEEE80211_RADIOTAP_F_TX_NOACK: frame will be considered one that is not
+				  expected to be ACKed, and will only be
+				  transmitted once, with no retransmissions.
+   IEEE80211_RADIOTAP_F_TX_NOSEQ: frame will have its sequence number retained
+				  as handed to mac80211 by userspace, useful
+				  when injecting fragments of a single packet.
+
 The injection code can also skip all other currently defined radiotap fields
 facilitating replay of captured radiotap headers directly.
 
diff --git a/include/net/ieee80211_radiotap.h b/include/net/ieee80211_radiotap.h
index 23c3f3d..210dbd9 100644
--- a/include/net/ieee80211_radiotap.h
+++ b/include/net/ieee80211_radiotap.h
@@ -240,6 +240,9 @@ enum ieee80211_radiotap_type {
 						 * retries */
 #define IEEE80211_RADIOTAP_F_TX_CTS	0x0002	/* used cts 'protection' */
 #define IEEE80211_RADIOTAP_F_TX_RTS	0x0004	/* used rts/cts handshake */
+#define IEEE80211_RADIOTAP_F_TX_NOACK	0x0008	/* frame should not be ACKed */
+#define IEEE80211_RADIOTAP_F_TX_NOSEQ	0x0010	/* sequence number handled
+						 * by userspace */
 
 /* Ugly macro to convert literal channel numbers into their mhz equivalents
  * There are certianly some conditions that will break this (like feeding it '30')
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 3fb04a8..b47435d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -650,6 +650,10 @@ ieee80211_tx_h_sequence(struct ieee80211_tx_data *tx)
 	u8 *qc;
 	int tid;
 
+	if (unlikely(!(info->flags & IEEE80211_TX_CTL_ASSIGN_SEQ)))
+		return TX_CONTINUE;
+	info->flags &= ~IEEE80211_TX_CTL_ASSIGN_SEQ;
+
 	/*
 	 * Packet injection may want to control the sequence
 	 * number, if we have no matching interface then we
@@ -901,6 +905,7 @@ __ieee80211_parse_tx_radiotap(struct ieee80211_tx_data *tx,
 	struct ieee80211_radiotap_header *rthdr =
 		(struct ieee80211_radiotap_header *) skb->data;
 	struct ieee80211_supported_band *sband;
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
 	int ret = ieee80211_radiotap_iterator_init(&iterator, rthdr, skb->len);
 
 	sband = tx->local->hw.wiphy->bands[tx->channel->band];
@@ -947,6 +952,13 @@ __ieee80211_parse_tx_radiotap(struct ieee80211_tx_data *tx,
 			if (*iterator.this_arg & IEEE80211_RADIOTAP_F_FRAG)
 				tx->flags |= IEEE80211_TX_FRAGMENTED;
 			break;
+		case IEEE80211_RADIOTAP_TX_FLAGS:
+			if (*iterator.this_arg & IEEE80211_RADIOTAP_F_TX_NOACK)
+				info->flags |= IEEE80211_TX_CTL_NO_ACK;
+			if (*iterator.this_arg & IEEE80211_RADIOTAP_F_TX_NOSEQ)
+				info->flags &= ~IEEE80211_TX_CTL_ASSIGN_SEQ;
+			break;
+
 
 		/*
 		 * Please update the file
@@ -999,6 +1011,8 @@ __ieee80211_tx_prepare(struct ieee80211_tx_data *tx,
 	 * it will be cleared/left by radiotap as desired.
 	 */
 	tx->flags |= IEEE80211_TX_FRAGMENTED;
+	/* Same here, controlled by radiotap and the stack */
+	info->flags |= IEEE80211_TX_CTL_ASSIGN_SEQ;
 
 	/* process and remove the injection radiotap header */
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
@@ -1062,13 +1076,10 @@ __ieee80211_tx_prepare(struct ieee80211_tx_data *tx,
 			return TX_QUEUED;
 	}
 
-	if (is_multicast_ether_addr(hdr->addr1)) {
-		tx->flags &= ~IEEE80211_TX_UNICAST;
+	if (is_multicast_ether_addr(hdr->addr1))
 		info->flags |= IEEE80211_TX_CTL_NO_ACK;
-	} else {
+	else
 		tx->flags |= IEEE80211_TX_UNICAST;
-		info->flags &= ~IEEE80211_TX_CTL_NO_ACK;
-	}
 
 	if (tx->flags & IEEE80211_TX_FRAGMENTED) {
 		if ((tx->flags & IEEE80211_TX_UNICAST) &&

Re: [Proposal]TX flags

[David Young]

On Wed, Apr 15, 2009 at 02:33:05AM +0200, G?bor Stefanik wrote:
> I propose the following suggested Radiotap features to be standardised:
> 
> * A new, 16-bit wide "TX flags" field (field number 15), containing
> the following flags:
>    - TX failed due to excessive retries,
>    - CTS-to-self protection,
>    - RTS/CTS-handshake,
>    - Frame shall not be ACKed ("No-ACK"), and
>    - Sequence number shall not be recalculated ("No-Seq").

Thanks.

FWIW, these assignments are compatible with NetBSD.  They appear to be
compatible with FreeBSD, too.

> If no one objects to this proposal in 7 days, I will resubmit it as a
> formal RFA, to allow eventual adoption.

Great!

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933

Re: [Proposal]TX flags

[Johannes Berg]

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

On Wed, 2009-04-15 at 02:33 +0200, Gábor Stefanik wrote:

>    - Define bit 1 (mask 0x0002) as "CTS-to-self protection",
>    - Define bit 2 (mask 0x0004) as "RTS/CTS handshake",

Since you're defining these for injection too I think it would be useful
to also define a way to _not_ use RTS or CTS, or define a way to use the
default that the device would use from dot11RTSThreshold for example.

Otherwise, when leaving out these bits, it wouldn't be clear whether
that means "do not use, overriding default" or "use default".

johannes

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

Re: [Proposal]TX flags

[Gábor Stefanik]

2009/4/16 Johannes Berg <johannes@sipsolutions.net>:
> On Wed, 2009-04-15 at 02:33 +0200, G=E1bor Stefanik wrote:
>
>> =A0 =A0- Define bit 1 (mask 0x0002) as "CTS-to-self protection",
>> =A0 =A0- Define bit 2 (mask 0x0004) as "RTS/CTS handshake",
>
> Since you're defining these for injection too I think it would be use=
ful
> to also define a way to _not_ use RTS or CTS, or define a way to use =
the
> default that the device would use from dot11RTSThreshold for example.
>
> Otherwise, when leaving out these bits, it wouldn't be clear whether
> that means "do not use, overriding default" or "use default".
>
> johannes
>

Here is how I think this can be done:
Because it's impossible to have a packet protected both by CTS-to-self
and by RTS/CTS, we can define the case where both the CTS and RTS
flags are set as "Automatically use RTS or CTS as needed". The same
should be true when the TX flags field is missing entirely. When the
TX flags field is present, and both flags are 0, that should be
interpreted as "don't protect the packet".

So, in summary, this is how things should be interpreted:

TX Flags absent: Use RTS & CTS as needed.
TX Flags present: {
RTS=3D0, CTS=3D0: Use neither RTS nor CTS.
RTS=3D0, CTS=3D1: Use CTS-to-self.
RTS=3D1, CTS=3D0: Use RTS/CTS-handshake.
RTS=3D1, CTS=3D1: Use RTS & CTS as needed.
}

Alternatively, the meanings of the {0,0} and {1,1} cases could be
switched around (making the {0,0} case more logical, at the expense of
the {1,1} one):

TX Flags absent: Use RTS & CTS as needed.
TX Flags present: {
RTS=3D0, CTS=3D0: Use RTS & CTS as needed.
RTS=3D0, CTS=3D1: Use CTS-to-self.
RTS=3D1, CTS=3D0: Use RTS/CTS-handshake.
RTS=3D1, CTS=3D1: Use neither RTS nor CTS.
}

(By reading the second proposal again, I find it more and more
sympathetic... but let the discussion decide.)

--G=E1bor

--=20
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Proposal]TX flags

[Johannes Berg]

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

On Thu, 2009-04-16 at 20:47 +0200, Gábor Stefanik wrote:

> Alternatively, the meanings of the {0,0} and {1,1} cases could be
> switched around (making the {0,0} case more logical, at the expense of
> the {1,1} one):
> 
> TX Flags absent: Use RTS & CTS as needed.
> TX Flags present: {
> RTS=0, CTS=0: Use RTS & CTS as needed.
> RTS=0, CTS=1: Use CTS-to-self.
> RTS=1, CTS=0: Use RTS/CTS-handshake.
> RTS=1, CTS=1: Use neither RTS nor CTS.
> }
> 
> (By reading the second proposal again, I find it more and more
> sympathetic... but let the discussion decide.)

That _works_, but is impossible to describe in any feature discovery.

johannes

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

Re: [Proposal]TX flags

[Michael Buesch]

On Thursday 16 April 2009 20:59:34 Johannes Berg wrote:
> On Thu, 2009-04-16 at 20:47 +0200, G=C3=A1bor Stefanik wrote:
>=20
> > Alternatively, the meanings of the {0,0} and {1,1} cases could be
> > switched around (making the {0,0} case more logical, at the expense=
 of
> > the {1,1} one):
> >=20
> > TX Flags absent: Use RTS & CTS as needed.
> > TX Flags present: {
> > RTS=3D0, CTS=3D0: Use RTS & CTS as needed.
> > RTS=3D0, CTS=3D1: Use CTS-to-self.
> > RTS=3D1, CTS=3D0: Use RTS/CTS-handshake.
> > RTS=3D1, CTS=3D1: Use neither RTS nor CTS.

The first and the last thing let my head explode, because it's not what=
 somebody
would expect from such bits. This kind of logic is also used in wext. A=
nd it's why I hate wext.
"bit0 means x, bit1 means y, buuuuuuuuuuuuuuuut iff both bits are set t=
he whole logic
is inverted and whatever..."
That complicates _every_ single test of the bit (always need if (bit0 i=
s set but not bit1))
It produces spaghetti code interpreting these bits with lots of branche=
s and special
conditions that nobody does understand by reading the code alone.
If you can't encode your functionality into a boolean, do _NOT_ use bit=
s to encode it.
Use integers to encode tristate or quadstate or whatever.
You essentially _did_ that already, if you look at your bits. You use t=
he two individual bits
as 2bit integer value. So why not spell it out and use an integer field=
 for that information?

--=20
Greetings, Michael.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Proposal]TX flags

[David Young]

On Thu, Apr 16, 2009 at 08:59:34PM +0200, Johannes Berg wrote:
> On Thu, 2009-04-16 at 20:47 +0200, G?bor Stefanik wrote:
> 
> > Alternatively, the meanings of the {0,0} and {1,1} cases could be
> > switched around (making the {0,0} case more logical, at the expense of
> > the {1,1} one):
> > 
> > TX Flags absent: Use RTS & CTS as needed.
> > TX Flags present: {
> > RTS=0, CTS=0: Use RTS & CTS as needed.
> > RTS=0, CTS=1: Use CTS-to-self.
> > RTS=1, CTS=0: Use RTS/CTS-handshake.
> > RTS=1, CTS=1: Use neither RTS nor CTS.
> > }
> > 
> > (By reading the second proposal again, I find it more and more
> > sympathetic... but let the discussion decide.)
> 
> That _works_, but is impossible to describe in any feature discovery.

The discovery mechanism that we have begun to discuss would have a hard
time describing that feature at its current level of development, but
that is not the only feature that it will have a hard time describing.
Feature discovery may need more development before we measure new
proposals against it.  What do you think?

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933

Re: [Proposal]TX flags

[David Young]

On Thu, Apr 16, 2009 at 09:10:04PM +0200, Michael Buesch wrote:
> On Thursday 16 April 2009 20:59:34 Johannes Berg wrote:
> > On Thu, 2009-04-16 at 20:47 +0200, G?bor Stefanik wrote:
> > 
> > > Alternatively, the meanings of the {0,0} and {1,1} cases could be
> > > switched around (making the {0,0} case more logical, at the expense of
> > > the {1,1} one):
> > > 
> > > TX Flags absent: Use RTS & CTS as needed.
> > > TX Flags present: {
> > > RTS=0, CTS=0: Use RTS & CTS as needed.
> > > RTS=0, CTS=1: Use CTS-to-self.
> > > RTS=1, CTS=0: Use RTS/CTS-handshake.
> > > RTS=1, CTS=1: Use neither RTS nor CTS.
> 
> The first and the last thing let my head explode, because it's not
> what somebody would expect from such bits. This kind of logic is also
> used in wext. And it's why I hate wext. "bit0 means x, bit1 means y,
> buuuuuuuuuuuuuuuut iff both bits are set the whole logic is inverted
> and whatever..."  That complicates _every_ single test of the bit
> (always need if (bit0 is set but not bit1)) It produces spaghetti code
> interpreting these bits with lots of branches and special conditions
> that nobody does understand by reading the code alone.  If you can't
> encode your functionality into a boolean, do _NOT_ use bits to encode
> it.  Use integers to encode tristate or quadstate or whatever.  You
> essentially _did_ that already, if you look at your bits. You use the
> two individual bits as 2bit integer value. So why not spell it out and
> use an integer field for that information?

G?bor,

I see the point that Michael is making.  What do you think?  Shall
we treat it as a 2-bit wide unsigned integer field in the Tx flags,
instead?

Dave

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933

Re: [Proposal]TX flags

[Johannes Berg]

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

On Thu, 2009-04-16 at 15:33 -0500, David Young wrote:

> > > TX Flags absent: Use RTS & CTS as needed.
> > > TX Flags present: {
> > > RTS=0, CTS=0: Use RTS & CTS as needed.
> > > RTS=0, CTS=1: Use CTS-to-self.
> > > RTS=1, CTS=0: Use RTS/CTS-handshake.
> > > RTS=1, CTS=1: Use neither RTS nor CTS.
> > > }
> > > 
> > > (By reading the second proposal again, I find it more and more
> > > sympathetic... but let the discussion decide.)
> > 
> > That _works_, but is impossible to describe in any feature discovery.
> 
> The discovery mechanism that we have begun to discuss would have a hard
> time describing that feature at its current level of development, but
> that is not the only feature that it will have a hard time describing.
> Feature discovery may need more development before we measure new
> proposals against it.  What do you think?

True, but why make the job harder than it is? Michael has a good point
too. And we have free bits, I don't see why we can't add one for "do
what I said" (wrt. rts/cts), which, if unset means to do be in automatic
mode and ignore the other bits.

Or we really do use a two-bit field with values;
	0 automatic
	1 neither
	2 rts
	3 cts
or something.

johannes

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

Re: [Proposal]TX flags

[Gábor Stefanik]

On Thu, Apr 16, 2009 at 10:48 PM, David Young <dyoung@pobox.com> wrote:
> On Thu, Apr 16, 2009 at 09:10:04PM +0200, Michael Buesch wrote:
>> On Thursday 16 April 2009 20:59:34 Johannes Berg wrote:
>> > On Thu, 2009-04-16 at 20:47 +0200, G?bor Stefanik wrote:
>> >
>> > > Alternatively, the meanings of the {0,0} and {1,1} cases could b=
e
>> > > switched around (making the {0,0} case more logical, at the expe=
nse of
>> > > the {1,1} one):
>> > >
>> > > TX Flags absent: Use RTS & CTS as needed.
>> > > TX Flags present: {
>> > > RTS=3D0, CTS=3D0: Use RTS & CTS as needed.
>> > > RTS=3D0, CTS=3D1: Use CTS-to-self.
>> > > RTS=3D1, CTS=3D0: Use RTS/CTS-handshake.
>> > > RTS=3D1, CTS=3D1: Use neither RTS nor CTS.
>>
>> The first and the last thing let my head explode, because it's not
>> what somebody would expect from such bits. This kind of logic is als=
o
>> used in wext. And it's why I hate wext. "bit0 means x, bit1 means y,
>> buuuuuuuuuuuuuuuut iff both bits are set the whole logic is inverted
>> and whatever..." =A0That complicates _every_ single test of the bit
>> (always need if (bit0 is set but not bit1)) It produces spaghetti co=
de
>> interpreting these bits with lots of branches and special conditions
>> that nobody does understand by reading the code alone. =A0If you can=
't
>> encode your functionality into a boolean, do _NOT_ use bits to encod=
e
>> it. =A0Use integers to encode tristate or quadstate or whatever. =A0=
You
>> essentially _did_ that already, if you look at your bits. You use th=
e
>> two individual bits as 2bit integer value. So why not spell it out a=
nd
>> use an integer field for that information?
>
> G?bor,
>
> I see the point that Michael is making. =A0What do you think? =A0Shal=
l
> we treat it as a 2-bit wide unsigned integer field in the Tx flags,
> instead?
>

IMO that is a good idea, if we accept having non-booleans in a flags
field. In that case, this proposal comes to my mind:
-Define the second and third bits (mask 0x0006) as a quad-state flag
indicating the use of RTS/CTS. So, we can have these values for the
flag (accessible as (TXFlags & 0x0006) >> 1):
0: neither
1: rts
2: cts
3: auto-select (only makes sense when sending & during feature discover=
y).

Also, a proposal for feature discovery:
The userspace can send a packet that consists of nothing but a
"christmas tree" radiotap header, with no payload, but with all fields
and flags set that are known by the userspace app. The response could
be another payloadless radiotap header, copying the bits set by
userspace, but unsetting the ones not supported by the hardware. This
way, the response packet has all flags set that are supported by both
the sender and the userspace app.

--=20
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Proposal]TX flags

[Johannes Berg]

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

On Fri, 2009-04-17 at 03:24 +0200, Gábor Stefanik wrote:

> > I see the point that Michael is making.  What do you think?  Shall
> > we treat it as a 2-bit wide unsigned integer field in the Tx flags,
> > instead?
> >
> 
> IMO that is a good idea, if we accept having non-booleans in a flags
> field. In that case, this proposal comes to my mind:
> -Define the second and third bits (mask 0x0006) as a quad-state flag
> indicating the use of RTS/CTS. So, we can have these values for the
> flag (accessible as (TXFlags & 0x0006) >> 1):
> 0: neither
> 1: rts
> 2: cts
> 3: auto-select (only makes sense when sending & during feature discovery).

Like I said -- this doesn't help for feature discovery because there you
may need to say "only 0 and 2 are valid values", or maybe "only 3 is a
valid value".

I also don't like _setting_ bits for automatic -- I have a nagging
feeling that leaving out the entire field should be more similar to
setting it to all-zero, rather than all-ones. It cannot ever be exactly
the same for most fields, but still...

> Also, a proposal for feature discovery:
> The userspace can send a packet that consists of nothing but a
> "christmas tree" radiotap header, with no payload, but with all fields
> and flags set that are known by the userspace app. The response could
> be another payloadless radiotap header, copying the bits set by
> userspace, but unsetting the ones not supported by the hardware. This
> way, the response packet has all flags set that are supported by both
> the sender and the userspace app.

Let's keep that out of this thread -- but let me note that this has no
advantages (and a disadvantage in complicating the API and requiring
parsing) over unilaterally sending those bits supported by the driver --
after all the app could do it the other way and unset whatever it cannot
support in the full bitmap the driver gave it.

johannes

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

Re: [Proposal]TX flags

[Michael Stahn]

Regarding to this proposal I suggest this additional Radiotap feature
to be standardised:

- Duration field shall not be recalculated ("No-Dur").

The approbiate mask for the Radiotap-Header would be 0x0020.
The No-Dur flag is used when sending frames from the userspace
to indicate that the frame already has its duration time
preconfigured and should not be recalculated by low-level.
This is useful for testing porpuses or tools like
aireplay which trust on the "send out what you put in"-behaviour,
especialliy on RTS-Frames, where duration-fields are reset to 0.
If fragmentation is set (Bit 1, mask 0x08), duration-recalculation
should be done after all because this would imply the given
duration is reused for every subsequent fragment and there
isn't an easy way to set different durations for all of them.


--Michael Stahn

Re: [Proposal]TX flags

[Bruno Randolf]

On Tuesday 23 March 2010 04:32:21 Michael Stahn wrote:
> Regarding to this proposal I suggest this additional Radiotap feature
> to be standardised:
> 
> - Duration field shall not be recalculated ("No-Dur").
> 
> The approbiate mask for the Radiotap-Header would be 0x0020.
> The No-Dur flag is used when sending frames from the userspace
> to indicate that the frame already has its duration time
> preconfigured and should not be recalculated by low-level.
> This is useful for testing porpuses or tools like
> aireplay which trust on the "send out what you put in"-behaviour,
> especialliy on RTS-Frames, where duration-fields are reset to 0.
> If fragmentation is set (Bit 1, mask 0x08), duration-recalculation
> should be done after all because this would imply the given
> duration is reused for every subsequent fragment and there
> isn't an easy way to set different durations for all of them.

similar to this we would also need to be able to specify that the beacon TSF 
should not be overwritten. maybe a general flag like "do not change" would do?

bruno

Re: [Proposal]TX flags

[Michael Stahn]

> similar to this we would also need to be able to specify that the beacon TSF 
> should not be overwritten. maybe a general flag like "do not change" would do?
 

Is TSF really overwritten at manual injection from userspace?
Yes such a flag would be handy, the question is: is there a
need for controlling every individual TX-option (min 3 with
Sequence, Duration and TSF). If not this would be a good
solution and easy to implement. Are there any other things
which are commonly changed?

Re: [Proposal]TX flags

[Bruno Randolf]

On Wednesday 24 March 2010 00:50:31 Michael Stahn wrote:
> > similar to this we would also need to be able to specify that the beacon
> > TSF should not be overwritten. maybe a general flag like "do not change"
> > would do?
> 
> Is TSF really overwritten at manual injection from userspace?

Yes, it is, at least on ath5k. And as jouni has pointed out recently this 
behaviour is expected by hostapd (maybe not for beacons but for probe 
request/response frames, i dont know).

Gabor Stefanik proposed a while ago that frames should only be modified by the 
driver in cooked monitor mode (COOK_FRAMES), that way we could avoid defining 
new radiotap flags. What's your view on that, Jouni?

bruno

Re: [Proposal]TX flags

[Pavel Roskin]

On Wed, 2010-03-24 at 20:15 +0900, Bruno Randolf wrote:
> On Wednesday 24 March 2010 00:50:31 Michael Stahn wrote:
> > > similar to this we would also need to be able to specify that the beacon
> > > TSF should not be overwritten. maybe a general flag like "do not change"
> > > would do?
> > 
> > Is TSF really overwritten at manual injection from userspace?
> 
> Yes, it is, at least on ath5k. And as jouni has pointed out recently this 
> behaviour is expected by hostapd (maybe not for beacons but for probe 
> request/response frames, i dont know).
> 
> Gabor Stefanik proposed a while ago that frames should only be modified by the 
> driver in cooked monitor mode (COOK_FRAMES), that way we could avoid defining 
> new radiotap flags. What's your view on that, Jouni?

Maybe we could create a socket option that would put a _socket_ into a
cooked monitor mode?  A "monitor mode socket" would accept and transmit
packets with radiotap and 802.11 headers and receive the packets that
the interface already receives, also with radiotap and 802.11 headers.
That interface could be optimized for use with hostapd and other similar
programs.  The driver could fill some data from the interface settings.

The monitor mode, on the other hand, could be optimized for the
"hardcore" stuff when the driver interferes as little as possible.  Then
no special flags would be needed to pass the frame as is.

-- 
Regards,
Pavel Roskin

Re: [Proposal]TX flags

[Gábor Stefanik]

On Wed, Mar 24, 2010 at 8:43 PM, Pavel Roskin <proski@gnu.org> wrote:
> On Wed, 2010-03-24 at 20:15 +0900, Bruno Randolf wrote:
>> On Wednesday 24 March 2010 00:50:31 Michael Stahn wrote:
>> > > similar to this we would also need to be able to specify that the beacon
>> > > TSF should not be overwritten. maybe a general flag like "do not change"
>> > > would do?
>> >
>> > Is TSF really overwritten at manual injection from userspace?
>>
>> Yes, it is, at least on ath5k. And as jouni has pointed out recently this
>> behaviour is expected by hostapd (maybe not for beacons but for probe
>> request/response frames, i dont know).
>>
>> Gabor Stefanik proposed a while ago that frames should only be modified by the
>> driver in cooked monitor mode (COOK_FRAMES), that way we could avoid defining
>> new radiotap flags. What's your view on that, Jouni?
>
> Maybe we could create a socket option that would put a _socket_ into a
> cooked monitor mode?  A "monitor mode socket" would accept and transmit
> packets with radiotap and 802.11 headers and receive the packets that
> the interface already receives, also with radiotap and 802.11 headers.
> That interface could be optimized for use with hostapd and other similar
> programs.  The driver could fill some data from the interface settings.
>
> The monitor mode, on the other hand, could be optimized for the
> "hardcore" stuff when the driver interferes as little as possible.  Then
> no special flags would be needed to pass the frame as is.
>
> --
> Regards,
> Pavel Roskin
>

I don't think that is needed, as we already support multiple monitor
interfaces (including a situation where some are cooked, others are
raw).

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [Proposal]TX flags

[Michael Stahn]

> > Maybe we could create a socket option that would put a _socket_ into a
> > cooked monitor mode?  A "monitor mode socket" would accept and transmit
> > packets with radiotap and 802.11 headers and receive the packets that
> > the interface already receives, also with radiotap and 802.11 headers.
> > That interface could be optimized for use with hostapd and other similar
> > programs.  The driver could fill some data from the interface settings.
> >
> > The monitor mode, on the other hand, could be optimized for the
> > "hardcore" stuff when the driver interferes as little as possible.  Then
> > no special flags would be needed to pass the frame as is.
> >
> > --
> > Regards,
> > Pavel Roskin
> >
> 
> I don't think that is needed, as we already support multiple monitor
> interfaces (including a situation where some are cooked, others are
> raw).
> 

well after all.. for everyone who's interested I made a quick fix
considering the duration-problem. Perhaps on 2.6.34 there'll be
a stable/standardised solution for all those tx-packet-manipulation-stuff.
This is pretty much the same solution as on sequence-numbering..

---
 net/mac80211/tx.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index cbe53ed..8abf015 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -969,11 +969,18 @@ ieee80211_tx_h_encrypt(struct ieee80211_tx_data *tx)
 static ieee80211_tx_result debug_noinline
 ieee80211_tx_h_calculate_duration(struct ieee80211_tx_data *tx)
 {
+       struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
        struct sk_buff *skb = tx->skb;
        struct ieee80211_hdr *hdr;
        int next_len;
        bool group_addr;
 
+       /*
+        * Packet injection may want to control duration time.
+        */
+       if (unlikely(info->control.vif->type == NL80211_IFTYPE_MONITOR))
+               return TX_CONTINUE;
+
        do {
                hdr = (void *) skb->data;
                if (unlikely(ieee80211_is_pspoll(hdr->frame_control)))
-- 
1.6.4.4

Re: [Proposal]TX flags

[Gábor Stefanik]

On Thu, Mar 25, 2010 at 2:22 AM, Michael Stahn <m1kes@web.de> wrote:
>> > Maybe we could create a socket option that would put a _socket_ into a
>> > cooked monitor mode?  A "monitor mode socket" would accept and transmit
>> > packets with radiotap and 802.11 headers and receive the packets that
>> > the interface already receives, also with radiotap and 802.11 headers.
>> > That interface could be optimized for use with hostapd and other similar
>> > programs.  The driver could fill some data from the interface settings.
>> >
>> > The monitor mode, on the other hand, could be optimized for the
>> > "hardcore" stuff when the driver interferes as little as possible.  Then
>> > no special flags would be needed to pass the frame as is.
>> >
>> > --
>> > Regards,
>> > Pavel Roskin
>> >
>>
>> I don't think that is needed, as we already support multiple monitor
>> interfaces (including a situation where some are cooked, others are
>> raw).
>>
>
> well after all.. for everyone who's interested I made a quick fix
> considering the duration-problem. Perhaps on 2.6.34 there'll be
> a stable/standardised solution for all those tx-packet-manipulation-stuff.
> This is pretty much the same solution as on sequence-numbering..
>
> ---
>  net/mac80211/tx.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index cbe53ed..8abf015 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -969,11 +969,18 @@ ieee80211_tx_h_encrypt(struct ieee80211_tx_data *tx)
>  static ieee80211_tx_result debug_noinline
>  ieee80211_tx_h_calculate_duration(struct ieee80211_tx_data *tx)
>  {
> +       struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
>        struct sk_buff *skb = tx->skb;
>        struct ieee80211_hdr *hdr;
>        int next_len;
>        bool group_addr;
>
> +       /*
> +        * Packet injection may want to control duration time.
> +        */
> +       if (unlikely(info->control.vif->type == NL80211_IFTYPE_MONITOR))

if(unlikely(info->flags & IEEE80211_TX_CTL_INJECTED) is better. Also,
check the cooked flag!

> +               return TX_CONTINUE;
> +
>        do {
>                hdr = (void *) skb->data;
>                if (unlikely(ieee80211_is_pspoll(hdr->frame_control)))
> --
> 1.6.4.4
>
>
> --
> 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
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)