Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <4ce32363-378c-4ea3-9a4e-d7274d4f7787@suse.com>
Date:   Fri, 1 Dec 2023 21:14:46 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3] net: usb: ax88179_178a: avoid failed operations when
 device is disconnected
To:     Jose Ignacio Tornos Martinez <jtornosm@redhat.com>, greg@kroah.com
Cc:     davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        netdev@vger.kernel.org, oneukum@suse.com, pabeni@redhat.com,
        stern@rowland.harvard.edu, stable@vger.kernel.org
References: <2023120130-repair-tackle-698e@gregkh>
 <20231201132647.178979-1-jtornosm@redhat.com>
Content-Language: en-US
From:   Oliver Neukum <oneukum@suse.com>
In-Reply-To: <20231201132647.178979-1-jtornosm@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N3NDMmdyZUJaajNGeHh2NFdVSXdIK1liM0d1cWhTbzZPZWIrbXFNc1c1U2N4?=
 =?utf-8?B?N2NxUS9JTnhQQytmanRldWFVanN1bGRuZXpUVlFZbWNISkVCZ3BPN0kwOU14?=
 =?utf-8?B?U0cxV0p4SnRmVlBNQzJMODVWL2ZMZjQyaG5vMnlFeHNlUzQ0R2xPM1NGb3g2?=
 =?utf-8?B?bXZuMTZvclJDVmNKZVhvQktLYkZEeFI1ZUp4ckRoYlJ3L00vcXlsUmpRVlk4?=
 =?utf-8?B?Ukl1NnlmZ3c4N0VPYlF6Nk5IbVR1MjJsbjM1QUxKVlduMEVSQVYwSnRuK2pk?=
 =?utf-8?B?K3lPR2pLL2FnYVdhQ2tWOEFCT0ZnS2QwK2dLdE8wUFFYR2x0ZVVPb1k5ZjM0?=
 =?utf-8?B?RWVsblRaWXdkcFBYVTJKa1RpU2phV1FJcCtHbEFrRjJiZkc3cGhTcFB5NEMv?=
 =?utf-8?B?djYydHVQRWwvSFQzQ1pDVWRaZnJxRFJKU3VlbFVzVU1iQ21nTEJpZHEyL1dy?=
 =?utf-8?B?dlBESEhZdHB1aGZUYlJYUFJid1AvYXZOeXFrZk14QzdiemZOK0pNOFRLN0Jm?=
 =?utf-8?B?b3NVQzZqRjhFdVJoajdRMDdUczYwZE1HNWhqZFdRcmhqMWxyQVRxRU1lbWZU?=
 =?utf-8?B?L3dDYkhnOSs5MHdHRlY1YXVuSERuM0VSeDVvODg2d3NzeUVXVnVDZDJkZGd2?=
 =?utf-8?B?cUFrT2R3N0x4UjNOQS84azB5WWtNNzUxOFBkbk9zMytIazBYUmY3MHhrV0Ji?=
 =?utf-8?B?V2lNb2k5K004VS9ib2thMkE1NWFFOTBMSnI5TWpvSWlTcEh4UVl1TWR2OFRU?=
 =?utf-8?B?UndPUWw4WjBUUHlkY0lnOU8veXFVYlBFM3Uzam5iZFgrTEYrQnU5U2RONVU3?=
 =?utf-8?B?clN5V0IvQWo2UkY0bi9heC9rYnFhNzVHMG0zS0JJRi9zK0lUWVRuVEdXNXlD?=
 =?utf-8?B?RU1YVGtkUGxnREprM0JCL3JmQ1ZSMVZkYVg1MXVobENQa0UyUnh4WXhoOG1O?=
 =?utf-8?B?YVM2M1VWbndQUUkzM2ZES05saUFTOXRUdjR0R0VLdzUvWThXSHZEUDNibnVF?=
 =?utf-8?B?VnA2VEluZHUrN3JxdWN1UlNhRUZTWlJnMWE3dnpQeTYyaGJ3aVlVbXkzcEtM?=
 =?utf-8?B?djhPaDhPWHVmZ1VCRzJndlJIOGNpTlNUaGQ0b0p2OVN2NmV5UEYrVFQyWFJa?=
 =?utf-8?B?OG5JZkJOcy9pU3FmNnZiUHJnaWdxMDdqZ202RDFOS20wUldTd0hFL3Z3UUhL?=
 =?utf-8?B?MUJQWEl4ZFpjaWJXTFRGNktwK2VCdVI0SHpJeEJ3cU9DSjBCTFpNSnp6TlJZ?=
 =?utf-8?B?dEpNUmdEOEE1aFZ1Qjdvc2Y2Wk5IU3MxNDBuS0k4ZW5EWDB5eGQvMmZ3VCtR?=
 =?utf-8?B?QjhnQWovbUI5QmhmdDhIdkpEZVRDN0Q0MGpoNEFlNTM1UjlDajhQRm4xZ3Nr?=
 =?utf-8?B?U080UElCUUY1dlU2VmpGdG5HVEtWcCt5ekQ5RUdhWG15Wlo3c1VwMGRic0ZE?=
 =?utf-8?B?VEJMR0hoVy9xUVlnZVJVZUVBeGp1SjlCNDExRzJXNExoSHVUdlJhdFZWeCtS?=
 =?utf-8?B?YXFxa1MrekJXcXJ2aGR4dWxBby9oZ0NsamREVlc4Unh5RW9QbTl6b2hNTTFF?=
 =?utf-8?B?bDlsaXVEbkdFQmF6Q3V5WWplcTFLSXZ1M2duVTVqTzkvTDl1eU9xTndqTFRX?=
 =?utf-8?B?K3RnVTNPZVZPdU5zbWp2Y3FpcFJlZlZ4dVhtUmVZb2lmSlVGeXVjYkZLd1Y4?=
 =?utf-8?B?WHN1OStKVFpNTGZZdWxjbE1RZS9DVndnaGhoanZSd1JSY3RoeUxpenpqT0pw?=
 =?utf-8?B?WEVjVUp2ZitwTFdUYmlZRHpqVzRITGpWK0gxQmdhT2NDUEEvYlNEbXdvc2NF?=
 =?utf-8?B?UTg1NXR3azBzYlROZUJuSHA1WFZaTEJxcHBic3czRWxEbGRvQ0dWejJxTXpF?=
 =?utf-8?B?TEF5MzR2Nm5BYTdiSkdYYjE0bEp0RHF5aTZKajRQaHdYT0pMRHg3TWdEdWJJ?=
 =?utf-8?B?M0FqRVFLQXVQUmljNDB6R2lyM3ByQWlYNjZ0T1dTTStKQlM5K2h6aUs4Yk9h?=
 =?utf-8?B?d01GdkhrU2E3amcxWlEydzVWVEd6MWpGcjVaYkJ3cWtqQ2R6QysvWEkrVU5M?=
 =?utf-8?B?OU13bWtrQmVFb3RRa2FyWktvM0VBNis2NHlSSTdUWHFsLzZQa2R1Mzk5dFZw?=
 =?utf-8?B?dHJyYXhlR2FUck9qZHVTS05xb21tSHY1bEZzZUZPVU9EV2JRRG5RcFd5UTJF?=
 =?utf-8?Q?KYyXTV56a8vfnBqpUEu9kdEQjeuoaVAgLCQcEvPz+NTB?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d719336f-4eb6-4cac-5d21-08dbf2aa2ad0
X-MS-Exchange-CrossTenant-AuthSource: VE1PR04MB6478.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Dec 2023 20:14:48.2663
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: K5+dJYt0ZX8MGKNnBdfhtAEyqMMfUXCwW0IdogWOhT8eqK1TJCyiUdgPS1vwamxbcSoJ+odPa3pLWUyz4l54Vg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB7653
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Fri, 01 Dec 2023 12:15:09 -0800 (PST)

On 01.12.23 14:26, Jose Ignacio Tornos Martinez wrote:
Hi,

this is much better.
   
> @@ -1661,14 +1668,19 @@ static int ax88179_reset(struct usbnet *dev)
>   
>   static int ax88179_stop(struct usbnet *dev)
>   {
> +	struct ax88179_data *ax179_data = dev->driver_priv;
>   	u16 tmp16;
>   
> +	ax179_data->stopping_unbinding = 1;

This is problematic. ndo_stop() is not limited to disconnection.
It is also used whenever an interface transitions from up to down.

> +
>   	ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
>   			 2, 2, &tmp16);
>   	tmp16 &= ~AX_MEDIUM_RECEIVE_EN;
>   	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
>   			  2, 2, &tmp16);
>   
> +	ax179_data->stopping_unbinding = 0;
> +
>   	return 0;
>   }
>   

On a general note, you are going for a belt and suspenders approach.
It seems to me that you have two options.

1. Do as Alan suggested and ignore ENODEV. You'd be acknowledging that
these devices are hotpluggable and therefore -ENODEV is not an error
2. Use only a flag. But if you do that, you are setting it in the wrong
place. It should be set in usbnet_disconnect()

O and, well, this is a very mior issue, but you've introduced a memory
ordering issue. You ought to use smp_wmb() after setting the flag and
smp_rmb() before reading it.

	Regards
		Oliver