[SLOF] [PATCH v2 07/11] libnet: Wire up pxelinux.cfg network booting

[Thomas Huth]

On 24.05.2018 11:01, Alexey Kardashevskiy wrote:
> On 19/5/18 1:45 am, Thomas Huth wrote:
>> In case the normal network loading failed, try to load a pxelinux.cfg
>> config file. If that succeeds, load the kernel and initrd with the
>> information that could be found in this file.
>>
>> Signed-off-by: Thomas Huth <thuth at redhat.com>
>> ---
>>  include/helpers.h    |  2 ++
>>  lib/libnet/netload.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  slof/helpers.c       | 15 ++++++++++---
>>  3 files changed, 73 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/helpers.h b/include/helpers.h
>> index 04ee771..9dfe3ae 100644
>> --- a/include/helpers.h
>> +++ b/include/helpers.h
>> @@ -36,6 +36,8 @@ extern void SLOF_pci_config_write16(long offset, long value);
>>  extern void SLOF_pci_config_write8(long offset, long value);
>>  extern void *SLOF_translate_my_address(void *addr);
>>  extern int write_mm_log(char *data, unsigned int len, unsigned short type);
>> +extern void SLOF_set_chosen_int(const char *s, long val);
>> +extern void SLOF_set_chosen_bytes(const char *s, const char *addr, size_t size);
>>  extern void SLOF_encode_bootp_response(void *addr, size_t size);
>>  extern void SLOF_encode_dhcp_response(void *addr, size_t size);
>>  
>> diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c
>> index ba1008c..2a2a7c5 100644
>> --- a/lib/libnet/netload.c
>> +++ b/lib/libnet/netload.c
>> @@ -26,6 +26,7 @@
>>  #include <helpers.h>
>>  #include "args.h"
>>  #include "netapps.h"
>> +#include "pxelinux.h"
>>  
>>  #define IP_INIT_DEFAULT 5
>>  #define IP_INIT_NONE    0
>> @@ -425,6 +426,60 @@ static int tftp_load(filename_ip_t *fnip, void *buffer, int len)
>>  	return rc;
>>  }
>>  
>> +#define CFG_BUF_SIZE 2048
>> +#define MAX_LKIA_ENTRIES 16
>> +static int net_pxelinux_cfg_load(filename_ip_t *fnip, char *loadbase,
>> +                                 int maxloadlen, uint8_t *mac)
> 
> 
> The name confused me, there is already pxelinux_load_cfg().
> net_pxelinux_load() may be?

Sure, I'll rename it.

>> +{
>> +	static char cfgbuf[CFG_BUF_SIZE];
> 
> This belongs to pxelinux_load_parse_cfg().

The pointer in the struct lkia entries[] below will point to the
original location in the above buffer, so this cfgbuf needs to remain
valid until the end of this function. That means, the memory for the
buffer should be managed in the calling function instead of doing it in
pxelinux_load_parse_cfg(). I should maybe update the comments there to
make this clear... Alternatively, I could also "strdup" the strings for
the struct lkia entries, but then this caller here has to make sure to
free the strings again once the information from struct lkia is not
needed anymore - and IMHO that's uglier and more error-prone than to
allocate the cfgbuf buffer here on the calling site.

>  And why static?

Allocating big buffers on the stack is always critical within paflof,
we've seen more than enough buffer overruns in the past already. But I
could malloc() the buffer instead if you prefer that.

>> +	struct lkia entries[MAX_LKIA_ENTRIES];
>> +	int def, num_entries, rc, ilen;
>> +
>> +	num_entries = pxelinux_load_parse_cfg(fnip, mac, NULL, cfgbuf,
>> +	                                      CFG_BUF_SIZE, entries,
>> +	                                      MAX_LKIA_ENTRIES, &def);
>> +	if (num_entries <= 0)
>> +		return -1;
>> +
>> +	/* Load kernel */
>> +	strncpy(fnip->filename, entries[def].kernel,
>> +		sizeof(fnip->filename) - 1);
>> +	fnip->filename[sizeof(fnip->filename) - 1] = 0;
>> +	rc = tftp_load(fnip, loadbase, maxloadlen);
>> +	if (rc < 0)
>> +		return rc;
>> +
>> +	/* Load ramdisk */
>> +	if (entries[def].initrd) {
>> +		loadbase += rc;
>> +		maxloadlen -= rc;
>> +		if (maxloadlen <= 0) {
>> +			puts("  Not enough space for loading the initrd!");
>> +			return -1;
>> +		}
>> +		strncpy(fnip->filename, entries[def].initrd,
>> +			sizeof(fnip->filename) - 1);
>> +		ilen = tftp_load(fnip, loadbase, maxloadlen);
>> +		if (ilen < 0)
>> +			return ilen;
>> +		/* The ELF loader will move the kernel to some spot in low mem
>> +		 * later, thus move the initrd to the end of the RAM instead */
>> +		memmove(loadbase + maxloadlen - ilen, loadbase, ilen);
>> +		/* Encode the initrd information in the device tree */
>> +		SLOF_set_chosen_int("linux,initrd-start",
>> +		                    (long)loadbase + maxloadlen - ilen);
>> +		SLOF_set_chosen_int("linux,initrd-end",
>> +		                    (long)loadbase + maxloadlen);
>> +	}
>> +
>> +	if (entries[def].append) {
>> +		SLOF_set_chosen_bytes("bootargs", entries[def].append,
>> +		                      strlen(entries[def].append) + 1);
>> +	}
>> +
>> +	return rc;
>> +}

 Thomas