DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 743C8214C5
Message-ID: <1539116037.3687.143.camel@sipsolutions.net>
Subject: Re: [PATCH] mac80211_hwsim: allow setting custom features/iftypes
From:   Johannes Berg <johannes@sipsolutions.net>
To:     James Prestwood <james.prestwood@linux.intel.com>,
        linux-wireless@vger.kernel.org
Date:   Tue, 09 Oct 2018 22:13:57 +0200
In-Reply-To: <0b8356b90053c16e674905ca78707d6a19de0701.camel@linux.intel.com>
References: <20181009174829.15163-1-james.prestwood@linux.intel.com>
         (sfid-20181009_194449_357742_49BD7697) <1539114071.3687.134.camel@sipsolutions.net>
         <0b8356b90053c16e674905ca78707d6a19de0701.camel@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Tue, 2018-10-09 at 13:12 -0700, James Prestwood wrote:

> Ok, that makes sense. The intent here was to test logic for detecting
> and handling supported driver features/iftypes, rather than actually
> using the features. But I do understand it would be strange for anyone
> trying to enable a feature, to find that all it does it set a feature
> bit (although this is exactly what we want :)).

:-)

In general, I guess what would work is to be able to *restrict* the
advertised features over what's currently the case, but I suspect that's
not something that would be so very much useful for you (unless we also
add more features to hwsim).

> Would it be acceptable
> (for now at least) if we just included the iftype piece? I can pull
> that out into a new patch if so.

As written, it doesn't make much sense, but again you could restrict the
feature set? There are also the pure software feature types etc., and
mac80211 will add those in some cases.

Similar to the features though, you wouldn't want to advertise e.g. NAN
over hwsim, since that requires special operations to be implemented.

I guess here also I could see perhaps a way to restrict the interface
types could be worthwhile?

Though I'd do the implementation with a single u32 attribute with a
bitmap, rather than the massive nested flags. Which, btw, you should've
used the nested policy support for that I added recently in commit
9a659a35ba17 ("netlink: allow NLA_NESTED to specify nested policy to
validate"), but that point is of course moot if we don't want to do a
nested attribute :-)

johannes