* [Qemu-devel] [PATCH 0/3] slirp: misc fixes @ 2011-04-11 19:10 Herve Poussineau 2011-04-11 19:10 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Herve Poussineau 0 siblings, 1 reply; 11+ messages in thread From: Herve Poussineau @ 2011-04-11 19:10 UTC (permalink / raw) To: qemu-devel These patches contain some fixes for the internal TFTP server. With these patches, MS Windows PE can be booted via PXE. slirp/tftp.c | 69 +++++++++++++++++++++++++++++++++++++++++---------------- slirp/tftp.h | 2 + 2 files changed, 51 insertions(+), 20 deletions(-) ^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option 2011-04-11 19:10 [Qemu-devel] [PATCH 0/3] slirp: misc fixes Herve Poussineau @ 2011-04-11 19:10 ` Herve Poussineau 2011-04-11 19:10 ` [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers Herve Poussineau ` (3 more replies) 0 siblings, 4 replies; 11+ messages in thread From: Herve Poussineau @ 2011-04-11 19:10 UTC (permalink / raw) To: qemu-devel; +Cc: Herv� Poussineau [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=utf-8, Size: 3059 bytes --] From: Hervé Poussineau <hpoussin@reactos.org> This option is described in RFC 1783. As this is only an optional field, we may ignore it in some situations and handle it in some others. Here, if client requests a block size bigger than the block size we emit (512 bytes), accept the option with a value of 512 Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> --- slirp/tftp.c | 40 ++++++++++++++++++++++++++++++++-------- 1 files changed, 32 insertions(+), 8 deletions(-) diff --git a/slirp/tftp.c b/slirp/tftp.c index 8055ccc..7ef44c9 100644 --- a/slirp/tftp.c +++ b/slirp/tftp.c @@ -116,13 +116,13 @@ static int tftp_read_data(struct tftp_session *spt, uint16_t block_nr, } static int tftp_send_oack(struct tftp_session *spt, - const char *key, uint32_t value, + const char *key[], uint32_t value[], int nb, struct tftp_t *recv_tp) { struct sockaddr_in saddr, daddr; struct mbuf *m; struct tftp_t *tp; - int n = 0; + int i, n = 0; m = m_get(spt->slirp); @@ -136,10 +136,12 @@ static int tftp_send_oack(struct tftp_session *spt, m->m_data += sizeof(struct udpiphdr); tp->tp_op = htons(TFTP_OACK); - n += snprintf(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%s", - key) + 1; - n += snprintf(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%u", - value) + 1; + for (i = 0; i < nb; i++) { + n += snprintf(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%s", + key[i]) + 1; + n += snprintf(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%u", + value[i]) + 1; + } saddr.sin_addr = recv_tp->ip.ip_dst; saddr.sin_port = recv_tp->udp.uh_dport; @@ -260,6 +262,9 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen) int s, k; size_t prefix_len; char *req_fname; + const char *option_name[2]; + uint32_t option_value[2]; + int nb_options = 0; /* check if a session already exists and if so terminate it */ s = tftp_session_find(slirp, tp); @@ -364,9 +369,28 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen) } } - tftp_send_oack(spt, "tsize", tsize, tp); - return; + option_name[nb_options] = "tsize"; + option_value[nb_options] = tsize; + nb_options++; } + if (strcasecmp(key, "blksize") == 0) { + int blksize = atoi(value); + + /* If blksize option is bigger than what we will + * emit, accept the option with our packet size. + * Otherwise, simply do as we didn't see the option. + */ + if (blksize >= 512) { + option_name[nb_options] = "blksize"; + option_value[nb_options] = 512; + nb_options++; + } + } + } + + if (nb_options > 0) { + tftp_send_oack(spt, option_name, option_value, nb_options, tp); + return; } tftp_send_data(spt, 1, tp); -- 1.6.0.2.GIT ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers 2011-04-11 19:10 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Herve Poussineau @ 2011-04-11 19:10 ` Herve Poussineau 2011-04-11 19:10 ` [Qemu-devel] [PATCH 3/3] slirp: improve TFTP performance Herve Poussineau 2011-04-18 14:02 ` [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers Aurelien Jarno 2011-04-16 9:15 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Stefan Hajnoczi ` (2 subsequent siblings) 3 siblings, 2 replies; 11+ messages in thread From: Herve Poussineau @ 2011-04-11 19:10 UTC (permalink / raw) To: qemu-devel; +Cc: Herv� Poussineau [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=utf-8, Size: 2136 bytes --] From: Hervé Poussineau <hpoussin@reactos.org> RFC 1350 does not mention block count-roll over. However, a lot of TFTP servers implement it to be able to transmit big files, so do it also Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> --- slirp/tftp.c | 9 +++++---- slirp/tftp.h | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/slirp/tftp.c b/slirp/tftp.c index 7ef44c9..7e63269 100644 --- a/slirp/tftp.c +++ b/slirp/tftp.c @@ -92,7 +92,7 @@ static int tftp_session_find(Slirp *slirp, struct tftp_t *tp) return -1; } -static int tftp_read_data(struct tftp_session *spt, uint16_t block_nr, +static int tftp_read_data(struct tftp_session *spt, uint32_t block_nr, uint8_t *buf, int len) { int fd; @@ -196,7 +196,7 @@ out: } static int tftp_send_data(struct tftp_session *spt, - uint16_t block_nr, + uint32_t block_nr, struct tftp_t *recv_tp) { struct sockaddr_in saddr, daddr; @@ -221,7 +221,7 @@ static int tftp_send_data(struct tftp_session *spt, m->m_data += sizeof(struct udpiphdr); tp->tp_op = htons(TFTP_DATA); - tp->x.tp_data.tp_block_nr = htons(block_nr); + tp->x.tp_data.tp_block_nr = htons(block_nr & 0xffff); saddr.sin_addr = recv_tp->ip.ip_dst; saddr.sin_port = recv_tp->udp.uh_dport; @@ -253,6 +253,7 @@ static int tftp_send_data(struct tftp_session *spt, tftp_session_terminate(spt); } + spt->block_nr = block_nr; return 0; } @@ -407,7 +408,7 @@ static void tftp_handle_ack(Slirp *slirp, struct tftp_t *tp, int pktlen) } if (tftp_send_data(&slirp->tftp_sessions[s], - ntohs(tp->x.tp_data.tp_block_nr) + 1, + slirp->tftp_sessions[s].block_nr + 1, tp) < 0) { return; } diff --git a/slirp/tftp.h b/slirp/tftp.h index 72e5e91..471f22e 100644 --- a/slirp/tftp.h +++ b/slirp/tftp.h @@ -36,6 +36,7 @@ struct tftp_session { struct in_addr client_ip; uint16_t client_port; + uint32_t block_nr; int timestamp; }; -- 1.6.0.2.GIT ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 3/3] slirp: improve TFTP performance 2011-04-11 19:10 ` [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers Herve Poussineau @ 2011-04-11 19:10 ` Herve Poussineau 2011-04-18 14:04 ` Aurelien Jarno 2011-04-18 14:02 ` [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers Aurelien Jarno 1 sibling, 1 reply; 11+ messages in thread From: Herve Poussineau @ 2011-04-11 19:10 UTC (permalink / raw) To: qemu-devel; +Cc: Herv� Poussineau [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset=utf-8, Size: 2366 bytes --] From: Hervé Poussineau <hpoussin@reactos.org> When transfering a file, keep it open during the whole transfer, instead of opening/closing it for each block. Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> --- slirp/tftp.c | 20 ++++++++++++-------- slirp/tftp.h | 1 + 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/slirp/tftp.c b/slirp/tftp.c index 7e63269..48748f1 100644 --- a/slirp/tftp.c +++ b/slirp/tftp.c @@ -37,6 +37,10 @@ static inline void tftp_session_update(struct tftp_session *spt) static void tftp_session_terminate(struct tftp_session *spt) { + if (spt->fd >= 0) { + close(spt->fd); + spt->fd = -1; + } qemu_free(spt->filename); spt->slirp = NULL; } @@ -54,7 +58,7 @@ static int tftp_session_allocate(Slirp *slirp, struct tftp_t *tp) /* sessions time out after 5 inactive seconds */ if ((int)(curtime - spt->timestamp) > 5000) { - qemu_free(spt->filename); + tftp_session_terminate(spt); goto found; } } @@ -64,6 +68,7 @@ static int tftp_session_allocate(Slirp *slirp, struct tftp_t *tp) found: memset(spt, 0, sizeof(*spt)); memcpy(&spt->client_ip, &tp->ip.ip_src, sizeof(spt->client_ip)); + spt->fd = -1; spt->client_port = tp->udp.uh_sport; spt->slirp = slirp; @@ -95,23 +100,22 @@ static int tftp_session_find(Slirp *slirp, struct tftp_t *tp) static int tftp_read_data(struct tftp_session *spt, uint32_t block_nr, uint8_t *buf, int len) { - int fd; int bytes_read = 0; - fd = open(spt->filename, O_RDONLY | O_BINARY); + if (spt->fd < 0) { + spt->fd = open(spt->filename, O_RDONLY | O_BINARY); + } - if (fd < 0) { + if (spt->fd < 0) { return -1; } if (len) { - lseek(fd, block_nr * 512, SEEK_SET); + lseek(spt->fd, block_nr * 512, SEEK_SET); - bytes_read = read(fd, buf, len); + bytes_read = read(spt->fd, buf, len); } - close(fd); - return bytes_read; } diff --git a/slirp/tftp.h b/slirp/tftp.h index 471f22e..51704e4 100644 --- a/slirp/tftp.h +++ b/slirp/tftp.h @@ -33,6 +33,7 @@ struct tftp_t { struct tftp_session { Slirp *slirp; char *filename; + int fd; struct in_addr client_ip; uint16_t client_port; -- 1.6.0.2.GIT ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] slirp: improve TFTP performance 2011-04-11 19:10 ` [Qemu-devel] [PATCH 3/3] slirp: improve TFTP performance Herve Poussineau @ 2011-04-18 14:04 ` Aurelien Jarno 0 siblings, 0 replies; 11+ messages in thread From: Aurelien Jarno @ 2011-04-18 14:04 UTC (permalink / raw) To: Herve Poussineau; +Cc: qemu-devel On Mon, Apr 11, 2011 at 07:10:54PM +0000, Herve Poussineau wrote: > From: Herv? Poussineau <hpoussin@reactos.org> > > When transfering a file, keep it open during the whole transfer, > instead of opening/closing it for each block. > > Signed-off-by: Herv? Poussineau <hpoussin@reactos.org> > --- > slirp/tftp.c | 20 ++++++++++++-------- > slirp/tftp.h | 1 + > 2 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/slirp/tftp.c b/slirp/tftp.c > index 7e63269..48748f1 100644 > --- a/slirp/tftp.c > +++ b/slirp/tftp.c > @@ -37,6 +37,10 @@ static inline void tftp_session_update(struct tftp_session *spt) > > static void tftp_session_terminate(struct tftp_session *spt) > { > + if (spt->fd >= 0) { > + close(spt->fd); > + spt->fd = -1; > + } > qemu_free(spt->filename); > spt->slirp = NULL; > } > @@ -54,7 +58,7 @@ static int tftp_session_allocate(Slirp *slirp, struct tftp_t *tp) > > /* sessions time out after 5 inactive seconds */ > if ((int)(curtime - spt->timestamp) > 5000) { > - qemu_free(spt->filename); > + tftp_session_terminate(spt); > goto found; > } > } > @@ -64,6 +68,7 @@ static int tftp_session_allocate(Slirp *slirp, struct tftp_t *tp) > found: > memset(spt, 0, sizeof(*spt)); > memcpy(&spt->client_ip, &tp->ip.ip_src, sizeof(spt->client_ip)); > + spt->fd = -1; > spt->client_port = tp->udp.uh_sport; > spt->slirp = slirp; > > @@ -95,23 +100,22 @@ static int tftp_session_find(Slirp *slirp, struct tftp_t *tp) > static int tftp_read_data(struct tftp_session *spt, uint32_t block_nr, > uint8_t *buf, int len) > { > - int fd; > int bytes_read = 0; > > - fd = open(spt->filename, O_RDONLY | O_BINARY); > + if (spt->fd < 0) { > + spt->fd = open(spt->filename, O_RDONLY | O_BINARY); > + } > > - if (fd < 0) { > + if (spt->fd < 0) { > return -1; > } > > if (len) { > - lseek(fd, block_nr * 512, SEEK_SET); > + lseek(spt->fd, block_nr * 512, SEEK_SET); > > - bytes_read = read(fd, buf, len); > + bytes_read = read(spt->fd, buf, len); > } > > - close(fd); > - > return bytes_read; > } Given you are rewriting this most of this function, it's probably the good moment to also fix the indentation to 4 spaces. > diff --git a/slirp/tftp.h b/slirp/tftp.h > index 471f22e..51704e4 100644 > --- a/slirp/tftp.h > +++ b/slirp/tftp.h > @@ -33,6 +33,7 @@ struct tftp_t { > struct tftp_session { > Slirp *slirp; > char *filename; > + int fd; > > struct in_addr client_ip; > uint16_t client_port; > -- > 1.6.0.2.GIT > Except the minor nit above: Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers 2011-04-11 19:10 ` [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers Herve Poussineau 2011-04-11 19:10 ` [Qemu-devel] [PATCH 3/3] slirp: improve TFTP performance Herve Poussineau @ 2011-04-18 14:02 ` Aurelien Jarno 1 sibling, 0 replies; 11+ messages in thread From: Aurelien Jarno @ 2011-04-18 14:02 UTC (permalink / raw) To: Herve Poussineau; +Cc: qemu-devel On Mon, Apr 11, 2011 at 07:10:53PM +0000, Herve Poussineau wrote: > From: Herv? Poussineau <hpoussin@reactos.org> > > RFC 1350 does not mention block count-roll over. However, a lot of TFTP servers > implement it to be able to transmit big files, so do it also > > Signed-off-by: Herv? Poussineau <hpoussin@reactos.org> > --- > slirp/tftp.c | 9 +++++---- > slirp/tftp.h | 1 + > 2 files changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> > diff --git a/slirp/tftp.c b/slirp/tftp.c > index 7ef44c9..7e63269 100644 > --- a/slirp/tftp.c > +++ b/slirp/tftp.c > @@ -92,7 +92,7 @@ static int tftp_session_find(Slirp *slirp, struct tftp_t *tp) > return -1; > } > > -static int tftp_read_data(struct tftp_session *spt, uint16_t block_nr, > +static int tftp_read_data(struct tftp_session *spt, uint32_t block_nr, > uint8_t *buf, int len) > { > int fd; > @@ -196,7 +196,7 @@ out: > } > > static int tftp_send_data(struct tftp_session *spt, > - uint16_t block_nr, > + uint32_t block_nr, > struct tftp_t *recv_tp) > { > struct sockaddr_in saddr, daddr; > @@ -221,7 +221,7 @@ static int tftp_send_data(struct tftp_session *spt, > m->m_data += sizeof(struct udpiphdr); > > tp->tp_op = htons(TFTP_DATA); > - tp->x.tp_data.tp_block_nr = htons(block_nr); > + tp->x.tp_data.tp_block_nr = htons(block_nr & 0xffff); > > saddr.sin_addr = recv_tp->ip.ip_dst; > saddr.sin_port = recv_tp->udp.uh_dport; > @@ -253,6 +253,7 @@ static int tftp_send_data(struct tftp_session *spt, > tftp_session_terminate(spt); > } > > + spt->block_nr = block_nr; > return 0; > } > > @@ -407,7 +408,7 @@ static void tftp_handle_ack(Slirp *slirp, struct tftp_t *tp, int pktlen) > } > > if (tftp_send_data(&slirp->tftp_sessions[s], > - ntohs(tp->x.tp_data.tp_block_nr) + 1, > + slirp->tftp_sessions[s].block_nr + 1, > tp) < 0) { > return; > } > diff --git a/slirp/tftp.h b/slirp/tftp.h > index 72e5e91..471f22e 100644 > --- a/slirp/tftp.h > +++ b/slirp/tftp.h > @@ -36,6 +36,7 @@ struct tftp_session { > > struct in_addr client_ip; > uint16_t client_port; > + uint32_t block_nr; > > int timestamp; > }; > -- > 1.6.0.2.GIT > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option 2011-04-11 19:10 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Herve Poussineau 2011-04-11 19:10 ` [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers Herve Poussineau @ 2011-04-16 9:15 ` Stefan Hajnoczi 2011-04-17 18:16 ` Hervé Poussineau 2011-04-17 12:39 ` Avi Kivity 2011-04-18 14:02 ` Aurelien Jarno 3 siblings, 1 reply; 11+ messages in thread From: Stefan Hajnoczi @ 2011-04-16 9:15 UTC (permalink / raw) To: Herve Poussineau; +Cc: qemu-devel On Mon, Apr 11, 2011 at 07:10:52PM +0000, Herve Poussineau wrote: > From: Herv? Poussineau <hpoussin@reactos.org> > > This option is described in RFC 1783. As this is only an optional field, > we may ignore it in some situations and handle it in some others. > Here, if client requests a block size bigger than the block size we emit > (512 bytes), accept the option with a value of 512 > > Signed-off-by: Herv? Poussineau <hpoussin@reactos.org> > --- > slirp/tftp.c | 40 ++++++++++++++++++++++++++++++++-------- > 1 files changed, 32 insertions(+), 8 deletions(-) Have you tested PXELINUX and gPXE? Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option 2011-04-16 9:15 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Stefan Hajnoczi @ 2011-04-17 18:16 ` Hervé Poussineau 2011-04-17 19:53 ` Stefan Hajnoczi 0 siblings, 1 reply; 11+ messages in thread From: Hervé Poussineau @ 2011-04-17 18:16 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Herve Poussineau, qemu-devel Hi, Yes, I've tried booting with PXELINUX and gPXE, and I didn't see any problem. Hervé Stefan Hajnoczi a écrit : > On Mon, Apr 11, 2011 at 07:10:52PM +0000, Herve Poussineau wrote: > >> From: Herv? Poussineau <hpoussin@reactos.org> >> >> This option is described in RFC 1783. As this is only an optional field, >> we may ignore it in some situations and handle it in some others. >> Here, if client requests a block size bigger than the block size we emit >> (512 bytes), accept the option with a value of 512 >> >> Signed-off-by: Herv? Poussineau <hpoussin@reactos.org> >> --- >> slirp/tftp.c | 40 ++++++++++++++++++++++++++++++++-------- >> 1 files changed, 32 insertions(+), 8 deletions(-) >> > > Have you tested PXELINUX and gPXE? > > Stefan > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option 2011-04-17 18:16 ` Hervé Poussineau @ 2011-04-17 19:53 ` Stefan Hajnoczi 0 siblings, 0 replies; 11+ messages in thread From: Stefan Hajnoczi @ 2011-04-17 19:53 UTC (permalink / raw) To: Hervé Poussineau; +Cc: qemu-devel On Sun, Apr 17, 2011 at 7:16 PM, Hervé Poussineau <hpoussin@reactos.org> wrote: > Hi, > > Yes, I've tried booting with PXELINUX and gPXE, and I didn't see any > problem. Great. Thanks for confirming. Stefan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option 2011-04-11 19:10 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Herve Poussineau 2011-04-11 19:10 ` [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers Herve Poussineau 2011-04-16 9:15 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Stefan Hajnoczi @ 2011-04-17 12:39 ` Avi Kivity 2011-04-18 14:02 ` Aurelien Jarno 3 siblings, 0 replies; 11+ messages in thread From: Avi Kivity @ 2011-04-17 12:39 UTC (permalink / raw) To: Herve Poussineau; +Cc: qemu-devel On 04/11/2011 10:10 PM, Herve Poussineau wrote: > From: Herv� Poussineau<hpoussin@reactos.org> Note: your charset is UTF-8 but this is Latin-1 (I think). Please use consistent charsets. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option 2011-04-11 19:10 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Herve Poussineau ` (2 preceding siblings ...) 2011-04-17 12:39 ` Avi Kivity @ 2011-04-18 14:02 ` Aurelien Jarno 3 siblings, 0 replies; 11+ messages in thread From: Aurelien Jarno @ 2011-04-18 14:02 UTC (permalink / raw) To: Herve Poussineau; +Cc: qemu-devel On Mon, Apr 11, 2011 at 07:10:52PM +0000, Herve Poussineau wrote: > From: Herv? Poussineau <hpoussin@reactos.org> > > This option is described in RFC 1783. As this is only an optional field, > we may ignore it in some situations and handle it in some others. > Here, if client requests a block size bigger than the block size we emit > (512 bytes), accept the option with a value of 512 > > Signed-off-by: Herv? Poussineau <hpoussin@reactos.org> > --- > slirp/tftp.c | 40 ++++++++++++++++++++++++++++++++-------- > 1 files changed, 32 insertions(+), 8 deletions(-) The code looks fine except a small indentation issue below. However I don't really see the point of such a patch. This patch basically look for this option, but beside acking it, the behavior doesn't change. On the other hand servers are free to not support all the possible option, in which case they should simply ignore them (i.e. the current behavior). > diff --git a/slirp/tftp.c b/slirp/tftp.c > index 8055ccc..7ef44c9 100644 > --- a/slirp/tftp.c > +++ b/slirp/tftp.c > @@ -116,13 +116,13 @@ static int tftp_read_data(struct tftp_session *spt, uint16_t block_nr, > } > > static int tftp_send_oack(struct tftp_session *spt, > - const char *key, uint32_t value, > + const char *key[], uint32_t value[], int nb, > struct tftp_t *recv_tp) > { > struct sockaddr_in saddr, daddr; > struct mbuf *m; > struct tftp_t *tp; > - int n = 0; > + int i, n = 0; > > m = m_get(spt->slirp); > > @@ -136,10 +136,12 @@ static int tftp_send_oack(struct tftp_session *spt, > m->m_data += sizeof(struct udpiphdr); > > tp->tp_op = htons(TFTP_OACK); > - n += snprintf(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%s", > - key) + 1; > - n += snprintf(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%u", > - value) + 1; > + for (i = 0; i < nb; i++) { > + n += snprintf(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%s", > + key[i]) + 1; > + n += snprintf(tp->x.tp_buf + n, sizeof(tp->x.tp_buf) - n, "%u", > + value[i]) + 1; > + } > > saddr.sin_addr = recv_tp->ip.ip_dst; > saddr.sin_port = recv_tp->udp.uh_dport; > @@ -260,6 +262,9 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen) > int s, k; > size_t prefix_len; > char *req_fname; > + const char *option_name[2]; > + uint32_t option_value[2]; > + int nb_options = 0; > > /* check if a session already exists and if so terminate it */ > s = tftp_session_find(slirp, tp); > @@ -364,9 +369,28 @@ static void tftp_handle_rrq(Slirp *slirp, struct tftp_t *tp, int pktlen) > } > } > > - tftp_send_oack(spt, "tsize", tsize, tp); > - return; > + option_name[nb_options] = "tsize"; > + option_value[nb_options] = tsize; > + nb_options++; > } > + if (strcasecmp(key, "blksize") == 0) { > + int blksize = atoi(value); > + > + /* If blksize option is bigger than what we will > + * emit, accept the option with our packet size. > + * Otherwise, simply do as we didn't see the option. > + */ > + if (blksize >= 512) { > + option_name[nb_options] = "blksize"; > + option_value[nb_options] = 512; > + nb_options++; > + } > + } > + } > + I known slirp/tftp.c has weird indentation, but you should probably try to match the indentation from "tsize" for this black, that is 4 space like in QEMU, except for the initial indentation. > + if (nb_options > 0) { > + tftp_send_oack(spt, option_name, option_value, nb_options, tp); > + return; > } > > tftp_send_data(spt, 1, tp); > -- > 1.6.0.2.GIT > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-04-18 14:04 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-04-11 19:10 [Qemu-devel] [PATCH 0/3] slirp: misc fixes Herve Poussineau 2011-04-11 19:10 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Herve Poussineau 2011-04-11 19:10 ` [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers Herve Poussineau 2011-04-11 19:10 ` [Qemu-devel] [PATCH 3/3] slirp: improve TFTP performance Herve Poussineau 2011-04-18 14:04 ` Aurelien Jarno 2011-04-18 14:02 ` [Qemu-devel] [PATCH 2/3] slirp: Handle more than 65535 blocks in TFTP transfers Aurelien Jarno 2011-04-16 9:15 ` [Qemu-devel] [PATCH 1/3] slirp: Implement TFTP Blocksize option Stefan Hajnoczi 2011-04-17 18:16 ` Hervé Poussineau 2011-04-17 19:53 ` Stefan Hajnoczi 2011-04-17 12:39 ` Avi Kivity 2011-04-18 14:02 ` Aurelien Jarno
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).