Page MenuHomeFreeBSD

iflib: Move network stack capabilities after device driver init
AbandonedPublic

Authored by kbowling on Jul 26 2023, 7:33 PM.
Tags
None
Referenced Files
F101955648: D41200.diff
Tue, Nov 5, 6:57 PM
Unknown Object (File)
Oct 1 2024, 2:24 AM
Unknown Object (File)
Oct 1 2024, 12:35 AM
Unknown Object (File)
Sep 12 2024, 1:47 PM
Unknown Object (File)
Aug 28 2024, 8:54 PM
Unknown Object (File)
Aug 4 2024, 12:20 PM
Unknown Object (File)
Jun 27 2024, 5:11 AM
Unknown Object (File)
May 21 2024, 2:45 AM

Details

Reviewers
marius
shurd
Group Reviewers
iflib
Summary

Move the capabilities setup after device driver's init call so it can alter the settings

Test Plan

Checked dd_init routines for various drivers and see no race between the new ordering

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Curious why the driver would need to change it in init... can devices actually change their supported offloads after attach? Aside from the initial setup, I would expect control of which offloads to use would be under the control of the user, not the driver... I would be very surprised if taking an interface down then up changed the offload flags.

Curious why the driver would need to change it in init... can devices actually change their supported offloads after attach? Aside from the initial setup, I would expect control of which offloads to use would be under the control of the user, not the driver... I would be very surprised if taking an interface down then up changed the offload flags.

Hi,

See D41170 for the intended usage after

/* Only do TSO on gigabit Ethernet for older parts due to errata */

The problem I am trying to solve is handling things like suspend/resume and media change since this is a client part.

I believe what I am doing will work because the network stack is responsible for marking mbufs for offload, thus if we drop these caps there will not any marked.

And of course open to a clue-by-4 if there is a cleaner way to do this.

Oh wow, so TSO works at 100Mbbs, but fails at 1Gbps? Crazy. I would personally be inclined to just disable TSO unconditionally in attach_pre(), but if someone *really* wants TSO for their "Fast" Ethernet connections, you may as well give it to them.

Oh wow, so TSO works at 100Mbbs, but fails at 1Gbps? Crazy. I would personally be inclined to just disable TSO unconditionally in attach_pre(), but if someone *really* wants TSO for their "Fast" Ethernet connections, you may as well give it to them.

Other way around. TSO on gigabit, no TSO on 10/100.

Ah yes, makes more sense to enable then... not sure how you'll manage to keep the admin preferences though. It looks like if I have TSO enabled and unplug the cable then plug it back in, TSO will become disabled?

Ah yes, makes more sense to enable then... not sure how you'll manage to keep the admin preferences though. It looks like if I have TSO enabled and unplug the cable then plug it back in, TSO will become disabled?

Yeah that does seem to be a small gap, if you wanted to administratively enable TSO on a 100mbit link it'd get whacked on a link flap.

Wouldn't it also whack TSO on a flap for 1G? It seems like if you want TSO, you'll have to explicitly re-enable it every time a 1G link is established.

Yeah, looks like em_if_update_admin_status() sets speed to zero when link is lost, so just not turning off TSO when speed == 0 would do the trick.

I also notice that SPEED_2500 is a thing.

Wouldn't it also whack TSO on a flap for 1G? It seems like if you want TSO, you'll have to explicitly re-enable it every time a 1G link is established.

Yeah the other PR would not promote a 100 to 1000 transition back to TSO, I could rework the other PR to store and restore some state and handle both of these cases while still triggering in IFDI_INIT to make it so.

So yeah, looks fine to me, but I'm very out of touch with current iflib, so I'm inclined to not approve with my iflib hat on. My biggest concern was that someone may think it's the right way to handle the bits.

I've found a way to do this without modifying iflib.