netdev - Re: [PATCH net-next] iavf: delete unused iavf_mac

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: <a686004d-8a5e-409c-9071-237ded8df68e@intel.com>
Date: Wed, 18 Oct 2023 14:30:13 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Michal Schmidt <mschmidt@...hat.com>, <intel-wired-lan@...ts.osuosl.org>
CC: Wojciech Drewek <wojciech.drewek@...el.com>, Jesse Brandeburg
	<jesse.brandeburg@...el.com>, Tony Nguyen <anthony.l.nguyen@...el.com>,
	<netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] iavf: delete unused iavf_mac_info fields



On 10/18/2023 4:15 AM, Michal Schmidt wrote:
> 'san_addr' and 'mac_fcoeq' members of struct iavf_mac_info are unused.
> 'type' is write-only. Delete all three.
> 
> The function iavf_set_mac_type that sets 'type' also checks if the PCI
> vendor ID is Intel. This is unnecessary. Delete the whole function.
> 
> If in the future there's a need for the MAC type (or other PCI
> ID-dependent data), I would prefer to use .driver_data in iavf_pci_tbl[]
> for this purpose.
> 

I suspect a handful of information across the Intel drivers could belong
in .driver_data instead of the current method we've used.

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>

Thanks,
Jake

> Signed-off-by: Michal Schmidt <mschmidt@...hat.com>
> ---
>  drivers/net/ethernet/intel/iavf/iavf_common.c | 32 -------------------
>  drivers/net/ethernet/intel/iavf/iavf_main.c   |  5 ---
>  .../net/ethernet/intel/iavf/iavf_prototype.h  |  2 --
>  drivers/net/ethernet/intel/iavf/iavf_type.h   | 12 -------
>  4 files changed, 51 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_common.c b/drivers/net/ethernet/intel/iavf/iavf_common.c
> index 1afd761d8052..8091e6feca01 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_common.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_common.c
> @@ -6,38 +6,6 @@
>  #include "iavf_prototype.h"
>  #include <linux/avf/virtchnl.h>
>  
> -/**
> - * iavf_set_mac_type - Sets MAC type
> - * @hw: pointer to the HW structure
> - *
> - * This function sets the mac type of the adapter based on the
> - * vendor ID and device ID stored in the hw structure.
> - **/
> -enum iavf_status iavf_set_mac_type(struct iavf_hw *hw)
> -{
> -	enum iavf_status status = 0;
> -
> -	if (hw->vendor_id == PCI_VENDOR_ID_INTEL) {
> -		switch (hw->device_id) {
> -		case IAVF_DEV_ID_X722_VF:
> -			hw->mac.type = IAVF_MAC_X722_VF;
> -			break;
> -		case IAVF_DEV_ID_VF:
> -		case IAVF_DEV_ID_VF_HV:
> -		case IAVF_DEV_ID_ADAPTIVE_VF:
> -			hw->mac.type = IAVF_MAC_VF;
> -			break;
> -		default:
> -			hw->mac.type = IAVF_MAC_GENERIC;
> -			break;
> -		}
> -	} else {
> -		status = IAVF_ERR_DEVICE_NOT_SUPPORTED;
> -	}
> -
> -	return status;
> -}
> -
>  /**
>   * iavf_aq_str - convert AQ err code to a string
>   * @hw: pointer to the HW structure
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 768bec67825a..c862ebcd2e39 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -2363,11 +2363,6 @@ static void iavf_startup(struct iavf_adapter *adapter)
>  	/* driver loaded, probe complete */
>  	adapter->flags &= ~IAVF_FLAG_PF_COMMS_FAILED;
>  	adapter->flags &= ~IAVF_FLAG_RESET_PENDING;
> -	status = iavf_set_mac_type(hw);
> -	if (status) {
> -		dev_err(&pdev->dev, "Failed to set MAC type (%d)\n", status);
> -		goto err;
> -	}
>  
>  	ret = iavf_check_reset_complete(hw);
>  	if (ret) {
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_prototype.h b/drivers/net/ethernet/intel/iavf/iavf_prototype.h
> index 940cb4203fbe..4a48e6171405 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_prototype.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf_prototype.h
> @@ -45,8 +45,6 @@ enum iavf_status iavf_aq_set_rss_lut(struct iavf_hw *hw, u16 seid,
>  enum iavf_status iavf_aq_set_rss_key(struct iavf_hw *hw, u16 seid,
>  				     struct iavf_aqc_get_set_rss_key_data *key);
>  
> -enum iavf_status iavf_set_mac_type(struct iavf_hw *hw);
> -
>  extern struct iavf_rx_ptype_decoded iavf_ptype_lookup[];
>  
>  static inline struct iavf_rx_ptype_decoded decode_rx_desc_ptype(u8 ptype)
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_type.h b/drivers/net/ethernet/intel/iavf/iavf_type.h
> index 9f1f523807c4..2b6a207fa441 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_type.h
> +++ b/drivers/net/ethernet/intel/iavf/iavf_type.h
> @@ -69,15 +69,6 @@ enum iavf_debug_mask {
>   * the Firmware and AdminQ are intended to insulate the driver from most of the
>   * future changes, but these structures will also do part of the job.
>   */
> -enum iavf_mac_type {
> -	IAVF_MAC_UNKNOWN = 0,
> -	IAVF_MAC_XL710,
> -	IAVF_MAC_VF,
> -	IAVF_MAC_X722,
> -	IAVF_MAC_X722_VF,
> -	IAVF_MAC_GENERIC,
> -};
> -
>  enum iavf_vsi_type {
>  	IAVF_VSI_MAIN	= 0,
>  	IAVF_VSI_VMDQ1	= 1,
> @@ -110,11 +101,8 @@ struct iavf_hw_capabilities {
>  };
>  
>  struct iavf_mac_info {
> -	enum iavf_mac_type type;
>  	u8 addr[ETH_ALEN];
>  	u8 perm_addr[ETH_ALEN];
> -	u8 san_addr[ETH_ALEN];
> -	u16 max_fcoeq;
>  };
>  
>  /* PCI bus types */