Re: [PATCH net-next] netdevsim: Register and unregister devlink traps on probe/remove device

On Sun, Oct 24, 2021 at 3:35 PM Ido Schimmel [off-list ref] wrote:

On Sun, Oct 24, 2021 at 11:42:11AM +0300, Leon Romanovsky wrote:

quoted

From: Leon Romanovsky <leonro@nvidia.com>

Align netdevsim to be like all other physical devices that register and
unregister devlink traps during their probe and removal respectively.

No, this is incorrect. Out of the three drivers that support both reload
and traps, both netdevsim and mlxsw unregister the traps during reload.
Here is another report from syzkaller about mlxsw [1].

Please revert both 22849b5ea595 ("devlink: Remove not-executed trap
policer notifications") and 8bbeed485823 ("devlink: Remove not-executed
trap group notifications").

Could we also revert 82465bec3e97 ("devlink: Delete reload
enable/disable interface")? This interface is needed because bnxt_en
cannot reorder devlink last. If Leon had fully carried out the
re-ordering in our driver he would have introduced a udev
phys_port_name regression because of:

cda2cab0771 ("bnxt_en: Move devlink_register before registering netdev")

and:

ab178b058c4 ("bnxt: remove ndo_get_phys_port_name implementation")

I think this went unnoticed for bnxt_en, because Michael had not yet
posted our devlink reload patches, which presently rely on the reload
enable/disable API. Absent horrible kludges in reload down/up which
currently depends on the netdev, there doesn't appear to be a clean
way to resolve the circular dependency without the interlocks this API
provides.

I imagine other subtle regressions are lying in wait.

Regards,
Edwin Peer