Hi, Michael Can you help to test whether this patch could solve your problem?
Thanks, Siyuan -----Original Message----- From: Michael Chang [mailto:mch...@suse.com] Sent: Friday, July 24, 2015 10:34 AM To: Fu, Siyuan Cc: edk2-devel@lists.sourceforge.net; Ye, Ting Subject: Re: [edk2] DHCPv6 IAID and NetRandomInitSeed problem in OVMF On Thu, Jul 23, 2015 at 10:13:20AM +0000, Fu, Siyuan wrote: > Michael, > > It's not because any benefit, it's because there is actually no place to put > the static local variable. The IaDescriptor.IaId is defined in UEFI spec, so > Dhcp6->Configure() function could only use caller provided IaId number and > can't modify it. And we couldn't use static local variable in NetLib, because > the library is statically linked to IP6 and PXE driver, so there are actually > 2 static variable in these 2 driver. > Thank you for the explaination, now I understood why. If you have any patch for this and need testing, please let me know and I'm glad to help. Thanks, Michael > Best Regards > Siyuan > > -----Original Message----- > From: Michael Chang [mailto:mch...@suse.com] > Sent: Thursday, July 23, 2015 5:43 PM > To: Fu, Siyuan > Cc: edk2-devel@lists.sourceforge.net; Ye, Ting > Subject: Re: [edk2] DHCPv6 IAID and NetRandomInitSeed problem in OVMF > > On Thu, Jul 23, 2015 at 09:01:38AM +0000, Fu, Siyuan wrote: > > Hi, Michael > > > > It's indeed a problem that NET_RANDOM (NetRandomInitSeed ()) may return > > same value if the time service is not accuracy enough on some platform. And > > as you said it's used to give unique but not necessarily to be real random > > number, so I think we could just add some salt into the seed. I think the > > platform monotonic counter may be a good choose, we can call > > gBS->GetNextMonotonicCount() and add the low 32 bits of MC to the seed. > > This could make the returned seed unique even GetTime() return all zero in > > the worst case. What do you think about it? > > It sounds good for me. But I'd like to know what is the benefit compared to > increment by static local variable? > > > > > For your recommendation #3, the IaId is configured by Ip6Dxe and PXE > > driver, and there are no interface to exchange the index between them. > > DHCP6 driver just use the IaId configured by caller and it couldn't modify > > it. And for 4, one of the design intention of PXE boot is "platform doesn't > > need any pre-configuration", so it's also not acceptable. > > Actually I'm not sure about the need to create automatic policy. Is there any > function or feature in UEFI preboot environment depend on it to work > correctly (likely it is for booting iSCSI disks, but that can be done in > other settings) ? > > Thanks, > Michael > > > > > Best Regards, > > Siyuan > > > > -----Original Message----- > > From: Michael Chang [mailto:mch...@suse.com] > > Sent: Thursday, July 23, 2015 3:33 PM > > To: edk2-devel@lists.sourceforge.net > > Subject: [edk2] DHCPv6 IAID and NetRandomInitSeed problem in OVMF > > > > Hi, > > > > I have problem in IPv6 PXE booting under OVMF that the process terminated > > immediately right after selecting the UEFI IPv6 boot entry. I have to > > change the IPv6 config policy from automatic to manual to get things > > working again for me. > > > > I looked a bit into this problem, it was terminated by the error in > > IaId check > > > > in NetworkPkg/Dhcp6Dxe/Dhcp6Impl.c::EfiDhcp6Configure function > > > > // > > // Make sure the (IaId, IaType) is unique over all the instances. > > // > > NET_LIST_FOR_EACH (Entry, &Service->Child) { > > Other = NET_LIST_USER_STRUCT (Entry, DHCP6_INSTANCE, Link); > > if (Other->IaCb.Ia != NULL && > > Other->IaCb.Ia->Descriptor.Type == > > Dhcp6CfgData->IaDescriptor.Type && > > Other->IaCb.Ia->Descriptor.IaId == Dhcp6CfgData->IaDescriptor.IaId > > ) { > > return EFI_INVALID_PARAMETER; > > } > > } > > > > Which would be saying clearly it was not "unique" among all the > > running > > DHCPv6 instances. At that time there should be only two: > > > > 1. The IPv6 config instance, while the policy is set to automatic > > would exchange information with server with the IaId initialized at > > > > NetworkPkg/Ip6Dxe/Ip6ConfigImpl.c::Ip6ConfigInitInstance > > > > Instance->IaId = NET_RANDOM (NetRandomInitSeed ()); > > > > for (Index = 0; Index < IpSb->SnpMode.HwAddressSize; Index++) { > > Instance->IaId |= (IpSb->SnpMode.CurrentAddress.Addr[Index] << > > ((Index << 3) & 31)); > > } > > > > 2. The IPv6 PXE Base code driver would start another DHCPv6 session > > with the IaId initialized at > > > > NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c::PxeBcCreateIp6Children > > > > // > > // Generate a random IAID for the Dhcp6 assigned address. > > // > > Private->IaId = NET_RANDOM (NetRandomInitSeed ()); > > if (Private->Snp != NULL) { > > for (Index = 0; Index < Private->Snp->Mode->HwAddressSize; Index++) { > > Private->IaId |= (Private->Snp->Mode->CurrentAddress.Addr[Index] << > > ((Index << 3) & 31)); > > } > > } > > > > Unfortunately both IaId collides, and quite surprisingly it's NET_RANDOM > > (NetRandomInitSeed ()), or NetRandomInitSeed () generates the same number > > for them. > > > > UINT32 > > EFIAPI > > NetRandomInitSeed ( > > VOID > > ) > > { > > EFI_TIME Time; > > UINT32 Seed; > > > > gRT->GetTime (&Time, NULL); > > Seed = (~Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | > > Time.Second); > > Seed ^= Time.Nanosecond; > > Seed ^= Time.Year << 7; > > > > return Seed; > > } > > > > The returned Time.Nanosecond is always zero as the timer resolution is l Hz > > for PC-AT CMOS RTC, that seems to be default real time clock driver used by > > OVMF. At least from .dsc file it says so .. > > > > > > PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeD > > xe > > .inf > > > > The NetRandomInitSeed is not random enough in this case, and the > > NET_RANDOM marco is even more confusing as it's not a random number > > generator either (so what is good for always feeding random "seed" > > to a deterministic mathematic fucntion ??) > > > > I don't have good idea how to fix the problem, but here are of my thoughs. > > > > 1. Use real timer clock driver which could provide higher resolution for > > time service. Qemu is able to provide HPET timer but I don't know how to > > configure OVMF to use it. > > > > 2. Use GetPerformanceCounter(), but that would increase image size by > > linking TimerLib. > > > > 3. The IaId has to be "unique" thus not necessarily to be random, probably > > it should be derived by some properties like MAC address and an assgined > > index (staring from zero) for the interface to be confiured. > > > > 4. Use manual as default IPv6 policy, but that just hide the problem. > > > > Hope that my information and findings helps. :) > > > > Thanks, > > Michael > > > > -------------------------------------------------------------------- > > -- > > -------- _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/edk2-devel
--- Begin Message ---NetRandomInitSeed() function use current time to initialize the random seed, while in some platform the time service is not accuracy that make the random seed collision. This patch add the monotonic count to the seed to avoid this. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Fu Siyuan <siyuan...@intel.com> --- MdeModulePkg/Include/Library/NetLib.h | 10 +++++----- MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 12 ++++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/MdeModulePkg/Include/Library/NetLib.h b/MdeModulePkg/Include/Library/NetLib.h index 7ad8dac..280c51a 100644 --- a/MdeModulePkg/Include/Library/NetLib.h +++ b/MdeModulePkg/Include/Library/NetLib.h @@ -528,17 +528,17 @@ NetPutUint32 ( IN OUT UINT8 *Buf, IN UINT32 Data ); /** - Initialize a random seed using current time. + Initialize a random seed using current time and monotonic count. - Get current time first. Then initialize a random seed based on some basic - mathematical operations on the hour, day, minute, second, nanosecond and year - of the current time. + Get current time and monotonic count first. Then initialize a random seed + based on some basic mathematics operation on the hour, day, minute, second, + nanosecond and year of the current time and the monotonic count value. - @return The random seed, initialized with current time. + @return The random seed initialized with current time. **/ UINT32 EFIAPI NetRandomInitSeed ( diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c index ce26b32..57e8f9f 100644 --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c @@ -851,15 +851,15 @@ Ip6Swap128 ( return Ip6; } /** - Initialize a random seed using current time. + Initialize a random seed using current time and monotonic count. - Get current time first. Then initialize a random seed based on some basic - mathematics operation on the hour, day, minute, second, nanosecond and year - of the current time. + Get current time and monotonic count first. Then initialize a random seed + based on some basic mathematics operation on the hour, day, minute, second, + nanosecond and year of the current time and the monotonic count value. @return The random seed initialized with current time. **/ UINT32 @@ -868,16 +868,20 @@ NetRandomInitSeed ( VOID ) { EFI_TIME Time; UINT32 Seed; + UINT64 MonotonicCount; gRT->GetTime (&Time, NULL); Seed = (~Time.Hour << 24 | Time.Day << 16 | Time.Minute << 8 | Time.Second); Seed ^= Time.Nanosecond; Seed ^= Time.Year << 7; + gBS->GetNextMonotonicCount (&MonotonicCount); + Seed += (UINT32) MonotonicCount; + return Seed; } /** -- 1.9.5.msysgit.1 _______________________________________________ edk2-devel mailing list edk2-de...@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
--- End Message ---
------------------------------------------------------------------------------
_______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
