| Message ID | 20190124180535.20216-1-malat@debian.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | mac80211: Remove attribute packed from struct 'action' | expand |
On Thu, 2019-01-24 at 19:05 +0100, Mathieu Malaterre wrote: > During refactor in commit 9e478066eae4 ("mac80211: fix MU-MIMO > follow-MAC mode") a new struct 'action' was declared with packed > attribute as: > > struct { > struct ieee80211_hdr_3addr hdr; > u8 category; > u8 action_code; > } __packed action; > > But since struct 'ieee80211_hdr_3addr' is declared with an aligned > keyword as: > > struct ieee80211_hdr { > __le16 frame_control; > __le16 duration_id; > u8 addr1[ETH_ALEN]; > u8 addr2[ETH_ALEN]; > u8 addr3[ETH_ALEN]; > __le16 seq_ctrl; > u8 addr4[ETH_ALEN]; > } __packed __aligned(2); > > Solve the ambiguity of placing aligned structure in a packed one by > removing the packed attribute from struct. This seems to be the behavior > of gcc anyway, since the following is still compiling: > > BUILD_BUG_ON(sizeof(action) != IEEE80211_MIN_ACTION_SIZE + 1); I'm not sure this will work on all platforms, didn't something like alpha pad out u8's to u32 when not requiring packing? I guess I'd feel better about using __packed __aligned(2) here as well, which should solve the warning too? johannes
On Thu, Jan 24, 2019 at 7:08 PM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Thu, 2019-01-24 at 19:05 +0100, Mathieu Malaterre wrote: > > During refactor in commit 9e478066eae4 ("mac80211: fix MU-MIMO > > follow-MAC mode") a new struct 'action' was declared with packed > > attribute as: > > > > struct { > > struct ieee80211_hdr_3addr hdr; > > u8 category; > > u8 action_code; > > } __packed action; > > > > But since struct 'ieee80211_hdr_3addr' is declared with an aligned > > keyword as: > > > > struct ieee80211_hdr { > > __le16 frame_control; > > __le16 duration_id; > > u8 addr1[ETH_ALEN]; > > u8 addr2[ETH_ALEN]; > > u8 addr3[ETH_ALEN]; > > __le16 seq_ctrl; > > u8 addr4[ETH_ALEN]; > > } __packed __aligned(2); > > > > Solve the ambiguity of placing aligned structure in a packed one by > > removing the packed attribute from struct. This seems to be the behavior > > of gcc anyway, since the following is still compiling: > > > > BUILD_BUG_ON(sizeof(action) != IEEE80211_MIN_ACTION_SIZE + 1); > > I'm not sure this will work on all platforms, didn't something like > alpha pad out u8's to u32 when not requiring packing? I was not aware of that. > I guess I'd feel better about using __packed __aligned(2) here as well, > which should solve the warning too? Indeed, I will re-spin a v2 then.
On Thu, 2019-01-24 at 19:14 +0100, Mathieu Malaterre wrote: > > I'm not sure this will work on all platforms, didn't something like > > alpha pad out u8's to u32 when not requiring packing? > > I was not aware of that. TBH, I'm not actually sure about that. Pure hearsay. If anyone knows, I'd like to know too :) johannes
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 45aad3d3108c..709359650149 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -231,7 +231,7 @@ static void ieee80211_handle_mu_mimo_mon(struct ieee80211_sub_if_data *sdata, struct ieee80211_hdr_3addr hdr; u8 category; u8 action_code; - } __packed action; + } action; if (!sdata) return;
During refactor in commit 9e478066eae4 ("mac80211: fix MU-MIMO follow-MAC mode") a new struct 'action' was declared with packed attribute as: struct { struct ieee80211_hdr_3addr hdr; u8 category; u8 action_code; } __packed action; But since struct 'ieee80211_hdr_3addr' is declared with an aligned keyword as: struct ieee80211_hdr { __le16 frame_control; __le16 duration_id; u8 addr1[ETH_ALEN]; u8 addr2[ETH_ALEN]; u8 addr3[ETH_ALEN]; __le16 seq_ctrl; u8 addr4[ETH_ALEN]; } __packed __aligned(2); Solve the ambiguity of placing aligned structure in a packed one by removing the packed attribute from struct. This seems to be the behavior of gcc anyway, since the following is still compiling: BUILD_BUG_ON(sizeof(action) != IEEE80211_MIN_ACTION_SIZE + 1); This removes the following warning (W=1): net/mac80211/rx.c:234:2: warning: alignment 1 of 'struct <anonymous>' is less than 2 [-Wpacked-not-aligned] Cc: Johannes Berg <johannes.berg@intel.com> Signed-off-by: Mathieu Malaterre <malat@debian.org> --- net/mac80211/rx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)