diff mbox

[dpdk-dev] vhost: don't stall if guest is slow

Message ID 1449535821-12384-1-git-send-email-stephen@networkplumber.org (mailing list archive)
State Accepted, archived
Headers

Commit Message

Stephen Hemminger Dec. 8, 2015, 12:50 a.m. UTC 
  When guest is booting (or any othertime guest is busy) it is possible
for the small receive ring (256) to get full. If this happens the
vhost library should just return normally. It's current behavior
of logging just creates massive log spew/overflow which could even
act as a DoS attack against host.

Reported-by: Nathan Law <nlaw@brocade.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_vhost/vhost_rxtx.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

Comments

Yuanhan Liu Dec. 8, 2015, 2:23 a.m. UTC | #1 
On Mon, Dec 07, 2015 at 04:50:21PM -0800, Stephen Hemminger wrote:
> When guest is booting (or any othertime guest is busy) it is possible
> for the small receive ring (256) to get full. If this happens the
> vhost library should just return normally. It's current behavior
> of logging just creates massive log spew/overflow which could even
> act as a DoS attack against host.
> 
> Reported-by: Nathan Law <nlaw@brocade.com>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Thanks.

	--yliu
Thomas Monjalon Dec. 8, 2015, 2:30 a.m. UTC | #2 
2015-12-08 10:23, Yuanhan Liu:
> On Mon, Dec 07, 2015 at 04:50:21PM -0800, Stephen Hemminger wrote:
> > When guest is booting (or any othertime guest is busy) it is possible
> > for the small receive ring (256) to get full. If this happens the
> > vhost library should just return normally. It's current behavior
> > of logging just creates massive log spew/overflow which could even
> > act as a DoS attack against host.
> > 
> > Reported-by: Nathan Law <nlaw@brocade.com>
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Yuanhan, please what is your opinion about the balance benefit/risk
of this patch for 2.2?
Yuanhan Liu Dec. 8, 2015, 3:12 a.m. UTC | #3 
On Tue, Dec 08, 2015 at 03:30:43AM +0100, Thomas Monjalon wrote:
> 2015-12-08 10:23, Yuanhan Liu:
> > On Mon, Dec 07, 2015 at 04:50:21PM -0800, Stephen Hemminger wrote:
> > > When guest is booting (or any othertime guest is busy) it is possible
> > > for the small receive ring (256) to get full. If this happens the
> > > vhost library should just return normally. It's current behavior
> > > of logging just creates massive log spew/overflow which could even
> > > act as a DoS attack against host.
> > > 
> > > Reported-by: Nathan Law <nlaw@brocade.com>
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > 
> > Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> 
> Yuanhan, please what is your opinion about the balance benefit/risk
> of this patch for 2.2?

Thomas,

I agree with Stephen that such log is not necessary as it's a fair
normal case. However, I see no too much benefit/risk for 2.2: it
just removes a debug log after all.

	--yliu
Thomas Monjalon Dec. 9, 2015, 8:02 p.m. UTC | #4 
2015-12-08 11:12, Yuanhan Liu:
> On Tue, Dec 08, 2015 at 03:30:43AM +0100, Thomas Monjalon wrote:
> > 2015-12-08 10:23, Yuanhan Liu:
> > > On Mon, Dec 07, 2015 at 04:50:21PM -0800, Stephen Hemminger wrote:
> > > > When guest is booting (or any othertime guest is busy) it is possible
> > > > for the small receive ring (256) to get full. If this happens the
> > > > vhost library should just return normally. It's current behavior
> > > > of logging just creates massive log spew/overflow which could even
> > > > act as a DoS attack against host.
> > > > 
> > > > Reported-by: Nathan Law <nlaw@brocade.com>
> > > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > 
> > > Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> > 
> > Yuanhan, please what is your opinion about the balance benefit/risk
> > of this patch for 2.2?
> 
> Thomas,
> 
> I agree with Stephen that such log is not necessary as it's a fair
> normal case. However, I see no too much benefit/risk for 2.2: it
> just removes a debug log after all.

Applied, thanks
diff mbox

Patch

diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 9322ce6..bbf3fac 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -510,17 +510,12 @@  virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
 
 			do {
 				avail_idx = *((volatile uint16_t *)&vq->avail->idx);
-				if (unlikely(res_cur_idx == avail_idx)) {
-					LOG_DEBUG(VHOST_DATA,
-						"(%"PRIu64") Failed "
-						"to get enough desc from "
-						"vring\n",
-						dev->device_fh);
+				if (unlikely(res_cur_idx == avail_idx))
 					goto merge_rx_exit;
-				} else {
-					update_secure_len(vq, res_cur_idx, &secure_len, &vec_idx);
-					res_cur_idx++;
-				}
+
+				update_secure_len(vq, res_cur_idx,
+						  &secure_len, &vec_idx);
+				res_cur_idx++;
 			} while (pkt_len > secure_len);
 
 			/* vq->last_used_idx_res is atomically updated. */