Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <93c99a81d89aafed21e8e22157b3bf21526be716.camel@sipsolutions.net>
Subject: Re: [PATCH v3 2/2] mac80211: do intersection with he mcs and nss
 set of peer and own
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Kalle Valo <kvalo@codeaurora.org>, Wen Gong <wgong@codeaurora.org>
Cc:     ath11k@lists.infradead.org, linux-wireless@vger.kernel.org
Date:   Fri, 01 Oct 2021 09:01:02 +0200
In-Reply-To: <87bl49mf02.fsf@codeaurora.org>
References: <1609816120-9411-1-git-send-email-wgong@codeaurora.org>
         <1609816120-9411-3-git-send-email-wgong@codeaurora.org>
         <b6c96c4ecdf9ec175d7f89e8600fb53768287cc2.camel@sipsolutions.net>
         <facd18458a7ecfc0afbfd06c8a57d849@codeaurora.org>
         <87bl49mf02.fsf@codeaurora.org>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 2021-10-01 at 09:32 +0300, Kalle Valo wrote:
> > 
> > Add "__packed" before the "__le16 *" should solve this warning by my
> > understand like this:

[snip]
> > 
> > -static void ieee80211_he_mcs_disable(__le16 *he_mcs)
> > +static void ieee80211_he_mcs_disable(__packed __le16 *he_mcs)
> > 

[snip]

> I don't know what Johannes thinks, but to me that looks like an ugly
> hack. Wouldn't use get_unaligned() or similar be cleaner?
> 
Well, then we've have to pass an untyped pointer (void *), which I guess
is fine? Since we do all kinds of le16_to_cpu() with it anyway, that'd
just become get_unaligned_le16().

That's probably the better choice.

But regardless, would the __packed even *work*? __attribute__((packed))
is documented as:

   This attribute, attached to a struct, union, or C++ class type
   definition, specifies that each of its members (other than zero-width
   bit-fields) is placed to minimize the memory required. This is
   equivalent to specifying the packed attribute on each of the members.
   
   When attached to an enum definition, the packed attribute indicates that
   the smallest integral type should be used. Specifying the -fshort-enums
   flag on the command line is equivalent to specifying the packed
   attribute on all enum definitions. 
   
   [snip example]
   
   You may only specify the packed attribute on the definition of an enum,
   struct, union, or class, not on a typedef that does not also define the
   enumerated type, structure, union, or class. 

So I'm not convinced it would actually *do* anything here at all, in the
proposed context?

johannes