| Message ID | 1450101088-14575-6-git-send-email-samuel.thibault@ens-lyon.org |
|---|---|
| State | New |
| Headers | show |
On 14/12/15 14:51, Samuel Thibault wrote: > From: Guillaume Subiron <maethor@subiron.org> > > solookup() was only compatible with TCP. Having the socket list in > argument, it is now compatible with UDP too. > > Some optimization code is factorized inside the function (the function > look at the last returned result before browsing the complete socket > list). > > 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 | 37 ++++++++++++++++++++++++------------- > slirp/socket.h | 5 +++-- > slirp/tcp_input.c | 13 +++---------- > slirp/udp.c | 21 ++------------------- > 4 files changed, 32 insertions(+), 44 deletions(-) > > diff --git a/slirp/socket.c b/slirp/socket.c > index d1034fb..8f73e90 100644 > --- a/slirp/socket.c > +++ b/slirp/socket.c > @@ -16,23 +16,34 @@ static void sofcantrcvmore(struct socket *so); > static void sofcantsendmore(struct socket *so); > > struct socket * > -solookup(struct socket *head, struct in_addr laddr, u_int lport, > +solookup(struct socket **last, struct socket *head, > + struct in_addr laddr, u_int lport, > struct in_addr faddr, u_int fport) > { > - struct socket *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 && > - so->so_faddr.s_addr == faddr.s_addr && > - so->so_fport == fport) > - break; > - } > + 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))) { > + return so; > + } > > - if (so == head) > - return (struct socket *)NULL; > - 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))) { I somewhat dislike the "!faddr.s_addr" check which is used to see whether it's the call from UDP instead of TCP and thus whether the foreign addr and port have to be checked ... this feels like it could also go wrong sometimes (couldn't faddr.s_addr be zero in the TCP case by accident?)... Anyway, since this gets reworked in the next patch already, and the check then looks fine afterwards, I think this should be enough here as a temporary solution, so I'm also fine if this patch gets merged as it is. Reviewed-by: Thomas Huth <thuth@redhat.com>
diff --git a/slirp/socket.c b/slirp/socket.c index d1034fb..8f73e90 100644 --- a/slirp/socket.c +++ b/slirp/socket.c @@ -16,23 +16,34 @@ static void sofcantrcvmore(struct socket *so); static void sofcantsendmore(struct socket *so); struct socket * -solookup(struct socket *head, struct in_addr laddr, u_int lport, +solookup(struct socket **last, struct socket *head, + struct in_addr laddr, u_int lport, struct in_addr faddr, u_int fport) { - struct socket *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 && - so->so_faddr.s_addr == faddr.s_addr && - so->so_fport == fport) - break; - } + 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))) { + return so; + } - if (so == head) - return (struct socket *)NULL; - 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))) { + *last = so; + return so; + } + } + return (struct socket *)NULL; } /* diff --git a/slirp/socket.h b/slirp/socket.h index b27bbb2..1c8c24c 100644 --- a/slirp/socket.h +++ b/slirp/socket.h @@ -87,8 +87,9 @@ struct socket { #define SS_HOSTFWD 0x1000 /* Socket describes host->guest forwarding */ #define SS_INCOMING 0x2000 /* Connection was initiated by a host on the internet */ -struct socket * solookup(struct socket *, struct in_addr, u_int, struct in_addr, u_int); -struct socket * socreate(Slirp *); +struct socket *solookup(struct socket **, struct socket *, + struct in_addr, u_int, struct in_addr, u_int); +struct socket *socreate(Slirp *); void sofree(struct socket *); int soread(struct socket *); void sorecvoob(struct socket *); diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c index 4c3191d..5492061 100644 --- a/slirp/tcp_input.c +++ b/slirp/tcp_input.c @@ -320,16 +320,9 @@ tcp_input(struct mbuf *m, int iphlen, struct socket *inso) * Locate pcb for segment. */ findso: - so = slirp->tcp_last_so; - if (so->so_fport != ti->ti_dport || - so->so_lport != ti->ti_sport || - so->so_laddr.s_addr != ti->ti_src.s_addr || - so->so_faddr.s_addr != ti->ti_dst.s_addr) { - so = solookup(&slirp->tcb, ti->ti_src, ti->ti_sport, - ti->ti_dst, ti->ti_dport); - if (so) - slirp->tcp_last_so = so; - } + so = solookup(&slirp->tcp_last_so, &slirp->tcb, + ti->ti_src, ti->ti_sport, + ti->ti_dst, ti->ti_dport); /* * If the state is CLOSED (i.e., TCB does not exist) then diff --git a/slirp/udp.c b/slirp/udp.c index 8203eb1..126ef82 100644 --- a/slirp/udp.c +++ b/slirp/udp.c @@ -151,25 +151,8 @@ udp_input(register struct mbuf *m, int iphlen) /* * Locate pcb for datagram. */ - so = slirp->udp_last_so; - if (so == &slirp->udb || so->so_lport != uh->uh_sport || - so->so_laddr.s_addr != ip->ip_src.s_addr) { - struct socket *tmp; - - for (tmp = slirp->udb.so_next; tmp != &slirp->udb; - tmp = tmp->so_next) { - if (tmp->so_lport == uh->uh_sport && - tmp->so_laddr.s_addr == ip->ip_src.s_addr) { - so = tmp; - break; - } - } - if (tmp == &slirp->udb) { - so = NULL; - } else { - slirp->udp_last_so = so; - } - } + so = solookup(&slirp->udp_last_so, &slirp->udb, + ip->ip_src, uh->uh_sport, (struct in_addr) {0}, 0); if (so == NULL) { /*