Date: Wed, 15 Feb 2012 10:46:35 +0100 From: Monthadar Al Jaberi <monthadar@gmail.com> To: Rui Paulo <rpaulo@freebsd.org> Cc: freebsd-wireless@freebsd.org, Bernhard Schmidt <bschmidt@freebsd.org> Subject: Re: Fragment and 11s inconsistency Message-ID: <CA%2BsBSoL8WPB5pGMBrQDM46kn=BRkqW1HrzeVuoBQ6S=NfjGz=Q@mail.gmail.com> In-Reply-To: <CA%2BsBSoJOjgxBvQa_kN0oDyS4AUeu7kKGUB37Yz3T=JfzMqGG8Q@mail.gmail.com> References: <CA%2BsBSoJGzabfyt8EXoT-g=GTJD0upX5jbRqfLWKQKzXu2FivaA@mail.gmail.com> <3F2E258F-1F82-48D7-AAAD-546BCB03EDE2@freebsd.org> <CA%2BsBSo%2BS-quy2p1O8k9TDZO1Csgg19pgTMv7y0VeXWSgq3=mzQ@mail.gmail.com> <CA%2BsBSoJOjgxBvQa_kN0oDyS4AUeu7kKGUB37Yz3T=JfzMqGG8Q@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
sorry for the noise, I found out that I had an error when reading out
the QoS control field, it is corrected now and I updated the comment.
I tested the code without fragmentation and it should work fine now.
On Wed, Feb 15, 2012 at 10:26 AM, Monthadar Al Jaberi
<monthadar@gmail.com> wrote:
> On Wed, Feb 15, 2012 at 9:26 AM, Monthadar Al Jaberi
> <monthadar@gmail.com> wrote:
>> On Wed, Feb 15, 2012 at 5:50 AM, Rui Paulo <rpaulo@freebsd.org> wrote:
>>> On 2012/02/14, at 14:14, Monthadar Al Jaberi wrote:
>>>
>>>> Hi,
>>>>
>>>> I cant verify this yet, but isn't there something wrong in current FreeBSD?
>>>>
>>>> lets say an 11s data frame need to be fragmented in ieee80211_encap:
>>>> if (addqos)
>>>> hdrsize = sizeof(struct ieee80211_qosframe);
>>>> else
>>>> hdrsize = sizeof(struct ieee80211_frame);
>>>> ...
>>>> if (vap->iv_opmode == IEEE80211_M_MBSS) {
>>>> ...
>>>> if (!IEEE80211_IS_MULTICAST(eh.ether_dhost))
>>>> hdrsize += IEEE80211_ADDR_LEN; /* unicast are 4-addr */
>>>> meshhdrsize = sizeof(struct ieee80211_meshcntl);
>>>> }
>>>> ...
>>>> if (__predict_true((m->m_flags & M_FF) == 0)) {
>>>> /*
>>>> * Normal frame.
>>>> */
>>>> m = ieee80211_mbuf_adjust(vap, hdrspace + meshhdrsize, key, m);
>>>> }
>>>> M_PREPEND(m, hdrspace + meshhdrsize, M_DONTWAIT);
>>>> if (txfrag && !ieee80211_fragment(vap, m, hdrsize,
>>>> key != NULL ? key->wk_cipher->ic_header : 0, vap->iv_fragthreshold))
>>>> goto bad;
>>>>
>>>> This means we send meshcontrol only in first segment, because we never
>>>> add meshhdrsize to hdrsize...
>>>
>>> Yes, this is a bug. Thanks for finding it.
>>>
>>>> but in mesh_input
>>>> switch (type) {
>>>> case IEEE80211_FC0_TYPE_DATA:
>>>> ...
>>>> meshdrlen = sizeof(struct ieee80211_meshcntl) +
>>>> (mc->mc_flags & 3) * IEEE80211_ADDR_LEN;
>>>> hdrspace += meshdrlen;
>>>> ...
>>>> /*
>>>> * Potentially forward packet. See table s36 (p140)
>>>> * for the rules. XXX tap fwd'd packets not for us?
>>>> */
>>>> if (dir == IEEE80211_FC1_DIR_FROMDS ||
>>>> !mesh_isucastforme(vap, wh, mc)) {
>>>> mesh_forward(vap, m, mc);
>>>> ...
>>>> if (!IEEE80211_IS_MULTICAST(wh->i_addr1)) {
>>>> m = ieee80211_defrag(ni, m, hdrspace);
>>>> if (m == NULL) {
>>>>
>>>> This seems wrong to me, how can we potentially forward before defragin
>>>> the frame? this will fail cause sub-frag have no mesh control as code
>>>> shows above.
>>>>
>>>> shouldnt the defrag code be moved above?
>>>
>>> Yes, another bug.
>>>
>>>> I am attaching a patch that both moves the defrag, validates that mesh
>>>> control is present and makes 11s data frames QoS and set Mesh control
>>>> bit present to 1 as the amendment specifies.
>>>
>>> There are some typos in your patch ("This is in constrast to Draf 4.0.").
>>>
>>> Can you please rewrite this code:
>>> + *(uint16_t *)qos = *(uint16_t *)
>>> + (((IEEE80211_IS_MULTICAST(wh->i_addr1) &&
>>> + dir == IEEE80211_FC1_DIR_FROMDS)) ?
>>> + ((struct ieee80211_qosframe *)wh)->i_qos :
>>> + ((struct ieee80211_qosframe_addr4 *)wh)->i_qos);
>>>
>>> In ieee80211_encap(), why didn't you add meshhdrsize to ieee80211_fragment()?
>>
>> Because the mesh control is not part of the header, it is in the frame
>> body and should only be present in the first fragment of a frame. So I
>> think this is correct.
>>
>
> Attaching patch again; this time Mesh Control field is set to not
> present for frags 1+.
>
>>>
>>> Would be nice to test this before committing.
>>
>> Yes, Adrian and Bernhard are looking into the fragment code between
>> net80211 and ath. It seems the code is broken.
>>
>> I am reattaching the patch thanks for your comments.
>>
>>>
>>> Thanks,
>>> --
>>> Rui Paulo
>>>
>>
>>
>>
>> --
>> Monthadar Al Jaberi
>
> br
>
> --
> Monthadar Al Jaberi
--
Monthadar Al Jaberi
[-- Attachment #2 --]
From 3c248c961509750d154ac44e03f6808c78c7a8cd Mon Sep 17 00:00:00 2001
From: Monthadar Al Jaberi <monthadar@gmail.com>
Date: Tue, 14 Feb 2012 16:47:43 +0100
Subject: [PATCH] Make mesh data frames to be quality of service (QoS).
* Introduce new flag for QoS control field;
* Change in mesh_input to validate that QoS is set and Mesh Control field
is present, also both bytes of the QoS are read;
* Moved defragmentation in mesh_input before we try to forward packet as
inferred from amendment spec, because Mesh Control field only present in first
fragment;
* Changed in ieee80211_encap to set QoS subtype and Mesh Control field present,
only first fragment have Mesh Control field present bit equal to 1;
---
sys/net80211/ieee80211.h | 7 +++
sys/net80211/ieee80211_mesh.c | 85 +++++++++++++++++++++++++++-----------
sys/net80211/ieee80211_output.c | 24 ++++++++++-
3 files changed, 88 insertions(+), 28 deletions(-)
diff --git a/sys/net80211/ieee80211.h b/sys/net80211/ieee80211.h
index 028afec..29bfa3c 100644
--- a/sys/net80211/ieee80211.h
+++ b/sys/net80211/ieee80211.h
@@ -199,6 +199,13 @@ struct ieee80211_qosframe_addr4 {
#define IEEE80211_QOS_EOSP 0x10 /* EndOfService Period*/
#define IEEE80211_QOS_EOSP_S 4
#define IEEE80211_QOS_TID 0x0f
+/* qos[1] byte used for all frames sent by mesh STAs in a mesh BSS */
+#define IEEE80211_QOS_MC 0x10 /* Mesh control */
+/* Mesh power save level*/
+#define IEEE80211_QOS_MESH_PSL 0x20
+/* Mesh Receiver Service Period Initiated */
+#define IEEE80211_QOS_RSPI 0x40
+/* bits 11 to 15 reserved */
/* does frame have QoS sequence control data */
#define IEEE80211_QOS_HAS_SEQ(wh) \
diff --git a/sys/net80211/ieee80211_mesh.c b/sys/net80211/ieee80211_mesh.c
index b92f695..8b1a6db 100644
--- a/sys/net80211/ieee80211_mesh.c
+++ b/sys/net80211/ieee80211_mesh.c
@@ -1047,9 +1047,9 @@ mesh_input(struct ieee80211_node *ni, struct mbuf *m, int rssi, int nf)
struct ieee80211_frame *wh;
const struct ieee80211_meshcntl *mc;
int hdrspace, meshdrlen, need_tap;
- uint8_t dir, type, subtype, qos;
+ uint8_t dir, type, subtype;
uint32_t seq;
- uint8_t *addr;
+ uint8_t *addr, qos[2];
ieee80211_seq rxseq;
KASSERT(ni != NULL, ("null node"));
@@ -1146,8 +1146,64 @@ mesh_input(struct ieee80211_node *ni, struct mbuf *m, int rssi, int nf)
vap->iv_stats.is_rx_wrongdir++;
goto err;
}
- /* pull up enough to get to the mesh control */
+
+ /* All Mesh data frames are QoS subtype */
+ if (!HAS_SEQ(type)) {
+ IEEE80211_DISCARD(vap, IEEE80211_MSG_INPUT,
+ wh, "data", "incorrect subtype 0x%x", subtype);
+ vap->iv_stats.is_rx_badsubtype++;
+ goto err;
+ }
+
+ /*
+ * Next up, any fragmentation.
+ * XXX: we defrag before we even try to forward,
+ * Mesh Control field is not present in sub-sequent
+ * fragmented frames. This is in contrast to Draft 4.0.
+ */
hdrspace = ieee80211_hdrspace(ic, wh);
+ if (!IEEE80211_IS_MULTICAST(wh->i_addr1)) {
+ m = ieee80211_defrag(ni, m, hdrspace);
+ if (m == NULL) {
+ /* Fragment dropped or frame not complete yet */
+ goto out;
+ }
+ }
+ wh = mtod(m, struct ieee80211_frame *); /* NB: after defrag */
+
+ /*
+ * Now we have a complete Mesh Data frame.
+ */
+
+ /*
+ * Only fromDStoDS data frames use 4 address qos frames
+ * as specified in amendment. Otherwise addr4 is located
+ * in the Mesh Control field and a 3 address qos frame
+ * is used.
+ */
+ if (IEEE80211_IS_DSTODS(wh))
+ *(uint16_t *)qos = *(uint16_t *)
+ ((struct ieee80211_qosframe_addr4 *)wh)->i_qos;
+ else
+ *(uint16_t *)qos = *(uint16_t *)
+ ((struct ieee80211_qosframe *)wh)->i_qos;
+
+ /*
+ * NB: The mesh STA sets the Mesh Control Present
+ * subfield to 1 in the Mesh Data frame containing
+ * an unfragmented MSDU, an A-MSDU, or the first
+ * fragment of an MSDU.
+ * After defrag it should always be present.
+ */
+ if (!(qos[1] & IEEE80211_QOS_MC)) {
+ IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_MESH,
+ ni->ni_macaddr, NULL,
+ "%s", "Mesh control field not present");
+ vap->iv_stats.is_rx_elem_missing++; /* XXX: kinda */
+ goto err;
+ }
+
+ /* pull up enough to get to the mesh control */
if (m->m_len < hdrspace + sizeof(struct ieee80211_meshcntl) &&
(m = m_pullup(m, hdrspace +
sizeof(struct ieee80211_meshcntl))) == NULL) {
@@ -1195,27 +1251,6 @@ mesh_input(struct ieee80211_node *ni, struct mbuf *m, int rssi, int nf)
/* NB: fall thru to deliver mcast frames locally */
}
- /*
- * Save QoS bits for use below--before we strip the header.
- */
- if (subtype == IEEE80211_FC0_SUBTYPE_QOS) {
- qos = (dir == IEEE80211_FC1_DIR_DSTODS) ?
- ((struct ieee80211_qosframe_addr4 *)wh)->i_qos[0] :
- ((struct ieee80211_qosframe *)wh)->i_qos[0];
- } else
- qos = 0;
- /*
- * Next up, any fragmentation.
- */
- if (!IEEE80211_IS_MULTICAST(wh->i_addr1)) {
- m = ieee80211_defrag(ni, m, hdrspace);
- if (m == NULL) {
- /* Fragment dropped or frame not complete yet */
- goto out;
- }
- }
- wh = NULL; /* no longer valid, catch any uses */
-
if (ieee80211_radiotap_active_vap(vap))
ieee80211_radiotap_rx(vap, m);
need_tap = 0;
@@ -1236,7 +1271,7 @@ mesh_input(struct ieee80211_node *ni, struct mbuf *m, int rssi, int nf)
IEEE80211_NODE_STAT(ni, rx_decap);
goto err;
}
- if (qos & IEEE80211_QOS_AMSDU) {
+ if (qos[0] & IEEE80211_QOS_AMSDU) {
m = ieee80211_decap_amsdu(ni, m);
if (m == NULL)
return IEEE80211_FC0_TYPE_DATA;
diff --git a/sys/net80211/ieee80211_output.c b/sys/net80211/ieee80211_output.c
index f6177d9..1bd9452 100644
--- a/sys/net80211/ieee80211_output.c
+++ b/sys/net80211/ieee80211_output.c
@@ -1069,9 +1069,11 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni,
* ap's require all data frames to be QoS-encapsulated
* once negotiated in which case we'll need to make this
* configurable.
+ * NB: mesh data frames are QoS.
*/
- addqos = (ni->ni_flags & (IEEE80211_NODE_QOS|IEEE80211_NODE_HT)) &&
- (m->m_flags & M_EAPOL) == 0;
+ addqos = ((ni->ni_flags & (IEEE80211_NODE_QOS|IEEE80211_NODE_HT)) ||
+ (vap->iv_opmode == IEEE80211_M_MBSS)) &&
+ (m->m_flags & M_EAPOL) == 0;
if (addqos)
hdrsize = sizeof(struct ieee80211_qosframe);
else
@@ -1277,7 +1279,12 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni,
qos[0] = tid & IEEE80211_QOS_TID;
if (ic->ic_wme.wme_wmeChanParams.cap_wmeParams[ac].wmep_noackPolicy)
qos[0] |= IEEE80211_QOS_ACKPOLICY_NOACK;
- qos[1] = 0;
+#ifdef IEEE80211_SUPPORT_MESH
+ if (vap->iv_opmode == IEEE80211_M_MBSS) {
+ qos[1] |= IEEE80211_QOS_MC;
+ } else
+#endif
+ qos[1] = 0;
wh->i_fc[0] |= IEEE80211_FC0_SUBTYPE_QOS;
if ((m->m_flags & M_AMPDU_MPDU) == 0) {
@@ -1402,9 +1409,20 @@ ieee80211_fragment(struct ieee80211vap *vap, struct mbuf *m0,
* we mark the first fragment with the MORE_FRAG bit
* it automatically is propagated to each fragment; we
* need only clear it on the last fragment (done below).
+ * NB: frag 1+ dont have Mesh Control field present.
*/
whf = mtod(m, struct ieee80211_frame *);
memcpy(whf, wh, hdrsize);
+#ifdef IEEE80211_SUPPORT_MESH
+ if (vap->iv_opmode == IEEE80211_M_MBSS) {
+ if (IEEE80211_IS_DSTODS(wh))
+ ((struct ieee80211_qosframe_addr4 *)
+ whf)->i_qos[1] &= ~IEEE80211_QOS_MC;
+ else
+ ((struct ieee80211_qosframe *)
+ whf)->i_qos[1] &= ~IEEE80211_QOS_MC;
+ }
+#endif
*(uint16_t *)&whf->i_seq[0] |= htole16(
(fragno & IEEE80211_SEQ_FRAG_MASK) <<
IEEE80211_SEQ_FRAG_SHIFT);
--
1.7.4.1
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2BsBSoL8WPB5pGMBrQDM46kn=BRkqW1HrzeVuoBQ6S=NfjGz=Q>