Re: [PATCH v2 03/10] nl80211: Prefer ether_addr_copy

[Johannes Berg <johannes@sipsolutions.net>]

> The requirement is to be __aligned(2). I've added 4 instances of
> ether_addr_copy with 8 addresses as arguments.  Of these, the 4
> src arguments are really the same type (i.e. nla_data acting on a
> const nlattr*), so I'll try to reason about the 5 total cases below -
> 1. cfg->dst_mac should be 16-bit aligned due to the layout of
> struct cfg80211_wowlan_tcp. Its offset is 10 or 12 bytes in the
> structure depending on the system.

I wouldn't want to rely on that, since internal structures can be
changed pretty easily. If necessary, add __aligned(2) to the struct
members where appropriate.

> 2 and 3. For mac_addr and mac_addr_mask, nl80211_parse_random_mac
> takes these in as u8* (and hence does not guarantee alignment?)
> Both the callers of this function today pass in arguments that are
> explicitly __aligned(2). But this cannot be said of future potential
> callers
> - so perhaps my patch introduces a bug?

That should be documented, but it's also pretty tricky to
review/maintain that.

I general, I don't really see much reason to use ether_addr_copy() in
any of these code paths - it's not really a performance thing (in part
due to the alignment, ether_addr_copy can be faster)

> 4. Based on struct cfg80211_acl_data, acl->mac_addrs[i] should be not
> guaranteed to be __aligned(2).

See above.

> 5. For all the nla_data src arguments, the nla_data function returns
> ((char*) foo + 5) for pointer foo. So likely not __aligned(2).

Those should be aligned since the data is copied into an SKB, and +5
doesn't seem right - nla_data() should be +4.

johannes