Skip to content

Commit

Permalink
slirp: switch to GPollFD
Browse files Browse the repository at this point in the history 
Slirp uses rfds/wfds/xfds more extensively than other QEMU components.

The rarely-used out-of-band TCP data feature is used.  That means we
need the full table of select(2) to g_poll(3) events:

  rfds -> G_IO_IN | G_IO_HUP | G_IO_ERR
  wfds -> G_IO_OUT | G_IO_ERR
  xfds -> G_IO_PRI

I came up with this table by looking at Linux fs/select.c which maps
select(2) to poll(2) internally.

Another detail to watch out for are the global variables that reference
rfds/wfds/xfds during slirp_select_poll().  sofcantrcvmore() and
sofcantsendmore() use these globals to clear fd_set bits.  When
sofcantrcvmore() is called, the wfds bit is cleared so that the write
handler will no longer be run for this iteration of the event loop.

This actually seems buggy to me since TCP connections can be half-closed
and we'd still want to handle data in half-duplex fashion.  I think the
real intention is to avoid running the read/write handler when the
socket has been fully closed.  This is indicated with the SS_NOFDREF
state bit so we now check for it before invoking the TCP write handler.
Note that UDP/ICMP code paths don't care because they are
connectionless.

Note that slirp/ has a lot of tabs and sometimes mixed tabs with spaces.
I followed the style of the surrounding code.

Signed-off-by: Stefan Hajnoczi <[email protected]>
Reviewed-by: Laszlo Ersek <[email protected]>
Message-id: [email protected]
Signed-off-by: Anthony Liguori <[email protected]>
  • Loading branch information
@stefanhaRH
stefanhaRH authored and Anthony Liguori committed Feb 21, 2013
1 parent cf1d078 commit 8917c3b
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 74 deletions.
4 changes: 2 additions & 2 deletions main-loop.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,13 +505,13 @@ int main_loop_wait(int nonblocking)

#ifdef CONFIG_SLIRP
slirp_update_timeout(&timeout);
slirp_select_fill(&nfds, &rfds, &wfds, &xfds);
slirp_pollfds_fill(gpollfds);
#endif
qemu_iohandler_fill(&nfds, &rfds, &wfds, &xfds);
ret = os_host_main_loop_wait(timeout);
qemu_iohandler_poll(&rfds, &wfds, &xfds, ret);
#ifdef CONFIG_SLIRP
slirp_select_poll(&rfds, &wfds, &xfds, (ret < 0));
slirp_pollfds_poll(gpollfds, (ret < 0));
#endif

qemu_run_all_timers();
Expand Down
6 changes: 2 additions & 4 deletions slirp/libslirp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
void slirp_cleanup(Slirp *slirp);

void slirp_update_timeout(uint32_t *timeout);
void slirp_select_fill(int *pnfds,
fd_set *readfds, fd_set *writefds, fd_set *xfds);
void slirp_pollfds_fill(GArray *pollfds);

void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
int select_error);
void slirp_pollfds_poll(GArray *pollfds, int select_error);

void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);

Expand Down
1 change: 0 additions & 1 deletion slirp/main.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ extern int ctty_closed;
extern char *slirp_tty;
extern char *exec_shell;
extern u_int curtime;
extern fd_set *global_readfds, *global_writefds, *global_xfds;
extern struct in_addr loopback_addr;
extern unsigned long loopback_mask;
extern char *username;
Expand Down
133 changes: 79 additions & 54 deletions slirp/slirp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {

static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };

/* XXX: suppress those select globals */
fd_set *global_readfds, *global_writefds, *global_xfds;

u_int curtime;
static u_int time_fasttimo, last_slowtimo;
static int do_slowtimo;
Expand Down Expand Up @@ -261,7 +258,6 @@ void slirp_cleanup(Slirp *slirp)

#define CONN_CANFSEND(so) (((so)->so_state & (SS_FCANTSENDMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
#define CONN_CANFRCV(so) (((so)->so_state & (SS_FCANTRCVMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
#define UPD_NFDS(x) if (nfds < (x)) nfds = (x)

void slirp_update_timeout(uint32_t *timeout)
{
Expand All @@ -270,23 +266,15 @@ void slirp_update_timeout(uint32_t *timeout)
}
}

void slirp_select_fill(int *pnfds,
fd_set *readfds, fd_set *writefds, fd_set *xfds)
void slirp_pollfds_fill(GArray *pollfds)
{
Slirp *slirp;
struct socket *so, *so_next;
int nfds;

if (QTAILQ_EMPTY(&slirp_instances)) {
return;
}

/* fail safe */
global_readfds = NULL;
global_writefds = NULL;
global_xfds = NULL;

nfds = *pnfds;
/*
* First, TCP sockets
*/
Expand All @@ -302,8 +290,12 @@ void slirp_select_fill(int *pnfds,

for (so = slirp->tcb.so_next; so != &slirp->tcb;
so = so_next) {
int events = 0;

so_next = so->so_next;

so->pollfds_idx = -1;

/*
* See if we need a tcp_fasttimo
*/
Expand All @@ -323,17 +315,25 @@ void slirp_select_fill(int *pnfds,
* Set for reading sockets which are accepting
*/
if (so->so_state & SS_FACCEPTCONN) {
FD_SET(so->s, readfds);
UPD_NFDS(so->s);
GPollFD pfd = {
.fd = so->s,
.events = G_IO_IN | G_IO_HUP | G_IO_ERR,
};
so->pollfds_idx = pollfds->len;
g_array_append_val(pollfds, pfd);
continue;
}

/*
* Set for writing sockets which are connecting
*/
if (so->so_state & SS_ISFCONNECTING) {
FD_SET(so->s, writefds);
UPD_NFDS(so->s);
GPollFD pfd = {
.fd = so->s,
.events = G_IO_OUT | G_IO_ERR,
};
so->pollfds_idx = pollfds->len;
g_array_append_val(pollfds, pfd);
continue;
}

Expand All @@ -342,8 +342,7 @@ void slirp_select_fill(int *pnfds,
* we have something to send
*/
if (CONN_CANFSEND(so) && so->so_rcv.sb_cc) {
FD_SET(so->s, writefds);
UPD_NFDS(so->s);
events |= G_IO_OUT | G_IO_ERR;
}

/*
Expand All @@ -352,9 +351,16 @@ void slirp_select_fill(int *pnfds,
*/
if (CONN_CANFRCV(so) &&
(so->so_snd.sb_cc < (so->so_snd.sb_datalen/2))) {
FD_SET(so->s, readfds);
FD_SET(so->s, xfds);
UPD_NFDS(so->s);
events |= G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_PRI;
}

if (events) {
GPollFD pfd = {
.fd = so->s,
.events = events,
};
so->pollfds_idx = pollfds->len;
g_array_append_val(pollfds, pfd);
}
}

Expand All @@ -365,6 +371,8 @@ void slirp_select_fill(int *pnfds,
so = so_next) {
so_next = so->so_next;

so->pollfds_idx = -1;

/*
* See if it's timed out
*/
Expand All @@ -388,8 +396,12 @@ void slirp_select_fill(int *pnfds,
* (XXX <= 4 ?)
*/
if ((so->so_state & SS_ISFCONNECTED) && so->so_queued <= 4) {
FD_SET(so->s, readfds);
UPD_NFDS(so->s);
GPollFD pfd = {
.fd = so->s,
.events = G_IO_IN | G_IO_HUP | G_IO_ERR,
};
so->pollfds_idx = pollfds->len;
g_array_append_val(pollfds, pfd);
}
}

Expand All @@ -400,6 +412,8 @@ void slirp_select_fill(int *pnfds,
so = so_next) {
so_next = so->so_next;

so->pollfds_idx = -1;

/*
* See if it's timed out
*/
Expand All @@ -413,17 +427,18 @@ void slirp_select_fill(int *pnfds,
}

if (so->so_state & SS_ISFCONNECTED) {
FD_SET(so->s, readfds);
UPD_NFDS(so->s);
GPollFD pfd = {
.fd = so->s,
.events = G_IO_IN | G_IO_HUP | G_IO_ERR,
};
so->pollfds_idx = pollfds->len;
g_array_append_val(pollfds, pfd);
}
}
}

*pnfds = nfds;
}

void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
int select_error)
void slirp_pollfds_poll(GArray *pollfds, int select_error)
{
Slirp *slirp;
struct socket *so, *so_next;
Expand All @@ -433,10 +448,6 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
return;
}

global_readfds = readfds;
global_writefds = writefds;
global_xfds = xfds;

curtime = qemu_get_clock_ms(rt_clock);

QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
Expand All @@ -462,28 +473,32 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
*/
for (so = slirp->tcb.so_next; so != &slirp->tcb;
so = so_next) {
int revents;

so_next = so->so_next;

/*
* FD_ISSET is meaningless on these sockets
* (and they can crash the program)
*/
revents = 0;
if (so->pollfds_idx != -1) {
revents = g_array_index(pollfds, GPollFD,
so->pollfds_idx).revents;
}

if (so->so_state & SS_NOFDREF || so->s == -1) {
continue;
}

/*
* Check for URG data
* This will soread as well, so no need to
* test for readfds below if this succeeds
* test for G_IO_IN below if this succeeds
*/
if (FD_ISSET(so->s, xfds)) {
if (revents & G_IO_PRI) {
sorecvoob(so);
}
/*
* Check sockets for reading
*/
else if (FD_ISSET(so->s, readfds)) {
else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
/*
* Check for incoming connections
*/
Expand All @@ -502,7 +517,8 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
/*
* Check sockets for writing
*/
if (FD_ISSET(so->s, writefds)) {
if (!(so->so_state & SS_NOFDREF) &&
(revents & (G_IO_OUT | G_IO_ERR))) {
/*
* Check for non-blocking, still-connecting sockets
*/
Expand Down Expand Up @@ -588,9 +604,18 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
*/
for (so = slirp->udb.so_next; so != &slirp->udb;
so = so_next) {
int revents;

so_next = so->so_next;

if (so->s != -1 && FD_ISSET(so->s, readfds)) {
revents = 0;
if (so->pollfds_idx != -1) {
revents = g_array_index(pollfds, GPollFD,
so->pollfds_idx).revents;
}

if (so->s != -1 &&
(revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
sorecvfrom(so);
}
}
Expand All @@ -600,25 +625,25 @@ void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds,
*/
for (so = slirp->icmp.so_next; so != &slirp->icmp;
so = so_next) {
so_next = so->so_next;
int revents;

so_next = so->so_next;

revents = 0;
if (so->pollfds_idx != -1) {
revents = g_array_index(pollfds, GPollFD,
so->pollfds_idx).revents;
}

if (so->s != -1 && FD_ISSET(so->s, readfds)) {
if (so->s != -1 &&
(revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
icmp_receive(so);
}
}
}

if_start(slirp);
}

/* clear global file descriptor sets.
* these reside on the stack in vl.c
* so they're unusable if we're not in
* slirp_select_fill or slirp_select_poll.
*/
global_readfds = NULL;
global_writefds = NULL;
global_xfds = NULL;
}

static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
Expand Down
9 changes: 0 additions & 9 deletions slirp/socket.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,9 +680,6 @@ sofcantrcvmore(struct socket *so)
{
if ((so->so_state & SS_NOFDREF) == 0) {
shutdown(so->s,0);
if(global_writefds) {
FD_CLR(so->s,global_writefds);
}
}
so->so_state &= ~(SS_ISFCONNECTING);
if (so->so_state & SS_FCANTSENDMORE) {
Expand All @@ -698,12 +695,6 @@ sofcantsendmore(struct socket *so)
{
if ((so->so_state & SS_NOFDREF) == 0) {
shutdown(so->s,1); /* send FIN to fhost */
if (global_readfds) {
FD_CLR(so->s,global_readfds);
}
if (global_xfds) {
FD_CLR(so->s,global_xfds);
}
}
so->so_state &= ~(SS_ISFCONNECTING);
if (so->so_state & SS_FCANTRCVMORE) {
Expand Down
2 changes: 2 additions & 0 deletions slirp/socket.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ struct socket {

int s; /* The actual socket */

int pollfds_idx; /* GPollFD GArray index */

Slirp *slirp; /* managing slirp instance */

/* XXX union these with not-yet-used sbuf params */
Expand Down
6 changes: 2 additions & 4 deletions stubs/slirp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ void slirp_update_timeout(uint32_t *timeout)
{
}

void slirp_select_fill(int *pnfds, fd_set *readfds,
fd_set *writefds, fd_set *xfds)
void slirp_pollfds_fill(GArray *pollfds)
{
}

void slirp_select_poll(fd_set *readfds, fd_set *writefds,
fd_set *xfds, int select_error)
void slirp_pollfds_poll(GArray *pollfds, int select_error)
{
}

0 comments on commit 8917c3b

Please sign in to comment.