| Message ID | 20210815033248.98111-2-mailhol.vincent@wanadoo.fr (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | add the netlink interface for CAN-FD Transmitter Delay Compensation (TDC) | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 4 maintainers not CCed: davem@davemloft.net kuba@kernel.org martin@strongswan.org wg@grandegger.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On 15.08.2021 12:32:42, Vincent Mailhol wrote: > The sanity checks on the control modes will reject any request related > to an unsupported features, even turning it off. > > Example on an interface which does not support CAN-FD: > > $ ip link set can0 type can bitrate 500000 fd off > RTNETLINK answers: Operation not supported > > This patch lets such command go through (but requests to turn on an > unsupported feature are, of course, still denied). > > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> I'm planing to send a pull request to net-next today. I want to do some more tests with this series, but this patch is more or less unrelated, so I can take it in this PR, should I? regards, Marc
On Thu. 19 Aug 2021 at 16:45, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > On 15.08.2021 12:32:42, Vincent Mailhol wrote: > > The sanity checks on the control modes will reject any request related > > to an unsupported features, even turning it off. > > > > Example on an interface which does not support CAN-FD: > > > > $ ip link set can0 type can bitrate 500000 fd off > > RTNETLINK answers: Operation not supported > > > > This patch lets such command go through (but requests to turn on an > > unsupported feature are, of course, still denied). > > > > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > > I'm planing to send a pull request to net-next today. I want to do some > more tests with this series Ack, I am also preparing a new version. But first, I am just waiting for your reply on the tdc-mode {auto, manual, off}. :) > but this patch is more or less unrelated, > so I can take it in this PR, should I? FYI, the reason to add it to the series is that when setting TDC to off, the ip tool sets both CAN_CTRLMODE_TDC_AUTO and CAN_CTRLMODE_TDC_MANUAL to zero (which the corresponding bits in can_ctrlmode::mask set to 1). Without this patch, netlink would return -ENOTSUPP if the driver only supported one of CAN_CTRLMODE_TDC_{AUTO,MANUAL}. Regardless, this patch makes sense as a standalone, I am fine if you include it in your PR. Also, if you want, you can include the latest patch of the series as well: https://lore.kernel.org/linux-can/20210815033248.98111-8-mailhol.vincent@wanadoo.fr/ It's a comment fix, it should be pretty harmless. Yours sincerely, Vincent
On 19.08.2021 18:24:27, Vincent MAILHOL wrote: > On Thu. 19 Aug 2021 at 16:45, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > On 15.08.2021 12:32:42, Vincent Mailhol wrote: > > > The sanity checks on the control modes will reject any request related > > > to an unsupported features, even turning it off. > > > > > > Example on an interface which does not support CAN-FD: > > > > > > $ ip link set can0 type can bitrate 500000 fd off > > > RTNETLINK answers: Operation not supported > > > > > > This patch lets such command go through (but requests to turn on an > > > unsupported feature are, of course, still denied). > > > > > > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > > > > I'm planing to send a pull request to net-next today. I want to do some > > more tests with this series > > Ack, I am also preparing a new version. But first, I am just > waiting for your reply on the tdc-mode {auto, manual, off}. :) I want to do some proper testing, if it's now working as I'm expecting, before continuing the discussion. :D > > but this patch is more or less unrelated, > > so I can take it in this PR, should I? > > FYI, the reason to add it to the series is that when setting TDC to > off, the ip tool sets both CAN_CTRLMODE_TDC_AUTO and > CAN_CTRLMODE_TDC_MANUAL to zero (which the corresponding bits in > can_ctrlmode::mask set to 1). Without this patch, netlink would > return -ENOTSUPP if the driver only supported one of > CAN_CTRLMODE_TDC_{AUTO,MANUAL}. Oh, I see. > Regardless, this patch makes sense as a standalone, I am fine if > you include it in your PR. Why not, reduces the patch amount on your side :) > Also, if you want, you can include the latest patch of the series as well: > https://lore.kernel.org/linux-can/20210815033248.98111-8-mailhol.vincent@wanadoo.fr/ > > It's a comment fix, it should be pretty harmless. Ok, makes sense. regards, Marc
diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c index 147c23d7dab7..80425636049d 100644 --- a/drivers/net/can/dev/netlink.c +++ b/drivers/net/can/dev/netlink.c @@ -116,7 +116,7 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[], maskedflags = cm->flags & cm->mask; /* check whether provided bits are allowed to be passed */ - if (cm->mask & ~(priv->ctrlmode_supported | ctrlstatic)) + if (maskedflags & ~(priv->ctrlmode_supported | ctrlstatic)) return -EOPNOTSUPP; /* do not check for static fd-non-iso if 'fd' is disabled */
The sanity checks on the control modes will reject any request related to an unsupported features, even turning it off. Example on an interface which does not support CAN-FD: $ ip link set can0 type can bitrate 500000 fd off RTNETLINK answers: Operation not supported This patch lets such command go through (but requests to turn on an unsupported feature are, of course, still denied). Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> --- drivers/net/can/dev/netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)