On Sat. 14 Aug 2021 at 20:12, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> On 14.08.2021 18:17:48, Vincent Mailhol wrote:
> [...]
>
> > static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> > struct nlattr *data[],
> > struct netlink_ext_ack *extack)
> > {
> > struct can_priv *priv = netdev_priv(dev);
> > + u32 tdc_mask = 0;
> > int err;
> >
> > /* We need synchronization with dev->stop() */
> > @@ -107,6 +179,7 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> > struct can_ctrlmode *cm;
> > u32 ctrlstatic;
> > u32 maskedflags;
> > + u32 tdc_flags;
> >
> > /* Do not allow changing controller mode while running */
> > if (dev->flags & IFF_UP)
> > @@ -138,7 +211,18 @@ static int can_changelink(struct net_device *dev, struct nlattr *tb[],
> > dev->mtu = CAN_MTU;
> > memset(&priv->data_bittiming, 0,
> > sizeof(priv->data_bittiming));
> > + memset(&priv->tdc, 0, sizeof(priv->tdc));
> > + priv->ctrlmode &= ~CAN_CTRLMODE_TDC_MASK;
> > }
> > +
> > + tdc_flags = cm->flags & CAN_CTRLMODE_TDC_MASK;
> > + /* CAN_CTRLMODE_TDC_{AUTO,MANUAL} are mutually exclusive */
> > + if (tdc_flags == CAN_CTRLMODE_TDC_MASK)
> > + return -EOPNOTSUPP;
> > + /* If one of CAN_CTRLMODE_TDC_* is set then TDC must be set */
> > + if (tdc_flags && !data[IFLA_CAN_TDC])
> > + return -EOPNOTSUPP;
>
> These don't need information form the can_priv, right? So these checks
> can be moved to can_validate()?
ACK. Actually, this comment applies not only to can_changelink()
but also to can_tdc_changelink(). I just sent a v5 where I made
sure to move all the checks that do not rely on can_priv to
can_validate().
Yours sincerely,
Vincent
