Re: [PATCH v6] qtnfmac: introduce new FullMAC driver for Quantenna chipsets

[Johannes Berg <johannes@sipsolutions.net>]

Good job :)

Let's merge this - the tiny fixes for things I found can come later.

Some (mostly trivial) comments:

 * I'm surprised you don't support WEP - nice :)
 * I don't think returning -EFAULT from qtnf_add_virtual_intf is a good
   idea - perhaps better propagate the error or use -EIO?
 * the forward declaration of struct bus in pearl/pcie_bus_priv.h seems unnecessary
 * you might want to use a whitelist in qtnf_set_wiphy_params(), but
   it's not really important as we should add capability bits for
   everything that we add
 * qtnf_mgmt_frame_register()/cfg80211_rx_mgmt() aren't used correctly
   - since there's filtering further than the frame type/subtype, you
   need to check the return value of cfg80211_rx_mgmt(). If, for
   example, userspace only registers for a certain action frame
   category, you need to still reject the remaining ones yourself. We
   could extend the API here to give you the whole filter data as well,
   if that helps. Otherwise you need to handle the return value from
   cfg80211_rx_mgmt().
 * The ENOENT handling in qtnf_dump_station() surprises me, especially
   since there's no checking that this doesn't result in duplicate
   cfg80211_del_sta() calls due to races - if
   qtnf_event_handle_sta_deauth() wins to remove it with
   qtnf_sta_list_del() but qtnf_dump_station() already had it looked
   up? Seems like the whole handling in that function either needs to
   be the same as in the event, or just be removed and return -ENOENT
 * There seems to be little point in dynamically allocating the
   iface_comb in qtnf_wiphy_register() vs. just embedding it in struct
   qtnf_wmac or so?
 * qtnf_rx_frame() is declared but not used?
 * -D__CHECK_ENDIAN was always wrong, it should be -D__CHECK_ENDIAN__,
   but it's now also enabled by default so can be removed
 * I'm not sure there's much point in printing the failure code address
   (which should be static) in pr_err("skb DMA mapping error: %pad\n",
   &skb_paddr);
 * I like the real use of NAPI :)
 * However, is there any specific reason you're not using
   napi_gro_receive() rather than netif_receive_skb()? It seems using
   GRO would help TCP streams, in particular by reducing the number of
   ACKs
 * with just a single (PCI-E) transport, I'm not sure I see much point
   in splitting it into a separate module, which necessitates the
   EXPORT_SYMBOL_GPL, which do take up a bit of space. But there are
   only 4, so it doesn't really matter :) One thing we did in iwlwifi
   was to not export if both are built-in, since nobody else should
   ever have any reason to access them.

Thanks all!

johannes