Skip to content

Commit 712ccaa

Browse files
committed
usnic: require libfabric >= v1.3 at run time
There are critical usnic libfabric AV insert bugs before v1.3, so don't allow any version prior to v1.3 at run time (still allow *compiling* with earlier versions, though, since the ABI guarantees allow us to compile with an earlier libfabric and run with a later libfabric). Switch to using fi_version() to check the version (instead of calling fi_getinfo()) as a potentially lighter-weight / simpler solution. This allows us to only call fi_getinfo() once. Signed-off-by: Jeff Squyres <[email protected]> (cherry picked from commit 345c07a)
1 parent c232dbe commit 712ccaa

File tree

1 file changed

+76
-53
lines changed

1 file changed

+76
-53
lines changed

opal/mca/btl/usnic/btl_usnic_component.c

Lines changed: 76 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -585,25 +585,6 @@ static void free_filter(usnic_if_filter_t *filter)
585585
free(filter);
586586
}
587587

588-
static int do_fi_getinfo(uint32_t version, struct fi_info **info_list)
589-
{
590-
struct fi_info hints = {0};
591-
struct fi_ep_attr ep_attr = {0};
592-
struct fi_fabric_attr fabric_attr = {0};
593-
594-
/* We only want providers named "usnic" that are of type EP_DGRAM */
595-
fabric_attr.prov_name = "usnic";
596-
ep_attr.type = FI_EP_DGRAM;
597-
598-
hints.caps = FI_MSG;
599-
hints.mode = FI_LOCAL_MR | FI_MSG_PREFIX;
600-
hints.addr_format = FI_SOCKADDR;
601-
hints.ep_attr = &ep_attr;
602-
hints.fabric_attr = &fabric_attr;
603-
604-
return fi_getinfo(version, NULL, 0, 0, &hints, info_list);
605-
}
606-
607588
/*
608589
* UD component initialization:
609590
* (1) read interface list from kernel and compare against component
@@ -638,40 +619,61 @@ static mca_btl_base_module_t** usnic_component_init(int* num_btl_modules,
638619
return NULL;
639620
}
640621

641-
/* This code understands libfabric API versions v1.0, v1.1, and
642-
v1.4. Even if we were compiled with libfabric API v1.0, we
643-
still want to request v1.1 -- here's why:
644-
645-
- In libfabric v1.0.0 (i.e., API v1.0), the usnic provider did
646-
not check the value of the "version" parameter passed into
647-
fi_getinfo()
648-
649-
- If you pass FI_VERSION(1,0) to libfabric v1.1.0 (i.e., API
650-
v1.1), the usnic provider will disable FI_MSG_PREFIX support
651-
(on the assumption that the application will not handle
652-
FI_MSG_PREFIX properly). This can happen if you compile OMPI
653-
against libfabric v1.0.0 (i.e., API v1.0) and run OMPI
654-
against libfabric v1.1.0 (i.e., API v1.1).
655-
656-
So never request API v1.0 -- always request a minimum of
657-
v1.1.
658-
659-
The usnic provider changed the strings in the fabric and domain
660-
names in API v1.4. With API <= v1.3:
622+
/* There are multiple dimensions to consider when requesting an
623+
API version number from libfabric:
624+
625+
1. This code understands libfabric API versions v1.0 through
626+
v1.4.
627+
628+
2. Open MPI may be *compiled* against one version of libfabric,
629+
but may be *running* with another.
630+
631+
3. There were usnic-specific bugs in Libfabric prior to
632+
libfabric v1.3.0 (where "v1.3.0" is the tarball/package
633+
version, not the API version; but happily, the API version
634+
was also 1.3 in Libfabric v1.3.0):
635+
636+
- In libfabric v1.0.0 (i.e., API v1.0), the usnic provider
637+
did not check the value of the "version" parameter passed
638+
into fi_getinfo()
639+
- If you pass FI_VERSION(1,0) to libfabric v1.1.0 (i.e., API
640+
v1.1), the usnic provider will disable FI_MSG_PREFIX
641+
support (on the assumption that the application will not
642+
handle FI_MSG_PREFIX properly). This can happen if you
643+
compile OMPI against libfabric v1.0.0 (i.e., API v1.0) and
644+
run OMPI against libfabric v1.1.0 (i.e., API v1.1).
645+
- Some critical AV bug fixes were included in libfabric
646+
v1.3.0; prior versions can fail in fi_av_* operations in
647+
unexpected ways (libnl: you win again!).
648+
649+
So always request a minimum API version of v1.3.
650+
651+
Note that the FI_MAJOR_VERSION and FI_MINOR_VERSION in
652+
<rdma/fabric.h> represent the API version, not the Libfabric
653+
package (i.e., tarball) version. As of Libfabric v1.3, there
654+
is currently no way to know a) what package version of
655+
Libfabric you were compiled against, and b) what package
656+
version of Libfabric you are running with.
657+
658+
Also note that the usnic provider changed the strings in the
659+
fabric and domain names in API v1.4. With API <= v1.3:
661660
662661
- fabric name is "usnic_X" (device name)
663662
- domain name is NULL
664663
665-
With libfabric API >= v1.4:
664+
With libfabric API >= v1.4, all Libfabric IP-based providers
665+
(including usnic) follow the same convention:
666666
667667
- fabric name is "a.b.c.d/e" (CIDR notation of network)
668668
- domain name is "usnic_X" (device name)
669669
670670
NOTE: The configure.m4 in this component will require libfabric
671-
>= v1.1.0 (i.e., it won't accept v1.0.0) because of a critical
672-
bug in the usnic provider in libfabric v1.0.0. However, the
673-
compatibility code with libfabric v1.0.0 in the usNIC BTL has
674-
been retained, for two reasons:
671+
>= v1.1.0 (i.e., it won't accept v1.0.0) because it needs
672+
access to the usNIC extension header structures that only
673+
became available in v1.1.0.
674+
675+
All that being said, the compatibility code with libfabric
676+
v1.0.0 in the usNIC BTL has been retained, for two reasons:
675677
676678
1. It's not harmful, nor overly complicated. So the
677679
compatibility code was not ripped out.
@@ -681,19 +683,40 @@ static mca_btl_base_module_t** usnic_component_init(int* num_btl_modules,
681683
Someday, #2 may no longer be true, and we may therefore rip out
682684
the libfabric v1.0.0 compatibility code. */
683685

684-
/* First try API version 1.4. If that doesn't work, try API
685-
version 1.1. */
686+
/* First, check to see if the libfabric we are running with is <=
687+
libfabric v1.3. If so, don't bother going further. */
686688
uint32_t libfabric_api;
687-
libfabric_api = FI_VERSION(1, 4);
688-
ret = do_fi_getinfo(libfabric_api, &info_list);
689-
// Libfabric core will return -FI_ENOSYS if it is too old
690-
if (-FI_ENOSYS == ret) {
691-
libfabric_api = FI_VERSION(1, 1);
692-
ret = do_fi_getinfo(libfabric_api, &info_list);
689+
libfabric_api = fi_version();
690+
if (libfabric_api < FI_VERSION(1, 3)) {
691+
opal_output_verbose(5, USNIC_OUT,
692+
"btl:usnic: disqualifiying myself because Libfabric does not support v1.3 of the API (v1.3 is *required* for correct usNIC functionality).");
693+
return NULL;
694+
}
695+
696+
/* Libfabric API 1.3 is fine. Above that, we know that Open MPI
697+
works with libfabric API v1.4, so just use that. */
698+
if (libfabric_api > FI_VERSION(1, 3)) {
699+
libfabric_api = FI_VERSION(1, 4);
693700
}
701+
702+
struct fi_info hints = {0};
703+
struct fi_ep_attr ep_attr = {0};
704+
struct fi_fabric_attr fabric_attr = {0};
705+
706+
/* We only want providers named "usnic" that are of type EP_DGRAM */
707+
fabric_attr.prov_name = "usnic";
708+
ep_attr.type = FI_EP_DGRAM;
709+
710+
hints.caps = FI_MSG;
711+
hints.mode = FI_LOCAL_MR | FI_MSG_PREFIX;
712+
hints.addr_format = FI_SOCKADDR;
713+
hints.ep_attr = &ep_attr;
714+
hints.fabric_attr = &fabric_attr;
715+
716+
ret = fi_getinfo(libfabric_api, NULL, 0, 0, &hints, &info_list);
694717
if (0 != ret) {
695718
opal_output_verbose(5, USNIC_OUT,
696-
"btl:usnic: disqualifiying myself due to fi_getinfo failure: %s (%d)", strerror(-ret), ret);
719+
"btl:usnic: disqualifiying myself due to fi_getinfo(3) failure: %s (%d)", strerror(-ret), ret);
697720
return NULL;
698721
}
699722

0 commit comments

Comments
 (0)