| Message ID | 1442333283-13119-30-git-send-email-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 15.09.2015 18:07, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > The number of eventfd that can be handled per peer is limited by the > number of vectors. Return an error when receiving too many of them. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > hw/misc/ivshmem.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > index b9c78cd..63e4c4f 100644 > --- a/hw/misc/ivshmem.c > +++ b/hw/misc/ivshmem.c > @@ -569,6 +569,13 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > } > > /* get a new eventfd */ > + if (peer->nb_eventfds >= s->vectors) { > + error_report("Too many eventfd received, device has %d vectors", > + s->vectors); > + close(incoming_fd); > + return; > + } > + > nth_eventfd = peer->nb_eventfds++; > > /* this is an eventfd for a particular peer VM */ > can the device still operate if we detect these errors at ivshmem_read time? I am referring also to the other checks happening in ivshmem_read doing if ([something fails]) { error_report("abcabc"); /* close(), ... */ return; } Can the device stop operating if these conditions happen? If so, do we have to put the device into a non-operating state, where all read/writes are ignored? Should there be a ivshmem status flag for ERROR? Should we exit(1)? note: I don't know what the "proper" behavior should be, but I am concerned about the runtime stability of the software which uses the device. Ciao, Claudio
Hi On Wed, Sep 16, 2015 at 2:14 PM, Claudio Fontana <claudio.fontana@huawei.com> wrote: > On 15.09.2015 18:07, marcandre.lureau@redhat.com wrote: >> From: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> The number of eventfd that can be handled per peer is limited by the >> number of vectors. Return an error when receiving too many of them. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> --- >> hw/misc/ivshmem.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c >> index b9c78cd..63e4c4f 100644 >> --- a/hw/misc/ivshmem.c >> +++ b/hw/misc/ivshmem.c >> @@ -569,6 +569,13 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) >> } >> >> /* get a new eventfd */ >> + if (peer->nb_eventfds >= s->vectors) { >> + error_report("Too many eventfd received, device has %d vectors", >> + s->vectors); >> + close(incoming_fd); >> + return; >> + } >> + >> nth_eventfd = peer->nb_eventfds++; >> >> /* this is an eventfd for a particular peer VM */ >> > > can the device still operate if we detect these errors at ivshmem_read time? > > I am referring also to the other checks happening in ivshmem_read doing > > if ([something fails]) { > error_report("abcabc"); > /* close(), ... */ > return; > } > > Can the device stop operating if these conditions happen? Yes, it simply closes the "new" incoming_fd. So if the server sends extra eventfd for peers that we can't handle, we won't be able to notify those peers. But the rest of the peers and function is still working. This is btw, what the current code is doing (only the variable is called tmp_fd before I removed it in "remove unnecessary dup()" patch) > If so, do we have to put the device into a non-operating state, where all read/writes are ignored? Should there be a ivshmem status flag for ERROR? > Should we exit(1)? > > note: I don't know what the "proper" behavior should be, but I am concerned about the runtime stability of the software which uses the device. It's likely a misconfiguration. So having error reported seems like a sane and enough thing to do.
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index b9c78cd..63e4c4f 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c @@ -569,6 +569,13 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) } /* get a new eventfd */ + if (peer->nb_eventfds >= s->vectors) { + error_report("Too many eventfd received, device has %d vectors", + s->vectors); + close(incoming_fd); + return; + } + nth_eventfd = peer->nb_eventfds++; /* this is an eventfd for a particular peer VM */