[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/4] efinet: UEFI IPv6 PXE support

From: Javier Martinez Canillas
Subject: Re: [PATCH 3/4] efinet: UEFI IPv6 PXE support
Date: Fri, 5 Jun 2020 14:05:24 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0

[adding Peter Jones as Cc that I forgot when sending the patches]

On 6/5/20 4:15 AM, Michael Chang wrote:
> On Thu, Jun 04, 2020 at 01:37:52PM +0200, Thomas Frauendorfer wrote:
>> Hi,
>> You replace the 'unused[52]' field before dhcp_discover with 51 bytes.
>> While the UEFI spec also defines the EFI_PXE_BASE_CODE_MODE struct in
>> this way it also specifies that an EFI_IP_ADDRESS is 16-byte buffer
>> aligned on a 4-byte boundary.
> True.
>> So there should probably be a grub_efi_uint8_t between tos and
>> station_ip to ensure the correct alignment
> You're probably right if the data type for `station_ip` is
> `grub_efi_pxe_ip_address_t`, but here it is `grub_efi_ip_address_t` declared
> as:
>   typedef grub_uint8_t grub_efi_ip_address_t[8] __attribute__ ((aligned(4)));
> So the compiler would have been taking care of the alignment already ...

Right, I don't think we need an explicit padding here for that reason.

> By the way, I found the mentioned hunk is different to what was posted on the
> list[1], which had relevant fields like this:
>   grub_uint8_t using_ipv6;
>   grub_uint8_t unused[16];
>   grub_efi_pxe_ip_address_t station_ip;
>   grub_efi_pxe_ip_address_t subnet_mask;
> Maybe Javier could help to shed some light on why the change was made ? Though
> I'm not against it, I'm still interested to know about it if they have any
> other concern or in case anything could be missing here. :)
> [1]

Yes, I decided to post the patches that we were carrying in our package, even
when they diverge a little bit from your original patches. I did that because
is what we have been testing for years and also contain some fixes for issues
found by Peter.

He was also the one who changed the definitions of these structures. I think
that was to make them more complete and closer to the spec, but he can
comment if I got the intention wrong.

I can post your original patches and then the squashed changes separately as
a follow-up if you prefer it that way.

Best regards,
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat

reply via email to

[Prev in Thread] Current Thread [Next in Thread]