[SLOF] [PATCH] netboot: Create bootp-response when bootp is used

Nikunj A Dadhania nikunj at linux.vnet.ibm.com
Tue Sep 26 13:32:38 AEST 2017


Alexey Kardashevskiy <aik at ozlabs.ru> writes:

> On 19/09/17 19:58, Nikunj A Dadhania wrote:
>> According to TFTP Booting extension, after the success of BOOTP, BOOTREPLY
>> packet should be copied to bootp-response property under "/chosen"
>> 
>> While in current case, even when DHCP was used, bootp-response was being set. So
>> set bootp-response when BOOTP is used and dhcp-response for DHCP
>
>
> How did you test it (what server does bootp)?

I did a reverse test, when using dhcp, bootp-response shouldnt be set.

>
>
>> 
>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>> ---
>>  include/helpers.h            |  2 ++
>>  lib/libnet/libnet.code       |  4 +---
>>  lib/libnet/netapps.h         |  4 ++--
>>  lib/libnet/netload.c         | 40 ++++++++++++++++++++++++++++++++++------
>>  slof/fs/packages/obp-tftp.fs | 13 +------------
>>  slof/helpers.c               | 20 ++++++++++++++++++++
>>  6 files changed, 60 insertions(+), 23 deletions(-)
>> 
>> diff --git a/include/helpers.h b/include/helpers.h
>> index c86c468..04ee771 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_encode_bootp_response(void *addr, size_t size);
>> +extern void SLOF_encode_dhcp_response(void *addr, size_t size);
>>  
>>  #define offset_of(type, member) ((long) &((type *)0)->member)
>>  #define container_of(ptr, type, member) ({                      \
>> diff --git a/lib/libnet/libnet.code b/lib/libnet/libnet.code
>> index 3602543..2746782 100644
>> --- a/lib/libnet/libnet.code
>> +++ b/lib/libnet/libnet.code
>> @@ -6,11 +6,9 @@ PRIM(NET_X2d_LOAD)
>>  	char *arg = TOS.a; POP;
>>  	int blocksize = TOS.n; POP;
>>  	int hugeload = TOS.n; POP;
>> -	void *replybuf = TOS.a; POP;
>>  	long maxlen = TOS.n; POP;
>>  	void *loadaddr = TOS.a;
>> -	TOS.n = netload(loadaddr, maxlen, replybuf, hugeload, blocksize,
>> -			arg, alen);
>> +	TOS.n = netload(loadaddr, maxlen, hugeload, blocksize, arg, alen);
>>  MIRP
>>  
>>  PRIM(NET_X2d_PING)
>> diff --git a/lib/libnet/netapps.h b/lib/libnet/netapps.h
>> index 2fea4a7..0e637e1 100644
>> --- a/lib/libnet/netapps.h
>> +++ b/lib/libnet/netapps.h
>> @@ -18,8 +18,8 @@
>>  
>>  struct filename_ip;
>>  
>> -extern int netload(char *buffer, int len, char *ret_buffer, int huge_load,
>> -		   int block_size, char *args_fs, int alen);
>> +extern int netload(char *buffer, int len, int huge_load, int block_size,
>> +                   char *args_fs, int alen);
>>  extern int ping(char *args_fs, int alen);
>>  extern int dhcp(char *ret_buffer, struct filename_ip *fn_ip,
>>  		unsigned int retries, int flags);
>> diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c
>> index cecb2a0..3e54ded 100644
>> --- a/lib/libnet/netload.c
>> +++ b/lib/libnet/netload.c
>> @@ -34,6 +34,7 @@
>>  #define IP_INIT_DHCPV6_STATELESS    3
>>  #define IP_INIT_IPV6_MANUAL         4
>>  
>> +#define MAX_PKT_SIZE         1720
>>  #define DEFAULT_BOOT_RETRIES 10
>>  #define DEFAULT_TFTP_RETRIES 20
>>  static int ip_version = 4;
>> @@ -493,8 +494,24 @@ static int tftp_load(filename_ip_t *fnip, unsigned char *buffer, int len,
>>  	return rc;
>>  }
>>  
>> -int netload(char *buffer, int len, char *ret_buffer, int huge_load,
>> -	    int block_size, char *args_fs, int alen)
>> +static void encode_response(char *pkt_buffer, size_t size, int ip_init)
>> +{
>> +	switch(ip_init) {
>> +	case IP_INIT_BOOTP:
>> +		SLOF_encode_bootp_response(pkt_buffer, size);
>> +		break;
>> +	case IP_INIT_DHCP:
>> +	case IP_INIT_DHCPV6_STATELESS:
>> +	case IP_INIT_DEFAULT:
>> +		SLOF_encode_dhcp_response(pkt_buffer, size);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +}
>> +
>> +int netload(char *buffer, int len, int huge_load, int block_size,
>> +	    char *args_fs, int alen)
>>  {
>>  	int rc;
>>  	filename_ip_t fn_ip;
>> @@ -506,6 +523,14 @@ int netload(char *buffer, int len, char *ret_buffer, int huge_load,
>>  			     0x00, 0x00, 0x00, 0x00, 
>>  			     0x00, 0x00, 0x00, 0x00 };
>>  	uint8_t own_mac[6];
>> +	char *pkt_buffer;
>> +
>> +	pkt_buffer = SLOF_alloc_mem(MAX_PKT_SIZE);
>> +	if (!pkt_buffer) {
>> +		puts("ERROR: Unable to allocate memory");
>> +		return -1;
>> +	}
>> +	memset(pkt_buffer, 0, MAX_PKT_SIZE);
>>  
>>  	puts("\n Initializing NIC");
>>  	memset(&fn_ip, 0, sizeof(filename_ip_t));
>> @@ -615,13 +640,13 @@ int netload(char *buffer, int len, char *ret_buffer, int huge_load,
>>  		else {
>>  			memcpy(&fn_ip.server_ip, obp_tftp_args.giaddr, 4);
>>  		}
>> -		rc = bootp(ret_buffer, &fn_ip, obp_tftp_args.bootp_retries);
>> +		rc = bootp(pkt_buffer, &fn_ip, obp_tftp_args.bootp_retries);
>
>
> Nit: why this rename?

Its no more a return buffer as it was named earlier.

>>  		break;
>>  	case IP_INIT_DHCP:
>> -		rc = dhcp(ret_buffer, &fn_ip, obp_tftp_args.bootp_retries, F_IPV4);
>> +		rc = dhcp(pkt_buffer, &fn_ip, obp_tftp_args.bootp_retries, F_IPV4);
>>  		break;
>>  	case IP_INIT_DHCPV6_STATELESS:
>> -		rc = dhcp(ret_buffer, &fn_ip,
>> +		rc = dhcp(pkt_buffer, &fn_ip,
>>  			  obp_tftp_args.bootp_retries, F_IPV6);
>>  		break;
>>  	case IP_INIT_IPV6_MANUAL:
>> @@ -637,7 +662,7 @@ int netload(char *buffer, int len, char *ret_buffer, int huge_load,
>>  		}
>>  		break;
>>  	case IP_INIT_DEFAULT:
>> -		rc = dhcp(ret_buffer, &fn_ip, obp_tftp_args.bootp_retries, 0);
>> +		rc = dhcp(pkt_buffer, &fn_ip, obp_tftp_args.bootp_retries, 0);
>>  		break;
>>  	case IP_INIT_NONE:
>>  	default:
>> @@ -732,5 +757,8 @@ int netload(char *buffer, int len, char *ret_buffer, int huge_load,
>>  
>>  	close(fn_ip.fd);
>>  
>> +	if (rc >= 0) {
>> +		encode_response(pkt_buffer, MAX_PKT_SIZE, obp_tftp_args.ip_init);
>> +	}
>>  	return rc;
>>  }
>> diff --git a/slof/fs/packages/obp-tftp.fs b/slof/fs/packages/obp-tftp.fs
>> index 63171d0..17fb980 100644
>> --- a/slof/fs/packages/obp-tftp.fs
>> +++ b/slof/fs/packages/obp-tftp.fs
>> @@ -28,24 +28,13 @@ VARIABLE huge-tftp-load 1 huge-tftp-load !
>>  
>>      60000000                        ( addr maxlen )
>>  
>> -    \ Allocate 1720 bytes to store the BOOTP-REPLY packet
>> -    6B8 alloc-mem dup >r            ( addr maxlen replybuf )
>> -    huge-tftp-load @  d# 1428       ( addr maxlen replybuf hugetftp blocksize )
>> +    huge-tftp-load @  d# 1428       ( addr maxlen hugetftp blocksize )
>>      \ Add OBP-TFTP Bootstring argument, e.g. "10.128.0.1,bootrom.bin,10.128.40.1"
>>      my-args
>>      net-load dup 0< IF drop 0 THEN
>>  
>> -    \ Recover buffer address of BOOTP-REPLY packet
>> -    r>
>> -
>>      r> r> over IF s" bootpath" set-chosen ELSE 2drop THEN
>>      r> r> over IF s" bootargs" set-chosen ELSE 2drop THEN
>> -
>> -    \ Store BOOTP-REPLY packet as property
>> -    dup 6B8 encode-bytes s" bootp-response" s" /chosen" find-node set-property
>> -
>> -    \ free buffer
>> -    6B8 free-mem
>
>
> alloc-mem above got replaced with SLOF_alloc_mem() but this free-mem has
> not got any replacement. Memory leak?

You are right, missed a free there. Will send an updated patch.

Regards
Nikunj



More information about the SLOF mailing list