Re: [PATCH net-next v3 1/2] net/sched: tbf: add extack to offload params

[Simon Horman]

From: 'Simon Horman' <horms@xxxxxxxxxx>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
net/sched: tbf: add extack to offload params

Drivers might have error messages to propagate to user space. Propagate
the netlink extack so that they can inform user space in a verbal way of
their limitations.

> diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
> index f2340164f579a..4576111fe0758 100644
> --- a/net/sched/sch_tbf.c
> +++ b/net/sched/sch_tbf.c
[ ... ]
> @@ -193,6 +197,7 @@ static void tbf_offload_graft(struct Qdisc *sch, struct Qdisc *new,
>  		.parent		= sch->parent,
>  		.child_handle	= new->handle,
>  		.command	= TC_TBF_GRAFT,
> +		.extack		= extack,
>  	};
>  
>  	qdisc_offload_graft_helper(qdisc_dev(sch), sch, new, old,

Will the driver's extack message be overwritten here?

Looking at qdisc_offload_graft_helper() in net/sched/sch_api.c, if the
driver returns an error and any qdisc is offloaded, it calls
NL_SET_ERR_MSG:

net/sched/sch_api.c:qdisc_offload_graft_helper() {
	err = dev->netdev_ops->ndo_setup_tc(dev, type, type_data);
	...
	if (any_qdisc_is_offloaded)
		NL_SET_ERR_MSG(extack, "Offloading graft operation failed.");
}

Does this unconditionally overwrite the specific error message just set
by the driver, defeating the patch's goal for graft operations?