raspberrypi / windows-drivers

Windows IOT drivers
MIT License
120 stars 33 forks source link

bcmgenet can only be installed on Windows build 1904x #23

Closed mariobalanica closed 3 years ago

mariobalanica commented 3 years ago

bcmgenet is written using the NetAdapterCx API, which is unstable and already had breaking changes for 2 times. @alotipac, the creator of the driver, has pushed some fixes to his fork of the old ms-iot repo: https://github.com/alotipac/rpi-iotcore/commits/rpi4-insider that make the driver work on the latest insider builds too (although it is unstable and significantly slower on builds 21301+).

We cannot merge those changes as Github Actions only provide the 19041 SDK/WDK, so CI builds will fail. The fixes are also not backwards compatible, meaning that we'd have to build 3 versions of the driver (and possibly more than that if Microsoft breaks the API again).

As a temporary workaround (until the API becomes stable), we could bundle our own version of NetAdapterCx with the driver, thus making it work with older versions of Windows too.

mariobalanica commented 3 years ago

@christopherco what do you think?

driver1998 commented 3 years ago

We might need to build NetAdapterCx ourselves if we want to redistribute it. But if it works, then I am all for it. And for those who don't know, NetAdapterCx is infact open source, in MIT License. https://github.com/microsoft/Network-Adapter-Class-Extension

christopherco commented 3 years ago

I don't really know the networking stack but I do view building our own NetAdapterCx as a last resort (and maybe we're in last resort territory already based on the history...)

By building our own, we'd miss out on any potential enhancements to NetAdapterCx as new preview builds come out. And if we find regressions, we could send that feedback to the NetAdapterCx devs (assuming we have a communication channel to use here) so they can fix it in a newer preview build and not officially ship something we eventually cannot use. By building our own NetAdapterCx, we wouldn't really be getting this feedback and I could easily see us supporting this "temporary workaround" permanently.

Regarding the CI builds, I think this comes down to which target OSes do we want to support. Then we align our CI to those. I currently see three main environments:

  1. RS5 (Windows IoT Core/Enterprise LTSC / 17763)
  2. Windows latest official release (version 2004 / 19041)
  3. Windows Insider Builds (latest preview builds)

Are all these targets interesting to support? Are there more target environments I missed? And I wonder how difficult it would be to set up CI for preview builds.

Disclaimer: Opinions expressed here are solely my own and do not express the views or opinions of my employer.

mariobalanica commented 3 years ago

Well, looks like https://github.com/microsoft/Network-Adapter-Class-Extension includes some headers that are not publicly available (or at least I couldn't find them).

driver1998 commented 3 years ago

Now that Windows 11 is near release, I guess we should revisit this again?

I just checked that Realtek released their NetAdapterCx based driver for Win11 only, so it seems to me that the interface should be stable now. https://www.realtek.com/zh-tw/component/zoo/category/network-interface-controllers-10-100-1000m-gigabit-ethernet-usb-3-0-software

Looks like 22000.194 only needs https://github.com/alotipac/rpi-iotcore/commit/ab26135488a68ebcbeda724945fdcd3bd207c572 and https://github.com/alotipac/rpi-iotcore/commit/63d7481e57a94d5d6a2d13b27487c6f7c79713a9, without the 21301 quirks. I haven't check the Nickel (Insider Dev, 224xx) builds yet, hopefully that won't break anything.

I guess we'll need to ship a Win10 version for 19041, and a Win11 version for 22000 and up, and skip all the insider builds in between (people can always refer to this issue and @alotipac's old patches if they want to play with old betas, but IMO we shouldn't encourage or support people running old betas as daily driver).

mariobalanica commented 3 years ago

https://github.com/raspberrypi/windows-drivers/commit/8ccd7fd2f93b8e079f8b937b6a924612dc08b257