2012-11-16 10:26:00

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] mac80211: support radiotap vendor namespace RX data

From: Johannes Berg <[email protected]>

In some cases, in particular for experimentation, it
can be useful to be able to add vendor namespace data
to received frames in addition to the normal radiotap
data.

Allow doing this through mac80211 by adding fields to
the RX status descriptor that describe the data while
the data itself is prepended to the frame.

Also add some example code to hwsim, but don't enable
it because it doesn't use a proper OUI identifier.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 17 ++++++++-
include/net/mac80211.h | 12 ++++++
net/mac80211/rx.c | 71 +++++++++++++++++++++++++++++------
3 files changed, 87 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 23a3e74..6b3cb6d 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -751,7 +751,11 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
continue;
}

- nskb = skb_copy(skb, GFP_ATOMIC);
+ /*
+ * reserve some space for our vendor and the normal
+ * radiotap header, since we're copying anyway
+ */
+ nskb = skb_copy_expand(skb, 64, 0, GFP_ATOMIC);
if (nskb == NULL)
continue;

@@ -769,6 +773,17 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
(data->tsf_offset - data2->tsf_offset) +
24 * 8 * 10 / txrate->bitrate);

+#if 0
+ memcpy(skb_push(nskb, 8), "ABCDEFGH", 8);
+ rx_status.vendor_radiotap_bitmap = BIT(0);
+ rx_status.vendor_radiotap_len = 8;
+ rx_status.vendor_radiotap_align = 8;
+ rx_status.vendor_radiotap_oui[0] = 0x00;
+ rx_status.vendor_radiotap_oui[1] = 0x00;
+ rx_status.vendor_radiotap_oui[2] = 0x00;
+ rx_status.vendor_radiotap_subns = 127;
+#endif
+
memcpy(IEEE80211_SKB_RXCB(nskb), &rx_status, sizeof(rx_status));
ieee80211_rx_irqsafe(data2->hw, nskb);
}
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1bec63e..fde8a99 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -825,12 +825,21 @@ enum mac80211_rx_flags {
* @ampdu_reference: A-MPDU reference number, must be a different value for
* each A-MPDU but the same for each subframe within one A-MPDU
* @ampdu_delimiter_crc: A-MPDU delimiter CRC
+ * @vendor_radiotap_bitmap: radiotap vendor namespace presence bitmap
+ * @vendor_radiotap_len: radiotap vendor namespace length
+ * @vendor_radiotap_align: radiotap vendor namespace alignment. Note
+ * that the actual data must be at the start of the SKB data
+ * already.
+ * @vendor_radiotap_oui: radiotap vendor namespace OUI
+ * @vendor_radiotap_subns: radiotap vendor sub namespace
*/
struct ieee80211_rx_status {
u64 mactime;
u32 device_timestamp;
u32 ampdu_reference;
u32 flag;
+ u32 vendor_radiotap_bitmap;
+ u16 vendor_radiotap_len;
u16 freq;
u8 rate_idx;
u8 vht_nss;
@@ -839,6 +848,9 @@ struct ieee80211_rx_status {
u8 antenna;
s8 signal;
u8 ampdu_delimiter_crc;
+ u8 vendor_radiotap_align;
+ u8 vendor_radiotap_oui[3];
+ u8 vendor_radiotap_subns;
};

/**
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 51c9a9f..5d12c02 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -40,6 +40,8 @@
static struct sk_buff *remove_monitor_info(struct ieee80211_local *local,
struct sk_buff *skb)
{
+ struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
+
if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS) {
if (likely(skb->len > FCS_LEN))
__pskb_trim(skb, skb->len - FCS_LEN);
@@ -51,6 +53,9 @@ static struct sk_buff *remove_monitor_info(struct ieee80211_local *local,
}
}

+ if (status->vendor_radiotap_len)
+ __pskb_pull(skb, status->vendor_radiotap_len);
+
return skb;
}

@@ -73,32 +78,48 @@ static inline int should_drop_frame(struct sk_buff *skb, int present_fcs_len)
}

static int
-ieee80211_rx_radiotap_len(struct ieee80211_local *local,
- struct ieee80211_rx_status *status)
+ieee80211_rx_radiotap_space(struct ieee80211_local *local,
+ struct ieee80211_rx_status *status)
{
int len;

/* always present fields */
len = sizeof(struct ieee80211_radiotap_header) + 9;

- if (ieee80211_have_rx_timestamp(status))
+ /* allocate extra bitmap */
+ if (status->vendor_radiotap_len)
+ len += 4;
+
+ if (ieee80211_have_rx_timestamp(status)) {
+ len = ALIGN(len, 8);
len += 8;
+ }
if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
len += 1;

- if (len & 1) /* padding for RX_FLAGS if necessary */
- len++;
+ /* padding for RX_FLAGS if necessary */
+ len = ALIGN(len, 2);

if (status->flag & RX_FLAG_HT) /* HT info */
len += 3;

if (status->flag & RX_FLAG_AMPDU_DETAILS) {
- /* padding */
- while (len & 3)
- len++;
+ len = ALIGN(len, 4);
len += 8;
}

+ if (status->vendor_radiotap_len) {
+ if (WARN_ON_ONCE(status->vendor_radiotap_align == 0))
+ status->vendor_radiotap_align = 1;
+ /* align standard part of vendor namespace */
+ len = ALIGN(len, 2);
+ /* allocate standard part of vendor namespace */
+ len += 6;
+ /* align vendor-defined part */
+ len = ALIGN(len, status->vendor_radiotap_align);
+ /* vendor-defined part is already in skb */
+ }
+
return len;
}

@@ -132,14 +153,25 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
(1 << IEEE80211_RADIOTAP_CHANNEL) |
(1 << IEEE80211_RADIOTAP_ANTENNA) |
(1 << IEEE80211_RADIOTAP_RX_FLAGS));
- rthdr->it_len = cpu_to_le16(rtap_len);
+ rthdr->it_len = cpu_to_le16(rtap_len + status->vendor_radiotap_len);

pos = (unsigned char *)(rthdr + 1);

+ if (status->vendor_radiotap_len) {
+ rthdr->it_present |=
+ cpu_to_le32(BIT(IEEE80211_RADIOTAP_VENDOR_NAMESPACE)) |
+ cpu_to_le32(BIT(IEEE80211_RADIOTAP_EXT));
+ put_unaligned_le32(status->vendor_radiotap_bitmap, pos);
+ pos += 4;
+ }
+
/* the order of the following fields is important */

/* IEEE80211_RADIOTAP_TSFT */
if (ieee80211_have_rx_timestamp(status)) {
+ /* padding */
+ while ((pos - (u8 *)rthdr) & 7)
+ *pos++ = 0;
put_unaligned_le64(
ieee80211_calculate_rx_timestamp(local, status,
mpdulen, 0),
@@ -211,7 +243,7 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
/* IEEE80211_RADIOTAP_RX_FLAGS */
/* ensure 2 byte alignment for the 2 byte field as required */
if ((pos - (u8 *)rthdr) & 1)
- pos++;
+ *pos++ = 0;
if (status->flag & RX_FLAG_FAILED_PLCP_CRC)
rx_flags |= IEEE80211_RADIOTAP_F_RX_BADPLCP;
put_unaligned_le16(rx_flags, pos);
@@ -261,6 +293,21 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
*pos++ = 0;
*pos++ = 0;
}
+
+ if (status->vendor_radiotap_len) {
+ /* ensure 2 byte alignment for the vendor field as required */
+ if ((pos - (u8 *)rthdr) & 1)
+ *pos++ = 0;
+ *pos++ = status->vendor_radiotap_oui[0];
+ *pos++ = status->vendor_radiotap_oui[1];
+ *pos++ = status->vendor_radiotap_oui[2];
+ *pos++ = status->vendor_radiotap_subns;
+ put_unaligned_le16(status->vendor_radiotap_len, pos);
+ pos += 2;
+ /* align the actual payload as requested */
+ while ((pos - (u8 *)rthdr) & (status->vendor_radiotap_align - 1))
+ *pos++ = 0;
+ }
}

/*
@@ -289,7 +336,7 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
*/

/* room for the radiotap header based on driver features */
- needed_headroom = ieee80211_rx_radiotap_len(local, status);
+ needed_headroom = ieee80211_rx_radiotap_space(local, status);

if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS)
present_fcs_len = FCS_LEN;
@@ -2595,7 +2642,7 @@ static void ieee80211_rx_cooked_monitor(struct ieee80211_rx_data *rx,
goto out_free_skb;

/* room for the radiotap header based on driver features */
- needed_headroom = ieee80211_rx_radiotap_len(local, status);
+ needed_headroom = ieee80211_rx_radiotap_space(local, status);

if (skb_headroom(skb) < needed_headroom &&
pskb_expand_head(skb, needed_headroom, 0, GFP_ATOMIC))
--
1.8.0



2012-11-19 14:45:01

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: support radiotap vendor namespace RX data

On Fri, 2012-11-16 at 12:06 +0100, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> In some cases, in particular for experimentation, it
> can be useful to be able to add vendor namespace data
> to received frames in addition to the normal radiotap
> data.
>
> Allow doing this through mac80211 by adding fields to
> the RX status descriptor that describe the data while
> the data itself is prepended to the frame.
>
> Also add some example code to hwsim, but don't enable
> it because it doesn't use a proper OUI identifier.

Applied.

johannes


2012-11-16 11:05:32

by Johannes Berg

[permalink] [raw]
Subject: [PATCH v2] mac80211: support radiotap vendor namespace RX data

From: Johannes Berg <[email protected]>

In some cases, in particular for experimentation, it
can be useful to be able to add vendor namespace data
to received frames in addition to the normal radiotap
data.

Allow doing this through mac80211 by adding fields to
the RX status descriptor that describe the data while
the data itself is prepended to the frame.

Also add some example code to hwsim, but don't enable
it because it doesn't use a proper OUI identifier.

Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 33 +++++++++++++++-
include/net/mac80211.h | 12 ++++++
net/mac80211/rx.c | 71 +++++++++++++++++++++++++++++------
3 files changed, 103 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 23a3e74..e60a878 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -751,7 +751,11 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
continue;
}

- nskb = skb_copy(skb, GFP_ATOMIC);
+ /*
+ * reserve some space for our vendor and the normal
+ * radiotap header, since we're copying anyway
+ */
+ nskb = skb_copy_expand(skb, 64, 0, GFP_ATOMIC);
if (nskb == NULL)
continue;

@@ -769,6 +773,33 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
(data->tsf_offset - data2->tsf_offset) +
24 * 8 * 10 / txrate->bitrate);

+#if 0
+ /*
+ * Don't enable this code by default as the OUI 00:00:00
+ * is registered to Xerox so we shouldn't use it here, it
+ * might find its way into pcap files.
+ * Note that this code requires the headroom in the SKB
+ * that was allocated earlier.
+ */
+ rx_status.vendor_radiotap_oui[0] = 0x00;
+ rx_status.vendor_radiotap_oui[1] = 0x00;
+ rx_status.vendor_radiotap_oui[2] = 0x00;
+ rx_status.vendor_radiotap_subns = 127;
+ /*
+ * Radiotap vendor namespaces can (and should) also be
+ * split into fields by using the standard radiotap
+ * presence bitmap mechanism. Use just BIT(0) here for
+ * the presence bitmap.
+ */
+ rx_status.vendor_radiotap_bitmap = BIT(0);
+ /* We have 8 bytes of (dummy) data */
+ rx_status.vendor_radiotap_len = 8;
+ /* For testing, also require it to be aligned */
+ rx_status.vendor_radiotap_align = 8;
+ /* push the data */
+ memcpy(skb_push(nskb, 8), "ABCDEFGH", 8);
+#endif
+
memcpy(IEEE80211_SKB_RXCB(nskb), &rx_status, sizeof(rx_status));
ieee80211_rx_irqsafe(data2->hw, nskb);
}
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1bec63e..fde8a99 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -825,12 +825,21 @@ enum mac80211_rx_flags {
* @ampdu_reference: A-MPDU reference number, must be a different value for
* each A-MPDU but the same for each subframe within one A-MPDU
* @ampdu_delimiter_crc: A-MPDU delimiter CRC
+ * @vendor_radiotap_bitmap: radiotap vendor namespace presence bitmap
+ * @vendor_radiotap_len: radiotap vendor namespace length
+ * @vendor_radiotap_align: radiotap vendor namespace alignment. Note
+ * that the actual data must be at the start of the SKB data
+ * already.
+ * @vendor_radiotap_oui: radiotap vendor namespace OUI
+ * @vendor_radiotap_subns: radiotap vendor sub namespace
*/
struct ieee80211_rx_status {
u64 mactime;
u32 device_timestamp;
u32 ampdu_reference;
u32 flag;
+ u32 vendor_radiotap_bitmap;
+ u16 vendor_radiotap_len;
u16 freq;
u8 rate_idx;
u8 vht_nss;
@@ -839,6 +848,9 @@ struct ieee80211_rx_status {
u8 antenna;
s8 signal;
u8 ampdu_delimiter_crc;
+ u8 vendor_radiotap_align;
+ u8 vendor_radiotap_oui[3];
+ u8 vendor_radiotap_subns;
};

/**
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 51c9a9f..5d12c02 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -40,6 +40,8 @@
static struct sk_buff *remove_monitor_info(struct ieee80211_local *local,
struct sk_buff *skb)
{
+ struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
+
if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS) {
if (likely(skb->len > FCS_LEN))
__pskb_trim(skb, skb->len - FCS_LEN);
@@ -51,6 +53,9 @@ static struct sk_buff *remove_monitor_info(struct ieee80211_local *local,
}
}

+ if (status->vendor_radiotap_len)
+ __pskb_pull(skb, status->vendor_radiotap_len);
+
return skb;
}

@@ -73,32 +78,48 @@ static inline int should_drop_frame(struct sk_buff *skb, int present_fcs_len)
}

static int
-ieee80211_rx_radiotap_len(struct ieee80211_local *local,
- struct ieee80211_rx_status *status)
+ieee80211_rx_radiotap_space(struct ieee80211_local *local,
+ struct ieee80211_rx_status *status)
{
int len;

/* always present fields */
len = sizeof(struct ieee80211_radiotap_header) + 9;

- if (ieee80211_have_rx_timestamp(status))
+ /* allocate extra bitmap */
+ if (status->vendor_radiotap_len)
+ len += 4;
+
+ if (ieee80211_have_rx_timestamp(status)) {
+ len = ALIGN(len, 8);
len += 8;
+ }
if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
len += 1;

- if (len & 1) /* padding for RX_FLAGS if necessary */
- len++;
+ /* padding for RX_FLAGS if necessary */
+ len = ALIGN(len, 2);

if (status->flag & RX_FLAG_HT) /* HT info */
len += 3;

if (status->flag & RX_FLAG_AMPDU_DETAILS) {
- /* padding */
- while (len & 3)
- len++;
+ len = ALIGN(len, 4);
len += 8;
}

+ if (status->vendor_radiotap_len) {
+ if (WARN_ON_ONCE(status->vendor_radiotap_align == 0))
+ status->vendor_radiotap_align = 1;
+ /* align standard part of vendor namespace */
+ len = ALIGN(len, 2);
+ /* allocate standard part of vendor namespace */
+ len += 6;
+ /* align vendor-defined part */
+ len = ALIGN(len, status->vendor_radiotap_align);
+ /* vendor-defined part is already in skb */
+ }
+
return len;
}

@@ -132,14 +153,25 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
(1 << IEEE80211_RADIOTAP_CHANNEL) |
(1 << IEEE80211_RADIOTAP_ANTENNA) |
(1 << IEEE80211_RADIOTAP_RX_FLAGS));
- rthdr->it_len = cpu_to_le16(rtap_len);
+ rthdr->it_len = cpu_to_le16(rtap_len + status->vendor_radiotap_len);

pos = (unsigned char *)(rthdr + 1);

+ if (status->vendor_radiotap_len) {
+ rthdr->it_present |=
+ cpu_to_le32(BIT(IEEE80211_RADIOTAP_VENDOR_NAMESPACE)) |
+ cpu_to_le32(BIT(IEEE80211_RADIOTAP_EXT));
+ put_unaligned_le32(status->vendor_radiotap_bitmap, pos);
+ pos += 4;
+ }
+
/* the order of the following fields is important */

/* IEEE80211_RADIOTAP_TSFT */
if (ieee80211_have_rx_timestamp(status)) {
+ /* padding */
+ while ((pos - (u8 *)rthdr) & 7)
+ *pos++ = 0;
put_unaligned_le64(
ieee80211_calculate_rx_timestamp(local, status,
mpdulen, 0),
@@ -211,7 +243,7 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
/* IEEE80211_RADIOTAP_RX_FLAGS */
/* ensure 2 byte alignment for the 2 byte field as required */
if ((pos - (u8 *)rthdr) & 1)
- pos++;
+ *pos++ = 0;
if (status->flag & RX_FLAG_FAILED_PLCP_CRC)
rx_flags |= IEEE80211_RADIOTAP_F_RX_BADPLCP;
put_unaligned_le16(rx_flags, pos);
@@ -261,6 +293,21 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
*pos++ = 0;
*pos++ = 0;
}
+
+ if (status->vendor_radiotap_len) {
+ /* ensure 2 byte alignment for the vendor field as required */
+ if ((pos - (u8 *)rthdr) & 1)
+ *pos++ = 0;
+ *pos++ = status->vendor_radiotap_oui[0];
+ *pos++ = status->vendor_radiotap_oui[1];
+ *pos++ = status->vendor_radiotap_oui[2];
+ *pos++ = status->vendor_radiotap_subns;
+ put_unaligned_le16(status->vendor_radiotap_len, pos);
+ pos += 2;
+ /* align the actual payload as requested */
+ while ((pos - (u8 *)rthdr) & (status->vendor_radiotap_align - 1))
+ *pos++ = 0;
+ }
}

/*
@@ -289,7 +336,7 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
*/

/* room for the radiotap header based on driver features */
- needed_headroom = ieee80211_rx_radiotap_len(local, status);
+ needed_headroom = ieee80211_rx_radiotap_space(local, status);

if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS)
present_fcs_len = FCS_LEN;
@@ -2595,7 +2642,7 @@ static void ieee80211_rx_cooked_monitor(struct ieee80211_rx_data *rx,
goto out_free_skb;

/* room for the radiotap header based on driver features */
- needed_headroom = ieee80211_rx_radiotap_len(local, status);
+ needed_headroom = ieee80211_rx_radiotap_space(local, status);

if (skb_headroom(skb) < needed_headroom &&
pskb_expand_head(skb, needed_headroom, 0, GFP_ATOMIC))
--
1.8.0




2012-11-22 19:13:19

by Wojciech Dubowik

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: support radiotap vendor namespace RX data

On 11/22/2012 09:34 AM, Johannes Berg wrote:
> On Thu, 2012-11-22 at 09:22 +0100, Wojciech Dubowik wrote:
>> Shouldn't all functions between ieee80211_rx
>> and remove_monitor_info map header with
>> (struct ieee80211_hdr *)(skb->data + status->vendor_radiotap_len) ?
>>
>> Otherwise function like should_drop_frame uses header mapped over
>> potential vendor data.
> Huh, yes, how did I miss that. It only applies to should_drop_frame()
> though, or do you see any other place?
Actually it seems that should_drop_frame is the only one.

Wojtek
> johannes
>


2012-11-22 20:44:34

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: support radiotap vendor namespace RX data

On Thu, 2012-11-22 at 09:22 +0100, Wojciech Dubowik wrote:
> Shouldn't all functions between ieee80211_rx
> and remove_monitor_info map header with
> (struct ieee80211_hdr *)(skb->data + status->vendor_radiotap_len) ?
>
> Otherwise function like should_drop_frame uses header mapped over
> potential vendor data.

Huh, yes, how did I miss that. It only applies to should_drop_frame()
though, or do you see any other place?

johannes


2012-11-23 07:30:30

by Wojciech Dubowik

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: support radiotap vendor namespace RX data

On 11/22/2012 09:40 AM, Wojciech Dubowik wrote:
> On 11/22/2012 09:41 AM, Johannes Berg wrote:
>> On Thu, 2012-11-22 at 09:34 +0100, Johannes Berg wrote:
>>> On Thu, 2012-11-22 at 09:22 +0100, Wojciech Dubowik wrote:
>>>> Shouldn't all functions between ieee80211_rx
>>>> and remove_monitor_info map header with
>>>> (struct ieee80211_hdr *)(skb->data + status->vendor_radiotap_len) ?
>>>>
>>>> Otherwise function like should_drop_frame uses header mapped over
>>>> potential vendor data.
>>> Huh, yes, how did I miss that. It only applies to should_drop_frame()
>>> though, or do you see any other place?
>> So I think this is sufficient?
> I guess so. I will test it today.
> Wojtek
I have tested with ath9k vendor data and it doesn't seem to drop frames.
I will post my patch as RFC
once I clean it up a bit. Just for reference.

Wojtek
>>
>> johannes
>>
>> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
>> index ec15a49..ec87902 100644
>> --- a/net/mac80211/rx.c
>> +++ b/net/mac80211/rx.c
>> @@ -62,13 +62,16 @@ static struct sk_buff *remove_monitor_info(struct
>> ieee80211_local *local,
>> static inline int should_drop_frame(struct sk_buff *skb, int
>> present_fcs_len)
>> {
>> struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
>> - struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
>> + struct ieee80211_hdr *hdr;
>> +
>> + hdr = (void *)(skb->data + status->vendor_radiotap_len);
>> if (status->flag & (RX_FLAG_FAILED_FCS_CRC |
>> RX_FLAG_FAILED_PLCP_CRC |
>> RX_FLAG_AMPDU_IS_ZEROLEN))
>> return 1;
>> - if (unlikely(skb->len < 16 + present_fcs_len))
>> + if (unlikely(skb->len < 16 + present_fcs_len +
>> + status->vendor_radiotap_len))
>> return 1;
>> if (ieee80211_is_ctl(hdr->frame_control) &&
>> !ieee80211_is_pspoll(hdr->frame_control) &&
>> @@ -341,8 +344,8 @@ ieee80211_rx_monitor(struct ieee80211_local
>> *local, struct sk_buff *origskb,
>> if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS)
>> present_fcs_len = FCS_LEN;
>> - /* make sure hdr->frame_control is on the linear part */
>> - if (!pskb_may_pull(origskb, 2)) {
>> + /* ensure hdr->frame_control and vendor radiotap data are in skb
>> head */
>> + if (!pskb_may_pull(origskb, 2 + status->vendor_radiotap_len)) {
>> dev_kfree_skb(origskb);
>> return NULL;
>> }
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2012-11-22 19:23:20

by Wojciech Dubowik

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: support radiotap vendor namespace RX data

On 11/22/2012 09:41 AM, Johannes Berg wrote:
> On Thu, 2012-11-22 at 09:34 +0100, Johannes Berg wrote:
>> On Thu, 2012-11-22 at 09:22 +0100, Wojciech Dubowik wrote:
>>> Shouldn't all functions between ieee80211_rx
>>> and remove_monitor_info map header with
>>> (struct ieee80211_hdr *)(skb->data + status->vendor_radiotap_len) ?
>>>
>>> Otherwise function like should_drop_frame uses header mapped over
>>> potential vendor data.
>> Huh, yes, how did I miss that. It only applies to should_drop_frame()
>> though, or do you see any other place?
> So I think this is sufficient?
I guess so. I will test it today.
Wojtek
>
> johannes
>
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index ec15a49..ec87902 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -62,13 +62,16 @@ static struct sk_buff *remove_monitor_info(struct ieee80211_local *local,
> static inline int should_drop_frame(struct sk_buff *skb, int present_fcs_len)
> {
> struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
> - struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> + struct ieee80211_hdr *hdr;
> +
> + hdr = (void *)(skb->data + status->vendor_radiotap_len);
>
> if (status->flag & (RX_FLAG_FAILED_FCS_CRC |
> RX_FLAG_FAILED_PLCP_CRC |
> RX_FLAG_AMPDU_IS_ZEROLEN))
> return 1;
> - if (unlikely(skb->len < 16 + present_fcs_len))
> + if (unlikely(skb->len < 16 + present_fcs_len +
> + status->vendor_radiotap_len))
> return 1;
> if (ieee80211_is_ctl(hdr->frame_control) &&
> !ieee80211_is_pspoll(hdr->frame_control) &&
> @@ -341,8 +344,8 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
> if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS)
> present_fcs_len = FCS_LEN;
>
> - /* make sure hdr->frame_control is on the linear part */
> - if (!pskb_may_pull(origskb, 2)) {
> + /* ensure hdr->frame_control and vendor radiotap data are in skb head */
> + if (!pskb_may_pull(origskb, 2 + status->vendor_radiotap_len)) {
> dev_kfree_skb(origskb);
> return NULL;
> }
>
>


2012-11-22 19:43:18

by Wojciech Dubowik

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: support radiotap vendor namespace RX data

Shouldn't all functions between ieee80211_rx
and remove_monitor_info map header with
(struct ieee80211_hdr *)(skb->data + status->vendor_radiotap_len) ?

Otherwise function like should_drop_frame uses header mapped over
potential vendor data.

Wojtek
> From: Johannes Berg <[email protected]>
>
> In some cases, in particular for experimentation, it
> can be useful to be able to add vendor namespace data
> to received frames in addition to the normal radiotap
> data.
>
> Allow doing this through mac80211 by adding fields to
> the RX status descriptor that describe the data while
> the data itself is prepended to the frame.
>
> Also add some example code to hwsim, but don't enable
> it because it doesn't use a proper OUI identifier.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> drivers/net/wireless/mac80211_hwsim.c | 33 +++++++++++++++-
> include/net/mac80211.h | 12 ++++++
> net/mac80211/rx.c | 71 +++++++++++++++++++++++++++++------
> 3 files changed, 103 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
> index 23a3e74..e60a878 100644
> --- a/drivers/net/wireless/mac80211_hwsim.c
> +++ b/drivers/net/wireless/mac80211_hwsim.c
> @@ -751,7 +751,11 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
> continue;
> }
>
> - nskb = skb_copy(skb, GFP_ATOMIC);
> + /*
> + * reserve some space for our vendor and the normal
> + * radiotap header, since we're copying anyway
> + */
> + nskb = skb_copy_expand(skb, 64, 0, GFP_ATOMIC);
> if (nskb == NULL)
> continue;
>
> @@ -769,6 +773,33 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
> (data->tsf_offset - data2->tsf_offset) +
> 24 * 8 * 10 / txrate->bitrate);
>
> +#if 0
> + /*
> + * Don't enable this code by default as the OUI 00:00:00
> + * is registered to Xerox so we shouldn't use it here, it
> + * might find its way into pcap files.
> + * Note that this code requires the headroom in the SKB
> + * that was allocated earlier.
> + */
> + rx_status.vendor_radiotap_oui[0] = 0x00;
> + rx_status.vendor_radiotap_oui[1] = 0x00;
> + rx_status.vendor_radiotap_oui[2] = 0x00;
> + rx_status.vendor_radiotap_subns = 127;
> + /*
> + * Radiotap vendor namespaces can (and should) also be
> + * split into fields by using the standard radiotap
> + * presence bitmap mechanism. Use just BIT(0) here for
> + * the presence bitmap.
> + */
> + rx_status.vendor_radiotap_bitmap = BIT(0);
> + /* We have 8 bytes of (dummy) data */
> + rx_status.vendor_radiotap_len = 8;
> + /* For testing, also require it to be aligned */
> + rx_status.vendor_radiotap_align = 8;
> + /* push the data */
> + memcpy(skb_push(nskb, 8), "ABCDEFGH", 8);
> +#endif
> +
> memcpy(IEEE80211_SKB_RXCB(nskb), &rx_status, sizeof(rx_status));
> ieee80211_rx_irqsafe(data2->hw, nskb);
> }
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 1bec63e..fde8a99 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -825,12 +825,21 @@ enum mac80211_rx_flags {
> * @ampdu_reference: A-MPDU reference number, must be a different value for
> * each A-MPDU but the same for each subframe within one A-MPDU
> * @ampdu_delimiter_crc: A-MPDU delimiter CRC
> + * @vendor_radiotap_bitmap: radiotap vendor namespace presence bitmap
> + * @vendor_radiotap_len: radiotap vendor namespace length
> + * @vendor_radiotap_align: radiotap vendor namespace alignment. Note
> + * that the actual data must be at the start of the SKB data
> + * already.
> + * @vendor_radiotap_oui: radiotap vendor namespace OUI
> + * @vendor_radiotap_subns: radiotap vendor sub namespace
> */
> struct ieee80211_rx_status {
> u64 mactime;
> u32 device_timestamp;
> u32 ampdu_reference;
> u32 flag;
> + u32 vendor_radiotap_bitmap;
> + u16 vendor_radiotap_len;
> u16 freq;
> u8 rate_idx;
> u8 vht_nss;
> @@ -839,6 +848,9 @@ struct ieee80211_rx_status {
> u8 antenna;
> s8 signal;
> u8 ampdu_delimiter_crc;
> + u8 vendor_radiotap_align;
> + u8 vendor_radiotap_oui[3];
> + u8 vendor_radiotap_subns;
> };
>
> /**
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 51c9a9f..5d12c02 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -40,6 +40,8 @@
> static struct sk_buff *remove_monitor_info(struct ieee80211_local *local,
> struct sk_buff *skb)
> {
> + struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
> +
> if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS) {
> if (likely(skb->len > FCS_LEN))
> __pskb_trim(skb, skb->len - FCS_LEN);
> @@ -51,6 +53,9 @@ static struct sk_buff *remove_monitor_info(struct ieee80211_local *local,
> }
> }
>
> + if (status->vendor_radiotap_len)
> + __pskb_pull(skb, status->vendor_radiotap_len);
> +
> return skb;
> }
>
> @@ -73,32 +78,48 @@ static inline int should_drop_frame(struct sk_buff *skb, int present_fcs_len)
> }
>
> static int
> -ieee80211_rx_radiotap_len(struct ieee80211_local *local,
> - struct ieee80211_rx_status *status)
> +ieee80211_rx_radiotap_space(struct ieee80211_local *local,
> + struct ieee80211_rx_status *status)
> {
> int len;
>
> /* always present fields */
> len = sizeof(struct ieee80211_radiotap_header) + 9;
>
> - if (ieee80211_have_rx_timestamp(status))
> + /* allocate extra bitmap */
> + if (status->vendor_radiotap_len)
> + len += 4;
> +
> + if (ieee80211_have_rx_timestamp(status)) {
> + len = ALIGN(len, 8);
> len += 8;
> + }
> if (local->hw.flags & IEEE80211_HW_SIGNAL_DBM)
> len += 1;
>
> - if (len & 1) /* padding for RX_FLAGS if necessary */
> - len++;
> + /* padding for RX_FLAGS if necessary */
> + len = ALIGN(len, 2);
>
> if (status->flag & RX_FLAG_HT) /* HT info */
> len += 3;
>
> if (status->flag & RX_FLAG_AMPDU_DETAILS) {
> - /* padding */
> - while (len & 3)
> - len++;
> + len = ALIGN(len, 4);
> len += 8;
> }
>
> + if (status->vendor_radiotap_len) {
> + if (WARN_ON_ONCE(status->vendor_radiotap_align == 0))
> + status->vendor_radiotap_align = 1;
> + /* align standard part of vendor namespace */
> + len = ALIGN(len, 2);
> + /* allocate standard part of vendor namespace */
> + len += 6;
> + /* align vendor-defined part */
> + len = ALIGN(len, status->vendor_radiotap_align);
> + /* vendor-defined part is already in skb */
> + }
> +
> return len;
> }
>
> @@ -132,14 +153,25 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
> (1 << IEEE80211_RADIOTAP_CHANNEL) |
> (1 << IEEE80211_RADIOTAP_ANTENNA) |
> (1 << IEEE80211_RADIOTAP_RX_FLAGS));
> - rthdr->it_len = cpu_to_le16(rtap_len);
> + rthdr->it_len = cpu_to_le16(rtap_len + status->vendor_radiotap_len);
>
> pos = (unsigned char *)(rthdr + 1);
>
> + if (status->vendor_radiotap_len) {
> + rthdr->it_present |=
> + cpu_to_le32(BIT(IEEE80211_RADIOTAP_VENDOR_NAMESPACE)) |
> + cpu_to_le32(BIT(IEEE80211_RADIOTAP_EXT));
> + put_unaligned_le32(status->vendor_radiotap_bitmap, pos);
> + pos += 4;
> + }
> +
> /* the order of the following fields is important */
>
> /* IEEE80211_RADIOTAP_TSFT */
> if (ieee80211_have_rx_timestamp(status)) {
> + /* padding */
> + while ((pos - (u8 *)rthdr) & 7)
> + *pos++ = 0;
> put_unaligned_le64(
> ieee80211_calculate_rx_timestamp(local, status,
> mpdulen, 0),
> @@ -211,7 +243,7 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
> /* IEEE80211_RADIOTAP_RX_FLAGS */
> /* ensure 2 byte alignment for the 2 byte field as required */
> if ((pos - (u8 *)rthdr) & 1)
> - pos++;
> + *pos++ = 0;
> if (status->flag & RX_FLAG_FAILED_PLCP_CRC)
> rx_flags |= IEEE80211_RADIOTAP_F_RX_BADPLCP;
> put_unaligned_le16(rx_flags, pos);
> @@ -261,6 +293,21 @@ ieee80211_add_rx_radiotap_header(struct ieee80211_local *local,
> *pos++ = 0;
> *pos++ = 0;
> }
> +
> + if (status->vendor_radiotap_len) {
> + /* ensure 2 byte alignment for the vendor field as required */
> + if ((pos - (u8 *)rthdr) & 1)
> + *pos++ = 0;
> + *pos++ = status->vendor_radiotap_oui[0];
> + *pos++ = status->vendor_radiotap_oui[1];
> + *pos++ = status->vendor_radiotap_oui[2];
> + *pos++ = status->vendor_radiotap_subns;
> + put_unaligned_le16(status->vendor_radiotap_len, pos);
> + pos += 2;
> + /* align the actual payload as requested */
> + while ((pos - (u8 *)rthdr) & (status->vendor_radiotap_align - 1))
> + *pos++ = 0;
> + }
> }
>
> /*
> @@ -289,7 +336,7 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
> */
>
> /* room for the radiotap header based on driver features */
> - needed_headroom = ieee80211_rx_radiotap_len(local, status);
> + needed_headroom = ieee80211_rx_radiotap_space(local, status);
>
> if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS)
> present_fcs_len = FCS_LEN;
> @@ -2595,7 +2642,7 @@ static void ieee80211_rx_cooked_monitor(struct ieee80211_rx_data *rx,
> goto out_free_skb;
>
> /* room for the radiotap header based on driver features */
> - needed_headroom = ieee80211_rx_radiotap_len(local, status);
> + needed_headroom = ieee80211_rx_radiotap_space(local, status);
>
> if (skb_headroom(skb) < needed_headroom &&
> pskb_expand_head(skb, needed_headroom, 0, GFP_ATOMIC))


2012-11-16 10:47:29

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] mac80211: support radiotap vendor namespace RX data

Johannes Berg <[email protected]> writes:

> From: Johannes Berg <[email protected]>
>
> In some cases, in particular for experimentation, it
> can be useful to be able to add vendor namespace data
> to received frames in addition to the normal radiotap
> data.
>
> Allow doing this through mac80211 by adding fields to
> the RX status descriptor that describe the data while
> the data itself is prepended to the frame.
>
> Also add some example code to hwsim, but don't enable
> it because it doesn't use a proper OUI identifier.
>
> Signed-off-by: Johannes Berg <[email protected]>

[...]

> @@ -769,6 +773,17 @@ static bool mac80211_hwsim_tx_frame_no_nl(struct ieee80211_hw *hw,
> (data->tsf_offset - data2->tsf_offset) +
> 24 * 8 * 10 / txrate->bitrate);
>
> +#if 0
> + memcpy(skb_push(nskb, 8), "ABCDEFGH", 8);
> + rx_status.vendor_radiotap_bitmap = BIT(0);
> + rx_status.vendor_radiotap_len = 8;
> + rx_status.vendor_radiotap_align = 8;
> + rx_status.vendor_radiotap_oui[0] = 0x00;
> + rx_status.vendor_radiotap_oui[1] = 0x00;
> + rx_status.vendor_radiotap_oui[2] = 0x00;
> + rx_status.vendor_radiotap_subns = 127;
> +#endif

I'm nitpicking (as usual :) but a comment would be nice here, for
example the last sentence from the commit log. Otherwise I'm sure
someone will wonder why the code is not compiled.

--
Kalle Valo

2012-11-22 20:44:33

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: support radiotap vendor namespace RX data

On Thu, 2012-11-22 at 09:34 +0100, Johannes Berg wrote:
> On Thu, 2012-11-22 at 09:22 +0100, Wojciech Dubowik wrote:
> > Shouldn't all functions between ieee80211_rx
> > and remove_monitor_info map header with
> > (struct ieee80211_hdr *)(skb->data + status->vendor_radiotap_len) ?
> >
> > Otherwise function like should_drop_frame uses header mapped over
> > potential vendor data.
>
> Huh, yes, how did I miss that. It only applies to should_drop_frame()
> though, or do you see any other place?

So I think this is sufficient?

johannes

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index ec15a49..ec87902 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -62,13 +62,16 @@ static struct sk_buff *remove_monitor_info(struct ieee80211_local *local,
static inline int should_drop_frame(struct sk_buff *skb, int present_fcs_len)
{
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
- struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+ struct ieee80211_hdr *hdr;
+
+ hdr = (void *)(skb->data + status->vendor_radiotap_len);

if (status->flag & (RX_FLAG_FAILED_FCS_CRC |
RX_FLAG_FAILED_PLCP_CRC |
RX_FLAG_AMPDU_IS_ZEROLEN))
return 1;
- if (unlikely(skb->len < 16 + present_fcs_len))
+ if (unlikely(skb->len < 16 + present_fcs_len +
+ status->vendor_radiotap_len))
return 1;
if (ieee80211_is_ctl(hdr->frame_control) &&
!ieee80211_is_pspoll(hdr->frame_control) &&
@@ -341,8 +344,8 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
if (local->hw.flags & IEEE80211_HW_RX_INCLUDES_FCS)
present_fcs_len = FCS_LEN;

- /* make sure hdr->frame_control is on the linear part */
- if (!pskb_may_pull(origskb, 2)) {
+ /* ensure hdr->frame_control and vendor radiotap data are in skb head */
+ if (!pskb_may_pull(origskb, 2 + status->vendor_radiotap_len)) {
dev_kfree_skb(origskb);
return NULL;
}



2012-11-23 07:39:18

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: support radiotap vendor namespace RX data

On Fri, 2012-11-23 at 08:25 +0100, Wojciech Dubowik wrote:
> On 11/22/2012 09:40 AM, Wojciech Dubowik wrote:
> > On 11/22/2012 09:41 AM, Johannes Berg wrote:
> >> On Thu, 2012-11-22 at 09:34 +0100, Johannes Berg wrote:
> >>> On Thu, 2012-11-22 at 09:22 +0100, Wojciech Dubowik wrote:
> >>>> Shouldn't all functions between ieee80211_rx
> >>>> and remove_monitor_info map header with
> >>>> (struct ieee80211_hdr *)(skb->data + status->vendor_radiotap_len) ?
> >>>>
> >>>> Otherwise function like should_drop_frame uses header mapped over
> >>>> potential vendor data.
> >>> Huh, yes, how did I miss that. It only applies to should_drop_frame()
> >>> though, or do you see any other place?
> >> So I think this is sufficient?
> > I guess so. I will test it today.
> > Wojtek
> I have tested with ath9k vendor data and it doesn't seem to drop frames.
> I will post my patch as RFC
> once I clean it up a bit. Just for reference.

Great, thanks. I'll send out the fix & merge it.

johannes