On Mon, Jul 27, 2015 at 05:52:48AM +0000, Fu, Siyuan wrote: > Hi, Michael > > Can you help to test whether this patch could solve your problem?
Yes. It works for me. :) Thanks, Michael > > 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 > Date: Mon, 27 Jul 2015 05:51:20 +0000 > From: "Fu, Siyuan" <siyuan...@intel.com> > To: "edk2-de...@lists.01.org" <edk2-de...@lists.01.org> > Subject: [edk2] [Patch] MdeModulePkg: Use monotonic count to initialize the > NetLib random seed. > > 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 ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
