Date: Wed, 15 Feb 2012 09:26:11 +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%2BsBSo%2BS-quy2p1O8k9TDZO1Csgg19pgTMv7y0VeXWSgq3=mzQ@mail.gmail.com> In-Reply-To: <3F2E258F-1F82-48D7-AAAD-546BCB03EDE2@freebsd.org> References: <CA%2BsBSoJGzabfyt8EXoT-g=GTJD0upX5jbRqfLWKQKzXu2FivaA@mail.gmail.com> <3F2E258F-1F82-48D7-AAAD-546BCB03EDE2@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
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.
>
> 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
[-- Attachment #2 --]
From c0feb17825a860b117b5e776bae9ddab3fedf5b9 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;
---
sys/net80211/ieee80211.h | 7 +++
sys/net80211/ieee80211_mesh.c | 85 +++++++++++++++++++++++++++-----------
sys/net80211/ieee80211_output.c | 13 +++++-
3 files changed, 77 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..e47d91a 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 Draf 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..8b2b501 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) {
--
1.7.4.1
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CA%2BsBSo%2BS-quy2p1O8k9TDZO1Csgg19pgTMv7y0VeXWSgq3=mzQ>