From patchwork Thu Aug 26 17:45:58 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Greear X-Patchwork-Id: 62794 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id DB35DB70DF for ; Fri, 27 Aug 2010 03:46:10 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753190Ab0HZRqF (ORCPT ); Thu, 26 Aug 2010 13:46:05 -0400 Received: from mail.candelatech.com ([208.74.158.172]:42202 "EHLO ns3.lanforge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753031Ab0HZRqE (ORCPT ); Thu, 26 Aug 2010 13:46:04 -0400 Received: from [192.168.100.195] (firewall.candelatech.com [70.89.124.249]) (authenticated bits=0) by ns3.lanforge.com (8.14.2/8.14.2) with ESMTP id o7QHjxCo012289 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 26 Aug 2010 10:46:00 -0700 Message-ID: <4C76A856.2040202@candelatech.com> Date: Thu, 26 Aug 2010 10:45:58 -0700 From: Ben Greear Organization: Candela Technologies User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100430 Fedora/3.0.4-2.fc11 Thunderbird/3.0.4 MIME-Version: 1.0 To: Arnd Bergmann CC: netdev@vger.kernel.org Subject: Re: [net-next 2/2] macvlan: Enable qdisc backoff logic. References: <1282762851-3612-1-git-send-email-greearb@candelatech.com> <201008252159.11227.arnd@arndb.de> <4C7581D2.3010007@candelatech.com> <201008261555.35250.arnd@arndb.de> In-Reply-To: <201008261555.35250.arnd@arndb.de> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 08/26/2010 06:55 AM, Arnd Bergmann wrote: > On Wednesday 25 August 2010, Ben Greear wrote: >> On 08/25/2010 12:59 PM, Arnd Bergmann wrote: >>> On Wednesday 25 August 2010 21:27:43 Ben Greear wrote: >>>>> I suppose we need to do something in macvtap to handle this as >>>>> well, right? A guest trying to send a frame through qemu >>>>> or vhost net into macvtap needs to be prevented from sending >>>>> more when we get into this path. Right now, we just ignore >>>>> the return value of macvlan_start_xmit. >>>> >>>> I have a similar, though slightly more complex, patch for 802.1q >>>> vlans, but I haven't looked at macvtap at all. >>>> >>>> If these two patches are accepted, I'll post the .1q patch as well. >>> >>> I think one of us needs to fix macvtap in order for your patch to >>> go in, because otherwise there is a memory leak or worse when >>> macvtap fails to retransmit the frame. >> >> With no change, the try_ logic will not be called, so it should >> be fully backwards compatible. > > How? The macvlan driver is used as the back-end for macvtap, > so it calls all the same functions: > > macvtap_write > -> macvtap_get_user > -> macvlan_start_xmit > -> macvlan->queue_xmit > -> try_dev_queue_xmit I think this will keep today's functionality. Someone that knows and uses this code might can figure out how to properly do backpressure to calling code and re-queue the skb instead of just deleting it when the underlying device complains of being busy. If this looks good, I'll do up an official patch set containing this and the other two patches I sent previously. Thanks, Ben > > Arnd > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 4256727..5abf0c0 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -571,9 +571,15 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, rcu_read_lock_bh(); vlan = rcu_dereference(q->vlan); - if (vlan) - macvlan_start_xmit(skb, vlan->dev); - else + if (vlan) { + /* TODO: Deal with BUSY properly by somehow re-queuing + * skb for later transmit and let calling logic know it + * needs to back off for a short time. + */ + if (macvlan_start_xmit(skb, vlan->dev) == NETDEV_TX_BUSY) + goto free_skb; + } else +free_skb: kfree_skb(skb); rcu_read_unlock_bh();