netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zoltan Kiss <zoltan.kiss@citrix.com>
To: David Miller <davem@davemloft.net>
Cc: <wei.liu2@citrix.com>, <Ian.Campbell@citrix.com>,
	<david.vrabel@citrix.com>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH net-next 0/2] xen-netback: Changes around carrier handling
Date: Thu, 7 Aug 2014 17:49:37 +0100	[thread overview]
Message-ID: <53E3AE21.50109@citrix.com> (raw)
In-Reply-To: <20140805.160748.1185917042908283028.davem@davemloft.net>

On 06/08/14 00:07, David Miller wrote:
> From: Zoltan Kiss <zoltan.kiss@citrix.com>
> Date: Mon, 4 Aug 2014 16:20:56 +0100
>
>> This series starts using carrier off as a way to purge packets when the guest is
>> not able (or willing) to receive them. It is a much faster way to get rid of
>> packets waiting for an overwhelmed guest.
>> The first patch changes current netback code where it relies currently on
>> netif_carrier_ok.
>> The second turns off the carrier if the guest times out on a queue, and only
>> turn it on again if that queue (or queues) resurrects.
>>
>> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
>> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
>
> Applied, but I have some reservations:
>
> 1) This is starting to bleed what is normally qdisc type policy into the
>     driver.
>
> 2) There are other drivers that could run into this kind of situation and
>     have similar concerns, therefore we should make sure we have a consistent
>     approach that such entities use to handle this problem.
>
> Part of the problem is that netif_carrier_off() only partially mimicks
> the situation.  It expresses the "transmitter is down so packets
> aren't going onto the wire" part, which keeps the watchdog from
> spitting out log messages ever time it fires.  But it doesn't deal
> with packet freeing policy meanwhile, which I guess is the part that
> this patch series is largely trying to address.

David Vrabel pointed out an important question in a reply to the 
previous version of this series: this patch deschedule NAPI if the 
carrier goes down. The backend doesn't receive packets from the guest. 
DavidVr and others said we shouldn't do this, the guest should be able 
to transmit even if it's not able/willing to receive. Other drivers 
doesn't deschedule NAPI at carrier off as well, however the "carrier 
off" information comes from the hardware, not from an untrusted guest 
who is not posting buffers on the receive ring.
I don't have any good argument why I did it the current way, other than 
a hunch that it feels more natural.
David, do you have an opinion on that?

Zoli

  parent reply	other threads:[~2014-08-07 16:49 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-04 15:20 [PATCH net-next 0/2] xen-netback: Changes around carrier handling Zoltan Kiss
2014-08-04 15:20 ` [PATCH net-next 1/2] xen-netback: Using a new state bit instead of carrier Zoltan Kiss
2014-08-05 12:45   ` Wei Liu
2014-08-06 18:25     ` Zoltan Kiss
2014-08-04 15:20 ` [PATCH net-next 2/2] xen-netback: Turn off the carrier if the guest is not able to receive Zoltan Kiss
2014-08-05 12:45   ` Wei Liu
2014-08-06 19:18     ` Zoltan Kiss
2014-08-04 15:34 ` [PATCH net-next 0/2] xen-netback: Changes around carrier handling Zoltan Kiss
2014-08-05 23:07 ` David Miller
2014-08-06  0:00   ` Wei Liu
2014-08-06  1:50     ` David Miller
2014-08-06  8:54       ` Wei Liu
2014-08-06 19:20   ` Zoltan Kiss
2014-08-06 21:01     ` David Miller
2014-08-07 15:51       ` Zoltan Kiss
2014-08-08  5:28         ` David Miller
2014-08-07 16:49   ` Zoltan Kiss [this message]
2014-08-08  5:29     ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53E3AE21.50109@citrix.com \
    --to=zoltan.kiss@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=davem@davemloft.net \
    --cc=david.vrabel@citrix.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).