Page MenuHomeFreeBSD

Mechanically convert usb ethernet drivers to DrvAPI
ClosedPublic

Authored by jhibbits on Dec 22 2022, 3:29 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 7, 12:08 PM
Unknown Object (File)
Wed, Nov 6, 4:12 PM
Unknown Object (File)
Wed, Nov 6, 12:04 PM
Unknown Object (File)
Mon, Nov 4, 12:09 AM
Unknown Object (File)
Thu, Oct 24, 7:03 PM
Unknown Object (File)
Wed, Oct 23, 12:45 PM
Unknown Object (File)
Fri, Oct 18, 11:08 AM
Unknown Object (File)
Wed, Oct 16, 11:40 PM
Subscribers

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 49046
Build 45935: arc lint + arc unit

Event Timeline

if_get_counter() isn't available yet, so remove the change for if_upgt which uses it.

zlei added inline comments.
sys/dev/usb/net/if_cdce.c
861
sys/dev/usb/net/if_cue.c
346
sys/dev/usb/net/if_ure.c
1005

This overwritten previous if_setsendqlen,
if_setsendqlen(ifp, ifqmaxlen);

1407
1419
sys/dev/usb/net/uhso.c
98

Small nits, align member sc_ifp

690
1784
sys/dev/usb/usb_pf.c
230
412
414
537
sys/dev/usb/net/if_ure.c
1005

I think we need another API for dealing with the ifq_drv_maxlen.

Update the patch to the one I've been building for a while now.

sys/dev/usb/net/if_axe.c
1181–1182

Missing ! operator

sys/dev/usb/net/if_axge.c
682–683

Missing ! operator

sys/dev/usb/net/if_mosreg.h
163 ↗(On Diff #116701)

I'd do this in D38581, and it should be MFCed as well.

sys/dev/usb/net/if_smsc.c
1142–1143

Missing ! operator.

Look good to me.

And be aware of

I think we need another API for dealing with the ifq_drv_maxlen.

sys/dev/usb/net/if_ure.c
1005

I think we need another API for dealing with the ifq_drv_maxlen.

Before that is done we might leave a TODO so this won't be forgotten.

This revision is now accepted and ready to land.Feb 15 2023, 8:16 AM
jhibbits added inline comments.
sys/dev/usb/net/if_ure.c
1005

Hmm... looking at this again, I *think* the first if_setsendqlen is unnecessary; ifqmaxlen defaults to 50, so what's being said here is that the driver can handle 10x more than the input queue, which seems quite bizarre. @hselasky, or @kevlo (driver author) can you comment on that?

sys/dev/usb/net/if_ure.c
1005

USB host controllers usually cannot interrupt more than 8000 times per second (High Speed and above), and queues should be large enough to buffer up to 5 Gbps worth of traffic for 125us (1/8000 second). Does that make any sense to you?

sys/dev/usb/net/if_ure.c
1005

Kind of, but I don't understand the disparity between the stack-side queue and the driver queue in this case. Would it make sense for them both to be 512, or is 50 stack and 512 driver really the right way?

I checked what re(4) uses, under the assumption that it's likely comparable to ure(4) (I could be very wrong on this assumption, though), and it uses 512 for both queues.

sys/dev/usb/net/if_ure.c
1005

ure(4) has special capabilities to queue multiple ethernet packets per USB host controller IRQ, and should use 512, while the others are more simple and only do one ethernet packet per IRQ. Basically the current interface queue lengths should be preserved.

jhibbits added a subscriber: jmg.

Adding @jmg, since he made the change to ure(4) to change the drv queue length to 512.

sys/dev/usb/net/if_ure.c
1005

Yes, the previous setting of ifqmaxlen should just be removed.

It's an artifact of mechanical replacement, as my diff that set to 512 was:

IFQ_SET_MAXLEN(&ifp->if_snd, ifqmaxlen);
  • ifp->if_snd.ifq_drv_maxlen = ifqmaxlen;

+ /*
+ * Try to keep two transfers full at a time.
+ * ~(TRANSFER_SIZE / 80 bytes/pkt * 2 buffers in flight)
+ */
+ ifp->if_snd.ifq_drv_maxlen = 512;

as FreeBSD doesn't have documentation on how to set queue length, I just modified the previous code that was there when the driver was originally imported.

As hselasky says, we need to keep a queue length of 512 in order to keep small packets streaming to the device. The max transfer size is 16k, so 16k/80 = 204.8, so * 2 and rounded up slightly is 512.

This should max out 1Gbps devices. I haven't trieds 2.5Gbps or 5Gbps devices to see if this or transfer size needs to be increased.