On 3/8/2024 5:25 PM, Hardik Gajjar wrote:
On Thu, Mar 07, 2024 at 11:12:07PM +0530, Krishna Kurapati PSSNV wrote:
[...]
I believe using gether_cleanup altogether may not be an optimal solution. The creation and deletion of network interfaces should align with the behavior of each specific network driver. For instance, in the case of NCM, the usb0 interface is created upon the creation of a directory in config/usb_gadget/gX/functions/ncm.usb0, and it is removed when the corresponding directory is deleted. This follows a standard flow observed in many network drivers, where interfaces are created during driver loading/probing and deleted during removal.
Hi Hardik, Yeah. I observed this behavior.
Typically, deleting the gadget on every disconnection is not considered a good practice, as it can negatively impact the user experience when accessing the gadget.
Got it. I was suspecting issues might come up during fast Plug-In/ Plug-Out cases as setting and cleanup of network interface might take some time.
In our specific scenario, retaining the usb0 network interface has proven to enhance performance and stabilize connections. Previous attempts to remove it resulted in an observed increase in time of 300ms, particularly at the start of Apple CarPlay sessions.
Thanks for this info. Makes sense to keep it in free_inst only. But my initial doubt only stemmed from the fact that actions taken during bind must be reversed during unbind.
Furthermore, it's important to highlight that in Qualcomm products and msm kernels, the inclusion of gether_cleanup in the unbind process was eventually reverted. While the specific reason for reverting the patch is unknown, it suggests that the addition may not have yielded the intended or required results Following is the revert patch details in msm-5.4 kernel, if you want check it. Revert "usb: gadget: f_ncm: allocate/free net device upon driver bind/unbind" This reverts commit 006d8adf555a8c6d34113f564ade312d68abd3b3. Move back the allocation of netdevice to alloc_inst(), one-time registration to bind(), deregistration and free to rm_inst(). The UI update issue will be taken up with proper stakeholders. Change-Id: I56448b08f6796a43ec5b0dfe0dd2d42cdc0eec14
Agree. This was merged first in 4.19 downstream (most probably for car play use case only) and then reverted in 5.4. But present 4.19 code in QC still keeps it in unbind path. The only reason I suspect it was reverted was to get back on to same page with upstream driver (atleast starting from 5.10, this was the reasoning to remove gether_cleanup in unbind path and put it back in free_inst).
Thanks, Krishna,
