The block returned from Mtftp4RemoveBlockNum is not the total received and saved block number if it works in passive (Slave) mode.
The issue was exposed by the EMS test. Cc: Ye Ting <ting...@intel.com> Cc: Fu Siyuan <siyuan...@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Wu Jiaxin <jiaxin...@intel.com> --- .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.h | 6 +++++- .../Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c | 16 +++++++++++----- .../Universal/Network/Mtftp4Dxe/Mtftp4Support.c | 10 +++++----- .../Universal/Network/Mtftp4Dxe/Mtftp4Support.h | 6 +++--- .../Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c | 6 +++--- 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h index de304f4e70..be2f8af6e4 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.h @@ -124,13 +124,17 @@ struct _MTFTP4_PROTOCOL { LIST_ENTRY Blocks; UINT16 WindowSize; // - // Record the total received block number and the already acked block number. + // Record the total received and saved block number. // UINT64 TotalBlock; + + // + // Record the acked block number. + // UINT64 AckedBlock; // // The server's communication end point: IP and two ports. one for // initial request, one for its selected port. diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c index fedf1cde46..6960e322a5 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Rrq.c @@ -152,10 +152,11 @@ Mtftp4RrqSaveBlock ( EFI_MTFTP4_TOKEN *Token; EFI_STATUS Status; UINT16 Block; UINT64 Start; UINT32 DataLen; + UINT64 BlockCounter; BOOLEAN Completed; Completed = FALSE; Token = Instance->Token; Block = NTOHS (Packet->Data.Block); @@ -172,14 +173,14 @@ Mtftp4RrqSaveBlock ( // // Remove this block number from the file hole. If Mtftp4RemoveBlockNum // returns EFI_NOT_FOUND, the block has been saved, don't save it again. // Note that : For bigger files, allowing the block counter to roll over - // to accept transfers of unlimited size. So TotalBlock is memorised as + // to accept transfers of unlimited size. So BlockCounter is memorised as // continuous block counter. // - Status = Mtftp4RemoveBlockNum (&Instance->Blocks, Block, Completed, &Instance->TotalBlock); + Status = Mtftp4RemoveBlockNum (&Instance->Blocks, Block, Completed, &BlockCounter); if (Status == EFI_NOT_FOUND) { return EFI_SUCCESS; } else if (EFI_ERROR (Status)) { return Status; @@ -198,11 +199,11 @@ Mtftp4RrqSaveBlock ( return EFI_ABORTED; } } if (Token->Buffer != NULL) { - Start = MultU64x32 (Instance->TotalBlock - 1, Instance->BlkSize); + Start = MultU64x32 (BlockCounter - 1, Instance->BlkSize); if (Start + DataLen <= Token->BufferSize) { CopyMem ((UINT8 *) Token->Buffer + Start, Packet->Data.Data, DataLen); // @@ -269,13 +270,13 @@ Mtftp4RrqHandleData ( Expected = Mtftp4GetNextBlockNum (&Instance->Blocks); ASSERT (Expected >= 0); // - // If we are active and received an unexpected packet, transmit + // If we are active (Master) and received an unexpected packet, transmit // the ACK for the block we received, then restart receiving the - // expected one. If we are passive, save the block. + // expected one. If we are passive (Slave), save the block. // if (Instance->Master && (Expected != BlockNum)) { // // If Expected is 0, (UINT16) (Expected - 1) is also the expected Ack number (65535). // @@ -286,10 +287,15 @@ Mtftp4RrqHandleData ( if (EFI_ERROR (Status)) { return Status; } + // + // Record the total received and saved block number. + // + Instance->TotalBlock ++; + // // Reset the passive client's timer whenever it received a // valid data packet. // if (!Instance->Master) { diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c index 71fd979b3a..5e282e9c4b 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.c @@ -156,12 +156,12 @@ Mtftp4SetLastBlockNum ( /** Remove the block number from the block range list. @param Head The block range list to remove from @param Num The block number to remove - @param Completed Whether Num is the last block number - @param TotalBlock The continuous block number in all + @param Completed Whether Num is the last block number. + @param BlockCounter The continuous block counter instead of the value after roll-over. @retval EFI_NOT_FOUND The block number isn't in the block range list @retval EFI_SUCCESS The block number has been removed from the list @retval EFI_OUT_OF_RESOURCES Failed to allocate resource @@ -169,11 +169,11 @@ Mtftp4SetLastBlockNum ( EFI_STATUS Mtftp4RemoveBlockNum ( IN LIST_ENTRY *Head, IN UINT16 Num, IN BOOLEAN Completed, - OUT UINT64 *TotalBlock + OUT UINT64 *BlockCounter ) { MTFTP4_BLOCK_RANGE *Range; MTFTP4_BLOCK_RANGE *NewRange; LIST_ENTRY *Entry; @@ -218,14 +218,14 @@ Mtftp4RemoveBlockNum ( // transfers of unlimited size. There is no consensus, however, whether // the counter should wrap around to zero or to one. Many implementations // wrap to zero, because this is the simplest to implement. Here we choose // this solution. // - *TotalBlock = Num; + *BlockCounter = Num; if (Range->Round > 0) { - *TotalBlock += Range->Bound + MultU64x32 ((UINTN) (Range->Round -1), (UINT32) (Range->Bound + 1)) + 1; + *BlockCounter += Range->Bound + MultU64x32 ((UINTN) (Range->Round -1), (UINT32) (Range->Bound + 1)) + 1; } if (Range->Start > Range->Bound) { Range->Start = 0; Range->Round ++; diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h index 6cc2756bc8..f7a6755fe8 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Support.h @@ -90,12 +90,12 @@ Mtftp4SetLastBlockNum ( /** Remove the block number from the block range list. @param Head The block range list to remove from @param Num The block number to remove - @param Completed Wether Num is the last block number - @param TotalBlock The continuous block number in all + @param Completed Whether Num is the last block number. + @param BlockCounter The continuous block counter instead of the value after roll-over. @retval EFI_NOT_FOUND The block number isn't in the block range list @retval EFI_SUCCESS The block number has been removed from the list @retval EFI_OUT_OF_RESOURCES Failed to allocate resource @@ -103,11 +103,11 @@ Mtftp4SetLastBlockNum ( EFI_STATUS Mtftp4RemoveBlockNum ( IN LIST_ENTRY *Head, IN UINT16 Num, IN BOOLEAN Completed, - OUT UINT64 *TotalBlock + OUT UINT64 *BlockCounter ); /** Set the timeout for the instance. User a longer time for passive instances. diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c index ea309e2d6b..ee70accbcd 100644 --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c @@ -147,11 +147,11 @@ Mtftp4WrqHandleAck ( OUT BOOLEAN *Completed ) { UINT16 AckNum; INTN Expected; - UINT64 TotalBlock; + UINT64 BlockCounter; *Completed = FALSE; AckNum = NTOHS (Packet->Ack.Block[0]); Expected = Mtftp4GetNextBlockNum (&Instance->Blocks); @@ -166,13 +166,13 @@ Mtftp4WrqHandleAck ( } // // Remove the acked block number, if this is the last block number, // tell the Mtftp4WrqInput to finish the transfer. This is the last - // block number if the block range are empty.. + // block number if the block range are empty. // - Mtftp4RemoveBlockNum (&Instance->Blocks, AckNum, *Completed,&TotalBlock); + Mtftp4RemoveBlockNum (&Instance->Blocks, AckNum, *Completed, &BlockCounter); Expected = Mtftp4GetNextBlockNum (&Instance->Blocks); if (Expected < 0) { -- 2.17.1.windows.2 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel