From patchwork Wed Apr 12 22:37:27 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tushar Dave X-Patchwork-Id: 750183 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 3w3JkK0R0Zz9sN9 for ; Thu, 13 Apr 2017 08:39:05 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755759AbdDLWjC (ORCPT ); Wed, 12 Apr 2017 18:39:02 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:42114 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755742AbdDLWi6 (ORCPT ); Wed, 12 Apr 2017 18:38:58 -0400 Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id v3CMcfLW008564 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 12 Apr 2017 22:38:41 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id v3CMcfPf020486 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 12 Apr 2017 22:38:41 GMT Received: from abhmp0014.oracle.com (abhmp0014.oracle.com [141.146.116.20]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id v3CMceZe026169; Wed, 12 Apr 2017 22:38:40 GMT Received: from [10.159.245.176] (/10.159.245.176) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 12 Apr 2017 15:38:40 -0700 Subject: Re: [PATCH] netpoll: Check for skb->queue_mapping To: Eric Dumazet References: <1491444383-29017-1-git-send-email-tushar.n.dave@oracle.com> <1491474377.10124.57.camel@edumazet-glaptop3.roam.corp.google.com> <5927b630-865e-b7af-33a1-1a6b772e3fb7@oracle.com> <1491506056.10124.81.camel@edumazet-glaptop3.roam.corp.google.com> Cc: davem@davemloft.net, edumazet@google.com, brouer@redhat.com, netdev@vger.kernel.org, sowmini.varadhan@oracle.com From: tndave Message-ID: Date: Wed, 12 Apr 2017 15:37:27 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1491506056.10124.81.camel@edumazet-glaptop3.roam.corp.google.com> X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 04/06/2017 12:14 PM, Eric Dumazet wrote: > On Thu, 2017-04-06 at 12:07 -0700, tndave wrote: > >>> + q_index = q_index % dev->real_num_tx_queues; >> cpu interrupted here and dev->real_num_tx_queues has reduced! >>> + skb_set_queue_mapping(skb, q_index); >>> + } >>> + txq = netdev_get_tx_queue(dev, q_index); >> or cpu interrupted here and dev->real_num_tx_queues has reduced! > > If dev->real_num_tx_queues can be changed while this code is running we > are in deep deep trouble. > > Better make sure that when control path does this change, device (and/pr > netpoll) is frozen and no packet can be sent. When control path is making change to real_num_tx_queues, underlying device is disabled; also netdev tx queues are stopped/disabled so certainly no transmit is happening. The corner case I was referring is if netpoll's queue_process() code is interrupted and while it is not running, control path makes change to dev->real_num_tx_queues and exits. Later on, interrupted queue_process() resume execution and it can end up with wrong skb->queue_mapping and txq. We can prevent this case with below change: Thanks for your patience. -Tushar > > > > diff --git a/net/core/netpoll.c b/net/core/netpoll.c index 9424673..29be246 100644 --- a/net/core/netpoll.c +++ b/net/core/netpoll.c @@ -105,15 +105,21 @@ static void queue_process(struct work_struct *work) while ((skb = skb_dequeue(&npinfo->txq))) { struct net_device *dev = skb->dev; struct netdev_queue *txq; + unsigned int q_index; if (!netif_device_present(dev) || !netif_running(dev)) { kfree_skb(skb); continue; } - txq = skb_get_tx_queue(dev, skb); - local_irq_save(flags); + /* check if skb->queue_mapping is still valid */ + q_index = skb_get_queue_mapping(skb); + if (unlikely(q_index >= dev->real_num_tx_queues)) { + q_index = q_index % dev->real_num_tx_queues; + skb_set_queue_mapping(skb, q_index); + } + txq = netdev_get_tx_queue(dev, q_index); HARD_TX_LOCK(dev, txq, smp_processor_id()); if (netif_xmit_frozen_or_stopped(txq) || netpoll_start_xmit(skb, dev, txq) != NETDEV_TX_OK) {