From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52461) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTQFT-00037p-18 for qemu-devel@nongnu.org; Wed, 10 Feb 2016 03:35:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTQFP-0000K3-PZ for qemu-devel@nongnu.org; Wed, 10 Feb 2016 03:35:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58453) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTQFP-0000Jw-FE for qemu-devel@nongnu.org; Wed, 10 Feb 2016 03:35:39 -0500 References: <145e7741b4d6eeac3d69ea9cf40110229167db0c.1454927009.git.samuel.thibault@ens-lyon.org> From: Thomas Huth Message-ID: <56BAF653.2040507@redhat.com> Date: Wed, 10 Feb 2016 09:35:31 +0100 MIME-Version: 1.0 In-Reply-To: <145e7741b4d6eeac3d69ea9cf40110229167db0c.1454927009.git.samuel.thibault@ens-lyon.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCHv7 5/9] slirp: Generalizing and neutralizing various TCP functions before adding IPv6 stuff List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Samuel Thibault , qemu-devel@nongnu.org Cc: zhanghailiang , Li Zhijian , Stefan Hajnoczi , Jason Wang , Dave Gilbert , Vasiliy Tolstov , Huangpeng , Gonglei , Jan Kiszka , Yang Hongyang , Guillaume Subiron On 08.02.2016 11:28, Samuel Thibault wrote: > From: Guillaume Subiron >=20 > Basically, this patch adds some switch in various TCP functions to > prepare them for the IPv6 case. >=20 > To have something to "switch" in tcp_input() and tcp_respond(), a new > argument is used to give them the sa_family of the addresses they are > working on. >=20 > This patch does not include the entailed reindentation, to make proofre= ad > easier. Reindentation is adressed in the following no-op patch. >=20 > Signed-off-by: Guillaume Subiron > Signed-off-by: Samuel Thibault > --- > slirp/ip_input.c | 2 +- > slirp/slirp.c | 6 ++++-- > slirp/slirp.h | 5 +++-- > slirp/tcp_input.c | 52 ++++++++++++++++++++++++++++++++++++++++++++--= ------ > slirp/tcp_output.c | 14 +++++++++++--- > slirp/tcp_subr.c | 37 +++++++++++++++++++++++++++++-------- > slirp/tcp_timer.c | 3 ++- > 7 files changed, 94 insertions(+), 25 deletions(-) ... > diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c > index 26b0c8b..0cc279b 100644 > --- a/slirp/tcp_input.c > +++ b/slirp/tcp_input.c > @@ -214,7 +214,7 @@ present: > * protocol specification dated September, 1981 very closely. > */ > void > -tcp_input(struct mbuf *m, int iphlen, struct socket *inso) > +tcp_input(struct mbuf *m, int iphlen, struct socket *inso, unsigned sh= ort af) > { > struct ip save_ip, *ip; > register struct tcpiphdr *ti; > @@ -256,6 +256,8 @@ tcp_input(struct mbuf *m, int iphlen, struct socket= *inso) > } > slirp =3D m->slirp; > =20 > + switch (af) { > + case AF_INET: > if (iphlen > sizeof(struct ip )) { > ip_stripoptions(m, (struct mbuf *)0); > iphlen=3Dsizeof(struct ip ); > @@ -297,6 +299,11 @@ tcp_input(struct mbuf *m, int iphlen, struct socke= t *inso) > if(cksum(m, len)) { > goto drop; > } > + break; > + > + default: > + goto drop; > + } > =20 > /* > * Check that TCP offset makes sense, > @@ -332,14 +339,20 @@ tcp_input(struct mbuf *m, int iphlen, struct sock= et *inso) > * Locate pcb for segment. > */ > findso: > - lhost.ss_family =3D AF_INET; > + lhost.ss_family =3D af; > + fhost.ss_family =3D af; > + switch (af) { > + case AF_INET: > lhost4 =3D (struct sockaddr_in *) &lhost; > lhost4->sin_addr =3D ti->ti_src; > lhost4->sin_port =3D ti->ti_sport; > - fhost.ss_family =3D AF_INET; > fhost4 =3D (struct sockaddr_in *) &fhost; > fhost4->sin_addr =3D ti->ti_dst; > fhost4->sin_port =3D ti->ti_dport; > + break; > + default: > + goto drop; > + } > =20 > so =3D solookup(&slirp->tcp_last_so, &slirp->tcb, &lhost, &fhost); > =20 > @@ -389,8 +402,17 @@ findso: > so->lhost.ss =3D lhost; > so->fhost.ss =3D fhost; > =20 > - if ((so->so_iptos =3D tcp_tos(so)) =3D=3D 0) > + so->so_iptos =3D tcp_tos(so); > + if (so->so_iptos =3D=3D 0) { > + switch (af) { > + case AF_INET: > so->so_iptos =3D ((struct ip *)ti)->ip_tos; I think you could also indent this here immediately ... it's only one line, so indenting immediately should not hurt readability here. > + break; > + default: > + goto drop; > + break; > + } > + } > =20 > tp =3D sototcpcb(so); > tp->t_state =3D TCPS_LISTEN; > @@ -569,7 +591,8 @@ findso: > * If this is destined for the control address, then flag to > * tcp_ctl once connected, otherwise connect > */ > - if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) =3D=3D > + if (af =3D=3D AF_INET && > + (so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) =3D=3D > slirp->vnetwork_addr.s_addr) { > if (so->so_faddr.s_addr !=3D slirp->vhost_addr.s_addr && > so->so_faddr.s_addr !=3D slirp->vnameserver_addr.s_addr) { > @@ -607,7 +630,7 @@ findso: > if(errno =3D=3D ECONNREFUSED) { > /* ACK the SYN, send RST to refuse the connection */ > tcp_respond(tp, ti, m, ti->ti_seq+1, (tcp_seq)0, > - TH_RST|TH_ACK); > + TH_RST|TH_ACK, af); > } else { > if(errno =3D=3D EHOSTUNREACH) code=3DICMP_UNREACH_HOST; > HTONL(ti->ti_seq); /* restore tcp header */ > @@ -616,7 +639,13 @@ findso: > HTONS(ti->ti_urp); > m->m_data -=3D sizeof(struct tcpiphdr)+off-sizeof(struct tcphdr= ); > m->m_len +=3D sizeof(struct tcpiphdr)+off-sizeof(struct tcphdr= ); > + switch (af) { > + case AF_INET: > *ip=3Dsave_ip; Could also be indented immediately. > + break; > + default: > + goto drop; > + } > icmp_send_error(m, ICMP_UNREACH, code, 0, strerror(errno)); > } > tcp_close(tp); > @@ -1289,11 +1318,11 @@ dropafterack: > dropwithreset: > /* reuses m if m!=3DNULL, m_free() unnecessary */ > if (tiflags & TH_ACK) > - tcp_respond(tp, ti, m, (tcp_seq)0, ti->ti_ack, TH_RST); > + tcp_respond(tp, ti, m, (tcp_seq)0, ti->ti_ack, TH_RST, af); > else { > if (tiflags & TH_SYN) ti->ti_len++; > tcp_respond(tp, ti, m, ti->ti_seq+ti->ti_len, (tcp_seq)0, > - TH_RST|TH_ACK); > + TH_RST|TH_ACK, af); > } > =20 > return; > @@ -1484,7 +1513,14 @@ tcp_mss(struct tcpcb *tp, u_int offer) > DEBUG_ARG("tp =3D %p", tp); > DEBUG_ARG("offer =3D %d", offer); > =20 > + switch (so->so_ffamily) { > + case AF_INET: > mss =3D min(IF_MTU, IF_MRU) - sizeof(struct tcphdr) + sizeof(struct i= p); dito, indent immediately. > + break; > + default: > + break; > + } > + > 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 7fc6a87..62ab1e5 100644 > --- a/slirp/tcp_output.c > +++ b/slirp/tcp_output.c > @@ -61,7 +61,8 @@ tcp_output(struct tcpcb *tp) > register long len, win; > int off, flags, error; > register struct mbuf *m; > - register struct tcpiphdr *ti; > + register struct tcpiphdr *ti, tcpiph_save; > + struct ip *ip; > u_char opt[MAX_TCPOPTLEN]; > unsigned optlen, hdrlen; > int idle, sendalot; > @@ -447,13 +448,15 @@ send: > * the template, but need a way to checksum without them. > */ > m->m_len =3D hdrlen + len; /* XXX Needed? m_len should be correct */ > + tcpiph_save =3D *(mtod(m, struct tcpiphdr *)); > =20 > - struct tcpiphdr tcpiph_save =3D *(mtod(m, struct tcpiphdr *)); > + switch (so->so_ffamily) { > + case AF_INET: > 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 =3D mtod(m, struct ip *); > =20 > ip->ip_len =3D m->m_len; > ip->ip_dst =3D tcpiph_save.ti_dst; > @@ -464,6 +467,11 @@ send: > ip->ip_tos =3D so->so_iptos; > =20 > error =3D ip_output(so, m); > + break; > + > + default: > + goto out; Hmm, this jumps to a "return (error)" statement ... but as far as I can see, error has never been initialized in this case? So I think you either should set the error variable explicitely here, or simply "return 1" immediately instead of doing the goto. > + } > =20 > if (error) { > out: Thomas