>From: Ferruh Yigit <ferruh.yi...@intel.com> >Sent: Thursday, May 14, 2020 7:33 AM > >On 5/6/2020 3:43 AM, Rasesh Mody wrote: >> Hi, >> >>> From: Thomas Monjalon <tho...@monjalon.net> >>> Sent: Tuesday, May 05, 2020 2:15 AM >>> >>> 05/05/2020 10:59, Ferruh Yigit: >>>> On 5/5/2020 7:44 AM, Jerin Jacob wrote: >>>>> On Tue, May 5, 2020 at 8:39 AM Rasesh Mody <rm...@marvell.com> >>> wrote: >>>>>> >>>>>> Some applications do not explicitly restore Tx queues setup during >>>>>> port re-configuration. This patch adds changes to check for >>>>>> released Tx queues and restore the setup if application doesn't >>>>>> explicitly does that. >>>>> >>>>> +ethdev maintainers. >>>>> >>>>> I think, Ideally, the fix should be in common code if we need to >>>>> support such applications. >>>> >>>> Is this a case application re-configures to increase the number of >>>> queues but doesn't setup new queues? >>>> If so this looks like application error and application should be >>>> fixed instead of recover this in the ethdev. >>> >>> +1 >>> >> >> This is a case of KNI application performing device re-configuration to >change MTU. The application explicitly calls Rx queue setup, however doesn't >call Tx queue setup. When MTU for KNI interface is changed it runs into a >segfault when trying to start Tx queues. > >Why it crash? Device re-configuration should not be changing the number of >the queues, it is always 1. What is missing/wrong without the Tx queue setup? >
QEDE PMD deallocates all fastpath resources unconditionally, including Tx queue, when re-configuring the device. When reallocating resources PMD relies on application to explicitly setup the Rx/Tx queue. Deallocation of all the resources is only required if the Rx/Tx queue configuration changes. So when KNI MTU change performs device re-configuration it exposes a bug, where there are no Tx queue resources setup. Then while starting the Tx queue in HW from PMD, application crashes. I'll submit a PMD fix for device re-configuration. >> Some other applications make use of rte_eth_dev_set_mtu() ethdev op, >which looks to be cleaner approach. >> > >+1. I don't know history if there is a specific reason this way >+selected, but >after release I think we can try this. Sure. Thanks! -Rasesh
