[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