Sun, Sep 06, 2020 at 05:44:28PM CEST, ido...@idosch.org wrote: >On Wed, Sep 02, 2020 at 06:32:12PM +0300, Aya Levin wrote:
[...] > >I understand how this struct allows you to re-use a lot of code between >per-device and per-port traps, but it's mainly enabled by the fact that >you use the same netlink commands for both per-device and per-port >traps. Is this OK? > >I see this is already done for health reporters, but it's inconsistent >with the devlink-param API: > >DEVLINK_CMD_PARAM_GET >DEVLINK_CMD_PARAM_SET >DEVLINK_CMD_PARAM_NEW >DEVLINK_CMD_PARAM_DEL > >DEVLINK_CMD_PORT_PARAM_GET >DEVLINK_CMD_PORT_PARAM_SET >DEVLINK_CMD_PORT_PARAM_NEW >DEVLINK_CMD_PORT_PARAM_DEL > >And also with the general device/port commands: > >DEVLINK_CMD_GET >DEVLINK_CMD_SET >DEVLINK_CMD_NEW >DEVLINK_CMD_DEL > >DEVLINK_CMD_PORT_GET >DEVLINK_CMD_PORT_SET >DEVLINK_CMD_PORT_NEW >DEVLINK_CMD_PORT_DEL > >Wouldn't it be cleaner to add new commands? > >DEVLINK_CMD_PORT_TRAP_GET >DEVLINK_CMD_PORT_TRAP_SET >DEVLINK_CMD_PORT_TRAP_NEW >DEVLINK_CMD_PORT_TRAP_DEL > >I think the health API is the exception in this case and therefore might >not be the best thing to mimic. IIUC, existing per-port health reporters >were exposed as per-device and later moved to be exposed as per-port >[1]: > >"This patchset comes to fix a design issue as some health reporters >report on errors and run recovery on device level while the actual >functionality is on port level. As for the current implemented devlink >health reporters it is relevant only to Tx and Rx reporters of mlx5, >which has only one port, so no real effect on functionality, but this >should be fixed before more drivers will use devlink health reporters." Yeah, this slipped trough my fingers unfortunatelly :/ But with introduction of per-port health reporters, we could introduce new commands, that would be no problem. Pity :/ > >[1] >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ac4cd4781eacd1fd185c85522e869bd5d3254b96 > >Since we still don't have per-port traps, we can design it better from >the start. I agree. Let's have a separate commands for per-port. [...]