On Sat, Jan 20, 2018 at 2:54 AM, Jiri Pirko <j...@resnulli.us> wrote: > 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'm afraid it doesn't stop there though :/ Even now if one install a filter on a netdev with offload abilities and no skip_* flag there is this: # tc filter add dev eth0 ingress bpf obj drop.o da Warning: TC offload is disabled on net device. I'm not sure we should warn the user if there are no skip_* flags, just because the device has *some* offload capabilities? Perhaps we should put the flags into tc_cls_common_offload and add a helper to only set extack if skip_sw is set.. > 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. Certainly, will do.
