Re: [PATCH] d80211: get rid of the WME bitfield
Jouni Malinen wrote: I think that this is always 16-bit aligned in the current implementation, but I would be a bit careful with this type of change here. This can be done easily with u8 since the qos_control field has 8 bits of used data and 8 bits of reserved field. My change was handling this as two separate 8-bit values, so no issues with potential unaligned accesses. Good point, I forgot about that. Never mind this patch then, if I get to it I'll make a new one. johannes - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] d80211: get rid of the WME bitfield
On Mon, Aug 14, 2006 at 10:15:14AM +0200, Johannes Berg wrote: > Somewhere, sometime, someone had to start getting rid of bitfields ;) Yes and you wouldn't believe how many times I have had to complain about this particular case internally with it always coming back as bitfield even after showing that it caused a unaligned access on one target platform.. However, I remember fixing this long time ago to use proper operation on u8 values. It looks like I just have forgotten to submit those changes to wireless-dev.git. There was quite a bit of additional cleanup, so it would probably be best if I were to take a closer look at propose an alternative patch for changing this and getting rid for struct qos_control. > +u16 *p = (u16 *) (skb->data + ieee80211_get_hdrlen(fc) - 2); > +u16 qos_hdr = skb->priority & QOS_CONTROL_TAG1D_MASK; > if (local->wifi_wme_noack_test) > -qc->ack_policy = 1; > +qos_hdr |= QOS_CONTROL_ACK_POLICY_NOACK << > +QOS_CONTROL_ACK_POLICY_SHIFT; > +*p = cpu_to_le16(qos_hdr); I think that this is always 16-bit aligned in the current implementation, but I would be a bit careful with this type of change here. This can be done easily with u8 since the qos_control field has 8 bits of used data and 8 bits of reserved field. My change was handling this as two separate 8-bit values, so no issues with potential unaligned accesses. -- Jouni MalinenPGP id EFC895FA - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] d80211: get rid of the WME bitfield
Somewhere, sometime, someone had to start getting rid of bitfields ;)
This one seemed an easy target :P
Signed-off-by: Johannes Berg <[EMAIL PROTECTED]>
--- wireless-dev.orig/net/d80211/wme.c2006-08-12 10:43:01.809644280
+0200
+++ wireless-dev/net/d80211/wme.c2006-08-12 10:51:35.909644280 +0200
@@ -246,15 +246,13 @@ static int wme_qdiscop_enqueue(struct sk
/* now we know the 1d priority, fill in the QoS header if there is one
*/
if (WLAN_FC_IS_QOS_DATA(fc)) {
-struct qos_control *qc = (struct qos_control *)
-(skb->data + ieee80211_get_hdrlen(fc) - 2);
-u8 *p = (u8 *) qc;
-*p++ = 0; /* do this due to gcc's lack of optimization on
- * bitfield ops */
-*p = 0;
-qc->tag1d = skb->priority;
+/* hard-coded QOS control header length! */
+u16 *p = (u16 *) (skb->data + ieee80211_get_hdrlen(fc) - 2);
+u16 qos_hdr = skb->priority & QOS_CONTROL_TAG1D_MASK;
if (local->wifi_wme_noack_test)
-qc->ack_policy = 1;
+qos_hdr |= QOS_CONTROL_ACK_POLICY_NOACK <<
+QOS_CONTROL_ACK_POLICY_SHIFT;
+*p = cpu_to_le16(qos_hdr);
}
if (unlikely(queue >= local->hw->queues)) {
--- wireless-dev.orig/net/d80211/wme.h2006-08-12 10:53:18.899644280
+0200
+++ wireless-dev/net/d80211/wme.h2006-08-12 10:53:21.629644280 +0200
@@ -24,26 +24,7 @@
#define QOS_CONTROL_TID_MASK 0x0f
#define QOS_CONTROL_ACK_POLICY_SHIFT 5
-/* This bit field structure should not be used; it can cause compiler to
- * generate unaligned accesses and inefficient code. */
-struct qos_control {
-#if defined(__LITTLE_ENDIAN_BITFIELD)
-u8tag1d:3, /* bits 0-2 */
- reserved1:1,
-eosp:1,
-ack_policy:2,
- reserved2:1;
-#elif defined (__BIG_ENDIAN_BITFIELD)
-u8reserved2:1,
-ack_policy:2,
-eosp:1,
-reserved1:1,
-tag1d:3; /* bits 0-2 */
-#else
-#error"Please fix "
-#endif
-u8 reserved;
-} __attribute__ ((packed));
+#define QOS_CONTROL_TAG1D_MASK 0x07
ieee80211_txrx_result
ieee80211_rx_h_parse_qos(struct ieee80211_txrx_data *rx);
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
