Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
From:   JustinLai0215 <justinlai0215@realtek.com>
To:     Jakub Kicinski <kuba@kernel.org>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "andrew@lunn.ch" <andrew@lunn.ch>,
        Ping-Ke Shih <pkshih@realtek.com>,
        Larry Chiu <larry.chiu@realtek.com>
Subject: RE: [PATCH net-next v14 03/13] rtase: Implement the rtase_down function
Thread-Topic: [PATCH net-next v14 03/13] rtase: Implement the rtase_down
 function
Thread-Index: AQHaKbuZ68k3qmPPt0euso8G9Dk2ErCliPGAgAM8MgA=
Date:   Thu, 14 Dec 2023 12:54:11 +0000
Message-ID: <39c9b2df2a02412b84f6e7946f012917@realtek.com>
References: <20231208094733.1671296-1-justinlai0215@realtek.com>
        <20231208094733.1671296-4-justinlai0215@realtek.com>
 <20231212112735.180d455f@kernel.org>
In-Reply-To: <20231212112735.180d455f@kernel.org>
Accept-Language: zh-TW, en-US
Content-Language: zh-TW
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 8BIT
MIME-Version: 1.0
Precedence: bulk

> 
> On Fri, 8 Dec 2023 17:47:23 +0800 Justin Lai wrote:
> > +     netif_stop_queue(dev);
> 
> You most likely want to stop the napi before you call this.
> Otherwise NAPI can do some clean up and restart the queue.
> 
> > +     /* give a racing hard_start_xmit a few cycles to complete */
> > +     synchronize_rcu();
> 
> Call netif_tx_disable() instead of stop_queue(), it takes the tx lock so you don't
> have to worry about in-flight packets.

Thanks for your suggestion, I will modify it.
> 
> > +     netif_carrier_off(dev);
> > +
> > +     for (i = 0; i < tp->int_nums; i++) {
> > +             ivec = &tp->int_vector[i];
> > +             synchronize_irq(ivec->irq);
> 
> Why?

This is redundant code, I will remove it.
> 
> > +             /* wait for any pending NAPI task to complete */
> > +             napi_disable(&ivec->napi);
> > +     }