[Bridge] [PATCH net-next v2 1/3] net: bridge: add support for user-controlled bool options

Andrew Lunn andrew at lunn.ch
Sat Nov 24 16:25:41 UTC 2018


On Sat, Nov 24, 2018 at 06:18:33PM +0200, nikolay at cumulusnetworks.com wrote:
> On 24 November 2018 18:10:41 EET, Andrew Lunn <andrew at lunn.ch> wrote:
> >> +int br_boolopt_toggle(struct net_bridge *br, enum br_boolopt_id opt,
> >bool on,
> >> +		      struct netlink_ext_ack *extack)
> >> +{
> >> +	switch (opt) {
> >> +	default:
> >> +		/* shouldn't be called with unsupported options */
> >> +		WARN_ON(1);
> >> +		break;
> >
> >So you return 0 here, meaning the br_debug() lower down will not
> >happen. Maybe return -EOPNOTSUPP?
> >
> 
> No, the idea here is that some option in the future might return an error. 
> This function cannot be called with unsupported option thus the warn. 

O.K, i was trying to make it easier to see which option caused it to
happen.

> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >
> >> +int br_boolopt_multi_toggle(struct net_bridge *br,
> >> +			    struct br_boolopt_multi *bm,
> >> +			    struct netlink_ext_ack *extack)
> >> +{
> >> +	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, extack);
> >> +		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;
> >> +		}
> >> +	}
> >
> >Does the semantics of extack allow you to return something even when
> >there is no error? If there are bits > BR_BOOLOPT_MAX you could return
> >0, but also add a warning in extack that some bits where not supported
> >by this kernel.
> 
> If we return 0 there's no reason to check extack. 

Well, the caller can check to see if extack is present, even on
success. This is extack, not extnack after all...

	 Andrew


More information about the Bridge mailing list