On Mon, 2023-11-27 at 17:43 -0800, Jakub Kicinski wrote: > On Mon, 27 Nov 2023 16:15:47 -0800 Zhang, Xuejun wrote: > > This is extension of ndo_set_tx_maxrate to include per queue parameters > > of tx_minrate and burst. > > > > devlink rate api includes tx_maxrate and tx_minrate, it is intended for > > port rate configurations. > > > > With regarding to tc mqprio, it is being used to configure queue group > > per tc. > > > > For sriov ndo ndo_set_vf_rate, that has been used for overall VF rate > > configuration, not for queue based rate configuration. > > > > It seems there are differences on intent of the aforementioned APIs. > > > > Our use case here is to allow user (i.e @ uAPI) to configure tx rates of > > max rate & min rate per VF queue.Hence we are inclined to > > ndo_set_tx_maxrate extension. > > I said: > > So since you asked for my opinion - my opinion is that step 1 is to > create a common representation of what we already have and feed it > to the drivers via a single interface. I could just be taking sysfs > maxrate and feeding it to the driver via the devlink rate interface. > If we have the right internals I give 0 cares about what uAPI you pick. > > https://lore.kernel.org/all/20231118084843.70c34...@kernel.org/ > > Again, the first step is creating a common kernel <> driver interface > which can be used to send to the driver the configuration from the > existing 4 interfaces.
Together with Simon, I spent some time on the above. We think the ndo_setup_tc(TC_SETUP_QDISC_TBF) hook could be used as common basis for this offloads, with some small extensions (adding a 'max_rate' param, too). The idea would be: - 'fixing' sch_btf so that the s/w path became a no-op when h/w offload is enabled - extend sch_btf to support max rate - do the relevant ice implementation - ndo_set_tx_maxrate could be replaced with the mentioned ndo call (the latter interface is a strict super-set of former) - ndo_set_vf_rate could also be replaced with the mentioned ndo call (with another small extension to the offload data) I think mqprio deserves it's own separate offload interface, as it covers multiple tasks other than shaping (grouping queues and mapping priority to classes) In the long run we could have a generic implementation of the ndo_setup_tc(TC_SETUP_QDISC_TBF) in term of devlink rate adding a generic way to fetch the devlink_port instance corresponding to the given netdev and mapping the TBF features to the devlink_rate API. Not starting this due to what Jiri mentioned [1]. WDYT? Thanks, Paolo and Simon [1] https://lore.kernel.org/netdev/ZORRzEBcUDEjMniz@nanopsycho/ _______________________________________________ Intel-wired-lan mailing list Intel-wired-lan@osuosl.org https://lists.osuosl.org/mailman/listinfo/intel-wired-lan