* [PATCH] net/i40e: fix read register return status check
@ 2024-11-15 19:14 Vladimir Medvedkin
2024-11-19 11:52 ` Bruce Richardson
0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Medvedkin @ 2024-11-15 19:14 UTC (permalink / raw)
To: dev; +Cc: beilei.xing, stable
'i40e_get_outer_vlan()' does not check 'i40e_aq_debug_read_register()'
return value. This patch fixes this issue.
Coverity issue: 445518
Fixes: 86eb05d6350b ("net/i40e: add flow validate function")
Cc: beilei.xing@intel.com
Cc: stable@dpdk.org
Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
drivers/net/i40e/i40e_flow.c | 78 ++++++++++++++++++++++++++++++------
1 file changed, 66 insertions(+), 12 deletions(-)
diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
index c6857727e8..5015eda461 100644
--- a/drivers/net/i40e/i40e_flow.c
+++ b/drivers/net/i40e/i40e_flow.c
@@ -1263,27 +1263,31 @@ i40e_flow_parse_attr(const struct rte_flow_attr *attr,
return 0;
}
-static uint16_t
-i40e_get_outer_vlan(struct rte_eth_dev *dev)
+static int
+i40e_get_outer_vlan(struct rte_eth_dev *dev, uint16_t *tpid)
{
struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
int qinq = dev->data->dev_conf.rxmode.offloads &
RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
uint64_t reg_r = 0;
uint16_t reg_id;
- uint16_t tpid;
+ int ret;
if (qinq)
reg_id = 2;
else
reg_id = 3;
- i40e_aq_debug_read_register(hw, I40E_GL_SWT_L2TAGCTRL(reg_id),
+ ret = i40e_aq_debug_read_register(hw, I40E_GL_SWT_L2TAGCTRL(reg_id),
®_r, NULL);
+ if (ret != I40E_SUCCESS) {
+ PMD_DRV_LOG(ERR, "Fail to debug read from c[%d]", reg_id);
+ return -EIO;
+ }
- tpid = (reg_r >> I40E_GL_SWT_L2TAGCTRL_ETHERTYPE_SHIFT) & 0xFFFF;
+ *tpid = (reg_r >> I40E_GL_SWT_L2TAGCTRL_ETHERTYPE_SHIFT) & 0xFFFF;
- return tpid;
+ return 0;
}
/* 1. Last in item should be NULL as range is not supported.
@@ -1303,6 +1307,8 @@ i40e_flow_parse_ethertype_pattern(struct rte_eth_dev *dev,
const struct rte_flow_item_eth *eth_spec;
const struct rte_flow_item_eth *eth_mask;
enum rte_flow_item_type item_type;
+ int ret;
+ uint16_t tpid;
for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
if (item->last) {
@@ -1361,8 +1367,24 @@ i40e_flow_parse_ethertype_pattern(struct rte_eth_dev *dev,
if (filter->ether_type == RTE_ETHER_TYPE_IPV4 ||
filter->ether_type == RTE_ETHER_TYPE_IPV6 ||
- filter->ether_type == RTE_ETHER_TYPE_LLDP ||
- filter->ether_type == i40e_get_outer_vlan(dev)) {
+ filter->ether_type == RTE_ETHER_TYPE_LLDP) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item,
+ "Unsupported ether_type in"
+ " control packet filter.");
+ return -rte_errno;
+ }
+
+ ret = i40e_get_outer_vlan(dev, &tpid);
+ if (ret != 0) {
+ rte_flow_error_set(error, EIO,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item,
+ "Can not get the Ethertype identifying the L2 tag");
+ return -rte_errno;
+ }
+ if (filter->ether_type == tpid) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ITEM,
item,
@@ -1370,6 +1392,7 @@ i40e_flow_parse_ethertype_pattern(struct rte_eth_dev *dev,
" control packet filter.");
return -rte_errno;
}
+
break;
default:
break;
@@ -1641,6 +1664,7 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
bool outer_ip = true;
uint8_t field_idx;
int ret;
+ uint16_t tpid;
memset(off_arr, 0, sizeof(off_arr));
memset(len_arr, 0, sizeof(len_arr));
@@ -1709,14 +1733,29 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
ether_type = rte_be_to_cpu_16(eth_spec->hdr.ether_type);
if (ether_type == RTE_ETHER_TYPE_IPV4 ||
- ether_type == RTE_ETHER_TYPE_IPV6 ||
- ether_type == i40e_get_outer_vlan(dev)) {
+ ether_type == RTE_ETHER_TYPE_IPV6) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ITEM,
item,
"Unsupported ether_type.");
return -rte_errno;
}
+ ret = i40e_get_outer_vlan(dev, &tpid);
+ if (ret != 0) {
+ rte_flow_error_set(error, EIO,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item,
+ "Can not get the Ethertype identifying the L2 tag");
+ return -rte_errno;
+ }
+ if (ether_type == tpid) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item,
+ "Unsupported ether_type.");
+ return -rte_errno;
+ }
+
input_set |= I40E_INSET_LAST_ETHER_TYPE;
filter->input.flow.l2_flow.ether_type =
eth_spec->hdr.ether_type;
@@ -1763,14 +1802,29 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
rte_be_to_cpu_16(vlan_spec->hdr.eth_proto);
if (ether_type == RTE_ETHER_TYPE_IPV4 ||
- ether_type == RTE_ETHER_TYPE_IPV6 ||
- ether_type == i40e_get_outer_vlan(dev)) {
+ ether_type == RTE_ETHER_TYPE_IPV6) {
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ITEM,
item,
"Unsupported inner_type.");
return -rte_errno;
}
+ ret = i40e_get_outer_vlan(dev, &tpid);
+ if (ret != 0) {
+ rte_flow_error_set(error, EIO,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item,
+ "Can not get the Ethertype identifying the L2 tag");
+ return -rte_errno;
+ }
+ if (ether_type == tpid) {
+ rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ITEM,
+ item,
+ "Unsupported ether_type.");
+ return -rte_errno;
+ }
+
input_set |= I40E_INSET_LAST_ETHER_TYPE;
filter->input.flow.l2_flow.ether_type =
vlan_spec->hdr.eth_proto;
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] net/i40e: fix read register return status check
2024-11-15 19:14 [PATCH] net/i40e: fix read register return status check Vladimir Medvedkin
@ 2024-11-19 11:52 ` Bruce Richardson
2024-11-19 12:02 ` Bruce Richardson
0 siblings, 1 reply; 3+ messages in thread
From: Bruce Richardson @ 2024-11-19 11:52 UTC (permalink / raw)
To: Vladimir Medvedkin; +Cc: dev, beilei.xing, stable
On Fri, Nov 15, 2024 at 07:14:25PM +0000, Vladimir Medvedkin wrote:
> 'i40e_get_outer_vlan()' does not check 'i40e_aq_debug_read_register()'
> return value. This patch fixes this issue.
I think a little more detail on the scope of the changes could be good
here. It fixes the issue by allowing the function to return error, or zero
on success rather than the value read. This change then causes some rework
in the calling code to handle the error code and to change the form of the
function call.
>
> Coverity issue: 445518
>
> Fixes: 86eb05d6350b ("net/i40e: add flow validate function")
> Cc: beilei.xing@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
Fix below looks good, some minor comments.
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> drivers/net/i40e/i40e_flow.c | 78 ++++++++++++++++++++++++++++++------
> 1 file changed, 66 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_flow.c b/drivers/net/i40e/i40e_flow.c
> index c6857727e8..5015eda461 100644
> --- a/drivers/net/i40e/i40e_flow.c
> +++ b/drivers/net/i40e/i40e_flow.c
> @@ -1263,27 +1263,31 @@ i40e_flow_parse_attr(const struct rte_flow_attr *attr,
> return 0;
> }
>
> -static uint16_t
> -i40e_get_outer_vlan(struct rte_eth_dev *dev)
> +static int
> +i40e_get_outer_vlan(struct rte_eth_dev *dev, uint16_t *tpid)
> {
> struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> int qinq = dev->data->dev_conf.rxmode.offloads &
> RTE_ETH_RX_OFFLOAD_VLAN_EXTEND;
> uint64_t reg_r = 0;
> uint16_t reg_id;
> - uint16_t tpid;
> + int ret;
>
> if (qinq)
> reg_id = 2;
> else
> reg_id = 3;
>
> - i40e_aq_debug_read_register(hw, I40E_GL_SWT_L2TAGCTRL(reg_id),
> + ret = i40e_aq_debug_read_register(hw, I40E_GL_SWT_L2TAGCTRL(reg_id),
> ®_r, NULL);
> + if (ret != I40E_SUCCESS) {
> + PMD_DRV_LOG(ERR, "Fail to debug read from c[%d]", reg_id);
This error message maybe could do with a little clarification. The "c"
prefix doesn't make sense to me.
> + return -EIO;
> + }
>
> - tpid = (reg_r >> I40E_GL_SWT_L2TAGCTRL_ETHERTYPE_SHIFT) & 0xFFFF;
> + *tpid = (reg_r >> I40E_GL_SWT_L2TAGCTRL_ETHERTYPE_SHIFT) & 0xFFFF;
>
> - return tpid;
> + return 0;
> }
>
> /* 1. Last in item should be NULL as range is not supported.
> @@ -1303,6 +1307,8 @@ i40e_flow_parse_ethertype_pattern(struct rte_eth_dev *dev,
> const struct rte_flow_item_eth *eth_spec;
> const struct rte_flow_item_eth *eth_mask;
> enum rte_flow_item_type item_type;
> + int ret;
> + uint16_t tpid;
>
> for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> if (item->last) {
> @@ -1361,8 +1367,24 @@ i40e_flow_parse_ethertype_pattern(struct rte_eth_dev *dev,
>
> if (filter->ether_type == RTE_ETHER_TYPE_IPV4 ||
> filter->ether_type == RTE_ETHER_TYPE_IPV6 ||
> - filter->ether_type == RTE_ETHER_TYPE_LLDP ||
> - filter->ether_type == i40e_get_outer_vlan(dev)) {
> + filter->ether_type == RTE_ETHER_TYPE_LLDP) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Unsupported ether_type in"
> + " control packet filter.");
Best not to split error messages across lines.
> + return -rte_errno;
> + }
> +
> + ret = i40e_get_outer_vlan(dev, &tpid);
> + if (ret != 0) {
> + rte_flow_error_set(error, EIO,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Can not get the Ethertype identifying the L2 tag");
> + return -rte_errno;
> + }
> + if (filter->ether_type == tpid) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM,
> item,
> @@ -1370,6 +1392,7 @@ i40e_flow_parse_ethertype_pattern(struct rte_eth_dev *dev,
> " control packet filter.");
> return -rte_errno;
> }
> +
> break;
> default:
> break;
> @@ -1641,6 +1664,7 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
> bool outer_ip = true;
> uint8_t field_idx;
> int ret;
> + uint16_t tpid;
>
> memset(off_arr, 0, sizeof(off_arr));
> memset(len_arr, 0, sizeof(len_arr));
> @@ -1709,14 +1733,29 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
> ether_type = rte_be_to_cpu_16(eth_spec->hdr.ether_type);
>
> if (ether_type == RTE_ETHER_TYPE_IPV4 ||
> - ether_type == RTE_ETHER_TYPE_IPV6 ||
> - ether_type == i40e_get_outer_vlan(dev)) {
> + ether_type == RTE_ETHER_TYPE_IPV6) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM,
> item,
> "Unsupported ether_type.");
> return -rte_errno;
> }
> + ret = i40e_get_outer_vlan(dev, &tpid);
> + if (ret != 0) {
> + rte_flow_error_set(error, EIO,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Can not get the Ethertype identifying the L2 tag");
> + return -rte_errno;
> + }
> + if (ether_type == tpid) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Unsupported ether_type.");
> + return -rte_errno;
> + }
> +
> input_set |= I40E_INSET_LAST_ETHER_TYPE;
> filter->input.flow.l2_flow.ether_type =
> eth_spec->hdr.ether_type;
> @@ -1763,14 +1802,29 @@ i40e_flow_parse_fdir_pattern(struct rte_eth_dev *dev,
> rte_be_to_cpu_16(vlan_spec->hdr.eth_proto);
>
> if (ether_type == RTE_ETHER_TYPE_IPV4 ||
> - ether_type == RTE_ETHER_TYPE_IPV6 ||
> - ether_type == i40e_get_outer_vlan(dev)) {
> + ether_type == RTE_ETHER_TYPE_IPV6) {
> rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ITEM,
> item,
> "Unsupported inner_type.");
> return -rte_errno;
> }
> + ret = i40e_get_outer_vlan(dev, &tpid);
> + if (ret != 0) {
> + rte_flow_error_set(error, EIO,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Can not get the Ethertype identifying the L2 tag");
> + return -rte_errno;
> + }
> + if (ether_type == tpid) {
> + rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ITEM,
> + item,
> + "Unsupported ether_type.");
> + return -rte_errno;
> + }
> +
> input_set |= I40E_INSET_LAST_ETHER_TYPE;
> filter->input.flow.l2_flow.ether_type =
> vlan_spec->hdr.eth_proto;
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] net/i40e: fix read register return status check
2024-11-19 11:52 ` Bruce Richardson
@ 2024-11-19 12:02 ` Bruce Richardson
0 siblings, 0 replies; 3+ messages in thread
From: Bruce Richardson @ 2024-11-19 12:02 UTC (permalink / raw)
To: Vladimir Medvedkin; +Cc: dev, stable
On Tue, Nov 19, 2024 at 11:52:38AM +0000, Bruce Richardson wrote:
> On Fri, Nov 15, 2024 at 07:14:25PM +0000, Vladimir Medvedkin wrote:
> > 'i40e_get_outer_vlan()' does not check 'i40e_aq_debug_read_register()'
> > return value. This patch fixes this issue.
>
> I think a little more detail on the scope of the changes could be good
> here. It fixes the issue by allowing the function to return error, or zero
> on success rather than the value read. This change then causes some rework
> in the calling code to handle the error code and to change the form of the
> function call.
>
> >
> > Coverity issue: 445518
> >
> > Fixes: 86eb05d6350b ("net/i40e: add flow validate function")
> > Cc: beilei.xing@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
>
> Fix below looks good, some minor comments.
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
Patch applied to dpdk-next-net-intel with commit log additions and the two
minor comments on the code fixed on apply.
Thanks,
/Bruce
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-19 12:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-11-15 19:14 [PATCH] net/i40e: fix read register return status check Vladimir Medvedkin
2024-11-19 11:52 ` Bruce Richardson
2024-11-19 12:02 ` Bruce Richardson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).