Thanks Johannes. Will send out V2 shortly. -yi > -----Original Message----- > From: Johannes Berg [mailto:johannes@xxxxxxxxxxxxxxxx] > Sent: 2009年5月21日 17:32 > To: Zhu, Yi > Cc: linville@xxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx; Ortiz, Samuel > Subject: Re: [PATCH 1/2] wireless: move some utility functions from mac80211 > to cfg80211 > > On Thu, 2009-05-21 at 10:18 +0800, Zhu Yi wrote: > > > --- a/include/net/cfg80211.h > > +++ b/include/net/cfg80211.h > > @@ -752,6 +752,32 @@ enum wiphy_params_flags { > > }; > > > > /** > > + * enum ieee80211_key_alg - key algorithm > > + * @ALG_WEP: WEP40 or WEP104 > > + * @ALG_TKIP: TKIP > > + * @ALG_CCMP: CCMP (AES) > > + * @ALG_AES_CMAC: AES-128-CMAC > > + */ > > +enum ieee80211_key_alg { > > + ALG_WEP, > > + ALG_TKIP, > > + ALG_CCMP, > > + ALG_AES_CMAC, > > +}; > > + > > +/** > > + * enum ieee80211_key_len - key length > > + * @LEN_WEP40: WEP 5-byte long key > > + * @LEN_WEP104: WEP 13-byte long key > > + */ > > +enum ieee80211_key_len { > > + LEN_WEP40 = 5, > > + LEN_WEP104 = 13, > > + LEN_CCMP = 16, > > + LEN_TKIP = 32, > > +}; > > This doesn't seem appropriate. ALG_* is purely mac80211 internals, and > shouldn't be in cfg80211's API since cfg80211 uses u32 values as cipher > suite specifiers. Why do you need these? > > And if you want to move the key lengths they probably should be in > ieee80211.h, have a proper prefix and also be used within cfg80211 > itself... > > > +/* Default mapping in classifier to work with default queue setup. */ > > +const int ieee802_1d_to_ac[8] = { 2, 3, 3, 2, 1, 1, 0, 0 }; > > +EXPORT_SYMBOL(ieee802_1d_to_ac); > > That seems a little odd, unless we want to specify how the default queue > setup should be ... which seems to make no sense. It probably will be > like this most of the time, but still, should this really be mandated by > cfg80211? There seems to be no reason to do that. I'd rather keep this > in every driver since the drivers may differ, and changing this for > mac80211 is unlikely. > > > +int ieee80211_data_to_8023(struct sk_buff *skb, struct net_device *dev, > > + enum nl80211_iftype iftype) > > Should probably pass in dev->dev_addr instead of dev for this and the > other direction? OTOH, I suppose it only makes sense to be used for a > netdev. > > > > > > +u16 cfg80211_classify80211(struct sk_buff *skb) > > +{ > > + struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data; > > + > > + if (!ieee80211_is_data(hdr->frame_control)) { > > + /* management frames go on AC_VO queue, but are sent > > + * without QoS control fields */ > > + return 0; > > + } > > + > > + if (ieee80211_is_data_qos(hdr->frame_control)) > > + skb->priority = cfg80211_classify8021d(skb); > > + else > > + skb->priority = 0; > > + > > + return ieee802_1d_to_ac[skb->priority]; > > +} > > +EXPORT_SYMBOL(cfg80211_classify80211); > > If you pass in suitable information for the ACM downgrade, it seems you > can move over that code too and remove one export. If your device > supports ACM you can always pass in 0 for the ACM bitfield, I think? It > seems a little weird to have this function and then write it in a way > mac80211 can't use it. > > johannes ?韬{.n?????%??檩??w?{.n???{??W????塄}?财??j:+v?? ? ???2??璀??摺?囤??z夸z罐?+?????w棹f
