[Click] [RFC] Update radiotap parser

[欧阳鑫]

在 2011年4月15日 下午4:55，Roberto Riggio <roberto.riggio at create-net.org>写道：

> Il 15/04/2011 09:44, 欧阳鑫 ha scritto:
> >
> > Hi, I have patched my click source files according to above. I want to
> > implement click on 802.11n mode, but I find the above patch
> >
> > does not deal with MCS arguments such
> >
> IEEE80211_RADIOTAP_MCS_HAVE_MCS，IEEE80211_RADIOTAP_MCS_BW_40，IEEE80211_RADIOTAP_MCS_SGI
> >
> >
> well they are anyway ignored by an unpatched kernel


     I have updated wireless driver ath9k to compat-wireless-2011-03-16 and
it has supported MCS arguments such as above.


> >
> > and so on. So I modify radiotapencap.cc, radiotapdecap.cc,
> > settxrate.cc, settxrate.hh, ettstat.cc and athdescdecap.cc. The patches
> >
> > are as follow:
> > diff -urN click-1.8.0_bg_patch/elements/roofnet/sr/ettstat.cc
> > click-1.8.0_patch_11n/elements/roofnet/sr/ettstat.cc
> > --- click-1.8.0_bg_patch/elements/roofnet/sr/ettstat.cc 2011-04-01
> > 15:09:40.000000000 +0800
> > +++ click-1.8.0_patch_11n/elements/roofnet/sr/ettstat.cc 2011-04-14
> > 15:59:10.252273517 +0800
> > @@ -420,7 +420,15 @@
> > struct click_wifi_extra *ceh = (struct click_wifi_extra *)
> p->user_anno();
> > ceh->magic = WIFI_EXTRA_MAGIC;
> > ceh->rate = rate;
> > -ceh->max_tries = 7;
> > +
> > + ceh->flags |= WIFI_EXTRA_RX_FLAG_HT;
> > + ceh->flags |= WIFI_EXTRA_RX_FLAG_40MHZ;
> > + ceh->flags |= WIFI_EXTRA_RX_FLAG_SHORT_GI;
> > + ceh->rateidx = _ads_rs_index;
> > + ceh->max_tries = 1;
> > +
> > checked_output_push(0, p);
> > }
>
> this should probably be part of the element configuration rather that
> hard coded.
>

I do not understand completely. I have tried to get rid of this part in
ettstat.cc and only add MCS arguments in settxrate.cc, but I find that the
rate of patckets is only 0Mb/s, I do not know why.



>
> > diff -urN click-1.8.0_bg_patch/elements/wifi/athdescdecap.cc
> > click-1.8.0_patch_11n/elements/wifi/athdescdecap.cc
> > --- click-1.8.0_bg_patch/elements/wifi/athdescdecap.cc 2011-04-01
> > 15:09:40.000000000 +0800
> > +++ click-1.8.0_patch_11n/elements/wifi/athdescdecap.cc 2011-04-14
> > 10:36:26.468273490 +0800
> > @@ -30,7 +30,7 @@
> >
> > AthdescDecap::AthdescDecap()
> > {
> > - static_assert(WIFI_EXTRA_ANNO_SIZE >= sizeof(click_wifi_extra));
> > + //static_assert(WIFI_EXTRA_ANNO_SIZE >= sizeof(click_wifi_extra));
> > }
> >
>
> is there a particular reason to remove this? anyway the atheros
> descriptor is supported only by the madwifi
> driver which does not support 11n cards.
>
>

Because I add one member  uint8_t rateidx of click_wifi_extra, so the size
of click_wifi_extra is 25, but WIFI_EXTRA_ANNO_SIZE is 24, the static_assert
is not pass , so I remove this. I do not know if I remove this if it will
have some effect on other functions.



>  > AthdescDecap::~AthdescDecap()
> > diff -urN click-1.8.0_bg_patch/elements/wifi/radiotapdecap.cc
> > click-1.8.0_patch_11n/elements/wifi/radiotapdecap.cc
> > --- click-1.8.0_bg_patch/elements/wifi/radiotapdecap.cc 2011-04-12
> > 10:33:15.000000000 +0800
> > +++ click-1.8.0_patch_11n/elements/wifi/radiotapdecap.cc 2011-04-14
> > 16:35:28.604273504 +0800
> > @@ -26,6 +26,7 @@
> > #include <clicknet/radiotap.h>
> > #include <clicknet/llc.h>
> >
> > @@ -105,6 +106,26 @@
> >
> > CLICK_DECLS
> >
> > +#define IEEE80211_MCS_RATE_COUNT 16
> > +int IEEE80211_RADIOTAP_MCS_RATE[IEEE80211_MCS_RATE_COUNT][2] = {
> > + {0, 150},
> > + {1, 300},
> > + {2, 450},
> > + {3, 600},
> > + {4, 900},
> > + {5, 1200},
> > + {6, 1350},
> > + {7, 1500},
> > + {8, 300},
> > + {9, 600},
> > + {10, 900},
> > + {11, 1200},
> > + {12, 1800},
> > + {13, 2400},
> > + {14, 2700},
> > + {15, 3000}
> > +};
> > +
> I do not get this, the MCS index already embed the rate information. As
> a matter of fact
> the corresponding radiotap field shall containt the index NOT the
> corresponding PHY
> data rate:
>
> http://www.radiotap.org/defined-fields/MCS
>


I add the data rate only in order to intuitively display data rate rather
according to index converting to data rate.



> > RadiotapDecap::RadiotapDecap()
> > {
> > }
> > @@ -221,9 +242,36 @@
> > if (flags & IEEE80211_RADIOTAP_F_TX_FAIL)
> > ceh->flags |= WIFI_EXTRA_TX_FAIL;
> > break;
> > + case IEEE80211_RADIOTAP_MCS:
> > + uint8_t known, flags;
> > + known = *(iter.this_arg);
> > + flags = *(iter.this_arg + 1);
> > +
> > + // set Band width
> > + if (known & IEEE80211_RADIOTAP_MCS_HAVE_BW) {
> > + if (flags & IEEE80211_RADIOTAP_MCS_BW_40)
> > + ceh->flags |= WIFI_EXTRA_RX_FLAG_HT;
> > + }
> where is this WIFI_EXTRA_RX_FLAG_HT defined?
>
> > + if (known & IEEE80211_RADIOTAP_MCS_HAVE_MCS)
> > + ceh->rateidx = *(iter.this_arg + 2);
> > + else
> > + // may not reasonable......
> > + ceh->rateidx = 0;
> > +
> > + // set Guard interval
> > + if (known & IEEE80211_RADIOTAP_MCS_HAVE_GI) {
> > + if (flags & IEEE80211_RADIOTAP_MCS_SGI)
> > + ceh->flags |= WIFI_EXTRA_RX_FLAG_SHORT_GI;
> and this
> > + }
> > + // set FMT
> > + if (known & IEEE80211_RADIOTAP_MCS_HAVE_FMT) {
> > + if (flags & IEEE80211_RADIOTAP_MCS_FMT_GF)
> > + ceh->flags |= WIFI_EXTRA_RX_FLAG_LDPC;
> > + }
> > +
> same here
> >
> > + ceh->rate = IEEE80211_RADIOTAP_MCS_RATE[ceh->rateidx][1] / 5;
> > + break;
> > +
> ok, i get you want to set rate to the actual phy rate. why an actual use
> for it?
> >
> > - //if (flags & IEEE80211_RADIOTAP_F_FCS) {
> > - // p->take(4);
> so what happens if the frame actually include the FCS ?
>  > }
> > }
> >
> > diff -urN click-1.8.0_bg_patch/elements/wifi/radiotapencap.cc
> > click-1.8.0_patch_11n/elements/wifi/radiotapencap.cc
> > --- click-1.8.0_bg_patch/elements/wifi/radiotapencap.cc 2011-04-12
> > 10:51:03.000000000 +0800
> > +++ click-1.8.0_patch_11n/elements/wifi/radiotapencap.cc 2011-04-14
> > 16:21:15.760273037 +0800
> > @@ -46,6 +46,14 @@
> > (1 << IEEE80211_RADIOTAP_EXT) | \
> > 0)
> >
> > +//566fix: mcs
> > +#define CLICK_RADIOTAP_MCS_PRESENT ( \
> > + (1 << IEEE80211_RADIOTAP_MCS) | \
> > + 0)
> > +
> > +#define IEEE80211_MCS_RATE_COUNT 16
> > +extern int IEEE80211_RADIOTAP_MCS_RATE[IEEE80211_MCS_RATE_COUNT][2];
> > +
> >
> > struct click_radiotap_header {
> > struct ieee80211_radiotap_header wt_ihdr;
> > @@ -66,6 +74,12 @@
> > u_int8_t wt_rts_retries;
> > u_int8_t wt_data_retries;
> >
> > + u_int8_t wt_known;
> > + u_int8_t wt_flags;
> > + u_int8_t wt_mcs;
> > +
> > u_int8_t wt_rate1;
> > u_int8_t wt_data_retries1;
> >
> > } __attribute__((__packed__));
> >
> >
> > @@ -206,6 +219,29 @@
> > }
> > }
> >
> > +
> > + if (ceh->flags & WIFI_EXTRA_RX_FLAG_HT) {
> > + crh->wt_ihdr.it_present |= CLICK_RADIOTAP_MCS_PRESENT;
> > + crh->wt_known = IEEE80211_RADIOTAP_MCS_HAVE_BW | \
> > + IEEE80211_RADIOTAP_MCS_HAVE_MCS | \
> > + IEEE80211_RADIOTAP_MCS_HAVE_GI | \
> > + IEEE80211_RADIOTAP_MCS_HAVE_FMT | \
> > + IEEE80211_RADIOTAP_MCS_HAVE_FEC;
> > + crh->wt_mcs = ceh->rateidx;
> > + if (ceh->flags & WIFI_EXTRA_RX_FLAG_40MHZ)
> > + crh->wt_flags |= IEEE80211_RADIOTAP_MCS_BW_40;
> > + if (ceh->flags & WIFI_EXTRA_RX_FLAG_SHORT_GI)
> > + crh->wt_flags |= IEEE80211_RADIOTAP_MCS_SGI;
> > + if (ceh->flags & WIFI_EXTRA_RX_FLAG_GF)
> > + crh->wt_flags |= IEEE80211_RADIOTAP_MCS_FMT_GF;
> > + crh->wt_rate = IEEE80211_RADIOTAP_MCS_RATE[ceh->rateidx][1] / 5;
> > +
> > + }
> > +
> >
> Here you add the three new MCS fields (known, flags, mcs), but the
> present bitmask is updated
> only if the WIFI_EXTRA_RX_FLAG_HT is set. As a result if
> WIFI_EXTRA_RX_FLAG_HT is not set
> the three fields are still there resulting in an invalid radiotap header.
>
> Finally, I would try to stabilize the radiotap parser and the
> decap/encap elements before
> adapting other elements such as ettstat, settxrate, etc.
>
 R.

Click can run with 802.11b/g mode on high rates, such as 48Mb/s, 54Mb/s(the
highest rate of 802.11g mode)and so on. But when I run click with 802.11n
mode, and I add some sources dealing with MCS arguments as above, but the
data rate is always 0Mbs/s, and  the rateidx is always 0. I do not know if
there is some important arguments but I do not deal with, so the rate is
always 0Mb/s and rateidx is always 0. If it is, I should modify which
sources of click?

Thank you very much!

Ouyang.