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]
Date: Wed, 18 Oct 2023 17:11:48 +0200
From: Michal Schmidt <mschmidt@...hat.com>
To: "Drewek, Wojciech" <wojciech.drewek@...el.com>, Ivan Vecera <ivecera@...hat.com>
Cc: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>, 
	"Keller, Jacob E" <jacob.e.keller@...el.com>, "Brandeburg, Jesse" <jesse.brandeburg@...el.com>, 
	"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, 
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next] iavf: delete unused iavf_mac_info fields

On Wed, Oct 18, 2023 at 1:26 PM Drewek, Wojciech
<wojciech.drewek@...el.com> wrote:
> > -----Original Message-----
> > From: Michal Schmidt <mschmidt@...hat.com>
> > Sent: Wednesday, October 18, 2023 1:15 PM
> > To: intel-wired-lan@...ts.osuosl.org
> > Cc: Keller, Jacob E <jacob.e.keller@...el.com>; Drewek, Wojciech
> > <wojciech.drewek@...el.com>; Brandeburg, Jesse
> > <jesse.brandeburg@...el.com>; Nguyen, Anthony L
> > <anthony.l.nguyen@...el.com>; netdev@...r.kernel.org
> > Subject: [PATCH net-next] iavf: delete unused iavf_mac_info fields
> >
> > '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.
> >
> > Signed-off-by: Michal Schmidt <mschmidt@...hat.com>
>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
>
> Nice cleanup, I've seen similar unused fields in i40e as well.
> Any plans for i40e cleanup?

No, I am not planning to look into i40e cleanups in the near future.
Ivan might want to do that though. [Adding him to the thread]
Michal

> > ---
> >  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 */
> > --
> > 2.41.0
>


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ