| Message ID | 20210815033248.98111-2-mailhol.vincent@wanadoo.fr |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,1/7] can: netlink: allow user to turn off unsupported features | expand |
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 -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
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 -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
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(-)