lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 10 Sep 2020 09:16:41 +0300 From: Aya Levin <ayal@...dia.com> To: Jiri Pirko <jiri@...nulli.us>, Ido Schimmel <idosch@...sch.org> CC: Aya Levin <ayal@...lanox.com>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...lanox.com>, <netdev@...r.kernel.org>, Moshe Shemesh <moshe@...lanox.com>, Eran Ben Elisha <eranbe@...lanox.com>, Ido Schimmel <idosch@...lanox.com>, <linux-kernel@...r.kernel.org> Subject: Re: [PATCH net-next RFC v1 2/4] devlink: Add devlink traps under devlink_ports context On 9/8/2020 5:04 PM, Jiri Pirko wrote: > Sun, Sep 06, 2020 at 05:44:28PM CEST, idosch@...sch.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. Thanks for your input I'm preparing V2 > > > [...] >
Powered by blists - more mailing lists