From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x232.google.com (mail-it0-x232.google.com [IPv6:2607:f8b0:4001:c0b::232]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2B55721E977EA for ; Wed, 6 Sep 2017 01:00:26 -0700 (PDT) Received: by mail-it0-x232.google.com with SMTP id f199so12458235ita.1 for ; Wed, 06 Sep 2017 01:03:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=iJjEtDKUJBXU0JS6DZj5W10C5bdny3aVJQR0YbKE1fE=; b=VcZvV4k51eeQPYVHmJm+3GCwP14hapNpHLOa0SQFxwqF7PIOZj2jd81BwMGteW/mDL nzOi5X7/qy0vRNt0kzSiw7vBqJ9zh7bz40n9p2FSTe4JYkXAP95fiLQ9X9l5KRHjDjF3 Cp9Gf9c0mHWTc0+kYaf9Qz7n8K5Pxie/dbAok= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=iJjEtDKUJBXU0JS6DZj5W10C5bdny3aVJQR0YbKE1fE=; b=OCgXRbjFSHCM+8tRJnwei99j/ITXRhmyepf3VcAsC0oWsdyP9M+CjiEV+5aCnOrJjf 2wAEwHwZrB1Sv0TwovJL+Zil7JH2TUdy1jvo1wrf8teff0GEtBj8t5078ZBbPKEnQNu2 AyuFA9rALyQIdAvNgFA2jaGx/f+jOe0Kr1do7r/rigP5pSv6fOQrcE30hj6ydL/LLXj9 9AaVvQK7IakQXvp4cduNAp7ql9ECc7HOk97b4QCH7iu86+5rEd7XzPVh30aBhZDue1jc XkOcZjpWSlOShpJ8MGL76fimX7IYteItTIEzvLcxOnyzII0zAq7cSrcae0c50baMdsUi gFig== X-Gm-Message-State: AHPjjUgGuwFUJeoO4PWg2ayft6EnrwGzjdfHOX5T2a/RBz1zJfkFM9ji 4xskOvFMwGgtKjrtiX9RvDWPYBYparqt X-Google-Smtp-Source: ADKCNb6SK4sTaK48cJt1Oa+RGB3LcERfqL1ROzN0JFzDfh8184rfJTUBnfF5tkLuvAOSGAEbMduvz7KdWQeOev7A+0I= X-Received: by 10.36.150.198 with SMTP id z189mr1907822itd.63.1504684994745; Wed, 06 Sep 2017 01:03:14 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.162.1 with HTTP; Wed, 6 Sep 2017 01:03:14 -0700 (PDT) In-Reply-To: <20170904080702.14488-2-siyuan.fu@intel.com> References: <20170904080702.14488-1-siyuan.fu@intel.com> <20170904080702.14488-2-siyuan.fu@intel.com> From: Ard Biesheuvel Date: Wed, 6 Sep 2017 09:03:14 +0100 Message-ID: To: Fu Siyuan Cc: "edk2-devel@lists.01.org" , Ye Ting , Wu Jiaxin Subject: Re: [Patch 1/2] MdeModulePkg/Ip4Dxe: fix a bug in IP4 driver for IpSec protocol notify. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Sep 2017 08:00:26 -0000 Content-Type: text/plain; charset="UTF-8" Hello Fu, On 4 September 2017 at 09:07, Fu Siyuan wrote: > The IP driver uses EfiCreateProtocolNotifyEvent() to register notify callback > function for IpSec protocol, but it didn't notice that the callback will always > be executed at least once, even the protocol wasn't in handle database. > As a result, the Ip4IpSecProcessPacket() will still always call LocateProtocol() > even the IpSec protocol is not installed, which will impact the network > performance. > > Cc: Ye Ting > Cc: Wu Jiaxin > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Fu Siyuan > --- > MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c | 14 +++++++++++--- > MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c | 10 ++-------- > 2 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c > index 792db5c..03ba458 100644 > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c > @@ -41,12 +41,20 @@ IpSec2InstalledCallback ( > IN VOID *Context > ) > { > + EFI_STATUS Status; > // > - // Close the event so it does not get called again. > + // Test if protocol was even found. > + // Notification function will be called at least once. > // > - gBS->CloseEvent (Event); > + Status = gBS->LocateProtocol (&gEfiIpSec2ProtocolGuid, NULL, &mIpSec); While moving this call here from below, you removed the (void **) cast, which breaks the build on GCC In function 'IpSec2InstalledCallback': 49:3: error: passing argument 3 of 'gBS->LocateProtocol' from incompatible pointer type [-Werror] Status = gBS->LocateProtocol (&gEfiIpSec2ProtocolGuid, NULL, &mIpSec); ^ 49:3: note: expected 'void **' but argument is of type 'struct EFI_IPSEC2_PROTOCOL **' cc1: all warnings being treated as errors Please fix. Kind regards, Ard, > + if (Status == EFI_SUCCESS && mIpSec != NULL) { > + // > + // Close the event so it does not get called again. > + // > + gBS->CloseEvent (Event); > > - mIpSec2Installed = TRUE; > + mIpSec2Installed = TRUE; > + } > } > > /** > diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c > index 09b8f2b..e694323 100644 > --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c > +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Input.c > @@ -1,7 +1,7 @@ > /** @file > IP4 input process. > > -Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved.
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
> (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
> > This program and the accompanying materials > @@ -518,6 +518,7 @@ Ip4IpSecProcessPacket ( > if (!mIpSec2Installed) { > goto ON_EXIT; > } > + ASSERT (mIpSec != NULL); > > Packet = *Netbuf; > RecycleEvent = NULL; > @@ -527,13 +528,6 @@ Ip4IpSecProcessPacket ( > FragmentCount = Packet->BlockOpNum; > > ZeroMem (&ZeroHead, sizeof (IP4_HEAD)); > - > - if (mIpSec == NULL) { > - gBS->LocateProtocol (&gEfiIpSec2ProtocolGuid, NULL, (VOID **) &mIpSec); > - if (mIpSec == NULL) { > - goto ON_EXIT; > - } > - } > > // > // Check whether the IPsec enable variable is set. > -- > 1.9.5.msysgit.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel