Hi!

> Our patches against latest ieee80211 branch can be found at
> http://kernel.org/pub/linux/kernel/people/jbenc/
> 
> Patches have to be applied in this order (also specified in 'series'
> file):
> 
> debug-macros-cleanup.patch
> ipw2100-cleanup-prefixes.patch
> ipw2100-cleanup-debug-prints.patch
> ipw2100-cleanup-static.patch
>     Various cleanups from Pavel Machek, Adrian Bunk and us.
> 
> ipw2100-fix-config-promisc.patch
>     CONFIG_IPW2100_PROMISC should be CONFIG_IPW2100_MONITOR.
> 
> ipw2100-open-fix.patch
>     Imre Deak's patch for enabling carrier after an open when card is
>     already associated.
> 
> ipw2200-remove-support-for-obsolete-kernels.patch
> ipw2200-remove-trap-and-unused-stuff.patch
>     Cleanups from Pavel Machek.
>
> add-new-definitions.patch
> convert-defines-to-enums.patch
>     Adds all definitions from 802.11x specs (from Gertjan van Wingerde.)
> 
> endian-aware-types.patch
>     Michael Wu's patch for replacing u16 with __le16/__be16.

It seems to be trivial/nice cleanups up to here. Is there any problem
with merging these?

> ieee-dev-alignment.patch

+static inline void *ieee80211_dev_to_priv(struct net_device *dev)
+{
+       return (char *)dev +
+               ((sizeof(struct net_device) + NETDEV_ALIGN_CONST)
+                       & ~NETDEV_ALIGN_CONST) +
+               ((sizeof(struct ieee80211_device) + NETDEV_ALIGN_CONST)
+                       & ~NETDEV_ALIGN_CONST);
+}

Should not this use container_of() and friends?

> ipw-fix-after-ieee-dev-alignment.patch
>     Organizes the priv field of struct ieee80211_device in a similar manner
>     as the priv field in struct net_device. Makes drivers use
>     ieee80211_device instead of net_device.

Once container_of() is used, this should be easy to go in, right?

> configuration-struct.patch

You probably do not want to use u64... it is going to be slow on
32-bit architectures.

+static inline int ieee80211_encrypt_frame(
+       struct ieee80211_device *ieee,
+       struct sk_buff *skb,
+       int hdr_len)

this looks pretty ugly to my eyes. Could not we keep it on two lines?

> ipw-fix-after-configuration-struct.patch
>     Cleanup and improvements of ieee80211 configuration. Adds support for
>     switching off software fragmentation, fixes MSDU encryption.

> add-seq-number.patch
>     Adds sequence numbers to transmitted frames.

Looks nice.. why is ieee->seq_number += 0x10; increased by 0x10?

> separate-from-ethernet.patch
>     Makes the 802.11 layer independent of ethernet. 802.11 frames are no
>     longer transformed from ethernet frames.

Okay, here it starts to be "interesting". Patches up to here seem
quite mergeable to me. This one will rename wifi from eth1 to wlan0,
right?

OTOH that should not be a big problem; wifi is not in mainline, yet.

+#define IEEE80211_SNAP_IS_BRIDGE_TUNNEL(snap) \
+               ((snap)->oui[0] == 0 && (snap)->oui[1] == 0 && (snap)->oui[2] 
== 0xf8)

You really need some shorter prefix, and shorter names would be
welcome, too. [Maybe these should be inline functions?

+#define IEEE80211_GET_SADDR(hdr) \
+               (IEEE80211_FC_GET_FROMDS(hdr) ? \
+                       (IEEE80211_FC_GET_TODS(hdr) ? (hdr)->addr4 : 
(hdr)->addr3) \
+                       : (hdr)->addr2)

]

> eth_p_802_11-ethertype.patch
> fix-packet_socket-arp.patch
>     Fixes of things broken by the previous patch. New ETH_P_802_11 ethertype
>     is introduced, packet socket is fixed.

+#if defined(CONFIG_IEEE80211) || defined(CONFIG_IEEE80211_MODULE)
+       case ARPHRD_IEEE80211:
+               arp->ar_hrd = htons(ARPHRD_ETHER);
+               arp->ar_pro = htons(ETH_P_IP);
+               break;
+#endif

I believe CONFIG_FOO is set if CONFIG_FOO_MODULE is set; therefore we
do not need as ugly ifdefs.

> fix-definitions-for-xmit.patch
> ipw-fix-after-definitions-for-xmit.patch
>     Preparation for the next patch. Cleanup and fix of rate and modulation
>     constants and variables.

+u8 ieee80211_rate_values[IEEE80211_NUM_RATES] = {
+       0x02, 0x04,
+       0x0b, 0x16,
+       0x2c, 0x42,
+       0x0c, 0x12, 0x18, 0x24, 0x30, 0x48, 0x60, 0x6c
+};
+EXPORT_SYMBOL(ieee80211_rate_values);

Could we get some comment here?

> xmit-skb.patch
> ipw-fix-after-xmit-skb.patch
>     Support for sending fragments one by one and for sending management
>     frames. Wiped out struct ieee80211_txb.
>     This patch needs to be improved - comments are greatly appreciated.
> 
> recv-skb.patch
> ipw-fix-after-recv-skb.patch
>     More generic handling of received frames. Initial support for handling
>     of incoming management frames.
> 
> networks.patch

+/* Allocate the table and initialize into an empty state. */
+int ieee80211_networks_init(struct ieee80211_device *ieee)
+{
+       spin_lock_init(&ieee->networks_lock);
+       ieee->networks = kmalloc(sizeof(*ieee->networks) * WNETIDX_MAX,
+                                GFP_KERNEL);
+       if (ieee->networks)
+               ieee80211_networks_flush(ieee);
+       return (ieee->networks != NULL);
+}

Normal calling convention is 0 on success, -ERRNO on error.

+       if (n->newer != n->older) {
+               ieee->networks[n->older].newer = n->newer;
+               ieee->networks[n->newer].older = n->older;
+       }
+       else
+               n->newer = WNETIDX_MAX;

I'd not put newline before else.

+       if (!time_after(network->last_scanned + DEFAULT_MAX_SCAN_AGE, jiffies)) 
{
+               IEEE80211_DEBUG_SCAN(
+                       "Not showing network '%s ("
+                       MAC_FMT ")' due to age (%lums).\n",
+                       escape_essid(network->ssid,
+                                    network->ssid_len),
+                       MAC_ARG(network->bssid),
+                       (jiffies - network->last_scanned) / (HZ / 100));
+               return 1;
+       }

Ouch. Private debug macros are ugly, and putting 4 words on line
because you are religious about 80-column-rule seems like very bad
idea.

+static inline int _iwe_stream_add_event(struct translate_scan *data,
+                                        struct iw_event *iwe, int len)
+{
+       char *new_start;
+       new_start = iwe_stream_add_event(data->start, data->stop, iwe, len);
+       if (unlikely(new_start == data->start))
+               return -E2BIG;
+       data->start = new_start;
+       return 0;
+}

Now this is strange. _foo that is wrapper around foo. Looks too
similar to __foo convention to my eyes. Is some better naming
possible?

BTW gcc is probably smart enough to figure out that return -E2BIG is
error return and therefore unlikely.

> ipw-fix-after-networks.patch
>     Hashtable for detected networks. Introduced functions for manipulation
>     with the hashtable to allow support of devices performing the whole
>     scanning in firmware.

Now... those patches look quite okay to me. It would be nice to get
first half in, soon...
                                                                Pavel

-- 
if you have sharp zaurus hardware you don't need... you know my address
-
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

Reply via email to