From: Thomas Huth <thuth@redhat.com>
To: Samuel Thibault <samuel.thibault@ens-lyon.org>, qemu-devel@nongnu.org
Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>,
Li Zhijian <lizhijian@cn.fujitsu.com>,
Stefan Hajnoczi <stefanha@gmail.com>,
Jason Wang <jasowang@redhat.com>,
Dave Gilbert <dgilbert@redhat.com>,
Vasiliy Tolstov <v.tolstov@selfip.ru>,
Huangpeng <peter.huangpeng@huawei.com>,
Gonglei <arei.gonglei@huawei.com>,
Jan Kiszka <jan.kiszka@siemens.com>,
Yang Hongyang <yanghy@cn.fujitsu.com>,
Guillaume Subiron <maethor@subiron.org>
Subject: Re: [Qemu-devel] [PATCH 7/9] slirp: Add sockaddr_equal, make solookup family-agnostic
Date: Mon, 14 Dec 2015 21:17:19 +0100 [thread overview]
Message-ID: <566F23CF.4050701@redhat.com> (raw)
In-Reply-To: <1450101088-14575-7-git-send-email-samuel.thibault@ens-lyon.org>
On 14/12/15 14:51, Samuel Thibault wrote:
> From: Guillaume Subiron <maethor@subiron.org>
>
> This patch makes solookup() compatible with varying address
> families, by using a new sockaddr_equal() function that compares
> two sockaddr_storage.
>
> This prepares for IPv6 support.
>
> Signed-off-by: Guillaume Subiron <maethor@subiron.org>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> slirp/socket.c | 21 ++++++---------------
> slirp/socket.h | 22 +++++++++++++++++++++-
> slirp/tcp_input.c | 20 +++++++++++---------
> slirp/udp.c | 8 ++++++--
> 4 files changed, 44 insertions(+), 27 deletions(-)
>
> 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);
>
> -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 = *last;
>
> /* Optimisation */
> - if (so != head &&
> - so->so_lport == lport &&
> - so->so_laddr.s_addr == laddr.s_addr &&
> - (!faddr.s_addr ||
> - (so->so_faddr.s_addr == faddr.s_addr &&
> - so->so_fport == fport))) {
> + if (so != 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 line.
> return so;
> }
>
> for (so = head->so_next; so != head; so = so->so_next) {
> - if (so->so_lport == lport &&
> - so->so_laddr.s_addr == laddr.s_addr &&
> - (!faddr.s_addr ||
> - (so->so_faddr.s_addr == faddr.s_addr &&
> - so->so_fport == fport))) {
> + if (sockaddr_equal(&(so->lhost.ss), lhost)
> + && (!fhost || sockaddr_equal(&(so->fhost.ss), fhost))) {
dito.
> *last = 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 the internet */
>
> +static inline int sockaddr_equal(struct sockaddr_storage *a,
> + struct sockaddr_storage *b)
> +{
> + if (a->ss_family != b->ss_family) {
> + return 0;
> + }
> +
> + switch (a->ss_family) {
> + case AF_INET:
> + {
> + struct sockaddr_in *a4 = (struct sockaddr_in *) a;
> + struct sockaddr_in *b4 = (struct sockaddr_in *) b;
> + return (a4->sin_addr.s_addr == b4->sin_addr.s_addr
> + && a4->sin_port == 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 = 0;
> u_long tiwin;
> int ret;
> + struct sockaddr_storage lhost, fhost;
> struct ex_list *ex_ptr;
> Slirp *slirp;
>
> @@ -320,9 +321,14 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso)
> * Locate pcb for segment.
> */
> findso:
> - so = solookup(&slirp->tcp_last_so, &slirp->tcb,
> - ti->ti_src, ti->ti_sport,
> - ti->ti_dst, ti->ti_dport);
> + lhost.ss_family = AF_INET;
> + ((struct sockaddr_in *)&lhost)->sin_addr = ti->ti_src;
> + ((struct sockaddr_in *)&lhost)->sin_port = ti->ti_sport;
> + fhost.ss_family = AF_INET;
> + ((struct sockaddr_in *)&fhost)->sin_addr = ti->ti_dst;
> + ((struct sockaddr_in *)&fhost)->sin_port = ti->ti_dport;
Couldn't you simply use "fhost.sin.sin_addr = ..." etc. instead of
casting everything via a pointer?
> + so = solookup(&slirp->tcp_last_so, &slirp->tcb, &lhost, &fhost);
>
> /*
> * 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);
>
> - so->so_lfamily = AF_INET;
> - so->so_laddr = ti->ti_src;
> - so->so_lport = ti->ti_sport;
> - so->so_ffamily = AF_INET;
> - so->so_faddr = ti->ti_dst;
> - so->so_fport = ti->ti_dport;
> + so->lhost.ss = lhost;
> + so->fhost.ss = fhost;
>
> if ((so->so_iptos = tcp_tos(so)) == 0)
> so->so_iptos = ((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;
>
> DEBUG_CALL("udp_input");
> DEBUG_ARG("m = %p", m);
> @@ -151,8 +152,11 @@ udp_input(register struct mbuf *m, int iphlen)
> /*
> * Locate pcb for datagram.
> */
> - so = solookup(&slirp->udp_last_so, &slirp->udb,
> - ip->ip_src, uh->uh_sport, (struct in_addr) {0}, 0);
> + lhost.ss_family = AF_INET;
> + ((struct sockaddr_in *)&lhost)->sin_addr = ip->ip_src;
> + ((struct sockaddr_in *)&lhost)->sin_port = uh->uh_sport;
dito
> + so = solookup(&slirp->udp_last_so, &slirp->udb, &lhost, NULL);
>
> if (so == 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 <thuth@redhat.com>
next prev parent reply other threads:[~2015-12-14 20:17 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-14 13:49 [Qemu-devel] [PATCHv6 0/9] slirp: Adding IPv6 support to Qemu -net user mode Samuel Thibault
2015-12-14 13:51 ` [Qemu-devel] [PATCH 1/9] slirp: goto bad in udp_input if sosendto fails Samuel Thibault
2015-12-14 13:51 ` [Qemu-devel] [PATCH 2/9] slirp: Generalizing and neutralizing ARP code Samuel Thibault
2015-12-14 14:24 ` Thomas Huth
2015-12-14 13:51 ` [Qemu-devel] [PATCH 3/9] slirp: Adding address family switch for incoming frames Samuel Thibault
2015-12-14 17:50 ` Thomas Huth
2015-12-14 22:06 ` Samuel Thibault
2015-12-14 22:07 ` Samuel Thibault
2015-12-15 6:47 ` Thomas Huth
2015-12-14 13:51 ` [Qemu-devel] [PATCH 4/9] slirp: Make Socket structure IPv6 compatible Samuel Thibault
2015-12-14 19:31 ` Thomas Huth
2015-12-14 13:51 ` [Qemu-devel] [PATCH 5/9] slirp: Factorizing address translation Samuel Thibault
2015-12-14 13:51 ` [Qemu-devel] [PATCH 6/9] slirp: Factorizing and cleaning solookup() Samuel Thibault
2015-12-14 20:05 ` Thomas Huth
2015-12-14 13:51 ` [Qemu-devel] [PATCH 7/9] slirp: Add sockaddr_equal, make solookup family-agnostic Samuel Thibault
2015-12-14 20:17 ` Thomas Huth [this message]
2015-12-14 22:24 ` Samuel Thibault
2015-12-14 13:51 ` [Qemu-devel] [PATCH 8/9] slirp: Make udp_attach IPv6 compatible Samuel Thibault
2015-12-14 13:51 ` [Qemu-devel] [PATCH 9/9] slirp: Adding family argument to tcp_fconnect() Samuel Thibault
2015-12-19 21:24 [Qemu-devel] [PATCHv7 0/9] slirp: Adding IPv6 support to Qemu -net user mode Samuel Thibault
2015-12-19 21:24 ` [Qemu-devel] [PATCH 1/9] slirp: goto bad in udp_input if sosendto fails Samuel Thibault
2015-12-19 21:25 ` [Qemu-devel] [PATCH 7/9] slirp: Add sockaddr_equal, make solookup family-agnostic Samuel Thibault
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=566F23CF.4050701@redhat.com \
--to=thuth@redhat.com \
--cc=arei.gonglei@huawei.com \
--cc=dgilbert@redhat.com \
--cc=jan.kiszka@siemens.com \
--cc=jasowang@redhat.com \
--cc=lizhijian@cn.fujitsu.com \
--cc=maethor@subiron.org \
--cc=peter.huangpeng@huawei.com \
--cc=qemu-devel@nongnu.org \
--cc=samuel.thibault@ens-lyon.org \
--cc=stefanha@gmail.com \
--cc=v.tolstov@selfip.ru \
--cc=yanghy@cn.fujitsu.com \
--cc=zhang.zhanghailiang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).