public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 0/2] Add 32bit subnet mask support for IP4 PXE
@ 2018-08-28  1:52 Fu Siyuan
  2018-08-28  1:52 ` [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot Fu Siyuan
  2018-08-28  1:52 ` [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask Fu Siyuan
  0 siblings, 2 replies; 12+ messages in thread
From: Fu Siyuan @ 2018-08-28  1:52 UTC (permalink / raw)
  To: edk2-devel

Fu Siyuan (2):
  MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot.
  ShellPkg: Update Ifconfig command to accept 32bit subnet mask.

 MdeModulePkg/Include/Library/NetLib.h         |   5 +-
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c    |  13 +-
 .../Universal/Network/Ip4Dxe/Ip4Common.h      |   2 +-
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c | 117 ++++++++++++++++--
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h |   5 +-
 .../Universal/Network/Ip4Dxe/Ip4Impl.c        |   2 +-
 .../Universal/Network/Ip4Dxe/Ip4Output.c      |  11 +-
 .../Universal/Network/Ip4Dxe/Ip4Route.c       |  21 +++-
 .../Universal/Network/Ip4Dxe/Ip4Route.h       |   5 +-
 .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c  |   6 +-
 .../UefiShellNetwork1CommandsLib/Ifconfig.c   |   1 +
 11 files changed, 155 insertions(+), 33 deletions(-)

-- 
2.18.0.windows.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot.
  2018-08-28  1:52 [Patch 0/2] Add 32bit subnet mask support for IP4 PXE Fu Siyuan
@ 2018-08-28  1:52 ` Fu Siyuan
  2018-08-28  6:50   ` Wu, Jiaxin
  2018-08-28  1:52 ` [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask Fu Siyuan
  1 sibling, 1 reply; 12+ messages in thread
From: Fu Siyuan @ 2018-08-28  1:52 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ye Ting, Wu Jiaxin

This patch updates IP4 stack to support 32bit subnet mask in PXE boot process.
When 32bit subnet mask is used, the IP4 driver couldn't use the subnet mask to determine
whether destination IP address is on-link or not, so it will always try to send all the
packets to the destination IP address directly first, if failed it will continue
to try the default gateway.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
---
 MdeModulePkg/Include/Library/NetLib.h         |   5 +-
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c    |  13 +-
 .../Universal/Network/Ip4Dxe/Ip4Common.h      |   2 +-
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c | 117 ++++++++++++++++--
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h |   5 +-
 .../Universal/Network/Ip4Dxe/Ip4Impl.c        |   2 +-
 .../Universal/Network/Ip4Dxe/Ip4Output.c      |  11 +-
 .../Universal/Network/Ip4Dxe/Ip4Route.c       |  21 +++-
 .../Universal/Network/Ip4Dxe/Ip4Route.h       |   5 +-
 .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c  |   6 +-
 10 files changed, 154 insertions(+), 33 deletions(-)

diff --git a/MdeModulePkg/Include/Library/NetLib.h b/MdeModulePkg/Include/Library/NetLib.h
index ef7bc429c1..b7ef99c7b5 100644
--- a/MdeModulePkg/Include/Library/NetLib.h
+++ b/MdeModulePkg/Include/Library/NetLib.h
@@ -422,8 +422,9 @@ NetGetIpClass (
 
   If all bits of the host address of IP are 0 or 1, IP is also not a valid unicast address,
   except when the originator is one of the endpoints of a point-to-point link with a 31-bit
-  mask (RFC3021).
-
+  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special network environment (e.g.
+  PPP link).
+  
   @param[in]  Ip                    The IP to check against.
   @param[in]  NetMask               The mask of the IP.
 
diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
index bf8f5523e6..63f4724062 100644
--- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
+++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
@@ -654,8 +654,9 @@ NetGetIpClass (
 
   If all bits of the host address of IP are 0 or 1, IP is also not a valid unicast address,
   except when the originator is one of the endpoints of a point-to-point link with a 31-bit
-  mask (RFC3021).
-
+  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special network environment (e.g.
+  PPP link).
+  
   @param[in]  Ip                    The IP to check against.
   @param[in]  NetMask               The mask of the IP.
 
@@ -669,18 +670,20 @@ NetIp4IsUnicast (
   IN IP4_ADDR               NetMask
   )
 {
+  INTN   MaskLength;
+  
   ASSERT (NetMask != 0);
 
   if (Ip == 0 || IP4_IS_LOCAL_BROADCAST (Ip)) {
     return FALSE;
   }
 
-  if (NetGetMaskLength (NetMask) != 31) {
+  MaskLength = NetGetMaskLength (NetMask);
+  ASSERT ((MaskLength >= 0) && (MaskLength <= IP4_MASK_NUM));
+  if (MaskLength < 31) {
     if (((Ip &~NetMask) == ~NetMask) || ((Ip &~NetMask) == 0)) {
       return FALSE;
     }
-  } else {
-    return TRUE;
   }
 
   return TRUE;
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
index e0fffc9d0d..994a81f4de 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
@@ -55,7 +55,7 @@ typedef struct _IP4_SERVICE    IP4_SERVICE;
 /// Compose the fragment field to be used in the IP4 header.
 ///
 #define IP4_HEAD_FRAGMENT_FIELD(Df, Mf, Offset) \
-    ((UINT16)(((Df) ? 0x4000 : 0) | ((Mf) ? 0x2000 : 0) | (((Offset) >> 3) & 0x1fff)))
+    ((UINT16)(((Df) ? IP4_HEAD_DF_MASK : 0) | ((Mf) ? IP4_HEAD_MF_MASK : 0) | (((Offset) >> 3) & IP4_HEAD_OFFSET_MASK)))
 
 #define IP4_LAST_FRAGMENT(FragmentField)  \
           (((FragmentField) & IP4_HEAD_MF_MASK) == 0)
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
index 6e0e3290c7..b0172283b7 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
@@ -138,6 +138,7 @@ Ip4CancelFrameArp (
   @param[in]  CallBack          Call back function to execute if transmission
                                 finished.
   @param[in]  Context           Opaque parameter to the call back.
+  @param[in]  IpSb              The pointer to the IP4 service binding instance.
 
   @retval   Token               The wrapped token if succeed
   @retval   NULL                The wrapped token if NULL
@@ -149,7 +150,8 @@ Ip4WrapLinkTxToken (
   IN IP4_PROTOCOL           *IpInstance     OPTIONAL,
   IN NET_BUF                *Packet,
   IN IP4_FRAME_CALLBACK     CallBack,
-  IN VOID                   *Context
+  IN VOID                   *Context,
+  IN IP4_SERVICE            *IpSb
   )
 {
   EFI_MANAGED_NETWORK_COMPLETION_TOKEN  *MnpToken;
@@ -170,6 +172,7 @@ Ip4WrapLinkTxToken (
 
   Token->Interface  = Interface;
   Token->IpInstance = IpInstance;
+  Token->IpSb       = IpSb;
   Token->CallBack   = CallBack;
   Token->Packet     = Packet;
   Token->Context    = Context;
@@ -792,6 +795,86 @@ Ip4FreeInterface (
   return EFI_SUCCESS;
 }
 
+/**
+  This function tries to send all the queued frames in ArpQue to the default gateway if 
+  the ARP resolve for direct destination address is failed when using /32 subnet mask.
+
+  @param[in]   ArpQue           The ARP queue of a failed request.
+  
+  @retval EFI_SUCCESS           All the queued frames have been send to the default route.
+  @retval Others                Failed to send the queued frames.
+  
+**/
+EFI_STATUS
+Ip4SendFrameToDefaultRoute (
+  IN  IP4_ARP_QUE               *ArpQue
+  )
+{
+  LIST_ENTRY                *Entry;
+  LIST_ENTRY                *Next;
+  IP4_ROUTE_CACHE_ENTRY     *RtCacheEntry;
+  IP4_LINK_TX_TOKEN         *Token;
+  IP4_ADDR                  Gateway;
+  EFI_STATUS                Status;
+  IP4_ROUTE_ENTRY           *DefaultRoute;
+  
+  //
+  // ARP resolve failed when using /32 subnet mask.
+  //
+  NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
+    RemoveEntryList (Entry);
+    Token = NET_LIST_USER_STRUCT (Entry, IP4_LINK_TX_TOKEN, Link);
+    ASSERT (Token->Interface->SubnetMask == IP4_ALLONE_ADDRESS);
+    //
+    // Find the default gateway IP address. The default route was saved to the RtCacheEntry->Tag in Ip4Route().
+    //
+    RtCacheEntry = NULL;
+    if (Token->IpInstance != NULL) {
+      RtCacheEntry = Ip4FindRouteCache (Token->IpInstance->RouteTable, NTOHL (ArpQue->Ip), Token->Interface->Ip);
+    }
+    if (RtCacheEntry == NULL) {
+      RtCacheEntry = Ip4FindRouteCache (Token->IpSb->DefaultRouteTable, NTOHL (ArpQue->Ip), Token->Interface->Ip);
+    }
+    if (RtCacheEntry == NULL) {
+      Status= EFI_NO_MAPPING;
+      goto ON_ERROR;
+    }
+    DefaultRoute = (IP4_ROUTE_ENTRY*)RtCacheEntry->Tag;
+    if (DefaultRoute == NULL) {
+      Status= EFI_NO_MAPPING;
+      goto ON_ERROR;
+    }
+    //
+    // Try to send the frame to the default route.
+    //
+    Gateway = DefaultRoute->NextHop;
+    if (ArpQue->Ip == Gateway) {
+      //
+      // ARP resolve for the default route is failed, return error to caller. 
+      //
+      Status= EFI_NO_MAPPING;
+      goto ON_ERROR;
+    }
+    RtCacheEntry->NextHop = Gateway;
+    Status = Ip4SendFrame (Token->Interface,Token->IpInstance,Token->Packet,Gateway,Token->CallBack,Token->Context,Token->IpSb);
+    if (EFI_ERROR (Status)) {
+      Status= EFI_NO_MAPPING;
+      goto ON_ERROR;
+    }
+    Ip4FreeRouteCacheEntry (RtCacheEntry);
+  }
+
+  return EFI_SUCCESS;
+  
+ON_ERROR:
+  if (RtCacheEntry != NULL) {
+    Ip4FreeRouteCacheEntry (RtCacheEntry);
+  }
+  Token->CallBack (Token->IpInstance, Token->Packet, Status, 0, Token->Context);
+  Ip4FreeLinkTxToken (Token);
+  return Status;
+}
+
 
 /**
   Callback function when ARP request are finished. It will cancelled
@@ -814,6 +897,7 @@ Ip4OnArpResolvedDpc (
   IP4_INTERFACE             *Interface;
   IP4_LINK_TX_TOKEN         *Token;
   EFI_STATUS                Status;
+  EFI_STATUS                IoStatus;
 
   ArpQue = (IP4_ARP_QUE *) Context;
   NET_CHECK_SIGNATURE (ArpQue, IP4_FRAME_ARP_SIGNATURE);
@@ -821,14 +905,23 @@ Ip4OnArpResolvedDpc (
   RemoveEntryList (&ArpQue->Link);
 
   //
-  // ARP resolve failed for some reason. Release all the frame
-  // and ARP queue itself. Ip4FreeArpQue will call the frame's
-  // owner back.
+  // ARP resolve failed for some reason. 
   //
   if (NET_MAC_EQUAL (&ArpQue->Mac, &mZeroMacAddress, ArpQue->Interface->HwaddrLen)) {
-    Ip4FreeArpQue (ArpQue, EFI_NO_MAPPING);
-
-    return ;
+    if (ArpQue->Interface->SubnetMask != IP4_ALLONE_ADDRESS) {
+      //
+      // Release all the frame and ARP queue itself. Ip4FreeArpQue will call the frame's
+      // owner back.
+      //
+      IoStatus = EFI_NO_MAPPING;
+    } else {
+      //
+      // ARP resolve failed when using 32bit subnet mask, try to send the packets to the
+      // default route.
+      //
+      IoStatus = Ip4SendFrameToDefaultRoute (ArpQue);
+    }
+    goto ON_EXIT;
   }
 
   //
@@ -836,6 +929,7 @@ Ip4OnArpResolvedDpc (
   // queue. It isn't necessary for us to cache the ARP binding because
   // we always check the ARP cache first before transmit.
   //
+  IoStatus = EFI_SUCCESS;
   Interface = ArpQue->Interface;
 
   NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
@@ -863,7 +957,8 @@ Ip4OnArpResolvedDpc (
     }
   }
 
-  Ip4FreeArpQue (ArpQue, EFI_SUCCESS);
+ON_EXIT:
+  Ip4FreeArpQue (ArpQue, IoStatus);
 }
 
 /**
@@ -957,6 +1052,7 @@ Ip4OnFrameSent (
                                 to.
   @param[in]  CallBack          Function to call back when transmit finished.
   @param[in]  Context           Opaque parameter to the call back.
+  @param[in]  IpSb              The pointer to the IP4 service binding instance.
 
   @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send the frame
   @retval EFI_NO_MAPPING        Can't resolve the MAC for the nexthop
@@ -971,7 +1067,8 @@ Ip4SendFrame (
   IN  NET_BUF               *Packet,
   IN  IP4_ADDR              NextHop,
   IN  IP4_FRAME_CALLBACK    CallBack,
-  IN  VOID                  *Context
+  IN  VOID                  *Context,
+  IN IP4_SERVICE            *IpSb
   )
 {
   IP4_LINK_TX_TOKEN         *Token;
@@ -982,7 +1079,7 @@ Ip4SendFrame (
 
   ASSERT (Interface->Configured);
 
-  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet, CallBack, Context);
+  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet, CallBack, Context, IpSb);
 
   if (Token == NULL) {
     return EFI_OUT_OF_RESOURCES;
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
index 909837131e..36e4ab3f7a 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
@@ -79,6 +79,7 @@ typedef struct {
   LIST_ENTRY                            Link;
 
   IP4_INTERFACE                         *Interface;
+  IP4_SERVICE                           *IpSb;
 
   IP4_PROTOCOL                          *IpInstance;
   IP4_FRAME_CALLBACK                    CallBack;
@@ -262,6 +263,7 @@ Ip4FreeInterface (
                                 to.
   @param[in]  CallBack          Function to call back when transmit finished.
   @param[in]  Context           Opaque parameter to the call back.
+  @param[in]  IpSb              The pointer to the IP4 service binding instance.
 
   @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send the frame
   @retval EFI_NO_MAPPING        Can't resolve the MAC for the nexthop
@@ -276,7 +278,8 @@ Ip4SendFrame (
   IN  NET_BUF               *Packet,
   IN  IP4_ADDR              NextHop,
   IN  IP4_FRAME_CALLBACK    CallBack,
-  IN  VOID                  *Context
+  IN  VOID                  *Context,
+  IN IP4_SERVICE            *IpSb
   );
 
 /**
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
index 6a26143e30..7c27db6753 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
@@ -1259,7 +1259,7 @@ EfiIp4Routes (
   // the gateway address must be a unicast on the connected network if not zero.
   //
   if ((Nexthop != IP4_ALLZERO_ADDRESS) &&
-      (!IP4_NET_EQUAL (Nexthop, IpIf->Ip, IpIf->SubnetMask) ||
+      ((IpIf->SubnetMask != IP4_ALLONE_ADDRESS && !IP4_NET_EQUAL (Nexthop, IpIf->Ip, IpIf->SubnetMask)) ||
         IP4_IS_BROADCAST (Ip4GetNetCast (Nexthop, IpIf)))) {
 
     Status = EFI_INVALID_PARAMETER;
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
index 1716f43576..bbbcf36eff 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
@@ -309,15 +309,15 @@ Ip4Output (
     // Route the packet unless overrided, that is, GateWay isn't zero.
     //
     if (IpInstance == NULL) {
-      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head->Src);
+      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head->Src, IpIf->SubnetMask);
     } else {
-      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head->Src);
+      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head->Src, IpIf->SubnetMask);
       //
       // If failed to route the packet by using the instance's route table,
       // try to use the default route table.
       //
       if (CacheEntry == NULL) {
-        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head->Src);
+        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head->Src, IpIf->SubnetMask);
       }
     }
 
@@ -386,7 +386,8 @@ Ip4Output (
                  Fragment,
                  GateWay,
                  Ip4SysPacketSent,
-                 Packet
+                 Packet,
+                 IpSb
                  );
 
       if (EFI_ERROR (Status)) {
@@ -429,7 +430,7 @@ Ip4Output (
   //    upper layer's packets.
   //
   Ip4PrependHead (Packet, Head, Option, OptLen);
-  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay, Callback, Context);
+  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay, Callback, Context, IpSb);
 
   if (EFI_ERROR (Status)) {
     goto ON_ERROR;
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
index d240d5343a..e648dee908 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
@@ -494,6 +494,8 @@ Ip4FindRouteEntry (
   @param[in]  RtTable               The route table to search from
   @param[in]  Dest                  The destination address to search for
   @param[in]  Src                   The source address to search for
+  @param[in]  SubnetMask            The subnet mask of the Src address, this field is
+                                    used to check if the station is using /32 subnet.
 
   @return NULL if failed to route packet, otherwise a route cache
           entry that can be used to route packet.
@@ -503,7 +505,8 @@ IP4_ROUTE_CACHE_ENTRY *
 Ip4Route (
   IN IP4_ROUTE_TABLE        *RtTable,
   IN IP4_ADDR               Dest,
-  IN IP4_ADDR               Src
+  IN IP4_ADDR               Src,
+  IN IP4_ADDR               SubnetMask
   )
 {
   LIST_ENTRY                *Head;
@@ -534,7 +537,10 @@ Ip4Route (
   //
   RtEntry = Ip4FindRouteEntry (RtTable, Dest);
 
-  if (RtEntry == NULL) {
+  //
+  // We will always try to use the direct destination address when /32 subnet mask is used.
+  //
+  if (RtEntry == NULL && SubnetMask != IP4_ALLONE_ADDRESS) {
     return NULL;
   }
 
@@ -544,16 +550,23 @@ Ip4Route (
   // network. Otherwise, it is an indirect route, the packet will be
   // sent to the next hop router.
   //
-  if ((RtEntry->Flag & IP4_DIRECT_ROUTE) != 0) {
+  // When using /32 subnet mask, the packet will always be sent to the direct
+  // destination first, if we can't find a matching route cache.
+  //
+  if (SubnetMask == IP4_ALLONE_ADDRESS || ((RtEntry->Flag & IP4_DIRECT_ROUTE) != 0)) {
     NextHop = Dest;
   } else {
     NextHop = RtEntry->NextHop;
   }
 
-  Ip4FreeRouteEntry (RtEntry);
+  if (RtEntry != NULL) {
+    Ip4FreeRouteEntry (RtEntry);
+  }
 
   //
   // Create a route cache entry, and tag it as spawned from this route entry
+  // For /32 subnet mask, the default route in RtEntry will be used if failed
+  // to send the packet to driect destination address.
   //
   RtCacheEntry = Ip4CreateRouteCacheEntry (Dest, Src, NextHop, (UINTN) RtEntry);
 
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
index 6269f4ceda..99b9cfbbac 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
@@ -194,6 +194,8 @@ Ip4FreeRouteCacheEntry (
   @param[in]  RtTable               The route table to search from
   @param[in]  Dest                  The destination address to search for
   @param[in]  Src                   The source address to search for
+  @param[in]  SubnetMask            The subnet mask of the Src address, this field is
+                                    used to check if the station is using /32 subnet.
 
   @return NULL if failed to route packet, otherwise a route cache
           entry that can be used to route packet.
@@ -203,7 +205,8 @@ IP4_ROUTE_CACHE_ENTRY *
 Ip4Route (
   IN IP4_ROUTE_TABLE        *RtTable,
   IN IP4_ADDR               Dest,
-  IN IP4_ADDR               Src
+  IN IP4_ADDR               Src,
+  IN IP4_ADDR               SubnetMask
   );
 
 /**
diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
index d5a1a8c303..03903640b8 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
@@ -509,8 +509,9 @@ Mtftp4Start (
     goto ON_ERROR;
   }
 
+  gBS->RestoreTPL(OldTpl);
+
   if (Token->Event != NULL) {
-    gBS->RestoreTPL (OldTpl);
     return EFI_SUCCESS;
   }
 
@@ -522,7 +523,6 @@ Mtftp4Start (
     This->Poll (This);
   }
 
-  gBS->RestoreTPL (OldTpl);
   return Token->Status;
 
 ON_ERROR:
@@ -682,7 +682,7 @@ EfiMtftp4Configure (
     }
 
     if ((Gateway != 0) &&
-        (!IP4_NET_EQUAL (Gateway, Ip, Netmask) || (Netmask != 0 && !NetIp4IsUnicast (Gateway, Netmask)))) {
+        ((Netmask != 0xFFFFFFFF && !IP4_NET_EQUAL (Gateway, Ip, Netmask)) || (Netmask != 0 && !NetIp4IsUnicast (Gateway, Netmask)))) {
 
       return EFI_INVALID_PARAMETER;
     }
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask.
  2018-08-28  1:52 [Patch 0/2] Add 32bit subnet mask support for IP4 PXE Fu Siyuan
  2018-08-28  1:52 ` [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot Fu Siyuan
@ 2018-08-28  1:52 ` Fu Siyuan
  2018-08-28 14:55   ` Carsey, Jaben
                     ` (2 more replies)
  1 sibling, 3 replies; 12+ messages in thread
From: Fu Siyuan @ 2018-08-28  1:52 UTC (permalink / raw)
  To: edk2-devel; +Cc: Ruiyu Ni, Ye Ting, Wu Jiaxin

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
---
 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
index 52415e0ad0..e9f644c739 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
@@ -1032,6 +1032,7 @@ IfConfigSetInterfaceInfo (
       SubnetMask  = NTOHL (SubnetMask);
       TempGateway = NTOHL (TempGateway);
       if ((SubnetMask != 0) &&
+          (SubnetMask != 0xFFFFFFFFu) && 
           !NetIp4IsUnicast (TempGateway, SubnetMask)) {
         ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INVALID_GATEWAY), gShellNetwork1HiiHandle, VarArg->Arg);
         ShellStatus = SHELL_INVALID_PARAMETER;
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot.
  2018-08-28  1:52 ` [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot Fu Siyuan
@ 2018-08-28  6:50   ` Wu, Jiaxin
  2018-08-28  7:12     ` Fu, Siyuan
  0 siblings, 1 reply; 12+ messages in thread
From: Wu, Jiaxin @ 2018-08-28  6:50 UTC (permalink / raw)
  To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ye, Ting

Hi Siyuan,

In Ip4SendFrameToDefaultRoute(), you tried to find the default gateway IP address by using the found RtCacheEntry->Tag, I'm confused why it's the default gateway? In my understanding, tag is the cache's corresponding route entry.  Besides, why we must send the frame to "DefaultRouter", should be the instance router first, then default router? If so, I suggest to rename the function to Ip4SendFrameToRoute(). Then, the logic in the function to retrieve the gateway should be: 
{
    if (IpInstance == NULL) {
      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head->Src, IpIf->SubnetMask);
    } else {
      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head->Src, IpIf->SubnetMask);
      if (CacheEntry == NULL) {
        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head->Src, IpIf->SubnetMask);
      }
    }

    if (CacheEntry == NULL) {
      return EFI_NOT_FOUND;
    }
    GateWay = CacheEntry->NextHop;
    Ip4FreeRouteCacheEntry (CacheEntry);
}

What do you think?

Thanks,
Jiaxin








> -----Original Message-----
> From: Fu, Siyuan
> Sent: Tuesday, August 28, 2018 9:53 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> support for IP4 PXE boot.
> 
> This patch updates IP4 stack to support 32bit subnet mask in PXE boot
> process.
> When 32bit subnet mask is used, the IP4 driver couldn't use the subnet mask
> to determine
> whether destination IP address is on-link or not, so it will always try to send
> all the
> packets to the destination IP address directly first, if failed it will continue
> to try the default gateway.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Include/Library/NetLib.h         |   5 +-
>  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c    |  13 +-
>  .../Universal/Network/Ip4Dxe/Ip4Common.h      |   2 +-
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c | 117
> ++++++++++++++++--
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h |   5 +-
>  .../Universal/Network/Ip4Dxe/Ip4Impl.c        |   2 +-
>  .../Universal/Network/Ip4Dxe/Ip4Output.c      |  11 +-
>  .../Universal/Network/Ip4Dxe/Ip4Route.c       |  21 +++-
>  .../Universal/Network/Ip4Dxe/Ip4Route.h       |   5 +-
>  .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c  |   6 +-
>  10 files changed, 154 insertions(+), 33 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Library/NetLib.h
> b/MdeModulePkg/Include/Library/NetLib.h
> index ef7bc429c1..b7ef99c7b5 100644
> --- a/MdeModulePkg/Include/Library/NetLib.h
> +++ b/MdeModulePkg/Include/Library/NetLib.h
> @@ -422,8 +422,9 @@ NetGetIpClass (
> 
>    If all bits of the host address of IP are 0 or 1, IP is also not a valid unicast
> address,
>    except when the originator is one of the endpoints of a point-to-point link
> with a 31-bit
> -  mask (RFC3021).
> -
> +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special network
> environment (e.g.
> +  PPP link).
> +
>    @param[in]  Ip                    The IP to check against.
>    @param[in]  NetMask               The mask of the IP.
> 
> diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> index bf8f5523e6..63f4724062 100644
> --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> @@ -654,8 +654,9 @@ NetGetIpClass (
> 
>    If all bits of the host address of IP are 0 or 1, IP is also not a valid unicast
> address,
>    except when the originator is one of the endpoints of a point-to-point link
> with a 31-bit
> -  mask (RFC3021).
> -
> +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special network
> environment (e.g.
> +  PPP link).
> +
>    @param[in]  Ip                    The IP to check against.
>    @param[in]  NetMask               The mask of the IP.
> 
> @@ -669,18 +670,20 @@ NetIp4IsUnicast (
>    IN IP4_ADDR               NetMask
>    )
>  {
> +  INTN   MaskLength;
> +
>    ASSERT (NetMask != 0);
> 
>    if (Ip == 0 || IP4_IS_LOCAL_BROADCAST (Ip)) {
>      return FALSE;
>    }
> 
> -  if (NetGetMaskLength (NetMask) != 31) {
> +  MaskLength = NetGetMaskLength (NetMask);
> +  ASSERT ((MaskLength >= 0) && (MaskLength <= IP4_MASK_NUM));
> +  if (MaskLength < 31) {
>      if (((Ip &~NetMask) == ~NetMask) || ((Ip &~NetMask) == 0)) {
>        return FALSE;
>      }
> -  } else {
> -    return TRUE;
>    }
> 
>    return TRUE;
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> index e0fffc9d0d..994a81f4de 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> @@ -55,7 +55,7 @@ typedef struct _IP4_SERVICE    IP4_SERVICE;
>  /// Compose the fragment field to be used in the IP4 header.
>  ///
>  #define IP4_HEAD_FRAGMENT_FIELD(Df, Mf, Offset) \
> -    ((UINT16)(((Df) ? 0x4000 : 0) | ((Mf) ? 0x2000 : 0) | (((Offset) >> 3) &
> 0x1fff)))
> +    ((UINT16)(((Df) ? IP4_HEAD_DF_MASK : 0) | ((Mf) ?
> IP4_HEAD_MF_MASK : 0) | (((Offset) >> 3) & IP4_HEAD_OFFSET_MASK)))
> 
>  #define IP4_LAST_FRAGMENT(FragmentField)  \
>            (((FragmentField) & IP4_HEAD_MF_MASK) == 0)
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> index 6e0e3290c7..b0172283b7 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> @@ -138,6 +138,7 @@ Ip4CancelFrameArp (
>    @param[in]  CallBack          Call back function to execute if transmission
>                                  finished.
>    @param[in]  Context           Opaque parameter to the call back.
> +  @param[in]  IpSb              The pointer to the IP4 service binding instance.
> 
>    @retval   Token               The wrapped token if succeed
>    @retval   NULL                The wrapped token if NULL
> @@ -149,7 +150,8 @@ Ip4WrapLinkTxToken (
>    IN IP4_PROTOCOL           *IpInstance     OPTIONAL,
>    IN NET_BUF                *Packet,
>    IN IP4_FRAME_CALLBACK     CallBack,
> -  IN VOID                   *Context
> +  IN VOID                   *Context,
> +  IN IP4_SERVICE            *IpSb
>    )
>  {
>    EFI_MANAGED_NETWORK_COMPLETION_TOKEN  *MnpToken;
> @@ -170,6 +172,7 @@ Ip4WrapLinkTxToken (
> 
>    Token->Interface  = Interface;
>    Token->IpInstance = IpInstance;
> +  Token->IpSb       = IpSb;
>    Token->CallBack   = CallBack;
>    Token->Packet     = Packet;
>    Token->Context    = Context;
> @@ -792,6 +795,86 @@ Ip4FreeInterface (
>    return EFI_SUCCESS;
>  }
> 
> +/**
> +  This function tries to send all the queued frames in ArpQue to the default
> gateway if
> +  the ARP resolve for direct destination address is failed when using /32
> subnet mask.
> +
> +  @param[in]   ArpQue           The ARP queue of a failed request.
> +
> +  @retval EFI_SUCCESS           All the queued frames have been send to the
> default route.
> +  @retval Others                Failed to send the queued frames.
> +
> +**/
> +EFI_STATUS
> +Ip4SendFrameToDefaultRoute (
> +  IN  IP4_ARP_QUE               *ArpQue
> +  )
> +{
> +  LIST_ENTRY                *Entry;
> +  LIST_ENTRY                *Next;
> +  IP4_ROUTE_CACHE_ENTRY     *RtCacheEntry;
> +  IP4_LINK_TX_TOKEN         *Token;
> +  IP4_ADDR                  Gateway;
> +  EFI_STATUS                Status;
> +  IP4_ROUTE_ENTRY           *DefaultRoute;
> +
> +  //
> +  // ARP resolve failed when using /32 subnet mask.
> +  //
> +  NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> +    RemoveEntryList (Entry);
> +    Token = NET_LIST_USER_STRUCT (Entry, IP4_LINK_TX_TOKEN, Link);
> +    ASSERT (Token->Interface->SubnetMask == IP4_ALLONE_ADDRESS);
> +    //
> +    // Find the default gateway IP address. The default route was saved to
> the RtCacheEntry->Tag in Ip4Route().
> +    //
> +    RtCacheEntry = NULL;
> +    if (Token->IpInstance != NULL) {
> +      RtCacheEntry = Ip4FindRouteCache (Token->IpInstance->RouteTable,
> NTOHL (ArpQue->Ip), Token->Interface->Ip);
> +    }
> +    if (RtCacheEntry == NULL) {
> +      RtCacheEntry = Ip4FindRouteCache (Token->IpSb->DefaultRouteTable,
> NTOHL (ArpQue->Ip), Token->Interface->Ip);
> +    }
> +    if (RtCacheEntry == NULL) {
> +      Status= EFI_NO_MAPPING;
> +      goto ON_ERROR;
> +    }
> +    DefaultRoute = (IP4_ROUTE_ENTRY*)RtCacheEntry->Tag;
> +    if (DefaultRoute == NULL) {
> +      Status= EFI_NO_MAPPING;
> +      goto ON_ERROR;
> +    }
> +    //
> +    // Try to send the frame to the default route.
> +    //
> +    Gateway = DefaultRoute->NextHop;
> +    if (ArpQue->Ip == Gateway) {
> +      //
> +      // ARP resolve for the default route is failed, return error to caller.
> +      //
> +      Status= EFI_NO_MAPPING;
> +      goto ON_ERROR;
> +    }
> +    RtCacheEntry->NextHop = Gateway;
> +    Status = Ip4SendFrame (Token->Interface,Token->IpInstance,Token-
> >Packet,Gateway,Token->CallBack,Token->Context,Token->IpSb);
> +    if (EFI_ERROR (Status)) {
> +      Status= EFI_NO_MAPPING;
> +      goto ON_ERROR;
> +    }
> +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> +  }
> +
> +  return EFI_SUCCESS;
> +
> +ON_ERROR:
> +  if (RtCacheEntry != NULL) {
> +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> +  }
> +  Token->CallBack (Token->IpInstance, Token->Packet, Status, 0, Token-
> >Context);
> +  Ip4FreeLinkTxToken (Token);
> +  return Status;
> +}
> +
> 
>  /**
>    Callback function when ARP request are finished. It will cancelled
> @@ -814,6 +897,7 @@ Ip4OnArpResolvedDpc (
>    IP4_INTERFACE             *Interface;
>    IP4_LINK_TX_TOKEN         *Token;
>    EFI_STATUS                Status;
> +  EFI_STATUS                IoStatus;
> 
>    ArpQue = (IP4_ARP_QUE *) Context;
>    NET_CHECK_SIGNATURE (ArpQue, IP4_FRAME_ARP_SIGNATURE);
> @@ -821,14 +905,23 @@ Ip4OnArpResolvedDpc (
>    RemoveEntryList (&ArpQue->Link);
> 
>    //
> -  // ARP resolve failed for some reason. Release all the frame
> -  // and ARP queue itself. Ip4FreeArpQue will call the frame's
> -  // owner back.
> +  // ARP resolve failed for some reason.
>    //
>    if (NET_MAC_EQUAL (&ArpQue->Mac, &mZeroMacAddress, ArpQue-
> >Interface->HwaddrLen)) {
> -    Ip4FreeArpQue (ArpQue, EFI_NO_MAPPING);
> -
> -    return ;
> +    if (ArpQue->Interface->SubnetMask != IP4_ALLONE_ADDRESS) {
> +      //
> +      // Release all the frame and ARP queue itself. Ip4FreeArpQue will call the
> frame's
> +      // owner back.
> +      //
> +      IoStatus = EFI_NO_MAPPING;
> +    } else {
> +      //
> +      // ARP resolve failed when using 32bit subnet mask, try to send the
> packets to the
> +      // default route.
> +      //
> +      IoStatus = Ip4SendFrameToDefaultRoute (ArpQue);
> +    }
> +    goto ON_EXIT;
>    }
> 
>    //
> @@ -836,6 +929,7 @@ Ip4OnArpResolvedDpc (
>    // queue. It isn't necessary for us to cache the ARP binding because
>    // we always check the ARP cache first before transmit.
>    //
> +  IoStatus = EFI_SUCCESS;
>    Interface = ArpQue->Interface;
> 
>    NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> @@ -863,7 +957,8 @@ Ip4OnArpResolvedDpc (
>      }
>    }
> 
> -  Ip4FreeArpQue (ArpQue, EFI_SUCCESS);
> +ON_EXIT:
> +  Ip4FreeArpQue (ArpQue, IoStatus);
>  }
> 
>  /**
> @@ -957,6 +1052,7 @@ Ip4OnFrameSent (
>                                  to.
>    @param[in]  CallBack          Function to call back when transmit finished.
>    @param[in]  Context           Opaque parameter to the call back.
> +  @param[in]  IpSb              The pointer to the IP4 service binding instance.
> 
>    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send the
> frame
>    @retval EFI_NO_MAPPING        Can't resolve the MAC for the nexthop
> @@ -971,7 +1067,8 @@ Ip4SendFrame (
>    IN  NET_BUF               *Packet,
>    IN  IP4_ADDR              NextHop,
>    IN  IP4_FRAME_CALLBACK    CallBack,
> -  IN  VOID                  *Context
> +  IN  VOID                  *Context,
> +  IN IP4_SERVICE            *IpSb
>    )
>  {
>    IP4_LINK_TX_TOKEN         *Token;
> @@ -982,7 +1079,7 @@ Ip4SendFrame (
> 
>    ASSERT (Interface->Configured);
> 
> -  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet, CallBack,
> Context);
> +  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet, CallBack,
> Context, IpSb);
> 
>    if (Token == NULL) {
>      return EFI_OUT_OF_RESOURCES;
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> index 909837131e..36e4ab3f7a 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> @@ -79,6 +79,7 @@ typedef struct {
>    LIST_ENTRY                            Link;
> 
>    IP4_INTERFACE                         *Interface;
> +  IP4_SERVICE                           *IpSb;
> 
>    IP4_PROTOCOL                          *IpInstance;
>    IP4_FRAME_CALLBACK                    CallBack;
> @@ -262,6 +263,7 @@ Ip4FreeInterface (
>                                  to.
>    @param[in]  CallBack          Function to call back when transmit finished.
>    @param[in]  Context           Opaque parameter to the call back.
> +  @param[in]  IpSb              The pointer to the IP4 service binding instance.
> 
>    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send the
> frame
>    @retval EFI_NO_MAPPING        Can't resolve the MAC for the nexthop
> @@ -276,7 +278,8 @@ Ip4SendFrame (
>    IN  NET_BUF               *Packet,
>    IN  IP4_ADDR              NextHop,
>    IN  IP4_FRAME_CALLBACK    CallBack,
> -  IN  VOID                  *Context
> +  IN  VOID                  *Context,
> +  IN IP4_SERVICE            *IpSb
>    );
> 
>  /**
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> index 6a26143e30..7c27db6753 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> @@ -1259,7 +1259,7 @@ EfiIp4Routes (
>    // the gateway address must be a unicast on the connected network if not
> zero.
>    //
>    if ((Nexthop != IP4_ALLZERO_ADDRESS) &&
> -      (!IP4_NET_EQUAL (Nexthop, IpIf->Ip, IpIf->SubnetMask) ||
> +      ((IpIf->SubnetMask != IP4_ALLONE_ADDRESS && !IP4_NET_EQUAL
> (Nexthop, IpIf->Ip, IpIf->SubnetMask)) ||
>          IP4_IS_BROADCAST (Ip4GetNetCast (Nexthop, IpIf)))) {
> 
>      Status = EFI_INVALID_PARAMETER;
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> index 1716f43576..bbbcf36eff 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> @@ -309,15 +309,15 @@ Ip4Output (
>      // Route the packet unless overrided, that is, GateWay isn't zero.
>      //
>      if (IpInstance == NULL) {
> -      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> >Src);
> +      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head->Src,
> IpIf->SubnetMask);
>      } else {
> -      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head->Src);
> +      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head->Src,
> IpIf->SubnetMask);
>        //
>        // If failed to route the packet by using the instance's route table,
>        // try to use the default route table.
>        //
>        if (CacheEntry == NULL) {
> -        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> >Src);
> +        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> >Src, IpIf->SubnetMask);
>        }
>      }
> 
> @@ -386,7 +386,8 @@ Ip4Output (
>                   Fragment,
>                   GateWay,
>                   Ip4SysPacketSent,
> -                 Packet
> +                 Packet,
> +                 IpSb
>                   );
> 
>        if (EFI_ERROR (Status)) {
> @@ -429,7 +430,7 @@ Ip4Output (
>    //    upper layer's packets.
>    //
>    Ip4PrependHead (Packet, Head, Option, OptLen);
> -  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay, Callback,
> Context);
> +  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay, Callback,
> Context, IpSb);
> 
>    if (EFI_ERROR (Status)) {
>      goto ON_ERROR;
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> index d240d5343a..e648dee908 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> @@ -494,6 +494,8 @@ Ip4FindRouteEntry (
>    @param[in]  RtTable               The route table to search from
>    @param[in]  Dest                  The destination address to search for
>    @param[in]  Src                   The source address to search for
> +  @param[in]  SubnetMask            The subnet mask of the Src address, this
> field is
> +                                    used to check if the station is using /32 subnet.
> 
>    @return NULL if failed to route packet, otherwise a route cache
>            entry that can be used to route packet.
> @@ -503,7 +505,8 @@ IP4_ROUTE_CACHE_ENTRY *
>  Ip4Route (
>    IN IP4_ROUTE_TABLE        *RtTable,
>    IN IP4_ADDR               Dest,
> -  IN IP4_ADDR               Src
> +  IN IP4_ADDR               Src,
> +  IN IP4_ADDR               SubnetMask
>    )
>  {
>    LIST_ENTRY                *Head;
> @@ -534,7 +537,10 @@ Ip4Route (
>    //
>    RtEntry = Ip4FindRouteEntry (RtTable, Dest);
> 
> -  if (RtEntry == NULL) {
> +  //
> +  // We will always try to use the direct destination address when /32 subnet
> mask is used.
> +  //
> +  if (RtEntry == NULL && SubnetMask != IP4_ALLONE_ADDRESS) {
>      return NULL;
>    }
> 
> @@ -544,16 +550,23 @@ Ip4Route (
>    // network. Otherwise, it is an indirect route, the packet will be
>    // sent to the next hop router.
>    //
> -  if ((RtEntry->Flag & IP4_DIRECT_ROUTE) != 0) {
> +  // When using /32 subnet mask, the packet will always be sent to the
> direct
> +  // destination first, if we can't find a matching route cache.
> +  //
> +  if (SubnetMask == IP4_ALLONE_ADDRESS || ((RtEntry->Flag &
> IP4_DIRECT_ROUTE) != 0)) {
>      NextHop = Dest;
>    } else {
>      NextHop = RtEntry->NextHop;
>    }
> 
> -  Ip4FreeRouteEntry (RtEntry);
> +  if (RtEntry != NULL) {
> +    Ip4FreeRouteEntry (RtEntry);
> +  }
> 
>    //
>    // Create a route cache entry, and tag it as spawned from this route entry
> +  // For /32 subnet mask, the default route in RtEntry will be used if failed
> +  // to send the packet to driect destination address.
>    //
>    RtCacheEntry = Ip4CreateRouteCacheEntry (Dest, Src, NextHop, (UINTN)
> RtEntry);
> 
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> index 6269f4ceda..99b9cfbbac 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> @@ -194,6 +194,8 @@ Ip4FreeRouteCacheEntry (
>    @param[in]  RtTable               The route table to search from
>    @param[in]  Dest                  The destination address to search for
>    @param[in]  Src                   The source address to search for
> +  @param[in]  SubnetMask            The subnet mask of the Src address, this
> field is
> +                                    used to check if the station is using /32 subnet.
> 
>    @return NULL if failed to route packet, otherwise a route cache
>            entry that can be used to route packet.
> @@ -203,7 +205,8 @@ IP4_ROUTE_CACHE_ENTRY *
>  Ip4Route (
>    IN IP4_ROUTE_TABLE        *RtTable,
>    IN IP4_ADDR               Dest,
> -  IN IP4_ADDR               Src
> +  IN IP4_ADDR               Src,
> +  IN IP4_ADDR               SubnetMask
>    );
> 
>  /**
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> index d5a1a8c303..03903640b8 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> @@ -509,8 +509,9 @@ Mtftp4Start (
>      goto ON_ERROR;
>    }
> 
> +  gBS->RestoreTPL(OldTpl);
> +
>    if (Token->Event != NULL) {
> -    gBS->RestoreTPL (OldTpl);
>      return EFI_SUCCESS;
>    }
> 
> @@ -522,7 +523,6 @@ Mtftp4Start (
>      This->Poll (This);
>    }
> 
> -  gBS->RestoreTPL (OldTpl);
>    return Token->Status;
> 
>  ON_ERROR:
> @@ -682,7 +682,7 @@ EfiMtftp4Configure (
>      }
> 
>      if ((Gateway != 0) &&
> -        (!IP4_NET_EQUAL (Gateway, Ip, Netmask) || (Netmask != 0
> && !NetIp4IsUnicast (Gateway, Netmask)))) {
> +        ((Netmask != 0xFFFFFFFF && !IP4_NET_EQUAL (Gateway, Ip, Netmask))
> || (Netmask != 0 && !NetIp4IsUnicast (Gateway, Netmask)))) {
> 
>        return EFI_INVALID_PARAMETER;
>      }
> --
> 2.18.0.windows.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot.
  2018-08-28  6:50   ` Wu, Jiaxin
@ 2018-08-28  7:12     ` Fu, Siyuan
  2018-08-28  8:52       ` Wu, Jiaxin
  0 siblings, 1 reply; 12+ messages in thread
From: Fu, Siyuan @ 2018-08-28  7:12 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting

Hi, Jiaxin

The "default route" means a route entry with zero SubnetAddress and zero SubnetMask, which will match all the dest address that can't match any other non-zero Subnet* route entry. 
The "instance route table" is a list of route entries which belong to an IP child instance.
The "default route table" is the route table used by these IP child instances with default IP address.
A default route (which is just one route entry) may belong to the default route table, or any instance route table.

The new function Ip4SendFrameToDefaultRoute() will always try to send the frames to the default entry, so the naming is correct. While how to determine the default route entry is not in this function, the Ip4Route() has been updated to handle the /32 subnet mask specially, and Ip4Output() will guarantee we check the instance route table first, then default route table (the code you quoted).

BestRegards
Fu Siyuan

> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Tuesday, August 28, 2018 2:51 PM
> To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>
> Subject: RE: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> support for IP4 PXE boot.
> 
> Hi Siyuan,
> 
> In Ip4SendFrameToDefaultRoute(), you tried to find the default gateway IP
> address by using the found RtCacheEntry->Tag, I'm confused why it's the
> default gateway? In my understanding, tag is the cache's corresponding
> route entry.  Besides, why we must send the frame to "DefaultRouter",
> should be the instance router first, then default router? If so, I suggest
> to rename the function to Ip4SendFrameToRoute(). Then, the logic in the
> function to retrieve the gateway should be:
> {
>     if (IpInstance == NULL) {
>       CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head->Src,
> IpIf->SubnetMask);
>     } else {
>       CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head->Src,
> IpIf->SubnetMask);
>       if (CacheEntry == NULL) {
>         CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> >Src, IpIf->SubnetMask);
>       }
>     }
> 
>     if (CacheEntry == NULL) {
>       return EFI_NOT_FOUND;
>     }
>     GateWay = CacheEntry->NextHop;
>     Ip4FreeRouteCacheEntry (CacheEntry);
> }
> 
> What do you think?
> 
> Thanks,
> Jiaxin
> 
> 
> 
> 
> 
> 
> 
> 
> > -----Original Message-----
> > From: Fu, Siyuan
> > Sent: Tuesday, August 28, 2018 9:53 AM
> > To: edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > Subject: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> > support for IP4 PXE boot.
> >
> > This patch updates IP4 stack to support 32bit subnet mask in PXE boot
> > process.
> > When 32bit subnet mask is used, the IP4 driver couldn't use the subnet
> mask
> > to determine
> > whether destination IP address is on-link or not, so it will always try
> to send
> > all the
> > packets to the destination IP address directly first, if failed it will
> continue
> > to try the default gateway.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > ---
> >  MdeModulePkg/Include/Library/NetLib.h         |   5 +-
> >  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c    |  13 +-
> >  .../Universal/Network/Ip4Dxe/Ip4Common.h      |   2 +-
> >  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c | 117
> > ++++++++++++++++--
> >  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h |   5 +-
> >  .../Universal/Network/Ip4Dxe/Ip4Impl.c        |   2 +-
> >  .../Universal/Network/Ip4Dxe/Ip4Output.c      |  11 +-
> >  .../Universal/Network/Ip4Dxe/Ip4Route.c       |  21 +++-
> >  .../Universal/Network/Ip4Dxe/Ip4Route.h       |   5 +-
> >  .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c  |   6 +-
> >  10 files changed, 154 insertions(+), 33 deletions(-)
> >
> > diff --git a/MdeModulePkg/Include/Library/NetLib.h
> > b/MdeModulePkg/Include/Library/NetLib.h
> > index ef7bc429c1..b7ef99c7b5 100644
> > --- a/MdeModulePkg/Include/Library/NetLib.h
> > +++ b/MdeModulePkg/Include/Library/NetLib.h
> > @@ -422,8 +422,9 @@ NetGetIpClass (
> >
> >    If all bits of the host address of IP are 0 or 1, IP is also not a
> valid unicast
> > address,
> >    except when the originator is one of the endpoints of a point-to-
> point link
> > with a 31-bit
> > -  mask (RFC3021).
> > -
> > +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special
> network
> > environment (e.g.
> > +  PPP link).
> > +
> >    @param[in]  Ip                    The IP to check against.
> >    @param[in]  NetMask               The mask of the IP.
> >
> > diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > index bf8f5523e6..63f4724062 100644
> > --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > @@ -654,8 +654,9 @@ NetGetIpClass (
> >
> >    If all bits of the host address of IP are 0 or 1, IP is also not a
> valid unicast
> > address,
> >    except when the originator is one of the endpoints of a point-to-
> point link
> > with a 31-bit
> > -  mask (RFC3021).
> > -
> > +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special
> network
> > environment (e.g.
> > +  PPP link).
> > +
> >    @param[in]  Ip                    The IP to check against.
> >    @param[in]  NetMask               The mask of the IP.
> >
> > @@ -669,18 +670,20 @@ NetIp4IsUnicast (
> >    IN IP4_ADDR               NetMask
> >    )
> >  {
> > +  INTN   MaskLength;
> > +
> >    ASSERT (NetMask != 0);
> >
> >    if (Ip == 0 || IP4_IS_LOCAL_BROADCAST (Ip)) {
> >      return FALSE;
> >    }
> >
> > -  if (NetGetMaskLength (NetMask) != 31) {
> > +  MaskLength = NetGetMaskLength (NetMask);
> > +  ASSERT ((MaskLength >= 0) && (MaskLength <= IP4_MASK_NUM));
> > +  if (MaskLength < 31) {
> >      if (((Ip &~NetMask) == ~NetMask) || ((Ip &~NetMask) == 0)) {
> >        return FALSE;
> >      }
> > -  } else {
> > -    return TRUE;
> >    }
> >
> >    return TRUE;
> > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > index e0fffc9d0d..994a81f4de 100644
> > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > @@ -55,7 +55,7 @@ typedef struct _IP4_SERVICE    IP4_SERVICE;
> >  /// Compose the fragment field to be used in the IP4 header.
> >  ///
> >  #define IP4_HEAD_FRAGMENT_FIELD(Df, Mf, Offset) \
> > -    ((UINT16)(((Df) ? 0x4000 : 0) | ((Mf) ? 0x2000 : 0) | (((Offset) >>
> 3) &
> > 0x1fff)))
> > +    ((UINT16)(((Df) ? IP4_HEAD_DF_MASK : 0) | ((Mf) ?
> > IP4_HEAD_MF_MASK : 0) | (((Offset) >> 3) & IP4_HEAD_OFFSET_MASK)))
> >
> >  #define IP4_LAST_FRAGMENT(FragmentField)  \
> >            (((FragmentField) & IP4_HEAD_MF_MASK) == 0)
> > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > index 6e0e3290c7..b0172283b7 100644
> > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > @@ -138,6 +138,7 @@ Ip4CancelFrameArp (
> >    @param[in]  CallBack          Call back function to execute if
> transmission
> >                                  finished.
> >    @param[in]  Context           Opaque parameter to the call back.
> > +  @param[in]  IpSb              The pointer to the IP4 service binding
> instance.
> >
> >    @retval   Token               The wrapped token if succeed
> >    @retval   NULL                The wrapped token if NULL
> > @@ -149,7 +150,8 @@ Ip4WrapLinkTxToken (
> >    IN IP4_PROTOCOL           *IpInstance     OPTIONAL,
> >    IN NET_BUF                *Packet,
> >    IN IP4_FRAME_CALLBACK     CallBack,
> > -  IN VOID                   *Context
> > +  IN VOID                   *Context,
> > +  IN IP4_SERVICE            *IpSb
> >    )
> >  {
> >    EFI_MANAGED_NETWORK_COMPLETION_TOKEN  *MnpToken;
> > @@ -170,6 +172,7 @@ Ip4WrapLinkTxToken (
> >
> >    Token->Interface  = Interface;
> >    Token->IpInstance = IpInstance;
> > +  Token->IpSb       = IpSb;
> >    Token->CallBack   = CallBack;
> >    Token->Packet     = Packet;
> >    Token->Context    = Context;
> > @@ -792,6 +795,86 @@ Ip4FreeInterface (
> >    return EFI_SUCCESS;
> >  }
> >
> > +/**
> > +  This function tries to send all the queued frames in ArpQue to the
> default
> > gateway if
> > +  the ARP resolve for direct destination address is failed when using
> /32
> > subnet mask.
> > +
> > +  @param[in]   ArpQue           The ARP queue of a failed request.
> > +
> > +  @retval EFI_SUCCESS           All the queued frames have been send to
> the
> > default route.
> > +  @retval Others                Failed to send the queued frames.
> > +
> > +**/
> > +EFI_STATUS
> > +Ip4SendFrameToDefaultRoute (
> > +  IN  IP4_ARP_QUE               *ArpQue
> > +  )
> > +{
> > +  LIST_ENTRY                *Entry;
> > +  LIST_ENTRY                *Next;
> > +  IP4_ROUTE_CACHE_ENTRY     *RtCacheEntry;
> > +  IP4_LINK_TX_TOKEN         *Token;
> > +  IP4_ADDR                  Gateway;
> > +  EFI_STATUS                Status;
> > +  IP4_ROUTE_ENTRY           *DefaultRoute;
> > +
> > +  //
> > +  // ARP resolve failed when using /32 subnet mask.
> > +  //
> > +  NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> > +    RemoveEntryList (Entry);
> > +    Token = NET_LIST_USER_STRUCT (Entry, IP4_LINK_TX_TOKEN, Link);
> > +    ASSERT (Token->Interface->SubnetMask == IP4_ALLONE_ADDRESS);
> > +    //
> > +    // Find the default gateway IP address. The default route was saved
> to
> > the RtCacheEntry->Tag in Ip4Route().
> > +    //
> > +    RtCacheEntry = NULL;
> > +    if (Token->IpInstance != NULL) {
> > +      RtCacheEntry = Ip4FindRouteCache (Token->IpInstance->RouteTable,
> > NTOHL (ArpQue->Ip), Token->Interface->Ip);
> > +    }
> > +    if (RtCacheEntry == NULL) {
> > +      RtCacheEntry = Ip4FindRouteCache (Token->IpSb->DefaultRouteTable,
> > NTOHL (ArpQue->Ip), Token->Interface->Ip);
> > +    }
> > +    if (RtCacheEntry == NULL) {
> > +      Status= EFI_NO_MAPPING;
> > +      goto ON_ERROR;
> > +    }
> > +    DefaultRoute = (IP4_ROUTE_ENTRY*)RtCacheEntry->Tag;
> > +    if (DefaultRoute == NULL) {
> > +      Status= EFI_NO_MAPPING;
> > +      goto ON_ERROR;
> > +    }
> > +    //
> > +    // Try to send the frame to the default route.
> > +    //
> > +    Gateway = DefaultRoute->NextHop;
> > +    if (ArpQue->Ip == Gateway) {
> > +      //
> > +      // ARP resolve for the default route is failed, return error to
> caller.
> > +      //
> > +      Status= EFI_NO_MAPPING;
> > +      goto ON_ERROR;
> > +    }
> > +    RtCacheEntry->NextHop = Gateway;
> > +    Status = Ip4SendFrame (Token->Interface,Token->IpInstance,Token-
> > >Packet,Gateway,Token->CallBack,Token->Context,Token->IpSb);
> > +    if (EFI_ERROR (Status)) {
> > +      Status= EFI_NO_MAPPING;
> > +      goto ON_ERROR;
> > +    }
> > +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +
> > +ON_ERROR:
> > +  if (RtCacheEntry != NULL) {
> > +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> > +  }
> > +  Token->CallBack (Token->IpInstance, Token->Packet, Status, 0, Token-
> > >Context);
> > +  Ip4FreeLinkTxToken (Token);
> > +  return Status;
> > +}
> > +
> >
> >  /**
> >    Callback function when ARP request are finished. It will cancelled
> > @@ -814,6 +897,7 @@ Ip4OnArpResolvedDpc (
> >    IP4_INTERFACE             *Interface;
> >    IP4_LINK_TX_TOKEN         *Token;
> >    EFI_STATUS                Status;
> > +  EFI_STATUS                IoStatus;
> >
> >    ArpQue = (IP4_ARP_QUE *) Context;
> >    NET_CHECK_SIGNATURE (ArpQue, IP4_FRAME_ARP_SIGNATURE);
> > @@ -821,14 +905,23 @@ Ip4OnArpResolvedDpc (
> >    RemoveEntryList (&ArpQue->Link);
> >
> >    //
> > -  // ARP resolve failed for some reason. Release all the frame
> > -  // and ARP queue itself. Ip4FreeArpQue will call the frame's
> > -  // owner back.
> > +  // ARP resolve failed for some reason.
> >    //
> >    if (NET_MAC_EQUAL (&ArpQue->Mac, &mZeroMacAddress, ArpQue-
> > >Interface->HwaddrLen)) {
> > -    Ip4FreeArpQue (ArpQue, EFI_NO_MAPPING);
> > -
> > -    return ;
> > +    if (ArpQue->Interface->SubnetMask != IP4_ALLONE_ADDRESS) {
> > +      //
> > +      // Release all the frame and ARP queue itself. Ip4FreeArpQue will
> call the
> > frame's
> > +      // owner back.
> > +      //
> > +      IoStatus = EFI_NO_MAPPING;
> > +    } else {
> > +      //
> > +      // ARP resolve failed when using 32bit subnet mask, try to send
> the
> > packets to the
> > +      // default route.
> > +      //
> > +      IoStatus = Ip4SendFrameToDefaultRoute (ArpQue);
> > +    }
> > +    goto ON_EXIT;
> >    }
> >
> >    //
> > @@ -836,6 +929,7 @@ Ip4OnArpResolvedDpc (
> >    // queue. It isn't necessary for us to cache the ARP binding because
> >    // we always check the ARP cache first before transmit.
> >    //
> > +  IoStatus = EFI_SUCCESS;
> >    Interface = ArpQue->Interface;
> >
> >    NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> > @@ -863,7 +957,8 @@ Ip4OnArpResolvedDpc (
> >      }
> >    }
> >
> > -  Ip4FreeArpQue (ArpQue, EFI_SUCCESS);
> > +ON_EXIT:
> > +  Ip4FreeArpQue (ArpQue, IoStatus);
> >  }
> >
> >  /**
> > @@ -957,6 +1052,7 @@ Ip4OnFrameSent (
> >                                  to.
> >    @param[in]  CallBack          Function to call back when transmit
> finished.
> >    @param[in]  Context           Opaque parameter to the call back.
> > +  @param[in]  IpSb              The pointer to the IP4 service binding
> instance.
> >
> >    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send the
> > frame
> >    @retval EFI_NO_MAPPING        Can't resolve the MAC for the nexthop
> > @@ -971,7 +1067,8 @@ Ip4SendFrame (
> >    IN  NET_BUF               *Packet,
> >    IN  IP4_ADDR              NextHop,
> >    IN  IP4_FRAME_CALLBACK    CallBack,
> > -  IN  VOID                  *Context
> > +  IN  VOID                  *Context,
> > +  IN IP4_SERVICE            *IpSb
> >    )
> >  {
> >    IP4_LINK_TX_TOKEN         *Token;
> > @@ -982,7 +1079,7 @@ Ip4SendFrame (
> >
> >    ASSERT (Interface->Configured);
> >
> > -  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet, CallBack,
> > Context);
> > +  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet, CallBack,
> > Context, IpSb);
> >
> >    if (Token == NULL) {
> >      return EFI_OUT_OF_RESOURCES;
> > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > index 909837131e..36e4ab3f7a 100644
> > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > @@ -79,6 +79,7 @@ typedef struct {
> >    LIST_ENTRY                            Link;
> >
> >    IP4_INTERFACE                         *Interface;
> > +  IP4_SERVICE                           *IpSb;
> >
> >    IP4_PROTOCOL                          *IpInstance;
> >    IP4_FRAME_CALLBACK                    CallBack;
> > @@ -262,6 +263,7 @@ Ip4FreeInterface (
> >                                  to.
> >    @param[in]  CallBack          Function to call back when transmit
> finished.
> >    @param[in]  Context           Opaque parameter to the call back.
> > +  @param[in]  IpSb              The pointer to the IP4 service binding
> instance.
> >
> >    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send the
> > frame
> >    @retval EFI_NO_MAPPING        Can't resolve the MAC for the nexthop
> > @@ -276,7 +278,8 @@ Ip4SendFrame (
> >    IN  NET_BUF               *Packet,
> >    IN  IP4_ADDR              NextHop,
> >    IN  IP4_FRAME_CALLBACK    CallBack,
> > -  IN  VOID                  *Context
> > +  IN  VOID                  *Context,
> > +  IN IP4_SERVICE            *IpSb
> >    );
> >
> >  /**
> > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > index 6a26143e30..7c27db6753 100644
> > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > @@ -1259,7 +1259,7 @@ EfiIp4Routes (
> >    // the gateway address must be a unicast on the connected network if
> not
> > zero.
> >    //
> >    if ((Nexthop != IP4_ALLZERO_ADDRESS) &&
> > -      (!IP4_NET_EQUAL (Nexthop, IpIf->Ip, IpIf->SubnetMask) ||
> > +      ((IpIf->SubnetMask != IP4_ALLONE_ADDRESS && !IP4_NET_EQUAL
> > (Nexthop, IpIf->Ip, IpIf->SubnetMask)) ||
> >          IP4_IS_BROADCAST (Ip4GetNetCast (Nexthop, IpIf)))) {
> >
> >      Status = EFI_INVALID_PARAMETER;
> > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > index 1716f43576..bbbcf36eff 100644
> > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > @@ -309,15 +309,15 @@ Ip4Output (
> >      // Route the packet unless overrided, that is, GateWay isn't zero.
> >      //
> >      if (IpInstance == NULL) {
> > -      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> > >Src);
> > +      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> >Src,
> > IpIf->SubnetMask);
> >      } else {
> > -      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head-
> >Src);
> > +      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head-
> >Src,
> > IpIf->SubnetMask);
> >        //
> >        // If failed to route the packet by using the instance's route
> table,
> >        // try to use the default route table.
> >        //
> >        if (CacheEntry == NULL) {
> > -        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> Head-
> > >Src);
> > +        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> Head-
> > >Src, IpIf->SubnetMask);
> >        }
> >      }
> >
> > @@ -386,7 +386,8 @@ Ip4Output (
> >                   Fragment,
> >                   GateWay,
> >                   Ip4SysPacketSent,
> > -                 Packet
> > +                 Packet,
> > +                 IpSb
> >                   );
> >
> >        if (EFI_ERROR (Status)) {
> > @@ -429,7 +430,7 @@ Ip4Output (
> >    //    upper layer's packets.
> >    //
> >    Ip4PrependHead (Packet, Head, Option, OptLen);
> > -  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay, Callback,
> > Context);
> > +  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay, Callback,
> > Context, IpSb);
> >
> >    if (EFI_ERROR (Status)) {
> >      goto ON_ERROR;
> > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > index d240d5343a..e648dee908 100644
> > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > @@ -494,6 +494,8 @@ Ip4FindRouteEntry (
> >    @param[in]  RtTable               The route table to search from
> >    @param[in]  Dest                  The destination address to search
> for
> >    @param[in]  Src                   The source address to search for
> > +  @param[in]  SubnetMask            The subnet mask of the Src address,
> this
> > field is
> > +                                    used to check if the station is
> using /32 subnet.
> >
> >    @return NULL if failed to route packet, otherwise a route cache
> >            entry that can be used to route packet.
> > @@ -503,7 +505,8 @@ IP4_ROUTE_CACHE_ENTRY *
> >  Ip4Route (
> >    IN IP4_ROUTE_TABLE        *RtTable,
> >    IN IP4_ADDR               Dest,
> > -  IN IP4_ADDR               Src
> > +  IN IP4_ADDR               Src,
> > +  IN IP4_ADDR               SubnetMask
> >    )
> >  {
> >    LIST_ENTRY                *Head;
> > @@ -534,7 +537,10 @@ Ip4Route (
> >    //
> >    RtEntry = Ip4FindRouteEntry (RtTable, Dest);
> >
> > -  if (RtEntry == NULL) {
> > +  //
> > +  // We will always try to use the direct destination address when /32
> subnet
> > mask is used.
> > +  //
> > +  if (RtEntry == NULL && SubnetMask != IP4_ALLONE_ADDRESS) {
> >      return NULL;
> >    }
> >
> > @@ -544,16 +550,23 @@ Ip4Route (
> >    // network. Otherwise, it is an indirect route, the packet will be
> >    // sent to the next hop router.
> >    //
> > -  if ((RtEntry->Flag & IP4_DIRECT_ROUTE) != 0) {
> > +  // When using /32 subnet mask, the packet will always be sent to the
> > direct
> > +  // destination first, if we can't find a matching route cache.
> > +  //
> > +  if (SubnetMask == IP4_ALLONE_ADDRESS || ((RtEntry->Flag &
> > IP4_DIRECT_ROUTE) != 0)) {
> >      NextHop = Dest;
> >    } else {
> >      NextHop = RtEntry->NextHop;
> >    }
> >
> > -  Ip4FreeRouteEntry (RtEntry);
> > +  if (RtEntry != NULL) {
> > +    Ip4FreeRouteEntry (RtEntry);
> > +  }
> >
> >    //
> >    // Create a route cache entry, and tag it as spawned from this route
> entry
> > +  // For /32 subnet mask, the default route in RtEntry will be used if
> failed
> > +  // to send the packet to driect destination address.
> >    //
> >    RtCacheEntry = Ip4CreateRouteCacheEntry (Dest, Src, NextHop, (UINTN)
> > RtEntry);
> >
> > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > index 6269f4ceda..99b9cfbbac 100644
> > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > @@ -194,6 +194,8 @@ Ip4FreeRouteCacheEntry (
> >    @param[in]  RtTable               The route table to search from
> >    @param[in]  Dest                  The destination address to search
> for
> >    @param[in]  Src                   The source address to search for
> > +  @param[in]  SubnetMask            The subnet mask of the Src address,
> this
> > field is
> > +                                    used to check if the station is
> using /32 subnet.
> >
> >    @return NULL if failed to route packet, otherwise a route cache
> >            entry that can be used to route packet.
> > @@ -203,7 +205,8 @@ IP4_ROUTE_CACHE_ENTRY *
> >  Ip4Route (
> >    IN IP4_ROUTE_TABLE        *RtTable,
> >    IN IP4_ADDR               Dest,
> > -  IN IP4_ADDR               Src
> > +  IN IP4_ADDR               Src,
> > +  IN IP4_ADDR               SubnetMask
> >    );
> >
> >  /**
> > diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > index d5a1a8c303..03903640b8 100644
> > --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > @@ -509,8 +509,9 @@ Mtftp4Start (
> >      goto ON_ERROR;
> >    }
> >
> > +  gBS->RestoreTPL(OldTpl);
> > +
> >    if (Token->Event != NULL) {
> > -    gBS->RestoreTPL (OldTpl);
> >      return EFI_SUCCESS;
> >    }
> >
> > @@ -522,7 +523,6 @@ Mtftp4Start (
> >      This->Poll (This);
> >    }
> >
> > -  gBS->RestoreTPL (OldTpl);
> >    return Token->Status;
> >
> >  ON_ERROR:
> > @@ -682,7 +682,7 @@ EfiMtftp4Configure (
> >      }
> >
> >      if ((Gateway != 0) &&
> > -        (!IP4_NET_EQUAL (Gateway, Ip, Netmask) || (Netmask != 0
> > && !NetIp4IsUnicast (Gateway, Netmask)))) {
> > +        ((Netmask != 0xFFFFFFFF && !IP4_NET_EQUAL (Gateway, Ip,
> Netmask))
> > || (Netmask != 0 && !NetIp4IsUnicast (Gateway, Netmask)))) {
> >
> >        return EFI_INVALID_PARAMETER;
> >      }
> > --
> > 2.18.0.windows.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot.
  2018-08-28  7:12     ` Fu, Siyuan
@ 2018-08-28  8:52       ` Wu, Jiaxin
  2018-08-28  9:41         ` Fu, Siyuan
  0 siblings, 1 reply; 12+ messages in thread
From: Wu, Jiaxin @ 2018-08-28  8:52 UTC (permalink / raw)
  To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ye, Ting


Hi Siyuan,

Below is my code review understanding, maybe something is wrong, please correct me.

> The "default route" means a route entry with zero SubnetAddress and zero
> SubnetMask, which will match all the dest address that can't match any other
> non-zero Subnet* route entry.
> The "instance route table" is a list of route entries which belong to an IP child
> instance.
> The "default route table" is the route table used by these IP child instances
> with default IP address.
> A default route (which is just one route entry) may belong to the default
> route table, or any instance route table.
> 

Yes, I agree. If there is a default route {Dest: 0.0.0.0, Mask: 0.0.0.0, NextHope: Gataway} in the instance route table, it must be set via IP->Routes().


> The new function Ip4SendFrameToDefaultRoute() will always try to send the
> frames to the default entry, so the naming is correct. 

According above, we want to send the packet to the router (Gataway) via the default route {Dest: 0.0.0.0, Mask: 0.0.0.0, NextHope: Gataway} info, right? But the comments in Ip4Route() said:
  //
  // When using /32 subnet mask, the packet will always be sent to the direct
  // destination first, if we can't find a matching route cache.
  //

> While how to
> determine the default route entry is not in this function, the Ip4Route() has
> been updated to handle the /32 subnet mask specially, and Ip4Output() will
> guarantee we check the instance route table first, then default route table
> (the code you quoted).

After the code I quoted below in Ip4Output(), the route cache entry found in Ip4SendFrameToDefaultRoute() is {Dest: X.X.X.X, Src: Y.Y.Y.Y, NextHope:  X.X.X.X, Tag: { Dest: X.X.X.X, Mask: 255.255.255.255, NextHope:  0.0.0.0} }, if so, looks the package will be sent to  NextHope:  0.0.0.0?


> 
> BestRegards
> Fu Siyuan
> 
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Tuesday, August 28, 2018 2:51 PM
> > To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>
> > Subject: RE: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> > support for IP4 PXE boot.
> >
> > Hi Siyuan,
> >
> > In Ip4SendFrameToDefaultRoute(), you tried to find the default gateway IP
> > address by using the found RtCacheEntry->Tag, I'm confused why it's the
> > default gateway? In my understanding, tag is the cache's corresponding
> > route entry.  Besides, why we must send the frame to "DefaultRouter",
> > should be the instance router first, then default router? If so, I suggest
> > to rename the function to Ip4SendFrameToRoute(). Then, the logic in the
> > function to retrieve the gateway should be:
> > {
> >     if (IpInstance == NULL) {
> >       CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> >Src,
> > IpIf->SubnetMask);
> >     } else {
> >       CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head->Src,
> > IpIf->SubnetMask);
> >       if (CacheEntry == NULL) {
> >         CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> > >Src, IpIf->SubnetMask);
> >       }
> >     }
> >
> >     if (CacheEntry == NULL) {
> >       return EFI_NOT_FOUND;
> >     }
> >     GateWay = CacheEntry->NextHop;
> >     Ip4FreeRouteCacheEntry (CacheEntry);
> > }
> >
> > What do you think?
> >
> > Thanks,
> > Jiaxin
> >
> >
> >
> >
> >
> >
> >
> >
> > > -----Original Message-----
> > > From: Fu, Siyuan
> > > Sent: Tuesday, August 28, 2018 9:53 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > > Subject: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> > > support for IP4 PXE boot.
> > >
> > > This patch updates IP4 stack to support 32bit subnet mask in PXE boot
> > > process.
> > > When 32bit subnet mask is used, the IP4 driver couldn't use the subnet
> > mask
> > > to determine
> > > whether destination IP address is on-link or not, so it will always try
> > to send
> > > all the
> > > packets to the destination IP address directly first, if failed it will
> > continue
> > > to try the default gateway.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > > ---
> > >  MdeModulePkg/Include/Library/NetLib.h         |   5 +-
> > >  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c    |  13 +-
> > >  .../Universal/Network/Ip4Dxe/Ip4Common.h      |   2 +-
> > >  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c | 117
> > > ++++++++++++++++--
> > >  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h |   5 +-
> > >  .../Universal/Network/Ip4Dxe/Ip4Impl.c        |   2 +-
> > >  .../Universal/Network/Ip4Dxe/Ip4Output.c      |  11 +-
> > >  .../Universal/Network/Ip4Dxe/Ip4Route.c       |  21 +++-
> > >  .../Universal/Network/Ip4Dxe/Ip4Route.h       |   5 +-
> > >  .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c  |   6 +-
> > >  10 files changed, 154 insertions(+), 33 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Include/Library/NetLib.h
> > > b/MdeModulePkg/Include/Library/NetLib.h
> > > index ef7bc429c1..b7ef99c7b5 100644
> > > --- a/MdeModulePkg/Include/Library/NetLib.h
> > > +++ b/MdeModulePkg/Include/Library/NetLib.h
> > > @@ -422,8 +422,9 @@ NetGetIpClass (
> > >
> > >    If all bits of the host address of IP are 0 or 1, IP is also not a
> > valid unicast
> > > address,
> > >    except when the originator is one of the endpoints of a point-to-
> > point link
> > > with a 31-bit
> > > -  mask (RFC3021).
> > > -
> > > +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special
> > network
> > > environment (e.g.
> > > +  PPP link).
> > > +
> > >    @param[in]  Ip                    The IP to check against.
> > >    @param[in]  NetMask               The mask of the IP.
> > >
> > > diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > index bf8f5523e6..63f4724062 100644
> > > --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > @@ -654,8 +654,9 @@ NetGetIpClass (
> > >
> > >    If all bits of the host address of IP are 0 or 1, IP is also not a
> > valid unicast
> > > address,
> > >    except when the originator is one of the endpoints of a point-to-
> > point link
> > > with a 31-bit
> > > -  mask (RFC3021).
> > > -
> > > +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special
> > network
> > > environment (e.g.
> > > +  PPP link).
> > > +
> > >    @param[in]  Ip                    The IP to check against.
> > >    @param[in]  NetMask               The mask of the IP.
> > >
> > > @@ -669,18 +670,20 @@ NetIp4IsUnicast (
> > >    IN IP4_ADDR               NetMask
> > >    )
> > >  {
> > > +  INTN   MaskLength;
> > > +
> > >    ASSERT (NetMask != 0);
> > >
> > >    if (Ip == 0 || IP4_IS_LOCAL_BROADCAST (Ip)) {
> > >      return FALSE;
> > >    }
> > >
> > > -  if (NetGetMaskLength (NetMask) != 31) {
> > > +  MaskLength = NetGetMaskLength (NetMask);
> > > +  ASSERT ((MaskLength >= 0) && (MaskLength <= IP4_MASK_NUM));
> > > +  if (MaskLength < 31) {
> > >      if (((Ip &~NetMask) == ~NetMask) || ((Ip &~NetMask) == 0)) {
> > >        return FALSE;
> > >      }
> > > -  } else {
> > > -    return TRUE;
> > >    }
> > >
> > >    return TRUE;
> > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > index e0fffc9d0d..994a81f4de 100644
> > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > @@ -55,7 +55,7 @@ typedef struct _IP4_SERVICE    IP4_SERVICE;
> > >  /// Compose the fragment field to be used in the IP4 header.
> > >  ///
> > >  #define IP4_HEAD_FRAGMENT_FIELD(Df, Mf, Offset) \
> > > -    ((UINT16)(((Df) ? 0x4000 : 0) | ((Mf) ? 0x2000 : 0) | (((Offset) >>
> > 3) &
> > > 0x1fff)))
> > > +    ((UINT16)(((Df) ? IP4_HEAD_DF_MASK : 0) | ((Mf) ?
> > > IP4_HEAD_MF_MASK : 0) | (((Offset) >> 3) & IP4_HEAD_OFFSET_MASK)))
> > >
> > >  #define IP4_LAST_FRAGMENT(FragmentField)  \
> > >            (((FragmentField) & IP4_HEAD_MF_MASK) == 0)
> > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > index 6e0e3290c7..b0172283b7 100644
> > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > @@ -138,6 +138,7 @@ Ip4CancelFrameArp (
> > >    @param[in]  CallBack          Call back function to execute if
> > transmission
> > >                                  finished.
> > >    @param[in]  Context           Opaque parameter to the call back.
> > > +  @param[in]  IpSb              The pointer to the IP4 service binding
> > instance.
> > >
> > >    @retval   Token               The wrapped token if succeed
> > >    @retval   NULL                The wrapped token if NULL
> > > @@ -149,7 +150,8 @@ Ip4WrapLinkTxToken (
> > >    IN IP4_PROTOCOL           *IpInstance     OPTIONAL,
> > >    IN NET_BUF                *Packet,
> > >    IN IP4_FRAME_CALLBACK     CallBack,
> > > -  IN VOID                   *Context
> > > +  IN VOID                   *Context,
> > > +  IN IP4_SERVICE            *IpSb
> > >    )
> > >  {
> > >    EFI_MANAGED_NETWORK_COMPLETION_TOKEN  *MnpToken;
> > > @@ -170,6 +172,7 @@ Ip4WrapLinkTxToken (
> > >
> > >    Token->Interface  = Interface;
> > >    Token->IpInstance = IpInstance;
> > > +  Token->IpSb       = IpSb;
> > >    Token->CallBack   = CallBack;
> > >    Token->Packet     = Packet;
> > >    Token->Context    = Context;
> > > @@ -792,6 +795,86 @@ Ip4FreeInterface (
> > >    return EFI_SUCCESS;
> > >  }
> > >
> > > +/**
> > > +  This function tries to send all the queued frames in ArpQue to the
> > default
> > > gateway if
> > > +  the ARP resolve for direct destination address is failed when using
> > /32
> > > subnet mask.
> > > +
> > > +  @param[in]   ArpQue           The ARP queue of a failed request.
> > > +
> > > +  @retval EFI_SUCCESS           All the queued frames have been send to
> > the
> > > default route.
> > > +  @retval Others                Failed to send the queued frames.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +Ip4SendFrameToDefaultRoute (
> > > +  IN  IP4_ARP_QUE               *ArpQue
> > > +  )
> > > +{
> > > +  LIST_ENTRY                *Entry;
> > > +  LIST_ENTRY                *Next;
> > > +  IP4_ROUTE_CACHE_ENTRY     *RtCacheEntry;
> > > +  IP4_LINK_TX_TOKEN         *Token;
> > > +  IP4_ADDR                  Gateway;
> > > +  EFI_STATUS                Status;
> > > +  IP4_ROUTE_ENTRY           *DefaultRoute;
> > > +
> > > +  //
> > > +  // ARP resolve failed when using /32 subnet mask.
> > > +  //
> > > +  NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> > > +    RemoveEntryList (Entry);
> > > +    Token = NET_LIST_USER_STRUCT (Entry, IP4_LINK_TX_TOKEN, Link);
> > > +    ASSERT (Token->Interface->SubnetMask == IP4_ALLONE_ADDRESS);
> > > +    //
> > > +    // Find the default gateway IP address. The default route was saved
> > to
> > > the RtCacheEntry->Tag in Ip4Route().
> > > +    //
> > > +    RtCacheEntry = NULL;
> > > +    if (Token->IpInstance != NULL) {
> > > +      RtCacheEntry = Ip4FindRouteCache (Token->IpInstance->RouteTable,
> > > NTOHL (ArpQue->Ip), Token->Interface->Ip);
> > > +    }
> > > +    if (RtCacheEntry == NULL) {
> > > +      RtCacheEntry = Ip4FindRouteCache (Token->IpSb-
> >DefaultRouteTable,
> > > NTOHL (ArpQue->Ip), Token->Interface->Ip);
> > > +    }
> > > +    if (RtCacheEntry == NULL) {
> > > +      Status= EFI_NO_MAPPING;
> > > +      goto ON_ERROR;
> > > +    }
> > > +    DefaultRoute = (IP4_ROUTE_ENTRY*)RtCacheEntry->Tag;
> > > +    if (DefaultRoute == NULL) {
> > > +      Status= EFI_NO_MAPPING;
> > > +      goto ON_ERROR;
> > > +    }
> > > +    //
> > > +    // Try to send the frame to the default route.
> > > +    //
> > > +    Gateway = DefaultRoute->NextHop;
> > > +    if (ArpQue->Ip == Gateway) {
> > > +      //
> > > +      // ARP resolve for the default route is failed, return error to
> > caller.
> > > +      //
> > > +      Status= EFI_NO_MAPPING;
> > > +      goto ON_ERROR;
> > > +    }
> > > +    RtCacheEntry->NextHop = Gateway;
> > > +    Status = Ip4SendFrame (Token->Interface,Token->IpInstance,Token-
> > > >Packet,Gateway,Token->CallBack,Token->Context,Token->IpSb);
> > > +    if (EFI_ERROR (Status)) {
> > > +      Status= EFI_NO_MAPPING;
> > > +      goto ON_ERROR;
> > > +    }
> > > +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> > > +  }
> > > +
> > > +  return EFI_SUCCESS;
> > > +
> > > +ON_ERROR:
> > > +  if (RtCacheEntry != NULL) {
> > > +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> > > +  }
> > > +  Token->CallBack (Token->IpInstance, Token->Packet, Status, 0, Token-
> > > >Context);
> > > +  Ip4FreeLinkTxToken (Token);
> > > +  return Status;
> > > +}
> > > +
> > >
> > >  /**
> > >    Callback function when ARP request are finished. It will cancelled
> > > @@ -814,6 +897,7 @@ Ip4OnArpResolvedDpc (
> > >    IP4_INTERFACE             *Interface;
> > >    IP4_LINK_TX_TOKEN         *Token;
> > >    EFI_STATUS                Status;
> > > +  EFI_STATUS                IoStatus;
> > >
> > >    ArpQue = (IP4_ARP_QUE *) Context;
> > >    NET_CHECK_SIGNATURE (ArpQue, IP4_FRAME_ARP_SIGNATURE);
> > > @@ -821,14 +905,23 @@ Ip4OnArpResolvedDpc (
> > >    RemoveEntryList (&ArpQue->Link);
> > >
> > >    //
> > > -  // ARP resolve failed for some reason. Release all the frame
> > > -  // and ARP queue itself. Ip4FreeArpQue will call the frame's
> > > -  // owner back.
> > > +  // ARP resolve failed for some reason.
> > >    //
> > >    if (NET_MAC_EQUAL (&ArpQue->Mac, &mZeroMacAddress, ArpQue-
> > > >Interface->HwaddrLen)) {
> > > -    Ip4FreeArpQue (ArpQue, EFI_NO_MAPPING);
> > > -
> > > -    return ;
> > > +    if (ArpQue->Interface->SubnetMask != IP4_ALLONE_ADDRESS) {
> > > +      //
> > > +      // Release all the frame and ARP queue itself. Ip4FreeArpQue will
> > call the
> > > frame's
> > > +      // owner back.
> > > +      //
> > > +      IoStatus = EFI_NO_MAPPING;
> > > +    } else {
> > > +      //
> > > +      // ARP resolve failed when using 32bit subnet mask, try to send
> > the
> > > packets to the
> > > +      // default route.
> > > +      //
> > > +      IoStatus = Ip4SendFrameToDefaultRoute (ArpQue);
> > > +    }
> > > +    goto ON_EXIT;
> > >    }
> > >
> > >    //
> > > @@ -836,6 +929,7 @@ Ip4OnArpResolvedDpc (
> > >    // queue. It isn't necessary for us to cache the ARP binding because
> > >    // we always check the ARP cache first before transmit.
> > >    //
> > > +  IoStatus = EFI_SUCCESS;
> > >    Interface = ArpQue->Interface;
> > >
> > >    NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> > > @@ -863,7 +957,8 @@ Ip4OnArpResolvedDpc (
> > >      }
> > >    }
> > >
> > > -  Ip4FreeArpQue (ArpQue, EFI_SUCCESS);
> > > +ON_EXIT:
> > > +  Ip4FreeArpQue (ArpQue, IoStatus);
> > >  }
> > >
> > >  /**
> > > @@ -957,6 +1052,7 @@ Ip4OnFrameSent (
> > >                                  to.
> > >    @param[in]  CallBack          Function to call back when transmit
> > finished.
> > >    @param[in]  Context           Opaque parameter to the call back.
> > > +  @param[in]  IpSb              The pointer to the IP4 service binding
> > instance.
> > >
> > >    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send
> the
> > > frame
> > >    @retval EFI_NO_MAPPING        Can't resolve the MAC for the nexthop
> > > @@ -971,7 +1067,8 @@ Ip4SendFrame (
> > >    IN  NET_BUF               *Packet,
> > >    IN  IP4_ADDR              NextHop,
> > >    IN  IP4_FRAME_CALLBACK    CallBack,
> > > -  IN  VOID                  *Context
> > > +  IN  VOID                  *Context,
> > > +  IN IP4_SERVICE            *IpSb
> > >    )
> > >  {
> > >    IP4_LINK_TX_TOKEN         *Token;
> > > @@ -982,7 +1079,7 @@ Ip4SendFrame (
> > >
> > >    ASSERT (Interface->Configured);
> > >
> > > -  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet, CallBack,
> > > Context);
> > > +  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet, CallBack,
> > > Context, IpSb);
> > >
> > >    if (Token == NULL) {
> > >      return EFI_OUT_OF_RESOURCES;
> > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > index 909837131e..36e4ab3f7a 100644
> > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > @@ -79,6 +79,7 @@ typedef struct {
> > >    LIST_ENTRY                            Link;
> > >
> > >    IP4_INTERFACE                         *Interface;
> > > +  IP4_SERVICE                           *IpSb;
> > >
> > >    IP4_PROTOCOL                          *IpInstance;
> > >    IP4_FRAME_CALLBACK                    CallBack;
> > > @@ -262,6 +263,7 @@ Ip4FreeInterface (
> > >                                  to.
> > >    @param[in]  CallBack          Function to call back when transmit
> > finished.
> > >    @param[in]  Context           Opaque parameter to the call back.
> > > +  @param[in]  IpSb              The pointer to the IP4 service binding
> > instance.
> > >
> > >    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send
> the
> > > frame
> > >    @retval EFI_NO_MAPPING        Can't resolve the MAC for the nexthop
> > > @@ -276,7 +278,8 @@ Ip4SendFrame (
> > >    IN  NET_BUF               *Packet,
> > >    IN  IP4_ADDR              NextHop,
> > >    IN  IP4_FRAME_CALLBACK    CallBack,
> > > -  IN  VOID                  *Context
> > > +  IN  VOID                  *Context,
> > > +  IN IP4_SERVICE            *IpSb
> > >    );
> > >
> > >  /**
> > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > index 6a26143e30..7c27db6753 100644
> > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > @@ -1259,7 +1259,7 @@ EfiIp4Routes (
> > >    // the gateway address must be a unicast on the connected network if
> > not
> > > zero.
> > >    //
> > >    if ((Nexthop != IP4_ALLZERO_ADDRESS) &&
> > > -      (!IP4_NET_EQUAL (Nexthop, IpIf->Ip, IpIf->SubnetMask) ||
> > > +      ((IpIf->SubnetMask != IP4_ALLONE_ADDRESS && !IP4_NET_EQUAL
> > > (Nexthop, IpIf->Ip, IpIf->SubnetMask)) ||
> > >          IP4_IS_BROADCAST (Ip4GetNetCast (Nexthop, IpIf)))) {
> > >
> > >      Status = EFI_INVALID_PARAMETER;
> > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > index 1716f43576..bbbcf36eff 100644
> > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > @@ -309,15 +309,15 @@ Ip4Output (
> > >      // Route the packet unless overrided, that is, GateWay isn't zero.
> > >      //
> > >      if (IpInstance == NULL) {
> > > -      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> > > >Src);
> > > +      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> > >Src,
> > > IpIf->SubnetMask);
> > >      } else {
> > > -      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head-
> > >Src);
> > > +      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head-
> > >Src,
> > > IpIf->SubnetMask);
> > >        //
> > >        // If failed to route the packet by using the instance's route
> > table,
> > >        // try to use the default route table.
> > >        //
> > >        if (CacheEntry == NULL) {
> > > -        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> > Head-
> > > >Src);
> > > +        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> > Head-
> > > >Src, IpIf->SubnetMask);
> > >        }
> > >      }
> > >
> > > @@ -386,7 +386,8 @@ Ip4Output (
> > >                   Fragment,
> > >                   GateWay,
> > >                   Ip4SysPacketSent,
> > > -                 Packet
> > > +                 Packet,
> > > +                 IpSb
> > >                   );
> > >
> > >        if (EFI_ERROR (Status)) {
> > > @@ -429,7 +430,7 @@ Ip4Output (
> > >    //    upper layer's packets.
> > >    //
> > >    Ip4PrependHead (Packet, Head, Option, OptLen);
> > > -  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay, Callback,
> > > Context);
> > > +  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay, Callback,
> > > Context, IpSb);
> > >
> > >    if (EFI_ERROR (Status)) {
> > >      goto ON_ERROR;
> > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > index d240d5343a..e648dee908 100644
> > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > @@ -494,6 +494,8 @@ Ip4FindRouteEntry (
> > >    @param[in]  RtTable               The route table to search from
> > >    @param[in]  Dest                  The destination address to search
> > for
> > >    @param[in]  Src                   The source address to search for
> > > +  @param[in]  SubnetMask            The subnet mask of the Src address,
> > this
> > > field is
> > > +                                    used to check if the station is
> > using /32 subnet.
> > >
> > >    @return NULL if failed to route packet, otherwise a route cache
> > >            entry that can be used to route packet.
> > > @@ -503,7 +505,8 @@ IP4_ROUTE_CACHE_ENTRY *
> > >  Ip4Route (
> > >    IN IP4_ROUTE_TABLE        *RtTable,
> > >    IN IP4_ADDR               Dest,
> > > -  IN IP4_ADDR               Src
> > > +  IN IP4_ADDR               Src,
> > > +  IN IP4_ADDR               SubnetMask
> > >    )
> > >  {
> > >    LIST_ENTRY                *Head;
> > > @@ -534,7 +537,10 @@ Ip4Route (
> > >    //
> > >    RtEntry = Ip4FindRouteEntry (RtTable, Dest);
> > >
> > > -  if (RtEntry == NULL) {
> > > +  //
> > > +  // We will always try to use the direct destination address when /32
> > subnet
> > > mask is used.
> > > +  //
> > > +  if (RtEntry == NULL && SubnetMask != IP4_ALLONE_ADDRESS) {
> > >      return NULL;
> > >    }
> > >
> > > @@ -544,16 +550,23 @@ Ip4Route (
> > >    // network. Otherwise, it is an indirect route, the packet will be
> > >    // sent to the next hop router.
> > >    //
> > > -  if ((RtEntry->Flag & IP4_DIRECT_ROUTE) != 0) {
> > > +  // When using /32 subnet mask, the packet will always be sent to the
> > > direct
> > > +  // destination first, if we can't find a matching route cache.
> > > +  //
> > > +  if (SubnetMask == IP4_ALLONE_ADDRESS || ((RtEntry->Flag &
> > > IP4_DIRECT_ROUTE) != 0)) {
> > >      NextHop = Dest;
> > >    } else {
> > >      NextHop = RtEntry->NextHop;
> > >    }
> > >
> > > -  Ip4FreeRouteEntry (RtEntry);
> > > +  if (RtEntry != NULL) {
> > > +    Ip4FreeRouteEntry (RtEntry);
> > > +  }
> > >
> > >    //
> > >    // Create a route cache entry, and tag it as spawned from this route
> > entry
> > > +  // For /32 subnet mask, the default route in RtEntry will be used if
> > failed
> > > +  // to send the packet to driect destination address.
> > >    //
> > >    RtCacheEntry = Ip4CreateRouteCacheEntry (Dest, Src, NextHop, (UINTN)
> > > RtEntry);
> > >
> > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > index 6269f4ceda..99b9cfbbac 100644
> > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > @@ -194,6 +194,8 @@ Ip4FreeRouteCacheEntry (
> > >    @param[in]  RtTable               The route table to search from
> > >    @param[in]  Dest                  The destination address to search
> > for
> > >    @param[in]  Src                   The source address to search for
> > > +  @param[in]  SubnetMask            The subnet mask of the Src address,
> > this
> > > field is
> > > +                                    used to check if the station is
> > using /32 subnet.
> > >
> > >    @return NULL if failed to route packet, otherwise a route cache
> > >            entry that can be used to route packet.
> > > @@ -203,7 +205,8 @@ IP4_ROUTE_CACHE_ENTRY *
> > >  Ip4Route (
> > >    IN IP4_ROUTE_TABLE        *RtTable,
> > >    IN IP4_ADDR               Dest,
> > > -  IN IP4_ADDR               Src
> > > +  IN IP4_ADDR               Src,
> > > +  IN IP4_ADDR               SubnetMask
> > >    );
> > >
> > >  /**
> > > diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > index d5a1a8c303..03903640b8 100644
> > > --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > @@ -509,8 +509,9 @@ Mtftp4Start (
> > >      goto ON_ERROR;
> > >    }
> > >
> > > +  gBS->RestoreTPL(OldTpl);
> > > +
> > >    if (Token->Event != NULL) {
> > > -    gBS->RestoreTPL (OldTpl);
> > >      return EFI_SUCCESS;
> > >    }
> > >
> > > @@ -522,7 +523,6 @@ Mtftp4Start (
> > >      This->Poll (This);
> > >    }
> > >
> > > -  gBS->RestoreTPL (OldTpl);
> > >    return Token->Status;
> > >
> > >  ON_ERROR:
> > > @@ -682,7 +682,7 @@ EfiMtftp4Configure (
> > >      }
> > >
> > >      if ((Gateway != 0) &&
> > > -        (!IP4_NET_EQUAL (Gateway, Ip, Netmask) || (Netmask != 0
> > > && !NetIp4IsUnicast (Gateway, Netmask)))) {
> > > +        ((Netmask != 0xFFFFFFFF && !IP4_NET_EQUAL (Gateway, Ip,
> > Netmask))
> > > || (Netmask != 0 && !NetIp4IsUnicast (Gateway, Netmask)))) {
> > >
> > >        return EFI_INVALID_PARAMETER;
> > >      }
> > > --
> > > 2.18.0.windows.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot.
  2018-08-28  8:52       ` Wu, Jiaxin
@ 2018-08-28  9:41         ` Fu, Siyuan
  2018-08-29  2:09           ` Wu, Jiaxin
  0 siblings, 1 reply; 12+ messages in thread
From: Fu, Siyuan @ 2018-08-28  9:41 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting

Hi, Jiaxin

The IP4 routing logic for /32 subnet mask can be explain as below, maybe I need to add it to the commit log or in code comments.

Ip4Output() is called to send a packet
	If a matching route cache (src, dest -> xxx) can be find, 
		send the packet to address xxx according to the route cache.
	else
		create a new route cache (src, dest -> dest)                               <- This is done in Ip4Route()
		if a default gateway is configured, save it to the route cache TAG.        <- This is done in Ip4Route()
		send the packet to dest                                                    <- This is done in Ip4SendFrame()
		if ARP resolve failed for dest
			find the matching route cache and check TAG to see if there is a default gateway          <- This is done in Ip4SendFrameToDefaultRoute ()
			if yes
				update the route cache to (src, dest -> default gw)
				send the packet to the default gw
	

BestRegards
Fu Siyuan


> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Tuesday, August 28, 2018 4:53 PM
> To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>
> Subject: RE: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> support for IP4 PXE boot.
> 
> 
> Hi Siyuan,
> 
> Below is my code review understanding, maybe something is wrong, please
> correct me.
> 
> > The "default route" means a route entry with zero SubnetAddress and zero
> > SubnetMask, which will match all the dest address that can't match any
> other
> > non-zero Subnet* route entry.
> > The "instance route table" is a list of route entries which belong to an
> IP child
> > instance.
> > The "default route table" is the route table used by these IP child
> instances
> > with default IP address.
> > A default route (which is just one route entry) may belong to the
> default
> > route table, or any instance route table.
> >
> 
> Yes, I agree. If there is a default route {Dest: 0.0.0.0, Mask: 0.0.0.0,
> NextHope: Gataway} in the instance route table, it must be set via IP-
> >Routes().
> 
> 
> > The new function Ip4SendFrameToDefaultRoute() will always try to send
> the
> > frames to the default entry, so the naming is correct.
> 
> According above, we want to send the packet to the router (Gataway) via
> the default route {Dest: 0.0.0.0, Mask: 0.0.0.0, NextHope: Gataway} info,
> right? But the comments in Ip4Route() said:
>   //
>   // When using /32 subnet mask, the packet will always be sent to the
> direct
>   // destination first, if we can't find a matching route cache.
>   //
> 
> > While how to
> > determine the default route entry is not in this function, the Ip4Route()
> has
> > been updated to handle the /32 subnet mask specially, and Ip4Output()
> will
> > guarantee we check the instance route table first, then default route
> table
> > (the code you quoted).
> 
> After the code I quoted below in Ip4Output(), the route cache entry found
> in Ip4SendFrameToDefaultRoute() is {Dest: X.X.X.X, Src: Y.Y.Y.Y, NextHope:
> X.X.X.X, Tag: { Dest: X.X.X.X, Mask: 255.255.255.255, NextHope:
> 0.0.0.0} }, if so, looks the package will be sent to  NextHope:  0.0.0.0?
> 
> 
> >
> > BestRegards
> > Fu Siyuan
> >
> > > -----Original Message-----
> > > From: Wu, Jiaxin
> > > Sent: Tuesday, August 28, 2018 2:51 PM
> > > To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>
> > > Subject: RE: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> > > support for IP4 PXE boot.
> > >
> > > Hi Siyuan,
> > >
> > > In Ip4SendFrameToDefaultRoute(), you tried to find the default gateway
> IP
> > > address by using the found RtCacheEntry->Tag, I'm confused why it's
> the
> > > default gateway? In my understanding, tag is the cache's corresponding
> > > route entry.  Besides, why we must send the frame to "DefaultRouter",
> > > should be the instance router first, then default router? If so, I
> suggest
> > > to rename the function to Ip4SendFrameToRoute(). Then, the logic in
> the
> > > function to retrieve the gateway should be:
> > > {
> > >     if (IpInstance == NULL) {
> > >       CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> > >Src,
> > > IpIf->SubnetMask);
> > >     } else {
> > >       CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head-
> >Src,
> > > IpIf->SubnetMask);
> > >       if (CacheEntry == NULL) {
> > >         CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> Head-
> > > >Src, IpIf->SubnetMask);
> > >       }
> > >     }
> > >
> > >     if (CacheEntry == NULL) {
> > >       return EFI_NOT_FOUND;
> > >     }
> > >     GateWay = CacheEntry->NextHop;
> > >     Ip4FreeRouteCacheEntry (CacheEntry);
> > > }
> > >
> > > What do you think?
> > >
> > > Thanks,
> > > Jiaxin
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Fu, Siyuan
> > > > Sent: Tuesday, August 28, 2018 9:53 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > > > Subject: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> > > > support for IP4 PXE boot.
> > > >
> > > > This patch updates IP4 stack to support 32bit subnet mask in PXE
> boot
> > > > process.
> > > > When 32bit subnet mask is used, the IP4 driver couldn't use the
> subnet
> > > mask
> > > > to determine
> > > > whether destination IP address is on-link or not, so it will always
> try
> > > to send
> > > > all the
> > > > packets to the destination IP address directly first, if failed it
> will
> > > continue
> > > > to try the default gateway.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > > > ---
> > > >  MdeModulePkg/Include/Library/NetLib.h         |   5 +-
> > > >  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c    |  13 +-
> > > >  .../Universal/Network/Ip4Dxe/Ip4Common.h      |   2 +-
> > > >  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c | 117
> > > > ++++++++++++++++--
> > > >  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h |   5 +-
> > > >  .../Universal/Network/Ip4Dxe/Ip4Impl.c        |   2 +-
> > > >  .../Universal/Network/Ip4Dxe/Ip4Output.c      |  11 +-
> > > >  .../Universal/Network/Ip4Dxe/Ip4Route.c       |  21 +++-
> > > >  .../Universal/Network/Ip4Dxe/Ip4Route.h       |   5 +-
> > > >  .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c  |   6 +-
> > > >  10 files changed, 154 insertions(+), 33 deletions(-)
> > > >
> > > > diff --git a/MdeModulePkg/Include/Library/NetLib.h
> > > > b/MdeModulePkg/Include/Library/NetLib.h
> > > > index ef7bc429c1..b7ef99c7b5 100644
> > > > --- a/MdeModulePkg/Include/Library/NetLib.h
> > > > +++ b/MdeModulePkg/Include/Library/NetLib.h
> > > > @@ -422,8 +422,9 @@ NetGetIpClass (
> > > >
> > > >    If all bits of the host address of IP are 0 or 1, IP is also not
> a
> > > valid unicast
> > > > address,
> > > >    except when the originator is one of the endpoints of a point-to-
> > > point link
> > > > with a 31-bit
> > > > -  mask (RFC3021).
> > > > -
> > > > +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special
> > > network
> > > > environment (e.g.
> > > > +  PPP link).
> > > > +
> > > >    @param[in]  Ip                    The IP to check against.
> > > >    @param[in]  NetMask               The mask of the IP.
> > > >
> > > > diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > > b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > > index bf8f5523e6..63f4724062 100644
> > > > --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > > +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > > @@ -654,8 +654,9 @@ NetGetIpClass (
> > > >
> > > >    If all bits of the host address of IP are 0 or 1, IP is also not
> a
> > > valid unicast
> > > > address,
> > > >    except when the originator is one of the endpoints of a point-to-
> > > point link
> > > > with a 31-bit
> > > > -  mask (RFC3021).
> > > > -
> > > > +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special
> > > network
> > > > environment (e.g.
> > > > +  PPP link).
> > > > +
> > > >    @param[in]  Ip                    The IP to check against.
> > > >    @param[in]  NetMask               The mask of the IP.
> > > >
> > > > @@ -669,18 +670,20 @@ NetIp4IsUnicast (
> > > >    IN IP4_ADDR               NetMask
> > > >    )
> > > >  {
> > > > +  INTN   MaskLength;
> > > > +
> > > >    ASSERT (NetMask != 0);
> > > >
> > > >    if (Ip == 0 || IP4_IS_LOCAL_BROADCAST (Ip)) {
> > > >      return FALSE;
> > > >    }
> > > >
> > > > -  if (NetGetMaskLength (NetMask) != 31) {
> > > > +  MaskLength = NetGetMaskLength (NetMask);
> > > > +  ASSERT ((MaskLength >= 0) && (MaskLength <= IP4_MASK_NUM));
> > > > +  if (MaskLength < 31) {
> > > >      if (((Ip &~NetMask) == ~NetMask) || ((Ip &~NetMask) == 0)) {
> > > >        return FALSE;
> > > >      }
> > > > -  } else {
> > > > -    return TRUE;
> > > >    }
> > > >
> > > >    return TRUE;
> > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > > index e0fffc9d0d..994a81f4de 100644
> > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > > @@ -55,7 +55,7 @@ typedef struct _IP4_SERVICE    IP4_SERVICE;
> > > >  /// Compose the fragment field to be used in the IP4 header.
> > > >  ///
> > > >  #define IP4_HEAD_FRAGMENT_FIELD(Df, Mf, Offset) \
> > > > -    ((UINT16)(((Df) ? 0x4000 : 0) | ((Mf) ? 0x2000 : 0) |
> (((Offset) >>
> > > 3) &
> > > > 0x1fff)))
> > > > +    ((UINT16)(((Df) ? IP4_HEAD_DF_MASK : 0) | ((Mf) ?
> > > > IP4_HEAD_MF_MASK : 0) | (((Offset) >> 3) & IP4_HEAD_OFFSET_MASK)))
> > > >
> > > >  #define IP4_LAST_FRAGMENT(FragmentField)  \
> > > >            (((FragmentField) & IP4_HEAD_MF_MASK) == 0)
> > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > > index 6e0e3290c7..b0172283b7 100644
> > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > > @@ -138,6 +138,7 @@ Ip4CancelFrameArp (
> > > >    @param[in]  CallBack          Call back function to execute if
> > > transmission
> > > >                                  finished.
> > > >    @param[in]  Context           Opaque parameter to the call back.
> > > > +  @param[in]  IpSb              The pointer to the IP4 service
> binding
> > > instance.
> > > >
> > > >    @retval   Token               The wrapped token if succeed
> > > >    @retval   NULL                The wrapped token if NULL
> > > > @@ -149,7 +150,8 @@ Ip4WrapLinkTxToken (
> > > >    IN IP4_PROTOCOL           *IpInstance     OPTIONAL,
> > > >    IN NET_BUF                *Packet,
> > > >    IN IP4_FRAME_CALLBACK     CallBack,
> > > > -  IN VOID                   *Context
> > > > +  IN VOID                   *Context,
> > > > +  IN IP4_SERVICE            *IpSb
> > > >    )
> > > >  {
> > > >    EFI_MANAGED_NETWORK_COMPLETION_TOKEN  *MnpToken;
> > > > @@ -170,6 +172,7 @@ Ip4WrapLinkTxToken (
> > > >
> > > >    Token->Interface  = Interface;
> > > >    Token->IpInstance = IpInstance;
> > > > +  Token->IpSb       = IpSb;
> > > >    Token->CallBack   = CallBack;
> > > >    Token->Packet     = Packet;
> > > >    Token->Context    = Context;
> > > > @@ -792,6 +795,86 @@ Ip4FreeInterface (
> > > >    return EFI_SUCCESS;
> > > >  }
> > > >
> > > > +/**
> > > > +  This function tries to send all the queued frames in ArpQue to
> the
> > > default
> > > > gateway if
> > > > +  the ARP resolve for direct destination address is failed when
> using
> > > /32
> > > > subnet mask.
> > > > +
> > > > +  @param[in]   ArpQue           The ARP queue of a failed request.
> > > > +
> > > > +  @retval EFI_SUCCESS           All the queued frames have been
> send to
> > > the
> > > > default route.
> > > > +  @retval Others                Failed to send the queued frames.
> > > > +
> > > > +**/
> > > > +EFI_STATUS
> > > > +Ip4SendFrameToDefaultRoute (
> > > > +  IN  IP4_ARP_QUE               *ArpQue
> > > > +  )
> > > > +{
> > > > +  LIST_ENTRY                *Entry;
> > > > +  LIST_ENTRY                *Next;
> > > > +  IP4_ROUTE_CACHE_ENTRY     *RtCacheEntry;
> > > > +  IP4_LINK_TX_TOKEN         *Token;
> > > > +  IP4_ADDR                  Gateway;
> > > > +  EFI_STATUS                Status;
> > > > +  IP4_ROUTE_ENTRY           *DefaultRoute;
> > > > +
> > > > +  //
> > > > +  // ARP resolve failed when using /32 subnet mask.
> > > > +  //
> > > > +  NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> > > > +    RemoveEntryList (Entry);
> > > > +    Token = NET_LIST_USER_STRUCT (Entry, IP4_LINK_TX_TOKEN, Link);
> > > > +    ASSERT (Token->Interface->SubnetMask == IP4_ALLONE_ADDRESS);
> > > > +    //
> > > > +    // Find the default gateway IP address. The default route was
> saved
> > > to
> > > > the RtCacheEntry->Tag in Ip4Route().
> > > > +    //
> > > > +    RtCacheEntry = NULL;
> > > > +    if (Token->IpInstance != NULL) {
> > > > +      RtCacheEntry = Ip4FindRouteCache (Token->IpInstance-
> >RouteTable,
> > > > NTOHL (ArpQue->Ip), Token->Interface->Ip);
> > > > +    }
> > > > +    if (RtCacheEntry == NULL) {
> > > > +      RtCacheEntry = Ip4FindRouteCache (Token->IpSb-
> > >DefaultRouteTable,
> > > > NTOHL (ArpQue->Ip), Token->Interface->Ip);
> > > > +    }
> > > > +    if (RtCacheEntry == NULL) {
> > > > +      Status= EFI_NO_MAPPING;
> > > > +      goto ON_ERROR;
> > > > +    }
> > > > +    DefaultRoute = (IP4_ROUTE_ENTRY*)RtCacheEntry->Tag;
> > > > +    if (DefaultRoute == NULL) {
> > > > +      Status= EFI_NO_MAPPING;
> > > > +      goto ON_ERROR;
> > > > +    }
> > > > +    //
> > > > +    // Try to send the frame to the default route.
> > > > +    //
> > > > +    Gateway = DefaultRoute->NextHop;
> > > > +    if (ArpQue->Ip == Gateway) {
> > > > +      //
> > > > +      // ARP resolve for the default route is failed, return error
> to
> > > caller.
> > > > +      //
> > > > +      Status= EFI_NO_MAPPING;
> > > > +      goto ON_ERROR;
> > > > +    }
> > > > +    RtCacheEntry->NextHop = Gateway;
> > > > +    Status = Ip4SendFrame (Token->Interface,Token-
> >IpInstance,Token-
> > > > >Packet,Gateway,Token->CallBack,Token->Context,Token->IpSb);
> > > > +    if (EFI_ERROR (Status)) {
> > > > +      Status= EFI_NO_MAPPING;
> > > > +      goto ON_ERROR;
> > > > +    }
> > > > +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> > > > +  }
> > > > +
> > > > +  return EFI_SUCCESS;
> > > > +
> > > > +ON_ERROR:
> > > > +  if (RtCacheEntry != NULL) {
> > > > +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> > > > +  }
> > > > +  Token->CallBack (Token->IpInstance, Token->Packet, Status, 0,
> Token-
> > > > >Context);
> > > > +  Ip4FreeLinkTxToken (Token);
> > > > +  return Status;
> > > > +}
> > > > +
> > > >
> > > >  /**
> > > >    Callback function when ARP request are finished. It will
> cancelled
> > > > @@ -814,6 +897,7 @@ Ip4OnArpResolvedDpc (
> > > >    IP4_INTERFACE             *Interface;
> > > >    IP4_LINK_TX_TOKEN         *Token;
> > > >    EFI_STATUS                Status;
> > > > +  EFI_STATUS                IoStatus;
> > > >
> > > >    ArpQue = (IP4_ARP_QUE *) Context;
> > > >    NET_CHECK_SIGNATURE (ArpQue, IP4_FRAME_ARP_SIGNATURE);
> > > > @@ -821,14 +905,23 @@ Ip4OnArpResolvedDpc (
> > > >    RemoveEntryList (&ArpQue->Link);
> > > >
> > > >    //
> > > > -  // ARP resolve failed for some reason. Release all the frame
> > > > -  // and ARP queue itself. Ip4FreeArpQue will call the frame's
> > > > -  // owner back.
> > > > +  // ARP resolve failed for some reason.
> > > >    //
> > > >    if (NET_MAC_EQUAL (&ArpQue->Mac, &mZeroMacAddress, ArpQue-
> > > > >Interface->HwaddrLen)) {
> > > > -    Ip4FreeArpQue (ArpQue, EFI_NO_MAPPING);
> > > > -
> > > > -    return ;
> > > > +    if (ArpQue->Interface->SubnetMask != IP4_ALLONE_ADDRESS) {
> > > > +      //
> > > > +      // Release all the frame and ARP queue itself. Ip4FreeArpQue
> will
> > > call the
> > > > frame's
> > > > +      // owner back.
> > > > +      //
> > > > +      IoStatus = EFI_NO_MAPPING;
> > > > +    } else {
> > > > +      //
> > > > +      // ARP resolve failed when using 32bit subnet mask, try to
> send
> > > the
> > > > packets to the
> > > > +      // default route.
> > > > +      //
> > > > +      IoStatus = Ip4SendFrameToDefaultRoute (ArpQue);
> > > > +    }
> > > > +    goto ON_EXIT;
> > > >    }
> > > >
> > > >    //
> > > > @@ -836,6 +929,7 @@ Ip4OnArpResolvedDpc (
> > > >    // queue. It isn't necessary for us to cache the ARP binding
> because
> > > >    // we always check the ARP cache first before transmit.
> > > >    //
> > > > +  IoStatus = EFI_SUCCESS;
> > > >    Interface = ArpQue->Interface;
> > > >
> > > >    NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> > > > @@ -863,7 +957,8 @@ Ip4OnArpResolvedDpc (
> > > >      }
> > > >    }
> > > >
> > > > -  Ip4FreeArpQue (ArpQue, EFI_SUCCESS);
> > > > +ON_EXIT:
> > > > +  Ip4FreeArpQue (ArpQue, IoStatus);
> > > >  }
> > > >
> > > >  /**
> > > > @@ -957,6 +1052,7 @@ Ip4OnFrameSent (
> > > >                                  to.
> > > >    @param[in]  CallBack          Function to call back when transmit
> > > finished.
> > > >    @param[in]  Context           Opaque parameter to the call back.
> > > > +  @param[in]  IpSb              The pointer to the IP4 service
> binding
> > > instance.
> > > >
> > > >    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send
> > the
> > > > frame
> > > >    @retval EFI_NO_MAPPING        Can't resolve the MAC for the
> nexthop
> > > > @@ -971,7 +1067,8 @@ Ip4SendFrame (
> > > >    IN  NET_BUF               *Packet,
> > > >    IN  IP4_ADDR              NextHop,
> > > >    IN  IP4_FRAME_CALLBACK    CallBack,
> > > > -  IN  VOID                  *Context
> > > > +  IN  VOID                  *Context,
> > > > +  IN IP4_SERVICE            *IpSb
> > > >    )
> > > >  {
> > > >    IP4_LINK_TX_TOKEN         *Token;
> > > > @@ -982,7 +1079,7 @@ Ip4SendFrame (
> > > >
> > > >    ASSERT (Interface->Configured);
> > > >
> > > > -  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet,
> CallBack,
> > > > Context);
> > > > +  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet,
> CallBack,
> > > > Context, IpSb);
> > > >
> > > >    if (Token == NULL) {
> > > >      return EFI_OUT_OF_RESOURCES;
> > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > > index 909837131e..36e4ab3f7a 100644
> > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > > @@ -79,6 +79,7 @@ typedef struct {
> > > >    LIST_ENTRY                            Link;
> > > >
> > > >    IP4_INTERFACE                         *Interface;
> > > > +  IP4_SERVICE                           *IpSb;
> > > >
> > > >    IP4_PROTOCOL                          *IpInstance;
> > > >    IP4_FRAME_CALLBACK                    CallBack;
> > > > @@ -262,6 +263,7 @@ Ip4FreeInterface (
> > > >                                  to.
> > > >    @param[in]  CallBack          Function to call back when transmit
> > > finished.
> > > >    @param[in]  Context           Opaque parameter to the call back.
> > > > +  @param[in]  IpSb              The pointer to the IP4 service
> binding
> > > instance.
> > > >
> > > >    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to send
> > the
> > > > frame
> > > >    @retval EFI_NO_MAPPING        Can't resolve the MAC for the
> nexthop
> > > > @@ -276,7 +278,8 @@ Ip4SendFrame (
> > > >    IN  NET_BUF               *Packet,
> > > >    IN  IP4_ADDR              NextHop,
> > > >    IN  IP4_FRAME_CALLBACK    CallBack,
> > > > -  IN  VOID                  *Context
> > > > +  IN  VOID                  *Context,
> > > > +  IN IP4_SERVICE            *IpSb
> > > >    );
> > > >
> > > >  /**
> > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > > index 6a26143e30..7c27db6753 100644
> > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > > @@ -1259,7 +1259,7 @@ EfiIp4Routes (
> > > >    // the gateway address must be a unicast on the connected network
> if
> > > not
> > > > zero.
> > > >    //
> > > >    if ((Nexthop != IP4_ALLZERO_ADDRESS) &&
> > > > -      (!IP4_NET_EQUAL (Nexthop, IpIf->Ip, IpIf->SubnetMask) ||
> > > > +      ((IpIf->SubnetMask != IP4_ALLONE_ADDRESS && !IP4_NET_EQUAL
> > > > (Nexthop, IpIf->Ip, IpIf->SubnetMask)) ||
> > > >          IP4_IS_BROADCAST (Ip4GetNetCast (Nexthop, IpIf)))) {
> > > >
> > > >      Status = EFI_INVALID_PARAMETER;
> > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > > index 1716f43576..bbbcf36eff 100644
> > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > > @@ -309,15 +309,15 @@ Ip4Output (
> > > >      // Route the packet unless overrided, that is, GateWay isn't
> zero.
> > > >      //
> > > >      if (IpInstance == NULL) {
> > > > -      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> Head-
> > > > >Src);
> > > > +      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> Head-
> > > >Src,
> > > > IpIf->SubnetMask);
> > > >      } else {
> > > > -      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst,
> Head-
> > > >Src);
> > > > +      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst,
> Head-
> > > >Src,
> > > > IpIf->SubnetMask);
> > > >        //
> > > >        // If failed to route the packet by using the instance's
> route
> > > table,
> > > >        // try to use the default route table.
> > > >        //
> > > >        if (CacheEntry == NULL) {
> > > > -        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> > > Head-
> > > > >Src);
> > > > +        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> > > Head-
> > > > >Src, IpIf->SubnetMask);
> > > >        }
> > > >      }
> > > >
> > > > @@ -386,7 +386,8 @@ Ip4Output (
> > > >                   Fragment,
> > > >                   GateWay,
> > > >                   Ip4SysPacketSent,
> > > > -                 Packet
> > > > +                 Packet,
> > > > +                 IpSb
> > > >                   );
> > > >
> > > >        if (EFI_ERROR (Status)) {
> > > > @@ -429,7 +430,7 @@ Ip4Output (
> > > >    //    upper layer's packets.
> > > >    //
> > > >    Ip4PrependHead (Packet, Head, Option, OptLen);
> > > > -  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay,
> Callback,
> > > > Context);
> > > > +  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay,
> Callback,
> > > > Context, IpSb);
> > > >
> > > >    if (EFI_ERROR (Status)) {
> > > >      goto ON_ERROR;
> > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > > index d240d5343a..e648dee908 100644
> > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > > @@ -494,6 +494,8 @@ Ip4FindRouteEntry (
> > > >    @param[in]  RtTable               The route table to search from
> > > >    @param[in]  Dest                  The destination address to
> search
> > > for
> > > >    @param[in]  Src                   The source address to search
> for
> > > > +  @param[in]  SubnetMask            The subnet mask of the Src
> address,
> > > this
> > > > field is
> > > > +                                    used to check if the station is
> > > using /32 subnet.
> > > >
> > > >    @return NULL if failed to route packet, otherwise a route cache
> > > >            entry that can be used to route packet.
> > > > @@ -503,7 +505,8 @@ IP4_ROUTE_CACHE_ENTRY *
> > > >  Ip4Route (
> > > >    IN IP4_ROUTE_TABLE        *RtTable,
> > > >    IN IP4_ADDR               Dest,
> > > > -  IN IP4_ADDR               Src
> > > > +  IN IP4_ADDR               Src,
> > > > +  IN IP4_ADDR               SubnetMask
> > > >    )
> > > >  {
> > > >    LIST_ENTRY                *Head;
> > > > @@ -534,7 +537,10 @@ Ip4Route (
> > > >    //
> > > >    RtEntry = Ip4FindRouteEntry (RtTable, Dest);
> > > >
> > > > -  if (RtEntry == NULL) {
> > > > +  //
> > > > +  // We will always try to use the direct destination address when
> /32
> > > subnet
> > > > mask is used.
> > > > +  //
> > > > +  if (RtEntry == NULL && SubnetMask != IP4_ALLONE_ADDRESS) {
> > > >      return NULL;
> > > >    }
> > > >
> > > > @@ -544,16 +550,23 @@ Ip4Route (
> > > >    // network. Otherwise, it is an indirect route, the packet will
> be
> > > >    // sent to the next hop router.
> > > >    //
> > > > -  if ((RtEntry->Flag & IP4_DIRECT_ROUTE) != 0) {
> > > > +  // When using /32 subnet mask, the packet will always be sent to
> the
> > > > direct
> > > > +  // destination first, if we can't find a matching route cache.
> > > > +  //
> > > > +  if (SubnetMask == IP4_ALLONE_ADDRESS || ((RtEntry->Flag &
> > > > IP4_DIRECT_ROUTE) != 0)) {
> > > >      NextHop = Dest;
> > > >    } else {
> > > >      NextHop = RtEntry->NextHop;
> > > >    }
> > > >
> > > > -  Ip4FreeRouteEntry (RtEntry);
> > > > +  if (RtEntry != NULL) {
> > > > +    Ip4FreeRouteEntry (RtEntry);
> > > > +  }
> > > >
> > > >    //
> > > >    // Create a route cache entry, and tag it as spawned from this
> route
> > > entry
> > > > +  // For /32 subnet mask, the default route in RtEntry will be used
> if
> > > failed
> > > > +  // to send the packet to driect destination address.
> > > >    //
> > > >    RtCacheEntry = Ip4CreateRouteCacheEntry (Dest, Src, NextHop,
> (UINTN)
> > > > RtEntry);
> > > >
> > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > > index 6269f4ceda..99b9cfbbac 100644
> > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > > @@ -194,6 +194,8 @@ Ip4FreeRouteCacheEntry (
> > > >    @param[in]  RtTable               The route table to search from
> > > >    @param[in]  Dest                  The destination address to
> search
> > > for
> > > >    @param[in]  Src                   The source address to search
> for
> > > > +  @param[in]  SubnetMask            The subnet mask of the Src
> address,
> > > this
> > > > field is
> > > > +                                    used to check if the station is
> > > using /32 subnet.
> > > >
> > > >    @return NULL if failed to route packet, otherwise a route cache
> > > >            entry that can be used to route packet.
> > > > @@ -203,7 +205,8 @@ IP4_ROUTE_CACHE_ENTRY *
> > > >  Ip4Route (
> > > >    IN IP4_ROUTE_TABLE        *RtTable,
> > > >    IN IP4_ADDR               Dest,
> > > > -  IN IP4_ADDR               Src
> > > > +  IN IP4_ADDR               Src,
> > > > +  IN IP4_ADDR               SubnetMask
> > > >    );
> > > >
> > > >  /**
> > > > diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > index d5a1a8c303..03903640b8 100644
> > > > --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > @@ -509,8 +509,9 @@ Mtftp4Start (
> > > >      goto ON_ERROR;
> > > >    }
> > > >
> > > > +  gBS->RestoreTPL(OldTpl);
> > > > +
> > > >    if (Token->Event != NULL) {
> > > > -    gBS->RestoreTPL (OldTpl);
> > > >      return EFI_SUCCESS;
> > > >    }
> > > >
> > > > @@ -522,7 +523,6 @@ Mtftp4Start (
> > > >      This->Poll (This);
> > > >    }
> > > >
> > > > -  gBS->RestoreTPL (OldTpl);
> > > >    return Token->Status;
> > > >
> > > >  ON_ERROR:
> > > > @@ -682,7 +682,7 @@ EfiMtftp4Configure (
> > > >      }
> > > >
> > > >      if ((Gateway != 0) &&
> > > > -        (!IP4_NET_EQUAL (Gateway, Ip, Netmask) || (Netmask != 0
> > > > && !NetIp4IsUnicast (Gateway, Netmask)))) {
> > > > +        ((Netmask != 0xFFFFFFFF && !IP4_NET_EQUAL (Gateway, Ip,
> > > Netmask))
> > > > || (Netmask != 0 && !NetIp4IsUnicast (Gateway, Netmask)))) {
> > > >
> > > >        return EFI_INVALID_PARAMETER;
> > > >      }
> > > > --
> > > > 2.18.0.windows.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask.
  2018-08-28  1:52 ` [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask Fu Siyuan
@ 2018-08-28 14:55   ` Carsey, Jaben
  2018-08-29  1:37     ` Ni, Ruiyu
  2018-08-30  8:28   ` Wu, Jiaxin
  2018-08-31  7:46   ` Ye, Ting
  2 siblings, 1 reply; 12+ messages in thread
From: Carsey, Jaben @ 2018-08-28 14:55 UTC (permalink / raw)
  To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Ye, Ting, Wu, Jiaxin

looks good to me.  Ray?

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu
> Siyuan
> Sent: Monday, August 27, 2018 6:53 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin
> <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch 2/2] ShellPkg: Update Ifconfig command to accept
> 32bit subnet mask.
> Importance: High
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> index 52415e0ad0..e9f644c739 100644
> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> @@ -1032,6 +1032,7 @@ IfConfigSetInterfaceInfo (
>        SubnetMask  = NTOHL (SubnetMask);
>        TempGateway = NTOHL (TempGateway);
>        if ((SubnetMask != 0) &&
> +          (SubnetMask != 0xFFFFFFFFu) &&
>            !NetIp4IsUnicast (TempGateway, SubnetMask)) {
>          ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INVALID_GATEWAY), gShellNetwork1HiiHandle, VarArg-
> >Arg);
>          ShellStatus = SHELL_INVALID_PARAMETER;
> --
> 2.18.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask.
  2018-08-28 14:55   ` Carsey, Jaben
@ 2018-08-29  1:37     ` Ni, Ruiyu
  0 siblings, 0 replies; 12+ messages in thread
From: Ni, Ruiyu @ 2018-08-29  1:37 UTC (permalink / raw)
  To: Carsey, Jaben, Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ye, Ting, Wu, Jiaxin

On 8/28/2018 10:55 PM, Carsey, Jaben wrote:
> looks good to me.  Ray?
> 
> Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu
>> Siyuan
>> Sent: Monday, August 27, 2018 6:53 PM
>> To: edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin
>> <jiaxin.wu@intel.com>
>> Subject: [edk2] [Patch 2/2] ShellPkg: Update Ifconfig command to accept
>> 32bit subnet mask.
>> Importance: High
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Ye Ting <ting.ye@intel.com>
>> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
>> ---
>>   ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
>> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
>> index 52415e0ad0..e9f644c739 100644
>> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
>> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
>> @@ -1032,6 +1032,7 @@ IfConfigSetInterfaceInfo (
>>         SubnetMask  = NTOHL (SubnetMask);
>>         TempGateway = NTOHL (TempGateway);
>>         if ((SubnetMask != 0) &&
>> +          (SubnetMask != 0xFFFFFFFFu) &&
>>             !NetIp4IsUnicast (TempGateway, SubnetMask)) {
>>           ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
>> (STR_IFCONFIG_INVALID_GATEWAY), gShellNetwork1HiiHandle, VarArg-
>>> Arg);
>>           ShellStatus = SHELL_INVALID_PARAMETER;
>> --
>> 2.18.0.windows.1
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

-- 
Thanks,
Ray


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot.
  2018-08-28  9:41         ` Fu, Siyuan
@ 2018-08-29  2:09           ` Wu, Jiaxin
  0 siblings, 0 replies; 12+ messages in thread
From: Wu, Jiaxin @ 2018-08-29  2:09 UTC (permalink / raw)
  To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ye, Ting

Against the below condition code in Ip4Route():

  //
  // We will always try to use the direct destination address when /32 subnet mask is used.
  //
  if (RtEntry == NULL && SubnetMask != IP4_ALLONE_ADDRESS) {
    return NULL;
  }

If there is no default route { Dest: 0.0.0.0, Mask: 0.0.0.0,NextHope: Gataway } in instance route table, while it exists in default route table, the failure will happen. So, we need update the code to handle such case.

Thanks,
Jiaxin

> -----Original Message-----
> From: Fu, Siyuan
> Sent: Tuesday, August 28, 2018 5:41 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>
> Subject: RE: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> support for IP4 PXE boot.
> 
> Hi, Jiaxin
> 
> The IP4 routing logic for /32 subnet mask can be explain as below, maybe I
> need to add it to the commit log or in code comments.
> 
> Ip4Output() is called to send a packet
> 	If a matching route cache (src, dest -> xxx) can be find,
> 		send the packet to address xxx according to the route cache.
> 	else
> 		create a new route cache (src, dest -> dest)                               <-
> This is done in Ip4Route()
> 		if a default gateway is configured, save it to the route cache
> TAG.        <- This is done in Ip4Route()
> 		send the packet to dest                                                    <- This is
> done in Ip4SendFrame()
> 		if ARP resolve failed for dest
> 			find the matching route cache and check TAG to see
> if there is a default gateway          <- This is done in
> Ip4SendFrameToDefaultRoute ()
> 			if yes
> 				update the route cache to (src, dest ->
> default gw)
> 				send the packet to the default gw
> 
> 
> BestRegards
> Fu Siyuan
> 
> 
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Tuesday, August 28, 2018 4:53 PM
> > To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>
> > Subject: RE: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask
> > support for IP4 PXE boot.
> >
> >
> > Hi Siyuan,
> >
> > Below is my code review understanding, maybe something is wrong,
> please
> > correct me.
> >
> > > The "default route" means a route entry with zero SubnetAddress and
> zero
> > > SubnetMask, which will match all the dest address that can't match any
> > other
> > > non-zero Subnet* route entry.
> > > The "instance route table" is a list of route entries which belong to an
> > IP child
> > > instance.
> > > The "default route table" is the route table used by these IP child
> > instances
> > > with default IP address.
> > > A default route (which is just one route entry) may belong to the
> > default
> > > route table, or any instance route table.
> > >
> >
> > Yes, I agree. If there is a default route {Dest: 0.0.0.0, Mask: 0.0.0.0,
> > NextHope: Gataway} in the instance route table, it must be set via IP-
> > >Routes().
> >
> >
> > > The new function Ip4SendFrameToDefaultRoute() will always try to send
> > the
> > > frames to the default entry, so the naming is correct.
> >
> > According above, we want to send the packet to the router (Gataway) via
> > the default route {Dest: 0.0.0.0, Mask: 0.0.0.0, NextHope: Gataway} info,
> > right? But the comments in Ip4Route() said:
> >   //
> >   // When using /32 subnet mask, the packet will always be sent to the
> > direct
> >   // destination first, if we can't find a matching route cache.
> >   //
> >
> > > While how to
> > > determine the default route entry is not in this function, the Ip4Route()
> > has
> > > been updated to handle the /32 subnet mask specially, and Ip4Output()
> > will
> > > guarantee we check the instance route table first, then default route
> > table
> > > (the code you quoted).
> >
> > After the code I quoted below in Ip4Output(), the route cache entry found
> > in Ip4SendFrameToDefaultRoute() is {Dest: X.X.X.X, Src: Y.Y.Y.Y, NextHope:
> > X.X.X.X, Tag: { Dest: X.X.X.X, Mask: 255.255.255.255, NextHope:
> > 0.0.0.0} }, if so, looks the package will be sent to  NextHope:  0.0.0.0?
> >
> >
> > >
> > > BestRegards
> > > Fu Siyuan
> > >
> > > > -----Original Message-----
> > > > From: Wu, Jiaxin
> > > > Sent: Tuesday, August 28, 2018 2:51 PM
> > > > To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
> > > > Cc: Ye, Ting <ting.ye@intel.com>
> > > > Subject: RE: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet
> mask
> > > > support for IP4 PXE boot.
> > > >
> > > > Hi Siyuan,
> > > >
> > > > In Ip4SendFrameToDefaultRoute(), you tried to find the default
> gateway
> > IP
> > > > address by using the found RtCacheEntry->Tag, I'm confused why it's
> > the
> > > > default gateway? In my understanding, tag is the cache's corresponding
> > > > route entry.  Besides, why we must send the frame to "DefaultRouter",
> > > > should be the instance router first, then default router? If so, I
> > suggest
> > > > to rename the function to Ip4SendFrameToRoute(). Then, the logic in
> > the
> > > > function to retrieve the gateway should be:
> > > > {
> > > >     if (IpInstance == NULL) {
> > > >       CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst, Head-
> > > >Src,
> > > > IpIf->SubnetMask);
> > > >     } else {
> > > >       CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst, Head-
> > >Src,
> > > > IpIf->SubnetMask);
> > > >       if (CacheEntry == NULL) {
> > > >         CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> > Head-
> > > > >Src, IpIf->SubnetMask);
> > > >       }
> > > >     }
> > > >
> > > >     if (CacheEntry == NULL) {
> > > >       return EFI_NOT_FOUND;
> > > >     }
> > > >     GateWay = CacheEntry->NextHop;
> > > >     Ip4FreeRouteCacheEntry (CacheEntry);
> > > > }
> > > >
> > > > What do you think?
> > > >
> > > > Thanks,
> > > > Jiaxin
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Fu, Siyuan
> > > > > Sent: Tuesday, August 28, 2018 9:53 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > > > > Subject: [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet
> mask
> > > > > support for IP4 PXE boot.
> > > > >
> > > > > This patch updates IP4 stack to support 32bit subnet mask in PXE
> > boot
> > > > > process.
> > > > > When 32bit subnet mask is used, the IP4 driver couldn't use the
> > subnet
> > > > mask
> > > > > to determine
> > > > > whether destination IP address is on-link or not, so it will always
> > try
> > > > to send
> > > > > all the
> > > > > packets to the destination IP address directly first, if failed it
> > will
> > > > continue
> > > > > to try the default gateway.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> > > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > > > > ---
> > > > >  MdeModulePkg/Include/Library/NetLib.h         |   5 +-
> > > > >  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c    |  13 +-
> > > > >  .../Universal/Network/Ip4Dxe/Ip4Common.h      |   2 +-
> > > > >  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c | 117
> > > > > ++++++++++++++++--
> > > > >  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h |   5 +-
> > > > >  .../Universal/Network/Ip4Dxe/Ip4Impl.c        |   2 +-
> > > > >  .../Universal/Network/Ip4Dxe/Ip4Output.c      |  11 +-
> > > > >  .../Universal/Network/Ip4Dxe/Ip4Route.c       |  21 +++-
> > > > >  .../Universal/Network/Ip4Dxe/Ip4Route.h       |   5 +-
> > > > >  .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c  |   6 +-
> > > > >  10 files changed, 154 insertions(+), 33 deletions(-)
> > > > >
> > > > > diff --git a/MdeModulePkg/Include/Library/NetLib.h
> > > > > b/MdeModulePkg/Include/Library/NetLib.h
> > > > > index ef7bc429c1..b7ef99c7b5 100644
> > > > > --- a/MdeModulePkg/Include/Library/NetLib.h
> > > > > +++ b/MdeModulePkg/Include/Library/NetLib.h
> > > > > @@ -422,8 +422,9 @@ NetGetIpClass (
> > > > >
> > > > >    If all bits of the host address of IP are 0 or 1, IP is also not
> > a
> > > > valid unicast
> > > > > address,
> > > > >    except when the originator is one of the endpoints of a point-to-
> > > > point link
> > > > > with a 31-bit
> > > > > -  mask (RFC3021).
> > > > > -
> > > > > +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special
> > > > network
> > > > > environment (e.g.
> > > > > +  PPP link).
> > > > > +
> > > > >    @param[in]  Ip                    The IP to check against.
> > > > >    @param[in]  NetMask               The mask of the IP.
> > > > >
> > > > > diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > > > b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > > > index bf8f5523e6..63f4724062 100644
> > > > > --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > > > +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> > > > > @@ -654,8 +654,9 @@ NetGetIpClass (
> > > > >
> > > > >    If all bits of the host address of IP are 0 or 1, IP is also not
> > a
> > > > valid unicast
> > > > > address,
> > > > >    except when the originator is one of the endpoints of a point-to-
> > > > point link
> > > > > with a 31-bit
> > > > > -  mask (RFC3021).
> > > > > -
> > > > > +  mask (RFC3021), or a 32bit NetMask (all 0xFF) is used for special
> > > > network
> > > > > environment (e.g.
> > > > > +  PPP link).
> > > > > +
> > > > >    @param[in]  Ip                    The IP to check against.
> > > > >    @param[in]  NetMask               The mask of the IP.
> > > > >
> > > > > @@ -669,18 +670,20 @@ NetIp4IsUnicast (
> > > > >    IN IP4_ADDR               NetMask
> > > > >    )
> > > > >  {
> > > > > +  INTN   MaskLength;
> > > > > +
> > > > >    ASSERT (NetMask != 0);
> > > > >
> > > > >    if (Ip == 0 || IP4_IS_LOCAL_BROADCAST (Ip)) {
> > > > >      return FALSE;
> > > > >    }
> > > > >
> > > > > -  if (NetGetMaskLength (NetMask) != 31) {
> > > > > +  MaskLength = NetGetMaskLength (NetMask);
> > > > > +  ASSERT ((MaskLength >= 0) && (MaskLength <= IP4_MASK_NUM));
> > > > > +  if (MaskLength < 31) {
> > > > >      if (((Ip &~NetMask) == ~NetMask) || ((Ip &~NetMask) == 0)) {
> > > > >        return FALSE;
> > > > >      }
> > > > > -  } else {
> > > > > -    return TRUE;
> > > > >    }
> > > > >
> > > > >    return TRUE;
> > > > > diff --git
> a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > > > index e0fffc9d0d..994a81f4de 100644
> > > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Common.h
> > > > > @@ -55,7 +55,7 @@ typedef struct _IP4_SERVICE    IP4_SERVICE;
> > > > >  /// Compose the fragment field to be used in the IP4 header.
> > > > >  ///
> > > > >  #define IP4_HEAD_FRAGMENT_FIELD(Df, Mf, Offset) \
> > > > > -    ((UINT16)(((Df) ? 0x4000 : 0) | ((Mf) ? 0x2000 : 0) |
> > (((Offset) >>
> > > > 3) &
> > > > > 0x1fff)))
> > > > > +    ((UINT16)(((Df) ? IP4_HEAD_DF_MASK : 0) | ((Mf) ?
> > > > > IP4_HEAD_MF_MASK : 0) | (((Offset) >> 3) &
> IP4_HEAD_OFFSET_MASK)))
> > > > >
> > > > >  #define IP4_LAST_FRAGMENT(FragmentField)  \
> > > > >            (((FragmentField) & IP4_HEAD_MF_MASK) == 0)
> > > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > > > index 6e0e3290c7..b0172283b7 100644
> > > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> > > > > @@ -138,6 +138,7 @@ Ip4CancelFrameArp (
> > > > >    @param[in]  CallBack          Call back function to execute if
> > > > transmission
> > > > >                                  finished.
> > > > >    @param[in]  Context           Opaque parameter to the call back.
> > > > > +  @param[in]  IpSb              The pointer to the IP4 service
> > binding
> > > > instance.
> > > > >
> > > > >    @retval   Token               The wrapped token if succeed
> > > > >    @retval   NULL                The wrapped token if NULL
> > > > > @@ -149,7 +150,8 @@ Ip4WrapLinkTxToken (
> > > > >    IN IP4_PROTOCOL           *IpInstance     OPTIONAL,
> > > > >    IN NET_BUF                *Packet,
> > > > >    IN IP4_FRAME_CALLBACK     CallBack,
> > > > > -  IN VOID                   *Context
> > > > > +  IN VOID                   *Context,
> > > > > +  IN IP4_SERVICE            *IpSb
> > > > >    )
> > > > >  {
> > > > >    EFI_MANAGED_NETWORK_COMPLETION_TOKEN  *MnpToken;
> > > > > @@ -170,6 +172,7 @@ Ip4WrapLinkTxToken (
> > > > >
> > > > >    Token->Interface  = Interface;
> > > > >    Token->IpInstance = IpInstance;
> > > > > +  Token->IpSb       = IpSb;
> > > > >    Token->CallBack   = CallBack;
> > > > >    Token->Packet     = Packet;
> > > > >    Token->Context    = Context;
> > > > > @@ -792,6 +795,86 @@ Ip4FreeInterface (
> > > > >    return EFI_SUCCESS;
> > > > >  }
> > > > >
> > > > > +/**
> > > > > +  This function tries to send all the queued frames in ArpQue to
> > the
> > > > default
> > > > > gateway if
> > > > > +  the ARP resolve for direct destination address is failed when
> > using
> > > > /32
> > > > > subnet mask.
> > > > > +
> > > > > +  @param[in]   ArpQue           The ARP queue of a failed request.
> > > > > +
> > > > > +  @retval EFI_SUCCESS           All the queued frames have been
> > send to
> > > > the
> > > > > default route.
> > > > > +  @retval Others                Failed to send the queued frames.
> > > > > +
> > > > > +**/
> > > > > +EFI_STATUS
> > > > > +Ip4SendFrameToDefaultRoute (
> > > > > +  IN  IP4_ARP_QUE               *ArpQue
> > > > > +  )
> > > > > +{
> > > > > +  LIST_ENTRY                *Entry;
> > > > > +  LIST_ENTRY                *Next;
> > > > > +  IP4_ROUTE_CACHE_ENTRY     *RtCacheEntry;
> > > > > +  IP4_LINK_TX_TOKEN         *Token;
> > > > > +  IP4_ADDR                  Gateway;
> > > > > +  EFI_STATUS                Status;
> > > > > +  IP4_ROUTE_ENTRY           *DefaultRoute;
> > > > > +
> > > > > +  //
> > > > > +  // ARP resolve failed when using /32 subnet mask.
> > > > > +  //
> > > > > +  NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> > > > > +    RemoveEntryList (Entry);
> > > > > +    Token = NET_LIST_USER_STRUCT (Entry, IP4_LINK_TX_TOKEN,
> Link);
> > > > > +    ASSERT (Token->Interface->SubnetMask ==
> IP4_ALLONE_ADDRESS);
> > > > > +    //
> > > > > +    // Find the default gateway IP address. The default route was
> > saved
> > > > to
> > > > > the RtCacheEntry->Tag in Ip4Route().
> > > > > +    //
> > > > > +    RtCacheEntry = NULL;
> > > > > +    if (Token->IpInstance != NULL) {
> > > > > +      RtCacheEntry = Ip4FindRouteCache (Token->IpInstance-
> > >RouteTable,
> > > > > NTOHL (ArpQue->Ip), Token->Interface->Ip);
> > > > > +    }
> > > > > +    if (RtCacheEntry == NULL) {
> > > > > +      RtCacheEntry = Ip4FindRouteCache (Token->IpSb-
> > > >DefaultRouteTable,
> > > > > NTOHL (ArpQue->Ip), Token->Interface->Ip);
> > > > > +    }
> > > > > +    if (RtCacheEntry == NULL) {
> > > > > +      Status= EFI_NO_MAPPING;
> > > > > +      goto ON_ERROR;
> > > > > +    }
> > > > > +    DefaultRoute = (IP4_ROUTE_ENTRY*)RtCacheEntry->Tag;
> > > > > +    if (DefaultRoute == NULL) {
> > > > > +      Status= EFI_NO_MAPPING;
> > > > > +      goto ON_ERROR;
> > > > > +    }
> > > > > +    //
> > > > > +    // Try to send the frame to the default route.
> > > > > +    //
> > > > > +    Gateway = DefaultRoute->NextHop;
> > > > > +    if (ArpQue->Ip == Gateway) {
> > > > > +      //
> > > > > +      // ARP resolve for the default route is failed, return error
> > to
> > > > caller.
> > > > > +      //
> > > > > +      Status= EFI_NO_MAPPING;
> > > > > +      goto ON_ERROR;
> > > > > +    }
> > > > > +    RtCacheEntry->NextHop = Gateway;
> > > > > +    Status = Ip4SendFrame (Token->Interface,Token-
> > >IpInstance,Token-
> > > > > >Packet,Gateway,Token->CallBack,Token->Context,Token->IpSb);
> > > > > +    if (EFI_ERROR (Status)) {
> > > > > +      Status= EFI_NO_MAPPING;
> > > > > +      goto ON_ERROR;
> > > > > +    }
> > > > > +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> > > > > +  }
> > > > > +
> > > > > +  return EFI_SUCCESS;
> > > > > +
> > > > > +ON_ERROR:
> > > > > +  if (RtCacheEntry != NULL) {
> > > > > +    Ip4FreeRouteCacheEntry (RtCacheEntry);
> > > > > +  }
> > > > > +  Token->CallBack (Token->IpInstance, Token->Packet, Status, 0,
> > Token-
> > > > > >Context);
> > > > > +  Ip4FreeLinkTxToken (Token);
> > > > > +  return Status;
> > > > > +}
> > > > > +
> > > > >
> > > > >  /**
> > > > >    Callback function when ARP request are finished. It will
> > cancelled
> > > > > @@ -814,6 +897,7 @@ Ip4OnArpResolvedDpc (
> > > > >    IP4_INTERFACE             *Interface;
> > > > >    IP4_LINK_TX_TOKEN         *Token;
> > > > >    EFI_STATUS                Status;
> > > > > +  EFI_STATUS                IoStatus;
> > > > >
> > > > >    ArpQue = (IP4_ARP_QUE *) Context;
> > > > >    NET_CHECK_SIGNATURE (ArpQue, IP4_FRAME_ARP_SIGNATURE);
> > > > > @@ -821,14 +905,23 @@ Ip4OnArpResolvedDpc (
> > > > >    RemoveEntryList (&ArpQue->Link);
> > > > >
> > > > >    //
> > > > > -  // ARP resolve failed for some reason. Release all the frame
> > > > > -  // and ARP queue itself. Ip4FreeArpQue will call the frame's
> > > > > -  // owner back.
> > > > > +  // ARP resolve failed for some reason.
> > > > >    //
> > > > >    if (NET_MAC_EQUAL (&ArpQue->Mac, &mZeroMacAddress,
> ArpQue-
> > > > > >Interface->HwaddrLen)) {
> > > > > -    Ip4FreeArpQue (ArpQue, EFI_NO_MAPPING);
> > > > > -
> > > > > -    return ;
> > > > > +    if (ArpQue->Interface->SubnetMask != IP4_ALLONE_ADDRESS) {
> > > > > +      //
> > > > > +      // Release all the frame and ARP queue itself. Ip4FreeArpQue
> > will
> > > > call the
> > > > > frame's
> > > > > +      // owner back.
> > > > > +      //
> > > > > +      IoStatus = EFI_NO_MAPPING;
> > > > > +    } else {
> > > > > +      //
> > > > > +      // ARP resolve failed when using 32bit subnet mask, try to
> > send
> > > > the
> > > > > packets to the
> > > > > +      // default route.
> > > > > +      //
> > > > > +      IoStatus = Ip4SendFrameToDefaultRoute (ArpQue);
> > > > > +    }
> > > > > +    goto ON_EXIT;
> > > > >    }
> > > > >
> > > > >    //
> > > > > @@ -836,6 +929,7 @@ Ip4OnArpResolvedDpc (
> > > > >    // queue. It isn't necessary for us to cache the ARP binding
> > because
> > > > >    // we always check the ARP cache first before transmit.
> > > > >    //
> > > > > +  IoStatus = EFI_SUCCESS;
> > > > >    Interface = ArpQue->Interface;
> > > > >
> > > > >    NET_LIST_FOR_EACH_SAFE (Entry, Next, &ArpQue->Frames) {
> > > > > @@ -863,7 +957,8 @@ Ip4OnArpResolvedDpc (
> > > > >      }
> > > > >    }
> > > > >
> > > > > -  Ip4FreeArpQue (ArpQue, EFI_SUCCESS);
> > > > > +ON_EXIT:
> > > > > +  Ip4FreeArpQue (ArpQue, IoStatus);
> > > > >  }
> > > > >
> > > > >  /**
> > > > > @@ -957,6 +1052,7 @@ Ip4OnFrameSent (
> > > > >                                  to.
> > > > >    @param[in]  CallBack          Function to call back when transmit
> > > > finished.
> > > > >    @param[in]  Context           Opaque parameter to the call back.
> > > > > +  @param[in]  IpSb              The pointer to the IP4 service
> > binding
> > > > instance.
> > > > >
> > > > >    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to
> send
> > > the
> > > > > frame
> > > > >    @retval EFI_NO_MAPPING        Can't resolve the MAC for the
> > nexthop
> > > > > @@ -971,7 +1067,8 @@ Ip4SendFrame (
> > > > >    IN  NET_BUF               *Packet,
> > > > >    IN  IP4_ADDR              NextHop,
> > > > >    IN  IP4_FRAME_CALLBACK    CallBack,
> > > > > -  IN  VOID                  *Context
> > > > > +  IN  VOID                  *Context,
> > > > > +  IN IP4_SERVICE            *IpSb
> > > > >    )
> > > > >  {
> > > > >    IP4_LINK_TX_TOKEN         *Token;
> > > > > @@ -982,7 +1079,7 @@ Ip4SendFrame (
> > > > >
> > > > >    ASSERT (Interface->Configured);
> > > > >
> > > > > -  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet,
> > CallBack,
> > > > > Context);
> > > > > +  Token = Ip4WrapLinkTxToken (Interface, IpInstance, Packet,
> > CallBack,
> > > > > Context, IpSb);
> > > > >
> > > > >    if (Token == NULL) {
> > > > >      return EFI_OUT_OF_RESOURCES;
> > > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > > > index 909837131e..36e4ab3f7a 100644
> > > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.h
> > > > > @@ -79,6 +79,7 @@ typedef struct {
> > > > >    LIST_ENTRY                            Link;
> > > > >
> > > > >    IP4_INTERFACE                         *Interface;
> > > > > +  IP4_SERVICE                           *IpSb;
> > > > >
> > > > >    IP4_PROTOCOL                          *IpInstance;
> > > > >    IP4_FRAME_CALLBACK                    CallBack;
> > > > > @@ -262,6 +263,7 @@ Ip4FreeInterface (
> > > > >                                  to.
> > > > >    @param[in]  CallBack          Function to call back when transmit
> > > > finished.
> > > > >    @param[in]  Context           Opaque parameter to the call back.
> > > > > +  @param[in]  IpSb              The pointer to the IP4 service
> > binding
> > > > instance.
> > > > >
> > > > >    @retval EFI_OUT_OF_RESOURCES  Failed to allocate resource to
> send
> > > the
> > > > > frame
> > > > >    @retval EFI_NO_MAPPING        Can't resolve the MAC for the
> > nexthop
> > > > > @@ -276,7 +278,8 @@ Ip4SendFrame (
> > > > >    IN  NET_BUF               *Packet,
> > > > >    IN  IP4_ADDR              NextHop,
> > > > >    IN  IP4_FRAME_CALLBACK    CallBack,
> > > > > -  IN  VOID                  *Context
> > > > > +  IN  VOID                  *Context,
> > > > > +  IN IP4_SERVICE            *IpSb
> > > > >    );
> > > > >
> > > > >  /**
> > > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > > > index 6a26143e30..7c27db6753 100644
> > > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
> > > > > @@ -1259,7 +1259,7 @@ EfiIp4Routes (
> > > > >    // the gateway address must be a unicast on the connected network
> > if
> > > > not
> > > > > zero.
> > > > >    //
> > > > >    if ((Nexthop != IP4_ALLZERO_ADDRESS) &&
> > > > > -      (!IP4_NET_EQUAL (Nexthop, IpIf->Ip, IpIf->SubnetMask) ||
> > > > > +      ((IpIf->SubnetMask != IP4_ALLONE_ADDRESS
> && !IP4_NET_EQUAL
> > > > > (Nexthop, IpIf->Ip, IpIf->SubnetMask)) ||
> > > > >          IP4_IS_BROADCAST (Ip4GetNetCast (Nexthop, IpIf)))) {
> > > > >
> > > > >      Status = EFI_INVALID_PARAMETER;
> > > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > > > index 1716f43576..bbbcf36eff 100644
> > > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Output.c
> > > > > @@ -309,15 +309,15 @@ Ip4Output (
> > > > >      // Route the packet unless overrided, that is, GateWay isn't
> > zero.
> > > > >      //
> > > > >      if (IpInstance == NULL) {
> > > > > -      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> > Head-
> > > > > >Src);
> > > > > +      CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> > Head-
> > > > >Src,
> > > > > IpIf->SubnetMask);
> > > > >      } else {
> > > > > -      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst,
> > Head-
> > > > >Src);
> > > > > +      CacheEntry = Ip4Route (IpInstance->RouteTable, Head->Dst,
> > Head-
> > > > >Src,
> > > > > IpIf->SubnetMask);
> > > > >        //
> > > > >        // If failed to route the packet by using the instance's
> > route
> > > > table,
> > > > >        // try to use the default route table.
> > > > >        //
> > > > >        if (CacheEntry == NULL) {
> > > > > -        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> > > > Head-
> > > > > >Src);
> > > > > +        CacheEntry = Ip4Route (IpSb->DefaultRouteTable, Head->Dst,
> > > > Head-
> > > > > >Src, IpIf->SubnetMask);
> > > > >        }
> > > > >      }
> > > > >
> > > > > @@ -386,7 +386,8 @@ Ip4Output (
> > > > >                   Fragment,
> > > > >                   GateWay,
> > > > >                   Ip4SysPacketSent,
> > > > > -                 Packet
> > > > > +                 Packet,
> > > > > +                 IpSb
> > > > >                   );
> > > > >
> > > > >        if (EFI_ERROR (Status)) {
> > > > > @@ -429,7 +430,7 @@ Ip4Output (
> > > > >    //    upper layer's packets.
> > > > >    //
> > > > >    Ip4PrependHead (Packet, Head, Option, OptLen);
> > > > > -  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay,
> > Callback,
> > > > > Context);
> > > > > +  Status = Ip4SendFrame (IpIf, IpInstance, Packet, GateWay,
> > Callback,
> > > > > Context, IpSb);
> > > > >
> > > > >    if (EFI_ERROR (Status)) {
> > > > >      goto ON_ERROR;
> > > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > > > index d240d5343a..e648dee908 100644
> > > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.c
> > > > > @@ -494,6 +494,8 @@ Ip4FindRouteEntry (
> > > > >    @param[in]  RtTable               The route table to search from
> > > > >    @param[in]  Dest                  The destination address to
> > search
> > > > for
> > > > >    @param[in]  Src                   The source address to search
> > for
> > > > > +  @param[in]  SubnetMask            The subnet mask of the Src
> > address,
> > > > this
> > > > > field is
> > > > > +                                    used to check if the station is
> > > > using /32 subnet.
> > > > >
> > > > >    @return NULL if failed to route packet, otherwise a route cache
> > > > >            entry that can be used to route packet.
> > > > > @@ -503,7 +505,8 @@ IP4_ROUTE_CACHE_ENTRY *
> > > > >  Ip4Route (
> > > > >    IN IP4_ROUTE_TABLE        *RtTable,
> > > > >    IN IP4_ADDR               Dest,
> > > > > -  IN IP4_ADDR               Src
> > > > > +  IN IP4_ADDR               Src,
> > > > > +  IN IP4_ADDR               SubnetMask
> > > > >    )
> > > > >  {
> > > > >    LIST_ENTRY                *Head;
> > > > > @@ -534,7 +537,10 @@ Ip4Route (
> > > > >    //
> > > > >    RtEntry = Ip4FindRouteEntry (RtTable, Dest);
> > > > >
> > > > > -  if (RtEntry == NULL) {
> > > > > +  //
> > > > > +  // We will always try to use the direct destination address when
> > /32
> > > > subnet
> > > > > mask is used.
> > > > > +  //
> > > > > +  if (RtEntry == NULL && SubnetMask != IP4_ALLONE_ADDRESS) {
> > > > >      return NULL;
> > > > >    }
> > > > >
> > > > > @@ -544,16 +550,23 @@ Ip4Route (
> > > > >    // network. Otherwise, it is an indirect route, the packet will
> > be
> > > > >    // sent to the next hop router.
> > > > >    //
> > > > > -  if ((RtEntry->Flag & IP4_DIRECT_ROUTE) != 0) {
> > > > > +  // When using /32 subnet mask, the packet will always be sent to
> > the
> > > > > direct
> > > > > +  // destination first, if we can't find a matching route cache.
> > > > > +  //
> > > > > +  if (SubnetMask == IP4_ALLONE_ADDRESS || ((RtEntry->Flag &
> > > > > IP4_DIRECT_ROUTE) != 0)) {
> > > > >      NextHop = Dest;
> > > > >    } else {
> > > > >      NextHop = RtEntry->NextHop;
> > > > >    }
> > > > >
> > > > > -  Ip4FreeRouteEntry (RtEntry);
> > > > > +  if (RtEntry != NULL) {
> > > > > +    Ip4FreeRouteEntry (RtEntry);
> > > > > +  }
> > > > >
> > > > >    //
> > > > >    // Create a route cache entry, and tag it as spawned from this
> > route
> > > > entry
> > > > > +  // For /32 subnet mask, the default route in RtEntry will be used
> > if
> > > > failed
> > > > > +  // to send the packet to driect destination address.
> > > > >    //
> > > > >    RtCacheEntry = Ip4CreateRouteCacheEntry (Dest, Src, NextHop,
> > (UINTN)
> > > > > RtEntry);
> > > > >
> > > > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > > > b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > > > index 6269f4ceda..99b9cfbbac 100644
> > > > > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > > > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Route.h
> > > > > @@ -194,6 +194,8 @@ Ip4FreeRouteCacheEntry (
> > > > >    @param[in]  RtTable               The route table to search from
> > > > >    @param[in]  Dest                  The destination address to
> > search
> > > > for
> > > > >    @param[in]  Src                   The source address to search
> > for
> > > > > +  @param[in]  SubnetMask            The subnet mask of the Src
> > address,
> > > > this
> > > > > field is
> > > > > +                                    used to check if the station is
> > > > using /32 subnet.
> > > > >
> > > > >    @return NULL if failed to route packet, otherwise a route cache
> > > > >            entry that can be used to route packet.
> > > > > @@ -203,7 +205,8 @@ IP4_ROUTE_CACHE_ENTRY *
> > > > >  Ip4Route (
> > > > >    IN IP4_ROUTE_TABLE        *RtTable,
> > > > >    IN IP4_ADDR               Dest,
> > > > > -  IN IP4_ADDR               Src
> > > > > +  IN IP4_ADDR               Src,
> > > > > +  IN IP4_ADDR               SubnetMask
> > > > >    );
> > > > >
> > > > >  /**
> > > > > diff --git
> a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > > b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > > index d5a1a8c303..03903640b8 100644
> > > > > --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > > +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > > @@ -509,8 +509,9 @@ Mtftp4Start (
> > > > >      goto ON_ERROR;
> > > > >    }
> > > > >
> > > > > +  gBS->RestoreTPL(OldTpl);
> > > > > +
> > > > >    if (Token->Event != NULL) {
> > > > > -    gBS->RestoreTPL (OldTpl);
> > > > >      return EFI_SUCCESS;
> > > > >    }
> > > > >
> > > > > @@ -522,7 +523,6 @@ Mtftp4Start (
> > > > >      This->Poll (This);
> > > > >    }
> > > > >
> > > > > -  gBS->RestoreTPL (OldTpl);
> > > > >    return Token->Status;
> > > > >
> > > > >  ON_ERROR:
> > > > > @@ -682,7 +682,7 @@ EfiMtftp4Configure (
> > > > >      }
> > > > >
> > > > >      if ((Gateway != 0) &&
> > > > > -        (!IP4_NET_EQUAL (Gateway, Ip, Netmask) || (Netmask != 0
> > > > > && !NetIp4IsUnicast (Gateway, Netmask)))) {
> > > > > +        ((Netmask != 0xFFFFFFFF && !IP4_NET_EQUAL (Gateway, Ip,
> > > > Netmask))
> > > > > || (Netmask != 0 && !NetIp4IsUnicast (Gateway, Netmask)))) {
> > > > >
> > > > >        return EFI_INVALID_PARAMETER;
> > > > >      }
> > > > > --
> > > > > 2.18.0.windows.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask.
  2018-08-28  1:52 ` [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask Fu Siyuan
  2018-08-28 14:55   ` Carsey, Jaben
@ 2018-08-30  8:28   ` Wu, Jiaxin
  2018-08-31  7:46   ` Ye, Ting
  2 siblings, 0 replies; 12+ messages in thread
From: Wu, Jiaxin @ 2018-08-30  8:28 UTC (permalink / raw)
  To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Ye, Ting

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


> -----Original Message-----
> From: Fu, Siyuan
> Sent: Tuesday, August 28, 2018 9:53 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin
> <jiaxin.wu@intel.com>
> Subject: [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit
> subnet mask.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> index 52415e0ad0..e9f644c739 100644
> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
> @@ -1032,6 +1032,7 @@ IfConfigSetInterfaceInfo (
>        SubnetMask  = NTOHL (SubnetMask);
>        TempGateway = NTOHL (TempGateway);
>        if ((SubnetMask != 0) &&
> +          (SubnetMask != 0xFFFFFFFFu) &&
>            !NetIp4IsUnicast (TempGateway, SubnetMask)) {
>          ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_IFCONFIG_INVALID_GATEWAY), gShellNetwork1HiiHandle, VarArg-
> >Arg);
>          ShellStatus = SHELL_INVALID_PARAMETER;
> --
> 2.18.0.windows.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask.
  2018-08-28  1:52 ` [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask Fu Siyuan
  2018-08-28 14:55   ` Carsey, Jaben
  2018-08-30  8:28   ` Wu, Jiaxin
@ 2018-08-31  7:46   ` Ye, Ting
  2 siblings, 0 replies; 12+ messages in thread
From: Ye, Ting @ 2018-08-31  7:46 UTC (permalink / raw)
  To: Fu, Siyuan, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Wu, Jiaxin

Reveiewed-by: Ye Ting <ting.ye@intel.com> 

-----Original Message-----
From: Fu, Siyuan 
Sent: Tuesday, August 28, 2018 9:53 AM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Ye, Ting <ting.ye@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
Subject: [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
---
 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
index 52415e0ad0..e9f644c739 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
@@ -1032,6 +1032,7 @@ IfConfigSetInterfaceInfo (
       SubnetMask  = NTOHL (SubnetMask);
       TempGateway = NTOHL (TempGateway);
       if ((SubnetMask != 0) &&
+          (SubnetMask != 0xFFFFFFFFu) && 
           !NetIp4IsUnicast (TempGateway, SubnetMask)) {
         ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_IFCONFIG_INVALID_GATEWAY), gShellNetwork1HiiHandle, VarArg->Arg);
         ShellStatus = SHELL_INVALID_PARAMETER;
-- 
2.18.0.windows.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-08-31  7:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-28  1:52 [Patch 0/2] Add 32bit subnet mask support for IP4 PXE Fu Siyuan
2018-08-28  1:52 ` [Patch 1/2] MdeModulePkg/Network: Add 32bit subnet mask support for IP4 PXE boot Fu Siyuan
2018-08-28  6:50   ` Wu, Jiaxin
2018-08-28  7:12     ` Fu, Siyuan
2018-08-28  8:52       ` Wu, Jiaxin
2018-08-28  9:41         ` Fu, Siyuan
2018-08-29  2:09           ` Wu, Jiaxin
2018-08-28  1:52 ` [Patch 2/2] ShellPkg: Update Ifconfig command to accept 32bit subnet mask Fu Siyuan
2018-08-28 14:55   ` Carsey, Jaben
2018-08-29  1:37     ` Ni, Ruiyu
2018-08-30  8:28   ` Wu, Jiaxin
2018-08-31  7:46   ` Ye, Ting

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox