On Tue, 2018-10-09 at 13:42 -0700, James Prestwood wrote:
> > 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.
Well I guess you really only need *disabled* ones since the default
would remain to have all enabled, and you can't change them on the fly,
only set them at radio creation, right?
> The reason I did it this way was to follow how NL80211 packaged up
> iftype support (NL80211_ATTR_SUPPORTED_IFTYPES).
Ok, but I still think a bitmap would make more sense, back then for some
reason I/we didn't (like to) do bitmaps, but these days I think it's the
much better representation.
> 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.
Sort of.
Yes, I think we would want to have a list/mask of *enabled* (extended)
features/interface types, unlike what I said above to just have a mask
of disabled features; if we do a mask of disabled ones then one
basically has to list all current and *future ones* to have predictable
output, that's not going to be feasible.
So yes, we want a list of *enabled* ones.
However, I think we want to reject configurations that list more enabled
features than we currently support, because otherwise again you end up
with unpredictability if the hwsim version changes underneath your tool
or test case.
So I think the algorithm should be:
valid_features = <current list>;
enabled_features = nla_get_whatever(...);
if (enabled_features & ~valid_features) {
NL_SET_ERR_ATTR_MSG(...);
return -EINVAL;
}
// use enabled_features instead of the current list later
johannes
