jnielsendotnet / isboot

FreeBSD kernel module to support booting from iSCSI via IBFT
4 stars 7 forks source link

Boot load order is after mellanox mlx4en module #9

Closed hsnyder closed 2 years ago

hsnyder commented 2 years ago

I am trying to get iSCSI boot working with an outboard Mellanox Connectx-2 NIC, and one issue I ran in to is that the mellanox driver module mlx4en is loaded after this module, meaning isboot will fail to find the NIC specified in the iBFT when it tries to look it up by MAC address.

The mlx4en module seems to use the linux compatibility kpi, and it's assigned to the subsystem SI_SUB_OFED_PREINIT, which is defined to be SI_SUB_ROOT_CONF-2. isboot is assigned to subsystem SI_SUB_OFED_PREINIT, which is a few entries earlier.

I was able to work around this particular issue by changing

-DECLARE_MODULE(isboot, mod_data, SI_SUB_PROTO_END, SI_ORDER_ANY);
+DECLARE_MODULE(isboot, mod_data, SI_SUB_ROOT_CONF-1, SI_ORDER_ANY);

in isboot.c. (though I still haven't gotten iSCSI boot working completely)

I'm quite new to FreeBSD kernel hacking, and I'm not sure if this workaround will have undesirable side-effects. Do you have any thoughts on the matter?

jnielsendotnet commented 2 years ago

I'm actually not much of a kernel hacker myself, I'm just trying to keep the code up-to-date and functional with some help from others. Maybe try this question on one of the FreeBSD mailing lists.

hsnyder commented 2 years ago

Thanks for your replies. Fair enough. I've asked the question on the freebsd-hackers mailing list and on the FreeBSD discord server. I will keep you advised of any responses.

hsnyder commented 2 years ago

Warner Losh replied to me on the mailing list and had the following to say.

In general, moving things later in the boot process to satisfy a prereq like this is fine (unless you move it past something else that depends on this having already started).

I'm not super familiar with this code, but I took a quick look. I'm not seeing anything that I would flag as an issue.

So it sounds like this is probably an okay change to make. I'll open a pull request in the coming days (can't do it right this moment). I'm going to close this issue. Thanks.

jnielsendotnet commented 1 year ago

FYI I make your suggested module load order change in commit https://github.com/jnielsendotnet/isboot/commit/fc76990b593bf386ca6138dc9797a48558845b90