Message ID | 20230524121836.2070879-16-jiri@resnulli.us (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | devlink: move port ops into separate structure | expand |
Context | Check | Description |
---|---|---|
netdev/series_format | success | Posting correctly formatted |
netdev/tree_selection | success | Clearly marked for net-next |
netdev/fixes_present | success | Fixes tag not required for -next series |
netdev/header_inline | success | No static functions without inline keyword in header files |
netdev/build_32bit | success | Errors and warnings before: 12 this patch: 12 |
netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
netdev/build_clang | success | Errors and warnings before: 10 this patch: 10 |
netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
netdev/deprecated_api | success | None detected |
netdev/check_selftest | success | No net selftest shell script |
netdev/verify_fixes | success | No Fixes tag |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 12 this patch: 12 |
netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 36 lines checked |
netdev/kdoc | success | Errors and warnings before: 2 this patch: 2 |
netdev/source_inline | success | Was 0 now: 0 |
On Wed, 24 May 2023 14:18:36 +0200 Jiri Pirko wrote: > + const struct devlink_port_ops *ops = devlink_port->ops; > struct nlattr *attr; > > if (tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] && > - (!devlink_port->ops || !devlink_port->ops->port_fn_hw_addr_set)) { > + (!ops || !ops->port_fn_hw_addr_set)) { I was kinda expected last patch will remove the !ops checks. Another series comes after this to convert more drivers?
Thu, May 25, 2023 at 06:55:35AM CEST, kuba@kernel.org wrote: >On Wed, 24 May 2023 14:18:36 +0200 Jiri Pirko wrote: >> + const struct devlink_port_ops *ops = devlink_port->ops; >> struct nlattr *attr; >> >> if (tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] && >> - (!devlink_port->ops || !devlink_port->ops->port_fn_hw_addr_set)) { >> + (!ops || !ops->port_fn_hw_addr_set)) { > >I was kinda expected last patch will remove the !ops checks. >Another series comes after this to convert more drivers? Well, there are still drivers that don't use the port at all ops. I can have them register with empty struct if you like, no strong opinition. I can do that as follow-up (this set has 15 patches already anyway). Let me know.
On Thu, 25 May 2023 07:58:09 +0200 Jiri Pirko wrote: > >I was kinda expected last patch will remove the !ops checks. > >Another series comes after this to convert more drivers? > > Well, there are still drivers that don't use the port at all ops. I can > have them register with empty struct if you like, no strong opinition. I > can do that as follow-up (this set has 15 patches already anyway). Let > me know. Hm. Or maybe we can hook in an empty ops struct in the core when driver passes NULL? No strong preference.
Thu, May 25, 2023 at 05:29:33PM CEST, kuba@kernel.org wrote: >On Thu, 25 May 2023 07:58:09 +0200 Jiri Pirko wrote: >> >I was kinda expected last patch will remove the !ops checks. >> >Another series comes after this to convert more drivers? >> >> Well, there are still drivers that don't use the port at all ops. I can >> have them register with empty struct if you like, no strong opinition. I >> can do that as follow-up (this set has 15 patches already anyway). Let >> me know. > >Hm. Or maybe we can hook in an empty ops struct in the core when driver >passes NULL? No strong preference. Okay, will check, thx!
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index b35dee4dddbc..fd2b1a40b61e 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -1185,16 +1185,17 @@ static int devlink_port_function_validate(struct devlink_port *devlink_port, struct nlattr **tb, struct netlink_ext_ack *extack) { + const struct devlink_port_ops *ops = devlink_port->ops; struct nlattr *attr; if (tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] && - (!devlink_port->ops || !devlink_port->ops->port_fn_hw_addr_set)) { + (!ops || !ops->port_fn_hw_addr_set)) { NL_SET_ERR_MSG_ATTR(extack, tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR], "Port doesn't support function attributes"); return -EOPNOTSUPP; } if (tb[DEVLINK_PORT_FN_ATTR_STATE] && - (!devlink_port->ops || !devlink_port->ops->port_fn_state_set)) { + (!ops || !ops->port_fn_state_set)) { NL_SET_ERR_MSG_ATTR(extack, tb[DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR], "Function does not support state setting"); return -EOPNOTSUPP; @@ -1205,15 +1206,13 @@ static int devlink_port_function_validate(struct devlink_port *devlink_port, caps = nla_get_bitfield32(attr); if (caps.selector & DEVLINK_PORT_FN_CAP_ROCE && - (!devlink_port->ops || - !devlink_port->ops->port_fn_roce_set)) { + (!ops || !ops->port_fn_roce_set)) { NL_SET_ERR_MSG_ATTR(extack, attr, "Port doesn't support RoCE function attribute"); return -EOPNOTSUPP; } if (caps.selector & DEVLINK_PORT_FN_CAP_MIGRATABLE) { - if (!devlink_port->ops || - !devlink_port->ops->port_fn_migratable_set) { + if (!ops || !ops->port_fn_migratable_set) { NL_SET_ERR_MSG_ATTR(extack, attr, "Port doesn't support migratable function attribute"); return -EOPNOTSUPP;