| Message ID | 1526578856-30967-4-git-send-email-thuth@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Support network booting with pxelinux.cfg files | expand |
On Thu, 17 May 2018 19:40:50 +0200 Thomas Huth <thuth@redhat.com> wrote: > When we will support loading of pxelinux.cfg files later, we have to call > the tftp load function multiple times from different places. To avoid that > we've also got to pass around the tftp_retries and ip_version information > via function parameters to all spots, let's rather put them into struct > filename_ip since we've got this struct filename_ip info available every- > where already. > > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > lib/libnet/netload.c | 11 ++++++----- > lib/libnet/tftp.c | 9 ++++----- > lib/libnet/tftp.h | 10 ++++++---- > 3 files changed, 16 insertions(+), 14 deletions(-) > > diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c > index 6b9bc57..add447c 100644 > --- a/lib/libnet/netload.c > +++ b/lib/libnet/netload.c > @@ -404,13 +404,12 @@ static void seed_rng(uint8_t mac[]) > srand(seed); > } > > -static int tftp_load(filename_ip_t *fnip, unsigned char *buffer, int len, > - unsigned int retries, int ip_vers) > +static int tftp_load(filename_ip_t *fnip, void *buffer, int len) > { > tftp_err_t tftp_err; > int rc; > > - rc = tftp(fnip, buffer, len, retries, &tftp_err, 1, 1428, ip_vers); > + rc = tftp(fnip, buffer, len, &tftp_err, 1, 1428); > > if (rc > 0) { > printf(" TFTP: Received %s (%d KBytes)\n", fnip->filename, > @@ -737,6 +736,9 @@ int netload(char *buffer, int len, char *args_fs, int alen) > fn_ip.filename[sizeof(fn_ip.filename)-1] = 0; > } > > + fn_ip.ip_version = ip_version; > + fn_ip.tftp_retries = obp_tftp_args.tftp_retries; > + > if (ip_version == 4) { > printf(" Requesting file \"%s\" via TFTP from %d.%d.%d.%d\n", > fn_ip.filename, > @@ -752,8 +754,7 @@ int netload(char *buffer, int len, char *args_fs, int alen) > } > > /* Do the TFTP load and print error message if necessary */ > - rc = tftp_load(&fn_ip, (unsigned char *)buffer, len, > - obp_tftp_args.tftp_retries, ip_version); > + rc = tftp_load(&fn_ip, buffer, len); > > if (obp_tftp_args.ip_init == IP_INIT_DHCP) > dhcp_send_release(fn_ip.fd); > diff --git a/lib/libnet/tftp.c b/lib/libnet/tftp.c > index 1656c27..d760f03 100644 > --- a/lib/libnet/tftp.c > +++ b/lib/libnet/tftp.c > @@ -497,20 +497,19 @@ void handle_tftp_dun(uint8_t err_code) > * @param _len size of destination buffer > * @param _retries max number of retries > * @param _tftp_err contains info about TFTP-errors (e.g. lost packets) > - * @param _mode NON ZERO - multicast, ZERO - unicast > + * @param _mode NON ZERO - huge load, ZERO - no roll-over > * @param _blocksize blocksize for DATA-packets > * @return ZERO - error condition occurs > * NON ZERO - size of received file > */ > int tftp(filename_ip_t * _fn_ip, unsigned char *_buffer, int _len, > - unsigned int _retries, tftp_err_t * _tftp_err, > - int32_t _mode, int32_t _blocksize, int _ip_version) > + tftp_err_t * _tftp_err, int32_t _mode, int32_t _blocksize) > { > - retries = _retries; > + retries = _fn_ip->tftp_retries; > fn_ip = _fn_ip; > len = _len; > huge_load = _mode; > - ip_version = _ip_version; > + ip_version = _fn_ip->ip_version; > tftp_errno = 0; > tftp_err = _tftp_err; > tftp_err->bad_tftp_packets = 0; > diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h > index e32e473..6f68d15 100644 > --- a/lib/libnet/tftp.h > +++ b/lib/libnet/tftp.h > @@ -29,8 +29,10 @@ struct filename_ip { > ip6_addr_t server_ip6; > ip6_addr_t dns_ip6; > char filename[256]; > - int fd; > -} __attribute__ ((packed)); > + int fd; > + int ip_version; > + int tftp_retries; > +}; Why dropping the packed attribute ? With this fixed or explained: Greg Kurz <groug@kaod.org> > typedef struct filename_ip filename_ip_t; > > typedef struct { > @@ -40,8 +42,8 @@ typedef struct { > uint32_t blocks_received; > } tftp_err_t; > > -int tftp(filename_ip_t *, unsigned char *, int, unsigned int, > - tftp_err_t *, int32_t mode, int32_t blocksize, int ip_version); > +int tftp(filename_ip_t *fnip, unsigned char *buf, int len, tftp_err_t *err, > + int32_t mode, int32_t blocksize); > > int32_t handle_tftp(int fd, uint8_t *, int32_t); > void handle_tftp_dun(uint8_t err_code);
On 18.05.2018 12:09, Greg Kurz wrote: > On Thu, 17 May 2018 19:40:50 +0200 > Thomas Huth <thuth@redhat.com> wrote: > >> When we will support loading of pxelinux.cfg files later, we have to call >> the tftp load function multiple times from different places. To avoid that >> we've also got to pass around the tftp_retries and ip_version information >> via function parameters to all spots, let's rather put them into struct >> filename_ip since we've got this struct filename_ip info available every- >> where already. >> >> Signed-off-by: Thomas Huth <thuth@redhat.com> >> --- >> lib/libnet/netload.c | 11 ++++++----- >> lib/libnet/tftp.c | 9 ++++----- >> lib/libnet/tftp.h | 10 ++++++---- >> 3 files changed, 16 insertions(+), 14 deletions(-) >> >> diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c >> index 6b9bc57..add447c 100644 >> --- a/lib/libnet/netload.c >> +++ b/lib/libnet/netload.c >> @@ -404,13 +404,12 @@ static void seed_rng(uint8_t mac[]) >> srand(seed); >> } >> >> -static int tftp_load(filename_ip_t *fnip, unsigned char *buffer, int len, >> - unsigned int retries, int ip_vers) >> +static int tftp_load(filename_ip_t *fnip, void *buffer, int len) >> { >> tftp_err_t tftp_err; >> int rc; >> >> - rc = tftp(fnip, buffer, len, retries, &tftp_err, 1, 1428, ip_vers); >> + rc = tftp(fnip, buffer, len, &tftp_err, 1, 1428); >> >> if (rc > 0) { >> printf(" TFTP: Received %s (%d KBytes)\n", fnip->filename, >> @@ -737,6 +736,9 @@ int netload(char *buffer, int len, char *args_fs, int alen) >> fn_ip.filename[sizeof(fn_ip.filename)-1] = 0; >> } >> >> + fn_ip.ip_version = ip_version; >> + fn_ip.tftp_retries = obp_tftp_args.tftp_retries; >> + >> if (ip_version == 4) { >> printf(" Requesting file \"%s\" via TFTP from %d.%d.%d.%d\n", >> fn_ip.filename, >> @@ -752,8 +754,7 @@ int netload(char *buffer, int len, char *args_fs, int alen) >> } >> >> /* Do the TFTP load and print error message if necessary */ >> - rc = tftp_load(&fn_ip, (unsigned char *)buffer, len, >> - obp_tftp_args.tftp_retries, ip_version); >> + rc = tftp_load(&fn_ip, buffer, len); >> >> if (obp_tftp_args.ip_init == IP_INIT_DHCP) >> dhcp_send_release(fn_ip.fd); >> diff --git a/lib/libnet/tftp.c b/lib/libnet/tftp.c >> index 1656c27..d760f03 100644 >> --- a/lib/libnet/tftp.c >> +++ b/lib/libnet/tftp.c >> @@ -497,20 +497,19 @@ void handle_tftp_dun(uint8_t err_code) >> * @param _len size of destination buffer >> * @param _retries max number of retries >> * @param _tftp_err contains info about TFTP-errors (e.g. lost packets) >> - * @param _mode NON ZERO - multicast, ZERO - unicast >> + * @param _mode NON ZERO - huge load, ZERO - no roll-over >> * @param _blocksize blocksize for DATA-packets >> * @return ZERO - error condition occurs >> * NON ZERO - size of received file >> */ >> int tftp(filename_ip_t * _fn_ip, unsigned char *_buffer, int _len, >> - unsigned int _retries, tftp_err_t * _tftp_err, >> - int32_t _mode, int32_t _blocksize, int _ip_version) >> + tftp_err_t * _tftp_err, int32_t _mode, int32_t _blocksize) >> { >> - retries = _retries; >> + retries = _fn_ip->tftp_retries; >> fn_ip = _fn_ip; >> len = _len; >> huge_load = _mode; >> - ip_version = _ip_version; >> + ip_version = _fn_ip->ip_version; >> tftp_errno = 0; >> tftp_err = _tftp_err; >> tftp_err->bad_tftp_packets = 0; >> diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h >> index e32e473..6f68d15 100644 >> --- a/lib/libnet/tftp.h >> +++ b/lib/libnet/tftp.h >> @@ -29,8 +29,10 @@ struct filename_ip { >> ip6_addr_t server_ip6; >> ip6_addr_t dns_ip6; >> char filename[256]; >> - int fd; >> -} __attribute__ ((packed)); >> + int fd; >> + int ip_version; >> + int tftp_retries; >> +}; > > Why dropping the packed attribute ? It does not seem to be necessary - this is just an internal structure which is never given to or read from the outside. I originally wanted to mention this in the patch description, but apparently I forgot to do so :-( > With this fixed or explained: > > Greg Kurz <groug@kaod.org> Reviewed-by? Acked-by? Thanks, Thomas
On Fri, 18 May 2018 13:30:39 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 18.05.2018 12:09, Greg Kurz wrote: > > On Thu, 17 May 2018 19:40:50 +0200 > > Thomas Huth <thuth@redhat.com> wrote: > > > >> When we will support loading of pxelinux.cfg files later, we have to call > >> the tftp load function multiple times from different places. To avoid that > >> we've also got to pass around the tftp_retries and ip_version information > >> via function parameters to all spots, let's rather put them into struct > >> filename_ip since we've got this struct filename_ip info available every- > >> where already. > >> > >> Signed-off-by: Thomas Huth <thuth@redhat.com> > >> --- > >> lib/libnet/netload.c | 11 ++++++----- > >> lib/libnet/tftp.c | 9 ++++----- > >> lib/libnet/tftp.h | 10 ++++++---- > >> 3 files changed, 16 insertions(+), 14 deletions(-) > >> > >> diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c > >> index 6b9bc57..add447c 100644 > >> --- a/lib/libnet/netload.c > >> +++ b/lib/libnet/netload.c > >> @@ -404,13 +404,12 @@ static void seed_rng(uint8_t mac[]) > >> srand(seed); > >> } > >> > >> -static int tftp_load(filename_ip_t *fnip, unsigned char *buffer, int len, > >> - unsigned int retries, int ip_vers) > >> +static int tftp_load(filename_ip_t *fnip, void *buffer, int len) > >> { > >> tftp_err_t tftp_err; > >> int rc; > >> > >> - rc = tftp(fnip, buffer, len, retries, &tftp_err, 1, 1428, ip_vers); > >> + rc = tftp(fnip, buffer, len, &tftp_err, 1, 1428); > >> > >> if (rc > 0) { > >> printf(" TFTP: Received %s (%d KBytes)\n", fnip->filename, > >> @@ -737,6 +736,9 @@ int netload(char *buffer, int len, char *args_fs, int alen) > >> fn_ip.filename[sizeof(fn_ip.filename)-1] = 0; > >> } > >> > >> + fn_ip.ip_version = ip_version; > >> + fn_ip.tftp_retries = obp_tftp_args.tftp_retries; > >> + > >> if (ip_version == 4) { > >> printf(" Requesting file \"%s\" via TFTP from %d.%d.%d.%d\n", > >> fn_ip.filename, > >> @@ -752,8 +754,7 @@ int netload(char *buffer, int len, char *args_fs, int alen) > >> } > >> > >> /* Do the TFTP load and print error message if necessary */ > >> - rc = tftp_load(&fn_ip, (unsigned char *)buffer, len, > >> - obp_tftp_args.tftp_retries, ip_version); > >> + rc = tftp_load(&fn_ip, buffer, len); > >> > >> if (obp_tftp_args.ip_init == IP_INIT_DHCP) > >> dhcp_send_release(fn_ip.fd); > >> diff --git a/lib/libnet/tftp.c b/lib/libnet/tftp.c > >> index 1656c27..d760f03 100644 > >> --- a/lib/libnet/tftp.c > >> +++ b/lib/libnet/tftp.c > >> @@ -497,20 +497,19 @@ void handle_tftp_dun(uint8_t err_code) > >> * @param _len size of destination buffer > >> * @param _retries max number of retries > >> * @param _tftp_err contains info about TFTP-errors (e.g. lost packets) > >> - * @param _mode NON ZERO - multicast, ZERO - unicast > >> + * @param _mode NON ZERO - huge load, ZERO - no roll-over > >> * @param _blocksize blocksize for DATA-packets > >> * @return ZERO - error condition occurs > >> * NON ZERO - size of received file > >> */ > >> int tftp(filename_ip_t * _fn_ip, unsigned char *_buffer, int _len, > >> - unsigned int _retries, tftp_err_t * _tftp_err, > >> - int32_t _mode, int32_t _blocksize, int _ip_version) > >> + tftp_err_t * _tftp_err, int32_t _mode, int32_t _blocksize) > >> { > >> - retries = _retries; > >> + retries = _fn_ip->tftp_retries; > >> fn_ip = _fn_ip; > >> len = _len; > >> huge_load = _mode; > >> - ip_version = _ip_version; > >> + ip_version = _fn_ip->ip_version; > >> tftp_errno = 0; > >> tftp_err = _tftp_err; > >> tftp_err->bad_tftp_packets = 0; > >> diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h > >> index e32e473..6f68d15 100644 > >> --- a/lib/libnet/tftp.h > >> +++ b/lib/libnet/tftp.h > >> @@ -29,8 +29,10 @@ struct filename_ip { > >> ip6_addr_t server_ip6; > >> ip6_addr_t dns_ip6; > >> char filename[256]; > >> - int fd; > >> -} __attribute__ ((packed)); > >> + int fd; > >> + int ip_version; > >> + int tftp_retries; > >> +}; > > > > Why dropping the packed attribute ? > > It does not seem to be necessary - this is just an internal structure > which is never given to or read from the outside. > I originally wanted to mention this in the patch description, but > apparently I forgot to do so :-( > Ok, no big deal I guess. > > With this fixed or explained: > > > > Greg Kurz <groug@kaod.org> > > Reviewed-by? Acked-by? > Oops, Reviewed-by :) > Thanks, > Thomas
diff --git a/lib/libnet/netload.c b/lib/libnet/netload.c index 6b9bc57..add447c 100644 --- a/lib/libnet/netload.c +++ b/lib/libnet/netload.c @@ -404,13 +404,12 @@ static void seed_rng(uint8_t mac[]) srand(seed); } -static int tftp_load(filename_ip_t *fnip, unsigned char *buffer, int len, - unsigned int retries, int ip_vers) +static int tftp_load(filename_ip_t *fnip, void *buffer, int len) { tftp_err_t tftp_err; int rc; - rc = tftp(fnip, buffer, len, retries, &tftp_err, 1, 1428, ip_vers); + rc = tftp(fnip, buffer, len, &tftp_err, 1, 1428); if (rc > 0) { printf(" TFTP: Received %s (%d KBytes)\n", fnip->filename, @@ -737,6 +736,9 @@ int netload(char *buffer, int len, char *args_fs, int alen) fn_ip.filename[sizeof(fn_ip.filename)-1] = 0; } + fn_ip.ip_version = ip_version; + fn_ip.tftp_retries = obp_tftp_args.tftp_retries; + if (ip_version == 4) { printf(" Requesting file \"%s\" via TFTP from %d.%d.%d.%d\n", fn_ip.filename, @@ -752,8 +754,7 @@ int netload(char *buffer, int len, char *args_fs, int alen) } /* Do the TFTP load and print error message if necessary */ - rc = tftp_load(&fn_ip, (unsigned char *)buffer, len, - obp_tftp_args.tftp_retries, ip_version); + rc = tftp_load(&fn_ip, buffer, len); if (obp_tftp_args.ip_init == IP_INIT_DHCP) dhcp_send_release(fn_ip.fd); diff --git a/lib/libnet/tftp.c b/lib/libnet/tftp.c index 1656c27..d760f03 100644 --- a/lib/libnet/tftp.c +++ b/lib/libnet/tftp.c @@ -497,20 +497,19 @@ void handle_tftp_dun(uint8_t err_code) * @param _len size of destination buffer * @param _retries max number of retries * @param _tftp_err contains info about TFTP-errors (e.g. lost packets) - * @param _mode NON ZERO - multicast, ZERO - unicast + * @param _mode NON ZERO - huge load, ZERO - no roll-over * @param _blocksize blocksize for DATA-packets * @return ZERO - error condition occurs * NON ZERO - size of received file */ int tftp(filename_ip_t * _fn_ip, unsigned char *_buffer, int _len, - unsigned int _retries, tftp_err_t * _tftp_err, - int32_t _mode, int32_t _blocksize, int _ip_version) + tftp_err_t * _tftp_err, int32_t _mode, int32_t _blocksize) { - retries = _retries; + retries = _fn_ip->tftp_retries; fn_ip = _fn_ip; len = _len; huge_load = _mode; - ip_version = _ip_version; + ip_version = _fn_ip->ip_version; tftp_errno = 0; tftp_err = _tftp_err; tftp_err->bad_tftp_packets = 0; diff --git a/lib/libnet/tftp.h b/lib/libnet/tftp.h index e32e473..6f68d15 100644 --- a/lib/libnet/tftp.h +++ b/lib/libnet/tftp.h @@ -29,8 +29,10 @@ struct filename_ip { ip6_addr_t server_ip6; ip6_addr_t dns_ip6; char filename[256]; - int fd; -} __attribute__ ((packed)); + int fd; + int ip_version; + int tftp_retries; +}; typedef struct filename_ip filename_ip_t; typedef struct { @@ -40,8 +42,8 @@ typedef struct { uint32_t blocks_received; } tftp_err_t; -int tftp(filename_ip_t *, unsigned char *, int, unsigned int, - tftp_err_t *, int32_t mode, int32_t blocksize, int ip_version); +int tftp(filename_ip_t *fnip, unsigned char *buf, int len, tftp_err_t *err, + int32_t mode, int32_t blocksize); int32_t handle_tftp(int fd, uint8_t *, int32_t); void handle_tftp_dun(uint8_t err_code);
When we will support loading of pxelinux.cfg files later, we have to call the tftp load function multiple times from different places. To avoid that we've also got to pass around the tftp_retries and ip_version information via function parameters to all spots, let's rather put them into struct filename_ip since we've got this struct filename_ip info available every- where already. Signed-off-by: Thomas Huth <thuth@redhat.com> --- lib/libnet/netload.c | 11 ++++++----- lib/libnet/tftp.c | 9 ++++----- lib/libnet/tftp.h | 10 ++++++---- 3 files changed, 16 insertions(+), 14 deletions(-)