Sat, Jan 20, 2018 at 11:33:31AM CET, jakub.kicin...@netronome.com wrote: >On Sat, Jan 20, 2018 at 2:22 AM, Jiri Pirko <j...@resnulli.us> wrote: >> Sat, Jan 20, 2018 at 11:12:25AM CET, jakub.kicin...@netronome.com wrote: >>>net/sched/sch_prio.c: if (!tc_can_offload(dev) || >>>!dev->netdev_ops->ndo_setup_tc) >>>net/sched/sch_prio.c: if (!tc_can_offload(dev) || >>>!dev->netdev_ops->ndo_setup_tc) >>>net/sched/sch_red.c: if (!tc_can_offload(dev) || >>>!dev->netdev_ops->ndo_setup_tc) >>>net/sched/sch_red.c: if (!tc_can_offload(dev) || >>>!dev->netdev_ops->ndo_setup_tc) >>> >>>Do you mean the qdisc offloads too? The whole lot? >> >> Yes. > >Actually looking at the qdisc code and destroy callbacks, if we plumb >it through everywhere won't that mean user will see error messages on >destroy of qdiscs/filters which were never offloaded? > >Just looking at prio_offload() as a simple example. prio_destroy() >will always call tc_can_offload().
Hmmm. You are right. Either we pass null from there (NL_SET_ERR_MSG can cope), or we leave tc_can_offload helper as is and let the caller to set the extack. I see that tc_can_offload_extack is probably good idea then. But please use it in all drivers that are calling tc_can_offload so the user experience is consistent for all. Thanks!
