From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47344) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8ZYn-0006Ls-Mc for qemu-devel@nongnu.org; Mon, 14 Dec 2015 15:17:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a8ZYk-0004rB-D0 for qemu-devel@nongnu.org; Mon, 14 Dec 2015 15:17:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58272) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8ZYk-0004r5-3O for qemu-devel@nongnu.org; Mon, 14 Dec 2015 15:17:26 -0500 References: <20151214134939.GA3142@var.bordeaux.inria.fr> <1450101088-14575-1-git-send-email-samuel.thibault@ens-lyon.org> <1450101088-14575-7-git-send-email-samuel.thibault@ens-lyon.org> From: Thomas Huth Message-ID: <566F23CF.4050701@redhat.com> Date: Mon, 14 Dec 2015 21:17:19 +0100 MIME-Version: 1.0 In-Reply-To: <1450101088-14575-7-git-send-email-samuel.thibault@ens-lyon.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 7/9] slirp: Add sockaddr_equal, make solookup family-agnostic 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 14/12/15 14:51, Samuel Thibault wrote: > From: Guillaume Subiron >=20 > This patch makes solookup() compatible with varying address > families, by using a new sockaddr_equal() function that compares > two sockaddr_storage. >=20 > This prepares for IPv6 support. >=20 > Signed-off-by: Guillaume Subiron > Signed-off-by: Samuel Thibault > --- > slirp/socket.c | 21 ++++++--------------- > slirp/socket.h | 22 +++++++++++++++++++++- > slirp/tcp_input.c | 20 +++++++++++--------- > slirp/udp.c | 8 ++++++-- > 4 files changed, 44 insertions(+), 27 deletions(-) >=20 > diff --git a/slirp/socket.c b/slirp/socket.c > index 8f73e90..a785b92 100644 > --- a/slirp/socket.c > +++ b/slirp/socket.c > @@ -15,29 +15,20 @@ > static void sofcantrcvmore(struct socket *so); > static void sofcantsendmore(struct socket *so); > =20 > -struct socket * > -solookup(struct socket **last, struct socket *head, > - struct in_addr laddr, u_int lport, > - struct in_addr faddr, u_int fport) > +struct socket *solookup(struct socket **last, struct socket *head, > + struct sockaddr_storage *lhost, struct sockaddr_storage *fhost= ) > { > struct socket *so =3D *last; > =20 > /* Optimisation */ > - if (so !=3D head && > - so->so_lport =3D=3D lport && > - so->so_laddr.s_addr =3D=3D laddr.s_addr && > - (!faddr.s_addr || > - (so->so_faddr.s_addr =3D=3D faddr.s_addr && > - so->so_fport =3D=3D fport))) { > + if (so !=3D head && sockaddr_equal(&(so->lhost.ss), lhost) > + && (!fhost || sockaddr_equal(&(so->fhost.ss), fhost))) { I think you could omit the paranthesis around "so->fhost.ss" in above lin= e. > return so; > } > =20 > for (so =3D head->so_next; so !=3D head; so =3D so->so_next) { > - if (so->so_lport =3D=3D lport && > - so->so_laddr.s_addr =3D=3D laddr.s_addr && > - (!faddr.s_addr || > - (so->so_faddr.s_addr =3D=3D faddr.s_addr && > - so->so_fport =3D=3D fport))) { > + if (sockaddr_equal(&(so->lhost.ss), lhost) > + && (!fhost || sockaddr_equal(&(so->fhost.ss), fhost)))= { dito. > *last =3D so; > return so; > } > diff --git a/slirp/socket.h b/slirp/socket.h > index 1c8c24c..1331af6 100644 > --- a/slirp/socket.h > +++ b/slirp/socket.h > @@ -87,8 +87,28 @@ struct socket { > #define SS_HOSTFWD 0x1000 /* Socket describes host->guest forwarding = */ > #define SS_INCOMING 0x2000 /* Connection was initiated by a host on t= he internet */ > =20 > +static inline int sockaddr_equal(struct sockaddr_storage *a, > + struct sockaddr_storage *b) > +{ > + if (a->ss_family !=3D b->ss_family) { > + return 0; > + } > + > + switch (a->ss_family) { > + case AF_INET: > + { > + struct sockaddr_in *a4 =3D (struct sockaddr_in *) a; > + struct sockaddr_in *b4 =3D (struct sockaddr_in *) b; > + return (a4->sin_addr.s_addr =3D=3D b4->sin_addr.s_addr > + && a4->sin_port =3D=3D b4->sin_port); You could omit the paranthesis around the check here. > + } > + default: > + assert(0); > + } > +} > + > struct socket *solookup(struct socket **, struct socket *, > - struct in_addr, u_int, struct in_addr, u_int); > + struct sockaddr_storage *, struct sockaddr_storage *); > struct socket *socreate(Slirp *); > void sofree(struct socket *); > int soread(struct socket *); > diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c > index 5492061..8c4fa62 100644 > --- a/slirp/tcp_input.c > +++ b/slirp/tcp_input.c > @@ -227,6 +227,7 @@ tcp_input(struct mbuf *m, int iphlen, struct socket= *inso) > int iss =3D 0; > u_long tiwin; > int ret; > + struct sockaddr_storage lhost, fhost; > struct ex_list *ex_ptr; > Slirp *slirp; > =20 > @@ -320,9 +321,14 @@ tcp_input(struct mbuf *m, int iphlen, struct socke= t *inso) > * Locate pcb for segment. > */ > findso: > - so =3D solookup(&slirp->tcp_last_so, &slirp->tcb, > - ti->ti_src, ti->ti_sport, > - ti->ti_dst, ti->ti_dport); > + lhost.ss_family =3D AF_INET; > + ((struct sockaddr_in *)&lhost)->sin_addr =3D ti->ti_src; > + ((struct sockaddr_in *)&lhost)->sin_port =3D ti->ti_sport; > + fhost.ss_family =3D AF_INET; > + ((struct sockaddr_in *)&fhost)->sin_addr =3D ti->ti_dst; > + ((struct sockaddr_in *)&fhost)->sin_port =3D ti->ti_dport; Couldn't you simply use "fhost.sin.sin_addr =3D ..." etc. instead of casting everything via a pointer? > + so =3D solookup(&slirp->tcp_last_so, &slirp->tcb, &lhost, &fhost); > =20 > /* > * If the state is CLOSED (i.e., TCB does not exist) then > @@ -367,12 +373,8 @@ findso: > sbreserve(&so->so_snd, TCP_SNDSPACE); > sbreserve(&so->so_rcv, TCP_RCVSPACE); > =20 > - so->so_lfamily =3D AF_INET; > - so->so_laddr =3D ti->ti_src; > - so->so_lport =3D ti->ti_sport; > - so->so_ffamily =3D AF_INET; > - so->so_faddr =3D ti->ti_dst; > - so->so_fport =3D ti->ti_dport; > + 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 ((struct ip *)ti)->ip_tos; > diff --git a/slirp/udp.c b/slirp/udp.c > index 126ef82..f2dd773 100644 > --- a/slirp/udp.c > +++ b/slirp/udp.c > @@ -70,6 +70,7 @@ udp_input(register struct mbuf *m, int iphlen) > int len; > struct ip save_ip; > struct socket *so; > + struct sockaddr_storage lhost; > =20 > DEBUG_CALL("udp_input"); > DEBUG_ARG("m =3D %p", m); > @@ -151,8 +152,11 @@ udp_input(register struct mbuf *m, int iphlen) > /* > * Locate pcb for datagram. > */ > - so =3D solookup(&slirp->udp_last_so, &slirp->udb, > - ip->ip_src, uh->uh_sport, (struct in_addr) {0}, 0); > + lhost.ss_family =3D AF_INET; > + ((struct sockaddr_in *)&lhost)->sin_addr =3D ip->ip_src; > + ((struct sockaddr_in *)&lhost)->sin_port =3D uh->uh_sport; dito > + so =3D solookup(&slirp->udp_last_so, &slirp->udb, &lhost, NULL); > =20 > if (so =3D=3D NULL) { > /* Apart from the somewhat cumbersome typecasts (which are IMHO a little bit ugly but still OK for me), the patch looks fine to me. Reviewed-by: Thomas Huth