Page MenuHomeFreeBSD

mlx4/OFED: replace the struct net_device with struct ifnet
ClosedPublic

Authored by bz on May 27 2021, 9:54 PM.

Details

Summary

Given all the code does operate on struct ifnet, the last step is to
rename struct net_device to struct ifnet (that is what it was defined
to in the LinuxKPi code).
While mlx4 and OFED are "shared" code the decision was made years ago
to not write it based on the netdevice KPI but the native ifnet KPI
for most of it. This commit simply spells this out and with that
frees "struct netdevice" to be re-done on LinuxKPI to become a more
native/mixed implementation over time as needed by, e.g., wireless
drivers.

Sponsored by: The FreeBSD Foundation
MFC after: 10 days

Test Plan

I did a tinderbox run WITH_OFED=yes for amd64 (with and without
further netdevice.h changes and both succeeded).
I have no hardware to test this on but in theory this should
be a nop.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 39702
Build 36591: arc lint + arc unit

Event Timeline

bz requested review of this revision.May 27 2021, 9:54 PM

I do understand that this looks like a pretty deep change making it hard to share code possibly still.
In fact this only puts the name right.
Per-OS abstractions have probably looong gone for this code, which would have been the better solution I assume.

sys/dev/mlx4/mlx4_ib/mlx4_ib_main.c
2373

This is one of the actual changes.
At the moment this works because they are the same.
Once they are two different structures I am trying to still keep this working (more or less elegantly in the future).

The same line is in mlx5.

sys/dev/mlx5/mlx5_ib/mlx5_ib_main.c
110

See comment above.

How do you plan to install the Linux net_device?
ibcore needs basically to access any native ifnet, and I think we may need to covert the netdev event notifier code to use native FreeBSD APIs, else your changes will break it, because the ifnet in question does not have a Linux net_device, so that it can deliver the notifier block events??

--HPS

sys/dev/mlx4/mlx4_ib/mlx4_ib_main.c
2373

Is it an idea to have a new function, netdev_notifier_info_to_ifnet(ptr) which does exactly this w/o the cast?

sys/ofed/drivers/infiniband/core/ib_roce_gid_mgmt.c
383

Ditto.

sys/dev/mlx4/mlx4_ib/mlx4_ib_main.c
2373

Over next step yes. My plan was to put the ifnet back to 'struct netdev_notifier_info' and then only if we request a net_device return that; otherwise return the ifnet. I just don't want to do three changes in one go. So if you are happy with an extra accessor function I am more than happy to add that!?

sys/dev/mlx4/mlx4_ib/mlx4_ib_main.c
2373

@hselasky I added D30522 which would give you that as you suggested; if you agreed with that; I'll update this here as well.

I'll have a look later today. Currently a bit busy. Thank you!

bz marked an inline comment as done.

Prepare changing netdev_notifier_info_to_dev() to netdev_notifier_info_to_ifp()
from D30522 and remove the extra casts.

If D30522 will go in, will you be okay with this as well or do we need to re-think?

Yes, this patch looks good. I've just been a bit busy :-)

This revision is now accepted and ready to land.Jun 7 2021, 7:19 AM

This was committed in 1411f52facc2b955584f2cb453b912a903e319ed but it seems auto-close did not work (or I have a typo on the D R: line?)