Skip to content

Commit

Permalink
NetworkPkg/NvmeOfDxe: Resolve NBFT population issues (#35)
Browse files Browse the repository at this point in the history 
- Adjust HFI population logic to no longer skip by MAC, instead check
  for a unique transport descriptor hash
- Discovery ctlr population logic no longer treats each
  DeviceAdapter as an individual Discovery ctlr; Discovery ctlr entries
now populated based on order in which unique Disc. ctlrs were found
- Properly map SSNS, Discovery ctlrs to their respective primary HFI
- Properly map SSNS records to their respective primary Discovery ctlr

Signed-off-by: Trevor cockrell <trevor_cockrell@dell.com>
  • Loading branch information
@trevor-cockrell
trevor-cockrell authored Aug 29, 2024
1 parent 759aab4 commit 6018114
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 60 deletions.
5 changes: 3 additions & 2 deletions NetworkPkg/NvmeOfDxe/NvmeOfDriver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
NvmeOfDxe driver is used to manage non-volatile memory subsystem which follows
NVM Express Over Fabric TCP specification.
Copyright (c) 2021 - 2023, Dell Inc. or its subsidiaries. All Rights Reserved.<BR>
Copyright (c) 2021 - 2024, Dell Inc. or its subsidiaries. All Rights Reserved.<BR>
Copyright (c) 2022 - 2023, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
Expand Down Expand Up @@ -293,7 +293,8 @@ extern NVMEOF_NQN_NID gNvmeOfNqnNidMap[];
extern UINT8 NqnNidMapINdex;

typedef struct _NVMEOF_NBFT {
UINT8 DeviceAdapterIndex;
UINT8 PrimaryHfiIndex;
UINT8 PrimaryDiscoveryCtlrIndex;
BOOLEAN IsDiscoveryNqn;
NVMEOF_DEVICE_PRIVATE_DATA *Device;
NVMEOF_ATTEMPT_CONFIG_NVDATA *AttemptData;
Expand Down
143 changes: 87 additions & 56 deletions NetworkPkg/NvmeOfDxe/NvmeOfNbft.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,14 +389,14 @@ NvmeOfFillHostFabricInterfaceSections (
NVMEOF_ATTEMPT_ENTRY *AttemptEntry;
NVMEOF_ATTEMPT_CONFIG_NVDATA *Attempt;
NVMEOF_NIC_INFO *NicInfo;
CHAR16 AttemptMacString[NVMEOF_MAX_MAC_STRING_LEN] = { 0 };
CHAR16 MacString[NVMEOF_MAX_MAC_STRING_LEN];
BOOLEAN AlreadyProcessed;
LIST_ENTRY *ProcessedEntry;
LIST_ENTRY *NextEntryProcessed;
NVMEOF_PROCESSED_MAC *ProcessedAdapter;
NVMEOF_PROCESSED_HFI *ProcessedAdapter;
UINT8 **HostOverridesHeapRef;
UINT8 NvmeofHostNqnOverrideTmp[NVMEOF_NAME_MAX_SIZE];
UINT32 HfiTrDescriptorHashTmp = 0;

//
// Get the offset of the first hfi section.
Expand All @@ -419,19 +419,6 @@ NvmeOfFillHostFabricInterfaceSections (
continue;
}

// Logic to skip already processed adapter by comparing MAC
AlreadyProcessed = FALSE;
NET_LIST_FOR_EACH_SAFE (ProcessedEntry, NextEntryProcessed, &gAddedAdaptersList) {
ProcessedAdapter = NET_LIST_USER_STRUCT (ProcessedEntry, NVMEOF_PROCESSED_MAC, Link);
if (AsciiStrCmp (Attempt->MacString, ProcessedAdapter->MacString) == 0) {
AlreadyProcessed = TRUE;
break;
}
}
if (AlreadyProcessed) {
continue;
}

//
// Fill the Host fabric interface header section.
//
Expand Down Expand Up @@ -513,26 +500,49 @@ NvmeOfFillHostFabricInterfaceSections (
MacString
);

// Get the Crc32 hash of the HFI transport descriptor minus the header
UINTN HfiTrHeaderSize = sizeof (EFI_ACPI_NVMEOF_BFT_HFI_TRANSPORT_INFO_DESCRIPTOR_HEADER);

HfiTrDescriptorHashTmp = CalculateCrc32 (
(UINT8 *)&HfiTcpTransportInfo + (UINT8)HfiTrHeaderSize,
sizeof (EFI_ACPI_NVMEOF_BFT_HFI_TRANSPORT_INFO_DESCRIPTOR_TCP) - HfiTrHeaderSize
);

// Logic to skip already processed HFI by comparing hash of transport descriptor
AlreadyProcessed = FALSE;
NET_LIST_FOR_EACH_SAFE (ProcessedEntry, NextEntryProcessed, &gAddedAdaptersList) {
ProcessedAdapter = NET_LIST_USER_STRUCT (ProcessedEntry, NVMEOF_PROCESSED_HFI, Link);
if (ProcessedAdapter->HfiTrDescriptorHash == HfiTrDescriptorHashTmp) {
AlreadyProcessed = TRUE;
break;
}
}

for (Index = 0; Index < gNvmeOfNbftListIndex; Index++) {
AsciiStrToUnicodeStrS (
gNvmeOfNbftList[Index].AttemptData->MacString,
AttemptMacString,
sizeof (AttemptMacString) / sizeof (AttemptMacString[0])
);
if (StrCmp (MacString, AttemptMacString) != 0) {
continue;
} else {
gNvmeOfNbftList[Index].DeviceAdapterIndex = HfiHeader->Index;
if (AsciiStrCmp (
gNvmeOfNbftList[Index].AttemptData->SubsysConfigData.NvmeofSubsysNqn,
NVMEOF_DISCOVERY_NQN
) == 0)
{
gNvmeOfNbftList[Index].IsDiscoveryNqn = TRUE;
// Update the NvmeOfNbftList with the primary HFI index
if (gNvmeOfNbftList[Index].AttemptData == Attempt) {
if (AlreadyProcessed) {
gNvmeOfNbftList[Index].PrimaryHfiIndex = ProcessedAdapter->HfiHeaderRef->Index;
} else {
gNvmeOfNbftList[Index].IsDiscoveryNqn = FALSE;
gNvmeOfNbftList[Index].PrimaryHfiIndex = HfiHeader->Index;
}
}

// Update the NvmeOfNbftList with the discovery NQN flag
if (AsciiStrCmp (
gNvmeOfNbftList[Index].AttemptData->SubsysConfigData.NvmeofSubsysNqn,
NVMEOF_DISCOVERY_NQN
) == 0)
{
gNvmeOfNbftList[Index].IsDiscoveryNqn = TRUE;
} else {
gNvmeOfNbftList[Index].IsDiscoveryNqn = FALSE;
}
}

// gNvmeOfNbft list has been updated; if the HFI was already processed then skip adding to NBFT
if (AlreadyProcessed) {
continue;
}

// Copy HFI tenasport info to heap and update the HFI header structure
Expand All @@ -543,15 +553,16 @@ NvmeOfFillHostFabricInterfaceSections (
NvmeOfAddHeapItem (Heap, NvData->HostName, HfiTcpTransportInfo.HostNameLength);

// Add an entry to processed adapter list
ProcessedAdapter = (NVMEOF_PROCESSED_MAC *)AllocateZeroPool (sizeof (NVMEOF_PROCESSED_MAC));
ProcessedAdapter = (NVMEOF_PROCESSED_HFI *)AllocateZeroPool (sizeof (NVMEOF_PROCESSED_HFI));
if (ProcessedAdapter == NULL) {
return;
}

CopyMem (ProcessedAdapter->MacString, Attempt->MacString, sizeof (ProcessedAdapter->MacString));
ProcessedAdapter->HfiHeaderRef = HfiHeader;
ProcessedAdapter->HostOverrideEnable = NvData->HostOverrideEnable;
ProcessedAdapter->HeapRef = HostOverridesHeapRef;
ProcessedAdapter->HfiHeaderRef = HfiHeader;
ProcessedAdapter->HfiTrDescriptorHash = HfiTrDescriptorHashTmp;
ProcessedAdapter->HostOverrideEnable = NvData->HostOverrideEnable;
ProcessedAdapter->HeapRef = HostOverridesHeapRef;
InsertTailList (&gAddedAdaptersList, &ProcessedAdapter->Link);

//
Expand Down Expand Up @@ -584,9 +595,10 @@ NvmeOfFillSubsystemNamespaceSection (
EFI_ACPI_NVMEOF_BFT_SUBSYSTEM_NAMESPACE_DESCRIPTOR *SubsystemNamespace;
EFI_ACPI_NVMEOF_BFT_CONTROL_STRUCTURE *Control;
EFI_ACPI_NVMEOF_BFT_SUBSYSTEM_EXT_INFO_DESCRIPTOR SsnsExtInfo = { 0 };
EFI_ACPI_NVMEOF_BFT_HFI_HEADER_DESCRIPTOR *HfiHeader;
UINTN DeviceIndex = 0;
UINT8 Index;
UINT8 IndexTmp;
UINT8 CurrentDiscIndex = 0;
LIST_ENTRY *ProcessedEntry;
LIST_ENTRY *NextEntryProcessed;
NVMEOF_PROCESSED_NAMESPACE *ProcessedNamespace;
Expand All @@ -598,8 +610,6 @@ NvmeOfFillSubsystemNamespaceSection (
SubsystemNamespace = (EFI_ACPI_NVMEOF_BFT_SUBSYSTEM_NAMESPACE_DESCRIPTOR *)((UINTN)Table +
Control->HfiDescOffset +
(Control->NumHfis * NBFT_ROUNDUP (sizeof (EFI_ACPI_NVMEOF_BFT_HFI_HEADER_DESCRIPTOR))));
HfiHeader = (EFI_ACPI_NVMEOF_BFT_HFI_HEADER_DESCRIPTOR *)((UINTN)Table +
Control->HostDescOffset + NBFT_ROUNDUP (sizeof (EFI_ACPI_NVMEOF_BFT_HOST_DESCRIPTOR)));

// Fill the offset of 1st namespace structure in control structure
Control->SubSytemDescOffset = (UINT16)((UINTN)SubsystemNamespace - (UINTN)Table);
Expand All @@ -613,7 +623,7 @@ NvmeOfFillSubsystemNamespaceSection (
Control->SubSystemVersion = EFI_ACPI_NVMEOF_BFT_SUBSYSTEM_DESCRIPTOR_VERSION;
Control->SubSystemDescLength = (UINT16)sizeof (EFI_ACPI_NVMEOF_BFT_SUBSYSTEM_NAMESPACE_DESCRIPTOR);
SubsystemNamespace->Index = (UINT16)DeviceIndex + 1;
SubsystemNamespace->Flags |= EFI_ACPI_NVMEOF_BFT_SUBSYSTEM_DESCRIPTOR_FLAG_BLOCK_VALID;
SubsystemNamespace->Flags = EFI_ACPI_NVMEOF_BFT_SUBSYSTEM_DESCRIPTOR_FLAG_BLOCK_VALID;
SubsystemNamespace->SubsystemTransportAdressLength = sizeof (EFI_IPv6_ADDRESS);
SubsystemNamespace->HfiAssociationLen = sizeof (UINT8);

Expand All @@ -627,8 +637,30 @@ NvmeOfFillSubsystemNamespaceSection (

// Transport type
SubsystemNamespace->TransportType = NVMEOF_TRANSPORT_TCP;

// Primary Discovery Controller index
AlreadyProcessed = FALSE;
if (gNvmeOfNbftList[Index].IsDiscoveryNqn) {
SubsystemNamespace->PrimaryDiscoveryCtrlrIndex = gNvmeOfNbftList[Index].DeviceAdapterIndex;
// Check if current NBFT entry is from an already processed attempt
for (IndexTmp = 0; IndexTmp < Index; IndexTmp++) {
if (AsciiStrCmp (
gNvmeOfNbftList[IndexTmp].AttemptData->AttemptName,
gNvmeOfNbftList[Index].AttemptData->AttemptName
) == 0)
{
AlreadyProcessed = TRUE;
break;
}
}

// Only increment the Discovery Controller index for new controllers (attempts)
if (AlreadyProcessed) {
gNvmeOfNbftList[Index].PrimaryDiscoveryCtlrIndex = gNvmeOfNbftList[IndexTmp].PrimaryDiscoveryCtlrIndex;
} else {
gNvmeOfNbftList[Index].PrimaryDiscoveryCtlrIndex = ++CurrentDiscIndex;
}

SubsystemNamespace->PrimaryDiscoveryCtrlrIndex = gNvmeOfNbftList[Index].PrimaryDiscoveryCtlrIndex;
// Update flag
SubsystemNamespace->Flags |= EFI_ACPI_NVMEOF_BFT_SUBSYSTEM_DESCRIPTOR_FLAG_DISCOVERED_NAMESPACE;
}
Expand Down Expand Up @@ -667,9 +699,9 @@ NvmeOfFillSubsystemNamespaceSection (
Len = AsciiStrLen (gNvmeOfNbftList[Index].FailTridInfo->trsvcid);
SubsystemNamespace->SubsystemTransportServiceIdLength = Len;
NvmeOfAddHeapItem (Heap, gNvmeOfNbftList[Index].FailTridInfo->trsvcid, Len);
SubsystemNamespace->PrimaryHfiDescriptorIndex = HfiHeader->Index;
SubsystemNamespace->PrimaryHfiDescriptorIndex = gNvmeOfNbftList[Index].PrimaryHfiIndex;
// HFI association
NvmeOfAddHeapItem (Heap, &(gNvmeOfNbftList[Index].DeviceAdapterIndex), sizeof (UINT8));
NvmeOfAddHeapItem (Heap, &(gNvmeOfNbftList[Index].PrimaryHfiIndex), sizeof (UINT8));
// Fill the subsystem NQN into the heap.
Len = (UINT16)AsciiStrLen (gNvmeOfNbftList[Index].FailTridInfo->subnqn);
NvmeOfAddHeapItem (Heap, gNvmeOfNbftList[Index].FailTridInfo->subnqn, Len);
Expand Down Expand Up @@ -697,7 +729,7 @@ NvmeOfFillSubsystemNamespaceSection (
}
}
if (AlreadyProcessed) {
NvmeOfAddHeapItem (Heap, &(gNvmeOfNbftList[Index].DeviceAdapterIndex), sizeof (UINT8));
NvmeOfAddHeapItem (Heap, &(gNvmeOfNbftList[Index].PrimaryHfiIndex), sizeof (UINT8));
SubsystemNamespace->HfiAssociationLen += sizeof (UINT8);
continue;
}
Expand Down Expand Up @@ -740,9 +772,9 @@ NvmeOfFillSubsystemNamespaceSection (
sizeof (SubsystemNamespace->Nid)
);

SubsystemNamespace->PrimaryHfiDescriptorIndex = HfiHeader->Index;
SubsystemNamespace->PrimaryHfiDescriptorIndex = gNvmeOfNbftList[Index].PrimaryHfiIndex;
// HFI association
NvmeOfAddHeapItem (Heap, &(gNvmeOfNbftList[Index].DeviceAdapterIndex), sizeof (UINT8));
NvmeOfAddHeapItem (Heap, &(gNvmeOfNbftList[Index].PrimaryHfiIndex), sizeof (UINT8));
SubsystemNamespace->HfiAssociationLen = sizeof (UINT8);

// Fill the subsystem NQN into the heap.
Expand Down Expand Up @@ -827,14 +859,12 @@ NvmeOfFillDiscoverySection (

// Find the number of discovery controllers in the list
for (Index = 0; Index < gNvmeOfNbftListIndex; Index++) {
if ((gNvmeOfNbftList[Index].DeviceAdapterIndex != AdapterIndex) &&
gNvmeOfNbftList[Index].IsDiscoveryNqn)
{
MaxDiscoveryDetailsIndex++;
AdapterIndex = gNvmeOfNbftList[Index].DeviceAdapterIndex;
if (gNvmeOfNbftList[Index].PrimaryDiscoveryCtlrIndex > MaxDiscoveryDetailsIndex) {
MaxDiscoveryDetailsIndex = gNvmeOfNbftList[Index].PrimaryDiscoveryCtlrIndex;
}
}

DEBUG ((DEBUG_INFO, "MaxDiscoveryDetailsIndex: %d\n", MaxDiscoveryDetailsIndex));
if (MaxDiscoveryDetailsIndex == 0) {
// No need to fill this structure as we have no discovery controller
Control->NumDiscoveryEntires = 0;
Expand All @@ -850,16 +880,17 @@ NvmeOfFillDiscoverySection (
}

Discovery->Index = 0;
AdapterIndex = -1;
AdapterIndex = 0;
UINT8 NumRecs = 0;

// Find the number of discovery records
for (Index = 0; Index < gNvmeOfNbftListIndex; Index++) {
if ((gNvmeOfNbftList[Index].DeviceAdapterIndex != AdapterIndex) &&
if ((gNvmeOfNbftList[Index].PrimaryDiscoveryCtlrIndex != AdapterIndex) &&
gNvmeOfNbftList[Index].IsDiscoveryNqn)
{
AdapterIndex = DiscoveryDetails[NumRecs].AdapterIndex =
gNvmeOfNbftList[Index].DeviceAdapterIndex;
gNvmeOfNbftList[Index].PrimaryDiscoveryCtlrIndex;
DiscoveryDetails[NumRecs].PrimaryHfiIndex = gNvmeOfNbftList[Index].PrimaryHfiIndex;
DiscoveryDetails[NumRecs].SecurityProfileIndex = 0;
DiscoveryDetails[NumRecs].Port =
gNvmeOfNbftList[Index].AttemptData->SubsysConfigData.NvmeofSubsysPortId;
Expand Down Expand Up @@ -891,7 +922,7 @@ NvmeOfFillDiscoverySection (
Control->DiscoDescLengh = (UINT16)sizeof (EFI_ACPI_NVMEOF_BFT_DISCOVERY_DESCRIPTOR);
Discovery->Flags = EFI_ACPI_NVMEOF_BFT_DISCOVERY_DESCRIPTOR_FLAG_BLOCK_VALID;

Discovery->HfiDescriptorIndex = DiscoveryDetails[Index].AdapterIndex;
Discovery->HfiDescriptorIndex = DiscoveryDetails[Index].PrimaryHfiIndex;
Discovery->SecurityProfileDescriptorIndex =
DiscoveryDetails[Index].SecurityProfileIndex;
NvmeOfIpToStr (
Expand Down Expand Up @@ -950,7 +981,7 @@ NvmeOfPublishNbft (
EFI_ACPI_DESCRIPTION_HEADER *Xsdt = NULL;
UINT8 Checksum;
UINT8 *Heap = NULL;
NVMEOF_PROCESSED_MAC *ProcessedAdapter = NULL;
NVMEOF_PROCESSED_HFI *ProcessedAdapter = NULL;
NVMEOF_PROCESSED_NAMESPACE *ProcessedNamespace = NULL;
NVMEOF_PROCESSED_IP_ADDR *ProcessedIpConfig = NULL;
LIST_ENTRY *ProcessedEntry = NULL;
Expand Down Expand Up @@ -1067,7 +1098,7 @@ NvmeOfPublishNbft (
Error:
// Remove entries from the processed adapter list
NET_LIST_FOR_EACH_SAFE (ProcessedEntry, NextProcessedEntry, &gAddedAdaptersList) {
ProcessedAdapter = NET_LIST_USER_STRUCT (ProcessedEntry, NVMEOF_PROCESSED_MAC, Link);
ProcessedAdapter = NET_LIST_USER_STRUCT (ProcessedEntry, NVMEOF_PROCESSED_HFI, Link);
if (ProcessedAdapter != NULL) {
RemoveEntryList (&ProcessedAdapter->Link);
FreePool (ProcessedAdapter);
Expand Down
6 changes: 4 additions & 2 deletions NetworkPkg/NvmeOfDxe/NvmeOfNbft.h
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/** @file
Function definitions for nBFT.
Copyright (c) 2021 - 2023, Dell Inc. or its subsidiaries. All Rights Reserved.<BR>
Copyright (c) 2021 - 2024, Dell Inc. or its subsidiaries. All Rights Reserved.<BR>
Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
Expand Down Expand Up @@ -35,7 +35,8 @@ typedef struct {
CHAR8 MacString[NVMEOF_MAX_MAC_STRING_LEN];
BOOLEAN HostOverrideEnable;
EFI_ACPI_NVMEOF_BFT_HFI_HEADER_DESCRIPTOR *HfiHeaderRef;
} NVMEOF_PROCESSED_MAC;
UINTN HfiTrDescriptorHash;
} NVMEOF_PROCESSED_HFI;

typedef struct {
LIST_ENTRY Link;
Expand All @@ -50,6 +51,7 @@ typedef struct {

typedef struct {
UINT8 AdapterIndex;
UINT8 PrimaryHfiIndex;
BOOLEAN Ipv6Flag;
UINT8 SecurityProfileIndex;
UINT16 Port;
Expand Down

0 comments on commit 6018114

Please sign in to comment.