net/e1000: fix incorrect return value if filter type not supported.

PATCH In review [email protected] 6 participants 14 messages Dec 8, 2020 – Apr 20, 2021 Network drivers

lore

Share & tools

lore

[email protected]

author

2020-12-08 08:10 UTC #0 lore raw

PATCH 1/1 net/e1000: fix incorrect return value if filter type not supported.

From: banxiaozhen <[email protected]>

Signed-off-by banxiaozhen <[email protected]>

---
 drivers/net/e1000/igb_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/e1000/igb_ethdev.c +1 -0

index 647aa8d99..8695426c2 100644
@@ -4561,6 +4561,7 @@ eth_igb_filter_ctrl(struct rte_eth_dev *dev __rte_unused,
 	default:
 		PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
 							filter_type);
+		ret = -EINVAL;
 		break;
 	}

Signature · 1 line

2.24.3 (Apple Git-128)

haz ben

2020-12-08 09:58 UTC #2 lore raw

my name is ban, xiaozhen.
the commit message is "net/e1000: fix incorrect return value if filter type not supported." does additional details should be add?

Quoted 40 lines · Wang, Haiyue <[email protected]> 于2020年12月8日周二下午5:04写道：

Quoted 7 lines
-----Original Message-----
From: banxiaozhen [email protected] <banxiaozhen [email protected]
Sent: Tuesday, December 8, 2020 16:10
To: Guo, Jia <[email protected]>; Wang, Haiyue <[email protected]>
Cc: [email protected]; banxiaozhen <[email protected]>
Subject: [PATCH] net/e1000: fix incorrect return value if filter type
not supported.
Quoted 4 lines
From: banxiaozhen <[email protected]>
Signed-off-by banxiaozhen <[email protected]>
The patch looks good.
The name format should be like "Banxiao Zhen", and the commit message is empty ?
Review
Quoted 5 lines
---
 drivers/net/e1000/igb_ethdev.c | 1 +
 1 file changed, 1 insertion(+)
(quoted diff) +0 -0
b/drivers/net/e1000/igb_ethdev.c
drivers/net/e1000/igb_ethdev.c eth_igb_filter_ctrl(struct rte_eth_dev *dev
Quoted 4 lines
index 647aa8d99..8695426c2 100644
drivers/net/e1000/igb_ethdev.c +0 -0
@@ -4561,6 +4561,7 @@ eth_igb_filter_ctrl(struct rte_eth_dev *dev
__rte_unused,
Quoted 9 lines
drivers/net/e1000/igb_ethdev.c +1 -1
      default:
              PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
                                                      filter_type);
+             ret = -EINVAL;
              break;
      }
 
--
2.24.3 (Apple Git-128)

From: haz ben <[email protected]>
Sent: Tuesday, December 8, 2020 17:58
To: Wang, Haiyue <[email protected]>
Cc: ban, xiaozhen <[email protected]>; Guo, Jia <[email protected]>; [email protected]
Subject: Re: [PATCH] net/e1000: fix incorrect return value if filter type not supported.

my name is ban, xiaozhen.

OK, ;-)

the commit message is "net/e1000: fix incorrect return value if filter type not supported." does additional details should be add?

It is not about additional, it is better for title + more descriptive message.

Quoted 10 lines · Wang, Haiyue <[email protected]<mailto:[email protected]>>…

-----Original Message-----
From: banxiaozhen [email protected]<mailto:[email protected]> <banxiaozhen [email protected]<mailto:[email protected]>>
Sent: Tuesday, December 8, 2020 16:10
To: Guo, Jia <[email protected]<mailto:[email protected]>>; Wang, Haiyue <[email protected]<mailto:[email protected]>>
Cc: [email protected]<mailto:[email protected]>; banxiaozhen <[email protected]<mailto:[email protected]>>
Subject: [PATCH] net/e1000: fix incorrect return value if filter type not supported.
From: banxiaozhen <[email protected]<mailto:[email protected]>>
Signed-off-by banxiaozhen <[email protected]<mailto:[email protected]>>

The patch looks good.

The name format should be like "Banxiao Zhen", and the commit message is empty ?

Quoted 18 lines

---
 drivers/net/e1000/igb_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/e1000/igb_ethdev.c +1 -1

index 647aa8d99..8695426c2 100644
@@ -4561,6 +4561,7 @@ eth_igb_filter_ctrl(struct rte_eth_dev *dev __rte_unused,
      default:
              PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
                                                      filter_type);
+             ret = -EINVAL;
              break;
      }
 
--

2.24.3 (Apple Git-128)

Wang, Haiyue

2020-12-08 11:05 UTC #4 lore raw

In reply to haz ben

my name is ban, xiaozhen.
the commit message is "net/e1000: fix incorrect return value if filter type not supported." does additional details should be add?

http://doc.dpdk.org/guides/contributing/patches.html#commit-messages-subject-line

1.
./devtools/checkpatches.sh -n1

### net/e1000: fix incorrect return value if filter type not supported.

WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 7 lines checked

0/1 valid patch

./devtools/check-git-log.sh -1
Wrong headline format:
net/e1000: fix incorrect return value if filter type not supported.
Headline too long:
net/e1000: fix incorrect return value if filter type not supported.
Missing 'Fixes' tag:
net/e1000: fix incorrect return value if filter type not supported.

Invalid patch(es) found - checked 1 patch

3. Please configure your mail system with plain text, not in rich text.

Quoted 10 lines · Wang, Haiyue <[email protected]<mailto:[email protected]>>…

-----Original Message-----
From: banxiaozhen [email protected]<mailto:[email protected]> <banxiaozhen [email protected]<mailto:[email protected]>>
Sent: Tuesday, December 8, 2020 16:10
To: Guo, Jia <[email protected]<mailto:[email protected]>>; Wang, Haiyue <[email protected]<mailto:[email protected]>>
Cc: [email protected]<mailto:[email protected]>; banxiaozhen <[email protected]<mailto:[email protected]>>
Subject: [PATCH] net/e1000: fix incorrect return value if filter type not supported.
From: banxiaozhen <[email protected]<mailto:[email protected]>>
Signed-off-by banxiaozhen <[email protected]<mailto:[email protected]>>

The patch looks good.

The name format should be like "Banxiao Zhen", and the commit message is empty ?

Quoted 18 lines

---
 drivers/net/e1000/igb_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/e1000/igb_ethdev.c +1 -1

index 647aa8d99..8695426c2 100644
@@ -4561,6 +4561,7 @@ eth_igb_filter_ctrl(struct rte_eth_dev *dev __rte_unused,
      default:
              PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
                                                      filter_type);
+             ret = -EINVAL;
              break;
      }
 
--

2.24.3 (Apple Git-128)

Xiaozhen Ban

2021-03-17 09:15 UTC #5 lore raw

PATCH 1/1 net/e1000: fix filter control return value

In reply to [email protected]

If filter_type not supported, eth_igb_filter_ctrl() will return incorrect value.

Fixes dd5c2dd7e5e5 ("igb: use generic filter control for ethertype filter")

Cc [email protected]

Signed-off-by Xiaozhen Ban <[email protected]>

---
 drivers/net/e1000/igb_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/e1000/igb_ethdev.c +1 -0

index 1716d6b90..325461247 100644
@@ -4599,6 +4599,7 @@ eth_igb_filter_ctrl(struct rte_eth_dev *dev __rte_unused,
 	default:
 		PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
 							filter_type);
+		ret = -EINVAL;
 		break;
 	}

Signature · 1 line

2.25.1

Wang, Haiyue

2021-03-17 11:23 UTC #6 lore raw

Re: [dpdk-dev] [PATCH] net/e1000: fix filter control return value

Quoted 22 lines

-----Original Message-----
From: Xiaozhen Ban <[email protected]>
Sent: Wednesday, March 17, 2021 17:16
To: Guo, Jia <[email protected]>; Wang, Haiyue <[email protected]>
Cc: [email protected]; [email protected]
Subject: [PATCH] net/e1000: fix filter control return value
If filter_type not supported, eth_igb_filter_ctrl() will return incorrect value.
Fixes dd5c2dd7e5e5 ("igb: use generic filter control for ethertype filter")
Cc [email protected]
Signed-off-by Xiaozhen Ban <[email protected]>
---
 drivers/net/e1000/igb_ethdev.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/e1000/igb_ethdev.c +0 -0
index 1716d6b90..325461247 100644

drivers/net/e1000/igb_ethdev.c eth_igb_filter_ctrl(struct rte_eth_dev *dev __rte_unused, · line 4605

Quoted 7 lines

drivers/net/e1000/igb_ethdev.c +1 -0

@@ -4599,6 +4599,7 @@ eth_igb_filter_ctrl(struct rte_eth_dev *dev __rte_unused,
 	default:
 		PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
 							filter_type);
+		ret = -EINVAL;
 		break;
 	}

New design will be applied:

https://patchwork.dpdk.org/project/dpdk/patch/[email protected]/

Quoted 2 lines

--
2.25.1

Xiaozhen Ban

2021-03-18 11:44 UTC #7 lore raw

Re: [dpdk-dev] [PATCH] net/e1000: fix filter control return value

OK, but I think this bug affects all stable release about 6 years before today.

Wang, Haiyue

2021-03-18 15:28 UTC #8 lore raw

Re: [dpdk-dev] [PATCH] net/e1000: fix filter control return value

Quoted 8 lines

-----Original Message-----
From: Xiaozhen Ban <[email protected]>
Sent: Thursday, March 18, 2021 19:44
To: Guo, Jia <[email protected]>; Wang, Haiyue <[email protected]>
Cc: [email protected]; [email protected]
Subject: RE: RE: [PATCH] net/e1000: fix filter control return value
OK, but I think this bug affects all stable release about 6 years before today.

I don't think so, since it is PMD internal ops, the real API 'rte_flow_ops_get' always use RTE_ETH_FILTER_GENERIC. ;-)

Xiaozhen Ban

2021-03-19 04:08 UTC #9 lore raw

Re: [dpdk-dev] [PATCH] net/e1000: fix filter control return value

Quoted 11 lines

Quoted 8 lines
-----Original Message-----
From: Xiaozhen Ban <benhaz1024 at gmail.com>
Sent: Thursday, March 18, 2021 19:44
To: Guo, Jia <jia.guo at intel.com>; Wang, Haiyue <haiyue.wang at intel.com>
Cc: dev at dpdk.org; stable at dpdk.org
Subject: RE: RE: [PATCH] net/e1000: fix filter control return value
OK, but I think this bug affects all stable release about 6 years before today.
I don't think so, since it is PMD internal ops, the real API 'rte_flow_ops_get' always use RTE_ETH_FILTER_GENERIC. ;-)

eth_igb_filter_ctrl() called by rte_eth_dev_filter_ctrl(), if application developer know how to use the parameter filter_type correctly, it's OK. otherwise he will get rte_eth_dev_filter_ctrl() return value is success but actually is failed.

Zhang, Qi Z

Acked-by Applied

2021-03-25 08:46 UTC #10 lore raw

Re: [dpdk-dev] [PATCH] net/e1000: fix filter control return value

Quoted 19 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Xiaozhen Ban
Sent: Friday, March 19, 2021 12:08 PM
To: Guo, Jia <[email protected]>; Wang, Haiyue <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] net/e1000: fix filter control return value
Quoted 9 lines
Quoted 9 lines
-----Original Message-----
From: Xiaozhen Ban <benhaz1024 at gmail.com>
Sent: Thursday, March 18, 2021 19:44
To: Guo, Jia <jia.guo at intel.com>; Wang, Haiyue <haiyue.wang at intel.com>
Cc: dev at dpdk.org; stable at dpdk.org
Subject: RE: RE: [PATCH] net/e1000: fix filter control return value
OK, but I think this bug affects all stable release about 6 years before today.

Quoted 7 lines

Quoted 2 lines
I don't think so, since it is PMD internal ops, the real API 'rte_flow_ops_get' always use RTE_ETH_FILTER_GENERIC. ;-)
eth_igb_filter_ctrl() called by rte_eth_dev_filter_ctrl(), if application developer know how to use the parameter filter_type correctly, it's OK. otherwise he will get rte_eth_dev_filter_ctrl() return value is success but actually is failed.

After check with Haiyue, we think the patch is still needed for LTS <= 19.11, so

Acked-by Qi Zhang <[email protected]>

Applied to dpdk-next-net-intel.

Thanks
Qi

Ferruh Yigit

2021-04-20 08:09 UTC #11 lore raw

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/e1000: fix filter control return value

Quoted 27 lines · Qi Z

Quoted 19 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Xiaozhen Ban
Sent: Friday, March 19, 2021 12:08 PM
To: Guo, Jia <[email protected]>; Wang, Haiyue <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] net/e1000: fix filter control return value
Quoted 9 lines
Quoted 9 lines
-----Original Message-----
From: Xiaozhen Ban <benhaz1024 at gmail.com>
Sent: Thursday, March 18, 2021 19:44
To: Guo, Jia <jia.guo at intel.com>; Wang, Haiyue <haiyue.wang at intel.com>
Cc: dev at dpdk.org; stable at dpdk.org
Subject: RE: RE: [PATCH] net/e1000: fix filter control return value
OK, but I think this bug affects all stable release about 6 years before today.
Quoted 7 lines
Quoted 2 lines
I don't think so, since it is PMD internal ops, the real API 'rte_flow_ops_get' always use RTE_ETH_FILTER_GENERIC. ;-)
eth_igb_filter_ctrl() called by rte_eth_dev_filter_ctrl(), if application developer know how to use the parameter filter_type correctly, it's OK. otherwise he will get rte_eth_dev_filter_ctrl() return value is success but actually is failed.

Quoted 6 lines

After check with Haiyue, we think the patch is still needed for LTS <= 19.11, so
Acked-by Qi Zhang <[email protected]>
Applied to dpdk-next-net-intel.

What is the status of this patch?
It seems it did not applied, and the status in the patchwork is "Superseded".

Zhang, Qi Z

2021-04-20 09:54 UTC #12 lore raw

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/e1000: fix filter control return value

Quoted 48 lines

-----Original Message-----
From: Yigit, Ferruh <[email protected]>
Sent: Tuesday, April 20, 2021 4:09 PM
To: Zhang, Qi Z <[email protected]>; Xiaozhen Ban <[email protected]>; Guo, Jia <[email protected]>; Wang, Haiyue <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] net/e1000: fix filter control return value
Quoted 19 lines · Qi Z
Quoted 19 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Xiaozhen Ban
Sent: Friday, March 19, 2021 12:08 PM
To: Guo, Jia <[email protected]>; Wang, Haiyue <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] net/e1000: fix filter control return value
Quoted 8 lines
Quoted 8 lines
-----Original Message-----
From: Xiaozhen Ban <benhaz1024 at gmail.com>
Sent: Thursday, March 18, 2021 19:44
To: Guo, Jia <jia.guo at intel.com>; Wang, Haiyue <haiyue.wang at intel.com>
Cc: dev at dpdk.org; stable at dpdk.org
Subject: RE: RE: [PATCH] net/e1000: fix filter control return value
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
OK, but I think this bug affects all stable release about 6 years before
today.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
I don't think so, since it is PMD internal ops, the real API
'rte_flow_ops_get'
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
always use RTE_ETH_FILTER_GENERIC. ;-)
eth_igb_filter_ctrl() called by rte_eth_dev_filter_ctrl(), if application developer know how to use the parameter filter_type correctly,
it's OK.
Quoted 6 lines
Quoted 2 lines
otherwise he will get rte_eth_dev_filter_ctrl() return value is success but actually is failed.
After check with Haiyue, we think the patch is still needed for LTS <= 19.11, so

Quoted 7 lines

Quoted 4 lines
Acked-by Qi Zhang <[email protected]>
Applied to dpdk-next-net-intel.
What is the status of this patch?
It seems it did not applied, and the status in the patchwork is "Superseded".

The patch is dropped during rebase, as the function eth_igb_filter_ctrl is removed at below code refectory
http://patchwork.dpdk.org/project/dpdk/patch/[email protected]/

this is a mistake as we still need this to be captured in LTS
unfortunately, its too late to capture it as the refectory code is already applied to main tree.

@Luca Boccassi
Could you capture this patch as an LTS only one?

Thanks
Qi

Luca Boccassi

2021-04-20 14:34 UTC #13 lore raw

Re: [dpdk-dev] [dpdk-stable] [PATCH] net/e1000: fix filter control return value

Quoted 55 lines · Qi Z

Quoted 44 lines
-----Original Message-----
From: Yigit, Ferruh <[email protected]>
Sent: Tuesday, April 20, 2021 4:09 PM
To: Zhang, Qi Z <[email protected]>; Xiaozhen Ban <[email protected]>; Guo, Jia <[email protected]>; Wang, Haiyue <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] net/e1000: fix filter control return value
Quoted 18 lines · Qi Z
Quoted 18 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Xiaozhen Ban
Sent: Friday, March 19, 2021 12:08 PM
To: Guo, Jia <[email protected]>; Wang, Haiyue <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [dpdk-dev] [PATCH] net/e1000: fix filter control return value
Quoted 8 lines
Quoted 8 lines
-----Original Message-----
From: Xiaozhen Ban <benhaz1024 at gmail.com>
Sent: Thursday, March 18, 2021 19:44
To: Guo, Jia <jia.guo at intel.com>; Wang, Haiyue <haiyue.wang at intel.com>
Cc: dev at dpdk.org; stable at dpdk.org
Subject: RE: RE: [PATCH] net/e1000: fix filter control return value
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
OK, but I think this bug affects all stable release about 6 years before
today.
Quoted 1 line
Quoted 1 line
Quoted 1 line
I don't think so, since it is PMD internal ops, the real API
'rte_flow_ops_get'
Quoted 4 lines
Quoted 4 lines
Quoted 1 line
always use RTE_ETH_FILTER_GENERIC. ;-)
eth_igb_filter_ctrl() called by rte_eth_dev_filter_ctrl(), if application developer know how to use the parameter filter_type correctly,
it's OK.
Quoted 6 lines
Quoted 2 lines
otherwise he will get rte_eth_dev_filter_ctrl() return value is success but actually is failed.
After check with Haiyue, we think the patch is still needed for LTS <= 19.11, so
Quoted 7 lines
Quoted 4 lines
Acked-by Qi Zhang <[email protected]>
Applied to dpdk-next-net-intel.
What is the status of this patch?
It seems it did not applied, and the status in the patchwork is "Superseded".
The patch is dropped during rebase, as the function eth_igb_filter_ctrl is removed at below code refectory
http://patchwork.dpdk.org/project/dpdk/patch/[email protected]/

Quoted 8 lines

this is a mistake as we still need this to be captured in LTS
unfortunately, its too late to capture it as the refectory code is already applied to main tree.
@Luca Boccassi
Could you capture this patch as an LTS only one?
Thanks
Qi

Please rebase it on 20.11 and test it, and if it works send it to [email protected] with --subject-prefix='PATCH 20.11'

Signature · 2 lines

Kind regards,
Luca Boccassi