| Message ID | 20240930110205.44278-1-minda.chen@starfivetech.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v2] net: stmmac: dwmac4: Add ip payload error statistics | expand |
On Mon, Sep 30, 2024 at 07:02:05PM +0800, Minda Chen wrote: > Add dwmac4 ip payload error statistics, and rename discripter bit macro > because latest version descriptor IPCE bit claims ip checksum error or > l4 segment length error. > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com> Reviewed-by: Simon Horman <horms@kernel.org>
On Mon, 30 Sep 2024 19:02:05 +0800 Minda Chen wrote: > Add dwmac4 ip payload error statistics, and rename discripter bit macro descriptor ^ > because latest version descriptor IPCE bit claims ip checksum error or > l4 segment length error. What is an L4 segment length error on Rx? Seems to me that reusing ip_payload_err here will be confusing
Hi Minda On Mon, Sep 30, 2024 at 07:02:05PM GMT, Minda Chen wrote: Since v3 is going to be required anyway, here are several nitpicks: > Add dwmac4 ip payload error statistics, and rename discripter bit macro > because latest version descriptor IPCE bit claims ip checksum error or > l4 segment length error. s/dwmac4/DW QoS Eth v4/v5 s/ip/IP L4-segment is a too broad definition in this case. The doc says about just three protocols: TCP, UDP, or ICMP, so s/l4/TCP, UDP, or ICMP Other than that the change looks good. Reviewed-by: Serge Semin <fancer.lancer@gmail.com> -Serge(y) > > Signed-off-by: Minda Chen <minda.chen@starfivetech.com> > --- > drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 2 ++ > drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c > index e99401bcc1f8..a5fb31eb0192 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c > @@ -118,6 +118,8 @@ static int dwmac4_wrback_get_rx_status(struct stmmac_extra_stats *x, > x->ipv4_pkt_rcvd++; > if (rdes1 & RDES1_IPV6_HEADER) > x->ipv6_pkt_rcvd++; > + if (rdes1 & RDES1_IP_PAYLOAD_ERROR) > + x->ip_payload_err++; > > if (message_type == RDES_EXT_NO_PTP) > x->no_ptp_rx_msg_type_ext++; > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h > index 6da070ccd737..1ce6f43d545a 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h > @@ -95,7 +95,7 @@ > #define RDES1_IPV4_HEADER BIT(4) > #define RDES1_IPV6_HEADER BIT(5) > #define RDES1_IP_CSUM_BYPASSED BIT(6) > -#define RDES1_IP_CSUM_ERROR BIT(7) > +#define RDES1_IP_PAYLOAD_ERROR BIT(7) > #define RDES1_PTP_MSG_TYPE_MASK GENMASK(11, 8) > #define RDES1_PTP_PACKET_TYPE BIT(12) > #define RDES1_PTP_VER BIT(13) > -- > 2.17.1 > >
Hi Jakub On Wed, Oct 02, 2024 at 06:58:01AM GMT, Jakub Kicinski wrote: > On Mon, 30 Sep 2024 19:02:05 +0800 Minda Chen wrote: > > Add dwmac4 ip payload error statistics, and rename discripter bit macro > > descriptor > ^ > > > because latest version descriptor IPCE bit claims ip checksum error or > > l4 segment length error. > > What is an L4 segment length error on Rx? > Seems to me that reusing ip_payload_err here will be confusing From the current "ip_payload_err" field semantics, Minda is correct to use it for the Rx IP-payload error statistics. Here is the definition of the IPCE flag (part of the RDES4 descriptor field) cited from the Synopsys DW QoS Eth v5 HW-manual: Bit Name Description 7 IPCE IP Payload Error When this bit is set, it indicates either of the following: ■ The 16-bit IP payload checksum (that is, the TCP, UDP, or ICMP checksum) calculated by the MAC does not match the corresponding checksum field in the received segment. ■ The TCP, UDP, or ICMP segment length does not match the payload length value in the IP Header field. ■ The TCP, UDP, or ICMP segment length is less than minimum allowed segment length for TCP, UDP, or ICMP. It almost word-by-word matches to what is defined for the ERDES4_IP_PAYLOAD_ERR flag (part of the Extended RDES4 descriptor field) in DW GMAC v3.x HW-manual for which the stmmac_stats::ip_payload_err field has been added in the first place. Note the name of the flag in the descriptor matches to what is declared in the stmmac_stats structure. So based on that I don't see any problem with the patch except some minor patch-log issues I mentioned in another message. -Serge(y) > -- > pw-bot: cr >
On Wed, 2 Oct 2024 21:35:30 +0300 Serge Semin wrote: > It almost word-by-word matches to what is defined for the > ERDES4_IP_PAYLOAD_ERR flag (part of the Extended RDES4 descriptor > field) in DW GMAC v3.x HW-manual for which the > stmmac_stats::ip_payload_err field has been added in the first place. > Note the name of the flag in the descriptor matches to what is declared in > the stmmac_stats structure. I misread the ERDES4_IP_PAYLOAD_ERR as a Tx flag, somehow. All good then.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c index e99401bcc1f8..a5fb31eb0192 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c @@ -118,6 +118,8 @@ static int dwmac4_wrback_get_rx_status(struct stmmac_extra_stats *x, x->ipv4_pkt_rcvd++; if (rdes1 & RDES1_IPV6_HEADER) x->ipv6_pkt_rcvd++; + if (rdes1 & RDES1_IP_PAYLOAD_ERROR) + x->ip_payload_err++; if (message_type == RDES_EXT_NO_PTP) x->no_ptp_rx_msg_type_ext++; diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h index 6da070ccd737..1ce6f43d545a 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h @@ -95,7 +95,7 @@ #define RDES1_IPV4_HEADER BIT(4) #define RDES1_IPV6_HEADER BIT(5) #define RDES1_IP_CSUM_BYPASSED BIT(6) -#define RDES1_IP_CSUM_ERROR BIT(7) +#define RDES1_IP_PAYLOAD_ERROR BIT(7) #define RDES1_PTP_MSG_TYPE_MASK GENMASK(11, 8) #define RDES1_PTP_PACKET_TYPE BIT(12) #define RDES1_PTP_VER BIT(13)
Add dwmac4 ip payload error statistics, and rename discripter bit macro because latest version descriptor IPCE bit claims ip checksum error or l4 segment length error. Signed-off-by: Minda Chen <minda.chen@starfivetech.com> --- drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c | 2 ++ drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.h | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)