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: Mon, 29 Apr 2024 21:57:56 +0200

> Date: Thu, 25 Apr 2024 15:28:26 +0200
> From: Stefan Sperling <stsp@stsp.name>
> 
> On Thu, Apr 25, 2024 at 03:14:16PM +0200, Mark Kettenis wrote:
> > > 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.
> 
> Sorry, it already went in.
> 
> This follow-up diff removes all RDES bits which are currently unused,
> keeping the new bits required by my Rx checksum diff.
> 
> There were already some unused bits before: DE, RE, OE, RWT, CE.

Yes, I think I added these when I was adding to code that checks for
receive errors.  I ended up only using the error summary bit though.

So maybe the number of #defines you added wasn't so excessive after
all.  And given that this already went in I'm not sure it is worth
removing the unused bits anymore...

> diff /usr/src
> commit - af976210f89c79f59cd420245f97eea9b3fd96b9
> path + /usr/src
> blob - e919c35115209443ee1084a4d12e5f904e5e28d3
> file + sys/dev/ic/dwqereg.h
> --- sys/dev/ic/dwqereg.h
> +++ sys/dev/ic/dwqereg.h
> @@ -240,7 +240,6 @@ struct dwqe_desc {
>  
>  /* 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)
>  
> @@ -257,25 +256,7 @@ struct dwqe_desc {
>  #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_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. */
>