netdev - Re: [PATCH v2 net-next 5/5] ionic: add lif_quiesce to wait for queue activity to stop

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190930163245.540577ea@cakuba.netronome.com>
Date:   Mon, 30 Sep 2019 16:32:45 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Shannon Nelson <snelson@...sando.io>
Cc:     netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH v2 net-next 5/5] ionic: add lif_quiesce to wait for
 queue activity to stop

On Mon, 30 Sep 2019 14:49:20 -0700, Shannon Nelson wrote:
> Even though we've already turned off the queue activity with
> the ionic_qcq_disable(), we need to wait for any device queues
> that are processing packets to drain down before we try to
> flush our packets and tear down the queues.
> 
> Signed-off-by: Shannon Nelson <snelson@...sando.io>

This one, in turn, seems like something that could cause
unpleasantness, and therefore Fixes: could be specified :)

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 372329389c84..fc4ab73bd608 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -242,6 +242,29 @@ static int ionic_qcq_disable(struct ionic_qcq *qcq)
>  	return ionic_adminq_post_wait(lif, &ctx);
>  }
>  
> +static int ionic_lif_quiesce(struct ionic_lif *lif)

Possibly static void?  As with most cleanup functions there isn't much
caller can do with the error..

> +{
> +	int err;

reverse xmas tree

> +	struct device *dev = lif->ionic->dev;
> +	struct ionic_admin_ctx ctx = {
> +		.work = COMPLETION_INITIALIZER_ONSTACK(ctx.work),
> +		.cmd.lif_setattr = {
> +			.opcode = IONIC_CMD_LIF_SETATTR,
> +			.attr = IONIC_LIF_ATTR_STATE,
> +			.index = lif->index,
> +			.state = IONIC_LIF_DISABLE
> +		},
> +	};
> +
> +	err = ionic_adminq_post_wait(lif, &ctx);
> +	if (err) {
> +		dev_err(dev, "failed to quiesce lif, error = %d\n", err);
> +		return err;
> +	}
> +
> +	return (0);

no parens needed here

> +}
> +
>  static void ionic_lif_qcq_deinit(struct ionic_lif *lif, struct ionic_qcq *qcq)
>  {
>  	struct ionic_dev *idev = &lif->ionic->idev;
> @@ -1589,6 +1612,7 @@ int ionic_stop(struct net_device *netdev)
>  	netif_tx_disable(netdev);
>  
>  	ionic_txrx_disable(lif);
> +	ionic_lif_quiesce(lif);
>  	ionic_txrx_deinit(lif);
>  	ionic_txrx_free(lif);
>