> Hi,
> 
> On Wed, Mar 13, 2019 at 12:13:52PM +0800, [email protected] wrote:
> > diff --git a/drivers/net/wireless/realtek/rtw88/pci.c
> b/drivers/net/wireless/realtek/rtw88/pci.c
> > new file mode 100644
> > index 0000000..cf3bffb
> > --- /dev/null
> > +++ b/drivers/net/wireless/realtek/rtw88/pci.c
> > @@ -0,0 +1,1211 @@
> ...
> > +static u8 ac_to_hwq[] = {
> > +   [0] = RTW_TX_QUEUE_VO,
> > +   [1] = RTW_TX_QUEUE_VI,
> > +   [2] = RTW_TX_QUEUE_BE,
> > +   [3] = RTW_TX_QUEUE_BK,
> > +};
> 
> I don't want to dig up and repeat the details of my comments, but I
> think you missed my earlier suggestion here: you should use the
> ieee80211_ac_numbers enum instead of bare 0-3 indexing.

Fixed, thanks. Will be in the next patches.

> 
> > +static u8 rtw_hw_queue_mapping(struct sk_buff *skb)
> > +{
> > +   struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
> > +   __le16 fc = hdr->frame_control;
> > +   u8 q_mapping = skb_get_queue_mapping(skb);
> > +   u8 queue;
> > +
> > +   if (unlikely(ieee80211_is_beacon(fc)))
> > +           queue = RTW_TX_QUEUE_BCN;
> > +   else if (unlikely(ieee80211_is_mgmt(fc) || ieee80211_is_ctl(fc)))
> > +           queue = RTW_TX_QUEUE_MGMT;
> > +   else
> > +           queue = ac_to_hwq[q_mapping];
> 
> It also seems wise to be defensive about the values of 'q_mapping', so
> we don't accidentally overstep the array if for some reason the
> implementation details of mac80211 change some day. e.g.:
> 
>       else if (WARN_ON_ONCE(q_mapping >= ARRAY_SIZE(ac_to_hwq)))
>               // Pick a default.

Yeah, should check if q_mapping does not fit into the array.
Thanks.

> 
> Brian
> 
> > +
> > +   return queue;
> > +}
> > +
> 

Yan-Hsuan

Reply via email to