Commit 2423cc20 authored by Stefan Mätje's avatar Stefan Mätje Committed by Marc Kleine-Budde
Browse files

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: default avatarStefan Mätje <stefan.maetje@esd.eu>
Link: https://patch.msgid.link/20240808164224.213522-1-stefan.maetje@esd.eu


Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parent dc2ddcd1

drivers/net/can/dev/netlink.c

+51 −51
Original line number Diff line number Diff line
@@ -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)