can: netlink: avoid call to do_set_data_bittiming callback with stale can_priv::ctrlmode

This patch moves the evaluation of data[IFLA_CAN_CTRLMODE] in function
can_changelink in front of the evaluation of data[IFLA_CAN_BITTIMING].

This avoids a call to do_set_data_bittiming providing a stale
can_priv::ctrlmode with a CAN_CTRLMODE_FD flag not matching the
requested state when switching between a CAN Classic and CAN-FD bitrate.

In the same manner the evaluation of data[IFLA_CAN_CTRLMODE] in function
can_validate is also moved in front of the evaluation of
data[IFLA_CAN_BITTIMING].

This is a preparation for patches where the nominal and data bittiming
may have interdependencies on the driver side depending on the
CAN_CTRLMODE_FD flag state.

Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
Link: https://patch.msgid.link/20240808164224.213522-1-stefan.maetje@esd.eu
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

diff --git a/drivers/net/can/dev/netlink.c b/drivers/net/can/dev/netlink.c
index dfdc039d92a6c114a1d5204c3a42ad170ca1b400..01aacdcda26066e0679337a4342dc2ff817f2071 100644 (file)
--- a/drivers/net/can/dev/netlink.c
+++ b/drivers/net/can/dev/netlink.c
@@ -65,15 +65,6 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
        if (!data)
                return 0;
 
-       if (data[IFLA_CAN_BITTIMING]) {
-               struct can_bittiming bt;
-
-               memcpy(&bt, nla_data(data[IFLA_CAN_BITTIMING]), sizeof(bt));
-               err = can_validate_bittiming(&bt, extack);
-               if (err)
-                       return err;
-       }
-
        if (data[IFLA_CAN_CTRLMODE]) {
                struct can_ctrlmode *cm = nla_data(data[IFLA_CAN_CTRLMODE]);
                u32 tdc_flags = cm->flags & CAN_CTRLMODE_TDC_MASK;
@@ -114,6 +105,15 @@ static int can_validate(struct nlattr *tb[], struct nlattr *data[],
                }
        }
 
+       if (data[IFLA_CAN_BITTIMING]) {
+               struct can_bittiming bt;
+
+               memcpy(&bt, nla_data(data[IFLA_CAN_BITTIMING]), sizeof(bt));
+               err = can_validate_bittiming(&bt, extack);
+               if (err)
+                       return err;
+       }
+
        if (is_can_fd) {
                if (!data[IFLA_CAN_BITTIMING] || !data[IFLA_CAN_DATA_BITTIMING])
                        return -EOPNOTSUPP;
@@ -195,48 +195,6 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
        /* We need synchronization with dev->stop() */
        ASSERT_RTNL();
 
-       if (data[IFLA_CAN_BITTIMING]) {
-               struct can_bittiming bt;
-
-               /* Do not allow changing bittiming while running */
-               if (dev->flags & IFF_UP)
-                       return -EBUSY;
-
-               /* Calculate bittiming parameters based on
-                * bittiming_const if set, otherwise pass bitrate
-                * directly via do_set_bitrate(). Bail out if neither
-                * is given.
-                */
-               if (!priv->bittiming_const && !priv->do_set_bittiming &&
-                   !priv->bitrate_const)
-                       return -EOPNOTSUPP;
-
-               memcpy(&bt, nla_data(data[IFLA_CAN_BITTIMING]), sizeof(bt));
-               err = can_get_bittiming(dev, &bt,
-                                       priv->bittiming_const,
-                                       priv->bitrate_const,
-                                       priv->bitrate_const_cnt,
-                                       extack);
-               if (err)
-                       return err;
-
-               if (priv->bitrate_max && bt.bitrate > priv->bitrate_max) {
-                       NL_SET_ERR_MSG_FMT(extack,
-                                          "arbitration bitrate %u bps surpasses transceiver capabilities of %u bps",
-                                          bt.bitrate, priv->bitrate_max);
-                       return -EINVAL;
-               }
-
-               memcpy(&priv->bittiming, &bt, sizeof(bt));
-
-               if (priv->do_set_bittiming) {
-                       /* Finally, set the bit-timing registers */
-                       err = priv->do_set_bittiming(dev);
-                       if (err)
-                               return err;
-               }
-       }
-
        if (data[IFLA_CAN_CTRLMODE]) {
                struct can_ctrlmode *cm;
                u32 ctrlstatic;
@@ -284,6 +242,48 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
                        priv->ctrlmode &= cm->flags | ~CAN_CTRLMODE_TDC_MASK;
        }
 
+       if (data[IFLA_CAN_BITTIMING]) {
+               struct can_bittiming bt;
+
+               /* Do not allow changing bittiming while running */
+               if (dev->flags & IFF_UP)
+                       return -EBUSY;
+
+               /* Calculate bittiming parameters based on
+                * bittiming_const if set, otherwise pass bitrate
+                * directly via do_set_bitrate(). Bail out if neither
+                * is given.
+                */
+               if (!priv->bittiming_const && !priv->do_set_bittiming &&
+                   !priv->bitrate_const)
+                       return -EOPNOTSUPP;
+
+               memcpy(&bt, nla_data(data[IFLA_CAN_BITTIMING]), sizeof(bt));
+               err = can_get_bittiming(dev, &bt,
+                                       priv->bittiming_const,
+                                       priv->bitrate_const,
+                                       priv->bitrate_const_cnt,
+                                       extack);
+               if (err)
+                       return err;
+
+               if (priv->bitrate_max && bt.bitrate > priv->bitrate_max) {
+                       NL_SET_ERR_MSG_FMT(extack,
+                                          "arbitration bitrate %u bps surpasses transceiver capabilities of %u bps",
+                                          bt.bitrate, priv->bitrate_max);
+                       return -EINVAL;
+               }
+
+               memcpy(&priv->bittiming, &bt, sizeof(bt));
+
+               if (priv->do_set_bittiming) {
+                       /* Finally, set the bit-timing registers */
+                       err = priv->do_set_bittiming(dev);
+                       if (err)
+                               return err;
+               }
+       }
+
        if (data[IFLA_CAN_RESTART_MS]) {
                /* Do not allow changing restart delay while running */
                if (dev->flags & IFF_UP)