Hi Pavel,
thank you for your comments on this.
Pavel Roskin wrote:
> Hello, Hauke!
>
> Thank you for doing this effort! I would prefer that we avoid patching
> as much as possible. Patches tend to break as the code changes. I
> think there are several cases where patching can be eliminated.
>
>> ++#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,35))
>> + struct netdev_hw_addr *ha;
>> ++#else
>> ++ struct dev_mc_list *ha;
>> ++#endif
>
> We could simply use this in some header:
>
> #define dev_mc_list netdev_hw_addr
This will be possible, but as I see, only in some easy parts where the
patch looks like the quote above.
I will change that and resend this patch.
>
>> + /* comoute mc addresses' hash value ,and put it into hash table */
>
> Someone had a bout of dyslexia, and it will be fixed, breaking the patch
> :-)
>
>> + netdev_for_each_mc_addr(ha, netdev) {
>> ++#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,35))
>> + hash_value = atl1c_hash_mc_addr(hw, ha->addr);
>> ++#else
>> ++ hash_value = atl1c_hash_mc_addr(hw, ha->dmi_addr);
>> ++#endif
>
> #define addr dmi_addr
>
> OK, this is likely to break if done in a header, but maybe it could be
> done in the C code away from the rest of the code.
I think the if defs are better than a conditional define in every file
that needs it. It will result in strange compile errors if someone uses
addr for an other addresses.
>
>> ++#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,35))
>> + __hw_addr_unsync(&local->mc_list, &dev->mc, dev->addr_len);
>> ++#else
>> ++ __dev_addr_unsync(&local->mc_list, &local->mc_count,
>> ++ &dev->mc_list, &dev->mc_count);
>> ++#endif
>
> Cannot we reimplement __hw_addr_unsync()?
This change depends on some changes structs in
net/mac80211/ieee80211_i.h I do not see a way to reimplement this.
>
>> ++#if (LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,35))
>> +
>> + __hw_addr_init(&local->mc_list);
>> +
>> ++#endif
>
> That could be an empty function.
Yes that could be done, but I think it is much more clear to use this
patch here, because we do not need __hw_addr_init, because
local->mc_list is of a completely different type.
Hauke
Attachment:
signature.asc
Description: OpenPGP digital signature
