Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 22 Nov 2022 17:26:50 +0100
From:   Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To:     Akihiko Odaki <akihiko.odaki@daynix.com>
CC:     <linux-kernel@vger.kernel.org>, <netdev@vger.kernel.org>,
        <intel-wired-lan@lists.osuosl.org>,
        Jesse Brandeburg <jesse.brandeburg@intel.com>,
        Tony Nguyen <anthony.l.nguyen@intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "Yan Vugenfirer" <yan@daynix.com>,
        Yuri Benditovich <yuri.benditovich@daynix.com>
Subject: Re: [PATCH v2] igb: Allocate MSI-X vector when testing
Message-ID: <Y3z4SlmtA/u/3ipA@boxer>
References: <20221122131145.68107-1-akihiko.odaki@daynix.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20221122131145.68107-1-akihiko.odaki@daynix.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?WtfLo6vdgKqesLYcis5gzO+MtWK27uwFVqJfDVzZWERMJ2uGV7M/HpCMf8pR?=
 =?us-ascii?Q?s0jBj8rM5k/MeKBUwFX/ckUXYfahw1Hq3MzaxSjXInWLKa+9jiA54k3k7eKr?=
 =?us-ascii?Q?frs6yqVY64khaXv1drLWti3mWhqbNDVuHSlU7RBpt+Ds/Krs+dLgeyCqJuyP?=
 =?us-ascii?Q?uWndAMvcqOrdrwCrqO+ViU0g/D5/98r8SjWOJXCLdT/wJsJurtZIDKVlX9iM?=
 =?us-ascii?Q?gj6SGYV8Jtmq13pUHkdE/mknM1NNTqnrdq9dD12Gxw+d7aRiaEZ9UDIe/C1A?=
 =?us-ascii?Q?iK5n7bylF6AZARx+BjKiby2fYSSgRaUQFSZ63XKGJaCi2AxBWoQDa0HFTx41?=
 =?us-ascii?Q?Bw5uBV+Km4LbuFZdWDZ/VDkdw1YMSHG0MpiPDqsDKAUvtDY6dbnkKLksbtAb?=
 =?us-ascii?Q?NwLngob+xecyqADCmBN9eGE4i1yfGtsTZcbbxWdZpfcxj82hoEGtVx6wCRi8?=
 =?us-ascii?Q?C/E6gyHVyIGNtWOa+Q2PWzhtez/PD+Bzp7XrtEeScyEBcWyChHV6npJC4Hji?=
 =?us-ascii?Q?3XOJjXlhn1NwLeE3kKQKarPmlmeUg1waoVX1UQkT9X7wTXDqyOuB1VOvKBey?=
 =?us-ascii?Q?WBUwITCLUTrpbx+B2ZvXwIORqT1f+PKruNtYhL5lN3IKH+CKOhvxl1pEi6vU?=
 =?us-ascii?Q?qgNXRKfy/4YNxHXUL9ZabWW2z7IRw5w8U3t9EhNB2xcYY9+pcq/GpDE0jdDP?=
 =?us-ascii?Q?17HdAaRNac/Gs325WkU0gg4HWEzcWGtdE3OUPjQJgoSp9570gpwyCvAWTcxI?=
 =?us-ascii?Q?A4oZr6iPRySfe1aGvB3gtDhdk1dw8lIIGYSwPzzwJ0m+QOaih//w+Z94lXe4?=
 =?us-ascii?Q?ctUmgBhVGUhcPde3dD6BB0+bMZeWARZ2tW0gw5nw/85jMVJ/y+FQjCTaOksh?=
 =?us-ascii?Q?yjeyMjrY5mxFPbY33QoIP96E1otAh2bpLsHXRSStvkJlr+v29eMQYf82QKlP?=
 =?us-ascii?Q?AYqVKEIRMhDAp0I/WFkIH+79C0gP/IeLhSmFYwwn5jMuqr3sw9SjAv2TVZea?=
 =?us-ascii?Q?TftUxFIC38fknkrXGXyw9LfWUkeOyzvVkR+ToJnrHcjcTix/Z6QQRIPMLSFw?=
 =?us-ascii?Q?AZ8GYL2/GE+8ZKgX3SaxXP9ozRop8qYRVbINGapzYnhvu5PdqUUlO7QtZIxI?=
 =?us-ascii?Q?XPKVfWERUKhxJIXkUC1Umkv8lu36CUsEshaaqYRYZeeV63KQ3BUih/CUHTOC?=
 =?us-ascii?Q?wCqQv4NwV3raDhzm2KmMfm0KzhS/K2BQi0fFhjHD50Y2MHaoy670hPCI4WUI?=
 =?us-ascii?Q?ytx8JSmU8zwU8ZJHl2+7CD7R1Dys0jPwKs9FrOEQfb78Qt5J0mpPDdsGxecG?=
 =?us-ascii?Q?6EaegP7sucj7PoO2JstpAltyevLJ8vMO1tsaHdGZMZlLVRJss53sy5Ni9cz2?=
 =?us-ascii?Q?hTeA4ZPqWJOv2uPVAujUZ/lMyq1OSZP9rvv6Z7kJ2T1rWRxPdfTC9JIaTmSN?=
 =?us-ascii?Q?cFYxABLTGC+uEqcKdfw0Y5Cuobi6HRU0sRotu2hxGiaWDFnNiu7uLFWNsZZo?=
 =?us-ascii?Q?5GUEcdh/aphFW2mvSv4lWtY4DWmiQ/8DBt6oCamnsLEM/YUgMOE8UAHi4hT0?=
 =?us-ascii?Q?y8P+Douv56whayatf8Yhj3P3rirquAIdkNJk3ng8VcONCLPEU7Ej904o0JqM?=
 =?us-ascii?Q?sQ=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 631f2858-c9cc-4882-4da0-08dacca65fc4
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6117.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Nov 2022 16:26:57.0934
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: vFdmLiKKWY7U0lzqOnk5H0tsH9yUdiixwkhU1GQwM0MazGd0FeC9K7tzmmcu86iovhHZhruqWSKUoTrHot39Ox90p04N4lxJYlXjQ/7Gahg=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6748
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Nov 22, 2022 at 10:11:45PM +0900, Akihiko Odaki wrote:
> Without this change, the interrupt test fail with MSI-X environment:
> 
> $ sudo ethtool -t enp0s2 offline
> [   43.921783] igb 0000:00:02.0: offline testing starting
> [   44.855824] igb 0000:00:02.0 enp0s2: igb: enp0s2 NIC Link is Down
> [   44.961249] igb 0000:00:02.0 enp0s2: igb: enp0s2 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
> [   51.272202] igb 0000:00:02.0: testing shared interrupt
> [   56.996975] igb 0000:00:02.0 enp0s2: igb: enp0s2 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
> The test result is FAIL
> The test extra info:
> Register test  (offline)	 0
> Eeprom test    (offline)	 0
> Interrupt test (offline)	 4
> Loopback test  (offline)	 0
> Link test   (on/offline)	 0
> 
> Here, "4" means an expected interrupt was not delivered.
> 
> This change routes interrupts correctly to the first MSI-X vector, and
> fixes the test:

Try to use imperative mood - simply stating something like "To fix this,
route IRQs correctly to the first MSI-X vector by xyz"

> 
> $ sudo ethtool -t enp0s2 offline
> [   42.762985] igb 0000:00:02.0: offline testing starting
> [   50.141967] igb 0000:00:02.0: testing shared interrupt
> [   56.163957] igb 0000:00:02.0 enp0s2: igb: enp0s2 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
> The test result is PASS
> The test extra info:
> Register test  (offline)	 0
> Eeprom test    (offline)	 0
> Interrupt test (offline)	 0
> Loopback test  (offline)	 0
> Link test   (on/offline)	 0

Looks better now, would be also good to explain what is the *actual* fix
by explaining the HW registers setting that you're doing.

Thanks!

> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> index e5f3e7680dc6..ff911af16a4b 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
> @@ -1413,6 +1413,8 @@ static int igb_intr_test(struct igb_adapter *adapter, u64 *data)
>  			*data = 1;
>  			return -1;
>  		}
> +		wr32(E1000_IVAR_MISC, E1000_IVAR_VALID << 8);
> +		wr32(E1000_EIMS, BIT(0));
>  	} else if (adapter->flags & IGB_FLAG_HAS_MSI) {
>  		shared_int = false;
>  		if (request_irq(irq,
> -- 
> 2.38.1
>