Page MenuHomeFreeBSD

Mechanically convert Xen netfront/netback(4) to DrvAPI
ClosedPublic

Authored by jhibbits on Dec 22 2022, 3:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 4, 7:12 AM
Unknown Object (File)
Mon, Mar 4, 7:12 AM
Unknown Object (File)
Mon, Mar 4, 7:12 AM
Unknown Object (File)
Sun, Mar 3, 5:48 AM
Unknown Object (File)
Sun, Mar 3, 5:35 AM
Unknown Object (File)
Dec 20 2023, 6:37 AM
Unknown Object (File)
Dec 17 2023, 5:50 AM
Unknown Object (File)
Dec 10 2023, 8:33 PM

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Alerting the others who might be up for trying. Looks vaguely possible, but at this instant I've got another thing on my queue.

zlei added inline comments.
sys/dev/xen/netback/netback.c
2257–2258

no need for parenthesis

sys/dev/xen/netfront/netfront.c
1040
2097

No need for parenthesis

2123

This is wrong.

Is there a time-frame for when this is desired? I was hoping to get my new build machine operational (new processor release firmware bugs, ugh) before doing another full build, but if the time-frame is short I could potentially do a test run sooner.

Sorry, I'm on parental leave and so far didn't have time to look into this. I will be back on the 8th of March, is there any rush to get this in now? If there's a rush I would be fine with this going in as long as someone can give a look and test it.

I agree with @zlei's comments, though this is nominally slated as a mechanical conversion so lack of cleanup could be forgiven. In other news, this does build and does appear to be usefully functional.

sys/dev/xen/netfront/netfront.c
2123

I concur. Though this looks weird, "Try to preserve as many options as possible." yet it appears to be clearing everything.

jhibbits added inline comments.
sys/dev/xen/netfront/netfront.c
2123

Yeah, this is very obviously wrong. It looks like the awk script got confused.

@ehem_freebsd_m5p.com the "Try to preserve" comment relates to the cap_enabled saving, before it clears everything. The wrong bit is the if_gethwassist() bit, which, that line needs deleted.

Looks good to me.

The change ifp->if_capenable = ifp->if_hwassist = 0; is introduced by c2d12e5e0d36 intentionally, while the comment remains untouched.
I think it is safe to preserve the same behavior in this Mechanically convert ;)

CC @royger if he will comment on this.

This revision is now accepted and ready to land.Mar 13 2023, 3:29 PM

For the heck of it, going through and finding more instances of the pattern @zlei noticed. Seems the awk script doesn't handle the pattern ~(FLAG_TO_REMOVE) well. These are all adjust on commit, not in need of additional review.

I still haven't caused this to fail, so I'm presuming it still works. Issue is I haven't buried myself in this portion of Xen code to say much or test very well. As such I haven't found issues with my limited tests.

sys/dev/xen/netback/netback.c
399

Spacing change here. Looks like the line below got de-spaced at some other point.

2196

Extra parentheses.

2233

Another set of extra parentheses.

2263

Yet another set of extras.

2281–2283

Two for the price of one!

2287

Last one.

sys/dev/xen/netfront/netfront.c
218

Another spacing change.