Howdy again, I have been messing about with the Wifi- and Radiotap-related elements and have come across a few bugs (well, in fairness, #2 is more of a missing feature). Also note that my packet-capture source is the FreeBSD ath driver whereas (I assume) most wireless Click users use MadWifi on Linux so some of these bugs might be hard or impossible to recreate on Linux.
1) WifiDecap does not properly handle cases where padding has been inserted after the 802.11 header (so as to word-align the next header). 2) After stripping the 802.11 header, WifiDecap always creates and pushes on an Ethernet header; a configuration parameter should exist to prevent this from occurring if so desired. 3) RadiotapDecap does not correctly read the "frame includes FCS" flag from radiotap headers, meaning packets are not stripped of their FCS (at the end of the packet) as intended. 4) RadiotapDecap does not re-set the mac-header pointer (p->set_mac_header) after stripping the radiotap header. To recreate bug #1, I have uploaded a pcap file (consisting of just 1 packet) here: http://www.eecs.harvard.edu/~ianrose/stuff/wifidecaptest.pcap The packet's 802.11 header is padded to bring its length up to a multiple of 4 bytes. When viewed with tcpdump (or wireshark, etc.) this packet is properly parsed and displayed as a UDP DNS request. However an error is output (bad IP version) when run through the following click configuration because it does not handle the padding correctly: > define($FILENAME wifidecaptest.pcap) > > FromDump($FILENAME, STOP true) > -> RadiotapDecap > -> WifiDecap > -> CheckIPHeader(14, VERBOSE true) > -> IPPrint > -> Discard; For "bug" #2, I have added an optional 'PUSH_ETH' configuration parameter to the WifiDecap element; its default value is true to maintain backwards compatibility. I identified bugs #3 and #4 by code inspection alone (as opposed to actual testing) and I do not have an examples on hand that demonstrate either the bugs or the fixes. I have included candidate patches that address all 4 of these issues below. Cheers, Ian ---------------------------------------------------- > --- click-1.7.0rc1/elements/wifi/radiotapdecap.cc 2009-08-06 > 17:32:06.518608000 -0400 > @@ -122,8 +122,19 @@ > struct ieee80211_radiotap_header *th = (struct > ieee80211_radiotap_header *) p->data(); > struct click_wifi_extra *ceh = WIFI_EXTRA_ANNO(p); > if (rt_check_header(th, p->length())) { > + memset((void*)ceh, 0, sizeof(struct click_wifi_extra)); > ceh->magic = WIFI_EXTRA_MAGIC; > > + if (rt_el_present(th, IEEE80211_RADIOTAP_FLAGS)) { > + u_int8_t flags = *((u_int8_t *) rt_el_offset(th, > IEEE80211_RADIOTAP_FLAGS)); > + if (flags & IEEE80211_RADIOTAP_F_DATAPAD) { > + ceh->pad = 1; > + } > + if (flags & IEEE80211_RADIOTAP_F_FCS) { > + p->take(4); > + } > + } > + > if (rt_el_present(th, IEEE80211_RADIOTAP_RATE)) { > ceh->rate = *((u_int8_t *) rt_el_offset(th, > IEEE80211_RADIOTAP_RATE)); > } > @@ -151,16 +162,13 @@ > ceh->flags |= WIFI_EXTRA_TX; > if (flags & IEEE80211_RADIOTAP_F_TX_FAIL) > ceh->flags |= WIFI_EXTRA_TX_FAIL; > - > - if (flags & IEEE80211_RADIOTAP_F_FCS) { > - p->take(4); > - } > } > > if (rt_el_present(th, IEEE80211_RADIOTAP_DATA_RETRIES)) > ceh->retries = *((u_int8_t *) rt_el_offset(th, > IEEE80211_RADIOTAP_DATA_RETRIES)); > > p->pull(th->it_len); > + p->set_mac_header(p->data()); // reset mac-header pointer > } > > return p; > +++ click-1.7.0rc1/elements/wifi/wifidecap.cc 2009-08-06 17:08:46.592049000 > -0400 > @@ -21,6 +21,7 @@ > #include <click/confparse.hh> > #include <click/error.hh> > #include <click/glue.hh> > +#include <click/packet_anno.hh> > #include <clicknet/wifi.h> > #include <clicknet/llc.h> > CLICK_DECLS > @@ -39,9 +40,11 @@ > > _debug = false; > _strict = false; > + _push_eth = true; > if (cp_va_kparse(conf, this, errh, > "DEBUG", 0, cpBool, &_debug, > "STRICT", 0, cpBool, &_strict, > + "PUSH_ETH", 0, cpBool, &_push_eth, > cpEnd) < 0) > return -1; > return 0; > @@ -64,6 +67,10 @@ > if (WIFI_QOS_HAS_SEQ(w)) > wifi_header_size += sizeof(uint16_t); > > + struct click_wifi_extra *ceh = WIFI_EXTRA_ANNO(p); > + if ((ceh->magic == WIFI_EXTRA_MAGIC) && ceh->pad && (wifi_header_size & 3)) > + wifi_header_size += 4 - (wifi_header_size & 3); > + > if (p->length() < wifi_header_size + sizeof(struct click_llc)) { > p->kill(); > return 0; > @@ -125,23 +132,26 @@ > } > > p_out->pull(wifi_header_size + sizeof(struct click_llc)); > - p_out = p_out->push_mac_header(14); > - if (!p_out) { > - return 0; > - } > > - memcpy(p_out->data(), dst.data(), 6); > - memcpy(p_out->data() + 6, src.data(), 6); > - memcpy(p_out->data() + 12, ðer_type, 2); > - > - if (_debug) { > - click_chatter("%{element}: dir %d src %s dst %s bssid %s eth > 0x%02x\n", > - this, > - dir, > - src.unparse().c_str(), > - dst.unparse().c_str(), > - bssid.unparse().c_str(), > - ether_type); > + if (_push_eth) { > + p_out = p_out->push_mac_header(14); > + if (!p_out) { > + return 0; > + } > + > + memcpy(p_out->data(), dst.data(), 6); > + memcpy(p_out->data() + 6, src.data(), 6); > + memcpy(p_out->data() + 12, ðer_type, 2); > + > + if (_debug) { > + click_chatter("%{element}: dir %d src %s dst %s bssid %s eth > 0x%02x\n", > + this, > + dir, > + src.unparse().c_str(), > + dst.unparse().c_str(), > + bssid.unparse().c_str(), > + ether_type); > + } > } > > return p_out; > --- click-1.7.0rc1/elements/wifi/wifidecap.hh 2009-08-06 17:47:45.706426000 > -0400 > @@ -53,6 +53,7 @@ > > bool _debug; > bool _strict; > + bool _push_eth; > private: > > }; _______________________________________________ click mailing list click@amsterdam.lcs.mit.edu https://amsterdam.lcs.mit.edu/mailman/listinfo/click
