From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1nf1xH-0004uT-VF for mharc-grub-devel@gnu.org; Thu, 14 Apr 2022 12:04:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:45428) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nf1xF-0004rm-2q for grub-devel@gnu.org; Thu, 14 Apr 2022 12:04:21 -0400 Received: from dibed.net-space.pl ([84.10.22.86]:34685) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_3DES_EDE_CBC_SHA1:192) (Exim 4.90_1) (envelope-from ) id 1nf1xC-0001uS-Hy for grub-devel@gnu.org; Thu, 14 Apr 2022 12:04:20 -0400 Received: from router-fw.i.net-space.pl ([192.168.52.1]:52422 "EHLO tomti.i.net-space.pl") by router-fw-old.i.net-space.pl with ESMTP id S2135887AbiDNQEM (ORCPT ); Thu, 14 Apr 2022 18:04:12 +0200 X-Comment: RFC 2476 MSA function at dibed.net-space.pl logged sender identity as: dkiper Date: Thu, 14 Apr 2022 18:04:10 +0200 From: Daniel Kiper To: Chad Kimes Cc: grub-devel@gnu.org Subject: Re: [PATCH v2 2/2] Configure VLAN from UEFI device used for PXE Message-ID: <20220414160410.rymfiehznootd2ld@tomti.i.net-space.pl> References: <20220321220732.557752-1-chkimes@github.com> <20220321220732.557752-3-chkimes@github.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220321220732.557752-3-chkimes@github.com> User-Agent: NeoMutt/20170113 (1.7.2) Received-SPF: pass client-ip=84.10.22.86; envelope-from=dkiper@net-space.pl; helo=dibed.net-space.pl X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Apr 2022 16:04:21 -0000 On Mon, Mar 21, 2022 at 06:07:32PM -0400, Chad Kimes via Grub-devel wrote: I would prefer if you copy some text from the cover letter here. > Signed-off-by: Chad Kimes > --- > grub-core/net/drivers/efi/efinet.c | 38 ++++++++++++++++++++++++++---- > 1 file changed, 33 insertions(+), 5 deletions(-) > > diff --git a/grub-core/net/drivers/efi/efinet.c b/grub-core/net/drivers/efi/efinet.c > index 381c138db..107e1f09e 100644 > --- a/grub-core/net/drivers/efi/efinet.c > +++ b/grub-core/net/drivers/efi/efinet.c > @@ -339,6 +339,10 @@ grub_efi_net_config_real (grub_efi_handle_t hnd, char **device, > { > struct grub_net_card *card; > grub_efi_device_path_t *dp; > + struct grub_net_network_level_interface *inter; > + grub_efi_device_path_t *vlan_dp; > + grub_efi_uint16_t vlan_dp_len; > + grub_efi_vlan_device_path_t *vlan; > > dp = grub_efi_get_device_path (hnd); > if (! dp) > @@ -387,11 +391,35 @@ grub_efi_net_config_real (grub_efi_handle_t hnd, char **device, > if (! pxe) > continue; > pxe_mode = pxe->mode; > - grub_net_configure_by_dhcp_ack (card->name, card, 0, > - (struct grub_net_bootp_packet *) > - &pxe_mode->dhcp_ack, > - sizeof (pxe_mode->dhcp_ack), > - 1, device, path); > + > + inter = grub_net_configure_by_dhcp_ack (card->name, card, 0, > + (struct grub_net_bootp_packet *) > + &pxe_mode->dhcp_ack, > + sizeof (pxe_mode->dhcp_ack), > + 1, device, path); > + > + if (inter) I prefer "if (inter != NULL)". > + { > + /* > + * search the device path for any VLAN subtype and use it > + * to configure the interface > + */ > + vlan_dp = dp; > + > + while (!GRUB_EFI_END_ENTIRE_DEVICE_PATH (vlan_dp)) > + { > + if (GRUB_EFI_DEVICE_PATH_TYPE (vlan_dp) == GRUB_EFI_MESSAGING_DEVICE_PATH_TYPE > + && GRUB_EFI_DEVICE_PATH_SUBTYPE (vlan_dp) == GRUB_EFI_VLAN_DEVICE_PATH_SUBTYPE) > + { > + vlan = (grub_efi_vlan_device_path_t *) vlan_dp; > + inter->vlantag = vlan->vlan_id; > + break; > + } > + > + vlan_dp_len = GRUB_EFI_DEVICE_PATH_LENGTH (vlan_dp); > + vlan_dp = (grub_efi_device_path_t *) ((char *) vlan_dp + vlan_dp_len); s/char */grub_uint8_t */ even if that translates to almost the same type. Simply I think grub_uint8_t is more natural here. Anyway, I will fix all these minor issues this time for you. So, Reviewed-by: Daniel Kiper Thank you for adding this feature! Daniel