Skip to content

[RHEL-22354] Completion of NBL chains #1346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 21, 2025

Conversation

ybendito
Copy link
Collaborator

No description provided.

The new logic to support chains is placed under ifdef currently
and the driver works in backward compatible mode when all the
procedures related to chain information in the NBL are stubs.

The new logic that takes in account NBL chain is:
For single NBL (not chained) - all 3 fields in m_Chain are 0.
For first NBL in chain:
    m_FirstInChain = 0
    m_NumChained is number of NBLs following the first one
    m_NumCompleted is incremented on each NBL completion
For following NBLs in chain:
    m_FirstInChain is CNBL with first NBL
    m_NumChained is serial number in chain
    m_NumCompleted is not used (0)
Each chained CNBL before mapping AddRef's the first one on
    creation and Release's it in destructor, so lifetime of
    the first CNBL is alive till one or more of chained ones
    alive.
The condition when the CNBL is done and can be released and
    completed:
    Single NBL - as before (all NBs are completed)
    First in chain - the same and all the chained NBLs are done
    Chained - the same and the first one is done
Such a way, all the CNBLs for chained NBLs become 'done' at the
    same moment

Signed-off-by: Yuri Benditovich <[email protected]>
Signed-off-by: Yuri Benditovich <[email protected]>
Connection is src:dest IP addresses and ports, so
for all the packets sent to specific connection the
hash should be the same. We collect NBL chain till all
the indirection indices are the same, then flush
the chain to the TX queue and start a new chain.

Signed-off-by: Yuri Benditovich <[email protected]>
Chain NBLs from the list of completed CNBLs as they appear
in the list and do not invert them. Later we're going to
add NBL chains from the 'first in chain' CNBLs.

Signed-off-by: Yuri Benditovich <[email protected]>
Keep the order of NBLs wherever possible, i.e. push them at
the end of waiting list and then process waiting list and
remove from it all the completed NBLs, so that earlier NBLs
are removed first.

Signed-off-by: Yuri Benditovich <[email protected]>
Jira link: https://issues.redhat.com/browse/RHEL-22354
See also: https://issues.redhat.com/browse/RHEL-85605

The NBL chain support is still disabled but the code for it
is functional. What it does if we enable it:
Deliver entire NBL chain if one was provided in Send() call.
Each CNBL created for each 'next' NBL (following to the first
one) on completion of all it's NBs detaches it's NBL and
attaches it to the m_NBL of the first one. Such a way when
all the CNBLs related to the same chain are ready, only first
one has m_NBL (with all the rest of NBLs chained to it).
For all the rest of CNBLs the m_NBL is NULL.
The order of NBLs in the chain might be slightly different
than original one (due to NB completions running simultaneously
in different flows), for that the chain is tested and repaired
if needed before it is indicated up.

Signed-off-by: Yuri Benditovich <[email protected]>
@ybendito ybendito requested a review from YanVugenfirer as a code owner April 12, 2025 12:53
Jira link: https://issues.redhat.com/browse/RHEL-22354
See also: https://issues.redhat.com/browse/RHEL-85605

Enable NBL chain processing.
See previous commit for technical details.

If in future for any purpose there is a need to
disable this functionality completely, it is enough
to define NBL_CHAINS as 0, then each CNBL is
independent and the way code works is backward-compatible.

Signed-off-by: Yuri Benditovich <[email protected]>
@ybendito
Copy link
Collaborator Author

ybendito commented Apr 14, 2025

Win11 - 3 failures:
Offload parity - test was ok, WTL file is empty, 2 reruns are ok
Detect Malicious Software - by mistake I've pressed Ctrl-C on it, rerun is ok
2c_Mini6Stress - BSOD in ndtest30, binding to adapter timed out. 2 reruns are OK. Seen before once (PlanIO) and no visible reason in our miniport how this depends on us (the miniport is in paused state, this state is reflected in NDIS state, no ndis operation in progress). Not reproduced in 10 runs of this test on my setup and also in any other 2c test.

IMO, can be merged.

bool HaveDetachedBuffers()
{
return m_MappedBuffersDetached != 0;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@@ -446,18 +446,37 @@ static VOID ParaNdis6_SendNetBufferLists(NDIS_HANDLE miniportAdapterContext,
}
#ifdef PARANDIS_SUPPORT_RSS
CNdisPassiveReadAutoLock autoLock(pContext->RSSParameters.rwLock);
if (pContext->nPathBundles > 1 && pContext->RSS2QueueMap != nullptr)
auto queueMap = pContext->RSS2QueueMap;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@@ -574,6 +593,13 @@ void CNBL::NBComplete()
m_MappedBuffersDetached.Release();
if (m_BuffersDone.AddRef() == (LONG)m_BuffersNumber)
{
AddHistory(__FUNCTION__, "", "", NULL, "Buffers", m_BuffersNumber);
ParaNdis_DebugHistory(this,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@@ -705,8 +706,12 @@ PNET_BUFFER_LIST CParaNdisTX::ProcessWaitingList(CRawCNBLList &completed)
NBL->Release();
if (CallCompletionForNBL(m_Context, RawNBL))
{
NET_BUFFER_LIST_NEXT_NBL(RawNBL) = CompletedNBLs;
CompletedNBLs = RawNBL;
*tail = RawNBL;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@@ -691,8 +691,7 @@ PNET_BUFFER_LIST CParaNdisTX::ProcessWaitingList(CRawCNBLList &completed)
{
TDPCSpinLocker LockedContext(m_WaitingListLock);

completed.ForEachDetachedIf([](CNBL *NBL) { return !NBL->IsSendDone(); },
[&](CNBL *NBL) { m_WaitingList.PushBack(NBL); });
completed.ForEachDetached([&](CNBL *NBL) { m_WaitingList.PushBack(NBL); });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@YanVugenfirer YanVugenfirer merged commit ab8149c into virtio-win:master Apr 21, 2025
8 of 9 checks passed
@ybendito ybendito deleted the RHEL-22354 branch April 23, 2025 05:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants