Page MenuHomeFreeBSD

iWARP driver for Intel X722 Adapter - initial commit
Needs ReviewPublic

Authored by bartosz.sobczak_intel.com on Jun 27 2017, 11:42 AM.

Details

Summary

This is an initial commit for iWARP FreeBSD driver for Intel(R) Ethernet Connection X722.
This module supports ixl driver in version 1.7.12-k or later.
This module supports Kernel Verbs API of the OFED software.

This module is considered a BETA quality release.

Test Plan

Testing has been done using krping tool, as well as some additional internal test tools that exercise Kernel Verbs API.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

Known Issues:

  • Loopback is not supported.
  • MTU changes are not supported.
  • IPv6 is not supported.
  • MW memory mode is not supported.
  • MR memory mode supports only single buffer.
  • The function ib_cq_resize is not supported.
  • The max number of registered cq, qp, pd or mr reported by the device may differ from the actual number of registrations achievable.
  • A kernel crash may occur when trying to run krping without ensuring that the two machines are able to ping each other.
  • A kernel crash may occur when trying to load the iw_ixl driver when hw.ixl.enable_iwarp=0 (fixed with if_ixl 1.7.13).
  • A kernel crash may occur when loading the iw_ixl driver on a card that is supported by if_ixl driver, but does not have iWARP capability (fixed with if_ixl 1.7.13).
sbruno requested changes to this revision.Jun 27 2017, 4:17 PM

This doesn't appear to be connected to the kernel build via sys/conf/files. It also does not have a kernel config option via sys/conf/options.

This revision now requires changes to proceed.Jun 27 2017, 4:17 PM

Module build is not connected vi sys/modules/Makefile

Missing a man page. Probably needs something for share/man/man4. Maybe an iw_ixl(4) page?

erj added a reviewer: np.Jun 27 2017, 4:47 PM
erj added a subscriber: erj.Jun 27 2017, 4:49 PM

I should add that we're going to update the ixl(4) driver soon because as @bartosz.sobczak_intel.com mentioned, it will fix a crash.

This doesn't appear to be connected to the kernel build via sys/conf/files. It also does not have a kernel config option via sys/conf/options.

This is a Beta release, so, first of all, i do not want the driver to be loaded with the kernel at startup (especially before if_ixl would be updated to 1.7.13 or later).
Also, as far as i'm concerned, the iw_cxgbe module (which is an iwarp driver for Chelsio cards) is not tied to the kernel as well.

Module build is not connected vi sys/modules/Makefile

Again, this is a Beta stage release. I think it would be best to build it only when it is needed, for people who want to use it, especially that there are some additional settings that needs to be inserted into kernel configuration for it to work (the OFED, and proper settings of the ixl driver).

For both of these issues I'd leave it as is for now, and consider further integration with the kernel in the future.

Missing a man page. Probably needs something for share/man/man4. Maybe an iw_ixl(4) page?

Actually, i have a man page ready for submission, I thought it would be a good idea to add it later. Do you think it would be better to post it in this commit?

Thanks for your input!
Bartek

np edited edge metadata.Jun 28 2017, 4:28 PM

This doesn't appear to be connected to the kernel build via sys/conf/files. It also does not have a kernel config option via sys/conf/options.

This is a Beta release, so, first of all, i do not want the driver to be loaded with the kernel at startup (especially before if_ixl would be updated to 1.7.13 or later).
Also, as far as i'm concerned, the iw_cxgbe module (which is an iwarp driver for Chelsio cards) is not tied to the kernel as well.

Module build is not connected vi sys/modules/Makefile

Again, this is a Beta stage release. I think it would be best to build it only when it is needed, for people who want to use it, especially that there are some additional settings that needs to be inserted into kernel configuration for it to work (the OFED, and proper settings of the ixl driver).

For both of these issues I'd leave it as is for now, and consider further integration with the kernel in the future.

We have a WITH_OFED knob for all the OFED components -- RDMA infrastructure and hardware drivers. iw_ixl could
also use that knob.

bartosz.sobczak_intel.com edited edge metadata.

Added manual page iw_ixl.4 to share/man/man4/

Adding the module for compilation with buildkernel.

Additional files modified:
sys/amd64/conf/NOTES
sys/conf/files.amd64
sys/modules/Makefile

wblock added a subscriber: wblock.Jul 7 2017, 3:38 PM
wblock added inline comments.
share/man/man4/iw_ixl.4
35

Appears that an (R) symbol after "Intel" was replaced with a question mark. Not sure about the star after "FreeBSD".

60

Please start new sentences on new lines.

63

s/ethernet/Ethernet/

71

The verb can be removed to make this a little clearer: s/Determines the maximum/The maximum/

s/msix/MSI-X/ (I think)
s/pci/PCI/

Also, please start new sentences on new lines.

73
The amount of debug messages to print.
75
Select which MPA version to use.
The default is 2.
80

As above:

The amount of debug messages to print.
85

Please start new sentences on new lines.
Please avoid the informal "you" (https://www.freebsd.org/doc/en_US.ISO8859-1/books/fdp-primer/writing-style.html)
So:

Note that 40G operation can generate high numbers of interrupts, which are often incorrectly interpreted as a storm condition in the kernel.
This can be resolved by setting:
108

Use .Lk for links:

.Lk http://support.intel.com/ .
110

Redundant "with", suggested:

If an issue with this driver is identified on a supported adapter,

Changes suggested by wblock are applied.
minor other changes to the readme.
small code cleanup.
server_inv bugfix.
version bump to 0.1.11.

iw_ixl.4 : few more links that should be marked with .Lk changed

I don't think these are very significant, but I get the following warnings during a build:

/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_utils.c:943:46: warning: taking address of packed member 'ip6_dst' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_ntohl(loc_addr, (__be32 *) & ip6h->ip6_dst);
                                                           ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_utils.c:944:46: warning: taking address of packed member 'ip6_src' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_ntohl(rem_addr, (__be32 *) & ip6h->ip6_src);
                                                           ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:488:36: warning: taking address of packed member 'ip6_src' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_htonl((__be32 *) & ip6h->ip6_src,
                                                 ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:490:36: warning: taking address of packed member 'ip6_dst' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_htonl((__be32 *) & ip6h->ip6_dst,
                                                 ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:3046:22: warning: taking address of packed member 'ip6_dst' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                                    (__be32 *) & ip6h->ip6_dst);
                                                 ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:3048:22: warning: taking address of packed member 'ip6_src' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                                    (__be32 *) & ip6h->ip6_src);

I don't think these are very significant, but I get the following warnings during a build:

/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_utils.c:943:46: warning: taking address of packed member 'ip6_dst' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_ntohl(loc_addr, (__be32 *) & ip6h->ip6_dst);
                                                           ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_utils.c:944:46: warning: taking address of packed member 'ip6_src' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_ntohl(rem_addr, (__be32 *) & ip6h->ip6_src);
                                                           ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:488:36: warning: taking address of packed member 'ip6_src' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_htonl((__be32 *) & ip6h->ip6_src,
                                                 ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:490:36: warning: taking address of packed member 'ip6_dst' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_htonl((__be32 *) & ip6h->ip6_dst,
                                                 ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:3046:22: warning: taking address of packed member 'ip6_dst' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                                    (__be32 *) & ip6h->ip6_dst);
                                                 ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:3048:22: warning: taking address of packed member 'ip6_src' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                                    (__be32 *) & ip6h->ip6_src);

Indeed, I don't find it too significant. I guess this is just a performance issue, which is insignificant given the amount of times this operation is done. And what is more important, this is related to the path which would be exercised only when using ipv6. The ipv6 is currently not supported, and there are some changes to the OFED going to be needed to make it work.
I may probably get rid of these warnings if it was really needed.

sbruno accepted this revision.Jul 27 2017, 6:37 PM

I will add this to the commit queue for today.

This revision is now accepted and ready to land.Jul 27 2017, 6:37 PM
erj added a comment.Jul 27 2017, 10:02 PM

@sbruno - I think Bartosz needed an update to the in-kernel ixl(4) for this to work properly.

@bartosz.sobczak_intel.com - I'm still working on fixing a critical SW6 bug; I need to get that fixed before I upstream the updated ixl(4) driver. I'm sorry for the delay, but it's difficult to debug.

erj added a comment.Jul 27 2017, 10:04 PM

I don't think these are very significant, but I get the following warnings during a build:

/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_utils.c:943:46: warning: taking address of packed member 'ip6_dst' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_ntohl(loc_addr, (__be32 *) & ip6h->ip6_dst);
                                                           ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_utils.c:944:46: warning: taking address of packed member 'ip6_src' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_ntohl(rem_addr, (__be32 *) & ip6h->ip6_src);
                                                           ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:488:36: warning: taking address of packed member 'ip6_src' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_htonl((__be32 *) & ip6h->ip6_src,
                                                 ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:490:36: warning: taking address of packed member 'ip6_dst' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                i40iw_copy_ip_htonl((__be32 *) & ip6h->ip6_dst,
                                                 ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:3046:22: warning: taking address of packed member 'ip6_dst' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                                    (__be32 *) & ip6h->ip6_dst);
                                                 ^~~~~~~~~~~~~
/home/sbruno/bsd/fbsd_head/sys/modules/ixl/iw_ixl/../../../dev/ixl/iwarp/iw_ixl_cm.c:3048:22: warning: taking address of packed member 'ip6_src' of class or structure 'ip6_hdr' may result in an unaligned pointer value [-Waddress-of-packed-member]
                                    (__be32 *) & ip6h->ip6_src);

Indeed, I don't find it too significant. I guess this is just a performance issue, which is insignificant given the amount of times this operation is done. And what is more important, this is related to the path which would be exercised only when using ipv6. The ipv6 is currently not supported, and there are some changes to the OFED going to be needed to make it work.
I may probably get rid of these warnings if it was really needed.

Also, this is incredibly minor, but the space between the "&" and variable names in the function arguments really bothers me, visually.

In D11378#243724, @erj wrote:

@sbruno - I think Bartosz needed an update to the in-kernel ixl(4) for this to work properly.

@bartosz.sobczak_intel.com - I'm still working on fixing a critical SW6 bug; I need to get that fixed before I upstream the updated ixl(4) driver. I'm sorry for the delay, but it's difficult to debug.

Do I need to hold off committing this until that bug is fixed?

bartosz.sobczak_intel.com edited edge metadata.

Removing additional errors in READMEs.
Improving casts handling - thanks for pointing this out!

This revision now requires review to proceed.Jul 31 2017, 11:28 AM
sbruno added a comment.Aug 7 2017, 6:07 PM

@erj are you going to commit this once you've made the IXL updates required for it to function?

erj added a comment.Aug 7 2017, 8:33 PM

@erj are you going to commit this once you've made the IXL updates required for it to function?

Yeah, that's the plan. If @bartosz.sobczak_intel.com is okay with the current version.

np added a comment.Nov 14 2017, 5:42 PM
In D11378#247102, @erj wrote:

@erj are you going to commit this once you've made the IXL updates required for it to function?

Yeah, that's the plan. If @bartosz.sobczak_intel.com is okay with the current version.

This will collide with the the projects/bsd_rdma_4_9 branch. Have you looked at that?

See this message too:
https://lists.freebsd.org/pipermail/freebsd-infiniband/2017-November/000364.html

In D11378#272139, @np wrote:
In D11378#247102, @erj wrote:

@erj are you going to commit this once you've made the IXL updates required for it to function?

Yeah, that's the plan. If @bartosz.sobczak_intel.com is okay with the current version.

This will collide with the the projects/bsd_rdma_4_9 branch. Have you looked at that?

See this message too:
https://lists.freebsd.org/pipermail/freebsd-infiniband/2017-November/000364.html

Thanks for warning!
I'm aware of work being done on bsd_rdma_4_9 branch, though this exact commit is not ready for it.
I'll adjust my work as soon as possible.

Several changes have been done to the driver code:

  • adjustments against future changes in if_ixl driver,
  • adjustments preparing for making VIMAGE a default option in kernel configuration,
  • adding support for BSD_RDMA_4_9 branch of FreeBSD-CURRENT kernel,
  • implementing IPv6 support for BSD_RDMA_4_9 branch,
  • fix minor problem with traffic on tagged vLAN,
  • fix for fast memory registration issue with stag update.

Please note, that this code is designed as a way to distribute the driver also for older systems, thus some ifdefs checking OS version are present.