From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161423AbbBDWlx (ORCPT ); Wed, 4 Feb 2015 17:41:53 -0500 Received: from mail-bl2on0125.outbound.protection.outlook.com ([65.55.169.125]:21769 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S964887AbbBDWlv (ORCPT ); Wed, 4 Feb 2015 17:41:51 -0500 From: Haiyang Zhang To: Jason Wang CC: "davem@davemloft.net" , "netdev@vger.kernel.org" , KY Srinivasan , "olaf@aepfle.de" , "linux-kernel@vger.kernel.org" , "driverdev-devel@linuxdriverproject.org" Subject: RE: [PATCH net] hyperv: Fix the error processing in netvsc_send() Thread-Topic: [PATCH net] hyperv: Fix the error processing in netvsc_send() Thread-Index: AQHQQExFPyD0DaGOdEuEBUqp+E+oB5zhCMQA Date: Wed, 4 Feb 2015 22:26:51 +0000 Message-ID: References: <1422563689-31036-1-git-send-email-haiyangz@microsoft.com> <1422613519.8840.0@smtp.corp.redhat.com> <1422859762.7028.2@smtp.corp.redhat.com> <1423034952.10558.3@smtp.corp.redhat.com> In-Reply-To: <1423034952.10558.3@smtp.corp.redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [96.237.174.38] authentication-results: redhat.com; dkim=none (message not signed) header.d=none; x-microsoft-antispam: BCL:0;PCL:0;RULEID:;SRVR:BN1PR0301MB0705;UriScan:; x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:;SRVR:BN1PR0301MB0705; x-forefront-prvs: 04772EA191 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(51704005)(13464003)(377454003)(164054003)(76176999)(74316001)(19580405001)(46102003)(66066001)(40100003)(93886004)(33656002)(19580395003)(76576001)(77156002)(50986999)(62966003)(92566002)(122556002)(102836002)(86362001)(110136001)(86612001)(87936001)(106116001)(54356999)(2656002)(2900100001)(2950100001);DIR:OUT;SFP:1102;SCL:1;SRVR:BN1PR0301MB0705;H:BN1PR0301MB0770.namprd03.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Feb 2015 22:26:51.4611 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN1PR0301MB0705 X-Microsoft-Antispam: BCL:0;PCL:0;RULEID:;SRVR:BN1PR0301MB0737; X-OriginatorOrg: microsoft.onmicrosoft.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id t14Mg0B7021213 > -----Original Message----- > From: Jason Wang [mailto:jasowang@redhat.com] > Sent: Wednesday, February 4, 2015 2:29 AM > > The EAGAIN error doesn't normally happen, because we set the hi water > > mark > > to stop send queue. > > This is not true since only txq was stopped which means only network > stack stop sending packets but not for control path e.g > rndis_filter_send_request() or other callers who call > vmbus_sendpacket() directly (e.g recv completion). > > For control path, user may meet several errors when they want to change > mac address under heavy load. > > What's more serious is netvsc_send_recv_completion(), it can not even > recover from more than 3 times of EAGAIN. > > I must say mixing data packets with control packets with the same > channel sounds really scary. Since control packets could be blocked or > even dropped because of data packets already queued during heavy load, > and you need to synchronize two paths carefully (e.g I didn't see any > tx lock were held if rndis_filter_send_request() call netsc_send() > which may stop or start a queue). The RING_AVAIL_PERCENT_HIWATER is defined to be 20, so the data traffic can only occupy 20% of the ring buffer before stopping the txq. So, this mechanism ensures the control messages are not blocked by data traffic. > > If in really rare case, the ring buffer is full and there > > is no outstanding sends, we can't stop queue here because there will > > be no > > send-completion msg to wake it up. > > Confused, I believe only txq is stopped but we may still get completion > interrupt in this case. If there is no outstanding sends in this queue (queue_sends[q_idx]), we won't receive any more send-completion msg. > > > And, the ring buffer is likely to be > > occupied by other special msg, e.g. receive-completion msg (not a > > normal case), > > so we can't assume there are available slots. > > Then why not checking hv_ringbuf_avail_percent() instead? And there's > no need to check queue_sends since it does not count recv completion. When ret == -EAGAIN, which means the ring is full, we don't need to check hv_ringbuf_avail_percent(). > > We don't request retry from > > the upper layer in this case to avoid possible busy retry. > > Can't we just do this by stopping txq and depending on tx interrupt to > wake it? There is no tx interrupt. Do you mean rx interrupt for the send-completion? In usual cases, when we hit the high water mark, the stopped queue depends on the send-completion msg to wake up. But, not in some special cases. As said above, we won't receive any more send-completion msg when there is no outstanding sends in this queue. Thanks, - Haiyang {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I