* [Intel-wired-lan] [PATCH net v1] i40e: Fix dropped jumbo frames statistics
@ 2022-05-27 8:07 Jedrzej Jagielski
2022-05-27 12:11 ` Paul Menzel
2022-05-27 16:45 ` Tony Nguyen
0 siblings, 2 replies; 4+ messages in thread
From: Jedrzej Jagielski @ 2022-05-27 8:07 UTC (permalink / raw)
To: intel-wired-lan
From: Lukasz Cieplicki <lukaszx.cieplicki@intel.com>
Dropped packets caused by too large frames were
not include in dropped RX packets statistics.
Issue was caused by avoid reading of GL_RXERR1 register.
Fixed by reading GL_RXERR1 register for each interface.
Fixes: 41a9e55c89be ("i40e: add missing VSI statistics")
Signed-off-by: Lukasz Cieplicki <lukaszx.cieplicki@intel.com>
Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e.h | 15 ++++
drivers/net/ethernet/intel/i40e/i40e_main.c | 74 +++++++++++++++++++
.../net/ethernet/intel/i40e/i40e_register.h | 13 ++++
drivers/net/ethernet/intel/i40e/i40e_type.h | 1 +
4 files changed, 103 insertions(+)
diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 55c6bce5da61..897bd57e2eb4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -1091,6 +1091,21 @@ static inline void i40e_write_fd_input_set(struct i40e_pf *pf,
(u32)(val & 0xFFFFFFFFULL));
}
+/**
+ * i40e_get_pf_count - get PCI PF Count.
+ * @hw: pointer to a hw.
+ *
+ * Reports the function number of the highest PCI physical
+ * function plus 1 as it is loaded from the NVM.
+ *
+ * Return: PCI PF Count.
+ **/
+static inline int i40e_get_pf_count(struct i40e_hw *hw)
+{
+ return rd32(hw, I40E_GLGEN_PCIFCNCNT)
+ & I40E_GLGEN_PCIFCNCNT_PCIPFCNT_MASK;
+}
+
/* needed by i40e_ethtool.c */
int i40e_up(struct i40e_vsi *vsi);
void i40e_down(struct i40e_vsi *vsi);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 46bb1169a004..94410b49995b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -549,6 +549,49 @@ void i40e_pf_reset_stats(struct i40e_pf *pf)
pf->hw_csum_rx_error = 0;
}
+/**
+ * i40e_compute_pci_to_hw_id - compute index form PCI function.
+ * @vsi: ptr to the VSI to read from.
+ * @hw: ptr to the hardware info.
+ **/
+static u8 i40e_compute_pci_to_hw_id(struct i40e_vsi *vsi, struct i40e_hw *hw)
+{
+ int pf_count;
+
+ pf_count = i40e_get_pf_count(hw);
+
+ if (vsi->type == I40E_VSI_SRIOV)
+ return hw->port * BIT(7) / pf_count + vsi->vf_id;
+
+ return hw->port + BIT(7);
+}
+
+/**
+ * i40e_stat_update64 - read and update a 64 bit stat from the chip.
+ * @hw: ptr to the hardware info.
+ * @hireg: the high 32 bit reg to read.
+ * @loreg: the low 32 bit reg to read.
+ * @offset_loaded: has the initial offset been loaded yet.
+ * @offset: ptr to current offset value.
+ * @stat: ptr to the stat.
+ *
+ * Since the device stats are not reset at PFReset, they likely will not
+ * be zeroed when the driver starts. We'll save the first values read
+ * and use them as offsets to be subtracted from the raw values in order
+ * to report stats that count from zero.
+ **/
+static void i40e_stat_update64(struct i40e_hw *hw, u32 hireg, u32 loreg,
+ bool offset_loaded, u64 *offset, u64 *stat)
+{
+ u64 new_data;
+
+ new_data = rd64(hw, loreg);
+
+ if (!offset_loaded || new_data < *offset)
+ *offset = new_data;
+ *stat = new_data - *offset;
+}
+
/**
* i40e_stat_update48 - read and update a 48 bit stat from the chip
* @hw: ptr to the hardware info
@@ -620,6 +663,33 @@ static void i40e_stat_update_and_clear32(struct i40e_hw *hw, u32 reg, u64 *stat)
*stat += new_data;
}
+/**
+ * i40e_stats_update_rx_discards - update rx_discards.
+ * @vsi: ptr to the VSI to be updated.
+ * @hw: ptr to the hardware info.
+ * @stat_idx: VSI's stat_counter_idx.
+ * @offset_loaded: ptr to the VSI's stat_offsets_loaded.
+ * @stat_offset: ptr to stat_offset to store first read of specific register.
+ * @stat: ptr to VSI's stat to be updated.
+ **/
+static void i40e_stats_update_rx_discards(struct i40e_vsi *vsi,
+ struct i40e_hw *hw, int stat_idx, bool offset_loaded,
+ struct i40e_eth_stats *stat_offset,
+ struct i40e_eth_stats *stat)
+{
+ u64 rx_rdpc, rx_rxerr;
+
+ i40e_stat_update32(hw, I40E_GLV_RDPC(stat_idx), offset_loaded,
+ &stat_offset->rx_discards, &rx_rdpc);
+ i40e_stat_update64(hw,
+ I40E_GL_RXERR1H(i40e_compute_pci_to_hw_id(vsi, hw)),
+ I40E_GL_RXERR1L(i40e_compute_pci_to_hw_id(vsi, hw)),
+ offset_loaded, &stat_offset->rx_discards_other,
+ &rx_rxerr);
+
+ stat->rx_discards = rx_rdpc + rx_rxerr;
+}
+
/**
* i40e_update_eth_stats - Update VSI-specific ethernet statistics counters.
* @vsi: the VSI to be updated
@@ -679,6 +749,10 @@ void i40e_update_eth_stats(struct i40e_vsi *vsi)
I40E_GLV_BPTCL(stat_idx),
vsi->stat_offsets_loaded,
&oes->tx_broadcast, &es->tx_broadcast);
+
+ i40e_stats_update_rx_discards(vsi, hw, stat_idx,
+ vsi->stat_offsets_loaded, oes, es);
+
vsi->stat_offsets_loaded = true;
}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_register.h b/drivers/net/ethernet/intel/i40e/i40e_register.h
index 1908eed4fa5e..7339003aa17c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_register.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_register.h
@@ -211,6 +211,11 @@
#define I40E_GLGEN_MSRWD_MDIWRDATA_SHIFT 0
#define I40E_GLGEN_MSRWD_MDIRDDATA_SHIFT 16
#define I40E_GLGEN_MSRWD_MDIRDDATA_MASK I40E_MASK(0xFFFF, I40E_GLGEN_MSRWD_MDIRDDATA_SHIFT)
+#define I40E_GLGEN_PCIFCNCNT 0x001C0AB4 /* Reset: PCIR */
+#define I40E_GLGEN_PCIFCNCNT_PCIPFCNT_SHIFT 0
+#define I40E_GLGEN_PCIFCNCNT_PCIPFCNT_MASK I40E_MASK(0x1F, I40E_GLGEN_PCIFCNCNT_PCIPFCNT_SHIFT)
+#define I40E_GLGEN_PCIFCNCNT_PCIVFCNT_SHIFT 16
+#define I40E_GLGEN_PCIFCNCNT_PCIVFCNT_MASK I40E_MASK(0xFF, I40E_GLGEN_PCIFCNCNT_PCIVFCNT_SHIFT)
#define I40E_GLGEN_RSTAT 0x000B8188 /* Reset: POR */
#define I40E_GLGEN_RSTAT_DEVSTATE_SHIFT 0
#define I40E_GLGEN_RSTAT_DEVSTATE_MASK I40E_MASK(0x3, I40E_GLGEN_RSTAT_DEVSTATE_SHIFT)
@@ -643,6 +648,14 @@
#define I40E_VFQF_HKEY1_MAX_INDEX 12
#define I40E_VFQF_HLUT1(_i, _VF) (0x00220000 + ((_i) * 1024 + (_VF) * 4)) /* _i=0...15, _VF=0...127 */ /* Reset: CORER */
#define I40E_VFQF_HLUT1_MAX_INDEX 15
+#define I40E_GL_RXERR1H(_i) (0x00318004 + ((_i) * 8)) /* _i=0...143 */ /* Reset: CORER */
+#define I40E_GL_RXERR1H_MAX_INDEX 143
+#define I40E_GL_RXERR1H_RXERR1H_SHIFT 0
+#define I40E_GL_RXERR1H_RXERR1H_MASK I40E_MASK(0xFFFFFFFF, I40E_GL_RXERR1H_RXERR1H_SHIFT)
+#define I40E_GL_RXERR1L(_i) (0x00318000 + ((_i) * 8)) /* _i=0...143 */ /* Reset: CORER */
+#define I40E_GL_RXERR1L_MAX_INDEX 143
+#define I40E_GL_RXERR1L_RXERR1L_SHIFT 0
+#define I40E_GL_RXERR1L_RXERR1L_MASK I40E_MASK(0xFFFFFFFF, I40E_GL_RXERR1L_RXERR1L_SHIFT)
#define I40E_GLPRT_BPRCH(_i) (0x003005E4 + ((_i) * 8)) /* _i=0...3 */ /* Reset: CORER */
#define I40E_GLPRT_BPRCL(_i) (0x003005E0 + ((_i) * 8)) /* _i=0...3 */ /* Reset: CORER */
#define I40E_GLPRT_BPTCH(_i) (0x00300A04 + ((_i) * 8)) /* _i=0...3 */ /* Reset: CORER */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
index 36a4ca1ffb1a..7b3f30beb757 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
@@ -1172,6 +1172,7 @@ struct i40e_eth_stats {
u64 tx_broadcast; /* bptc */
u64 tx_discards; /* tdpc */
u64 tx_errors; /* tepc */
+ u64 rx_discards_other; /* rxerr1 */
};
/* Statistics collected per VEB per TC */
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Intel-wired-lan] [PATCH net v1] i40e: Fix dropped jumbo frames statistics
2022-05-27 8:07 [Intel-wired-lan] [PATCH net v1] i40e: Fix dropped jumbo frames statistics Jedrzej Jagielski
@ 2022-05-27 12:11 ` Paul Menzel
2022-05-31 9:13 ` Jagielski, Jedrzej
2022-05-27 16:45 ` Tony Nguyen
1 sibling, 1 reply; 4+ messages in thread
From: Paul Menzel @ 2022-05-27 12:11 UTC (permalink / raw)
To: intel-wired-lan
Dear Jedrzej, dear Lukasz,
Thank you for the patch. Some nits.
Am 27.05.22 um 10:07 schrieb Jedrzej Jagielski:
> From: Lukasz Cieplicki <lukaszx.cieplicki@intel.com>
>
> Dropped packets caused by too large frames were
> not include in dropped RX packets statistics.
include*d*
Please also reflow for 75 characters per line (also below, 7that means
no need to wrap lines after sentences, or separate paragraphs by exactly
one line).
> Issue was caused by avoid reading of GL_RXERR1 register.
s/was caused/is caused/
avoid*ing*? Maybe: by not reading the GL_RXERR1 register.
Also, maybe add the definition/description of the register to the commit
message.
> Fixed by reading GL_RXERR1 register for each interface.
Fix it by ?
How do you reproduce the issue? It?d great if you added it to the commit
message.
>
> Fixes: 41a9e55c89be ("i40e: add missing VSI statistics")
> Signed-off-by: Lukasz Cieplicki <lukaszx.cieplicki@intel.com>
> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 15 ++++
> drivers/net/ethernet/intel/i40e/i40e_main.c | 74 +++++++++++++++++++
> .../net/ethernet/intel/i40e/i40e_register.h | 13 ++++
> drivers/net/ethernet/intel/i40e/i40e_type.h | 1 +
> 4 files changed, 103 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
> index 55c6bce5da61..897bd57e2eb4 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -1091,6 +1091,21 @@ static inline void i40e_write_fd_input_set(struct i40e_pf *pf,
> (u32)(val & 0xFFFFFFFFULL));
> }
>
> +/**
> + * i40e_get_pf_count - get PCI PF Count.
count?
> + * @hw: pointer to a hw.
> + *
> + * Reports the function number of the highest PCI physical
> + * function plus 1 as it is loaded from the NVM.
> + *
> + * Return: PCI PF Count.
> + **/
> +static inline int i40e_get_pf_count(struct i40e_hw *hw)
Why not return `unsigned int` as `igb_rd32()` returns `u32`?
> +{
> + return rd32(hw, I40E_GLGEN_PCIFCNCNT)
> + & I40E_GLGEN_PCIFCNCNT_PCIPFCNT_MASK;
> +}
> +
> /* needed by i40e_ethtool.c */
> int i40e_up(struct i40e_vsi *vsi);
> void i40e_down(struct i40e_vsi *vsi);
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 46bb1169a004..94410b49995b 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -549,6 +549,49 @@ void i40e_pf_reset_stats(struct i40e_pf *pf)
> pf->hw_csum_rx_error = 0;
> }
>
> +/**
> + * i40e_compute_pci_to_hw_id - compute index form PCI function.
> + * @vsi: ptr to the VSI to read from.
> + * @hw: ptr to the hardware info.
> + **/
> +static u8 i40e_compute_pci_to_hw_id(struct i40e_vsi *vsi, struct i40e_hw *hw)
Why not return `unsigned int` [1]?
> +{
> + int pf_count;
> +
> + pf_count = i40e_get_pf_count(hw);
> +
> + if (vsi->type == I40E_VSI_SRIOV)
> + return hw->port * BIT(7) / pf_count + vsi->vf_id;
> +
> + return hw->port + BIT(7);
> +}
> +
> +/**
> + * i40e_stat_update64 - read and update a 64 bit stat from the chip.
> + * @hw: ptr to the hardware info.
> + * @hireg: the high 32 bit reg to read.
> + * @loreg: the low 32 bit reg to read.
> + * @offset_loaded: has the initial offset been loaded yet.
> + * @offset: ptr to current offset value.
> + * @stat: ptr to the stat.
> + *
> + * Since the device stats are not reset at PFReset, they likely will not
Can this be said for sure? Likely does not sound ensuring.
> + * be zeroed when the driver starts. We'll save the first values read
> + * and use them as offsets to be subtracted from the raw values in order
> + * to report stats that count from zero.
> + **/
> +static void i40e_stat_update64(struct i40e_hw *hw, u32 hireg, u32 loreg,
> + bool offset_loaded, u64 *offset, u64 *stat)
> +{
> + u64 new_data;
> +
> + new_data = rd64(hw, loreg);
> +
> + if (!offset_loaded || new_data < *offset)
> + *offset = new_data;
> + *stat = new_data - *offset;
> +}
> +
> /**
> * i40e_stat_update48 - read and update a 48 bit stat from the chip
> * @hw: ptr to the hardware info
> @@ -620,6 +663,33 @@ static void i40e_stat_update_and_clear32(struct i40e_hw *hw, u32 reg, u64 *stat)
> *stat += new_data;
> }
>
> +/**
> + * i40e_stats_update_rx_discards - update rx_discards.
> + * @vsi: ptr to the VSI to be updated.
> + * @hw: ptr to the hardware info.
> + * @stat_idx: VSI's stat_counter_idx.
> + * @offset_loaded: ptr to the VSI's stat_offsets_loaded.
> + * @stat_offset: ptr to stat_offset to store first read of specific register.
> + * @stat: ptr to VSI's stat to be updated.
> + **/
> +static void i40e_stats_update_rx_discards(struct i40e_vsi *vsi,
> + struct i40e_hw *hw, int stat_idx, bool offset_loaded,
> + struct i40e_eth_stats *stat_offset,
> + struct i40e_eth_stats *stat)
> +{
> + u64 rx_rdpc, rx_rxerr;
> +
> + i40e_stat_update32(hw, I40E_GLV_RDPC(stat_idx), offset_loaded,
> + &stat_offset->rx_discards, &rx_rdpc);
> + i40e_stat_update64(hw,
> + I40E_GL_RXERR1H(i40e_compute_pci_to_hw_id(vsi, hw)),
> + I40E_GL_RXERR1L(i40e_compute_pci_to_hw_id(vsi, hw)),
> + offset_loaded, &stat_offset->rx_discards_other,
> + &rx_rxerr);
> +
> + stat->rx_discards = rx_rdpc + rx_rxerr;
> +}
> +
> /**
> * i40e_update_eth_stats - Update VSI-specific ethernet statistics counters.
> * @vsi: the VSI to be updated
> @@ -679,6 +749,10 @@ void i40e_update_eth_stats(struct i40e_vsi *vsi)
> I40E_GLV_BPTCL(stat_idx),
> vsi->stat_offsets_loaded,
> &oes->tx_broadcast, &es->tx_broadcast);
> +
> + i40e_stats_update_rx_discards(vsi, hw, stat_idx,
> + vsi->stat_offsets_loaded, oes, es);
> +
> vsi->stat_offsets_loaded = true;
> }
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_register.h b/drivers/net/ethernet/intel/i40e/i40e_register.h
> index 1908eed4fa5e..7339003aa17c 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_register.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_register.h
> @@ -211,6 +211,11 @@
> #define I40E_GLGEN_MSRWD_MDIWRDATA_SHIFT 0
> #define I40E_GLGEN_MSRWD_MDIRDDATA_SHIFT 16
> #define I40E_GLGEN_MSRWD_MDIRDDATA_MASK I40E_MASK(0xFFFF, I40E_GLGEN_MSRWD_MDIRDDATA_SHIFT)
> +#define I40E_GLGEN_PCIFCNCNT 0x001C0AB4 /* Reset: PCIR */
> +#define I40E_GLGEN_PCIFCNCNT_PCIPFCNT_SHIFT 0
> +#define I40E_GLGEN_PCIFCNCNT_PCIPFCNT_MASK I40E_MASK(0x1F, I40E_GLGEN_PCIFCNCNT_PCIPFCNT_SHIFT)
> +#define I40E_GLGEN_PCIFCNCNT_PCIVFCNT_SHIFT 16
> +#define I40E_GLGEN_PCIFCNCNT_PCIVFCNT_MASK I40E_MASK(0xFF, I40E_GLGEN_PCIFCNCNT_PCIVFCNT_SHIFT)
> #define I40E_GLGEN_RSTAT 0x000B8188 /* Reset: POR */
> #define I40E_GLGEN_RSTAT_DEVSTATE_SHIFT 0
> #define I40E_GLGEN_RSTAT_DEVSTATE_MASK I40E_MASK(0x3, I40E_GLGEN_RSTAT_DEVSTATE_SHIFT)
> @@ -643,6 +648,14 @@
> #define I40E_VFQF_HKEY1_MAX_INDEX 12
> #define I40E_VFQF_HLUT1(_i, _VF) (0x00220000 + ((_i) * 1024 + (_VF) * 4)) /* _i=0...15, _VF=0...127 */ /* Reset: CORER */
> #define I40E_VFQF_HLUT1_MAX_INDEX 15
> +#define I40E_GL_RXERR1H(_i) (0x00318004 + ((_i) * 8)) /* _i=0...143 */ /* Reset: CORER */
> +#define I40E_GL_RXERR1H_MAX_INDEX 143
> +#define I40E_GL_RXERR1H_RXERR1H_SHIFT 0
> +#define I40E_GL_RXERR1H_RXERR1H_MASK I40E_MASK(0xFFFFFFFF, I40E_GL_RXERR1H_RXERR1H_SHIFT)
> +#define I40E_GL_RXERR1L(_i) (0x00318000 + ((_i) * 8)) /* _i=0...143 */ /* Reset: CORER */
> +#define I40E_GL_RXERR1L_MAX_INDEX 143
> +#define I40E_GL_RXERR1L_RXERR1L_SHIFT 0
> +#define I40E_GL_RXERR1L_RXERR1L_MASK I40E_MASK(0xFFFFFFFF, I40E_GL_RXERR1L_RXERR1L_SHIFT)
> #define I40E_GLPRT_BPRCH(_i) (0x003005E4 + ((_i) * 8)) /* _i=0...3 */ /* Reset: CORER */
> #define I40E_GLPRT_BPRCL(_i) (0x003005E0 + ((_i) * 8)) /* _i=0...3 */ /* Reset: CORER */
> #define I40E_GLPRT_BPTCH(_i) (0x00300A04 + ((_i) * 8)) /* _i=0...3 */ /* Reset: CORER */
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
> index 36a4ca1ffb1a..7b3f30beb757 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
> @@ -1172,6 +1172,7 @@ struct i40e_eth_stats {
> u64 tx_broadcast; /* bptc */
> u64 tx_discards; /* tdpc */
> u64 tx_errors; /* tepc */
> + u64 rx_discards_other; /* rxerr1 */
> };
>
> /* Statistics collected per VEB per TC */
Kind regards,
Paul
[1]: https://notabs.org/coding/smallIntsBigPenalty.htm
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Intel-wired-lan] [PATCH net v1] i40e: Fix dropped jumbo frames statistics
2022-05-27 8:07 [Intel-wired-lan] [PATCH net v1] i40e: Fix dropped jumbo frames statistics Jedrzej Jagielski
2022-05-27 12:11 ` Paul Menzel
@ 2022-05-27 16:45 ` Tony Nguyen
1 sibling, 0 replies; 4+ messages in thread
From: Tony Nguyen @ 2022-05-27 16:45 UTC (permalink / raw)
To: intel-wired-lan
On 5/27/2022 1:07 AM, Jedrzej Jagielski wrote:
> From: Lukasz Cieplicki <lukaszx.cieplicki@intel.com>
>
> +/**
> + * i40e_stats_update_rx_discards - update rx_discards.
> + * @vsi: ptr to the VSI to be updated.
> + * @hw: ptr to the hardware info.
> + * @stat_idx: VSI's stat_counter_idx.
> + * @offset_loaded: ptr to the VSI's stat_offsets_loaded.
> + * @stat_offset: ptr to stat_offset to store first read of specific register.
> + * @stat: ptr to VSI's stat to be updated.
> + **/
> +static void i40e_stats_update_rx_discards(struct i40e_vsi *vsi,
> + struct i40e_hw *hw, int stat_idx, bool offset_loaded,
> + struct i40e_eth_stats *stat_offset,
> + struct i40e_eth_stats *stat)
Checkpatch reports an alignment issue with this. I'd recommend using
this style to help resolve this:
static void
i40e_stats_update_rx_discards(struct i40e_vsi *vsi,
struct i40e_hw *hw, int stat_idx, ...
Look at the ice driver for more examples.
> +{
> + u64 rx_rdpc, rx_rxerr;
> +
> + i40e_stat_update32(hw, I40E_GLV_RDPC(stat_idx), offset_loaded,
> + &stat_offset->rx_discards, &rx_rdpc);
> + i40e_stat_update64(hw,
> + I40E_GL_RXERR1H(i40e_compute_pci_to_hw_id(vsi, hw)),
> + I40E_GL_RXERR1L(i40e_compute_pci_to_hw_id(vsi, hw)),
> + offset_loaded, &stat_offset->rx_discards_other,
> + &rx_rxerr);
> +
> + stat->rx_discards = rx_rdpc + rx_rxerr;
> +}
> +
> /**
> * i40e_update_eth_stats - Update VSI-specific ethernet statistics counters.
> * @vsi: the VSI to be updated
> @@ -679,6 +749,10 @@ void i40e_update_eth_stats(struct i40e_vsi *vsi)
> I40E_GLV_BPTCL(stat_idx),
> vsi->stat_offsets_loaded,
> &oes->tx_broadcast, &es->tx_broadcast);
> +
> + i40e_stats_update_rx_discards(vsi, hw, stat_idx,
> + vsi->stat_offsets_loaded, oes, es);
Alignment issue here as well.
> +
> vsi->stat_offsets_loaded = true;
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Intel-wired-lan] [PATCH net v1] i40e: Fix dropped jumbo frames statistics
2022-05-27 12:11 ` Paul Menzel
@ 2022-05-31 9:13 ` Jagielski, Jedrzej
0 siblings, 0 replies; 4+ messages in thread
From: Jagielski, Jedrzej @ 2022-05-31 9:13 UTC (permalink / raw)
To: intel-wired-lan
Hello Paul,
>Dear Jedrzej, dear Lukasz,
>
>
>Thank you for the patch. Some nits.
>
>Am 27.05.22 um 10:07 schrieb Jedrzej Jagielski:
>> From: Lukasz Cieplicki <lukaszx.cieplicki@intel.com>
>>
>> Dropped packets caused by too large frames were
>> not include in dropped RX packets statistics.
>
>include*d*
>
>Please also reflow for 75 characters per line (also below, 7that means
>no need to wrap lines after sentences, or separate paragraphs by exactly
>one line).
>
Sure, it will be fixed.
>> Issue was caused by avoid reading of GL_RXERR1 register.
>
>s/was caused/is caused/
>
>avoid*ing*? Maybe: by not reading the GL_RXERR1 register.
>
>Also, maybe add the definition/description of the register to the commit
>message.
Commit msg will be corrected. Brief description of that register
will be also added
>
>> Fixed by reading GL_RXERR1 register for each interface.
>
>Fix it by ?
>
>How do you reproduce the issue? It?d great if you added it to the commit
>message.
So repro steps will be added.
>
>>
>> Fixes: 41a9e55c89be ("i40e: add missing VSI statistics")
>> Signed-off-by: Lukasz Cieplicki <lukaszx.cieplicki@intel.com>
>> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e.h | 15 ++++
>> drivers/net/ethernet/intel/i40e/i40e_main.c | 74 +++++++++++++++++++
>> .../net/ethernet/intel/i40e/i40e_register.h | 13 ++++
>> drivers/net/ethernet/intel/i40e/i40e_type.h | 1 +
>> 4 files changed, 103 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
>> index 55c6bce5da61..897bd57e2eb4 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
>> @@ -1091,6 +1091,21 @@ static inline void i40e_write_fd_input_set(struct i40e_pf *pf,
>> (u32)(val & 0xFFFFFFFFULL));
>> }
>>
>> +/**
>> + * i40e_get_pf_count - get PCI PF Count.
>
>count?
>
>> + * @hw: pointer to a hw.
>> + *
>> + * Reports the function number of the highest PCI physical
>> + * function plus 1 as it is loaded from the NVM.
>> + *
>> + * Return: PCI PF Count.
>> + **/
>> +static inline int i40e_get_pf_count(struct i40e_hw *hw)
>
>Why not return `unsigned int` as `igb_rd32()` returns `u32`?
>
>> +{
>> + return rd32(hw, I40E_GLGEN_PCIFCNCNT)
>> + & I40E_GLGEN_PCIFCNCNT_PCIPFCNT_MASK;
>> +}
>> +
>> /* needed by i40e_ethtool.c */
>> int i40e_up(struct i40e_vsi *vsi);
>> void i40e_down(struct i40e_vsi *vsi);
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index 46bb1169a004..94410b49995b 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -549,6 +549,49 @@ void i40e_pf_reset_stats(struct i40e_pf *pf)
>> pf->hw_csum_rx_error = 0;
>> }
>>
>> +/**
>> + * i40e_compute_pci_to_hw_id - compute index form PCI function.
>> + * @vsi: ptr to the VSI to read from.
>> + * @hw: ptr to the hardware info.
>> + **/
>> +static u8 i40e_compute_pci_to_hw_id(struct i40e_vsi *vsi, struct i40e_hw *hw)
>
>Why not return `unsigned int` [1]?
Sure, I believe that in the both above cases types which you
proposed could be used and this is a good tweak.
>
>> +{
>> + int pf_count;
>> +
>> + pf_count = i40e_get_pf_count(hw);
>> +
>> + if (vsi->type == I40E_VSI_SRIOV)
>> + return hw->port * BIT(7) / pf_count + vsi->vf_id;
>> +
>> + return hw->port + BIT(7);
>> +}
>> +
>> +/**
>> + * i40e_stat_update64 - read and update a 64 bit stat from the chip.
>> + * @hw: ptr to the hardware info.
>> + * @hireg: the high 32 bit reg to read.
>> + * @loreg: the low 32 bit reg to read.
>> + * @offset_loaded: has the initial offset been loaded yet.
>> + * @offset: ptr to current offset value.
>> + * @stat: ptr to the stat.
>> + *
>> + * Since the device stats are not reset at PFReset, they likely will not
>
>Can this be said for sure? Likely does not sound ensuring.
>
>> + * be zeroed when the driver starts. We'll save the first values read
>> + * and use them as offsets to be subtracted from the raw values in order
>> + * to report stats that count from zero.
>> + **/
>> +static void i40e_stat_update64(struct i40e_hw *hw, u32 hireg, u32 loreg,
>> + bool offset_loaded, u64 *offset, u64 *stat)
>> +{
>> + u64 new_data;
>> +
>> + new_data = rd64(hw, loreg);
>> +
>> + if (!offset_loaded || new_data < *offset)
>> + *offset = new_data;
>> + *stat = new_data - *offset;
>> +}
>> +
>> /**
>> * i40e_stat_update48 - read and update a 48 bit stat from the chip
>> * @hw: ptr to the hardware info
>> @@ -620,6 +663,33 @@ static void i40e_stat_update_and_clear32(struct i40e_hw *hw, u32 reg, u64 *stat)
>> *stat += new_data;
>> }
>>
>> +/**
>> + * i40e_stats_update_rx_discards - update rx_discards.
>> + * @vsi: ptr to the VSI to be updated.
>> + * @hw: ptr to the hardware info.
>> + * @stat_idx: VSI's stat_counter_idx.
>> + * @offset_loaded: ptr to the VSI's stat_offsets_loaded.
>> + * @stat_offset: ptr to stat_offset to store first read of specific register.
>> + * @stat: ptr to VSI's stat to be updated.
>> + **/
>> +static void i40e_stats_update_rx_discards(struct i40e_vsi *vsi,
>> + struct i40e_hw *hw, int stat_idx, bool offset_loaded,
>> + struct i40e_eth_stats *stat_offset,
>> + struct i40e_eth_stats *stat)
>> +{
>> + u64 rx_rdpc, rx_rxerr;
>> +
>> + i40e_stat_update32(hw, I40E_GLV_RDPC(stat_idx), offset_loaded,
>> + &stat_offset->rx_discards, &rx_rdpc);
>> + i40e_stat_update64(hw,
>> + I40E_GL_RXERR1H(i40e_compute_pci_to_hw_id(vsi, hw)),
>> + I40E_GL_RXERR1L(i40e_compute_pci_to_hw_id(vsi, hw)),
>> + offset_loaded, &stat_offset->rx_discards_other,
>> + &rx_rxerr);
>> +
>> + stat->rx_discards = rx_rdpc + rx_rxerr;
>> +}
>> +
>> /**
>> * i40e_update_eth_stats - Update VSI-specific ethernet statistics counters.
>> * @vsi: the VSI to be updated
>> @@ -679,6 +749,10 @@ void i40e_update_eth_stats(struct i40e_vsi *vsi)
>> I40E_GLV_BPTCL(stat_idx),
>> vsi->stat_offsets_loaded,
>> &oes->tx_broadcast, &es->tx_broadcast);
>> +
>> + i40e_stats_update_rx_discards(vsi, hw, stat_idx,
>> + vsi->stat_offsets_loaded, oes, es);
>> +
>> vsi->stat_offsets_loaded = true;
>> }
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_register.h b/drivers/net/ethernet/intel/i40e/i40e_register.h
>> index 1908eed4fa5e..7339003aa17c 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_register.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_register.h
>> @@ -211,6 +211,11 @@
>> #define I40E_GLGEN_MSRWD_MDIWRDATA_SHIFT 0
>> #define I40E_GLGEN_MSRWD_MDIRDDATA_SHIFT 16
>> #define I40E_GLGEN_MSRWD_MDIRDDATA_MASK I40E_MASK(0xFFFF, I40E_GLGEN_MSRWD_MDIRDDATA_SHIFT)
>> +#define I40E_GLGEN_PCIFCNCNT 0x001C0AB4 /* Reset: PCIR */
>> +#define I40E_GLGEN_PCIFCNCNT_PCIPFCNT_SHIFT 0
>> +#define I40E_GLGEN_PCIFCNCNT_PCIPFCNT_MASK I40E_MASK(0x1F, I40E_GLGEN_PCIFCNCNT_PCIPFCNT_SHIFT)
>> +#define I40E_GLGEN_PCIFCNCNT_PCIVFCNT_SHIFT 16
>> +#define I40E_GLGEN_PCIFCNCNT_PCIVFCNT_MASK I40E_MASK(0xFF, I40E_GLGEN_PCIFCNCNT_PCIVFCNT_SHIFT)
>> #define I40E_GLGEN_RSTAT 0x000B8188 /* Reset: POR */
>> #define I40E_GLGEN_RSTAT_DEVSTATE_SHIFT 0
>> #define I40E_GLGEN_RSTAT_DEVSTATE_MASK I40E_MASK(0x3, I40E_GLGEN_RSTAT_DEVSTATE_SHIFT)
>> @@ -643,6 +648,14 @@
>> #define I40E_VFQF_HKEY1_MAX_INDEX 12
>> #define I40E_VFQF_HLUT1(_i, _VF) (0x00220000 + ((_i) * 1024 + (_VF) * 4)) /* _i=0...15, _VF=0...127 */ /* Reset: CORER */
>> #define I40E_VFQF_HLUT1_MAX_INDEX 15
>> +#define I40E_GL_RXERR1H(_i) (0x00318004 + ((_i) * 8)) /* _i=0...143 */ /* Reset: CORER */
>> +#define I40E_GL_RXERR1H_MAX_INDEX 143
>> +#define I40E_GL_RXERR1H_RXERR1H_SHIFT 0
>> +#define I40E_GL_RXERR1H_RXERR1H_MASK I40E_MASK(0xFFFFFFFF, I40E_GL_RXERR1H_RXERR1H_SHIFT)
>> +#define I40E_GL_RXERR1L(_i) (0x00318000 + ((_i) * 8)) /* _i=0...143 */ /* Reset: CORER */
>> +#define I40E_GL_RXERR1L_MAX_INDEX 143
>> +#define I40E_GL_RXERR1L_RXERR1L_SHIFT 0
>> +#define I40E_GL_RXERR1L_RXERR1L_MASK I40E_MASK(0xFFFFFFFF, I40E_GL_RXERR1L_RXERR1L_SHIFT)
>> #define I40E_GLPRT_BPRCH(_i) (0x003005E4 + ((_i) * 8)) /* _i=0...3 */ /* Reset: CORER */
>> #define I40E_GLPRT_BPRCL(_i) (0x003005E0 + ((_i) * 8)) /* _i=0...3 */ /* Reset: CORER */
>> #define I40E_GLPRT_BPTCH(_i) (0x00300A04 + ((_i) * 8)) /* _i=0...3 */ /* Reset: CORER */
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h b/drivers/net/ethernet/intel/i40e/i40e_type.h
>> index 36a4ca1ffb1a..7b3f30beb757 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_type.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
>> @@ -1172,6 +1172,7 @@ struct i40e_eth_stats {
>> u64 tx_broadcast; /* bptc */
>> u64 tx_discards; /* tdpc */
>> u64 tx_errors; /* tepc */
>> + u64 rx_discards_other; /* rxerr1 */
>> };
>>
>> /* Statistics collected per VEB per TC */
>
>
>Kind regards,
>
>Paul
Thanks for your suggestions.
Best regards,
Jedrzej
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-31 9:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-27 8:07 [Intel-wired-lan] [PATCH net v1] i40e: Fix dropped jumbo frames statistics Jedrzej Jagielski
2022-05-27 12:11 ` Paul Menzel
2022-05-31 9:13 ` Jagielski, Jedrzej
2022-05-27 16:45 ` Tony Nguyen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox