Re: [PATCH v2 1/3] MdeModulePkg: Update NetLib interface to support classless addressing.

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Siyuan,

One comment for the NetIp4IsUnicast(), we should add more check for BROADCAST IP since the it's also not a valid unicast address.

  if (Ip == 0 || IP4_IS_LOCAL_BROADCAST (Ip)) {
    return FALSE;
  }

Others is good to me.

Reviewed-By: Wu Jiaxin <jiaxin.wu@intel.com>

Best Regards!
Jiaxin

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu
> Siyuan
> Sent: Thursday, October 27, 2016 6:52 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Zhang, Lubo <lubo.zhang@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [PATCH v2 1/3] MdeModulePkg: Update NetLib interface to
> support classless addressing.
> 
> V2:
> Add macro IP4_IS_UNSPECIFIED.
> 
> The classful addressing (IP class A/B/C) has been deprecated according to
> RFC4632. This patch updates the NetLib NetGetIpClass() and NetIp4IsUnicast()
> accordingly.
> 
> NetGetIpClass()
> The function is kept for compatibility, while the caller of this function could only
> check the returned value against with IP4_ADDR_CLASSD (multicast) or
> IP4_ADDR_CLASSE (reserved) now. The function has been updated to note this.
> 
> NetIp4IsUnicast()
> The NetMask becomes a required parameter to check the unicast address.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Zhang Lubo <lubo.zhang@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Include/Library/NetLib.h      | 23 ++++++++++++++---------
>  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 26 ++++++++++++--------------
>  2 files changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Library/NetLib.h
> b/MdeModulePkg/Include/Library/NetLib.h
> index c5c0fc2..26709af 100644
> --- a/MdeModulePkg/Include/Library/NetLib.h
> +++ b/MdeModulePkg/Include/Library/NetLib.h
> @@ -43,9 +43,9 @@ typedef UINT16          TCP_PORTNO;
>  //
>  // The address classification
>  //
> -#define  IP4_ADDR_CLASSA       1
> -#define  IP4_ADDR_CLASSB       2
> -#define  IP4_ADDR_CLASSC       3
> +#define  IP4_ADDR_CLASSA       1     // Deprecated
> +#define  IP4_ADDR_CLASSB       2     // Deprecated
> +#define  IP4_ADDR_CLASSC       3     // Deprecated
>  #define  IP4_ADDR_CLASSD       4
>  #define  IP4_ADDR_CLASSE       5
> 
> @@ -231,6 +231,7 @@ typedef struct {
>  // Test the IP's attribute, All the IPs are in host byte order.
>  //
>  #define IP4_IS_MULTICAST(Ip)              (((Ip) & 0xF0000000) == 0xE0000000)
> +#define IP4_IS_UNSPECIFIED(Ip)            ((Ip) == 0)
>  #define IP4_IS_LOCAL_BROADCAST(Ip)        ((Ip) == 0xFFFFFFFF)
>  #define IP4_NET_EQUAL(Ip1, Ip2, NetMask)  (((Ip1) & (NetMask)) == ((Ip2) &
> (NetMask)))
>  #define IP4_IS_VALID_NETMASK(Ip)          (NetGetMaskLength (Ip) !=
> (IP4_MASK_MAX + 1))
> @@ -379,6 +380,11 @@ NetGetMaskLength (
>    Return the class of the IP address, such as class A, B, C.
>    Addr is in host byte order.
> 
> +  [ATTENTION]
> +  Classful addressing (IP class A/B/C) has been deprecated according to
> RFC4632.
> +  Caller of this function could only check the returned value against
> + IP4_ADDR_CLASSD (multicast) or IP4_ADDR_CLASSE (reserved) now.
> +
>    The address of class A  starts with 0.
>    If the address belong to class A, return IP4_ADDR_CLASSA.
>    The address of class B  starts with 10.
> @@ -404,17 +410,16 @@ NetGetIpClass (
> 
>  /**
>    Check whether the IP is a valid unicast address according to
> -  the netmask. If NetMask is zero, use the IP address's class to get the default
> mask.
> +  the netmask.
> 
> -  If Ip is 0, IP is not a valid unicast address.
> -  Class D address is used for multicasting and class E address is reserved for
> future. If Ip
> -  belongs to class D or class E, Ip is not a valid unicast address.
> -  If all bits of the host address of Ip are 0 or 1, Ip is not a valid unicast address.
> +  ASSERT if NetMask is zero.
> +
> +  If all bits of the host address of IP are 0 or 1, IP is also not a valid unicast
> address.
> 
>    @param[in]  Ip                    The IP to check against.
>    @param[in]  NetMask               The mask of the IP.
> 
> -  @return TRUE if Ip is a valid unicast address on the network, otherwise FALSE.
> +  @return TRUE if IP is a valid unicast address on the network, otherwise
> FALSE.
> 
>  **/
>  BOOLEAN
> diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> index 148bebf..f520845 100644
> --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> @@ -580,6 +580,11 @@ NetGetMaskLength (
>    Return the class of the IP address, such as class A, B, C.
>    Addr is in host byte order.
> 
> +  [ATTENTION]
> +  Classful addressing (IP class A/B/C) has been deprecated according to
> RFC4632.
> +  Caller of this function could only check the returned value against
> + IP4_ADDR_CLASSD (multicast) or IP4_ADDR_CLASSE (reserved) now.
> +
>    The address of class A  starts with 0.
>    If the address belong to class A, return IP4_ADDR_CLASSA.
>    The address of class B  starts with 10.
> @@ -628,11 +633,10 @@ NetGetIpClass (
> 
>  /**
>    Check whether the IP is a valid unicast address according to
> -  the netmask. If NetMask is zero, use the IP address's class to get the default
> mask.
> +  the netmask.
> 
> -  If Ip is 0, IP is not a valid unicast address.
> -  Class D address is used for multicasting and class E address is reserved for
> future. If Ip
> -  belongs to class D or class E, IP is not a valid unicast address.
> +  ASSERT if NetMask is zero.
> +
>    If all bits of the host address of IP are 0 or 1, IP is also not a valid unicast
> address.
> 
>    @param[in]  Ip                    The IP to check against.
> @@ -648,18 +652,12 @@ NetIp4IsUnicast (
>    IN IP4_ADDR               NetMask
>    )
>  {
> -  INTN                      Class;
> -
> -  Class = NetGetIpClass (Ip);
> -
> -  if ((Ip == 0) || (Class >= IP4_ADDR_CLASSD)) {
> +  ASSERT (NetMask != 0);
> +
> +  if (Ip == 0) {
>      return FALSE;
>    }
> -
> -  if (NetMask == 0) {
> -    NetMask = gIp4AllMasks[Class << 3];
> -  }
> -
> +
>    if (((Ip &~NetMask) == ~NetMask) || ((Ip &~NetMask) == 0)) {
>      return FALSE;
>    }
> --
> 2.7.4.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel