From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mark McLoughlin <markmc@redhat.com>
Subject: Re: [PATCH 2/3] kvm: qemu: handle link status in
 qemu_sendv_packet()
Date: Thu, 08 Jan 2009 18:37:34 +0000
Message-ID: <1231439854.25753.2.camel@localhost.localdomain>
References: <> <1228821613-28724-1-git-send-email-markmc@redhat.com>
	 <1228821613-28724-2-git-send-email-markmc@redhat.com>
	 <1228821613-28724-3-git-send-email-markmc@redhat.com>
	 <493E875D.3010803@codemonkey.ws>
Reply-To: Mark McLoughlin <markmc@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Cc: Avi Kivity <avi@redhat.com>, kvm@vger.kernel.org,
	Rusty Russell <rusty@rustcorp.com.au>
To: Anthony Liguori <anthony@codemonkey.ws>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:33932 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751354AbZAHShq (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 8 Jan 2009 13:37:46 -0500
In-Reply-To: <493E875D.3010803@codemonkey.ws>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Tue, 2008-12-09 at 08:57 -0600, Anthony Liguori wrote:
> Mark McLoughlin wrote:
> > Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> > ---
> >  qemu/net.c |   18 +++++++++++++++++-
> >  1 files changed, 17 insertions(+), 1 deletions(-)
> >
> > diff --git a/qemu/net.c b/qemu/net.c
> > index 16a0990..f23a17f 100644
> > --- a/qemu/net.c
> > +++ b/qemu/net.c
> > @@ -400,6 +400,17 @@ static ssize_t vc_sendv_compat(VLANClientState *vc, const struct iovec *iov,
> >      return offset;
> >  }
> >  
> > +static ssize_t calc_iov_length(const struct iovec *iov, int iovcnt)
> > +{
> > +    size_t offset = 0;
> > +    int i;
> > +
> > +    for (i = 0; i < iovcnt; i++)
> > +	offset += iov[i].iov_len;
> > +
> > +    return offset;
> > +}
> > +
> >  ssize_t qemu_sendv_packet(VLANClientState *vc1, const struct iovec *iov,
> >  			  int iovcnt)
> >  {
> > @@ -407,13 +418,18 @@ ssize_t qemu_sendv_packet(VLANClientState *vc1, const struct iovec *iov,
> >      VLANClientState *vc;
> >      ssize_t max_len = 0;
> >  
> > +    if (vc1->link_down)
> > +	return calc_iov_length(iov, iovcnt);
> > +
> >      for (vc = vlan->first_client; vc != NULL; vc = vc->next) {
> >  	ssize_t len = 0;
> >  
> >  	if (vc == vc1)
> >  	    continue;
> >  
> > -	if (vc->fd_readv)
> > +	if (vc->link_down)
> > +	    len = calc_iov_length(iov, iovcnt);
> >   
> 
> Instead of returning a success and silently dropping the packet, maybe 
> it would be better to return an error and let the card deal with 
> dropping the packet.
> 
> In real hardware, the link down would mean the TX queue would fill up 
> because packets aren't able to be sent.

I failed to figure out for real what typical hardware would do, e.g. by
reading the e1000 spec, but I figure the current behaviour is good
enough for now?

Re-based patches follow.

Cheers,
Mark.