Message ID | 20170724223138.17164-1-rams@vmware.com |
---|---|
State | Accepted |
Headers | show |
Thanks for refactoring it. Acked-by: Anand Kumar <kumaranand@vmware.com> Thanks, Anand Kumar On 7/24/17, 3:31 PM, "ovs-dev-bounces@openvswitch.org on behalf of Shashank Ram" <ovs-dev-bounces@openvswitch.org on behalf of rams@vmware.com> wrote: Previously, the function would take the curNbl and nextNbl as inputs, and modify the linked list, and merge the input linked list with the newly generated newNbl list. This is confusing for the caller, and the function has unnecessary logic for merging linked lists that instead the caller should take care of. This is because the OvsCreateNewNBLsFromMultipleNBs() is a generic API that can be used by other functions as well, and its natural for different callers to have different needs. This patch refactors the behavior of OvsCreateNewNBLsFromMultipleNBs to take in the curNbl and lastNbl, and it returns a linked list of NBLs and sets the HEAD and TAIL of the new list obtained from the curNbl. If the caller wants to chain a new linked list at the HEAD or TAIL, it can make use of the curNbl and lastNbl to do so. Signed-off-by: Shashank Ram <rams@vmware.com> --- datapath-windows/ovsext/PacketIO.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/datapath-windows/ovsext/PacketIO.c b/datapath-windows/ovsext/PacketIO.c index a90b556..81c574e 100644 --- a/datapath-windows/ovsext/PacketIO.c +++ b/datapath-windows/ovsext/PacketIO.c @@ -49,7 +49,7 @@ static VOID OvsCompleteNBLIngress(POVS_SWITCH_CONTEXT switchContext, static NTSTATUS OvsCreateNewNBLsFromMultipleNBs( POVS_SWITCH_CONTEXT switchContext, PNET_BUFFER_LIST *curNbl, - PNET_BUFFER_LIST *nextNbl); + PNET_BUFFER_LIST *lastNbl); VOID OvsInitCompletionList(OvsCompletionList *completionList, @@ -212,7 +212,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, NDIS_SWITCH_PORT_ID sourcePort = 0; NDIS_SWITCH_NIC_INDEX sourceIndex = 0; PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail; - PNET_BUFFER_LIST curNbl = NULL, nextNbl = NULL; + PNET_BUFFER_LIST curNbl = NULL, nextNbl = NULL, lastNbl = NULL; ULONG sendCompleteFlags; UCHAR dispatch; LOCK_STATE_EX lockState, dpLockState; @@ -282,7 +282,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, /* Create a NET_BUFFER_LIST for each NET_BUFFER. */ status = OvsCreateNewNBLsFromMultipleNBs(switchContext, &curNbl, - &nextNbl); + &lastNbl); if (!NT_SUCCESS(status)) { RtlInitUnicodeString(&filterReason, L"Cannot allocate NBLs with single NB."); @@ -292,6 +292,10 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, NDIS_STATUS_RESOURCES); continue; } + + lastNbl->Next = nextNbl; + nextNbl = curNbl->Next; + curNbl->Next = NULL; } { OvsFlow *flow; @@ -500,11 +504,10 @@ OvsExtCancelSendNBL(NDIS_HANDLE filterModuleContext, static NTSTATUS OvsCreateNewNBLsFromMultipleNBs(POVS_SWITCH_CONTEXT switchContext, PNET_BUFFER_LIST *curNbl, - PNET_BUFFER_LIST *nextNbl) + PNET_BUFFER_LIST *lastNbl) { NTSTATUS status = STATUS_SUCCESS; PNET_BUFFER_LIST newNbls = NULL; - PNET_BUFFER_LIST lastNbl = NULL; PNET_BUFFER_LIST nbl = NULL; BOOLEAN error = TRUE; @@ -520,16 +523,15 @@ OvsCreateNewNBLsFromMultipleNBs(POVS_SWITCH_CONTEXT switchContext, nbl = newNbls; while (nbl) { - lastNbl = nbl; + *lastNbl = nbl; nbl = NET_BUFFER_LIST_NEXT_NBL(nbl); } - lastNbl->Next = *nextNbl; - *nextNbl = newNbls->Next; + + (*curNbl)->Next = NULL; OvsCompleteNBL(switchContext, *curNbl, TRUE); *curNbl = newNbls; - (*curNbl)->Next = NULL; error = FALSE; } while (error); -- 2.9.3.windows.2 _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=Q5z9tBe-nAOpE7LIHSPV8uy5-437agMXvkeHHMkR8Us&m=Ha-_2gKEsqU5lJ7UF1ghlNSvRsFiPZ1mg9r07TefECE&s=-362eyYjcktBRRr9mkK07U1qeOyl5qTUc35wyOi6HBU&e=
Guru, could you please apply this patch. Thanks, Shashank
On Mon, Jul 24, 2017 at 03:31:38PM -0700, Shashank Ram wrote: > Previously, the function would take the curNbl and nextNbl > as inputs, and modify the linked list, and merge the input > linked list with the newly generated newNbl list. > > This is confusing for the caller, and the function has > unnecessary logic for merging linked lists that instead > the caller should take care of. This is because the > OvsCreateNewNBLsFromMultipleNBs() is a generic API > that can be used by other functions as well, and its > natural for different callers to have different needs. > > This patch refactors the behavior of OvsCreateNewNBLsFromMultipleNBs > to take in the curNbl and lastNbl, and it returns > a linked list of NBLs and sets the HEAD and TAIL of the > new list obtained from the curNbl. If the caller wants > to chain a new linked list at the HEAD or TAIL, it > can make use of the curNbl and lastNbl to do so. I applied this to master. Thank you!
diff --git a/datapath-windows/ovsext/PacketIO.c b/datapath-windows/ovsext/PacketIO.c index a90b556..81c574e 100644 --- a/datapath-windows/ovsext/PacketIO.c +++ b/datapath-windows/ovsext/PacketIO.c @@ -49,7 +49,7 @@ static VOID OvsCompleteNBLIngress(POVS_SWITCH_CONTEXT switchContext, static NTSTATUS OvsCreateNewNBLsFromMultipleNBs( POVS_SWITCH_CONTEXT switchContext, PNET_BUFFER_LIST *curNbl, - PNET_BUFFER_LIST *nextNbl); + PNET_BUFFER_LIST *lastNbl); VOID OvsInitCompletionList(OvsCompletionList *completionList, @@ -212,7 +212,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, NDIS_SWITCH_PORT_ID sourcePort = 0; NDIS_SWITCH_NIC_INDEX sourceIndex = 0; PNDIS_SWITCH_FORWARDING_DETAIL_NET_BUFFER_LIST_INFO fwdDetail; - PNET_BUFFER_LIST curNbl = NULL, nextNbl = NULL; + PNET_BUFFER_LIST curNbl = NULL, nextNbl = NULL, lastNbl = NULL; ULONG sendCompleteFlags; UCHAR dispatch; LOCK_STATE_EX lockState, dpLockState; @@ -282,7 +282,7 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, /* Create a NET_BUFFER_LIST for each NET_BUFFER. */ status = OvsCreateNewNBLsFromMultipleNBs(switchContext, &curNbl, - &nextNbl); + &lastNbl); if (!NT_SUCCESS(status)) { RtlInitUnicodeString(&filterReason, L"Cannot allocate NBLs with single NB."); @@ -292,6 +292,10 @@ OvsStartNBLIngress(POVS_SWITCH_CONTEXT switchContext, NDIS_STATUS_RESOURCES); continue; } + + lastNbl->Next = nextNbl; + nextNbl = curNbl->Next; + curNbl->Next = NULL; } { OvsFlow *flow; @@ -500,11 +504,10 @@ OvsExtCancelSendNBL(NDIS_HANDLE filterModuleContext, static NTSTATUS OvsCreateNewNBLsFromMultipleNBs(POVS_SWITCH_CONTEXT switchContext, PNET_BUFFER_LIST *curNbl, - PNET_BUFFER_LIST *nextNbl) + PNET_BUFFER_LIST *lastNbl) { NTSTATUS status = STATUS_SUCCESS; PNET_BUFFER_LIST newNbls = NULL; - PNET_BUFFER_LIST lastNbl = NULL; PNET_BUFFER_LIST nbl = NULL; BOOLEAN error = TRUE; @@ -520,16 +523,15 @@ OvsCreateNewNBLsFromMultipleNBs(POVS_SWITCH_CONTEXT switchContext, nbl = newNbls; while (nbl) { - lastNbl = nbl; + *lastNbl = nbl; nbl = NET_BUFFER_LIST_NEXT_NBL(nbl); } - lastNbl->Next = *nextNbl; - *nextNbl = newNbls->Next; + + (*curNbl)->Next = NULL; OvsCompleteNBL(switchContext, *curNbl, TRUE); *curNbl = newNbls; - (*curNbl)->Next = NULL; error = FALSE; } while (error);
Previously, the function would take the curNbl and nextNbl as inputs, and modify the linked list, and merge the input linked list with the newly generated newNbl list. This is confusing for the caller, and the function has unnecessary logic for merging linked lists that instead the caller should take care of. This is because the OvsCreateNewNBLsFromMultipleNBs() is a generic API that can be used by other functions as well, and its natural for different callers to have different needs. This patch refactors the behavior of OvsCreateNewNBLsFromMultipleNBs to take in the curNbl and lastNbl, and it returns a linked list of NBLs and sets the HEAD and TAIL of the new list obtained from the curNbl. If the caller wants to chain a new linked list at the HEAD or TAIL, it can make use of the curNbl and lastNbl to do so. Signed-off-by: Shashank Ram <rams@vmware.com> --- datapath-windows/ovsext/PacketIO.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)