netdev - Re: [PATCH net-next v7] net: txgbe: Add build support for txgbe

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: <20220621223716.5b936d93@kernel.org>
Date:   Tue, 21 Jun 2022 22:37:16 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jiawen Wu <jiawenwu@...stnetic.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH net-next v7] net: txgbe: Add build support for txgbe

On Tue, 21 Jun 2022 10:32:09 +0800 Jiawen Wu wrote:
> +	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
> +	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
> +		pci_using_dac = 1;

dma_set_mask_and_coherent() ...

> +	} else {
> +		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));

... but really this entire dance is unnecessary. Please see commit
c38f30683956 ("vmxnet3: Remove useless DMA-32 fallback configuration")
for references to why that's the case.

> +		if (err) {
> +			err = dma_set_coherent_mask(&pdev->dev,
> +						    DMA_BIT_MASK(32));
> +			if (err) {
> +				dev_err(&pdev->dev,
> +					"No usable DMA configuration, aborting\n");
> +				goto err_dma;
> +			}
> +		}
> +		pci_using_dac = 0;
> +	}
> +
> +	err = pci_request_selected_regions(pdev,
> +					   pci_select_bars(pdev, IORESOURCE_MEM),
> +					   txgbe_driver_name);
> +	if (err) {
> +		dev_err(&pdev->dev,
> +			"pci_request_selected_regions failed 0x%x\n", err);
> +		goto err_dma;
> +	}
> +
> +	pci_enable_pcie_error_reporting(pdev);
> +	pci_set_master(pdev);
> +
> +	netdev = devm_alloc_etherdev_mqs(&pdev->dev,
> +					 sizeof(struct txgbe_adapter),
> +					 TXGBE_MAX_TX_QUEUES,
> +					 TXGBE_MAX_RX_QUEUES);
> +	if (!netdev) {
> +		err = -ENOMEM;
> +		goto err_alloc_etherdev;
> +	}
> +
> +	SET_NETDEV_DEV(netdev, &pdev->dev);
> +
> +	adapter = netdev_priv(netdev);
> +	adapter->netdev = netdev;
> +	adapter->pdev = pdev;
> +
> +	adapter->io_addr = devm_ioremap(&pdev->dev,
> +					pci_resource_start(pdev, 0),
> +					pci_resource_len(pdev, 0));
> +	if (!adapter->io_addr) {
> +		err = -EIO;
> +		goto err_alloc_etherdev;

See, now this jumps to err_alloc_etherdev which I can't wrap my head
around. Please name the labels after the target, not the source...

> +	}
> +
> +	if (pci_using_dac)
> +		netdev->features |= NETIF_F_HIGHDMA;
> +
> +	pci_set_drvdata(pdev, adapter);
> +
> +	return 0;
> +
> +err_alloc_etherdev:

... so for example:

err_pci_release_regions:

> +	pci_release_selected_regions(pdev,
> +				     pci_select_bars(pdev, IORESOURCE_MEM));
> +err_dma:

and

err_pci_disable_dev:

> +	pci_disable_device(pdev);
> +	return err;