[PATCH v6 06/14] efi_loader: efi_net: add efi_net_set_addr, efi_net_get_addr

[Jerome Forissier]

On 12/3/24 11:51, Heinrich Schuchardt wrote:
> On 28.11.24 15:44, Adriano Cordova 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 v5)
>> Changes in v5:
>> - When possible use sizeof in memcpy instead of hardcoded value
>>
>> Changes in v4:
>>   - Removed unnecessary casts
>>   - Changed char *ipstr by char ipstr[].
>>
>> (no changes since v2)
>>   include/efi_loader.h     |  16 +++++
>>   lib/efi_loader/efi_net.c | 125 +++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 141 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..3491d4c481 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,127 @@ out_of_resources:
>>       printf("ERROR: Out of memory\n");
>>       return EFI_OUT_OF_RESOURCES;
>>   }
>> +
>> +/**
>> + * efi_net_get_addr() - get IP address information
>> + *
>> + * Copy the current IP address, mask, and gateway into the
>> + * efi_ipv4_address structs pointed to by ip, mask and gw,
>> + * respectively.
>> + *
>> + * @ip:        pointer to an efi_ipv4_address struct to
>> + *        be filled with the current IP address
>> + * @mask:    pointer to an efi_ipv4_address struct to
>> + *        be filled with the current network mask
>> + * @gw:        pointer to an efi_ipv4_address struct to be
>> + *        filled with the current network gateway
>> + */
>> +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";
>> +    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) {
>> +        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(ip, &tmp, sizeof(tmp));
>> +    }
>> +
>> +    env = env_get(maskstr);
>> +    if (env && mask) {
>> +        tmp = string_to_ip(env);
>> +        memcpy(mask, &tmp, sizeof(tmp));
>> +    }
>> +    env = env_get(gwstr);
>> +    if (env && gw) {
>> +        tmp = string_to_ip(env);
>> +        memcpy(gw, &tmp, sizeof(tmp));
>> +    }
>> +#else
>> +    if (ip)
>> +        memcpy(ip, &net_ip, sizeof(net_ip));
>> +    if (mask)
>> +        memcpy(mask, &net_netmask, sizeof(net_netmask));
>> +#endif
>> +}
>> +
>> +/**
>> + * efi_net_set_addr() - set IP address information
>> + *
>> + * Set the current IP address, mask, and gateway to the
>> + * efi_ipv4_address structs pointed to by ip, mask and gw,
>> + * respectively.
>> + *
>> + * @ip:        pointer to new IP address
>> + * @mask:    pointer to new network mask to set
>> + * @gw:        pointer to new network gateway
>> + */
>> +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;
>> +    }
> 
> The same logic can be found in LWIP's dhcp_loop(), get_udev_ipv4_info().
> 
> After this series I think it is time to carve it out into a function.
> 
> It is unclear to me form where the idx > 99 in the LWIP stack is
> derived. struct udevice device defines seq_ as integer.
> 
>> +
>> +    if (idx) {
>> +        sprintf(ipstr, "ipaddr%d", idx);
>> +        sprintf(maskstr, "netmask%d", idx);
>> +        sprintf(gwstr, "gatewayip%d", idx);
> 
> @Jerome,
> 
> The usage of environment variables ipaddr#, netmask#, gatewayip# by the
> LWIP stack should be documented in doc/usage/environment.rst. Could you,
> please, provide a patch.

https://lists.denx.de/pipermail/u-boot/2024-December/573696.html

Thanks!

-- 
Jerome

> 
> For this patch
> 
> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> 
>> +    }
>> +
>> +    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(&net_ip, ip, sizeof(*ip));
>> +    if (mask)
>> +        memcpy(&net_netmask, mask, sizeof(*mask));
>> +#endif
>> +}
>