[PATCH v3 07/15] efi_loader: efi_net: add efi_net_set_addr, efi_net_get_addr

[Ilias Apalodimas]

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?


> +               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