On Wed, Nov 05, 2014 at 03:29:13AM +0200, Kalle Valo wrote: > Rajkumar Manoharan <rmanohar@xxxxxxxxxxxxxxxx> writes: > > > On Tue, Nov 04, 2014 at 01:34:37AM +0200, Kalle Valo wrote: > >> Rajkumar Manoharan <rmanohar@xxxxxxxxxxxxxxxx> writes: > >> > >> > -DECLARE_EVENT_CLASS(ath10k_data_event, > >> > +#define ATH10K_FRM_HDR_LEN \ > >> > + ieee80211_hdrlen(((struct ieee80211_hdr *)data)->frame_control) > >> > >> This macro does not look good. I would recommend to follow what Johannes > >> suggested: > >> > > > > v2 does the same. tracing functions just take ar, skb->data and skb->len. > > header check is handled inside tracing funtions. > > > > I do not understand your concerns. :( > > Sorry, I was confusing. I meant that wouldn't it be better to pass the > skb pointer instead of skb-data and skb->len? I understood that was what > Johannes suggested. > > But ATH10K_FRM_HDR_LEN is the problem. To simplify this, what if you > just always send the first 30 bytes (or whatever would be the max header > length)? Also should you check that skb->len is at least the length > defined by ieee80211_hdrlen()? > The goal of header & payload split is to reduce the amount of data being collected. Sending maximum header size always will unnecessarily increase the trace.dat size as hdr_size for mgmt and beacon always be 24. This will also increase tracing traffic when it is being collected in remote machine. since tracepoints are being called only after the preprocessing, skb->len never be lesser than 802.11 header size. The macro is defined to ease readability and reduce # of line changes. I originally thought of naming the macro as IEEE80211_HDR_LEN. But such definition does not look correct inside driver. -Rajkumar -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
