[PATCH v3 07/15] efi_loader: efi_net: add efi_net_set_addr, efi_net_get_addr
Jerome Forissier
jerome.forissier at linaro.org
Mon Nov 18 14:45:45 CET 2024
On 11/18/24 13:35, Adriano Córdova wrote:
> Hi Ilias,
>
> El lun, 18 nov 2024 a las 9:22, Ilias Apalodimas (<
> ilias.apalodimas at linaro.org>) escribió:
>
>> Hi Adriano,
>>
>> On Mon, 11 Nov 2024 at 23:10, Adriano Cordova <adrianox at gmail.com> wrote:
>>>
>>> Add the functions efi_net_set_addr and efi_net_get_addr to set
>>> and get the ip address from efi code in a network agnostic way.
>>> This could also go in net_common, or be compiled conditionally
>>> for each network stack.
>>>
>>> Signed-off-by: Adriano Cordova <adrianox at gmail.com>
>>> ---
>>>
>>> (no changes since v2)
>>>
>>> include/efi_loader.h | 16 +++++++
>>> lib/efi_loader/efi_net.c | 100 +++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 116 insertions(+)
>>>
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index 39809eac1b..612bc42816 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -125,6 +125,22 @@ static inline void efi_set_bootdev(const char *dev,
>> const char *devnr,
>>> size_t buffer_size) { }
>>> #endif
>>>
>>> +#if CONFIG_IS_ENABLED(NETDEVICES) && CONFIG_IS_ENABLED(EFI_LOADER)
>>> +void efi_net_get_addr(struct efi_ipv4_address *ip,
>>> + struct efi_ipv4_address *mask,
>>> + struct efi_ipv4_address *gw);
>>> +void efi_net_set_addr(struct efi_ipv4_address *ip,
>>> + struct efi_ipv4_address *mask,
>>> + struct efi_ipv4_address *gw);
>>> +#else
>>> +static inline void efi_net_get_addr(struct efi_ipv4_address *ip,
>>> + struct efi_ipv4_address *mask,
>>> + struct efi_ipv4_address *gw) { }
>>> +static inline void efi_net_set_addr(struct efi_ipv4_address *ip,
>>> + struct efi_ipv4_address *mask,
>>> + struct efi_ipv4_address *gw) { }
>>> +#endif
>>> +
>>> /* Maximum number of configuration tables */
>>> #define EFI_MAX_CONFIGURATION_TABLES 16
>>>
>>> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
>>> index 7cd536705f..dd84b3f18d 100644
>>> --- a/lib/efi_loader/efi_net.c
>>> +++ b/lib/efi_loader/efi_net.c
>>> @@ -17,6 +17,7 @@
>>>
>>> #include <efi_loader.h>
>>> #include <malloc.h>
>>> +#include <vsprintf.h>
>>> #include <net.h>
>>>
>>> static const efi_guid_t efi_net_guid = EFI_SIMPLE_NETWORK_PROTOCOL_GUID;
>>> @@ -997,3 +998,102 @@ out_of_resources:
>>> printf("ERROR: Out of memory\n");
>>> return EFI_OUT_OF_RESOURCES;
>>> }
>>> +
>>> +void efi_net_get_addr(struct efi_ipv4_address *ip,
>>> + struct efi_ipv4_address *mask,
>>> + struct efi_ipv4_address *gw)
>>> +{
>>> +#ifdef CONFIG_NET_LWIP
>>> + char *ipstr = "ipaddr\0\0";
>>> + char *maskstr = "netmask\0\0";
>>> + char *gwstr = "gatewayip\0\0";
>>
>> You generally can't define strings like that and expect to change
>> them. The compiler places the string literal somewhere in memory which
>> is immutable.
>> If you want to keep the code as is, you need to define these as ipstr[].
>>
>>> + int idx;
>>> + struct in_addr tmp;
>>> + char *env;
>>> +
>>> + idx = dev_seq(eth_get_dev());
>>> +
>>> + if (idx < 0 || idx > 99) {
>>> + log_err("unexpected idx %d\n", idx);
>>> + return;
>>> + }
>>> +
>>> + if (idx) {
>>
>> Is 0 not legal here?
>> And why is the behavior differnt with and without LWIP?
>>
>
> This is how the lwip code deals with the environment variables for the IP
> address, see
> get_udev_ipv4_info in
> https://github.com/u-boot/u-boot/blob/a38390284ad4261723d3a2411ba988828e994535/net/lwip/net-lwip.c#L91
> In here I am trying to be consistent with that.
It's a mistake and an oversight on my part. Ilias is right. I will send a
patch to fix lwIP.
Thanks,
--
Jerome
> In legacy the network stack
> just uses the variables net_ip and net_netmask.
>
>
>>> + sprintf(ipstr, "ipaddr%d", idx);
>>> + sprintf(maskstr, "netmask%d", idx);
>>> + sprintf(gwstr, "gatewayip%d", idx);
>>> + }
>>> +
>>> + env = env_get(ipstr);
>>> + if (env && ip) {
>>> + tmp = string_to_ip(env);
>>> + memcpy((void *)ip, (void *)&tmp, 4);
>>> + }
>>> +
>>> + env = env_get(maskstr);
>>> + if (env && mask) {
>>> + tmp = string_to_ip(env);
>>> + memcpy((void *)mask, (void *)&tmp, 4);
>>> + }
>>> + env = env_get(gwstr);
>>> + if (env && gw) {
>>> + tmp = string_to_ip(env);
>>> + memcpy((void *)gw, (void *)&tmp, 4);
>>> + }
>>> +#else
>>> + if (ip)
>>> + memcpy((void *)ip, (void *)&net_ip, 4);
>>> + if (mask)
>>> + memcpy((void *)mask, (void *)&net_netmask, 4);
>>
>> Get rid of casts etc.
>>
>>> +#endif
>>> +}
>>> +
>>> +void efi_net_set_addr(struct efi_ipv4_address *ip,
>>> + struct efi_ipv4_address *mask,
>>> + struct efi_ipv4_address *gw)
>>> +{
>>> +#ifdef CONFIG_NET_LWIP
>>> + char *ipstr = "ipaddr\0\0";
>>> + char *maskstr = "netmask\0\0";
>>> + char *gwstr = "gatewayip\0\0";
>>> + int idx;
>>> + struct in_addr *addr;
>>> + char tmp[46];
>>> +
>>> + idx = dev_seq(eth_get_dev());
>>> +
>>> + if (idx < 0 || idx > 99) {
>>> + log_err("unexpected idx %d\n", idx);
>>> + return;
>>> + }
>>> +
>>> + if (idx) {
>>> + sprintf(ipstr, "ipaddr%d", idx);
>>> + sprintf(maskstr, "netmask%d", idx);
>>> + sprintf(gwstr, "gatewayip%d", idx);
>>> + }
>>> +
>>> + if (ip) {
>>> + addr = (struct in_addr *)ip;
>>> + ip_to_string(*addr, tmp);
>>> + env_set(ipstr, tmp);
>>> + }
>>> +
>>> + if (mask) {
>>> + addr = (struct in_addr *)mask;
>>> + ip_to_string(*addr, tmp);
>>> + env_set(maskstr, tmp);
>>> + }
>>> +
>>> + if (gw) {
>>> + addr = (struct in_addr *)gw;
>>> + ip_to_string(*addr, tmp);
>>> + env_set(gwstr, tmp);
>>> + }
>>> +#else
>>> + if (ip)
>>> + memcpy((void *)&net_ip, (void *)ip, 4);
>>> + if (mask)
>>> + memcpy((void *)&net_netmask, (void *)mask, 4);
>>> +#endif
>>> +}
>>> --
>>> 2.43.0
>>>
>>
>> Cheers
>> /Ilias
>>
>
> Thanks,
> Adriano
>
More information about the U-Boot
mailing list