| Message ID | 20241111210959.560738-8-adrianox@gmail.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: efi http and ipconfig drivers | expand |
Hi Adriano, On Mon, 11 Nov 2024 at 23:10, Adriano Cordova <adrianox@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@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
Hi Ilias, El lun, 18 nov 2024 a las 9:22, Ilias Apalodimas (< ilias.apalodimas@linaro.org>) escribió: > Hi Adriano, > > On Mon, 11 Nov 2024 at 23:10, Adriano Cordova <adrianox@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@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. 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
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@linaro.org>) escribió: > >> Hi Adriano, >> >> On Mon, 11 Nov 2024 at 23:10, Adriano Cordova <adrianox@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@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,
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"; + 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((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); +#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 +}
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@gmail.com> --- (no changes since v2) include/efi_loader.h | 16 +++++++ lib/efi_loader/efi_net.c | 100 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+)