On 22/11/2018 17:49, Andrew Lunn wrote:
>> +/* br_boolopt_toggle - change user-controlled boolean option
>> + *
>> + * @br: bridge device
>> + * @opt: id of the option to change
>> + * @on: new option value
>> + *
>> + * Changes the value of the respective boolean option to @on taking care of
>> + * any internal option value mapping and configuration.
>> + */
>> +int br_boolopt_toggle(struct net_bridge *br, enum br_boolopt_id opt, bool on)
>> +{
>> + int err = -ENOENT;
>> +
>> + switch (opt) {
>> + default:
>> + break;
>> + }
>> +
>> + return err;
>> +}
>> +
>
>
>> +int br_boolopt_multi_toggle(struct net_bridge *br,
>> + struct br_boolopt_multi *bm)
>> +{
>> + unsigned long bitmap = bm->optmask;
>> + int err = 0;
>> + int opt_id;
>> +
>> + for_each_set_bit(opt_id, &bitmap, BR_BOOLOPT_MAX) {
>> + bool on = !!(bm->optval & BIT(opt_id));
>> +
>> + err = br_boolopt_toggle(br, opt_id, on);
>> + if (err) {
>> + br_debug(br, "boolopt multi-toggle error: option: %d current: %d new: %d error: %d\n",
>> + opt_id, br_boolopt_get(br, opt_id), on, err);
>> + break;
>> + }
>
> An old kernel with a new iproute2 might partially succeed in toggling
> some low bits, but then silently ignore a high bit that is not
> supported by the kernel. As a user, i want to know the kernel does not
> support an option i'm trying to toggle.
>
That is already how netlink option setting works, if the option isn't supported
it is silently ignored. I tried to stay as close to the current behaviour as possible.
It also applies to partial option changes, some options will be changed until an error
is encountered.
> Can you walk all the bits in the u32 from the MSB to the LSB? That
> should avoid this problem.
>
I did wonder about this and left it as netlink option behaviour but
I can leave the walk as is and just check if the highest order set bit >= BR_BOOLOPT_MAX
and err out with ENOTSUPP for example. Will reconsider for v2.
> Andrew
>
