Those functions that still use ieee80211_get_hdrlen are moved over
to use the little endian frame control.
Signed-off-by: Harvey Harrison <[email protected]>
---
net/mac80211/rx.c | 68 ++++++++++++++++++----------------------------------
net/mac80211/tx.c | 2 +-
2 files changed, 25 insertions(+), 45 deletions(-)
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index fab443d..e90a935 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -811,7 +811,7 @@ ieee80211_reassemble_add(struct ieee80211_sub_if_data *sdata,
static inline struct ieee80211_fragment_entry *
ieee80211_reassemble_find(struct ieee80211_sub_if_data *sdata,
- u16 fc, unsigned int frag, unsigned int seq,
+ unsigned int frag, unsigned int seq,
int rx_queue, struct ieee80211_hdr *hdr)
{
struct ieee80211_fragment_entry *entry;
@@ -820,7 +820,6 @@ ieee80211_reassemble_find(struct ieee80211_sub_if_data *sdata,
idx = sdata->fragment_next;
for (i = 0; i < IEEE80211_FRAGMENT_MAX; i++) {
struct ieee80211_hdr *f_hdr;
- u16 f_fc;
idx--;
if (idx < 0)
@@ -832,10 +831,13 @@ ieee80211_reassemble_find(struct ieee80211_sub_if_data *sdata,
entry->last_frag + 1 != frag)
continue;
- f_hdr = (struct ieee80211_hdr *) entry->skb_list.next->data;
- f_fc = le16_to_cpu(f_hdr->frame_control);
+ f_hdr = (struct ieee80211_hdr *)entry->skb_list.next->data;
- if ((fc & IEEE80211_FCTL_FTYPE) != (f_fc & IEEE80211_FCTL_FTYPE) ||
+ /*
+ * Check ftype and addresses are equal, else check next fragment
+ */
+ if (((hdr->frame_control ^ f_hdr->frame_control) &
+ cpu_to_le16(IEEE80211_FCTL_FTYPE)) ||
compare_ether_addr(hdr->addr1, f_hdr->addr1) != 0 ||
compare_ether_addr(hdr->addr2, f_hdr->addr2) != 0)
continue;
@@ -860,11 +862,11 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
struct sk_buff *skb;
DECLARE_MAC_BUF(mac);
- hdr = (struct ieee80211_hdr *) rx->skb->data;
+ hdr = (struct ieee80211_hdr *)rx->skb->data;
sc = le16_to_cpu(hdr->seq_ctrl);
frag = sc & IEEE80211_SCTL_FRAG;
- if (likely((!(rx->fc & IEEE80211_FCTL_MOREFRAGS) && frag == 0) ||
+ if (likely((!ieee80211_has_morefrags(hdr->frame_control) && frag == 0) ||
(rx->skb)->len < 24 ||
is_multicast_ether_addr(hdr->addr1))) {
/* not fragmented */
@@ -879,7 +881,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
entry = ieee80211_reassemble_add(rx->sdata, frag, seq,
rx->queue, &(rx->skb));
if (rx->key && rx->key->conf.alg == ALG_CCMP &&
- (rx->fc & IEEE80211_FCTL_PROTECTED)) {
+ ieee80211_has_protected(hdr->frame_control)) {
/* Store CCMP PN so that we can verify that the next
* fragment has a sequential PN value. */
entry->ccmp = 1;
@@ -893,8 +895,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
/* This is a fragment for a frame that should already be pending in
* fragment cache. Add this fragment to the end of the pending entry.
*/
- entry = ieee80211_reassemble_find(rx->sdata, rx->fc, frag, seq,
- rx->queue, hdr);
+ entry = ieee80211_reassemble_find(rx->sdata, frag, seq, rx->queue, hdr);
if (!entry) {
I802_DEBUG_INC(rx->local->rx_handlers_drop_defrag);
return RX_DROP_MONITOR;
@@ -919,7 +920,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
memcpy(entry->last_pn, pn, CCMP_PN_LEN);
}
- skb_pull(rx->skb, ieee80211_get_hdrlen(rx->fc));
+ skb_pull(rx->skb, ieee80211_hdrlen(hdr->frame_control));
__skb_queue_tail(&entry->skb_list, rx->skb);
entry->last_frag = frag;
entry->extra_len += rx->skb->len;
@@ -1086,7 +1087,7 @@ ieee80211_data_to_8023(struct ieee80211_rx_data *rx)
{
struct net_device *dev = rx->dev;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) rx->skb->data;
- u16 fc, hdrlen, ethertype;
+ u16 hdrlen, ethertype;
u8 *payload;
u8 dst[ETH_ALEN];
u8 src[ETH_ALEN] __aligned(2);
@@ -1097,12 +1098,10 @@ ieee80211_data_to_8023(struct ieee80211_rx_data *rx)
DECLARE_MAC_BUF(mac3);
DECLARE_MAC_BUF(mac4);
- fc = rx->fc;
-
- if (unlikely(!WLAN_FC_DATA_PRESENT(fc)))
+ if (unlikely(!ieee80211_is_data_present(hdr->frame_control)))
return -1;
- hdrlen = ieee80211_get_hdrlen(fc);
+ hdrlen = ieee80211_hdrlen(hdr->frame_control);
if (ieee80211_vif_is_mesh(&sdata->vif)) {
int meshhdrlen = ieee80211_get_mesh_hdrlen(
@@ -1128,44 +1127,25 @@ ieee80211_data_to_8023(struct ieee80211_rx_data *rx)
* 1 0 BSSID SA DA n/a
* 1 1 RA TA DA SA
*/
+ memcpy(dst, ieee80211_get_DA(hdr), ETH_ALEN);
+ memcpy(src, ieee80211_get_SA(hdr), ETH_ALEN);
- switch (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
- case IEEE80211_FCTL_TODS:
- /* BSSID SA DA */
- memcpy(dst, hdr->addr3, ETH_ALEN);
- memcpy(src, hdr->addr2, ETH_ALEN);
-
+ if (ieee80211_has_a4(hdr->frame_control)) {
+ if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_WDS &&
+ sdata->vif.type != IEEE80211_IF_TYPE_MESH_POINT))
+ return -1;
+ } else if (ieee80211_has_tods(hdr->frame_control)) {
if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_AP &&
sdata->vif.type != IEEE80211_IF_TYPE_VLAN))
return -1;
- break;
- case (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS):
- /* RA TA DA SA */
- memcpy(dst, hdr->addr3, ETH_ALEN);
- memcpy(src, hdr->addr4, ETH_ALEN);
-
- if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_WDS &&
- sdata->vif.type != IEEE80211_IF_TYPE_MESH_POINT))
- return -1;
- break;
- case IEEE80211_FCTL_FROMDS:
- /* DA BSSID SA */
- memcpy(dst, hdr->addr1, ETH_ALEN);
- memcpy(src, hdr->addr3, ETH_ALEN);
-
+ } else if (ieee80211_has_fromds(hdr->frame_control)) {
if (sdata->vif.type != IEEE80211_IF_TYPE_STA ||
(is_multicast_ether_addr(dst) &&
!compare_ether_addr(src, dev->dev_addr)))
return -1;
- break;
- case 0:
- /* DA SA BSSID */
- memcpy(dst, hdr->addr1, ETH_ALEN);
- memcpy(src, hdr->addr2, ETH_ALEN);
-
+ } else {
if (sdata->vif.type != IEEE80211_IF_TYPE_IBSS)
return -1;
- break;
}
if (unlikely(skb->len - hdrlen < 8))
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 2a97322..309012d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1000,7 +1000,7 @@ __ieee80211_tx_prepare(struct ieee80211_tx_data *tx,
else if (test_and_clear_sta_flags(tx->sta, WLAN_STA_CLEAR_PS_FILT))
info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
- hdrlen = ieee80211_get_hdrlen(tx->fc);
+ hdrlen = ieee80211_hdrlen(hdr->frame_control);
if (skb->len > hdrlen + sizeof(rfc1042_header) + 2) {
u8 *pos = &skb->data[hdrlen + sizeof(rfc1042_header)];
tx->ethertype = (pos[0] << 8) | pos[1];
--
1.5.6.1.322.ge904b
On Tue, 2008-07-08 at 14:45 -0700, Harvey Harrison wrote:
> Well, I suppose I can also take another crack at getting my updated byteorder
> patches in that allows cpu_to_le16, etc to be in a case statement, then the
> switch could be preserved and the byteswaps can be done at compile time.
I thought we had __constant_cpu_to_le16 or something like that for that?
johannes
>
> - switch (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
> - case IEEE80211_FCTL_TODS:
> + if (ieee80211_has_a4(hdr->frame_control)) {
> + if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_WDS &&
> + sdata->vif.type != IEEE80211_IF_TYPE_MESH_POINT))
> + return -1;
> + } else if (ieee80211_has_tods(hdr->frame_control)) {
I don't particularly like converting a switch statement to chained ifs
in this path, you even put the most unlikely one first. With the switch,
the compiler should be able to generate just two compares for each path
(binary tree), while with this the common STA path already needs three.
We could instead open-code the binary tree that the compiler should
create for the switch, I guess, i.e.
if (has_tods()) {
if (has_fromds()) {
// a4 code
} else {
// tods code
}
} else {
if (has_fromds()) {
// fromds code
} else {
// none
}
}
but I think the switch is more readable.
johannes
On Tue, 2008-07-08 at 16:13 -0700, Harvey Harrison wrote:
> On Wed, 2008-07-09 at 01:04 +0200, Johannes Berg wrote:
> > On Tue, 2008-07-08 at 15:25 -0700, Harvey Harrison wrote:
> > > Those functions that still use ieee80211_get_hdrlen are moved over
> > > to use the little endian frame control.
> >
> > > - switch (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
> > > - case IEEE80211_FCTL_TODS:
> > > - /* BSSID SA DA */
> > > - memcpy(dst, hdr->addr3, ETH_ALEN);
> > > - memcpy(src, hdr->addr2, ETH_ALEN);
> > > -
> > > + switch (hdr->frame_control &
> > > + cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
> > > + case __constant_cpu_to_le16(IEEE80211_FCTL_TODS):
> > > if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_AP &&
> > > sdata->vif.type != IEEE80211_IF_TYPE_VLAN))
> > > return -1;
> > > break;
> >
> > Looks good, but didn't you just lose all the memcpys?
> >
>
> See the three lines immediately before the part you quoted.
Oh I see, sorry. Wouldn't it be more efficient to keep it inlined in the
switch though? Or is the compiler actually smart enough to move the
inline into there?
johannes
Those functions that still use ieee80211_get_hdrlen are moved over
to use the little endian frame control.
Signed-off-by: Harvey Harrison <[email protected]>
---
net/mac80211/rx.c | 62 ++++++++++++++++++++--------------------------------
net/mac80211/tx.c | 2 +-
2 files changed, 25 insertions(+), 39 deletions(-)
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index fab443d..dfe4184 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -811,7 +811,7 @@ ieee80211_reassemble_add(struct ieee80211_sub_if_data *sdata,
static inline struct ieee80211_fragment_entry *
ieee80211_reassemble_find(struct ieee80211_sub_if_data *sdata,
- u16 fc, unsigned int frag, unsigned int seq,
+ unsigned int frag, unsigned int seq,
int rx_queue, struct ieee80211_hdr *hdr)
{
struct ieee80211_fragment_entry *entry;
@@ -820,7 +820,6 @@ ieee80211_reassemble_find(struct ieee80211_sub_if_data *sdata,
idx = sdata->fragment_next;
for (i = 0; i < IEEE80211_FRAGMENT_MAX; i++) {
struct ieee80211_hdr *f_hdr;
- u16 f_fc;
idx--;
if (idx < 0)
@@ -832,10 +831,13 @@ ieee80211_reassemble_find(struct ieee80211_sub_if_data *sdata,
entry->last_frag + 1 != frag)
continue;
- f_hdr = (struct ieee80211_hdr *) entry->skb_list.next->data;
- f_fc = le16_to_cpu(f_hdr->frame_control);
+ f_hdr = (struct ieee80211_hdr *)entry->skb_list.next->data;
- if ((fc & IEEE80211_FCTL_FTYPE) != (f_fc & IEEE80211_FCTL_FTYPE) ||
+ /*
+ * Check ftype and addresses are equal, else check next fragment
+ */
+ if (((hdr->frame_control ^ f_hdr->frame_control) &
+ cpu_to_le16(IEEE80211_FCTL_FTYPE)) ||
compare_ether_addr(hdr->addr1, f_hdr->addr1) != 0 ||
compare_ether_addr(hdr->addr2, f_hdr->addr2) != 0)
continue;
@@ -860,11 +862,11 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
struct sk_buff *skb;
DECLARE_MAC_BUF(mac);
- hdr = (struct ieee80211_hdr *) rx->skb->data;
+ hdr = (struct ieee80211_hdr *)rx->skb->data;
sc = le16_to_cpu(hdr->seq_ctrl);
frag = sc & IEEE80211_SCTL_FRAG;
- if (likely((!(rx->fc & IEEE80211_FCTL_MOREFRAGS) && frag == 0) ||
+ if (likely((!ieee80211_has_morefrags(hdr->frame_control) && frag == 0) ||
(rx->skb)->len < 24 ||
is_multicast_ether_addr(hdr->addr1))) {
/* not fragmented */
@@ -879,7 +881,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
entry = ieee80211_reassemble_add(rx->sdata, frag, seq,
rx->queue, &(rx->skb));
if (rx->key && rx->key->conf.alg == ALG_CCMP &&
- (rx->fc & IEEE80211_FCTL_PROTECTED)) {
+ ieee80211_has_protected(hdr->frame_control)) {
/* Store CCMP PN so that we can verify that the next
* fragment has a sequential PN value. */
entry->ccmp = 1;
@@ -893,8 +895,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
/* This is a fragment for a frame that should already be pending in
* fragment cache. Add this fragment to the end of the pending entry.
*/
- entry = ieee80211_reassemble_find(rx->sdata, rx->fc, frag, seq,
- rx->queue, hdr);
+ entry = ieee80211_reassemble_find(rx->sdata, frag, seq, rx->queue, hdr);
if (!entry) {
I802_DEBUG_INC(rx->local->rx_handlers_drop_defrag);
return RX_DROP_MONITOR;
@@ -919,7 +920,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
memcpy(entry->last_pn, pn, CCMP_PN_LEN);
}
- skb_pull(rx->skb, ieee80211_get_hdrlen(rx->fc));
+ skb_pull(rx->skb, ieee80211_hdrlen(hdr->frame_control));
__skb_queue_tail(&entry->skb_list, rx->skb);
entry->last_frag = frag;
entry->extra_len += rx->skb->len;
@@ -1086,7 +1087,7 @@ ieee80211_data_to_8023(struct ieee80211_rx_data *rx)
{
struct net_device *dev = rx->dev;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) rx->skb->data;
- u16 fc, hdrlen, ethertype;
+ u16 hdrlen, ethertype;
u8 *payload;
u8 dst[ETH_ALEN];
u8 src[ETH_ALEN] __aligned(2);
@@ -1097,12 +1098,10 @@ ieee80211_data_to_8023(struct ieee80211_rx_data *rx)
DECLARE_MAC_BUF(mac3);
DECLARE_MAC_BUF(mac4);
- fc = rx->fc;
-
- if (unlikely(!WLAN_FC_DATA_PRESENT(fc)))
+ if (unlikely(!ieee80211_is_data_present(hdr->frame_control)))
return -1;
- hdrlen = ieee80211_get_hdrlen(fc);
+ hdrlen = ieee80211_hdrlen(hdr->frame_control);
if (ieee80211_vif_is_mesh(&sdata->vif)) {
int meshhdrlen = ieee80211_get_mesh_hdrlen(
@@ -1128,41 +1127,28 @@ ieee80211_data_to_8023(struct ieee80211_rx_data *rx)
* 1 0 BSSID SA DA n/a
* 1 1 RA TA DA SA
*/
+ memcpy(dst, ieee80211_get_DA(hdr), ETH_ALEN);
+ memcpy(src, ieee80211_get_SA(hdr), ETH_ALEN);
- switch (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
- case IEEE80211_FCTL_TODS:
- /* BSSID SA DA */
- memcpy(dst, hdr->addr3, ETH_ALEN);
- memcpy(src, hdr->addr2, ETH_ALEN);
-
+ switch (hdr->frame_control &
+ cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
+ case __constant_cpu_to_le16(IEEE80211_FCTL_TODS):
if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_AP &&
sdata->vif.type != IEEE80211_IF_TYPE_VLAN))
return -1;
break;
- case (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS):
- /* RA TA DA SA */
- memcpy(dst, hdr->addr3, ETH_ALEN);
- memcpy(src, hdr->addr4, ETH_ALEN);
-
- if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_WDS &&
+ case __constant_cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS):
+ if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_WDS &&
sdata->vif.type != IEEE80211_IF_TYPE_MESH_POINT))
return -1;
break;
- case IEEE80211_FCTL_FROMDS:
- /* DA BSSID SA */
- memcpy(dst, hdr->addr1, ETH_ALEN);
- memcpy(src, hdr->addr3, ETH_ALEN);
-
+ case __constant_cpu_to_le16(IEEE80211_FCTL_FROMDS):
if (sdata->vif.type != IEEE80211_IF_TYPE_STA ||
(is_multicast_ether_addr(dst) &&
!compare_ether_addr(src, dev->dev_addr)))
return -1;
break;
- case 0:
- /* DA SA BSSID */
- memcpy(dst, hdr->addr1, ETH_ALEN);
- memcpy(src, hdr->addr2, ETH_ALEN);
-
+ case __constant_cpu_to_le16(0):
if (sdata->vif.type != IEEE80211_IF_TYPE_IBSS)
return -1;
break;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 2a97322..309012d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1000,7 +1000,7 @@ __ieee80211_tx_prepare(struct ieee80211_tx_data *tx,
else if (test_and_clear_sta_flags(tx->sta, WLAN_STA_CLEAR_PS_FILT))
info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
- hdrlen = ieee80211_get_hdrlen(tx->fc);
+ hdrlen = ieee80211_hdrlen(hdr->frame_control);
if (skb->len > hdrlen + sizeof(rfc1042_header) + 2) {
u8 *pos = &skb->data[hdrlen + sizeof(rfc1042_header)];
tx->ethertype = (pos[0] << 8) | pos[1];
--
1.5.6.1.322.ge904b
On Wed, 2008-07-09 at 01:04 +0200, Johannes Berg wrote:
> On Tue, 2008-07-08 at 15:25 -0700, Harvey Harrison wrote:
> > Those functions that still use ieee80211_get_hdrlen are moved over
> > to use the little endian frame control.
>
> > - switch (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
> > - case IEEE80211_FCTL_TODS:
> > - /* BSSID SA DA */
> > - memcpy(dst, hdr->addr3, ETH_ALEN);
> > - memcpy(src, hdr->addr2, ETH_ALEN);
> > -
> > + switch (hdr->frame_control &
> > + cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
> > + case __constant_cpu_to_le16(IEEE80211_FCTL_TODS):
> > if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_AP &&
> > sdata->vif.type != IEEE80211_IF_TYPE_VLAN))
> > return -1;
> > break;
>
> Looks good, but didn't you just lose all the memcpys?
>
See the three lines immediately before the part you quoted.
Harvey
On Tue, 2008-07-08 at 23:28 +0200, Johannes Berg wrote:
> >
> > - switch (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
> > - case IEEE80211_FCTL_TODS:
>
> > + if (ieee80211_has_a4(hdr->frame_control)) {
> > + if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_WDS &&
> > + sdata->vif.type != IEEE80211_IF_TYPE_MESH_POINT))
> > + return -1;
> > + } else if (ieee80211_has_tods(hdr->frame_control)) {
>
> I don't particularly like converting a switch statement to chained ifs
> in this path, you even put the most unlikely one first. With the switch,
> the compiler should be able to generate just two compares for each path
> (binary tree), while with this the common STA path already needs three.
>
> We could instead open-code the binary tree that the compiler should
> create for the switch, I guess, i.e.
Well, I suppose I can also take another crack at getting my updated byteorder
patches in that allows cpu_to_le16, etc to be in a case statement, then the
switch could be preserved and the byteswaps can be done at compile time.
I'll go take another crack at it I suppose, but it seems to meet with stunning
silence whenever I post it :-(
Harvey
On Tue, 2008-07-08 at 15:25 -0700, Harvey Harrison wrote:
> Those functions that still use ieee80211_get_hdrlen are moved over
> to use the little endian frame control.
> - switch (fc & (IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
> - case IEEE80211_FCTL_TODS:
> - /* BSSID SA DA */
> - memcpy(dst, hdr->addr3, ETH_ALEN);
> - memcpy(src, hdr->addr2, ETH_ALEN);
> -
> + switch (hdr->frame_control &
> + cpu_to_le16(IEEE80211_FCTL_TODS | IEEE80211_FCTL_FROMDS)) {
> + case __constant_cpu_to_le16(IEEE80211_FCTL_TODS):
> if (unlikely(sdata->vif.type != IEEE80211_IF_TYPE_AP &&
> sdata->vif.type != IEEE80211_IF_TYPE_VLAN))
> return -1;
> break;
Looks good, but didn't you just lose all the memcpys?
johannes