Re: [PATCH] mac80211: fix alignment issue with compare_ether_addr()

["Tomas Winkler" <tomasw@gmail.com>]

On Wed, May 28, 2008 at 8:33 PM, Senthilkumar Balasubramanian
<Senthilkumar.Balasubramanian@atheros.com> wrote:
>
> ________________________________________
> From: Tomas Winkler [tomasw@gmail.com]
> Sent: Wednesday, May 28, 2008 10:37 PM
> To: Senthilkumar Balasubramanian
> Cc: John W. Linville; johannes@sipsolutions.net; ron.rindjunsky@intel.com; linux-wireless@vger.kernel.org; Luis Rodriguez
> Subject: Re: [PATCH] mac80211: fix alignment issue with compare_ether_addr()
>
> On Wed, May 28, 2008 at 7:50 PM, Senthilkumar Balasubramanian
> <Senthilkumar.Balasubramanian@atheros.com> wrote:
>> ________________________________________
>> From: John W. Linville [linville@tuxdriver.com]
>> Sent: Wednesday, May 28, 2008 8:46 PM
>> To: Senthilkumar Balasubramanian
>> Cc: johannes@sipsolutions.net; ron.rindjunsky@intel.com; linux-wireless@vger.kernel.org; Luis Rodriguez
>> Subject: Re: [PATCH] mac80211: fix alignment issue with compare_ether_addr()
>>
>> On Wed, May 28, 2008 at 08:20:48PM +0530, Senthil Balasubramanian wrote:
>>> This addresses an alignment issue with compare_ether_addr().
>>> The addresses passed to compare_ether_addr should be two bytes aligned.
>>> It may function properly in x86 platform. However may not work properly
>>> on IA-64 or ARM processor.
>>
>>> --- a/net/mac80211/rx.c
>>> +++ b/net/mac80211/rx.c
>>> @@ -1116,7 +1116,9 @@ ieee80211_data_to_8023(struct ieee80211_rx_data *rx)
>>>       u16 fc, hdrlen, ethertype;
>>>       u8 *payload;
>>>       u8 dst[ETH_ALEN];
>>> -     u8 src[ETH_ALEN];
>>> +     /* Should be aligned on 2 bytes for compare_ether_addr() */
>>> +     u16 src_aligned[ETH_ALEN >> 1];
>>> +     u8 *src = (u8 *)src_aligned;
>>>       struct sk_buff *skb = rx->skb;
>>>       struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>>>       DECLARE_MAC_BUF(mac);
>>
>> Any reason you couldn't just do this?
>>
>> I just thought of using the type u16 instead of attribute here. that's all. No specific reason.
>>
>> @@ -1116,7 +1116,7 @@ ieee80211_data_to_8023(struct ieee80211_rx_data *rx)
>>        u16 fc, hdrlen, ethertype;
>>        u8 *payload;
>>        u8 dst[ETH_ALEN];
>> -       u8 src[ETH_ALEN];
>> +       u8 src[ETH_ALEN] __attribute__ ((aligned(2)));
>>        struct sk_buff *skb = rx->skb;
>>        struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>>        DECLARE_MAC_BUF(mac);
>>
>> It seems to compile w/o errors on i686, and it seems more clear to me.
>> Will that not work?
>>
>> Yes. It should also work. I will re-create the patch and send it to you. I will also include Johannes suggestion of just using __aligned macro.
>>
> Can we split the two issues in seperate patches. It's better for tracking.
>
> They are not two different issues. All of them addresses the same alignment issue only.  Initially I thought of using u16 to get it aligned on 2 byte and then I started using the attribute stuff as in page_group_addr[] and others required initializations for the u8 array.
>

I mean reordering buffer allocation and alignment are not the same issues.

Thanks
Tomas