From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49632) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWZ40-0006h2-IV for qemu-devel@nongnu.org; Thu, 18 Feb 2016 19:36:54 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWZ3v-0006pE-Hj for qemu-devel@nongnu.org; Thu, 18 Feb 2016 19:36:52 -0500 Received: from domu-toccata.ens-lyon.fr ([140.77.166.138]:38724 helo=sonata.ens-lyon.org) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWZ3v-0006p6-5f for qemu-devel@nongnu.org; Thu, 18 Feb 2016 19:36:47 -0500 Date: Fri, 19 Feb 2016 01:36:45 +0100 From: Samuel Thibault Message-ID: <20160219003645.GV2679@var.home> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCHv7 4/9] slirp: Factorizing tcpiphdr structure with an union List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Thomas Huth , zhanghailiang , Li Zhijian , Stefan Hajnoczi , Jason Wang , Dave Gilbert , Vasiliy Tolstov , Huangpeng , Gonglei , Jan Kiszka , Guillaume Subiron Hello, Just to make sure: we have not received comments on this patch. (that said, it's the most complex part of the series, so I'm not surprised if it takes more time :) ) Samuel Samuel Thibault, on Sun 14 Feb 2016 18:47:38 +0100, wrote: > From: Guillaume Subiron >=20 > This patch factorizes the tcpiphdr structure to put the IPv4 fields in > an union, for addition of version 6 in further patch. > Using some macros, retrocompatibility of the existing code is assured. >=20 > This patch also fixes the SLIRP_MSIZE and margin computation in various > functions, and makes them compatible with the new tcpiphdr structure, > whose size will be bigger than sizeof(struct tcphdr) + sizeof(struct ip= ) >=20 > Signed-off-by: Guillaume Subiron > Signed-off-by: Samuel Thibault > --- > slirp/if.h | 4 ++-- > slirp/mbuf.c | 3 ++- > slirp/slirp.c | 15 ++++++++------- > slirp/socket.c | 13 ++++++++++++- > slirp/tcp_input.c | 31 ++++++++++++++++++++----------- > slirp/tcp_output.c | 18 +++++++++++++----- > slirp/tcp_subr.c | 31 ++++++++++++++++++++++--------- > slirp/tcpip.h | 31 +++++++++++++++++++++++-------- > 8 files changed, 102 insertions(+), 44 deletions(-) >=20 > diff --git a/slirp/if.h b/slirp/if.h > index 3327023..c7a5c57 100644 > --- a/slirp/if.h > +++ b/slirp/if.h > @@ -17,7 +17,7 @@ > #define IF_MRU 1500 > #define IF_COMP IF_AUTOCOMP /* Flags for compression */ > =20 > -/* 2 for alignment, 14 for ethernet, 40 for TCP/IP */ > -#define IF_MAXLINKHDR (2 + 14 + 40) > +/* 2 for alignment, 14 for ethernet */ > +#define IF_MAXLINKHDR (2 + ETH_HLEN) > =20 > #endif > diff --git a/slirp/mbuf.c b/slirp/mbuf.c > index c959758..f081c69 100644 > --- a/slirp/mbuf.c > +++ b/slirp/mbuf.c > @@ -24,7 +24,8 @@ > * Find a nice value for msize > * XXX if_maxlinkhdr already in mtu > */ > -#define SLIRP_MSIZE (IF_MTU + IF_MAXLINKHDR + offsetof(struct mbuf, m_= dat) + 6) > +#define SLIRP_MSIZE\ > + (offsetof(struct mbuf, m_dat) + IF_MAXLINKHDR + TCPIPHDR_DELTA + I= F_MTU) > =20 > void > m_init(Slirp *slirp) > diff --git a/slirp/slirp.c b/slirp/slirp.c > index 5f42ada..c2c4597 100644 > --- a/slirp/slirp.c > +++ b/slirp/slirp.c > @@ -754,15 +754,16 @@ void slirp_input(Slirp *slirp, const uint8_t *pkt= , int pkt_len) > m =3D m_get(slirp); > if (!m) > return; > - /* Note: we add to align the IP header */ > - if (M_FREEROOM(m) < pkt_len + 2) { > - m_inc(m, pkt_len + 2); > + /* Note: we add 2 to align the IP header on 4 bytes, > + * and add the margin for the tcpiphdr overhead */ > + if (M_FREEROOM(m) < pkt_len + TCPIPHDR_DELTA + 2) { > + m_inc(m, pkt_len + TCPIPHDR_DELTA + 2); > } > - m->m_len =3D pkt_len + 2; > - memcpy(m->m_data + 2, pkt, pkt_len); > + m->m_len =3D pkt_len + TCPIPHDR_DELTA + 2; > + memcpy(m->m_data + TCPIPHDR_DELTA + 2, pkt, pkt_len); > =20 > - m->m_data +=3D 2 + ETH_HLEN; > - m->m_len -=3D 2 + ETH_HLEN; > + m->m_data +=3D TCPIPHDR_DELTA + 2 + ETH_HLEN; > + m->m_len -=3D TCPIPHDR_DELTA + 2 + ETH_HLEN; > =20 > if (proto =3D=3D ETH_P_IP) { > ip_input(m); > diff --git a/slirp/socket.c b/slirp/socket.c > index b79ddec..d4b02c8 100644 > --- a/slirp/socket.c > +++ b/slirp/socket.c > @@ -483,7 +483,18 @@ sorecvfrom(struct socket *so) > if (!m) { > return; > } > - m->m_data +=3D IF_MAXLINKHDR; > + switch (so->so_ffamily) { > + case AF_INET: > + m->m_data +=3D IF_MAXLINKHDR + sizeof(struct udpiphdr); > + break; > + case AF_INET6: > + m->m_data +=3D IF_MAXLINKHDR + sizeof(struct ip6) > + + sizeof(struct udphdr); > + break; > + default: > + g_assert_not_reached(); > + break; > + } > =20 > /* > * XXX Shouldn't FIONREAD packets destined for port 53, > diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c > index 5f845da..26b0c8b 100644 > --- a/slirp/tcp_input.c > +++ b/slirp/tcp_input.c > @@ -256,11 +256,6 @@ tcp_input(struct mbuf *m, int iphlen, struct socke= t *inso) > } > slirp =3D m->slirp; > =20 > - /* > - * Get IP and TCP header together in first mbuf. > - * Note: IP leaves IP header in first mbuf. > - */ > - ti =3D mtod(m, struct tcpiphdr *); > if (iphlen > sizeof(struct ip )) { > ip_stripoptions(m, (struct mbuf *)0); > iphlen=3Dsizeof(struct ip ); > @@ -277,14 +272,28 @@ tcp_input(struct mbuf *m, int iphlen, struct sock= et *inso) > save_ip.ip_len+=3D iphlen; > =20 > /* > + * Get IP and TCP header together in first mbuf. > + * Note: IP leaves IP header in first mbuf. > + */ > + m->m_data -=3D sizeof(struct tcpiphdr) - (sizeof(struct ip) > + + sizeof(struct tcphdr)); > + m->m_len +=3D sizeof(struct tcpiphdr) - (sizeof(struct ip) > + + sizeof(struct tcphdr)); > + ti =3D mtod(m, struct tcpiphdr *); > + > + /* > * Checksum extended TCP header and data. > */ > - tlen =3D ((struct ip *)ti)->ip_len; > - tcpiphdr2qlink(ti)->next =3D tcpiphdr2qlink(ti)->prev =3D NULL= ; > - memset(&ti->ti_i.ih_mbuf, 0 , sizeof(struct mbuf_ptr)); > - ti->ti_x1 =3D 0; > + tlen =3D ip->ip_len; > + tcpiphdr2qlink(ti)->next =3D tcpiphdr2qlink(ti)->prev =3D NULL; > + memset(&ti->ih_mbuf, 0 , sizeof(struct mbuf_ptr)); > + memset(&ti->ti, 0, sizeof(ti->ti)); > + ti->ti_x0 =3D 0; > + ti->ti_src =3D save_ip.ip_src; > + ti->ti_dst =3D save_ip.ip_dst; > + ti->ti_pr =3D save_ip.ip_p; > ti->ti_len =3D htons((uint16_t)tlen); > - len =3D sizeof(struct ip ) + tlen; > + len =3D ((sizeof(struct tcpiphdr) - sizeof(struct tcphdr)) + tlen); > if(cksum(m, len)) { > goto drop; > } > @@ -1475,7 +1484,7 @@ tcp_mss(struct tcpcb *tp, u_int offer) > DEBUG_ARG("tp =3D %p", tp); > DEBUG_ARG("offer =3D %d", offer); > =20 > - mss =3D min(IF_MTU, IF_MRU) - sizeof(struct tcpiphdr); > + mss =3D min(IF_MTU, IF_MRU) - sizeof(struct tcphdr) + sizeof(struct i= p); > if (offer) > mss =3D min(mss, offer); > mss =3D max(mss, 32); > diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c > index 34e4d2e..7fc6a87 100644 > --- a/slirp/tcp_output.c > +++ b/slirp/tcp_output.c > @@ -448,15 +448,23 @@ send: > */ > m->m_len =3D hdrlen + len; /* XXX Needed? m_len should be correct */ > =20 > - { > + struct tcpiphdr tcpiph_save =3D *(mtod(m, struct tcpiphdr *)); > + m->m_data +=3D sizeof(struct tcpiphdr) - sizeof(struct tcphdr) > + - sizeof(struct ip); > + m->m_len -=3D sizeof(struct tcpiphdr) - sizeof(struct tcphdr) > + - sizeof(struct ip); > + struct ip *ip =3D mtod(m, struct ip *); > =20 > - ((struct ip *)ti)->ip_len =3D m->m_len; > + ip->ip_len =3D m->m_len; > + ip->ip_dst =3D tcpiph_save.ti_dst; > + ip->ip_src =3D tcpiph_save.ti_src; > + ip->ip_p =3D tcpiph_save.ti_pr; > =20 > - ((struct ip *)ti)->ip_ttl =3D IPDEFTTL; > - ((struct ip *)ti)->ip_tos =3D so->so_iptos; > + ip->ip_ttl =3D IPDEFTTL; > + ip->ip_tos =3D so->so_iptos; > =20 > error =3D ip_output(so, m); > - } > + > if (error) { > out: > return (error); > diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c > index b1aa1f2..cd021df 100644 > --- a/slirp/tcp_subr.c > +++ b/slirp/tcp_subr.c > @@ -76,9 +76,10 @@ tcp_template(struct tcpcb *tp) > register struct tcpiphdr *n =3D &tp->t_template; > =20 > n->ti_mbuf =3D NULL; > - n->ti_x1 =3D 0; > + memset(&n->ti, 0, sizeof(n->ti)); > + n->ti_x0 =3D 0; > n->ti_pr =3D IPPROTO_TCP; > - n->ti_len =3D htons(sizeof (struct tcpiphdr) - sizeof (struct ip)); > + n->ti_len =3D htons(sizeof(struct tcphdr)); > n->ti_src =3D so->so_faddr; > n->ti_dst =3D so->so_laddr; > n->ti_sport =3D so->so_fport; > @@ -131,6 +132,7 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, = struct mbuf *m, > m->m_data +=3D IF_MAXLINKHDR; > *mtod(m, struct tcpiphdr *) =3D *ti; > ti =3D mtod(m, struct tcpiphdr *); > + memset(&ti->ti, 0, sizeof(ti->ti)); > flags =3D TH_ACK; > } else { > /* > @@ -150,8 +152,8 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, = struct mbuf *m, > tlen +=3D sizeof (struct tcpiphdr); > m->m_len =3D tlen; > =20 > - ti->ti_mbuf =3D NULL; > - ti->ti_x1 =3D 0; > + ti->ti_mbuf =3D NULL; > + ti->ti_x0 =3D 0; > ti->ti_seq =3D htonl(seq); > ti->ti_ack =3D htonl(ack); > ti->ti_x2 =3D 0; > @@ -164,12 +166,23 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti= , struct mbuf *m, > ti->ti_urp =3D 0; > ti->ti_sum =3D 0; > ti->ti_sum =3D cksum(m, tlen); > - ((struct ip *)ti)->ip_len =3D tlen; > =20 > - if(flags & TH_RST) > - ((struct ip *)ti)->ip_ttl =3D MAXTTL; > - else > - ((struct ip *)ti)->ip_ttl =3D IPDEFTTL; > + struct tcpiphdr tcpiph_save =3D *(mtod(m, struct tcpiphdr *)); > + m->m_data +=3D sizeof(struct tcpiphdr) - sizeof(struct tcphdr) > + - sizeof(struct ip); > + m->m_len -=3D sizeof(struct tcpiphdr) - sizeof(struct tcphdr) > + - sizeof(struct ip); > + struct ip *ip =3D mtod(m, struct ip *); > + ip->ip_len =3D tlen; > + ip->ip_dst =3D tcpiph_save.ti_dst; > + ip->ip_src =3D tcpiph_save.ti_src; > + ip->ip_p =3D tcpiph_save.ti_pr; > + > + if (flags & TH_RST) { > + ip->ip_ttl =3D MAXTTL; > + } else { > + ip->ip_ttl =3D IPDEFTTL; > + } > =20 > (void) ip_output((struct socket *)0, m); > } > diff --git a/slirp/tcpip.h b/slirp/tcpip.h > index 7974ce3..3c5d127 100644 > --- a/slirp/tcpip.h > +++ b/slirp/tcpip.h > @@ -37,15 +37,23 @@ > * Tcp+ip header, after ip options removed. > */ > struct tcpiphdr { > - struct ipovly ti_i; /* overlaid ip structure */ > - struct tcphdr ti_t; /* tcp header */ > + struct mbuf_ptr ih_mbuf; /* backpointer to mbuf */ > + union { > + struct { > + struct in_addr ih_src; /* source internet address */ > + struct in_addr ih_dst; /* destination internet address */ > + uint8_t ih_x1; /* (unused) */ > + uint8_t ih_pr; /* protocol */ > + } ti_i4; > + } ti; > + uint16_t ti_x0; > + uint16_t ti_len; /* protocol length */ > + struct tcphdr ti_t; /* tcp header */ > }; > -#define ti_mbuf ti_i.ih_mbuf.mptr > -#define ti_x1 ti_i.ih_x1 > -#define ti_pr ti_i.ih_pr > -#define ti_len ti_i.ih_len > -#define ti_src ti_i.ih_src > -#define ti_dst ti_i.ih_dst > +#define ti_mbuf ih_mbuf.mptr > +#define ti_pr ti.ti_i4.ih_pr > +#define ti_src ti.ti_i4.ih_src > +#define ti_dst ti.ti_i4.ih_dst > #define ti_sport ti_t.th_sport > #define ti_dport ti_t.th_dport > #define ti_seq ti_t.th_seq > @@ -65,6 +73,13 @@ struct tcpiphdr { > #define tcpfrag_list_end(F, T) (tcpiphdr2qlink(F) =3D=3D (struct qlink= *)(T)) > #define tcpfrag_list_empty(T) ((T)->seg_next =3D=3D (struct tcpiphdr*)= (T)) > =20 > +/* This is the difference between the size of a tcpiphdr structure, an= d the > + * size of actual ip+tcp headers, rounded up since we need to align da= ta. */ > +#define TCPIPHDR_DELTA\ > + (max(0,\ > + (sizeof(struct tcpiphdr)\ > + - sizeof(struct ip) - sizeof(struct tcphdr) + 3) & ~3)) > + > /* > * Just a clean way to get to the first byte > * of the packet > --=20 > 2.7.0 >=20 >=20 --=20 Samuel > X..., c'est un millefeuille avec une couche de cr=E8me patissi=E8re, un= e > de sauce tomate et une de cr=E8me d'anchois... Mais c'est vrai que > c'est un syst=E8me ouvert: tu peux y rajouter des p=E9pites de chocolat= ... -+- Ol in Guide du linuxien pervers - "Remettez m'en une couche !" -+-