Message ID | 20210119212507.1043636-44-maxime.coquelin@redhat.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Maxime Coquelin |
Headers | show |
Series | net/virtio: Virtio PMD rework | expand |
Context | Check | Description |
---|---|---|
ci/checkpatch | success | coding style OK |
Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Wednesday, January 20, 2021 5:25 AM > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; olivier.matz@6wind.com; > amorenoz@redhat.com; david.marchand@redhat.com > Cc: Maxime Coquelin <maxime.coquelin@redhat.com> > Subject: [PATCH v2 43/44] net/virtio: improve Vhost-user error logging > > This patch improves error logging in vhost_user_read, > especially printing errno when recv() fails. > > Suggested-by: Adrian Moreno <amorenoz@redhat.com> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > --- > drivers/net/virtio/virtio_user/vhost_user.c | 29 ++++++++++++--------- > 1 file changed, 17 insertions(+), 12 deletions(-) > > diff --git a/drivers/net/virtio/virtio_user/vhost_user.c > b/drivers/net/virtio/virtio_user/vhost_user.c > index f046655af6..be91c99cea 100644 > --- a/drivers/net/virtio/virtio_user/vhost_user.c > +++ b/drivers/net/virtio/virtio_user/vhost_user.c > @@ -148,38 +148,43 @@ vhost_user_read(int fd, struct vhost_user_msg *msg) > int ret, sz_hdr = VHOST_USER_HDR_SIZE, sz_payload; > > ret = recv(fd, (void *)msg, sz_hdr, 0); > - if (ret < sz_hdr) { > + if (ret < 0) { > + PMD_DRV_LOG(ERR, "Failed to recv msg header: %s", strerror(errno)); > + return -1; > + } else if (ret < sz_hdr) { > PMD_DRV_LOG(ERR, "Failed to recv msg hdr: %d instead of %d.", > ret, sz_hdr); > - goto fail; > + return -1; > } > > /* validate msg flags */ > if (msg->flags != (valid_flags)) { > PMD_DRV_LOG(ERR, "Failed to recv msg: flags %x instead of %x.", > msg->flags, valid_flags); > - goto fail; > + return -1; Since you are here, also add '0x' before '%x' here? Thanks, Chenbo > } > > sz_payload = msg->size; > > - if ((size_t)sz_payload > sizeof(msg->payload)) > - goto fail; > + if ((size_t)sz_payload > sizeof(msg->payload)) { > + PMD_DRV_LOG(ERR, "Payload size overflow, header says %d but > max %zu\n", > + sz_payload, sizeof(msg->payload)); > + return -1; > + } > > if (sz_payload) { > ret = recv(fd, (void *)((char *)msg + sz_hdr), sz_payload, 0); > - if (ret < sz_payload) { > - PMD_DRV_LOG(ERR, > - "Failed to recv msg payload: %d instead of %d.", > + if (ret < 0) { > + PMD_DRV_LOG(ERR, "Failed to recv msg payload: %s", > strerror(errno)); > + return -1; > + } else if (ret < sz_payload) { > + PMD_DRV_LOG(ERR, "Failed to recv msg payload: %d instead > of %u.", > ret, msg->size); > - goto fail; > + return -1; > } > } > > return 0; > - > -fail: > - return -1; > } > > static int > -- > 2.29.2
On 1/22/21 10:11 AM, Xia, Chenbo wrote: > Hi Maxime, > >> -----Original Message----- >> From: Maxime Coquelin <maxime.coquelin@redhat.com> >> Sent: Wednesday, January 20, 2021 5:25 AM >> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; olivier.matz@6wind.com; >> amorenoz@redhat.com; david.marchand@redhat.com >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com> >> Subject: [PATCH v2 43/44] net/virtio: improve Vhost-user error logging >> >> This patch improves error logging in vhost_user_read, >> especially printing errno when recv() fails. >> >> Suggested-by: Adrian Moreno <amorenoz@redhat.com> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> >> --- >> drivers/net/virtio/virtio_user/vhost_user.c | 29 ++++++++++++--------- >> 1 file changed, 17 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c >> b/drivers/net/virtio/virtio_user/vhost_user.c >> index f046655af6..be91c99cea 100644 >> --- a/drivers/net/virtio/virtio_user/vhost_user.c >> +++ b/drivers/net/virtio/virtio_user/vhost_user.c >> @@ -148,38 +148,43 @@ vhost_user_read(int fd, struct vhost_user_msg *msg) >> int ret, sz_hdr = VHOST_USER_HDR_SIZE, sz_payload; >> >> ret = recv(fd, (void *)msg, sz_hdr, 0); >> - if (ret < sz_hdr) { >> + if (ret < 0) { >> + PMD_DRV_LOG(ERR, "Failed to recv msg header: %s", strerror(errno)); >> + return -1; >> + } else if (ret < sz_hdr) { >> PMD_DRV_LOG(ERR, "Failed to recv msg hdr: %d instead of %d.", >> ret, sz_hdr); >> - goto fail; >> + return -1; >> } >> >> /* validate msg flags */ >> if (msg->flags != (valid_flags)) { >> PMD_DRV_LOG(ERR, "Failed to recv msg: flags %x instead of %x.", >> msg->flags, valid_flags); >> - goto fail; >> + return -1; > > Since you are here, also add '0x' before '%x' here? Done. > Thanks, > Chenbo Thanks, Maxime
> -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Monday, January 25, 2021 11:04 PM > To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org; olivier.matz@6wind.com; > amorenoz@redhat.com; david.marchand@redhat.com > Subject: Re: [PATCH v2 43/44] net/virtio: improve Vhost-user error logging > > > > On 1/22/21 10:11 AM, Xia, Chenbo wrote: > > Hi Maxime, > > > >> -----Original Message----- > >> From: Maxime Coquelin <maxime.coquelin@redhat.com> > >> Sent: Wednesday, January 20, 2021 5:25 AM > >> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; > olivier.matz@6wind.com; > >> amorenoz@redhat.com; david.marchand@redhat.com > >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com> > >> Subject: [PATCH v2 43/44] net/virtio: improve Vhost-user error logging > >> > >> This patch improves error logging in vhost_user_read, > >> especially printing errno when recv() fails. > >> > >> Suggested-by: Adrian Moreno <amorenoz@redhat.com> > >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > >> --- > >> drivers/net/virtio/virtio_user/vhost_user.c | 29 ++++++++++++--------- > >> 1 file changed, 17 insertions(+), 12 deletions(-) > >> > >> diff --git a/drivers/net/virtio/virtio_user/vhost_user.c > >> b/drivers/net/virtio/virtio_user/vhost_user.c > >> index f046655af6..be91c99cea 100644 > >> --- a/drivers/net/virtio/virtio_user/vhost_user.c > >> +++ b/drivers/net/virtio/virtio_user/vhost_user.c > >> @@ -148,38 +148,43 @@ vhost_user_read(int fd, struct vhost_user_msg *msg) > >> int ret, sz_hdr = VHOST_USER_HDR_SIZE, sz_payload; > >> > >> ret = recv(fd, (void *)msg, sz_hdr, 0); > >> - if (ret < sz_hdr) { > >> + if (ret < 0) { > >> + PMD_DRV_LOG(ERR, "Failed to recv msg header: %s", strerror(errno)); > >> + return -1; > >> + } else if (ret < sz_hdr) { > >> PMD_DRV_LOG(ERR, "Failed to recv msg hdr: %d instead of %d.", > >> ret, sz_hdr); > >> - goto fail; > >> + return -1; > >> } > >> > >> /* validate msg flags */ > >> if (msg->flags != (valid_flags)) { > >> PMD_DRV_LOG(ERR, "Failed to recv msg: flags %x instead of %x.", > >> msg->flags, valid_flags); > >> - goto fail; > >> + return -1; > > > > Since you are here, also add '0x' before '%x' here? > > Done. > > > Thanks, > > Chenbo > > Thanks, > Maxime Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
diff --git a/drivers/net/virtio/virtio_user/vhost_user.c b/drivers/net/virtio/virtio_user/vhost_user.c index f046655af6..be91c99cea 100644 --- a/drivers/net/virtio/virtio_user/vhost_user.c +++ b/drivers/net/virtio/virtio_user/vhost_user.c @@ -148,38 +148,43 @@ vhost_user_read(int fd, struct vhost_user_msg *msg) int ret, sz_hdr = VHOST_USER_HDR_SIZE, sz_payload; ret = recv(fd, (void *)msg, sz_hdr, 0); - if (ret < sz_hdr) { + if (ret < 0) { + PMD_DRV_LOG(ERR, "Failed to recv msg header: %s", strerror(errno)); + return -1; + } else if (ret < sz_hdr) { PMD_DRV_LOG(ERR, "Failed to recv msg hdr: %d instead of %d.", ret, sz_hdr); - goto fail; + return -1; } /* validate msg flags */ if (msg->flags != (valid_flags)) { PMD_DRV_LOG(ERR, "Failed to recv msg: flags %x instead of %x.", msg->flags, valid_flags); - goto fail; + return -1; } sz_payload = msg->size; - if ((size_t)sz_payload > sizeof(msg->payload)) - goto fail; + if ((size_t)sz_payload > sizeof(msg->payload)) { + PMD_DRV_LOG(ERR, "Payload size overflow, header says %d but max %zu\n", + sz_payload, sizeof(msg->payload)); + return -1; + } if (sz_payload) { ret = recv(fd, (void *)((char *)msg + sz_hdr), sz_payload, 0); - if (ret < sz_payload) { - PMD_DRV_LOG(ERR, - "Failed to recv msg payload: %d instead of %d.", + if (ret < 0) { + PMD_DRV_LOG(ERR, "Failed to recv msg payload: %s", strerror(errno)); + return -1; + } else if (ret < sz_payload) { + PMD_DRV_LOG(ERR, "Failed to recv msg payload: %d instead of %u.", ret, msg->size); - goto fail; + return -1; } } return 0; - -fail: - return -1; } static int
This patch improves error logging in vhost_user_read, especially printing errno when recv() fails. Suggested-by: Adrian Moreno <amorenoz@redhat.com> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/net/virtio/virtio_user/vhost_user.c | 29 ++++++++++++--------- 1 file changed, 17 insertions(+), 12 deletions(-)