Date: Wed, 15 Feb 2012 10:26:51 +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%2BsBSoJOjgxBvQa_kN0oDyS4AUeu7kKGUB37Yz3T=JfzMqGG8Q@mail.gmail.com> In-Reply-To: <CA%2BsBSo%2BS-quy2p1O8k9TDZO1Csgg19pgTMv7y0VeXWSgq3=mzQ@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>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
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
[-- Attachment #2 --]
From 7ac4a1ff0934e9271372a669d8e832e794d8107e 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..d71b08e 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 group addressed Mesh data frames are 3 address
+ * qos frames among the different Mesh Data frames as
+ * specified in amendment.
+ */
+ if (IEEE80211_IS_MULTICAST(wh->i_addr1) &&
+ dir == IEEE80211_FC1_DIR_FROMDS)
+ *(uint16_t *)qos = *(uint16_t *)
+ ((struct ieee80211_qosframe *)wh)->i_qos;
+ else
+ *(uint16_t *)qos = *(uint16_t *)
+ ((struct ieee80211_qosframe_addr4 *)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
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2BsBSoJOjgxBvQa_kN0oDyS4AUeu7kKGUB37Yz3T=JfzMqGG8Q>