public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Fu, Siyuan" <siyuan.fu@intel.com>,
	"edk2-devel@lists.01.org" <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.
Date: Tue, 28 Aug 2018 08:52:56 +0000	[thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B72741648E304@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <B1FF2E9001CE9041BD10B825821D5BC58B5DED3D@SHSMSX103.ccr.corp.intel.com>


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



  reply	other threads:[~2018-08-28  8:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=895558F6EA4E3B41AC93A00D163B72741648E304@SHSMSX103.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox