This series adds a netlink interface for the TDC parameters using
netlink nested attributes.
The first patch remove a redundant check. The second patch is the real
thing: the TDC netlink interface.
In March, I introduced the Transmitter Delay Compensation (TDC) to the
kernel though two patches:
- commit 289ea9e4ae59 ("can: add new CAN FD bittiming parameters:
Transmitter Delay Compensation (TDC)")
- commit c25cc7993243 ("can: bittiming: add calculation for CAN FD
Transmitter Delay Compensation (TDC)")
The netlink interface was missing from this series because the initial
patch needed rework in order to make it more flexible for future
changes.
At that time, Marc suggested to take inspiration from the recently
released ethtool-netlink interface.
Ref: https://lore.kernel.org/linux-can/[email protected]/
ethtool uses nested attributes (c.f. NLA_NESTED type in validation
policy). A bit of trivia: the NLA_NESTED type was introduced in
version 2.6.15 of the kernel and thus actually predates Socket CAN.
Ref: commit bfa83a9e03cf ("[NETLINK]: Type-safe netlink messages/attributes interface")
I sent a v1 as an RFC which got zero comments, so I am assuming that
the overall design is OK :)
Now, I feel confident enough to drop the RFC tag. Thanks for your review!
For those who would like to test it, please refer to this iproute2 patch:
https://lore.kernel.org/linux-can/[email protected]/t/#u
** Changelog **
The nested structure (IFLAC_CAN_TDC*) remains unchanged since RFC v1.
The v2 fixes several issue in can_tdc_get_size() and
can_tdc_fill_info(). Namely: can_tdc_get_size() returned an incorrect
size if TDC was not implemented and can_tdc_fill_info() did not
include a fail path with nla_nest_cancel().
Vincent Mailhol (2):
can: netlink: remove redundant check in can_validate()
can: netlink: add interface for CAN-FD Transmitter Delay Compensation
(TDC)
drivers/net/can/dev/netlink.c | 140 ++++++++++++++++++++++++++++++-
include/uapi/linux/can/netlink.h | 26 +++++-
2 files changed, 160 insertions(+), 6 deletions(-)
--
2.31.1
can_validate() does a first check:
| if (is_can_fd) {
| if (!data[IFLA_CAN_BITTIMING] || !data[IFLA_CAN_DATA_BITTIMING])
| return -EOPNOTSUPP;
| }
If that first if succeeds, we know that if is_can_fd is true then
data[IFLA_CAN_BITTIMING is set.
However, the next if switch does not leverage on above knowledge and
redoes the check:
| if (data[IFLA_CAN_DATA_BITTIMING]) {
| if (!is_can_fd || !data[IFLA_CAN_BITTIMING])
| ^~~~~~~~~~~~~~~~~~~~~~~~
| return -EOPNOTSUPP;
| }
This patch removes that redundant check.
Signed-off-by: Vincent Mailhol <[email protected]>
---
drivers/net/can/dev/netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index e38c2566aff4..32c603a09809 100644
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -47,7 +47,7 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
}
if (data[IFLA_CAN_DATA_BITTIMING]) {
- if (!is_can_fd || !data[IFLA_CAN_BITTIMING])
+ if (!is_can_fd)
return -EOPNOTSUPP;
}
--
2.31.1