Re: [patch 1/5] d80211: remove bitfields from ieee80211_tx_control
I am not particularily attached to bitfields or no bitfields. I am interested in getting d80211 merged. Bitfields have been discussed as an important TODO. Perhaps this can serve as a starting point for discussion of the tasks to complete before d80211 is merged? On Mon, Oct 16, 2006 at 12:34:07PM -0700, Simon Barber wrote: > Removing the bitfields makes the code much harder to read and maintain. I agree that we end up with more characters in the file, the bitfield syntax is more concise. However, I don't find it 'much' harder to read or maintain, it is a matter of taste. > Here we are working around a problem with the compiler by making the > code ugly - rather than fixing the compiler. The compilers are getting > better and better (GCC 4 has much better handling of this type of > optimization) but the code will remain ugly for ever. Well at least as far as code size goes gcc 4.1.2 still produces slightly larger code with the bitfields. -David - 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 1/5] d80211: remove bitfields from ieee80211_tx_control
On Mon, Oct 16, 2006 at 06:07:25PM +0200, Michael Buesch wrote: > On Friday 13 October 2006 21:20, David Kimdon wrote: > > All one-bit bitfields have been subsumed into the new 'flags' > > structure member and the new IEEE80211_TXCTL_* definitions. The > > multiple bit members were converted to u8, s8 or u16 as appropriate. I'll answer based on take1, which I just sent to the list. > > And, eh, did this increase or decrease the struct size? structure sizes did not change. > Does this generate better or worse code? It generates slightly smaller code with gcc 4.1.2. -David - 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 1/5] d80211: remove bitfields from ieee80211_tx_control
On Monday 16 October 2006 21:34, Simon Barber wrote: > Removing the bitfields makes the code much harder to read and maintain. > Here we are working around a problem with the compiler by making the > code ugly - rather than fixing the compiler. The compilers are getting > better and better (GCC 4 has much better handling of this type of > optimization) but the code will remain ugly for ever. Yeah, that's my opinion on this, too. But I still like the unsigned int foo:16; => u16 foo; type of conversions. -- Greetings Michael. - 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 1/5] d80211: remove bitfields from ieee80211_tx_control
Removing the bitfields makes the code much harder to read and maintain. Here we are working around a problem with the compiler by making the code ugly - rather than fixing the compiler. The compilers are getting better and better (GCC 4 has much better handling of this type of optimization) but the code will remain ugly for ever. Simon -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Michael Buesch Sent: Monday, October 16, 2006 9:07 AM To: David Kimdon Cc: netdev@vger.kernel.org; John W. Linville; Jiri Benc Subject: Re: [patch 1/5] d80211: remove bitfields from ieee80211_tx_control On Friday 13 October 2006 21:20, David Kimdon wrote: > All one-bit bitfields have been subsumed into the new 'flags' > structure member and the new IEEE80211_TXCTL_* definitions. The > multiple bit members were converted to u8, s8 or u16 as appropriate. And, eh, did this increase or decrease the struct size? Does this generate better or worse code? -- Greetings Michael. - 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 - 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 1/5] d80211: remove bitfields from ieee80211_tx_control
On Friday 13 October 2006 21:20, David Kimdon wrote: > All one-bit bitfields have been subsumed into the new 'flags' > structure member and the new IEEE80211_TXCTL_* definitions. The > multiple bit members were converted to u8, s8 or u16 as appropriate. And, eh, did this increase or decrease the struct size? Does this generate better or worse code? -- Greetings Michael. - 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 1/5] d80211: remove bitfields from ieee80211_tx_control
All one-bit bitfields have been subsumed into the new 'flags'
structure member and the new IEEE80211_TXCTL_* definitions. The
multiple bit members were converted to u8, s8 or u16 as appropriate.
Signed-off-by: David Kimdon <[EMAIL PROTECTED]>
Index: wireless-dev/include/net/d80211.h
===
--- wireless-dev.orig/include/net/d80211.h
+++ wireless-dev/include/net/d80211.h
@@ -143,23 +143,30 @@ struct ieee80211_tx_control {
* specific value for the rate (from
* struct ieee80211_rate) */
/* 1 = only first attempt, 2 = one retry, .. */
- unsigned int retry_limit:8;
+ u8 retry_limit;
/* duration field for RTS/CTS frame */
- unsigned int rts_cts_duration:16;
- unsigned int req_tx_status:1; /* request TX status callback for this
- * frame */
- unsigned int do_not_encrypt:1; /* send this frame without encryption;
- * e.g., for EAPOL frames */
- unsigned int use_rts_cts:1; /* Use RTS-CTS before sending frame. */
- unsigned int use_cts_protect:1; /* Use CTS protection for the frame
-* (e.g., for combined 802.11g /
-* 802.11b networks) */
-unsigned int no_ack:1; /* Tell the low level not to wait for an ack */
- unsigned int rate_ctrl_probe:1;
- unsigned int clear_dst_mask:1;
- unsigned int requeue:1;
- unsigned int first_fragment:1; /* This is a first fragment of the
-* frame */
+ u16 rts_cts_duration;
+
+#define IEEE80211_TXCTL_REQ_TX_STATUS (1<<0)/* request TX status callback for
+ * this frame */
+#define IEEE80211_TXCTL_DO_NOT_ENCRYPT (1<<1) /* send this frame without
+ * encryption; e.g., for EAPOL
+ * frames */
+#define IEEE80211_TXCTL_USE_RTS_CTS(1<<2) /* use RTS-CTS before sending
+ * frame */
+#define IEEE80211_TXCTL_USE_CTS_PROTECT(1<<3) /* use CTS protection
for the
+ * frame (e.g., for combined
+ * 802.11g / 802.11b networks) */
+#define IEEE80211_TXCTL_NO_ACK (1<<4) /* tell the low level not to
+ * wait for an ack */
+#define IEEE80211_TXCTL_RATE_CTRL_PROBE(1<<5)
+#define IEEE80211_TXCTL_CLEAR_DST_MASK (1<<6)
+#define IEEE80211_TXCTL_REQUEUE(1<<7)
+#define IEEE80211_TXCTL_FIRST_FRAGMENT (1<<8) /* this is a first fragment of
+ * the frame */
+ u32 flags; /* tx control flags defined
+ * above */
+
/* following three flags are only used with Atheros Super A/G */
unsigned int compress:1;
unsigned int turbo_prime_notify:1; /* notify HostAPd after frame
@@ -168,18 +175,16 @@ struct ieee80211_tx_control {
unsigned int atheros_xr:1; /* only used with Atheros XR */
-unsigned int power_level:8; /* per-packet transmit power level, in dBm
-*/
- unsigned int antenna_sel:4; /* 0 = default/diversity,
-* 1 = Ant0, 2 = Ant1 */
- int key_idx:8; /* -1 = do not encrypt, >= 0 keyidx from hw->set_key()
- */
- int icv_len:8; /* Length of the ICV/MIC field in octets */
- int iv_len:8; /* Length of the IV field in octets */
- unsigned int queue:4; /* hardware queue to use for this frame;
- * 0 = highest, hw->queues-1 = lowest */
- unsigned int sw_retry_attempt:4; /* no. of times hw has tried to
- * transmit frame (not incl. hw retries) */
+ u8 power_level; /* per-packet transmit power level, in dBm */
+ u8 antenna_sel; /* 0 = default/diversity, 1 = Ant0, 2 = Ant1 */
+ s8 key_idx; /* -1 = do not encrypt, >= 0 keyidx from
+* hw->set_key() */
+ u8 icv_len; /* length of the ICV/MIC field in octets */
+ u8 iv_len; /* length of the IV field in octets */
+ u8 queue; /* hardware queue to use for this frame;
+* 0 = highest, hw->queues-1 = lowest */
+ u8 sw_retry_attempt;/* number of times hw has tried to
+* transmit frame (not incl. hw retries) */
int rateidx; /* internal 80211.o rateidx */
int alt_retry_rate; /* retry rate for the last retries, given as the
Index: wireless-dev/net/d80211/ieee80211.c
