netdev - Re: [PATCH net-next v2 03/19] bnxt_en: implement devlink dev reload driver

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YXwqKCJkDOfvUce8@unreal>
Date:   Fri, 29 Oct 2021 20:06:48 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Michael Chan <michael.chan@...adcom.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, kuba@...nel.org,
        edwin.peer@...adcom.com, gospo@...adcom.com, jiri@...dia.com
Subject: Re: [PATCH net-next v2 03/19] bnxt_en: implement devlink dev reload
 driver_reinit

On Fri, Oct 29, 2021 at 03:47:40AM -0400, Michael Chan wrote:
> From: Edwin Peer <edwin.peer@...adcom.com>
> 
> The RTNL lock must be held between down and up to prevent interleaving
> state changes, especially since external state changes might release
> and allocate different driver resource subsets that would otherwise
> need to be tracked and carefully handled. If the down function fails,
> then devlink will not call the corresponding up function, thus the
> lock is released in the down error paths.
> 
> v2: Don't use devlink_reload_disable() and devlink_reload_enable().
> Instead, check that the netdev is not in unregistered state before
> proceeding with reload.
> 
> Signed-off-by: Edwin Peer <edwin.peer@...adcom.com>
> Signed-Off-by: Michael Chan <michael.chan@...adcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 14 +--
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  5 +
>  .../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 97 +++++++++++++++++++
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.c |  2 -
>  drivers/net/ethernet/broadcom/bnxt/bnxt_ptp.h |  3 +
>  5 files changed, 110 insertions(+), 11 deletions(-)

<...>

> +static int bnxt_dl_reload_down(struct devlink *dl, bool netns_change,
> +			       enum devlink_reload_action action,
> +			       enum devlink_reload_limit limit,
> +			       struct netlink_ext_ack *extack)
> +{
> +	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
> +	int rc = 0;
> +
> +	switch (action) {
> +	case DEVLINK_RELOAD_ACTION_DRIVER_REINIT: {
> +		if (BNXT_PF(bp) && bp->pf.active_vfs) {
> +			NL_SET_ERR_MSG_MOD(extack,
> +					   "reload is unsupported when VFs are allocated\n");
> +			return -EOPNOTSUPP;
> +		}

This is racy against bnxt_sriov_enable and bnxt_sriov_configure

Thanks