[3.19.y-ckt stable] Patch "can: fix handling of unmodifiable configuration options" has been added to the 3.19.y-ckt tree
Kamal Mostafa
kamal at canonical.com
Thu Jul 7 17:32:10 UTC 2016
On Thu, Jul 07, 2016 at 08:28:22AM +0200, Oliver Hartkopp wrote:
> Hello Kamal,
>
> I'm fine with this path for 3.19.y-ckt.
>
> Although this patch has a follow-up patch which should by applied too:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=bce271f255dae8335dc4d2ee2c4531e09cc67f5a
>
> It just hit the mainline tree and will show up in the other stable trees for
> sure.
>
> Many thanks,
> Oliver
Thanks very much Oliver -- I'll include that follow-up fix as well.
-Kamal
>
>
> On 07/06/2016 11:00 PM, Kamal Mostafa wrote:
> >This is a note to let you know that I have just added a patch titled
> >
> > can: fix handling of unmodifiable configuration options
> >
> >to the linux-3.19.y-queue branch of the 3.19.y-ckt extended stable tree
> >which can be found at:
> >
> > https://git.launchpad.net/~canonical-kernel/linux/+git/linux-stable-ckt/log/?h=linux-3.19.y-queue
> >
> >This patch is scheduled to be released in version 3.19.8-ckt23.
> >
> >If you, or anyone else, feels it should not be added to this tree, please
> >reply to this email.
> >
> >For more information about the 3.19.y-ckt tree, see
> >https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
> >
> >Thanks.
> >-Kamal
> >
> >---8<------------------------------------------------------------
> >
> >From dd128a3a90d4971b3df969fd7d51fa891457713a Mon Sep 17 00:00:00 2001
> >From: Oliver Hartkopp <socketcan at hartkopp.net>
> >Date: Mon, 21 Mar 2016 20:18:21 +0100
> >Subject: can: fix handling of unmodifiable configuration options
> >
> >commit bb208f144cf3f59d8f89a09a80efd04389718907 upstream.
> >
> >As described in 'can: m_can: tag current CAN FD controllers as non-ISO'
> >(6cfda7fbebe) it is possible to define fixed configuration options by
> >setting the according bit in 'ctrlmode' and clear it in 'ctrlmode_supported'.
> >This leads to the incovenience that the fixed configuration bits can not be
> >passed by netlink even when they have the correct values (e.g. non-ISO, FD).
> >
> >This patch fixes that issue and not only allows fixed set bit values to be set
> >again but now requires(!) to provide these fixed values at configuration time.
> >A valid CAN FD configuration consists of a nominal/arbitration bittiming, a
> >data bittiming and a control mode with CAN_CTRLMODE_FD set - which is now
> >enforced by a new can_validate() function. This fix additionally removed the
> >inconsistency that was prohibiting the support of 'CANFD-only' controller
> >drivers, like the RCar CAN FD.
> >
> >For this reason a new helper can_set_static_ctrlmode() has been introduced to
> >provide a proper interface to handle static enabled CAN controller options.
> >
> >Reported-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram at bp.renesas.com>
> >Signed-off-by: Oliver Hartkopp <socketcan at hartkopp.net>
> >Reviewed-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram at bp.renesas.com>
> >Signed-off-by: Marc Kleine-Budde <mkl at pengutronix.de>
> >Signed-off-by: Kamal Mostafa <kamal at canonical.com>
> >---
> > drivers/net/can/dev.c | 56 +++++++++++++++++++++++++++++++++++++++----
> > drivers/net/can/m_can/m_can.c | 2 +-
> > include/linux/can/dev.h | 22 +++++++++++++++--
> > 3 files changed, 73 insertions(+), 7 deletions(-)
> >
> >diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
> >index 8202ab3..8fa4d9e 100644
> >--- a/drivers/net/can/dev.c
> >+++ b/drivers/net/can/dev.c
> >@@ -693,11 +693,17 @@ int can_change_mtu(struct net_device *dev, int new_mtu)
> > /* allow change of MTU according to the CANFD ability of the device */
> > switch (new_mtu) {
> > case CAN_MTU:
> >+ /* 'CANFD-only' controllers can not switch to CAN_MTU */
> >+ if (priv->ctrlmode_static & CAN_CTRLMODE_FD)
> >+ return -EINVAL;
> >+
> > priv->ctrlmode &= ~CAN_CTRLMODE_FD;
> > break;
> >
> > case CANFD_MTU:
> >- if (!(priv->ctrlmode_supported & CAN_CTRLMODE_FD))
> >+ /* check for potential CANFD ability */
> >+ if (!(priv->ctrlmode_supported & CAN_CTRLMODE_FD) &&
> >+ !(priv->ctrlmode_static & CAN_CTRLMODE_FD))
> > return -EINVAL;
> >
> > priv->ctrlmode |= CAN_CTRLMODE_FD;
> >@@ -779,6 +785,35 @@ static const struct nla_policy can_policy[IFLA_CAN_MAX + 1] = {
> > = { .len = sizeof(struct can_bittiming_const) },
> > };
> >
> >+static int can_validate(struct nlattr *tb[], struct nlattr *data[])
> >+{
> >+ bool is_can_fd = false;
> >+
> >+ /* Make sure that valid CAN FD configurations always consist of
> >+ * - nominal/arbitration bittiming
> >+ * - data bittiming
> >+ * - control mode with CAN_CTRLMODE_FD set
> >+ */
> >+
> >+ if (data[IFLA_CAN_CTRLMODE]) {
> >+ struct can_ctrlmode *cm = nla_data(data[IFLA_CAN_CTRLMODE]);
> >+
> >+ is_can_fd = cm->flags & cm->mask & CAN_CTRLMODE_FD;
> >+ }
> >+
> >+ if (is_can_fd) {
> >+ if (!data[IFLA_CAN_BITTIMING] || !data[IFLA_CAN_DATA_BITTIMING])
> >+ return -EOPNOTSUPP;
> >+ }
> >+
> >+ if (data[IFLA_CAN_DATA_BITTIMING]) {
> >+ if (!is_can_fd || !data[IFLA_CAN_BITTIMING])
> >+ return -EOPNOTSUPP;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> > static int can_changelink(struct net_device *dev,
> > struct nlattr *tb[], struct nlattr *data[])
> > {
> >@@ -810,19 +845,31 @@ static int can_changelink(struct net_device *dev,
> >
> > if (data[IFLA_CAN_CTRLMODE]) {
> > struct can_ctrlmode *cm;
> >+ u32 ctrlstatic;
> >+ u32 maskedflags;
> >
> > /* Do not allow changing controller mode while running */
> > if (dev->flags & IFF_UP)
> > return -EBUSY;
> > cm = nla_data(data[IFLA_CAN_CTRLMODE]);
> >+ ctrlstatic = priv->ctrlmode_static;
> >+ maskedflags = cm->flags & cm->mask;
> >+
> >+ /* check whether provided bits are allowed to be passed */
> >+ if (cm->mask & ~(priv->ctrlmode_supported | ctrlstatic))
> >+ return -EOPNOTSUPP;
> >+
> >+ /* do not check for static fd-non-iso if 'fd' is disabled */
> >+ if (!(maskedflags & CAN_CTRLMODE_FD))
> >+ ctrlstatic &= ~CAN_CTRLMODE_FD_NON_ISO;
> >
> >- /* check whether changed bits are allowed to be modified */
> >- if (cm->mask & ~priv->ctrlmode_supported)
> >+ /* make sure static options are provided by configuration */
> >+ if ((maskedflags & ctrlstatic) != ctrlstatic)
> > return -EOPNOTSUPP;
> >
> > /* clear bits to be modified and copy the flag values */
> > priv->ctrlmode &= ~cm->mask;
> >- priv->ctrlmode |= (cm->flags & cm->mask);
> >+ priv->ctrlmode |= maskedflags;
> >
> > /* CAN_CTRLMODE_FD can only be set when driver supports FD */
> > if (priv->ctrlmode & CAN_CTRLMODE_FD)
> >@@ -963,6 +1010,7 @@ static struct rtnl_link_ops can_link_ops __read_mostly = {
> > .maxtype = IFLA_CAN_MAX,
> > .policy = can_policy,
> > .setup = can_setup,
> >+ .validate = can_validate,
> > .newlink = can_newlink,
> > .changelink = can_changelink,
> > .get_size = can_get_size,
> >diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> >index 2445298..62143cd 100644
> >--- a/drivers/net/can/m_can/m_can.c
> >+++ b/drivers/net/can/m_can/m_can.c
> >@@ -957,7 +957,7 @@ static struct net_device *alloc_m_can_dev(void)
> > priv->can.do_get_berr_counter = m_can_get_berr_counter;
> >
> > /* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.1 */
> >- priv->can.ctrlmode = CAN_CTRLMODE_FD_NON_ISO;
> >+ can_set_static_ctrlmode(dev, CAN_CTRLMODE_FD_NON_ISO);
> >
> > /* CAN_CTRLMODE_FD_NON_ISO can not be changed with M_CAN IP v3.0.1 */
> > priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> >diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
> >index c05ff0f..a48d7b6 100644
> >--- a/include/linux/can/dev.h
> >+++ b/include/linux/can/dev.h
> >@@ -39,8 +39,11 @@ struct can_priv {
> > struct can_clock clock;
> >
> > enum can_state state;
> >- u32 ctrlmode;
> >- u32 ctrlmode_supported;
> >+
> >+ /* CAN controller features - see include/uapi/linux/can/netlink.h */
> >+ u32 ctrlmode; /* current options setting */
> >+ u32 ctrlmode_supported; /* options that can be modified by netlink */
> >+ u32 ctrlmode_static; /* static enabled options for driver/hardware */
> >
> > int restart_ms;
> > struct timer_list restart_timer;
> >@@ -105,6 +108,21 @@ static inline bool can_is_canfd_skb(const struct sk_buff *skb)
> > return skb->len == CANFD_MTU;
> > }
> >
> >+/* helper to define static CAN controller features at device creation time */
> >+static inline void can_set_static_ctrlmode(struct net_device *dev,
> >+ u32 static_mode)
> >+{
> >+ struct can_priv *priv = netdev_priv(dev);
> >+
> >+ /* alloc_candev() succeeded => netdev_priv() is valid at this point */
> >+ priv->ctrlmode = static_mode;
> >+ priv->ctrlmode_static = static_mode;
> >+
> >+ /* override MTU which was set by default in can_setup()? */
> >+ if (static_mode & CAN_CTRLMODE_FD)
> >+ dev->mtu = CANFD_MTU;
> >+}
> >+
> > /* get data length from can_dlc with sanitized can_dlc */
> > u8 can_dlc2len(u8 can_dlc);
> >
> >--
> >2.7.4
> >
More information about the kernel-team
mailing list