On Sat, Aug 10, 2024 at 11:38:19PM +0200, Mark Kettenis wrote:
> > Date: Sat, 10 Aug 2024 23:11:35 +0200
> > From: Patrick Wildt <patrick@blueri.se>
> > 
> > Hi,
> > 
> > a friend of mine has been complaining about watchdog timeouts on RK3588.
> > I compared the driver with other ones that are stable, like dwqe(4), and
> > found a few differences in behaviour.  One thing I noticed is that the
> > DMA syncs on rge_encap() most certainly don't work correctly, because it
> > only syncs the *last* descriptor in the chain.
> > 
> > I think this diff should make sure all TX descs are synced.
> 
> Looks good to me; ok kettenis@

i think the free space checks on the ring are wrong too. the way i read
the current code, we could fill the ring too much and overwrite some
descriptors from another packet. you would have to be extremely unlucky
for this to happen though.

ok?

Index: pci/if_rge.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/if_rge.c,v
diff -u -p -r1.28 if_rge.c
--- pci/if_rge.c	10 Aug 2024 21:53:06 -0000	1.28
+++ pci/if_rge.c	10 Aug 2024 23:59:46 -0000
@@ -581,7 +581,7 @@ rge_start(struct ifqueue *ifq)
 	free -= idx;
 
 	for (;;) {
-		if (RGE_TX_NSEGS >= free + 2) {
+		if (free < RGE_TX_NSEGS + 2) {
 			ifq_set_oactive(&ifp->if_snd);
 			break;
 		}
Index: ic/re.c
===================================================================
RCS file: /cvs/src/sys/dev/ic/re.c,v
diff -u -p -r1.217 re.c
--- ic/re.c	19 Jan 2024 03:46:14 -0000	1.217
+++ ic/re.c	10 Aug 2024 23:59:46 -0000
@@ -1834,7 +1834,7 @@ re_start(struct ifqueue *ifq)
 	free -= idx;
 
 	for (;;) {
-		if (sc->rl_ldata.rl_tx_ndescs >= free + 2) {
+		if (free < sc->rl_ldata.rl_tx_ndescs + 2) {
 			ifq_set_oactive(ifq);
 			break;
 		}