From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lj1-f178.google.com (mail-lj1-f178.google.com [209.85.208.178]) by mx.groups.io with SMTP id smtpd.web10.2986.1595524724410154888 for ; Thu, 23 Jul 2020 10:18:45 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@broadcom.com header.s=google header.b=Hj1BXvyl; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: broadcom.com, ip: 209.85.208.178, mailfrom: vladimir.olovyannikov@broadcom.com) Received: by mail-lj1-f178.google.com with SMTP id x9so7182128ljc.5 for ; Thu, 23 Jul 2020 10:18:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=from:references:in-reply-to:mime-version:thread-index:date :message-id:subject:to:cc; bh=m7YbxYTlP0T7XHfRXq1d5HxHBLmmA7Fvx91YMHPcPpM=; b=Hj1BXvyluB8ZO/f3xqzjQqnS+dmhI6bHSraF75D5CBGpJh4qopNFIfWkvdOk1px7gf eKN3WcO+rsQJJjwYmqisG81/alTmE3wFVl7GVfsJYdPOmsBbJox9EUVzCHIqNJ8TyL89 +qObTXR3D6ghYEKnNhalSnku0l6BRdF5EcJJU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:references:in-reply-to:mime-version :thread-index:date:message-id:subject:to:cc; bh=m7YbxYTlP0T7XHfRXq1d5HxHBLmmA7Fvx91YMHPcPpM=; b=M8EMuoSuFScZ7JgHnRAdC+aOA2BJiux+eXuSSMRI0AzuBiXyUZvsMFu8+RtxGXp3+v eqxUck9sY69aEWjRs2lJdbUuSMkUZeKiirZvddb2v5EDCrYcUDBJyKAiaskizh7rUyx7 zzw2+6x9Xz9P6gS3ipSnfSyUbxu2SDr9lCfGBjenJTtdvtKDhgGXJDUy3l6WAzQVjMVG XQNvKWpzY3tpCVnHLTCE2acTyU66T9jzZQeZK2epnMlM6UUC/xjzHnaaAIAS+KxOP2bi GFzTiS9YgxGCI5YKmYtGaowwAIah0oK8hgK/N1Cyx8gVD3vIZlZ61i/kEO/VaKLN3TtN KI9w== X-Gm-Message-State: AOAM532Cj/sYhWu73ScwDsabw3FILwnB7NR+e03vTBqbipw+qAM8cUOQ vYgMM7qcLGMIn6EgEamxIkR64xE1vd4dxhEzTmYJpw== X-Google-Smtp-Source: ABdhPJzl7FiqQ6HG36hjk/f5CtPH9mUN+k8Ux2Omz4Jj8C+1el5a6OVHbkEGSryAy9vacVku+eeFzba8Yx9BaO/t1UM= X-Received: by 2002:a2e:7804:: with SMTP id t4mr2433440ljc.8.1595524722477; Thu, 23 Jul 2020 10:18:42 -0700 (PDT) From: "Vladimir Olovyannikov" References: <20200713183137.9825-1-vladimir.olovyannikov@broadcom.com> <6b8ae026-a39c-3b68-4280-fc532b5954d6@redhat.com> In-Reply-To: <6b8ae026-a39c-3b68-4280-fc532b5954d6@redhat.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQHWOTapeOQNV4NQfAeaKbZPaHyXkQFPgG3JApDrki+o9rD74A== Date: Thu, 23 Jul 2020 10:18:39 -0700 Message-ID: <8de76fe2aa9ca0f8ab45fe8245428453@mail.gmail.com> Subject: Re: [edk2-devel] [PATCH v3 1/1] ShellPkg/DynamicCommand: add HttpDynamicCommand To: Laszlo Ersek , devel@edk2.groups.io Cc: Samer El-Haj-Mahmoud , Zhichao Gao , Maciej Rabeda , Jiaxin Wu , Siyuan Fu , Ray Ni , Liming Gao , Nd Content-Type: text/plain; charset="UTF-8" Hi Laszlo, Thank you for testing! > -----Original Message----- > From: Laszlo Ersek > Sent: Thursday, July 23, 2020 8:03 AM > To: devel@edk2.groups.io; vladimir.olovyannikov@broadcom.com > Cc: Samer El-Haj-Mahmoud ; Zhichao > Gao ; Maciej Rabeda > ; Jiaxin Wu ; Siyuan > Fu ; Ray Ni ; Liming Gao > ; Nd > Subject: Re: [edk2-devel] [PATCH v3 1/1] ShellPkg/DynamicCommand: add > HttpDynamicCommand > > On 07/23/20 16:54, Laszlo Ersek wrote: > > On 07/13/20 20:31, Vladimir Olovyannikov via groups.io wrote: > >> Introduce an http client utilizing EDK2 HTTP protocol, to allow fast > >> image downloading from http/https servers. > >> HTTP download speed is usually faster than tftp. > >> The client is based on the same approach as tftp dynamic command, and > >> uses the same UEFI Shell command line parameters. This makes it easy > >> integrating http into existing UEFI Shell scripts. > >> Note that to enable HTTP download, feature Pcd > >> gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections must be set > to > >> TRUE. > >> > >> Signed-off-by: Vladimir Olovyannikov > >> > >> Tested-By: Samer El-Haj-Mahmoud > >> Cc: Zhichao Gao > >> Cc: Maciej Rabeda > >> Cc: Jiaxin Wu > >> Cc: Siyuan Fu > >> Cc: Ray Ni > >> Cc: Liming Gao > >> Cc: Nd > >> --- > >> .../DynamicCommand/HttpDynamicCommand/Http.c | 1700 > +++++++++++++++++ > >> .../DynamicCommand/HttpDynamicCommand/Http.h | 84 + > >> .../HttpDynamicCommand/Http.uni | 113 ++ > >> .../HttpDynamicCommand/HttpApp.c | 53 + > >> .../HttpDynamicCommand/HttpApp.inf | 58 + > >> .../HttpDynamicCommand/HttpDynamicCommand.c | 134 ++ > >> .../HttpDynamicCommand/HttpDynamicCommand.inf | 63 + > >> ShellPkg/Include/Guid/ShellLibHiiGuid.h | 5 + > >> ShellPkg/ShellPkg.dec | 1 + > >> ShellPkg/ShellPkg.dsc | 5 + > >> 10 files changed, 2216 insertions(+) create mode 100644 > >> ShellPkg/DynamicCommand/HttpDynamicCommand/Http.c > >> create mode 100644 > ShellPkg/DynamicCommand/HttpDynamicCommand/Http.h > >> create mode 100644 > >> ShellPkg/DynamicCommand/HttpDynamicCommand/Http.uni > >> create mode 100644 > >> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.c > >> create mode 100644 > >> ShellPkg/DynamicCommand/HttpDynamicCommand/HttpApp.inf > >> create mode 100644 > >> > ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand > .c > >> create mode 100644 > >> > ShellPkg/DynamicCommand/HttpDynamicCommand/HttpDynamicCommand > .inf > > > > I tested this, in an OVMF IA32X64 VM, and in an ArmVirtQemu (AARCH64) > VM. > > > > I tested HTTP download, HTTPS download, and also willfully triggered a > > Volume Full failure (checked free space on the FS with "vol" first, > > then attempted to download a larger file than that). Also attempted a > > download to read-only media (refused correctly with "Write Protected"). > > > > I tested FQDNs and IPv4 addresses in the URLs. > > > > I used "wget-style" URLs, mainly. > > > > For setting the CA certificates, I used the method described in > > OvmfPkg/README, in section "HTTPS Boot". > > > > Compared the local files downloaded by this command vs. local files > > downloaded from the same URLs using other tools (browser, wget etc). I > > used a few hundred KB large files for this. > > > > One suggestion: if the download fails for some reason, but the local > > file creation succeeded, then upon exit, the local file should be > > deleted. Otherwise an incomplete (possibly zero size) file is left in > > the filesystem. I was thinking about this. Sometimes, an error file returned by HTTP server (say, on 404 or 503) has more information than just 404, 503, etc. That was the only reason I do not delete it. I still return error code to the caller, so the caller can read it (maybe, to provide more information to the user), and/or delete it right away. What do you think? > > > > Tested-by: Laszlo Ersek > > Another comment relating to usage. (This is by no means an observation for > the code, at best it could be included in some developer documentation or > in > some wiki page.) > > Having access to the DebugLib outputs of the drivers that provide the > underlying protocols for this shell command is very useful. For example > those > logs can report certificate validation issues. > > Such access is usually not difficult with virtual machines, but on > physical > machines, it could be more challenging. I guess one way around that is to > use > such DebugLib instances in the platform firmware (i.e. in the underlying > protocol drivers) that write to the UEFI consoles. That tends to mess up > the > display in the shell window, but it does provide more information. I agree. Note that usually those libs have DEBUG(()) messages which are compiled out in RELEASE builds. > > Thanks > Laszlo