Hi From: Andrew Rybchenko > On 10/31/19 7:33 PM, Pavan Nikhilesh Bhagavatula wrote: > >> From: Pavan Nikhilesh Bhagavatula > >>> Hi Matan, > >>> > >>>> Hi Pavan > >>>> > >>>> From: Pavan Nikhilesh <pbhagavat...@marvell.com> > >>>>> Some PMDs cannot work when certain offloads are enable/disabled, > >>>>> as a workaround PMDs auto enable/disable offloads internally and > >>>>> expose it through dev->data->dev_conf.rxmode.offloads. > >>>>> > >>>>> After device specific dev_configure is called compare the > >>>>> requested offloads to the offloads exposed by the PMD and, if the > >>>>> PMD failed to enable a given offload then log it and return > >>>>> -EINVAL from rte_eth_dev_configure, else if the PMD failed to > >>>>> disable a given offload log and continue with rte_eth_dev_configure. > >>>>> > >>>> > >>>> rte_eth_dev_configure can be called more than 1 time in the device > >>>> life time, How can you know what is the minimum offload > >>>> configurations required by the port after the first call? > >>>> Maybe putting it in dev info is better, what do you think? > >>>> > >>> > >>> We only return -EINVAL in the case where we enable an offload > >>> advertised by dev_info and the port still fails to enable it. > >> > >> Are you sure it is ok that devices may disable\enable offloads under > >> the hood without user notification? > > > > Some devices already do it. The above check adds validation for the same. > > The problem is that some offloads cannot be disabled.
Yes, I understand it. > If application does not request Rx checksum offload since it does use it, it > is > not a problem to report it. Yes, for RX checksum I tend to agree that application doesn't care if the PMD will calculate the checksum in spite of the offload is disabled. But what's about other offloads: For example in RX: LRO, CRC_KEEP, VLAN_STRIP, JUMBO If the PMD will stay them on while the app is disabling it, It can cause a problems to the application (affects the packet length). For example in TX: TSO, VLAN, MULTI_SEG..... > Of course, it could be a problem if the offload is > used, but application wants to disable it, for example, for debugging > purposes. > In this case, the solution is to mask offloads on application level, which is > not > ideal as well. Why not ideal? If application can know the limitation of offloads disabling (for example to read capability on it) The application has all information to take decisions. > Anyway, the patch just tries to highlight difference of applied from > requested. So, it is a step forward. > Also, the patch will fail configure if an offload is requested, but not > enabled. > > >> Can't it break applications? > >> Why does the device expose unsupported offloads in dev info? > >> Does it update the running offload usynchronically? Race? > >> Can you explain also your specific use case? > >> > >> > >>>> Matan