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: Tue, 28 Nov 2023 13:31:56 +0530
From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil@...adcom.com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, 
	edumazet@...gle.com, netdev@...r.kernel.org, Ivan Vecera <ivecera@...hat.com>, 
	Przemek Kitszel <przemyslaw.kitszel@...el.com>, Wojciech Drewek <wojciech.drewek@...el.com>, 
	Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com>
Subject: Re: [PATCH net-next 1/5] i40e: Delete unused and useless i40e_pf fields

On Tue, Nov 28, 2023 at 2:41 AM Tony Nguyen <anthony.l.nguyen@...el.com>
wrote:

> From: Ivan Vecera <ivecera@...hat.com>
>
> Removed fields:
> .fc_autoneg_status
>     Since commit c56999f94876 ("i40e/i40evf: Add set_fc and init of
>     FC settings") write-only and otherwise unused
> .eeprom_version
>     Write-only and otherwise unused
> .atr_sample_rate
>     Has only one possible value (I40E_DEFAULT_ATR_SAMPLE_RATE). Remove
>     it and replace its occurrences by I40E_DEFAULT_ATR_SAMPLE_RATE
> .adminq_work_limit
>     Has only one possible value (I40E_AQ_WORK_LIMIT). Remove it and
>     replace its occurrences by I40E_AQ_WORK_LIMIT
> .tx_sluggish_count
>     Unused, never written
> .pf_seid
>     Used to store VSI downlink seid and it is referenced only once
>     in the same codepath. There is no need to save it into i40e_pf.
>     Remove it and use downlink_seid directly in the mentioned log
>     message.
> .instance
>     Write only. Remove it as well as ugly static local variable
>     'pfs_found' in i40e_probe.
> .int_policy
> .switch_kobj
> .ptp_pps_work
> .ptp_extts1_work
> .ptp_pps_start
> .pps_delay
> .ptp_pin
> .override_q_count
>     All these unused at all
>
> Prior the patch:
> pahole -Ci40e_pf drivers/net/ethernet/intel/i40e/i40e.ko | tail -5
>         /* size: 5368, cachelines: 84, members: 127 */
>         /* sum members: 5297, holes: 20, sum holes: 71 */
>         /* paddings: 6, sum paddings: 19 */
>         /* last cacheline: 56 bytes */
> };
>
> After the patch:
> pahole -Ci40e_pf drivers/net/ethernet/intel/i40e/i40e.ko | tail -5
>         /* size: 4976, cachelines: 78, members: 112 */
>         /* sum members: 4905, holes: 17, sum holes: 71 */
>         /* paddings: 6, sum paddings: 19 */
>         /* last cacheline: 48 bytes */
> };
>
> Signed-off-by: Ivan Vecera <ivecera@...hat.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com> (A
> Contingent worker at Intel)
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e.h         | 16 ----------------
>  drivers/net/ethernet/intel/i40e/i40e_debugfs.c |  3 ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c    | 18 ++++--------------
>  3 files changed, 4 insertions(+), 33 deletions(-)
>

Looks good to me.

Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@...adcom.com>

>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
> b/drivers/net/ethernet/intel/i40e/i40e.h
> index 6022944d04f8..9b701615c7c6 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -468,9 +468,7 @@ struct i40e_pf {
>         struct i40e_hw hw;
>         DECLARE_BITMAP(state, __I40E_STATE_SIZE__);
>         struct msix_entry *msix_entries;
> -       bool fc_autoneg_status;
>
> -       u16 eeprom_version;
>         u16 num_vmdq_vsis;         /* num vmdq vsis this PF has set up */
>         u16 num_vmdq_qps;          /* num queue pairs per vmdq pool */
>         u16 num_vmdq_msix;         /* num queue vectors per vmdq pool */
> @@ -486,7 +484,6 @@ struct i40e_pf {
>         u16 rss_size_max;          /* HW defined max RSS queues */
>         u16 fdir_pf_filter_count;  /* num of guaranteed filters for this
> PF */
>         u16 num_alloc_vsi;         /* num VSIs this driver supports */
> -       u8 atr_sample_rate;
>         bool wol_en;
>
>         struct hlist_head fdir_filter_list;
> @@ -524,12 +521,10 @@ struct i40e_pf {
>         struct hlist_head cloud_filter_list;
>         u16 num_cloud_filters;
>
> -       enum i40e_interrupt_policy int_policy;
>         u16 rx_itr_default;
>         u16 tx_itr_default;
>         u32 msg_enable;
>         char int_name[I40E_INT_NAME_STR_LEN];
> -       u16 adminq_work_limit; /* num of admin receive queue desc to
> process */
>         unsigned long service_timer_period;
>         unsigned long service_timer_previous;
>         struct timer_list service_timer;
> @@ -543,7 +538,6 @@ struct i40e_pf {
>         u32 tx_timeout_count;
>         u32 tx_timeout_recovery_level;
>         unsigned long tx_timeout_last_recovery;
> -       u32 tx_sluggish_count;
>         u32 hw_csum_rx_error;
>         u32 led_status;
>         u16 corer_count; /* Core reset count */
> @@ -565,17 +559,13 @@ struct i40e_pf {
>         struct i40e_lump_tracking *irq_pile;
>
>         /* switch config info */
> -       u16 pf_seid;
>         u16 main_vsi_seid;
>         u16 mac_seid;
> -       struct kobject *switch_kobj;
>  #ifdef CONFIG_DEBUG_FS
>         struct dentry *i40e_dbg_pf;
>  #endif /* CONFIG_DEBUG_FS */
>         bool cur_promisc;
>
> -       u16 instance; /* A unique number per i40e_pf instance in the
> system */
> -
>         /* sr-iov config info */
>         struct i40e_vf *vf;
>         int num_alloc_vfs;      /* actual number of VFs allocated */
> @@ -669,9 +659,7 @@ struct i40e_pf {
>         unsigned long ptp_tx_start;
>         struct hwtstamp_config tstamp_config;
>         struct timespec64 ptp_prev_hw_time;
> -       struct work_struct ptp_pps_work;
>         struct work_struct ptp_extts0_work;
> -       struct work_struct ptp_extts1_work;
>         ktime_t ptp_reset_start;
>         struct mutex tmreg_lock; /* Used to protect the SYSTIME registers.
> */
>         u32 ptp_adj_mult;
> @@ -679,10 +667,7 @@ struct i40e_pf {
>         u32 tx_hwtstamp_skipped;
>         u32 rx_hwtstamp_cleared;
>         u32 latch_event_flags;
> -       u64 ptp_pps_start;
> -       u32 pps_delay;
>         spinlock_t ptp_rx_lock; /* Used to protect Rx timestamp registers.
> */
> -       struct ptp_pin_desc ptp_pin[3];
>         unsigned long latch_events[4];
>         bool ptp_tx;
>         bool ptp_rx;
> @@ -695,7 +680,6 @@ struct i40e_pf {
>         u32 fd_inv;
>         u16 phy_led_val;
>
> -       u16 override_q_count;
>         u16 last_sw_conf_flags;
>         u16 last_sw_conf_valid_flags;
>         /* List to keep previous DDP profiles to be rolled back in the
> future */
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> index 88240571721a..ef70ddbe9c2f 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> @@ -1028,9 +1028,6 @@ static ssize_t i40e_dbg_command_write(struct file
> *filp,
>                                  "emp reset count: %d\n", pf->empr_count);
>                         dev_info(&pf->pdev->dev,
>                                  "pf reset count: %d\n", pf->pfr_count);
> -                       dev_info(&pf->pdev->dev,
> -                                "pf tx sluggish count: %d\n",
> -                                pf->tx_sluggish_count);
>                 } else if (strncmp(&cmd_buf[5], "port", 4) == 0) {
>                         struct i40e_aqc_query_port_ets_config_resp
> *bw_data;
>                         struct i40e_dcbx_config *cfg =
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 7ded598a68e6..d71331a8a972 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -3465,7 +3465,7 @@ static int i40e_configure_tx_ring(struct i40e_ring
> *ring)
>
>         /* some ATR related tx ring init */
>         if (test_bit(I40E_FLAG_FD_ATR_ENA, vsi->back->flags)) {
> -               ring->atr_sample_rate = vsi->back->atr_sample_rate;
> +               ring->atr_sample_rate = I40E_DEFAULT_ATR_SAMPLE_RATE;
>                 ring->atr_count = 0;
>         } else {
>                 ring->atr_sample_rate = 0;
> @@ -10226,9 +10226,9 @@ static void i40e_clean_adminq_subtask(struct
> i40e_pf *pf)
>                                  opcode);
>                         break;
>                 }
> -       } while (i++ < pf->adminq_work_limit);
> +       } while (i++ < I40E_AQ_WORK_LIMIT);
>
> -       if (i < pf->adminq_work_limit)
> +       if (i < I40E_AQ_WORK_LIMIT)
>                 clear_bit(__I40E_ADMINQ_EVENT_PENDING, pf->state);
>
>         /* re-enable Admin queue interrupt cause */
> @@ -12769,7 +12769,6 @@ static int i40e_sw_init(struct i40e_pf *pf)
>         if ((pf->hw.func_caps.fd_filters_guaranteed > 0) ||
>             (pf->hw.func_caps.fd_filters_best_effort > 0)) {
>                 set_bit(I40E_FLAG_FD_ATR_ENA, pf->flags);
> -               pf->atr_sample_rate = I40E_DEFAULT_ATR_SAMPLE_RATE;
>                 if (test_bit(I40E_FLAG_MFP_ENA, pf->flags) &&
>                     pf->hw.num_partitions > 1)
>                         dev_info(&pf->pdev->dev,
> @@ -12815,7 +12814,6 @@ static int i40e_sw_init(struct i40e_pf *pf)
>                                         I40E_MAX_VF_COUNT);
>         }
>  #endif /* CONFIG_PCI_IOV */
> -       pf->eeprom_version = 0xDEAD;
>         pf->lan_veb = I40E_NO_VEB;
>         pf->lan_vsi = I40E_NO_VSI;
>
> @@ -14976,12 +14974,11 @@ static void i40e_setup_pf_switch_element(struct
> i40e_pf *pf,
>                  * the PF's VSI
>                  */
>                 pf->mac_seid = uplink_seid;
> -               pf->pf_seid = downlink_seid;
>                 pf->main_vsi_seid = seid;
>                 if (printconfig)
>                         dev_info(&pf->pdev->dev,
>                                  "pf_seid=%d main_vsi_seid=%d\n",
> -                                pf->pf_seid, pf->main_vsi_seid);
> +                                downlink_seid, pf->main_vsi_seid);
>                 break;
>         case I40E_SWITCH_ELEMENT_TYPE_PF:
>         case I40E_SWITCH_ELEMENT_TYPE_VF:
> @@ -15160,10 +15157,6 @@ static int i40e_setup_pf_switch(struct i40e_pf
> *pf, bool reinit, bool lock_acqui
>         /* fill in link information and enable LSE reporting */
>         i40e_link_event(pf);
>
> -       /* Initialize user-specific link properties */
> -       pf->fc_autoneg_status = ((pf->hw.phy.link_info.an_info &
> -                                 I40E_AQ_AN_COMPLETED) ? true : false);
> -
>         i40e_ptp_init(pf);
>
>         if (!lock_acquired)
> @@ -15637,7 +15630,6 @@ static int i40e_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>  #endif /* CONFIG_I40E_DCB */
>         struct i40e_pf *pf;
>         struct i40e_hw *hw;
> -       static u16 pfs_found;
>         u16 wol_nvm_bits;
>         char nvm_ver[32];
>         u16 link_status;
> @@ -15715,7 +15707,6 @@ static int i40e_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>         hw->bus.device = PCI_SLOT(pdev->devfn);
>         hw->bus.func = PCI_FUNC(pdev->devfn);
>         hw->bus.bus_id = pdev->bus->number;
> -       pf->instance = pfs_found;
>
>         /* Select something other than the 802.1ad ethertype for the
>          * switch to use internally and drop on ingress.
> @@ -15777,7 +15768,6 @@ static int i40e_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>         }
>         hw->aq.arq_buf_size = I40E_MAX_AQ_BUF_SIZE;
>         hw->aq.asq_buf_size = I40E_MAX_AQ_BUF_SIZE;
> -       pf->adminq_work_limit = I40E_AQ_WORK_LIMIT;
>
>         snprintf(pf->int_name, sizeof(pf->int_name) - 1,
>                  "%s-%s:misc",
> --
> 2.41.0
>
>
>

-- 
Regards,
Kalesh A P

Content of type "text/html" skipped

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4239 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ