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.
>
> Fixes: a4996bd89c42 ("ethdev: new Rx/Tx offloads API")
> Fixes: 0ebce6129bc6 ("net/dpaa2: support new ethdev offload APIs")
> Fixes: f9416bbafd98 ("net/enic: remove VLAN filter handler")
> Fixes: 4f7d9e383e5c ("fm10k: update vlan offload features")
> Fixes: fdba3bf15c7b ("net/hinic: add VLAN filter and offload")
> Fixes: b96fb2f0d22b ("net/i40e: handle QinQ strip")
> Fixes: d4a27a3b092a ("nfp: add basic features")
> Fixes: 56139e85abec ("net/octeontx: support VLAN filter offload")
> Fixes: ba1b3b081edf ("net/octeontx2: support VLAN offloads")
> Fixes: d87246a43759 ("net/qede: enable and disable VLAN filtering")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Chengchang Tang <tangchengch...@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.hu...@huawei.com>
> Acked-by: Andrew Rybchenko <arybche...@solarflare.com>
> Acked-by: Hyong Youb Kim <hyon...@cisco.com>
> Acked-by: Sachin Saxena <sachin.sax...@oss.nxp.com>
> Acked-by: Xiaoyun wang <cloud.wangxiao...@huawei.com>
> Acked-by: Harman Kalra <hka...@marvell.com>
Looks like a lot of reviews were already done. I missed this patch.
Please could you make sure API maintainers are Cc'ed?
You can use --cc-cmd devtools/get-maintainer.sh
> @@ -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"?
> + if ((dev_offloads & dev_info.rx_offload_capa) != dev_offloads) {
> + new_offloads = dev_offloads & ~orig_offloads;
> + RTE_ETHDEV_LOG(ERR,
> + "Ethdev port_id=%u requested new added VLAN offloads "
> + "0x%" PRIx64 " must be within Rx offloads capabilities "
> + "0x%" PRIx64 " in %s()\n",
> + port_id, new_offloads, dev_info.rx_offload_capa,
> + __func__);
> + return -EINVAL;
> + }
