From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: dwqe Rx IP header checksum offload
To: Stefan Sperling <stsp@stsp.name>
Cc: ustuehler@growit.io, tech@openbsd.org
Date: Thu, 25 Apr 2024 15:14:16 +0200

> Date: Wed, 24 Apr 2024 11:55:12 +0200
> From: Stefan Sperling <stsp@stsp.name>
> 
> On Sat, Apr 13, 2024 at 02:16:42AM +0200, Uwe Stuehler wrote:
> > Here's an updated diff that checks if the information in rxd->sd_tdes1
> > is valid and then whether (RDES1_IP_HDR_ERROR | RDES1_IP_CSUM_ERROR) is
> > set before marking the packet as hardware checksum failed and that all
> > three bits are clear before it marks the packet as hardware checksummed.
> 
> I would like to start with a simpler change: Let's adjust our macros
> for Rx descriptors. They are currently a confusing mix between the
> read and writeback formats. And they are incomplete for purposes
> such as checksum offloading.
> 
> This patch does not change the driver's behaviour but should help
> with making sense of things going forward.
> 
> ok?

I don't think it makes sense to add bits that we're not using.

>  M  sys/dev/ic/dwqereg.h  |  35+  5-
> 
> 1 file changed, 35 insertions(+), 5 deletions(-)
> 
> diff e6c7c102cf5d9891f32552a42895134a59937045 d5cd332729e0a5bb8c36ede324cb8340cfa43a8f
> commit - e6c7c102cf5d9891f32552a42895134a59937045
> commit + d5cd332729e0a5bb8c36ede324cb8340cfa43a8f
> blob - bc6de32a5a5e80968082f683f9536d64b944b518
> blob + f51d3b0a4717e4e0fd6dd1dd33b9301e27ff261f
> --- sys/dev/ic/dwqereg.h
> +++ sys/dev/ic/dwqereg.h
> @@ -233,14 +233,44 @@ struct dwqe_desc {
>  #define TDES3_FS		(1 << 29)
>  #define TDES3_OWN		(1U << 31)
>  
> -/* Rx bits */
> +/* Rx bits (read format; host to device) */
> +#define RDES3_BUF1V		(1 << 24)
> +#define RDES3_BUF2V		(1 << 25)
> +#define RDES3_IC		(1 << 30)
> +#define RDES3_OWN		(1U << 31)
> +
> +/* Rx bits (writeback format; device to host) */
> +#define RDES1_IP_PAYLOAD_TYPE	0x7
> +#define   RDES1_IP_PAYLOAD_UNKNOWN	0x0
> +#define   RDES1_IP_PAYLOAD_UDP		0x1
> +#define   RDES1_IP_PAYLOAD_TCP		0x2
> +#define   RDES1_IP_PAYLOAD_ICMP		0x3
> +#define RDES1_IP_HDR_ERROR	(1 << 3)
> +#define RDES1_IPV4_HDR		(1 << 4)
> +#define RDES1_IPV6_HDR		(1 << 5)
> +#define RDES1_IP_CSUM_BYPASS	(1 << 6)
> +#define RDES1_IP_PAYLOAD_ERROR	(1 << 7)
> +#define RDES3_LENGTH		(0x7fff << 0)
>  #define RDES3_ES		(1 << 15)
> +#define RDES3_LENTYPE		0x70000
> +#define   RDES3_LENTYPE_LENGTH	(0x0 << 16)
> +#define   RDES3_LENTYPE_TYPE	(0x1 << 16)
> +				/* 0x2 is reserved */
> +#define   RDES3_LENTYPE_ARP	(0x3 << 16)
> +#define   RDES3_LENTYPE_VLAN	(0x4 << 16)
> +#define   RDES3_LENTYPE_2VLAN	(0x5 << 16)
> +#define   RDES3_LENTYPE_MACCTL	(0x6 << 16)
> +#define   RDES3_LENTYPE_OAM	(0x7 << 16)
>  #define RDES3_DE		(1 << 19)
>  #define RDES3_RE		(1 << 20)
>  #define RDES3_OE		(1 << 21)
>  #define RDES3_RWT		(1 << 22)
> +#define RDES3_GP		(1 << 23)
>  #define RDES3_CE		(1 << 24)
> -#define RDES3_BUF1V		(1 << 24)
> -#define RDES3_IC		(1 << 30)
> -#define RDES3_OWN		(1U << 31)
> -#define RDES3_LENGTH		(0x7fff << 0)
> +#define RDES3_RDES0_VALID	(1 << 25)
> +#define RDES3_RDES1_VALID	(1 << 26)
> +#define RDES3_RDES2_VALID	(1 << 27)
> +#define RDES3_LD		(1 << 28)
> +#define RDES3_FD		(1 << 29)
> +#define RDES3_CTXT		(1 << 30)
> +/* Bit 31 is the OWN bit, as in "read" format. */
> 
>