This series converts the filter code in the qede driver
to use NL_SET_ERR_MSG_*(extack, ...) for error handling.
Patch 1-12 converts qede_parse_flow_attr() to use extack,
along with all it's static helper functions.
qede_parse_flow_attr() is used in two places:
- qede_add_tc_flower_fltr()
- qede_flow_spec_to_rule()
In the latter call site extack is faked in the same way as
is done in mlxsw (patch 12).
While the conversion is going on, some error messages are silenced
in between patch 1-12. If wanted could squash patch 1-12 in a v3, but
I felt that it would be easier to review as 12 more trivial patches.
Patch 13 and 14, finishes up by converting qede_parse_actions(),
and ensures that extack is propagated to it, in both call contexts.
---
Changelog:
v2:
- Reworked to always add extack as last argument. (Requested by Przemek)
v1: https://lore.kernel.org/netdev/[email protected]/
Asbjørn Sloth Tønnesen (14):
net: qede: use extack in qede_flow_parse_ports()
net: qede: use extack in qede_set_v6_tuple_to_profile()
net: qede: use extack in qede_set_v4_tuple_to_profile()
net: qede: use extack in qede_flow_parse_v6_common()
net: qede: use extack in qede_flow_parse_v4_common()
net: qede: use extack in qede_flow_parse_tcp_v6()
net: qede: use extack in qede_flow_parse_tcp_v4()
net: qede: use extack in qede_flow_parse_udp_v6()
net: qede: use extack in qede_flow_parse_udp_v4()
net: qede: add extack in qede_add_tc_flower_fltr()
net: qede: use extack in qede_parse_flow_attr()
net: qede: use faked extack in qede_flow_spec_to_rule()
net: qede: propagate extack through qede_flow_spec_validate()
net: qede: use extack in qede_parse_actions()
.../net/ethernet/qlogic/qede/qede_filter.c | 114 ++++++++++--------
1 file changed, 63 insertions(+), 51 deletions(-)
--
2.43.0
Since qede_parse_flow_attr() now does error reporting
through extack, then give it a fake extack and extract the
error message afterwards if one was set.
The extracted error message is then passed on through
DP_NOTICE(), including messages that was earlier issued
with DP_INFO().
This fake extack approach is already used by
mlxsw_env_linecard_modules_power_mode_apply() in
drivers/net/ethernet/mellanox/mlxsw/core_env.c
Only compile tested.
Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
---
Note:
Even through _msg is marked in include/linux/netlink.h as
"don't access directly, use NL_SET_ERR_MSG", then the comment
above NL_SET_ERR_MSG, seams to indicate that it should be fine
to access it directly if for reading, as is done other places.
I could also add a NL_GET_ERR_MSG but I would rather not do that
in this series.
---
drivers/net/ethernet/qlogic/qede/qede_filter.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index 8c1c15b73125..b83432744a03 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -1990,6 +1990,7 @@ static int qede_flow_spec_to_rule(struct qede_dev *edev,
{
struct ethtool_rx_flow_spec_input input = {};
struct ethtool_rx_flow_rule *flow;
+ struct netlink_ext_ack extack;
__be16 proto;
int err;
@@ -2017,7 +2018,7 @@ static int qede_flow_spec_to_rule(struct qede_dev *edev,
if (IS_ERR(flow))
return PTR_ERR(flow);
- err = qede_parse_flow_attr(proto, flow->rule, t, NULL);
+ err = qede_parse_flow_attr(proto, flow->rule, t, &extack);
if (err)
goto err_out;
@@ -2025,6 +2026,8 @@ static int qede_flow_spec_to_rule(struct qede_dev *edev,
err = qede_flow_spec_validate(edev, &flow->rule->action, t,
fs->location);
err_out:
+ if (extack._msg)
+ DP_NOTICE(edev, "%s\n", extack._msg);
ethtool_rx_flow_rule_destroy(flow);
return err;
--
2.43.0
Define extack locally, to reduce line lengths and aid future users.
Only compile tested.
Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
---
drivers/net/ethernet/qlogic/qede/qede_filter.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index 69dbd615b653..09ffcb86924b 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -1876,6 +1876,7 @@ qede_parse_flow_attr(struct qede_dev *edev, __be16 proto,
int qede_add_tc_flower_fltr(struct qede_dev *edev, __be16 proto,
struct flow_cls_offload *f)
{
+ struct netlink_ext_ack *extack = f->common.extack;
struct qede_arfs_fltr_node *n;
struct qede_arfs_tuple t;
int min_hlen, rc;
@@ -1903,7 +1904,7 @@ int qede_add_tc_flower_fltr(struct qede_dev *edev, __be16 proto,
}
/* parse tc actions and get the vf_id */
- rc = qede_parse_actions(edev, &f->rule->action, f->common.extack);
+ rc = qede_parse_actions(edev, &f->rule->action, extack);
if (rc)
goto unlock;
--
2.43.0
Convert qede_flow_parse_tcp_v6() to take extack,
and drop the edev argument.
Pass extack in call to qede_flow_parse_v6_common().
In call to qede_flow_parse_tcp_v6(), use NULL as extack
for now, until a subsequent patch makes extack available.
Only compile tested.
Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
---
drivers/net/ethernet/qlogic/qede/qede_filter.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index 28e4d54dbca1..ddf1d6b0fc83 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -1786,13 +1786,13 @@ qede_flow_parse_v4_common(struct flow_rule *rule,
}
static int
-qede_flow_parse_tcp_v6(struct qede_dev *edev, struct flow_rule *rule,
- struct qede_arfs_tuple *tuple)
+qede_flow_parse_tcp_v6(struct flow_rule *rule, struct qede_arfs_tuple *tuple,
+ struct netlink_ext_ack *extack)
{
tuple->ip_proto = IPPROTO_TCP;
tuple->eth_proto = htons(ETH_P_IPV6);
- return qede_flow_parse_v6_common(rule, tuple, NULL);
+ return qede_flow_parse_v6_common(rule, tuple, extack);
}
static int
@@ -1862,7 +1862,7 @@ qede_parse_flow_attr(struct qede_dev *edev, __be16 proto,
if (ip_proto == IPPROTO_TCP && proto == htons(ETH_P_IP))
rc = qede_flow_parse_tcp_v4(edev, rule, tuple);
else if (ip_proto == IPPROTO_TCP && proto == htons(ETH_P_IPV6))
- rc = qede_flow_parse_tcp_v6(edev, rule, tuple);
+ rc = qede_flow_parse_tcp_v6(rule, tuple, NULL);
else if (ip_proto == IPPROTO_UDP && proto == htons(ETH_P_IP))
rc = qede_flow_parse_udp_v4(edev, rule, tuple);
else if (ip_proto == IPPROTO_UDP && proto == htons(ETH_P_IPV6))
--
2.43.0
Convert qede_flow_parse_v6_common() to take extack,
and drop the edev argument.
Convert DP_NOTICE call to use NL_SET_ERR_MSG_MOD instead.
Pass extack in calls to qede_flow_parse_ports() and
qede_set_v6_tuple_to_profile().
In calls to qede_flow_parse_v6_common(), use NULL as extack
for now, until a subsequent patch makes extack available.
Only compile tested.
Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
---
drivers/net/ethernet/qlogic/qede/qede_filter.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index 6f4c4a5d6c77..f5f8bbe8a64c 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -1722,8 +1722,9 @@ qede_flow_parse_ports(struct flow_rule *rule, struct qede_arfs_tuple *t,
}
static int
-qede_flow_parse_v6_common(struct qede_dev *edev, struct flow_rule *rule,
- struct qede_arfs_tuple *t)
+qede_flow_parse_v6_common(struct flow_rule *rule,
+ struct qede_arfs_tuple *t,
+ struct netlink_ext_ack *extack)
{
struct in6_addr zero_addr, addr;
int err;
@@ -1739,8 +1740,8 @@ qede_flow_parse_v6_common(struct qede_dev *edev, struct flow_rule *rule,
memcmp(&match.mask->src, &addr, sizeof(addr))) ||
(memcmp(&match.key->dst, &zero_addr, sizeof(addr)) &&
memcmp(&match.mask->dst, &addr, sizeof(addr)))) {
- DP_NOTICE(edev,
- "Do not support IPv6 address prefix/mask\n");
+ NL_SET_ERR_MSG_MOD(extack,
+ "Do not support IPv6 address prefix/mask");
return -EINVAL;
}
@@ -1748,11 +1749,11 @@ qede_flow_parse_v6_common(struct qede_dev *edev, struct flow_rule *rule,
memcpy(&t->dst_ipv6, &match.key->dst, sizeof(addr));
}
- err = qede_flow_parse_ports(rule, t, NULL);
+ err = qede_flow_parse_ports(rule, t, extack);
if (err)
return err;
- return qede_set_v6_tuple_to_profile(t, &zero_addr, NULL);
+ return qede_set_v6_tuple_to_profile(t, &zero_addr, extack);
}
static int
@@ -1789,7 +1790,7 @@ qede_flow_parse_tcp_v6(struct qede_dev *edev, struct flow_rule *rule,
tuple->ip_proto = IPPROTO_TCP;
tuple->eth_proto = htons(ETH_P_IPV6);
- return qede_flow_parse_v6_common(edev, rule, tuple);
+ return qede_flow_parse_v6_common(rule, tuple, NULL);
}
static int
@@ -1809,7 +1810,7 @@ qede_flow_parse_udp_v6(struct qede_dev *edev, struct flow_rule *rule,
tuple->ip_proto = IPPROTO_UDP;
tuple->eth_proto = htons(ETH_P_IPV6);
- return qede_flow_parse_v6_common(edev, rule, tuple);
+ return qede_flow_parse_v6_common(rule, tuple, NULL);
}
static int
--
2.43.0
Convert qede_flow_parse_v4_common() to take extack,
and drop the edev argument.
Convert DP_NOTICE call to use NL_SET_ERR_MSG_MOD instead.
Pass extack in calls to qede_flow_parse_ports() and
qede_set_v4_tuple_to_profile().
In calls to qede_flow_parse_v4_common(), use NULL as extack
for now, until a subsequent patch makes extack available.
Only compile tested.
Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
---
drivers/net/ethernet/qlogic/qede/qede_filter.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c
index f5f8bbe8a64c..28e4d54dbca1 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_filter.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c
@@ -1757,8 +1757,9 @@ qede_flow_parse_v6_common(struct flow_rule *rule,
}
static int
-qede_flow_parse_v4_common(struct qede_dev *edev, struct flow_rule *rule,
- struct qede_arfs_tuple *t)
+qede_flow_parse_v4_common(struct flow_rule *rule,
+ struct qede_arfs_tuple *t,
+ struct netlink_ext_ack *extack)
{
int err;
@@ -1768,7 +1769,8 @@ qede_flow_parse_v4_common(struct qede_dev *edev, struct flow_rule *rule,
flow_rule_match_ipv4_addrs(rule, &match);
if ((match.key->src && match.mask->src != htonl(U32_MAX)) ||
(match.key->dst && match.mask->dst != htonl(U32_MAX))) {
- DP_NOTICE(edev, "Do not support ipv4 prefix/masks\n");
+ NL_SET_ERR_MSG_MOD(extack,
+ "Do not support ipv4 prefix/masks");
return -EINVAL;
}
@@ -1776,11 +1778,11 @@ qede_flow_parse_v4_common(struct qede_dev *edev, struct flow_rule *rule,
t->dst_ipv4 = match.key->dst;
}
- err = qede_flow_parse_ports(rule, t, NULL);
+ err = qede_flow_parse_ports(rule, t, extack);
if (err)
return err;
- return qede_set_v4_tuple_to_profile(t, NULL);
+ return qede_set_v4_tuple_to_profile(t, extack);
}
static int
@@ -1800,7 +1802,7 @@ qede_flow_parse_tcp_v4(struct qede_dev *edev, struct flow_rule *rule,
tuple->ip_proto = IPPROTO_TCP;
tuple->eth_proto = htons(ETH_P_IP);
- return qede_flow_parse_v4_common(edev, rule, tuple);
+ return qede_flow_parse_v4_common(rule, tuple, NULL);
}
static int
@@ -1820,7 +1822,7 @@ qede_flow_parse_udp_v4(struct qede_dev *edev, struct flow_rule *rule,
tuple->ip_proto = IPPROTO_UDP;
tuple->eth_proto = htons(ETH_P_IP);
- return qede_flow_parse_v4_common(edev, rule, tuple);
+ return qede_flow_parse_v4_common(rule, tuple, NULL);
}
static int
--
2.43.0
On Wed, May 08, 2024 at 02:33:48PM +0000, Asbjørn Sloth Tønnesen wrote:
> This series converts the filter code in the qede driver
> to use NL_SET_ERR_MSG_*(extack, ...) for error handling.
>
> Patch 1-12 converts qede_parse_flow_attr() to use extack,
> along with all it's static helper functions.
>
> qede_parse_flow_attr() is used in two places:
> - qede_add_tc_flower_fltr()
> - qede_flow_spec_to_rule()
>
> In the latter call site extack is faked in the same way as
> is done in mlxsw (patch 12).
>
> While the conversion is going on, some error messages are silenced
> in between patch 1-12. If wanted could squash patch 1-12 in a v3, but
> I felt that it would be easier to review as 12 more trivial patches.
FWIIW, I like the easy to review approach taken here :)
> Patch 13 and 14, finishes up by converting qede_parse_actions(),
> and ensures that extack is propagated to it, in both call contexts.
..
On Wed, May 08, 2024 at 02:33:52PM +0000, Asbjørn Sloth Tønnesen wrote:
> Convert qede_flow_parse_v6_common() to take extack,
> and drop the edev argument.
>
> Convert DP_NOTICE call to use NL_SET_ERR_MSG_MOD instead.
>
> Pass extack in calls to qede_flow_parse_ports() and
> qede_set_v6_tuple_to_profile().
>
> In calls to qede_flow_parse_v6_common(), use NULL as extack
> for now, until a subsequent patch makes extack available.
>
> Only compile tested.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
On Wed, May 08, 2024 at 02:33:54PM +0000, Asbjørn Sloth Tønnesen wrote:
> Convert qede_flow_parse_tcp_v6() to take extack,
> and drop the edev argument.
>
> Pass extack in call to qede_flow_parse_v6_common().
>
> In call to qede_flow_parse_tcp_v6(), use NULL as extack
> for now, until a subsequent patch makes extack available.
>
> Only compile tested.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
On Wed, May 08, 2024 at 02:33:53PM +0000, Asbjørn Sloth Tønnesen wrote:
> Convert qede_flow_parse_v4_common() to take extack,
> and drop the edev argument.
>
> Convert DP_NOTICE call to use NL_SET_ERR_MSG_MOD instead.
>
> Pass extack in calls to qede_flow_parse_ports() and
> qede_set_v4_tuple_to_profile().
>
> In calls to qede_flow_parse_v4_common(), use NULL as extack
> for now, until a subsequent patch makes extack available.
>
> Only compile tested.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
On Wed, May 08, 2024 at 02:33:58PM +0000, Asbjørn Sloth Tønnesen wrote:
> Define extack locally, to reduce line lengths and aid future users.
>
> Only compile tested.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
On Wed, May 08, 2024 at 02:34:00PM +0000, Asbjørn Sloth Tønnesen wrote:
> Since qede_parse_flow_attr() now does error reporting
> through extack, then give it a fake extack and extract the
> error message afterwards if one was set.
>
> The extracted error message is then passed on through
> DP_NOTICE(), including messages that was earlier issued
> with DP_INFO().
>
> This fake extack approach is already used by
> mlxsw_env_linecard_modules_power_mode_apply() in
> drivers/net/ethernet/mellanox/mlxsw/core_env.c
>
> Only compile tested.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <[email protected]>
>
> ---
> Note:
> Even through _msg is marked in include/linux/netlink.h as
> "don't access directly, use NL_SET_ERR_MSG", then the comment
> above NL_SET_ERR_MSG, seams to indicate that it should be fine
> to access it directly if for reading, as is done other places.
> I could also add a NL_GET_ERR_MSG but I would rather not do that
> in this series.
Reviewed-by: Simon Horman <[email protected]>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <[email protected]>:
On Wed, 8 May 2024 14:33:48 +0000 you wrote:
> This series converts the filter code in the qede driver
> to use NL_SET_ERR_MSG_*(extack, ...) for error handling.
>
> Patch 1-12 converts qede_parse_flow_attr() to use extack,
> along with all it's static helper functions.
>
> qede_parse_flow_attr() is used in two places:
> - qede_add_tc_flower_fltr()
> - qede_flow_spec_to_rule()
>
> [...]
Here is the summary with links:
- [net-next,v2,01/14] net: qede: use extack in qede_flow_parse_ports()
https://git.kernel.org/netdev/net-next/c/a7c9540e967b
- [net-next,v2,02/14] net: qede: use extack in qede_set_v6_tuple_to_profile()
https://git.kernel.org/netdev/net-next/c/6f88f1257a40
- [net-next,v2,03/14] net: qede: use extack in qede_set_v4_tuple_to_profile()
https://git.kernel.org/netdev/net-next/c/f63a9dc507f9
- [net-next,v2,04/14] net: qede: use extack in qede_flow_parse_v6_common()
https://git.kernel.org/netdev/net-next/c/a62944d11ae1
- [net-next,v2,05/14] net: qede: use extack in qede_flow_parse_v4_common()
https://git.kernel.org/netdev/net-next/c/f2f993835b26
- [net-next,v2,06/14] net: qede: use extack in qede_flow_parse_tcp_v6()
https://git.kernel.org/netdev/net-next/c/b1a18d5781d4
- [net-next,v2,07/14] net: qede: use extack in qede_flow_parse_tcp_v4()
https://git.kernel.org/netdev/net-next/c/f84d52776ccf
- [net-next,v2,08/14] net: qede: use extack in qede_flow_parse_udp_v6()
https://git.kernel.org/netdev/net-next/c/b73ad5c7a72e
- [net-next,v2,09/14] net: qede: use extack in qede_flow_parse_udp_v4()
https://git.kernel.org/netdev/net-next/c/9c8f5ed8849c
- [net-next,v2,10/14] net: qede: add extack in qede_add_tc_flower_fltr()
https://git.kernel.org/netdev/net-next/c/f833a6555e9e
- [net-next,v2,11/14] net: qede: use extack in qede_parse_flow_attr()
https://git.kernel.org/netdev/net-next/c/d6883bceb254
- [net-next,v2,12/14] net: qede: use faked extack in qede_flow_spec_to_rule()
https://git.kernel.org/netdev/net-next/c/eb705d734525
- [net-next,v2,13/14] net: qede: propagate extack through qede_flow_spec_validate()
https://git.kernel.org/netdev/net-next/c/d2a437efd017
- [net-next,v2,14/14] net: qede: use extack in qede_parse_actions()
https://git.kernel.org/netdev/net-next/c/841548793bd6
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html