[v2] net/i40e: fix Rx packet statistics
Checks
Commit Message
Some packets are discarded by the NIC because they are larger than
the MTU, these packets should be counted as "RX error" instead of
"RX packet", for example:
pkt1 = Ether()/IP()/Raw('x' * 1400)
pkt2 = Ether()/IP()/Raw('x' * 1500)
---------------- Forward statistics for port 0 -----------------
RX-packets: 2 RX-dropped: 0 RX-total: 2
TX-packets: 1 TX-dropped: 0 TX-total: 1
----------------------------------------------------------------
Here the packet pkt2 has been discarded, but still was counted
by "RX-packets"
The register 'GL_RXERR1' can count above discarded packets.
This patch adds reading and calculation of the 'GL_RXERR1' counter
when reporting DPDK statistics.
Fixes: f4a91c38b4ad ("i40e: add extended stats")
Cc: stable@dpdk.org
Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
---
drivers/net/i40e/i40e_ethdev.c | 16 +++++++++++++---
drivers/net/i40e/i40e_ethdev.h | 10 ++++++++++
2 files changed, 23 insertions(+), 3 deletions(-)
Comments
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Alvin Zhang
> Sent: Tuesday, September 28, 2021 11:23 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Guo, Junfeng
> <junfeng.guo@intel.com>; ktraynor@redhat.com
> Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zhang@intel.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] net/i40e: fix Rx packet statistics
>
> Some packets are discarded by the NIC because they are larger than the MTU,
> these packets should be counted as "RX error" instead of "RX packet", for
> example:
>
> pkt1 = Ether()/IP()/Raw('x' * 1400)
> pkt2 = Ether()/IP()/Raw('x' * 1500)
>
> ---------------- Forward statistics for port 0 -----------------
> RX-packets: 2 RX-dropped: 0 RX-total: 2
> TX-packets: 1 TX-dropped: 0 TX-total: 1
> ----------------------------------------------------------------
>
> Here the packet pkt2 has been discarded, but still was counted
> by "RX-packets"
>
> The register 'GL_RXERR1' can count above discarded packets.
> This patch adds reading and calculation of the 'GL_RXERR1' counter when
> reporting DPDK statistics.
>
> Fixes: f4a91c38b4ad ("i40e: add extended stats")
> Cc: stable@dpdk.org
>
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 16 +++++++++++++---
> drivers/net/i40e/i40e_ethdev.h | 10 ++++++++++
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7a2a828..7a207b2 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -532,7 +532,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf
> *pf,
> /* store statistics names and its offset in stats structure */ struct
> rte_i40e_xstats_name_off {
> char name[RTE_ETH_XSTATS_NAME_SIZE];
> - unsigned offset;
> + int offset;
> };
>
> static const struct rte_i40e_xstats_name_off rte_i40e_stats_strings[] = { @@
> -542,6 +542,8 @@ struct rte_i40e_xstats_name_off {
> {"rx_dropped_packets", offsetof(struct i40e_eth_stats, rx_discards)},
> {"rx_unknown_protocol_packets", offsetof(struct i40e_eth_stats,
> rx_unknown_protocol)},
> + {"rx_size_error_packets", offsetof(struct i40e_pf, rx_err1) -
> + offsetof(struct i40e_pf, stats)},
> {"tx_unicast_packets", offsetof(struct i40e_eth_stats, tx_unicast)},
> {"tx_multicast_packets", offsetof(struct i40e_eth_stats, tx_multicast)},
> {"tx_broadcast_packets", offsetof(struct i40e_eth_stats, tx_broadcast)},
> @@ -3238,6 +3240,10 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
> pf->offset_loaded,
> &os->eth.rx_unknown_protocol,
> &ns->eth.rx_unknown_protocol);
> + i40e_stat_update_48(hw, I40E_GL_RXERR1_H(hw->pf_id +
> I40E_MAX_VF),
> + I40E_GL_RXERR1_L(hw->pf_id + I40E_MAX_VF),
> + pf->offset_loaded, &pf->rx_err1_offset,
> + &pf->rx_err1);
> i40e_stat_update_48_in_64(hw, I40E_GLPRT_GOTCH(hw->port),
> I40E_GLPRT_GOTCL(hw->port),
> pf->offset_loaded, &os->eth.tx_bytes, @@ -3437,7
> +3443,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
> stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> pf->main_vsi->eth_stats.rx_multicast +
> pf->main_vsi->eth_stats.rx_broadcast -
> - pf->main_vsi->eth_stats.rx_discards;
> + pf->main_vsi->eth_stats.rx_discards -
> + pf->rx_err1;
> stats->opackets = ns->eth.tx_unicast +
> ns->eth.tx_multicast +
> ns->eth.tx_broadcast;
> @@ -3451,7 +3458,8 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
> pf->main_vsi->eth_stats.rx_discards;
> stats->ierrors = ns->crc_errors +
> ns->rx_length_errors + ns->rx_undersize +
> - ns->rx_oversize + ns->rx_fragments + ns->rx_jabber;
> + ns->rx_oversize + ns->rx_fragments + ns->rx_jabber +
> + pf->rx_err1;
>
> if (pf->vfs) {
> for (i = 0; i < pf->vf_num; i++) {
> @@ -6232,6 +6240,8 @@ struct i40e_vsi *
> memset(&pf->stats_offset, 0, sizeof(struct i40e_hw_port_stats));
> memset(&pf->internal_stats, 0, sizeof(struct i40e_eth_stats));
> memset(&pf->internal_stats_offset, 0, sizeof(struct i40e_eth_stats));
> + pf->rx_err1 = 0;
> + pf->rx_err1_offset = 0;
>
> ret = i40e_pf_get_switch_config(pf);
> if (ret != I40E_SUCCESS) {
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index cd6deab..846c8d4 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -19,6 +19,13 @@
> #include "base/i40e_type.h"
> #include "base/virtchnl.h"
>
> +#define I40E_GL_RXERR1_H(_i) (0x00318004 + ((_i) * 8))
> +/**
> + * _i=0...143,
> + * counters 0-127 are for the 128 VFs,
> + * counters 128-143 are for the 16 PFs
> + */
I assume above comment is for I40E_GL_RXERR1_H, so it should above it?
> +
> #define I40E_VLAN_TAG_SIZE 4
>
> #define I40E_AQ_LEN 32
> @@ -1134,6 +1141,9 @@ struct i40e_pf {
>
> struct i40e_hw_port_stats stats_offset;
> struct i40e_hw_port_stats stats;
> + u64 rx_err1; /* rxerr1 */
> + u64 rx_err1_offset;
> +
> /* internal packet statistics, it should be excluded from the total */
> struct i40e_eth_stats internal_stats_offset;
> struct i40e_eth_stats internal_stats;
> --
> 1.8.3.1
> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Tuesday, September 28, 2021 7:17 PM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Guo, Junfeng <junfeng.guo@intel.com>;
> ktraynor@redhat.com
> Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zhang@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx packet statistics
>
>
>
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Alvin Zhang
> > Sent: Tuesday, September 28, 2021 11:23 AM
> > To: Xing, Beilei <beilei.xing@intel.com>; Guo, Junfeng
> > <junfeng.guo@intel.com>; ktraynor@redhat.com
> > Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zhang@intel.com>;
> > stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix Rx packet statistics
> >
> > Some packets are discarded by the NIC because they are larger than the
> > MTU, these packets should be counted as "RX error" instead of "RX
> > packet", for
> > example:
> >
> > pkt1 = Ether()/IP()/Raw('x' * 1400)
> > pkt2 = Ether()/IP()/Raw('x' * 1500)
> >
> > ---------------- Forward statistics for port 0 -----------------
> > RX-packets: 2 RX-dropped: 0 RX-total: 2
> > TX-packets: 1 TX-dropped: 0 TX-total: 1
> > ----------------------------------------------------------------
> >
> > Here the packet pkt2 has been discarded, but still was counted
> > by "RX-packets"
> >
> > The register 'GL_RXERR1' can count above discarded packets.
> > This patch adds reading and calculation of the 'GL_RXERR1' counter
> > when reporting DPDK statistics.
> >
> > Fixes: f4a91c38b4ad ("i40e: add extended stats")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 16 +++++++++++++---
> > drivers/net/i40e/i40e_ethdev.h | 10 ++++++++++
> > 2 files changed, 23 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 7a2a828..7a207b2 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -532,7 +532,7 @@ static int i40e_sw_tunnel_filter_insert(struct
> > i40e_pf *pf,
> > /* store statistics names and its offset in stats structure */
> > struct rte_i40e_xstats_name_off { char
> > name[RTE_ETH_XSTATS_NAME_SIZE]; -unsigned offset;
> > +int offset;
> > };
> >
> > static const struct rte_i40e_xstats_name_off rte_i40e_stats_strings[]
> > = { @@
> > -542,6 +542,8 @@ struct rte_i40e_xstats_name_off {
> > {"rx_dropped_packets", offsetof(struct i40e_eth_stats, rx_discards)},
> > {"rx_unknown_protocol_packets", offsetof(struct i40e_eth_stats,
> > rx_unknown_protocol)},
> > +{"rx_size_error_packets", offsetof(struct i40e_pf, rx_err1) -
> > + offsetof(struct i40e_pf, stats)},
> > {"tx_unicast_packets", offsetof(struct i40e_eth_stats, tx_unicast)},
> > {"tx_multicast_packets", offsetof(struct i40e_eth_stats,
> > tx_multicast)}, {"tx_broadcast_packets", offsetof(struct
> > i40e_eth_stats, tx_broadcast)}, @@ -3238,6 +3240,10 @@ void
> > i40e_flex_payload_reg_set_default(struct
> > i40e_hw *hw)
> > pf->offset_loaded,
> > &os->eth.rx_unknown_protocol,
> > &ns->eth.rx_unknown_protocol);
> > +i40e_stat_update_48(hw, I40E_GL_RXERR1_H(hw->pf_id +
> > I40E_MAX_VF),
> > + I40E_GL_RXERR1_L(hw->pf_id + I40E_MAX_VF),
> > + pf->offset_loaded, &pf->rx_err1_offset,
> > + &pf->rx_err1);
> > i40e_stat_update_48_in_64(hw, I40E_GLPRT_GOTCH(hw->port),
> > I40E_GLPRT_GOTCL(hw->port),
> > pf->offset_loaded, &os->eth.tx_bytes, @@ -3437,7
> > +3443,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
> > stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> > pf->main_vsi->eth_stats.rx_multicast +
> > pf->main_vsi->eth_stats.rx_broadcast -
> > -pf->main_vsi->eth_stats.rx_discards;
> > +pf->main_vsi->eth_stats.rx_discards - rx_err1;
> > stats->opackets = ns->eth.tx_unicast + ns->eth.tx_multicast +
> > ns->eth.tx_broadcast; @@ -3451,7 +3458,8 @@ void
> > i40e_flex_payload_reg_set_default(struct
> > i40e_hw *hw)
> > pf->main_vsi->eth_stats.rx_discards;
> > stats->ierrors = ns->crc_errors +
> > ns->rx_length_errors + ns->rx_undersize +
> > -ns->rx_oversize + ns->rx_fragments + ns->rx_jabber;
> > +ns->rx_oversize + ns->rx_fragments + ns->rx_jabber +
> > +pf->rx_err1;
> >
> > if (pf->vfs) {
> > for (i = 0; i < pf->vf_num; i++) {
> > @@ -6232,6 +6240,8 @@ struct i40e_vsi * memset(&pf->stats_offset, 0,
> > sizeof(struct i40e_hw_port_stats)); memset(&pf->internal_stats, 0,
> > sizeof(struct i40e_eth_stats)); memset(&pf->internal_stats_offset, 0,
> > sizeof(struct i40e_eth_stats));
> > +pf->rx_err1 = 0;
> > +pf->rx_err1_offset = 0;
> >
> > ret = i40e_pf_get_switch_config(pf);
> > if (ret != I40E_SUCCESS) {
> > diff --git a/drivers/net/i40e/i40e_ethdev.h
> > b/drivers/net/i40e/i40e_ethdev.h index cd6deab..846c8d4 100644
> > --- a/drivers/net/i40e/i40e_ethdev.h
> > +++ b/drivers/net/i40e/i40e_ethdev.h
> > @@ -19,6 +19,13 @@
> > #include "base/i40e_type.h"
> > #include "base/virtchnl.h"
> >
> > +#define I40E_GL_RXERR1_H(_i)(0x00318004 + ((_i) * 8))
> > +/**
> > + * _i=0...143,
> > + * counters 0-127 are for the 128 VFs,
> > + * counters 128-143 are for the 16 PFs */
>
> I assume above comment is for I40E_GL_RXERR1_H, so it should above it?
Ok, I will update it in v2.
Thanks
>
> > +
> > #define I40E_VLAN_TAG_SIZE 4
> >
> > #define I40E_AQ_LEN 32
> > @@ -1134,6 +1141,9 @@ struct i40e_pf {
> >
> > struct i40e_hw_port_stats stats_offset; struct i40e_hw_port_stats
> > stats;
> > +u64 rx_err1;/* rxerr1 */
> > +u64 rx_err1_offset;
> > +
> > /* internal packet statistics, it should be excluded from the total
> > */ struct i40e_eth_stats internal_stats_offset; struct
> > i40e_eth_stats internal_stats;
> > --
> > 1.8.3.1
>
@@ -532,7 +532,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
/* store statistics names and its offset in stats structure */
struct rte_i40e_xstats_name_off {
char name[RTE_ETH_XSTATS_NAME_SIZE];
- unsigned offset;
+ int offset;
};
static const struct rte_i40e_xstats_name_off rte_i40e_stats_strings[] = {
@@ -542,6 +542,8 @@ struct rte_i40e_xstats_name_off {
{"rx_dropped_packets", offsetof(struct i40e_eth_stats, rx_discards)},
{"rx_unknown_protocol_packets", offsetof(struct i40e_eth_stats,
rx_unknown_protocol)},
+ {"rx_size_error_packets", offsetof(struct i40e_pf, rx_err1) -
+ offsetof(struct i40e_pf, stats)},
{"tx_unicast_packets", offsetof(struct i40e_eth_stats, tx_unicast)},
{"tx_multicast_packets", offsetof(struct i40e_eth_stats, tx_multicast)},
{"tx_broadcast_packets", offsetof(struct i40e_eth_stats, tx_broadcast)},
@@ -3238,6 +3240,10 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
pf->offset_loaded,
&os->eth.rx_unknown_protocol,
&ns->eth.rx_unknown_protocol);
+ i40e_stat_update_48(hw, I40E_GL_RXERR1_H(hw->pf_id + I40E_MAX_VF),
+ I40E_GL_RXERR1_L(hw->pf_id + I40E_MAX_VF),
+ pf->offset_loaded, &pf->rx_err1_offset,
+ &pf->rx_err1);
i40e_stat_update_48_in_64(hw, I40E_GLPRT_GOTCH(hw->port),
I40E_GLPRT_GOTCL(hw->port),
pf->offset_loaded, &os->eth.tx_bytes,
@@ -3437,7 +3443,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
pf->main_vsi->eth_stats.rx_multicast +
pf->main_vsi->eth_stats.rx_broadcast -
- pf->main_vsi->eth_stats.rx_discards;
+ pf->main_vsi->eth_stats.rx_discards -
+ pf->rx_err1;
stats->opackets = ns->eth.tx_unicast +
ns->eth.tx_multicast +
ns->eth.tx_broadcast;
@@ -3451,7 +3458,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
pf->main_vsi->eth_stats.rx_discards;
stats->ierrors = ns->crc_errors +
ns->rx_length_errors + ns->rx_undersize +
- ns->rx_oversize + ns->rx_fragments + ns->rx_jabber;
+ ns->rx_oversize + ns->rx_fragments + ns->rx_jabber +
+ pf->rx_err1;
if (pf->vfs) {
for (i = 0; i < pf->vf_num; i++) {
@@ -6232,6 +6240,8 @@ struct i40e_vsi *
memset(&pf->stats_offset, 0, sizeof(struct i40e_hw_port_stats));
memset(&pf->internal_stats, 0, sizeof(struct i40e_eth_stats));
memset(&pf->internal_stats_offset, 0, sizeof(struct i40e_eth_stats));
+ pf->rx_err1 = 0;
+ pf->rx_err1_offset = 0;
ret = i40e_pf_get_switch_config(pf);
if (ret != I40E_SUCCESS) {
@@ -19,6 +19,13 @@
#include "base/i40e_type.h"
#include "base/virtchnl.h"
+#define I40E_GL_RXERR1_H(_i) (0x00318004 + ((_i) * 8))
+/**
+ * _i=0...143,
+ * counters 0-127 are for the 128 VFs,
+ * counters 128-143 are for the 16 PFs
+ */
+
#define I40E_VLAN_TAG_SIZE 4
#define I40E_AQ_LEN 32
@@ -1134,6 +1141,9 @@ struct i40e_pf {
struct i40e_hw_port_stats stats_offset;
struct i40e_hw_port_stats stats;
+ u64 rx_err1; /* rxerr1 */
+ u64 rx_err1_offset;
+
/* internal packet statistics, it should be excluded from the total */
struct i40e_eth_stats internal_stats_offset;
struct i40e_eth_stats internal_stats;