Remove fabrics_ext, because this module would break the promise of seamless interopability.

lib/fabrics/include/mxl/fabrics.h

@@ -233,9 +233,6 @@ extern "C"
      * \param in_startSlice The start slice in the slice range to transfer. This is inclusive.
      * \param in_endSlice The end slice in the slice range to transfer. This is exclusive.
      * \return The result code. \see mxlStatus
-     * \note This function assumes: (1) the underlying buffer layout matches the MXL grain data layout, and (2) ring buffer entries for local and
-     * remote regions can be calculated via modulo operation: `grainIndex % regions.size()`.If these assumptions do not hold, use
-     * mxlFabricsExtInitiatorTransferGrain() instead, see fabrics_ext.h.
      */
     MXL_EXPORT
     mxlStatus mxlFabricsInitiatorTransferGrain(mxlFabricsInitiator in_initiator, uint64_t in_grainIndex, uint16_t in_startSlice,

lib/fabrics/ofi/src/fabrics.cpp

@@ -561,47 +561,3 @@ mxlStatus mxlFabricsFreeTargetInfo(mxlFabricsTargetInfo in_info)
         },
         "Failed to free target info object");
 }
-
-extern "C" MXL_EXPORT
-mxlStatus mxlFabricsExtGetRegions(mxlFabricsExtRegionsConfig const* in_config, mxlFabricsRegions* out_regions)
-{
-    if ((in_config == nullptr) || (out_regions == nullptr))
-    {
-        return MXL_ERR_INVALID_ARG;
-    }
-
-    return ofi::try_run(
-        [&]()
-        {
-            auto regions = ofi::mxlFabricsRegionsFromUser(*in_config);
-
-            // We are leaking the ownership, the user is responsible for calling mxlFabricsRegionsFree to free the memory.
-            auto regionPtr = std::make_unique<ofi::MxlRegions>(regions).release();
-
-            *out_regions = regionPtr->toAPI();
-
-            return MXL_STATUS_OK;
-        },
-        "Failed to create regions object");
-}
-
-extern "C" MXL_EXPORT
-mxlStatus mxlFabricsExtInitiatorTransferGrain(mxlFabricsInitiator in_initiator, mxlFabricsTargetInfo const in_targetInfo, uint64_t in_localIndex,
-    uint64_t in_remoteIndex, uint64_t in_payloadOffset, uint16_t in_startSlice, uint16_t in_endSlice)
-{
-    if ((in_initiator == nullptr) || (in_targetInfo == nullptr))
-    {
-        return MXL_ERR_INVALID_ARG;
-    }
-
-    return ofi::try_run(
-        [&]()
-        {
-            auto targetInfo = ofi::TargetInfo::fromAPI(in_targetInfo);
-            ofi::InitiatorWrapper::fromAPI(in_initiator)
-                ->transferGrainToTarget(targetInfo->id, in_localIndex, in_remoteIndex, in_payloadOffset, in_startSlice, in_endSlice);
-
-            return MXL_STATUS_OK;
-        },
-        "Failed to transfer grain to target");
-}

lib/fabrics/ofi/src/internal/Region.cpp

@@ -60,16 +60,6 @@ namespace mxl::lib::fabrics::ofi
         return std::holds_alternative<Host>(_inner);
     }
 
-    Region::Location Region::Location::fromAPI(mxlFabricsExtMemoryRegionLocation loc)
-    {
-        switch (loc.type)
-        {
-            case MXL_PAYLOAD_LOCATION_HOST_MEMORY:   return Location::host();
-            case MXL_PAYLOAD_LOCATION_DEVICE_MEMORY: return Location::cuda(static_cast<int>(loc.deviceId));
-            default:                                 throw Exception::invalidArgument("Invalid memory region type");
-        }
-    }
-
     std::string Region::Location::toString() const noexcept
     {
         return std::visit(
@@ -183,21 +173,4 @@ namespace mxl::lib::fabrics::ofi
             throw Exception::make(MXL_ERR_UNKNOWN, "Unsupported flow fromat {}", flow.flowInfo()->config.common.format);
         }
     }
-
-    MxlRegions mxlFabricsRegionsFromUser(mxlFabricsExtRegionsConfig const& config)
-    {
-        auto outRegions = std::vector<Region>{};
-        for (std::size_t i = 0; i < config.regionsCount; i++)
-        {
-            outRegions.emplace_back(config.regions[i].addr, config.regions[i].size, Region::Location::fromAPI(config.regions[i].loc));
-        }
-        if (config.format == MXL_DATA_FORMAT_VIDEO)
-        {
-            return {std::move(outRegions), DataLayout::fromVideo(std::to_array(config.sliceSize))};
-        }
-        else
-        {
-            throw Exception::invalidArgument("Unsupported data format {}", static_cast<int>(config.format));
-        }
-    }
 }

lib/fabrics/ofi/src/internal/Region.hpp

@@ -13,7 +13,6 @@
 #include <mxl-internal/FlowData.hpp>
 #include <rdma/fi_domain.h>
 #include "mxl/fabrics.h"
-#include "mxl/fabrics_ext.h"
 #include "DataLayout.hpp"
 
 namespace mxl::lib::fabrics::ofi
@@ -41,11 +40,6 @@ namespace mxl::lib::fabrics::ofi
             [[nodiscard]]
             static Location cuda(int deviceId) noexcept;
 
-            /** \brief Convert between external and internal versions of this type
-             */
-            [[nodiscard]]
-            static Location fromAPI(mxlFabricsExtMemoryRegionLocation loc);
-
             /** \brief Return the device id. For host location 0 is returned.
              */
             [[nodiscard]]
@@ -203,11 +197,15 @@ namespace mxl::lib::fabrics::ofi
     class MxlRegions
     {
     public:
+        MxlRegions(std::vector<Region> regions, DataLayout dataLayout)
+            : _regions(std::move(regions))
+            , _layout(std::move(dataLayout))
+        {}
+
         /** \brief Convert between external and internal versions of this type
          */
         [[nodiscard]]
         static MxlRegions* fromAPI(mxlFabricsRegions regions) noexcept;
-        /** \copydoc fromAPI() */
 
         [[nodiscard]]
         mxlFabricsRegions toAPI() noexcept;
@@ -222,13 +220,6 @@ namespace mxl::lib::fabrics::ofi
 
     private:
         friend MxlRegions mxlFabricsRegionsFromFlow(FlowData const& flow);
-        friend MxlRegions mxlFabricsRegionsFromUser(mxlFabricsExtRegionsConfig const& config);
-
-    private:
-        MxlRegions(std::vector<Region> regions, DataLayout dataLayout)
-            : _regions(std::move(regions))
-            , _layout(std::move(dataLayout))
-        {}
 
     private:
         std::vector<Region> _regions;
@@ -240,10 +231,4 @@ namespace mxl::lib::fabrics::ofi
      */
     [[nodiscard]]
     MxlRegions mxlFabricsRegionsFromFlow(FlowData const& flow);
-
-    /** \brief Convert user-provided memory regions to MxlRegions.
-     * Used to convert mxlFabricsMemoryRegion arrays provided by the user.
-     */
-    [[nodiscard]]
-    MxlRegions mxlFabricsRegionsFromUser(mxlFabricsExtRegionsConfig const& config);
 }

lib/tests/fabrics/ofi/Util.hpp

@@ -11,7 +11,7 @@
 #include <fmt/format.h>
 #include <rdma/fabric.h>
 #include "mxl/fabrics.h"
-#include "mxl/fabrics_ext.h"
+#include "DataLayout.hpp"
 #include "Domain.hpp"
 #include "Region.hpp"
 
@@ -72,64 +72,40 @@ namespace mxl::lib::fabrics::ofi
         return domain;
     }
 
+    inline MxlRegions getEmptyVideoMxlRegions()
+    {
+        return MxlRegions({}, DataLayout::fromVideo({8, 0, 0, 0}));
+    }
+
     inline std::pair<MxlRegions, InnerRegions> getHostRegionGroups()
     {
+        /// Warning: Do not modify the values below, you will break many tests
         auto innerRegions = std::vector<std::vector<std::uint8_t>>{
             std::vector<std::uint8_t>(256),
             std::vector<std::uint8_t>(512),
             std::vector<std::uint8_t>(1024),
             std::vector<std::uint8_t>(2048),
         };
 
-        /// Warning: Do not modify the values below, you will break many tests
-        // clang-format off
-        auto mxlFabricsRegions =  std::vector<mxlFabricsExtMemoryRegion>{
-            mxlFabricsExtMemoryRegion{
-                .addr = reinterpret_cast<std::uintptr_t>(innerRegions[0].data()),
-                .size = innerRegions[0].size(),
-                .loc = {.type = MXL_PAYLOAD_LOCATION_HOST_MEMORY, .deviceId = 0},
-            },
-            mxlFabricsExtMemoryRegion{
-                .addr = reinterpret_cast<std::uintptr_t>(innerRegions[1].data()),
-                .size = innerRegions[1].size(),
-                .loc = {.type = MXL_PAYLOAD_LOCATION_HOST_MEMORY, .deviceId = 0},
-            },
-            mxlFabricsExtMemoryRegion{
-                .addr = reinterpret_cast<std::uintptr_t>(innerRegions[2].data()),
-                .size = innerRegions[2].size(),
-                .loc = {.type = MXL_PAYLOAD_LOCATION_HOST_MEMORY, .deviceId = 0},
-            },
-            mxlFabricsExtMemoryRegion{
-                .addr = reinterpret_cast<std::uintptr_t>(innerRegions[3].data()),
-                .size = innerRegions[3].size(),
-                .loc = {.type = MXL_PAYLOAD_LOCATION_HOST_MEMORY, .deviceId = 0},
-            }
-        };
-
-
-        auto config = mxlFabricsExtRegionsConfig{.regions=mxlFabricsRegions.data(), .regionsCount = mxlFabricsRegions.size(), .sliceSize={8,0,0,0},.format=MXL_DATA_FORMAT_VIDEO};
-
-        // clang-format on
+        std::vector<Region> regions;
+        for (auto const& innerRegion : innerRegions)
+        {
+            regions.emplace_back(*innerRegion.data(), innerRegion.size());
+        }
 
-        return {mxlFabricsRegionsFromUser(config), innerRegions};
+        auto mxlRegions = MxlRegions(regions, DataLayout::fromVideo({8, 0, 0, 0}));
+        return {mxlRegions, innerRegions};
     }
 
-    inline std::pair<mxlFabricsRegions, InnerRegions> getUserMxlRegions()
+    inline MxlRegions getMxlRegions(std::vector<std::vector<std::uint8_t>> const& innerRegions,
+        DataLayout dataLayout = DataLayout::fromVideo({8, 0, 0, 0}))
     {
-        auto regions = InnerRegions{InnerRegion(256)};
-        auto memoryRegions = std::vector<mxlFabricsExtMemoryRegion>{
-            mxlFabricsExtMemoryRegion{.addr = reinterpret_cast<std::uintptr_t>(regions[0].data()),
-                                      .size = regions[0].size(),
-                                      .loc = {.type = MXL_PAYLOAD_LOCATION_HOST_MEMORY, .deviceId = 0}},
-        };
-        auto config = mxlFabricsExtRegionsConfig{
-            .regions = memoryRegions.data(), .regionsCount = memoryRegions.size(), .sliceSize = {8, 0, 0, 0},
-                    .format = MXL_DATA_FORMAT_VIDEO
-        };
-
-        mxlFabricsRegions outRegions;
-        mxlFabricsExtGetRegions(&config, &outRegions);
-
-        return {outRegions, regions};
+        std::vector<Region> regions;
+        for (auto const& innerRegion : innerRegions)
+        {
+            regions.emplace_back(*innerRegion.data(), innerRegion.size());
+        }
+        return {regions, dataLayout};
     }
+
 }

lib/tests/fabrics/ofi/test_Region.cpp

@@ -36,38 +36,3 @@ TEST_CASE("ofi: RegionGroup view and iovec conversion", "[ofi][RegionGroup]")
     REQUIRE(iovecs[1].iov_base == reinterpret_cast<void*>(0x2000));
     REQUIRE(iovecs[1].iov_len == 128);
 }
-
-TEST_CASE("ofi: RegionGroups fromGroups and view", "[ofi][RegionGroups]")
-{
-    // clang-format off
-    auto inputRegions = std::array<mxlFabricsExtMemoryRegion, 1>{
-                mxlFabricsExtMemoryRegion{
-                    .addr = 0x3000,
-                    .size = 256,
-                    .loc = {.type = MXL_PAYLOAD_LOCATION_HOST_MEMORY, .deviceId = 0},
-                }};
-    // clang-format on
-
-    auto config = mxlFabricsExtRegionsConfig{
-        .regions = inputRegions.data(),
-        .regionsCount = 1,
-        .sliceSize = {8, 0, 0, 0},
-        .format = MXL_DATA_FORMAT_VIDEO,
-    };
-
-    auto mxlFabricsRegions = mxlFabricsRegionsFromUser(config);
-
-    REQUIRE(mxlFabricsRegions.regions().size() == 1);
-
-    auto const& region = mxlFabricsRegions.regions()[0];
-    REQUIRE(region.base == 0x3000);
-    REQUIRE(region.size == 256);
-    REQUIRE(region.loc.isHost());
-
-    auto dataLayout = mxlFabricsRegions.dataLayout();
-
-    REQUIRE(dataLayout.isVideo());
-
-    auto videoLayout = dataLayout.asVideo();
-    REQUIRE(videoLayout.sliceSizes[0] == 8);
-}