On Tue, 2018-10-09 at 22:13 +0200, Johannes Berg wrote:
> 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).
Actually this would work perfectly. Just being able to disable/restrict
features will work fine. I can change the attribute to take a mask of
disabled/enabled features, but only features that hwsim actually
supports.
>
> > 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 :-)
The reason I did it this way was to follow how NL80211 packaged up
iftype support (NL80211_ATTR_SUPPORTED_IFTYPES). We could model this
after how we want the feature support, where we only allow
disabling/enabling features/iftypes that are already supported in the
default configuration. So, for iftypes, my code could remain the same
where I build up a mask of iftypes, but then AND that with a the
default configuration.
Thanks,
James
>
> johannes
