* [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 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-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-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
` (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
* 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 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
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).