-----Original Message-----
From: dev <dev-boun...@dpdk.org> On Behalf Of Jeff Guo
Sent: Tuesday, July 7, 2020 10:36
To: Wei Hu (Xavier) <xavier.hu...@huawei.com>; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v6 2/2] ethdev: fix VLAN offloads set if no
relative capabilities
On 7/6/2020 3:06 PM, Wei Hu (Xavier) wrote:
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>
Acked-by: Jeff Guo <jia....@intel.com>
---
v5 -> v6: add the related history patch into the Fixes commit log.
v4 -> v5: no change.
v3 -> v4: Delete "next_mask" label and modify the function that when the
offload is not supported the function fail.
v2 -> v3: Add __rte_unused to avoid unused parameter 'dev' and 'mask'
warning.
v1 -> v2: Cleanup duplicated checks which are done in some PMDs.
---
drivers/net/dpaa2/dpaa2_ethdev.c | 12 +++---------
drivers/net/enic/enic_ethdev.c | 12 ------------
drivers/net/fm10k/fm10k_ethdev.c | 23 ++---------------------
drivers/net/hinic/hinic_pmd_ethdev.c | 6 ------
drivers/net/i40e/i40e_ethdev.c | 5 -----
drivers/net/nfp/nfp_net.c | 5 -----
drivers/net/octeontx/octeontx_ethdev_ops.c | 10 ----------
drivers/net/octeontx2/otx2_vlan.c | 5 -----
drivers/net/qede/qede_ethdev.c | 3 ---
lib/librte_ethdev/rte_ethdev.c | 21 +++++++++++++++++++++
10 files changed, 26 insertions(+), 76 deletions(-)
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c
b/drivers/net/dpaa2/dpaa2_ethdev.c
index a1f1919..489d744 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -145,7 +145,7 @@ dpaa2_vlan_offload_set(struct rte_eth_dev *dev,
int mask)
{
struct dpaa2_dev_priv *priv = dev->data->dev_private;
struct fsl_mc_io *dpni = dev->process_private;
- int ret;
+ int ret = 0;
PMD_INIT_FUNC_TRACE();
@@ -153,7 +153,7 @@ dpaa2_vlan_offload_set(struct rte_eth_dev *dev,
int mask)
/* VLAN Filter not avaialble */
if (!priv->max_vlan_filters) {
DPAA2_PMD_INFO("VLAN filter not available");
- goto next_mask;
+ return -ENOTSUP;
}
if (dev->data->dev_conf.rxmode.offloads & @@ -166,14
+166,8 @@
dpaa2_vlan_offload_set(struct rte_eth_dev *dev, int mask)
if (ret < 0)
DPAA2_PMD_INFO("Unable to set vlan filter = %d",
ret);
}
-next_mask:
- if (mask & ETH_VLAN_EXTEND_MASK) {
- if (dev->data->dev_conf.rxmode.offloads &
- DEV_RX_OFFLOAD_VLAN_EXTEND)
- DPAA2_PMD_INFO("VLAN extend offload not
supported");
- }
- return 0;
+ return ret;
}
static int
diff --git a/drivers/net/enic/enic_ethdev.c
b/drivers/net/enic/enic_ethdev.c index 6a3580f..30a599d 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -367,18 +367,6 @@ static int enicpmd_vlan_offload_set(struct
rte_eth_dev *eth_dev, int mask)
enic->ig_vlan_strip_en = 0;
}
- if ((mask & ETH_VLAN_FILTER_MASK) &&
- (offloads & DEV_RX_OFFLOAD_VLAN_FILTER)) {
- dev_warning(enic,
- "Configuration of VLAN filter is not supported\n");
- }
-
- if ((mask & ETH_VLAN_EXTEND_MASK) &&
- (offloads & DEV_RX_OFFLOAD_VLAN_EXTEND)) {
- dev_warning(enic,
- "Configuration of extended VLAN is not
supported\n");
- }
-
return enic_set_vlan_strip(enic);
}
diff --git a/drivers/net/fm10k/fm10k_ethdev.c
b/drivers/net/fm10k/fm10k_ethdev.c
index cb0dd3b..b574693 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -1562,28 +1562,9 @@ fm10k_vlan_filter_set(struct rte_eth_dev *dev,
uint16_t vlan_id, int on)
}
static int
-fm10k_vlan_offload_set(struct rte_eth_dev *dev, int mask)
+fm10k_vlan_offload_set(struct rte_eth_dev *dev __rte_unused,
+ int mask __rte_unused)
{
- if (mask & ETH_VLAN_STRIP_MASK) {
- if (!(dev->data->dev_conf.rxmode.offloads &
- DEV_RX_OFFLOAD_VLAN_STRIP))
- PMD_INIT_LOG(ERR, "VLAN stripping is "
- "always on in fm10k");
- }
-
- if (mask & ETH_VLAN_EXTEND_MASK) {
- if (dev->data->dev_conf.rxmode.offloads &
- DEV_RX_OFFLOAD_VLAN_EXTEND)
- PMD_INIT_LOG(ERR, "VLAN QinQ is not "
- "supported in fm10k");
- }
-
- if (mask & ETH_VLAN_FILTER_MASK) {
- if (!(dev->data->dev_conf.rxmode.offloads &
- DEV_RX_OFFLOAD_VLAN_FILTER))
- PMD_INIT_LOG(ERR, "VLAN filter is always on in
fm10k");
- }
-
return 0;
}
diff --git a/drivers/net/hinic/hinic_pmd_ethdev.c
b/drivers/net/hinic/hinic_pmd_ethdev.c
index 0d3d62b..67e6afc 100644
--- a/drivers/net/hinic/hinic_pmd_ethdev.c
+++ b/drivers/net/hinic/hinic_pmd_ethdev.c
@@ -1709,12 +1709,6 @@ static int hinic_vlan_offload_set(struct
rte_eth_dev *dev, int mask)
nic_dev->proc_dev_name, dev->data->port_id);
}
- if (mask & ETH_VLAN_EXTEND_MASK) {
- PMD_DRV_LOG(ERR, "Don't support vlan qinq, device: %s,
port_id: %d",
- nic_dev->proc_dev_name, dev->data->port_id);
- return -ENOTSUP;
- }
-
return 0;
}
diff --git a/drivers/net/i40e/i40e_ethdev.c
b/drivers/net/i40e/i40e_ethdev.c index 472ce2a..c89290d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3865,11 +3865,6 @@ i40e_vlan_offload_set(struct rte_eth_dev
*dev, int mask)
struct i40e_vsi *vsi = pf->main_vsi;
struct rte_eth_rxmode *rxmode;
- if (mask & ETH_QINQ_STRIP_MASK) {
- PMD_DRV_LOG(ERR, "Strip qinq is not supported.");
- return -ENOTSUP;
- }
-
rxmode = &dev->data->dev_conf.rxmode;
if (mask & ETH_VLAN_FILTER_MASK) {
if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
diff --git
a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c index
88e3f01..474719b 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -2353,11 +2353,6 @@ nfp_net_vlan_offload_set(struct rte_eth_dev
*dev, int mask)
hw = NFP_NET_DEV_PRIVATE_TO_HW(dev->data->dev_private);
new_ctrl = 0;
- if ((mask & ETH_VLAN_FILTER_OFFLOAD) ||
- (mask & ETH_VLAN_EXTEND_OFFLOAD))
- PMD_DRV_LOG(INFO, "No support for
ETH_VLAN_FILTER_OFFLOAD or"
- " ETH_VLAN_EXTEND_OFFLOAD");
-
/* Enable vlan strip if it is not configured yet */
if ((mask & ETH_VLAN_STRIP_OFFLOAD) &&
!(hw->ctrl & NFP_NET_CFG_CTRL_RXVLAN)) diff --git
a/drivers/net/octeontx/octeontx_ethdev_ops.c
b/drivers/net/octeontx/octeontx_ethdev_ops.c
index ff627a6..dbe13ce 100644
--- a/drivers/net/octeontx/octeontx_ethdev_ops.c
+++ b/drivers/net/octeontx/octeontx_ethdev_ops.c
@@ -43,16 +43,6 @@ octeontx_dev_vlan_offload_set(struct rte_eth_dev
*dev, int mask)
rxmode = &dev->data->dev_conf.rxmode;
- if (mask & ETH_VLAN_EXTEND_MASK) {
- octeontx_log_err("Extend offload not supported");
- return -ENOTSUP;
- }
-
- if (mask & ETH_VLAN_STRIP_MASK) {
- octeontx_log_err("VLAN strip offload not supported");
- return -ENOTSUP;
- }
-
if (mask & ETH_VLAN_FILTER_MASK) {
if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER) {
rc = octeontx_vlan_hw_filter(nic, true); diff --git
a/drivers/net/octeontx2/otx2_vlan.c
b/drivers/net/octeontx2/otx2_vlan.c
index 322a565..7357b06 100644
--- a/drivers/net/octeontx2/otx2_vlan.c
+++ b/drivers/net/octeontx2/otx2_vlan.c
@@ -717,11 +717,6 @@ otx2_nix_vlan_offload_set(struct rte_eth_dev
*eth_dev, int mask)
rxmode = ð_dev->data->dev_conf.rxmode;
- if (mask & ETH_VLAN_EXTEND_MASK) {
- otx2_err("Extend offload not supported");
- return -ENOTSUP;
- }
-
if (mask & ETH_VLAN_STRIP_MASK) {
if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP) {
offloads |= DEV_RX_OFFLOAD_VLAN_STRIP; diff --
git
a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 7f5a34b..f2ed03b 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1031,9 +1031,6 @@ static int qede_vlan_offload_set(struct
rte_eth_dev *eth_dev, int mask)
}
}
- if (mask & ETH_VLAN_EXTEND_MASK)
- DP_ERR(edev, "Extend VLAN not supported\n");
-
qdev->vlan_offload_mask = mask;
DP_INFO(edev, "VLAN offload mask %d\n", mask); diff --git
a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 50c3f18..078bb25 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -3258,12 +3258,14 @@ rte_eth_dev_set_vlan_ether_type(uint16_t
port_id,
int
rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask)
{
+ struct rte_eth_dev_info dev_info;
struct rte_eth_dev *dev;
int ret = 0;
int mask = 0;
int cur, org = 0;
uint64_t orig_offloads;
uint64_t dev_offloads;
+ uint64_t new_offloads;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
dev = &rte_eth_devices[port_id];
@@ -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
+ */
+ 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;
+ }
+
RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -
ENOTSUP);
dev->data->dev_conf.rxmode.offloads = dev_offloads;
ret = (*dev->dev_ops->vlan_offload_set)(dev, mask);
