Expand some comments.

Fix some indentation while we're at it.

Change-Id: Ic25bebadd8c2c3941e6f965b48e22a6a1aac6168
Reviewed-on: https://code.wireshark.org/review/10769
Reviewed-by: Guy Harris <guy@alum.mit.edu>

2 files changed, 41 insertions, 19 deletions

diff --git a/epan/dissectors/packet-ieee80211.c b/epan/dissectors/packet-ieee80211.c
index 6820f5c11f..f7a566453a 100644
--- a/epan/dissectors/packet-ieee80211.c
+++ b/epan/dissectors/packet-ieee80211.c
@@ -5361,6 +5361,12 @@ extra_one_mul_two_base_custom(gchar *result, guint32 value)
  * frame is transmitted by a mesh STA and the Mesh Control Present subfield of
  * the QoS Control field is 1)...
  *
+ * 8.2.4.5.1 "QoS Control field structure", table 8-4, in 802.11-2012,
+ * seems to indicate that the bit that means "Mesh Control Present" in
+ * frames sent by mesh STAs in a mesh BSS is part of the TXOP Limit field,
+ * the AP PS Buffer State field, the TXOP Duration Requested field, or the
+ * Queue Size field in some data frames in non-mesh BSSes.
+ *
  * We need a statefull sniffer for that.  For now, use heuristics.
  *
  * Notably, only mesh data frames contain the Mesh Control field in the header.
@@ -5637,11 +5643,11 @@ capture_ieee80211_common (const guchar * pd, int offset, int len,
     case DATA_QOS_DATA_CF_POLL:
     case DATA_QOS_DATA_CF_ACK_POLL:
     {
-      /* Data frames that actually contain *data* */
+      /* These are data frames that actually contain *data*. */
       hdr_length = (FCF_ADDR_SELECTOR(fcf) == DATA_ADDR_T4) ? DATA_LONG_HDR_LEN : DATA_SHORT_HDR_LEN;
 
       if (DATA_FRAME_IS_QOS(COMPOSE_FRAME_TYPE(fcf))) {
-        /* QoS frame */
+        /* QoS frame, so the header includes a QoS field */
         guint16 qosoff;  /* Offset of the 2-byte QoS field */
         guint8 mesh_flags;
 
@@ -5670,7 +5676,8 @@ capture_ieee80211_common (const guchar * pd, int offset, int len,
 
         if (datapad) {
           /*
-           * Add in Atheros padding between the 802.11 header and body.
+           * Include the padding between the 802.11 header and the body,
+           * as "helpfully" provided by some Atheros adapters.
            *
            * XXX - would the mesh header be part of the header or the body
            * from the point of view of the Atheros adapters that insert
@@ -5719,12 +5726,12 @@ capture_ieee80211_common (const guchar * pd, int offset, int len,
         /* XXX - this requires us to parse the header to find the source
            and destination addresses. */
         if (BYTES_ARE_IN_FRAME(offset+hdr_length, len, 12)) {
-            /* We have two MAC addresses after the header. */
-            if ((memcmp(&pd[offset+hdr_length+6], pinfo->dl_src.data, 6) == 0) ||
-                (memcmp(&pd[offset+hdr_length+6], pinfo->dl_dst.data, 6) == 0)) {
-              capture_eth (pd, offset + hdr_length, len, ld);
-              return;
-            }
+          /* We have two MAC addresses after the header. */
+          if ((memcmp(&pd[offset+hdr_length+6], pinfo->dl_src.data, 6) == 0) ||
+              (memcmp(&pd[offset+hdr_length+6], pinfo->dl_dst.data, 6) == 0)) {
+            capture_eth (pd, offset + hdr_length, len, ld);
+            return;
+          }
         }
 #endif
         if ((pd[offset+hdr_length] == 0xff) && (pd[offset+hdr_length+1] == 0xff))
@@ -12816,11 +12823,18 @@ dissect_ht_info_ie_1_0(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, int
  * and that everything except for "AC Constraint" and "RDG/More Cowbell^W
  * PPDU" is different for the VHT version.
  *
- * I read this as meaning that management frames and QoS data frames that
- * aren't HT or VHT frames should never have the Order field set, and
- * those that *are* HT or VHT frames should have it set only if there's
- * an HT Control field, so there's no need to check the radio information
- * to see whether the frame is an HT or VHT frame or not.
+ * I read the second clause of 8.2.4.1.10 "Order field", as modified by
+ * 802.11ac, as meaning that, for QoS data and management frames, the
+ * Order field will *only* be set to 1 for HT or VHT frames, and therefore
+ * that we do *not* have to determine, from radio metadata, whether the
+ * frame was transmitted as an HT or VHT frame.
+ *
+ * (See bug 11351, in which a frame with an HT Control field, with a
+ * radiotap header, lacks the MCS or VHT fields in the radiotap header,
+ * so Wireshark has no clue that it's an HT or VHT field, and misdissected
+ * the packet.  Omnipeek, which also appeared to have no clue that it was
+ * an HT or VHT field - it called it an 802.11b frame - *did* dissect the
+ * HT Control field.)
  */
 
 static void
@@ -16498,7 +16512,11 @@ dissect_ieee80211_common (tvbuff_t *tvb, packet_info *pinfo,
   case MGT_FRAME:
     hdr_len = MGT_FRAME_HDR_LEN;
     if (HAS_HT_CONTROL(FCF_FLAGS(fcf))) {
-      /* Frame has a 4-byte HT Control field */
+      /*
+       * Management frames with the Order bit set have an HT Control field;
+       * see 8.2.4.1.10 "Order field".  If they're not HT frames, they should
+       * never have the Order bit set.
+       */
       hdr_len += 4;
       htc_len = 4;
     }
@@ -16577,7 +16595,11 @@ dissect_ieee80211_common (tvbuff_t *tvb, packet_info *pinfo,
       hdr_len += 2; /* Include the QoS field in the header length */
 
       if (HAS_HT_CONTROL(FCF_FLAGS(fcf))) {
-        /* Frame has a 4-byte HT Control field */
+        /*
+         * QoS data frames with the Order bit set have an HT Control field;
+         * see 8.2.4.1.10 "Order field".  If they're not HT frames, they
+         * should never have the Order bit set.
+         */
         hdr_len += 4;
         htc_len = 4;
       }
diff --git a/epan/dissectors/packet-ieee80211.h b/epan/dissectors/packet-ieee80211.h
index 9d47e8947a..ea24b173be 100644
--- a/epan/dissectors/packet-ieee80211.h
+++ b/epan/dissectors/packet-ieee80211.h
@@ -132,7 +132,7 @@ gboolean is_broadcast_bssid(const address *bssid);
 #define FLAG_POWER_MGT        0x10
 #define FLAG_MORE_DATA        0x20
 #define FLAG_PROTECTED        0x40
-#define FLAG_ORDER            0x80
+#define FLAG_ORDER            0x80    /* overloaded for "has HT control" */
 
 /*
  * Test bits in the flags field.
@@ -148,8 +148,8 @@ gboolean is_broadcast_bssid(const address *bssid);
 #define POWER_MGT_STATUS(x)    ((x) & FLAG_POWER_MGT)
 #define HAS_MORE_DATA(x)       ((x) & FLAG_MORE_DATA)
 #define IS_PROTECTED(x)        ((x) & FLAG_PROTECTED)
-#define IS_STRICTLY_ORDERED(x) ((x) & FLAG_ORDER) /* non-QoS data frames */
-#define HAS_HT_CONTROL(x)      ((x) & FLAG_ORDER) /* management and QoS data frames */
+#define IS_STRICTLY_ORDERED(x) ((x) & FLAG_ORDER)      /* for non-QoS data frames */
+#define HAS_HT_CONTROL(x)      ((x) & FLAG_ORDER)      /* for management and QoS data frames */
 
 /*
  * Extract subfields from the flags field.