[Qemu-devel] [PULL 0/2] slirp updates: MIN/MAX, tftp dynamic blocks

[Qemu-devel] [PULL 0/2] slirp updates: MIN/MAX, tftp dynamic blocks

[Samuel Thibault]

The following changes since commit 82ecffa8c050bf5bbc13329e9b65eac1caa5b55c:

  Open 2.9 development tree (2016-12-20 16:20:16 +0000)

are available in the git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 9443598d7e2f2c0a6493d97b3f11dd04837b08e8:

  slirp: support dynamic block size for TFTP transfers (2016-12-21 00:02:15 +0100)

----------------------------------------------------------------
slirp updates

----------------------------------------------------------------
Hervé Poussineau (1):
      slirp: support dynamic block size for TFTP transfers

Yuval Shaia (1):
      slirp, disas: Replace min/max with MIN/MAX macros

 disas/m68k.c       |  8 ++------
 slirp/dhcpv6.c     |  2 +-
 slirp/ip6_icmp.c   |  2 +-
 slirp/slirp.c      |  2 +-
 slirp/slirp.h      |  5 -----
 slirp/tcp_input.c  | 16 ++++++++--------
 slirp/tcp_output.c |  6 +++---
 slirp/tcp_timer.c  |  2 +-
 slirp/tcpip.h      |  2 +-
 slirp/tftp.c       | 26 ++++++++++++++------------
 slirp/tftp.h       |  8 +++++---
 11 files changed, 37 insertions(+), 42 deletions(-)

[Qemu-devel] [PULL 1/2] slirp, disas: Replace min/max with MIN/MAX macros

[Samuel Thibault]

From: Yuval Shaia <yuval.shaia@oracle.com>

Signed-off-by: Yuval Shaia <yuval.shaia@oracle.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 disas/m68k.c       |  8 ++------
 slirp/dhcpv6.c     |  2 +-
 slirp/ip6_icmp.c   |  2 +-
 slirp/slirp.c      |  2 +-
 slirp/slirp.h      |  5 -----
 slirp/tcp_input.c  | 16 ++++++++--------
 slirp/tcp_output.c |  6 +++---
 slirp/tcp_timer.c  |  2 +-
 slirp/tcpip.h      |  2 +-
 9 files changed, 18 insertions(+), 27 deletions(-)

diff --git a/disas/m68k.c b/disas/m68k.c
index 8e7c3f76c4..073abb9efd 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -4698,10 +4698,6 @@ get_field (const unsigned char *data, enum floatformat_byteorders order,
   return result;
 }
 
-#ifndef min
-#define min(a, b) ((a) < (b) ? (a) : (b))
-#endif
-
 /* Convert from FMT to a double.
    FROM is the address of the extended float.
    Store the double in *TO.  */
@@ -4733,7 +4729,7 @@ floatformat_to_double (const struct floatformat *fmt,
       nan = 0;
       while (mant_bits_left > 0)
 	{
-	  mant_bits = min (mant_bits_left, 32);
+          mant_bits = MIN(mant_bits_left, 32);
 
 	  if (get_field (ufrom, fmt->byteorder, fmt->totalsize,
 			 mant_off, mant_bits) != 0)
@@ -4793,7 +4789,7 @@ floatformat_to_double (const struct floatformat *fmt,
 
   while (mant_bits_left > 0)
     {
-      mant_bits = min (mant_bits_left, 32);
+      mant_bits = MIN(mant_bits_left, 32);
 
       mant = get_field (ufrom, fmt->byteorder, fmt->totalsize,
 			 mant_off, mant_bits);
diff --git a/slirp/dhcpv6.c b/slirp/dhcpv6.c
index 02c51c7756..d266611e85 100644
--- a/slirp/dhcpv6.c
+++ b/slirp/dhcpv6.c
@@ -168,7 +168,7 @@ static void dhcpv6_info_request(Slirp *slirp, struct sockaddr_in6 *srcsas,
                         sa[0], sa[1], sa[2], sa[3], sa[4], sa[5], sa[6], sa[7],
                         sa[8], sa[9], sa[10], sa[11], sa[12], sa[13], sa[14],
                         sa[15], slirp->bootp_filename);
-        slen = min(slen, smaxlen);
+        slen = MIN(slen, smaxlen);
         *resp++ = slen >> 8;                    /* option-len high byte */
         *resp++ = slen;                         /* option-len low byte */
         resp += slen;
diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c
index 6d18e28985..298a48dd25 100644
--- a/slirp/ip6_icmp.c
+++ b/slirp/ip6_icmp.c
@@ -95,7 +95,7 @@ void icmp6_send_error(struct mbuf *m, uint8_t type, uint8_t code)
 #endif
 
     rip->ip_nh = IPPROTO_ICMPV6;
-    const int error_data_len = min(m->m_len,
+    const int error_data_len = MIN(m->m_len,
             IF_MTU - (sizeof(struct ip6) + ICMP6_ERROR_MINLEN));
     rip->ip_pl = htons(ICMP6_ERROR_MINLEN + error_data_len);
     t->m_len = sizeof(struct ip6) + ntohs(rip->ip_pl);
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 6e2b4e5a90..60539de7a3 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -774,7 +774,7 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
 static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
 {
     struct slirp_arphdr *ah = (struct slirp_arphdr *)(pkt + ETH_HLEN);
-    uint8_t arp_reply[max(ETH_HLEN + sizeof(struct slirp_arphdr), 64)];
+    uint8_t arp_reply[MAX(ETH_HLEN + sizeof(struct slirp_arphdr), 64)];
     struct ethhdr *reh = (struct ethhdr *)arp_reply;
     struct slirp_arphdr *rah = (struct slirp_arphdr *)(arp_reply + ETH_HLEN);
     int ar_op;
diff --git a/slirp/slirp.h b/slirp/slirp.h
index a1f3139134..3877f667f0 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -292,9 +292,4 @@ int tcp_emu(struct socket *, struct mbuf *);
 int tcp_ctl(struct socket *);
 struct tcpcb *tcp_drop(struct tcpcb *tp, int err);
 
-#ifndef _WIN32
-#define min(x,y) ((x) < (y) ? (x) : (y))
-#define max(x,y) ((x) > (y) ? (x) : (y))
-#endif
-
 #endif
diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
index c5063a918d..edb98f06f3 100644
--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -596,7 +596,7 @@ findso:
           win = sbspace(&so->so_rcv);
 	  if (win < 0)
 	    win = 0;
-	  tp->rcv_wnd = max(win, (int)(tp->rcv_adv - tp->rcv_nxt));
+          tp->rcv_wnd = MAX(win, (int)(tp->rcv_adv - tp->rcv_nxt));
 	}
 
 	switch (tp->t_state) {
@@ -1065,8 +1065,8 @@ trimthenstep6:
 				else if (++tp->t_dupacks == TCPREXMTTHRESH) {
 					tcp_seq onxt = tp->snd_nxt;
 					u_int win =
-					    min(tp->snd_wnd, tp->snd_cwnd) / 2 /
-						tp->t_maxseg;
+                                                MIN(tp->snd_wnd, tp->snd_cwnd) /
+                                                2 / tp->t_maxseg;
 
 					if (win < 2)
 						win = 2;
@@ -1138,7 +1138,7 @@ trimthenstep6:
 
 		  if (cw > tp->snd_ssthresh)
 		    incr = incr * incr / cw;
-		  tp->snd_cwnd = min(cw + incr, TCP_MAXWIN<<tp->snd_scale);
+                  tp->snd_cwnd = MIN(cw + incr, TCP_MAXWIN << tp->snd_scale);
 		}
 		if (acked > so->so_snd.sb_cc) {
 			tp->snd_wnd -= so->so_snd.sb_cc;
@@ -1586,11 +1586,11 @@ tcp_mss(struct tcpcb *tp, u_int offer)
 
 	switch (so->so_ffamily) {
 	case AF_INET:
-	    mss = min(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
+            mss = MIN(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
 	                              + sizeof(struct ip);
 	    break;
 	case AF_INET6:
-	    mss = min(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
+            mss = MIN(IF_MTU, IF_MRU) - sizeof(struct tcphdr)
 	                              + sizeof(struct ip6);
 	    break;
 	default:
@@ -1598,8 +1598,8 @@ tcp_mss(struct tcpcb *tp, u_int offer)
 	}
 
 	if (offer)
-		mss = min(mss, offer);
-	mss = max(mss, 32);
+            mss = MIN(mss, offer);
+        mss = MAX(mss, 32);
 	if (mss < tp->t_maxseg || offer != 0)
 	   tp->t_maxseg = mss;
 
diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c
index 819db27348..90b5c376f7 100644
--- a/slirp/tcp_output.c
+++ b/slirp/tcp_output.c
@@ -88,7 +88,7 @@ tcp_output(struct tcpcb *tp)
 again:
 	sendalot = 0;
 	off = tp->snd_nxt - tp->snd_una;
-	win = min(tp->snd_wnd, tp->snd_cwnd);
+        win = MIN(tp->snd_wnd, tp->snd_cwnd);
 
 	flags = tcp_outflags[tp->t_state];
 
@@ -127,7 +127,7 @@ again:
 		}
 	}
 
-	len = min(so->so_snd.sb_cc, win) - off;
+        len = MIN(so->so_snd.sb_cc, win) - off;
 
 	if (len < 0) {
 		/*
@@ -193,7 +193,7 @@ again:
 		 * taking into account that we are limited by
 		 * TCP_MAXWIN << tp->rcv_scale.
 		 */
-		long adv = min(win, (long)TCP_MAXWIN << tp->rcv_scale) -
+                long adv = MIN(win, (long)TCP_MAXWIN << tp->rcv_scale) -
 			(tp->rcv_adv - tp->rcv_nxt);
 
 		if (adv >= (long) (2 * tp->t_maxseg))
diff --git a/slirp/tcp_timer.c b/slirp/tcp_timer.c
index f9060c7bf8..52ef5f9100 100644
--- a/slirp/tcp_timer.c
+++ b/slirp/tcp_timer.c
@@ -233,7 +233,7 @@ tcp_timers(register struct tcpcb *tp, int timer)
 		 * to go below this.)
 		 */
 		{
-		u_int win = min(tp->snd_wnd, tp->snd_cwnd) / 2 / tp->t_maxseg;
+                u_int win = MIN(tp->snd_wnd, tp->snd_cwnd) / 2 / tp->t_maxseg;
 		if (win < 2)
 			win = 2;
 		tp->snd_cwnd = tp->t_maxseg;
diff --git a/slirp/tcpip.h b/slirp/tcpip.h
index 7bdb971c5d..07dbf2c432 100644
--- a/slirp/tcpip.h
+++ b/slirp/tcpip.h
@@ -85,7 +85,7 @@ struct tcpiphdr {
 /* This is the difference between the size of a tcpiphdr structure, and the
  * size of actual ip+tcp headers, rounded up since we need to align data.  */
 #define TCPIPHDR_DELTA\
-    (max(0,\
+    (MAX(0,\
          (sizeof(struct tcpiphdr)\
           - sizeof(struct ip) - sizeof(struct tcphdr) + 3) & ~3))
 
-- 
2.11.0

[Qemu-devel] [PULL 2/2] slirp: support dynamic block size for TFTP transfers

[Samuel Thibault]

From: Hervé Poussineau <hpoussin@reactos.org>

The blocksize option is defined in RFC 1783 and RFC 2348.
We now support block sizes between 1 and 1428 bytes, instead of 512 only.

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/tftp.c | 26 ++++++++++++++------------
 slirp/tftp.h |  8 +++++---
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/slirp/tftp.c b/slirp/tftp.c
index c1859066cc..50e714807d 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -72,6 +72,7 @@ static int tftp_session_allocate(Slirp *slirp, struct sockaddr_storage *srcsas,
   memset(spt, 0, sizeof(*spt));
   spt->client_addr = *srcsas;
   spt->fd = -1;
+  spt->block_size = 512;
   spt->client_port = tp->udp.uh_sport;
   spt->slirp = slirp;
 
@@ -115,7 +116,7 @@ static int tftp_read_data(struct tftp_session *spt, uint32_t block_nr,
     }
 
     if (len) {
-        lseek(spt->fd, block_nr * 512, SEEK_SET);
+        lseek(spt->fd, block_nr * spt->block_size, SEEK_SET);
 
         bytes_read = read(spt->fd, buf, len);
     }
@@ -189,7 +190,8 @@ static int tftp_send_oack(struct tftp_session *spt,
                       values[i]) + 1;
     }
 
-    m->m_len = sizeof(struct tftp_t) - 514 + n - sizeof(struct udphdr);
+    m->m_len = sizeof(struct tftp_t) - (TFTP_BLOCKSIZE_MAX + 2) + n
+               - sizeof(struct udphdr);
     tftp_udp_output(spt, m, recv_tp);
 
     return 0;
@@ -214,7 +216,7 @@ static void tftp_send_error(struct tftp_session *spt,
   tp->x.tp_error.tp_error_code = htons(errorcode);
   pstrcpy((char *)tp->x.tp_error.tp_msg, sizeof(tp->x.tp_error.tp_msg), msg);
 
-  m->m_len = sizeof(struct tftp_t) - 514 + 3 + strlen(msg)
+  m->m_len = sizeof(struct tftp_t) - (TFTP_BLOCKSIZE_MAX + 2) + 3 + strlen(msg)
              - sizeof(struct udphdr);
   tftp_udp_output(spt, m, recv_tp);
 
@@ -240,7 +242,8 @@ static void tftp_send_next_block(struct tftp_session *spt,
   tp->tp_op = htons(TFTP_DATA);
   tp->x.tp_data.tp_block_nr = htons((spt->block_nr + 1) & 0xffff);
 
-  nobytes = tftp_read_data(spt, spt->block_nr, tp->x.tp_data.tp_buf, 512);
+  nobytes = tftp_read_data(spt, spt->block_nr, tp->x.tp_data.tp_buf,
+                           spt->block_size);
 
   if (nobytes < 0) {
     m_free(m);
@@ -252,10 +255,11 @@ static void tftp_send_next_block(struct tftp_session *spt,
     return;
   }
 
-  m->m_len = sizeof(struct tftp_t) - (512 - nobytes) - sizeof(struct udphdr);
+  m->m_len = sizeof(struct tftp_t) - (TFTP_BLOCKSIZE_MAX - nobytes)
+             - sizeof(struct udphdr);
   tftp_udp_output(spt, m, recv_tp);
 
-  if (nobytes == 512) {
+  if (nobytes == spt->block_size) {
     tftp_session_update(spt);
   }
   else {
@@ -385,13 +389,11 @@ static void tftp_handle_rrq(Slirp *slirp, struct sockaddr_storage *srcsas,
       } else 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) {
+          /* Accept blksize up to our maximum size */
+          if (blksize > 0) {
+              spt->block_size = MIN(blksize, TFTP_BLOCKSIZE_MAX);
               option_name[nb_options] = "blksize";
-              option_value[nb_options] = 512;
+              option_value[nb_options] = spt->block_size;
               nb_options++;
           }
       }
diff --git a/slirp/tftp.h b/slirp/tftp.h
index 2cd276dec6..a4c4a64e64 100644
--- a/slirp/tftp.h
+++ b/slirp/tftp.h
@@ -15,6 +15,7 @@
 #define TFTP_OACK   6
 
 #define TFTP_FILENAME_MAX 512
+#define TFTP_BLOCKSIZE_MAX 1428
 
 struct tftp_t {
   struct udphdr udp;
@@ -22,13 +23,13 @@ struct tftp_t {
   union {
     struct {
       uint16_t tp_block_nr;
-      uint8_t tp_buf[512];
+      uint8_t tp_buf[TFTP_BLOCKSIZE_MAX];
     } tp_data;
     struct {
       uint16_t tp_error_code;
-      uint8_t tp_msg[512];
+      uint8_t tp_msg[TFTP_BLOCKSIZE_MAX];
     } tp_error;
-    char tp_buf[512 + 2];
+    char tp_buf[TFTP_BLOCKSIZE_MAX + 2];
   } x;
 } __attribute__((packed));
 
@@ -36,6 +37,7 @@ struct tftp_session {
     Slirp *slirp;
     char *filename;
     int fd;
+    uint16_t block_size;
 
     struct sockaddr_storage client_addr;
     uint16_t client_port;
-- 
2.11.0

Re: [Qemu-devel] [PULL 0/2] slirp updates: MIN/MAX, tftp dynamic blocks

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20161220230732.17862-1-samuel.thibault@ens-lyon.org
Subject: [Qemu-devel] [PULL 0/2] slirp updates: MIN/MAX, tftp dynamic blocks

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20161220230732.17862-1-samuel.thibault@ens-lyon.org -> patchew/20161220230732.17862-1-samuel.thibault@ens-lyon.org
Switched to a new branch 'test'
9c5544e slirp: support dynamic block size for TFTP transfers
8de1d56 slirp, disas: Replace min/max with MIN/MAX macros

=== OUTPUT BEGIN ===
Checking PATCH 1/2: slirp, disas: Replace min/max with MIN/MAX macros...
Checking PATCH 2/2: slirp: support dynamic block size for TFTP transfers...
ERROR: suspect code indent for conditional statements (10, 14)
#90: FILE: slirp/tftp.c:393:
+          if (blksize > 0) {
+              spt->block_size = MIN(blksize, TFTP_BLOCKSIZE_MAX);

total: 1 errors, 0 warnings, 101 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PULL 0/2] slirp updates: MIN/MAX, tftp dynamic blocks

[Samuel Thibault]

no-reply@patchew.org, on Tue 20 Dec 2016 15:15:43 -0800, wrote:
> Your series seems to have some coding style problems. See output below for
> more information:


> ERROR: suspect code indent for conditional statements (10, 14)
> #90: FILE: slirp/tftp.c:393:
> +          if (blksize > 0) {
> +              spt->block_size = MIN(blksize, TFTP_BLOCKSIZE_MAX);

This is a false positive.

Samuel

Re: [Qemu-devel] [PULL 0/2] slirp updates: MIN/MAX, tftp dynamic blocks

[Peter Maydell]

On 20 December 2016 at 23:07, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
> The following changes since commit 82ecffa8c050bf5bbc13329e9b65eac1caa5b55c:
>
>   Open 2.9 development tree (2016-12-20 16:20:16 +0000)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 9443598d7e2f2c0a6493d97b3f11dd04837b08e8:
>
>   slirp: support dynamic block size for TFTP transfers (2016-12-21 00:02:15 +0100)
>
> ----------------------------------------------------------------
> slirp updates
>
> ----------------------------------------------------------------
> Hervé Poussineau (1):
>       slirp: support dynamic block size for TFTP transfers
>
> Yuval Shaia (1):
>       slirp, disas: Replace min/max with MIN/MAX macros
>

Applied, thanks.

-- PMM