Discussion:
[edk2] [patch] NetworkPkg: Fix an error that the call function declared implicitly.
Zhang Lubo
2015-07-14 01:49:32 UTC
Permalink
AsmReadTsc() implementation which is decelerated in BaseLib.h covers both IA32, X64 and
IPF architecture, so it should specify which implementation to use when invoked.
1. Different Macro definition in DnsImpl.c to specify the implementation.
2. Modefy the NetworkPkg.dsc file to let the DnsDxe.inf is effective to different Arch.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
NetworkPkg/DnsDxe/DnsImpl.c | 23 ++++++++++++++++++++++-
NetworkPkg/NetworkPkg.dsc | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c
index b196d18..93b1835 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -13,10 +13,31 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/

#include "DnsImpl.h"

/**
+ Reads the current value of Time Stamp Counter (TSC) or the Interval Timer Counter Register (ITC)
+
+ @return The current value of TSC or ITC
+
+**/
+
+UINT64
+ReadTime (
+ VOID
+ )
+{
+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
+ return AsmReadTsc ();
+#elif defined (MDE_CPU_IPF)
+ return AsmReadItc ();
+#else
+#error ReadTime not supported for this architecture!
+#endif
+}
+
+/**
Remove TokenEntry from TokenMap.

@param[in] TokenMap All DNSv4 Token entrys.
@param[in] TokenEntry TokenEntry need to be removed.

@@ -1573,11 +1594,11 @@ ConstructDNSQueryIp (

//
// Fill header
//
DnsHeader = (DNS_HEADER *)Frag.Bulk;
- DnsHeader->Identification = (UINT16)AsmReadTsc ();
+ DnsHeader->Identification = (UINT16)ReadTime();
DnsHeader->Flags.Uint16 = 0x0000;
DnsHeader->Flags.Bits.RD = 1;
DnsHeader->Flags.Bits.OpCode = DNS_FLAGS_OPCODE_STANDARD;
DnsHeader->Flags.Bits.QR = DNS_FLAGS_QR_QUERY;
DnsHeader->QuestionsNum = 1;
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc
index 1d7fd85..11e0ce0 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -98,11 +98,10 @@
NetworkPkg/Ip6Dxe/Ip6Dxe.inf
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
- NetworkPkg/DnsDxe/DnsDxe.inf
NetworkPkg/HttpDxe/HttpDxe.inf
NetworkPkg/HttpBootDxe/HttpBootDxe.inf

NetworkPkg/Application/IfConfig6/IfConfig6.inf
NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
@@ -111,5 +110,6 @@
[Components.IA32, Components.X64, Components.IPF]
NetworkPkg/IpSecDxe/IpSecDxe.inf
NetworkPkg/IScsiDxe/IScsiDxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
NetworkPkg/Application/Ping6/Ping6.inf
+ NetworkPkg/DnsDxe/DnsDxe.inf
--
1.9.5.msysgit.1
Fu, Siyuan
2015-07-14 01:55:29 UTC
Permalink
Lubo,

What's the DnsHeader->Identification means? Why it use the value of TSC register?

Siyuan

-----Original Message-----
From: Zhang, Lubo
Sent: Tuesday, July 14, 2015 9:50 AM
To: Fu, Siyuan; Ye, Ting; Wu, Jiaxin; edk2-***@lists.sourceforge.net
Subject: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

AsmReadTsc() implementation which is decelerated in BaseLib.h covers both IA32, X64 and IPF architecture, so it should specify which implementation to use when invoked.
1. Different Macro definition in DnsImpl.c to specify the implementation.
2. Modefy the NetworkPkg.dsc file to let the DnsDxe.inf is effective to different Arch.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
NetworkPkg/DnsDxe/DnsImpl.c | 23 ++++++++++++++++++++++-
NetworkPkg/NetworkPkg.dsc | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index b196d18..93b1835 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -13,10 +13,31 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/

#include "DnsImpl.h"

/**
+ Reads the current value of Time Stamp Counter (TSC) or the Interval
+ Timer Counter Register (ITC)
+
+ @return The current value of TSC or ITC
+
+**/
+
+UINT64
+ReadTime (
+ VOID
+ )
+{
+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
+ return AsmReadTsc ();
+#elif defined (MDE_CPU_IPF)
+ return AsmReadItc ();
+#else
+#error ReadTime not supported for this architecture!
+#endif
+}
+
+/**
Remove TokenEntry from TokenMap.

@param[in] TokenMap All DNSv4 Token entrys.
@param[in] TokenEntry TokenEntry need to be removed.

@@ -1573,11 +1594,11 @@ ConstructDNSQueryIp (

//
// Fill header
//
DnsHeader = (DNS_HEADER *)Frag.Bulk;
- DnsHeader->Identification = (UINT16)AsmReadTsc ();
+ DnsHeader->Identification = (UINT16)ReadTime();
DnsHeader->Flags.Uint16 = 0x0000;
DnsHeader->Flags.Bits.RD = 1;
DnsHeader->Flags.Bits.OpCode = DNS_FLAGS_OPCODE_STANDARD;
DnsHeader->Flags.Bits.QR = DNS_FLAGS_QR_QUERY;
DnsHeader->QuestionsNum = 1;
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc index 1d7fd85..11e0ce0 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -98,11 +98,10 @@
NetworkPkg/Ip6Dxe/Ip6Dxe.inf
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
- NetworkPkg/DnsDxe/DnsDxe.inf
NetworkPkg/HttpDxe/HttpDxe.inf
NetworkPkg/HttpBootDxe/HttpBootDxe.inf

NetworkPkg/Application/IfConfig6/IfConfig6.inf
NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
@@ -111,5 +110,6 @@
[Components.IA32, Components.X64, Components.IPF]
NetworkPkg/IpSecDxe/IpSecDxe.inf
NetworkPkg/IScsiDxe/IScsiDxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
NetworkPkg/Application/Ping6/Ping6.inf
+ NetworkPkg/DnsDxe/DnsDxe.inf
--
1.9.5.msysgit.1
Andrew Fish
2015-07-14 02:04:33 UTC
Permalink
Why not use some abstraction like the TimerLib?

Sent from my iPhone
Post by Fu, Siyuan
Lubo,
What's the DnsHeader->Identification means? Why it use the value of TSC register?
Siyuan
-----Original Message-----
From: Zhang, Lubo
Sent: Tuesday, July 14, 2015 9:50 AM
Subject: [patch] NetworkPkg: Fix an error that the call function declared implicitly.
AsmReadTsc() implementation which is decelerated in BaseLib.h covers both IA32, X64 and IPF architecture, so it should specify which implementation to use when invoked.
1. Different Macro definition in DnsImpl.c to specify the implementation.
2. Modefy the NetworkPkg.dsc file to let the DnsDxe.inf is effective to different Arch.
Contributed-under: TianoCore Contribution Agreement 1.0
---
NetworkPkg/DnsDxe/DnsImpl.c | 23 ++++++++++++++++++++++-
NetworkPkg/NetworkPkg.dsc | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index b196d18..93b1835 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -13,10 +13,31 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
#include "DnsImpl.h"
/**
+ Reads the current value of Time Stamp Counter (TSC) or the Interval
+ Timer Counter Register (ITC)
+
+
+**/
+
+UINT64
+ReadTime (
+ VOID
+ )
+{
+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
+ return AsmReadTsc ();
+#elif defined (MDE_CPU_IPF)
+ return AsmReadItc ();
+#else
+#error ReadTime not supported for this architecture!
+#endif
+}
+
+/**
Remove TokenEntry from TokenMap.
@param[in] TokenMap All DNSv4 Token entrys.
@param[in] TokenEntry TokenEntry need to be removed.
@@ -1573,11 +1594,11 @@ ConstructDNSQueryIp (
//
// Fill header
//
DnsHeader = (DNS_HEADER *)Frag.Bulk;
- DnsHeader->Identification = (UINT16)AsmReadTsc ();
+ DnsHeader->Identification = (UINT16)ReadTime();
DnsHeader->Flags.Uint16 = 0x0000;
DnsHeader->Flags.Bits.RD = 1;
DnsHeader->Flags.Bits.OpCode = DNS_FLAGS_OPCODE_STANDARD;
DnsHeader->Flags.Bits.QR = DNS_FLAGS_QR_QUERY;
DnsHeader->QuestionsNum = 1;
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc index 1d7fd85..11e0ce0 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -98,11 +98,10 @@
NetworkPkg/Ip6Dxe/Ip6Dxe.inf
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
- NetworkPkg/DnsDxe/DnsDxe.inf
NetworkPkg/HttpDxe/HttpDxe.inf
NetworkPkg/HttpBootDxe/HttpBootDxe.inf
NetworkPkg/Application/IfConfig6/IfConfig6.inf
NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
@@ -111,5 +110,6 @@
[Components.IA32, Components.X64, Components.IPF]
NetworkPkg/IpSecDxe/IpSecDxe.inf
NetworkPkg/IScsiDxe/IScsiDxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
NetworkPkg/Application/Ping6/Ping6.inf
+ NetworkPkg/DnsDxe/DnsDxe.inf
--
1.9.5.msysgit.1
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Wu, Jiaxin
2015-07-14 02:04:12 UTC
Permalink
It's used to generate any kind of query. This value should be a random value, so TSC register maybe called.

-----Original Message-----
From: Fu, Siyuan
Sent: Tuesday, July 14, 2015 9:55 AM
To: Zhang, Lubo; Ye, Ting; Wu, Jiaxin; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

Lubo,

What's the DnsHeader->Identification means? Why it use the value of TSC register?

Siyuan

-----Original Message-----
From: Zhang, Lubo
Sent: Tuesday, July 14, 2015 9:50 AM
To: Fu, Siyuan; Ye, Ting; Wu, Jiaxin; edk2-***@lists.sourceforge.net
Subject: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

AsmReadTsc() implementation which is decelerated in BaseLib.h covers both IA32, X64 and IPF architecture, so it should specify which implementation to use when invoked.
1. Different Macro definition in DnsImpl.c to specify the implementation.
2. Modefy the NetworkPkg.dsc file to let the DnsDxe.inf is effective to different Arch.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
NetworkPkg/DnsDxe/DnsImpl.c | 23 ++++++++++++++++++++++-
NetworkPkg/NetworkPkg.dsc | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index b196d18..93b1835 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -13,10 +13,31 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/

#include "DnsImpl.h"

/**
+ Reads the current value of Time Stamp Counter (TSC) or the Interval
+ Timer Counter Register (ITC)
+
+ @return The current value of TSC or ITC
+
+**/
+
+UINT64
+ReadTime (
+ VOID
+ )
+{
+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
+ return AsmReadTsc ();
+#elif defined (MDE_CPU_IPF)
+ return AsmReadItc ();
+#else
+#error ReadTime not supported for this architecture!
+#endif
+}
+
+/**
Remove TokenEntry from TokenMap.

@param[in] TokenMap All DNSv4 Token entrys.
@param[in] TokenEntry TokenEntry need to be removed.

@@ -1573,11 +1594,11 @@ ConstructDNSQueryIp (

//
// Fill header
//
DnsHeader = (DNS_HEADER *)Frag.Bulk;
- DnsHeader->Identification = (UINT16)AsmReadTsc ();
+ DnsHeader->Identification = (UINT16)ReadTime();
DnsHeader->Flags.Uint16 = 0x0000;
DnsHeader->Flags.Bits.RD = 1;
DnsHeader->Flags.Bits.OpCode = DNS_FLAGS_OPCODE_STANDARD;
DnsHeader->Flags.Bits.QR = DNS_FLAGS_QR_QUERY;
DnsHeader->QuestionsNum = 1;
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc index 1d7fd85..11e0ce0 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -98,11 +98,10 @@
NetworkPkg/Ip6Dxe/Ip6Dxe.inf
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
- NetworkPkg/DnsDxe/DnsDxe.inf
NetworkPkg/HttpDxe/HttpDxe.inf
NetworkPkg/HttpBootDxe/HttpBootDxe.inf

NetworkPkg/Application/IfConfig6/IfConfig6.inf
NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
@@ -111,5 +110,6 @@
[Components.IA32, Components.X64, Components.IPF]
NetworkPkg/IpSecDxe/IpSecDxe.inf
NetworkPkg/IScsiDxe/IScsiDxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
NetworkPkg/Application/Ping6/Ping6.inf
+ NetworkPkg/DnsDxe/DnsDxe.inf
--
1.9.5.msysgit.1
Fu, Siyuan
2015-07-14 02:09:09 UTC
Permalink
Jiaxin

A soft network stack driver should not depend on a platform specific API. If it's just a random number to identify the message, you could use the NetLib API NetRandomInitSeed().

-----Original Message-----
From: Wu, Jiaxin
Sent: Tuesday, July 14, 2015 10:04 AM
To: Fu, Siyuan; Zhang, Lubo; Ye, Ting; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

It's used to generate any kind of query. This value should be a random value, so TSC register maybe called.

-----Original Message-----
From: Fu, Siyuan
Sent: Tuesday, July 14, 2015 9:55 AM
To: Zhang, Lubo; Ye, Ting; Wu, Jiaxin; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

Lubo,

What's the DnsHeader->Identification means? Why it use the value of TSC register?

Siyuan

-----Original Message-----
From: Zhang, Lubo
Sent: Tuesday, July 14, 2015 9:50 AM
To: Fu, Siyuan; Ye, Ting; Wu, Jiaxin; edk2-***@lists.sourceforge.net
Subject: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

AsmReadTsc() implementation which is decelerated in BaseLib.h covers both IA32, X64 and IPF architecture, so it should specify which implementation to use when invoked.
1. Different Macro definition in DnsImpl.c to specify the implementation.
2. Modefy the NetworkPkg.dsc file to let the DnsDxe.inf is effective to different Arch.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
NetworkPkg/DnsDxe/DnsImpl.c | 23 ++++++++++++++++++++++-
NetworkPkg/NetworkPkg.dsc | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index b196d18..93b1835 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -13,10 +13,31 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/

#include "DnsImpl.h"

/**
+ Reads the current value of Time Stamp Counter (TSC) or the Interval
+ Timer Counter Register (ITC)
+
+ @return The current value of TSC or ITC
+
+**/
+
+UINT64
+ReadTime (
+ VOID
+ )
+{
+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
+ return AsmReadTsc ();
+#elif defined (MDE_CPU_IPF)
+ return AsmReadItc ();
+#else
+#error ReadTime not supported for this architecture!
+#endif
+}
+
+/**
Remove TokenEntry from TokenMap.

@param[in] TokenMap All DNSv4 Token entrys.
@param[in] TokenEntry TokenEntry need to be removed.

@@ -1573,11 +1594,11 @@ ConstructDNSQueryIp (

//
// Fill header
//
DnsHeader = (DNS_HEADER *)Frag.Bulk;
- DnsHeader->Identification = (UINT16)AsmReadTsc ();
+ DnsHeader->Identification = (UINT16)ReadTime();
DnsHeader->Flags.Uint16 = 0x0000;
DnsHeader->Flags.Bits.RD = 1;
DnsHeader->Flags.Bits.OpCode = DNS_FLAGS_OPCODE_STANDARD;
DnsHeader->Flags.Bits.QR = DNS_FLAGS_QR_QUERY;
DnsHeader->QuestionsNum = 1;
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc index 1d7fd85..11e0ce0 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -98,11 +98,10 @@
NetworkPkg/Ip6Dxe/Ip6Dxe.inf
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
- NetworkPkg/DnsDxe/DnsDxe.inf
NetworkPkg/HttpDxe/HttpDxe.inf
NetworkPkg/HttpBootDxe/HttpBootDxe.inf

NetworkPkg/Application/IfConfig6/IfConfig6.inf
NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
@@ -111,5 +110,6 @@
[Components.IA32, Components.X64, Components.IPF]
NetworkPkg/IpSecDxe/IpSecDxe.inf
NetworkPkg/IScsiDxe/IScsiDxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
NetworkPkg/Application/Ping6/Ping6.inf
+ NetworkPkg/DnsDxe/DnsDxe.inf
--
1.9.5.msysgit.1
Wu, Jiaxin
2015-07-14 02:13:48 UTC
Permalink
Siyuan, you are right, That is a good suggestion.

Lubo, you can revise the patch to v2 according to Siyuan's opinion.

-----Original Message-----
From: Fu, Siyuan
Sent: Tuesday, July 14, 2015 10:09 AM
To: Wu, Jiaxin; Zhang, Lubo; Ye, Ting; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

Jiaxin

A soft network stack driver should not depend on a platform specific API. If it's just a random number to identify the message, you could use the NetLib API NetRandomInitSeed().

-----Original Message-----
From: Wu, Jiaxin
Sent: Tuesday, July 14, 2015 10:04 AM
To: Fu, Siyuan; Zhang, Lubo; Ye, Ting; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

It's used to generate any kind of query. This value should be a random value, so TSC register maybe called.

-----Original Message-----
From: Fu, Siyuan
Sent: Tuesday, July 14, 2015 9:55 AM
To: Zhang, Lubo; Ye, Ting; Wu, Jiaxin; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

Lubo,

What's the DnsHeader->Identification means? Why it use the value of TSC register?

Siyuan

-----Original Message-----
From: Zhang, Lubo
Sent: Tuesday, July 14, 2015 9:50 AM
To: Fu, Siyuan; Ye, Ting; Wu, Jiaxin; edk2-***@lists.sourceforge.net
Subject: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

AsmReadTsc() implementation which is decelerated in BaseLib.h covers both IA32, X64 and IPF architecture, so it should specify which implementation to use when invoked.
1. Different Macro definition in DnsImpl.c to specify the implementation.
2. Modefy the NetworkPkg.dsc file to let the DnsDxe.inf is effective to different Arch.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
NetworkPkg/DnsDxe/DnsImpl.c | 23 ++++++++++++++++++++++-
NetworkPkg/NetworkPkg.dsc | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index b196d18..93b1835 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -13,10 +13,31 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/

#include "DnsImpl.h"

/**
+ Reads the current value of Time Stamp Counter (TSC) or the Interval
+ Timer Counter Register (ITC)
+
+ @return The current value of TSC or ITC
+
+**/
+
+UINT64
+ReadTime (
+ VOID
+ )
+{
+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
+ return AsmReadTsc ();
+#elif defined (MDE_CPU_IPF)
+ return AsmReadItc ();
+#else
+#error ReadTime not supported for this architecture!
+#endif
+}
+
+/**
Remove TokenEntry from TokenMap.

@param[in] TokenMap All DNSv4 Token entrys.
@param[in] TokenEntry TokenEntry need to be removed.

@@ -1573,11 +1594,11 @@ ConstructDNSQueryIp (

//
// Fill header
//
DnsHeader = (DNS_HEADER *)Frag.Bulk;
- DnsHeader->Identification = (UINT16)AsmReadTsc ();
+ DnsHeader->Identification = (UINT16)ReadTime();
DnsHeader->Flags.Uint16 = 0x0000;
DnsHeader->Flags.Bits.RD = 1;
DnsHeader->Flags.Bits.OpCode = DNS_FLAGS_OPCODE_STANDARD;
DnsHeader->Flags.Bits.QR = DNS_FLAGS_QR_QUERY;
DnsHeader->QuestionsNum = 1;
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc index 1d7fd85..11e0ce0 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -98,11 +98,10 @@
NetworkPkg/Ip6Dxe/Ip6Dxe.inf
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
- NetworkPkg/DnsDxe/DnsDxe.inf
NetworkPkg/HttpDxe/HttpDxe.inf
NetworkPkg/HttpBootDxe/HttpBootDxe.inf

NetworkPkg/Application/IfConfig6/IfConfig6.inf
NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
@@ -111,5 +110,6 @@
[Components.IA32, Components.X64, Components.IPF]
NetworkPkg/IpSecDxe/IpSecDxe.inf
NetworkPkg/IScsiDxe/IScsiDxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
NetworkPkg/Application/Ping6/Ping6.inf
+ NetworkPkg/DnsDxe/DnsDxe.inf
--
1.9.5.msysgit.1
Zhang, Lubo
2015-07-14 02:17:48 UTC
Permalink
Got it !

-----Original Message-----
From: Wu, Jiaxin
Sent: Tuesday, July 14, 2015 10:14 AM
To: Fu, Siyuan; Zhang, Lubo; Ye, Ting; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

Siyuan, you are right, That is a good suggestion.

Lubo, you can revise the patch to v2 according to Siyuan's opinion.

-----Original Message-----
From: Fu, Siyuan
Sent: Tuesday, July 14, 2015 10:09 AM
To: Wu, Jiaxin; Zhang, Lubo; Ye, Ting; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

Jiaxin

A soft network stack driver should not depend on a platform specific API. If it's just a random number to identify the message, you could use the NetLib API NetRandomInitSeed().

-----Original Message-----
From: Wu, Jiaxin
Sent: Tuesday, July 14, 2015 10:04 AM
To: Fu, Siyuan; Zhang, Lubo; Ye, Ting; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

It's used to generate any kind of query. This value should be a random value, so TSC register maybe called.

-----Original Message-----
From: Fu, Siyuan
Sent: Tuesday, July 14, 2015 9:55 AM
To: Zhang, Lubo; Ye, Ting; Wu, Jiaxin; edk2-***@lists.sourceforge.net
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

Lubo,

What's the DnsHeader->Identification means? Why it use the value of TSC register?

Siyuan

-----Original Message-----
From: Zhang, Lubo
Sent: Tuesday, July 14, 2015 9:50 AM
To: Fu, Siyuan; Ye, Ting; Wu, Jiaxin; edk2-***@lists.sourceforge.net
Subject: [patch] NetworkPkg: Fix an error that the call function declared implicitly.

AsmReadTsc() implementation which is decelerated in BaseLib.h covers both IA32, X64 and IPF architecture, so it should specify which implementation to use when invoked.
1. Different Macro definition in DnsImpl.c to specify the implementation.
2. Modefy the NetworkPkg.dsc file to let the DnsDxe.inf is effective to different Arch.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <***@intel.com>
---
NetworkPkg/DnsDxe/DnsImpl.c | 23 ++++++++++++++++++++++-
NetworkPkg/NetworkPkg.dsc | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index b196d18..93b1835 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -13,10 +13,31 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/

#include "DnsImpl.h"

/**
+ Reads the current value of Time Stamp Counter (TSC) or the Interval
+ Timer Counter Register (ITC)
+
+ @return The current value of TSC or ITC
+
+**/
+
+UINT64
+ReadTime (
+ VOID
+ )
+{
+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
+ return AsmReadTsc ();
+#elif defined (MDE_CPU_IPF)
+ return AsmReadItc ();
+#else
+#error ReadTime not supported for this architecture!
+#endif
+}
+
+/**
Remove TokenEntry from TokenMap.

@param[in] TokenMap All DNSv4 Token entrys.
@param[in] TokenEntry TokenEntry need to be removed.

@@ -1573,11 +1594,11 @@ ConstructDNSQueryIp (

//
// Fill header
//
DnsHeader = (DNS_HEADER *)Frag.Bulk;
- DnsHeader->Identification = (UINT16)AsmReadTsc ();
+ DnsHeader->Identification = (UINT16)ReadTime();
DnsHeader->Flags.Uint16 = 0x0000;
DnsHeader->Flags.Bits.RD = 1;
DnsHeader->Flags.Bits.OpCode = DNS_FLAGS_OPCODE_STANDARD;
DnsHeader->Flags.Bits.QR = DNS_FLAGS_QR_QUERY;
DnsHeader->QuestionsNum = 1;
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc index 1d7fd85..11e0ce0 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -98,11 +98,10 @@
NetworkPkg/Ip6Dxe/Ip6Dxe.inf
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
- NetworkPkg/DnsDxe/DnsDxe.inf
NetworkPkg/HttpDxe/HttpDxe.inf
NetworkPkg/HttpBootDxe/HttpBootDxe.inf

NetworkPkg/Application/IfConfig6/IfConfig6.inf
NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
@@ -111,5 +110,6 @@
[Components.IA32, Components.X64, Components.IPF]
NetworkPkg/IpSecDxe/IpSecDxe.inf
NetworkPkg/IScsiDxe/IScsiDxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
NetworkPkg/Application/Ping6/Ping6.inf
+ NetworkPkg/DnsDxe/DnsDxe.inf
--
1.9.5.msysgit.1
Andrew Fish
2015-07-14 02:29:42 UTC
Permalink
Sent from my iPhone
Post by Wu, Jiaxin
It's used to generate any kind of query. This value should be a random value, so TSC register maybe called.
But that is CPU and platform specific? For example an OS can disable TSC and this code would break an emulator. How does this code compile for both the ARM architectures?

Why not use the existing library API, GetPerformanceCounter() that abstracts the implementation.

Thanks

Andrew Fish
Post by Wu, Jiaxin
-----Original Message-----
From: Fu, Siyuan
Sent: Tuesday, July 14, 2015 9:55 AM
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.
Lubo,
What's the DnsHeader->Identification means? Why it use the value of TSC register?
Siyuan
-----Original Message-----
From: Zhang, Lubo
Sent: Tuesday, July 14, 2015 9:50 AM
Subject: [patch] NetworkPkg: Fix an error that the call function declared implicitly.
AsmReadTsc() implementation which is decelerated in BaseLib.h covers both IA32, X64 and IPF architecture, so it should specify which implementation to use when invoked.
1. Different Macro definition in DnsImpl.c to specify the implementation.
2. Modefy the NetworkPkg.dsc file to let the DnsDxe.inf is effective to different Arch.
Contributed-under: TianoCore Contribution Agreement 1.0
---
NetworkPkg/DnsDxe/DnsImpl.c | 23 ++++++++++++++++++++++-
NetworkPkg/NetworkPkg.dsc | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index b196d18..93b1835 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -13,10 +13,31 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
#include "DnsImpl.h"
/**
+ Reads the current value of Time Stamp Counter (TSC) or the Interval
+ Timer Counter Register (ITC)
+
+
+**/
+
+UINT64
+ReadTime (
+ VOID
+ )
+{
+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
+ return AsmReadTsc ();
+#elif defined (MDE_CPU_IPF)
+ return AsmReadItc ();
+#else
+#error ReadTime not supported for this architecture!
+#endif
+}
+
+/**
Remove TokenEntry from TokenMap.
@param[in] TokenMap All DNSv4 Token entrys.
@param[in] TokenEntry TokenEntry need to be removed.
@@ -1573,11 +1594,11 @@ ConstructDNSQueryIp (
//
// Fill header
//
DnsHeader = (DNS_HEADER *)Frag.Bulk;
- DnsHeader->Identification = (UINT16)AsmReadTsc ();
+ DnsHeader->Identification = (UINT16)ReadTime();
DnsHeader->Flags.Uint16 = 0x0000;
DnsHeader->Flags.Bits.RD = 1;
DnsHeader->Flags.Bits.OpCode = DNS_FLAGS_OPCODE_STANDARD;
DnsHeader->Flags.Bits.QR = DNS_FLAGS_QR_QUERY;
DnsHeader->QuestionsNum = 1;
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc index 1d7fd85..11e0ce0 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -98,11 +98,10 @@
NetworkPkg/Ip6Dxe/Ip6Dxe.inf
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
- NetworkPkg/DnsDxe/DnsDxe.inf
NetworkPkg/HttpDxe/HttpDxe.inf
NetworkPkg/HttpBootDxe/HttpBootDxe.inf
NetworkPkg/Application/IfConfig6/IfConfig6.inf
NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
@@ -111,5 +110,6 @@
[Components.IA32, Components.X64, Components.IPF]
NetworkPkg/IpSecDxe/IpSecDxe.inf
NetworkPkg/IScsiDxe/IScsiDxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
NetworkPkg/Application/Ping6/Ping6.inf
+ NetworkPkg/DnsDxe/DnsDxe.inf
--
1.9.5.msysgit.1
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Fu, Siyuan
2015-07-14 02:33:43 UTC
Permalink
Hi, Fish

You are right, it should not use a platform specific API.
NetLib has defined function NetRandomInitSeed() for this case, I have suggested Lubo to use it instead of read TSC.

Siyuan

-----Original Message-----
From: Andrew Fish [mailto:***@apple.com]
Sent: Tuesday, July 14, 2015 10:30 AM
To: Wu, Jiaxin
Cc: Fu, Siyuan; Zhang, Lubo; Ye, Ting; edk2-***@lists.sourceforge.net
Subject: Re: [edk2] [patch] NetworkPkg: Fix an error that the call function declared implicitly.



Sent from my iPhone
Post by Wu, Jiaxin
It's used to generate any kind of query. This value should be a random value, so TSC register maybe called.
But that is CPU and platform specific? For example an OS can disable TSC and this code would break an emulator. How does this code compile for both the ARM architectures?

Why not use the existing library API, GetPerformanceCounter() that abstracts the implementation.

Thanks

Andrew Fish
Post by Wu, Jiaxin
-----Original Message-----
From: Fu, Siyuan
Sent: Tuesday, July 14, 2015 9:55 AM
To: Zhang, Lubo; Ye, Ting; Wu, Jiaxin;
Subject: RE: [patch] NetworkPkg: Fix an error that the call function declared implicitly.
Lubo,
What's the DnsHeader->Identification means? Why it use the value of TSC register?
Siyuan
-----Original Message-----
From: Zhang, Lubo
Sent: Tuesday, July 14, 2015 9:50 AM
Subject: [patch] NetworkPkg: Fix an error that the call function declared implicitly.
AsmReadTsc() implementation which is decelerated in BaseLib.h covers both IA32, X64 and IPF architecture, so it should specify which implementation to use when invoked.
1. Different Macro definition in DnsImpl.c to specify the implementation.
2. Modefy the NetworkPkg.dsc file to let the DnsDxe.inf is effective to different Arch.
Contributed-under: TianoCore Contribution Agreement 1.0
---
NetworkPkg/DnsDxe/DnsImpl.c | 23 ++++++++++++++++++++++-
NetworkPkg/NetworkPkg.dsc | 2 +-
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c
index b196d18..93b1835 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -13,10 +13,31 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
#include "DnsImpl.h"
/**
+ Reads the current value of Time Stamp Counter (TSC) or the Interval
+ Timer Counter Register (ITC)
+
+
+**/
+
+UINT64
+ReadTime (
+ VOID
+ )
+{
+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
+ return AsmReadTsc ();
+#elif defined (MDE_CPU_IPF)
+ return AsmReadItc ();
+#else
+#error ReadTime not supported for this architecture!
+#endif
+}
+
+/**
Remove TokenEntry from TokenMap.
@param[in] TokenMap All DNSv4 Token entrys.
@param[in] TokenEntry TokenEntry need to be removed.
@@ -1573,11 +1594,11 @@ ConstructDNSQueryIp (
//
// Fill header
//
DnsHeader = (DNS_HEADER *)Frag.Bulk;
- DnsHeader->Identification = (UINT16)AsmReadTsc ();
+ DnsHeader->Identification = (UINT16)ReadTime();
DnsHeader->Flags.Uint16 = 0x0000;
DnsHeader->Flags.Bits.RD = 1;
DnsHeader->Flags.Bits.OpCode = DNS_FLAGS_OPCODE_STANDARD;
DnsHeader->Flags.Bits.QR = DNS_FLAGS_QR_QUERY;
DnsHeader->QuestionsNum = 1;
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc
index 1d7fd85..11e0ce0 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -98,11 +98,10 @@
NetworkPkg/Ip6Dxe/Ip6Dxe.inf
NetworkPkg/TcpDxe/TcpDxe.inf
NetworkPkg/Udp6Dxe/Udp6Dxe.inf
NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
- NetworkPkg/DnsDxe/DnsDxe.inf
NetworkPkg/HttpDxe/HttpDxe.inf
NetworkPkg/HttpBootDxe/HttpBootDxe.inf
NetworkPkg/Application/IfConfig6/IfConfig6.inf
NetworkPkg/Application/IpsecConfig/IpSecConfig.inf
@@ -111,5 +110,6 @@
[Components.IA32, Components.X64, Components.IPF]
NetworkPkg/IpSecDxe/IpSecDxe.inf
NetworkPkg/IScsiDxe/IScsiDxe.inf
NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
NetworkPkg/Application/Ping6/Ping6.inf
+ NetworkPkg/DnsDxe/DnsDxe.inf
--
1.9.5.msysgit.1
----------------------------------------------------------------------
-------- Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Loading...