| Message ID | 20240507104421.1628139-2-ast@fiberby.net (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: qede: convert filter code to use extack | expand |
On 5/7/24 12:44, Asbjørn Sloth Tønnesen wrote: > Convert qede_flow_parse_ports to use extack, > and drop the edev argument. > > Convert DP_NOTICE call to use NL_SET_ERR_MSG_MOD instead. > > In calls to qede_flow_parse_ports(), use NULL as extack > for now, until a subsequent patch makes extack available. > > Only compile tested. > > Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> > --- > drivers/net/ethernet/qlogic/qede/qede_filter.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c > index ded48523c383..3995baa2daa6 100644 > --- a/drivers/net/ethernet/qlogic/qede/qede_filter.c > +++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c > @@ -1700,7 +1700,7 @@ static int qede_parse_actions(struct qede_dev *edev, > } > > static int > -qede_flow_parse_ports(struct qede_dev *edev, struct flow_rule *rule, > +qede_flow_parse_ports(struct netlink_ext_ack *extack, struct flow_rule *rule, > struct qede_arfs_tuple *t) there are ~40 cases in drivers/net/ethernet that have an extack param as not the last one, and over 1250 that have an extack as the last param. My grepping was very naive, and counted both forward declarations and implementations, but it's clear what is the preference. Could you please convert the series to be that way? > { > if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_PORTS)) { > @@ -1709,7 +1709,8 @@ qede_flow_parse_ports(struct qede_dev *edev, struct flow_rule *rule, > flow_rule_match_ports(rule, &match); > if ((match.key->src && match.mask->src != htons(U16_MAX)) || > (match.key->dst && match.mask->dst != htons(U16_MAX))) { > - DP_NOTICE(edev, "Do not support ports masks\n"); > + NL_SET_ERR_MSG_MOD(extack, > + "Do not support ports masks"); > return -EINVAL; > } > > @@ -1747,7 +1748,7 @@ 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(edev, rule, t); > + err = qede_flow_parse_ports(NULL, rule, t); > if (err) > return err; > > @@ -1774,7 +1775,7 @@ qede_flow_parse_v4_common(struct qede_dev *edev, struct flow_rule *rule, > t->dst_ipv4 = match.key->dst; > } > > - err = qede_flow_parse_ports(edev, rule, t); > + err = qede_flow_parse_ports(NULL, rule, t); > if (err) > return err; >
Hi Przemek, Thank you for the review! On 5/8/24 10:07 AM, Przemek Kitszel wrote: > On 5/7/24 12:44, Asbjørn Sloth Tønnesen wrote: >> Convert qede_flow_parse_ports to use extack, >> and drop the edev argument. >> >> Convert DP_NOTICE call to use NL_SET_ERR_MSG_MOD instead. >> >> In calls to qede_flow_parse_ports(), use NULL as extack >> for now, until a subsequent patch makes extack available. >> >> Only compile tested. >> >> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> >> --- >> drivers/net/ethernet/qlogic/qede/qede_filter.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c >> index ded48523c383..3995baa2daa6 100644 >> --- a/drivers/net/ethernet/qlogic/qede/qede_filter.c >> +++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c >> @@ -1700,7 +1700,7 @@ static int qede_parse_actions(struct qede_dev *edev, >> } >> static int >> -qede_flow_parse_ports(struct qede_dev *edev, struct flow_rule *rule, >> +qede_flow_parse_ports(struct netlink_ext_ack *extack, struct flow_rule *rule, >> struct qede_arfs_tuple *t) > > there are ~40 cases in drivers/net/ethernet that have an extack param as > not the last one, and over 1250 that have an extack as the last param. > My grepping was very naive, and counted both forward declarations and > implementations, but it's clear what is the preference. > > Could you please convert the series to be that way? Sure, will do that in v2.
diff --git a/drivers/net/ethernet/qlogic/qede/qede_filter.c b/drivers/net/ethernet/qlogic/qede/qede_filter.c index ded48523c383..3995baa2daa6 100644 --- a/drivers/net/ethernet/qlogic/qede/qede_filter.c +++ b/drivers/net/ethernet/qlogic/qede/qede_filter.c @@ -1700,7 +1700,7 @@ static int qede_parse_actions(struct qede_dev *edev, } static int -qede_flow_parse_ports(struct qede_dev *edev, struct flow_rule *rule, +qede_flow_parse_ports(struct netlink_ext_ack *extack, struct flow_rule *rule, struct qede_arfs_tuple *t) { if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_PORTS)) { @@ -1709,7 +1709,8 @@ qede_flow_parse_ports(struct qede_dev *edev, struct flow_rule *rule, flow_rule_match_ports(rule, &match); if ((match.key->src && match.mask->src != htons(U16_MAX)) || (match.key->dst && match.mask->dst != htons(U16_MAX))) { - DP_NOTICE(edev, "Do not support ports masks\n"); + NL_SET_ERR_MSG_MOD(extack, + "Do not support ports masks"); return -EINVAL; } @@ -1747,7 +1748,7 @@ 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(edev, rule, t); + err = qede_flow_parse_ports(NULL, rule, t); if (err) return err; @@ -1774,7 +1775,7 @@ qede_flow_parse_v4_common(struct qede_dev *edev, struct flow_rule *rule, t->dst_ipv4 = match.key->dst; } - err = qede_flow_parse_ports(edev, rule, t); + err = qede_flow_parse_ports(NULL, rule, t); if (err) return err;
Convert qede_flow_parse_ports to use extack, and drop the edev argument. Convert DP_NOTICE call to use NL_SET_ERR_MSG_MOD instead. In calls to qede_flow_parse_ports(), use NULL as extack for now, until a subsequent patch makes extack available. Only compile tested. Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net> --- drivers/net/ethernet/qlogic/qede/qede_filter.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)