08/07/2020 05:37, Wei Hu (Xavier): > On 2020/7/7 22:11, Thomas Monjalon wrote: > > 06/07/2020 09:06, Wei Hu (Xavier): > >> Currently, there is a potential problem that calling the API function > >> rte_eth_dev_set_vlan_offload to start VLAN hardware offloads which the > >> driver does not support. If the PMD driver does not support certain VLAN > >> hardware offloads and does not check for it, the hardware setting will > >> not change, but the VLAN offloads in dev->data->dev_conf.rxmode.offloads > >> will be turned on. > >> > >> It is supposed to check the hardware capabilities to decide whether the > >> relative callback needs to be called just like the behavior in the API > >> function named rte_eth_dev_configure. And it is also needed to cleanup > >> duplicated checks which are done in some PMDs. Also, note that it is > >> behaviour change for some PMDs which simply ignore (with error/warning log > >> message) unsupported VLAN offloads, but now it will fail. [...] > >> @@ -3317,6 +3319,25 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, int > >> offload_mask) > >> if (mask == 0) > >> return ret; > >> > >> + ret = rte_eth_dev_info_get(port_id, &dev_info); > >> + if (ret != 0) > >> + return ret; > >> + > >> + /* > >> + * New added Rx VLAN offloading which are not enabled in > >> + * rte_eth_dev_configure() must be within its device capabilities > >> + */ > > What means "New added Rx VLAN offloading"? > > The parameter offload_mask of rte_eth_dev_set_vlan_offload() function > includes some Rx VLAN offload, and some of them maybe are not enabled > in rte_eth_dev_configure().
OK I don't understand why checking only new features. All enabled features must be within capabilities, right?
