Skip to content

Commit

Permalink
Merge patch series "can: add ethtool support and reporting of timesta…
Browse files Browse the repository at this point in the history
…mping capabilities"

Vincent Mailhol <mailhol.vincent@wanadoo.fr> says:

====================

This series revolves around ethtool and timestamping. Its ultimate
goal is that the timestamping implementation within socketCAN meets
the specification of other network drivers in the kernel. This way,
tcpdump or other tools derived from libpcap can be used to do
timestamping on CAN devices.

* Example on a device with hardware timestamp support *

Before this series:
| # tcpdump -j adapter_unsynced -i can0
| tcpdump: WARNING: When trying to set timestamp type
| 'adapter_unsynced' on can0: That type of time stamp is not supported
| by that device

After applying this series, the warning disappears and tcpdump can be
used to get RX hardware timestamps.

This series is articulated in three major parts.

* Part 1: Add TX software timestamps and report the software
  timestamping capabilities through ethtool.

All the drivers using can_put_echo_skb() already support TX software
timestamps. However, the five drivers not using this function (namely
can327, janz-ican3, slcan, vcan and vxcan) lack such support. Patch 1
to 4 adds this support.  Finally, patch 5 advertises the timesamping
capabilities of all drivers which do not support hardware timestamps.

* Part 2: add TX hardware timestapms

This part is a single patch. In SocketCAN TX hardware is equal to the
RX hardware timestamps of the corresponding loopback frame. Reuse the
TX hardware timestamp to populate the RX hardware timestamp. While the
need of this feature can be debatable, we implement it here so that
generic timestamping tools which are agnostic of the specificity of
SocketCAN can still obtain the value. For example, tcpdump expects for
both TX and RX hardware timestamps to be supported in order to do:
| # tcpdump -j adapter_unsynced -i canX

* Part 3: report the hardware timestamping capabilities and implement
  the hardware timestamps ioctls.

The kernel documentation specifies in [1] that, for the drivers which
support hardware timestamping, SIOCSHWTSTAMP ioctl must be supported
and that SIOCGHWTSTAMP ioctl should be supported. Currently, none of
the CAN drivers do so. This is a gap.

Furthermore, even if not specified, the tools based on libpcap
(e.g. tcpdump) also expect ethtool_ops::get_ts_info to be implemented.

This last part first adds some generic implementation of
net_device_ops::ndo_eth_ioctl and ethtool_ops::get_ts_info which can
be used by the drivers with hardware timestamping capabilities.

It then uses those generic functions to add ioctl and reporting
functionalities to the drivers with hardware timestamping support
(namely: mcp251xfd, etas_es58x, kvaser_{pciefd,usb}, peak_{canfd,usb})

[1] Kernel doc: Timestamping, section 3.1 "Hardware Timestamping
    Implementation: Device Drivers"
Link: https://docs.kernel.org/networking/timestamping.html#hardware-timestamping-implementation-device-drivers

* Testing *

I also developed a tool to test all the different timestamps. For
those who would also like to test it, please have a look at:
https://lore.kernel.org/linux-can/20220725134345.432367-1-mailhol.vincent@wanadoo.fr/T/

* Changelog *

changes since v3: https://lore.kernel.org/all/20220726102454.95096-1-mailhol.vincent@wanadoo.fr

  * The peak drivers (both PCI and USB) do not support hardware TX
    timestamps (only RX). Implement specific ioctl and ethtool
    callback functions for this device.

changes since v2: https://lore.kernel.org/all/20220725155354.482986-1-mailhol.vincent@wanadoo.fr

  * The c_can, flexcan, mcp251xfd and the slcan drivers already
    declared a struct ethtool_ops. Do not declare again the same
    structure and instead populate the .get_ts_info() field of the
    existing structures.

changes since v1: https://lore.kernel.org/all/20220725133208.432176-1-mailhol.vincent@wanadoo.fr

  * First series had a patch to implement
    ethtool_ops::get_drvinfo. This proved to be useless. This patch
    was removed and all the clean-up patches made in preparation of
    that one were moved to a separate series:

    https://lore.kernel.org/linux-can/20220725153124.467061-1-mailhol.vincent@wanadoo.fr/T/#u

====================

Link: https://lore.kernel.org/all/20220727101641.198847-1-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
  • Loading branch information
Marc Kleine-Budde committed Jul 28, 2022
2 parents 7c862ee + bedd948 commit 12a18d7
Show file tree
Hide file tree
Showing 46 changed files with 365 additions and 3 deletions.
6 changes: 6 additions & 0 deletions drivers/net/can/at91_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <linux/clk.h>
#include <linux/errno.h>
#include <linux/ethtool.h>
#include <linux/if_arp.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
Expand Down Expand Up @@ -1152,6 +1153,10 @@ static const struct net_device_ops at91_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops at91_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

static ssize_t mb0_id_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
Expand Down Expand Up @@ -1293,6 +1298,7 @@ static int at91_can_probe(struct platform_device *pdev)
}

dev->netdev_ops = &at91_netdev_ops;
dev->ethtool_ops = &at91_ethtool_ops;
dev->irq = irq;
dev->flags |= IFF_ECHO;

Expand Down
1 change: 1 addition & 0 deletions drivers/net/can/c_can/c_can_ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,5 @@ static void c_can_get_ringparam(struct net_device *netdev,

const struct ethtool_ops c_can_ethtool_ops = {
.get_ringparam = c_can_get_ringparam,
.get_ts_info = ethtool_op_get_ts_info,
};
7 changes: 7 additions & 0 deletions drivers/net/can/can327.c
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,8 @@ static netdev_tx_t can327_netdev_start_xmit(struct sk_buff *skb,
dev->stats.tx_packets++;
dev->stats.tx_bytes += frame->can_id & CAN_RTR_FLAG ? 0 : frame->len;

skb_tx_timestamp(skb);

out:
kfree_skb(skb);
return NETDEV_TX_OK;
Expand All @@ -848,6 +850,10 @@ static const struct net_device_ops can327_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops can327_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

static bool can327_is_valid_rx_char(u8 c)
{
static const bool lut_char_is_valid['z'] = {
Expand Down Expand Up @@ -1032,6 +1038,7 @@ static int can327_ldisc_open(struct tty_struct *tty)
/* Configure netdev interface */
elm->dev = dev;
dev->netdev_ops = &can327_netdev_ops;
dev->ethtool_ops = &can327_ethtool_ops;

/* Mark ldisc channel as alive */
elm->tty = tty;
Expand Down
6 changes: 6 additions & 0 deletions drivers/net/can/cc770/cc770.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <linux/ptrace.h>
#include <linux/string.h>
#include <linux/errno.h>
#include <linux/ethtool.h>
#include <linux/netdevice.h>
#include <linux/if_arp.h>
#include <linux/if_ether.h>
Expand Down Expand Up @@ -836,6 +837,10 @@ static const struct net_device_ops cc770_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops cc770_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

int register_cc770dev(struct net_device *dev)
{
struct cc770_priv *priv = netdev_priv(dev);
Expand All @@ -846,6 +851,7 @@ int register_cc770dev(struct net_device *dev)
return err;

dev->netdev_ops = &cc770_netdev_ops;
dev->ethtool_ops = &cc770_ethtool_ops;

dev->flags |= IFF_ECHO; /* we support local echo */

Expand Down
6 changes: 6 additions & 0 deletions drivers/net/can/ctucanfd/ctucanfd_base.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <linux/clk.h>
#include <linux/errno.h>
#include <linux/ethtool.h>
#include <linux/init.h>
#include <linux/bitfield.h>
#include <linux/interrupt.h>
Expand Down Expand Up @@ -1301,6 +1302,10 @@ static const struct net_device_ops ctucan_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops ctucan_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

int ctucan_suspend(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
Expand Down Expand Up @@ -1377,6 +1382,7 @@ int ctucan_probe_common(struct device *dev, void __iomem *addr, int irq, unsigne
set_drvdata_fnc(dev, ndev);
SET_NETDEV_DEV(ndev, dev);
ndev->netdev_ops = &ctucan_netdev_ops;
ndev->ethtool_ops = &ctucan_ethtool_ops;

/* Getting the can_clk info */
if (!can_clk_rate) {
Expand Down
50 changes: 50 additions & 0 deletions drivers/net/can/dev/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,56 @@ int can_change_mtu(struct net_device *dev, int new_mtu)
}
EXPORT_SYMBOL_GPL(can_change_mtu);

/* generic implementation of netdev_ops::ndo_eth_ioctl for CAN devices
* supporting hardware timestamps
*/
int can_eth_ioctl_hwts(struct net_device *netdev, struct ifreq *ifr, int cmd)
{
struct hwtstamp_config hwts_cfg = { 0 };

switch (cmd) {
case SIOCSHWTSTAMP: /* set */
if (copy_from_user(&hwts_cfg, ifr->ifr_data, sizeof(hwts_cfg)))
return -EFAULT;
if (hwts_cfg.tx_type == HWTSTAMP_TX_ON &&
hwts_cfg.rx_filter == HWTSTAMP_FILTER_ALL)
return 0;
return -ERANGE;

case SIOCGHWTSTAMP: /* get */
hwts_cfg.tx_type = HWTSTAMP_TX_ON;
hwts_cfg.rx_filter = HWTSTAMP_FILTER_ALL;
if (copy_to_user(ifr->ifr_data, &hwts_cfg, sizeof(hwts_cfg)))
return -EFAULT;
return 0;

default:
return -EOPNOTSUPP;
}
}
EXPORT_SYMBOL(can_eth_ioctl_hwts);

/* generic implementation of ethtool_ops::get_ts_info for CAN devices
* supporting hardware timestamps
*/
int can_ethtool_op_get_ts_info_hwts(struct net_device *dev,
struct ethtool_ts_info *info)
{
info->so_timestamping =
SOF_TIMESTAMPING_TX_SOFTWARE |
SOF_TIMESTAMPING_RX_SOFTWARE |
SOF_TIMESTAMPING_SOFTWARE |
SOF_TIMESTAMPING_TX_HARDWARE |
SOF_TIMESTAMPING_RX_HARDWARE |
SOF_TIMESTAMPING_RAW_HARDWARE;
info->phc_index = -1;
info->tx_types = BIT(HWTSTAMP_TX_ON);
info->rx_filters = BIT(HWTSTAMP_FILTER_ALL);

return 0;
}
EXPORT_SYMBOL(can_ethtool_op_get_ts_info_hwts);

/* Common open function when the device gets opened.
*
* This function should be called in the open function of the device
Expand Down
6 changes: 6 additions & 0 deletions drivers/net/can/dev/skb.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ int can_put_echo_skb(struct sk_buff *skb, struct net_device *dev,
/* save frame_len to reuse it when transmission is completed */
can_skb_prv(skb)->frame_len = frame_len;

if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;

skb_tx_timestamp(skb);

/* save this skb for tx interrupt echo handling */
Expand Down Expand Up @@ -107,6 +110,9 @@ __can_get_echo_skb(struct net_device *dev, unsigned int idx, u8 *len_ptr,
struct can_skb_priv *can_skb_priv = can_skb_prv(skb);
struct canfd_frame *cf = (struct canfd_frame *)skb->data;

if (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS)
skb_tstamp_tx(skb, skb_hwtstamps(skb));

/* get the real payload length for netdev statistics */
if (cf->can_id & CAN_RTR_FLAG)
*len_ptr = 0;
Expand Down
1 change: 1 addition & 0 deletions drivers/net/can/flexcan/flexcan-ethtool.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,5 @@ const struct ethtool_ops flexcan_ethtool_ops = {
.get_priv_flags = flexcan_get_priv_flags,
.set_priv_flags = flexcan_set_priv_flags,
.get_sset_count = flexcan_get_sset_count,
.get_ts_info = ethtool_op_get_ts_info,
};
6 changes: 6 additions & 0 deletions drivers/net/can/grcan.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <linux/interrupt.h>
#include <linux/netdevice.h>
#include <linux/delay.h>
#include <linux/ethtool.h>
#include <linux/io.h>
#include <linux/can/dev.h>
#include <linux/spinlock.h>
Expand Down Expand Up @@ -1561,6 +1562,10 @@ static const struct net_device_ops grcan_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops grcan_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

static int grcan_setup_netdev(struct platform_device *ofdev,
void __iomem *base,
int irq, u32 ambafreq, bool txbug)
Expand All @@ -1577,6 +1582,7 @@ static int grcan_setup_netdev(struct platform_device *ofdev,
dev->irq = irq;
dev->flags |= IFF_ECHO;
dev->netdev_ops = &grcan_netdev_ops;
dev->ethtool_ops = &grcan_ethtool_ops;
dev->sysfs_groups[0] = &sysfs_grcan_group;

priv = netdev_priv(dev);
Expand Down
6 changes: 6 additions & 0 deletions drivers/net/can/ifi_canfd/ifi_canfd.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/ethtool.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/kernel.h>
Expand Down Expand Up @@ -925,6 +926,10 @@ static const struct net_device_ops ifi_canfd_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops ifi_canfd_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

static int ifi_canfd_plat_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
Expand Down Expand Up @@ -962,6 +967,7 @@ static int ifi_canfd_plat_probe(struct platform_device *pdev)
ndev->irq = irq;
ndev->flags |= IFF_ECHO; /* we support local echo */
ndev->netdev_ops = &ifi_canfd_netdev_ops;
ndev->ethtool_ops = &ifi_canfd_ethtool_ops;

priv = netdev_priv(ndev);
priv->ndev = ndev;
Expand Down
8 changes: 8 additions & 0 deletions drivers/net/can/janz-ican3.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <linux/module.h>
#include <linux/interrupt.h>
#include <linux/delay.h>
#include <linux/ethtool.h>
#include <linux/platform_device.h>

#include <linux/netdevice.h>
Expand Down Expand Up @@ -1277,6 +1278,8 @@ static void ican3_put_echo_skb(struct ican3_dev *mod, struct sk_buff *skb)
if (!skb)
return;

skb_tx_timestamp(skb);

/* save this skb for tx interrupt echo handling */
skb_queue_tail(&mod->echoq, skb);
}
Expand Down Expand Up @@ -1752,6 +1755,10 @@ static const struct net_device_ops ican3_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops ican3_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

/*
* Low-level CAN Device
*/
Expand Down Expand Up @@ -1923,6 +1930,7 @@ static int ican3_probe(struct platform_device *pdev)
mod->free_page = DPM_FREE_START;

ndev->netdev_ops = &ican3_netdev_ops;
ndev->ethtool_ops = &ican3_ethtool_ops;
ndev->flags |= IFF_ECHO;
SET_NETDEV_DEV(ndev, &pdev->dev);

Expand Down
7 changes: 7 additions & 0 deletions drivers/net/can/kvaser_pciefd.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/device.h>
#include <linux/ethtool.h>
#include <linux/pci.h>
#include <linux/can/dev.h>
#include <linux/timer.h>
Expand Down Expand Up @@ -919,10 +920,15 @@ static void kvaser_pciefd_bec_poll_timer(struct timer_list *data)
static const struct net_device_ops kvaser_pciefd_netdev_ops = {
.ndo_open = kvaser_pciefd_open,
.ndo_stop = kvaser_pciefd_stop,
.ndo_eth_ioctl = can_eth_ioctl_hwts,
.ndo_start_xmit = kvaser_pciefd_start_xmit,
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops kvaser_pciefd_ethtool_ops = {
.get_ts_info = can_ethtool_op_get_ts_info_hwts,
};

static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
{
int i;
Expand All @@ -939,6 +945,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)

can = netdev_priv(netdev);
netdev->netdev_ops = &kvaser_pciefd_netdev_ops;
netdev->ethtool_ops = &kvaser_pciefd_ethtool_ops;
can->reg_base = pcie->reg_base + KVASER_PCIEFD_KCAN0_BASE +
i * KVASER_PCIEFD_KCAN_BASE_OFFSET;

Expand Down
6 changes: 6 additions & 0 deletions drivers/net/can/m_can/m_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/

#include <linux/bitfield.h>
#include <linux/ethtool.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/kernel.h>
Expand Down Expand Up @@ -1829,10 +1830,15 @@ static const struct net_device_ops m_can_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops m_can_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

static int register_m_can_dev(struct net_device *dev)
{
dev->flags |= IFF_ECHO; /* we support local echo */
dev->netdev_ops = &m_can_netdev_ops;
dev->ethtool_ops = &m_can_ethtool_ops;

return register_candev(dev);
}
Expand Down
5 changes: 5 additions & 0 deletions drivers/net/can/mscan/mscan.c
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,10 @@ static const struct net_device_ops mscan_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops mscan_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

int register_mscandev(struct net_device *dev, int mscan_clksrc)
{
struct mscan_priv *priv = netdev_priv(dev);
Expand Down Expand Up @@ -676,6 +680,7 @@ struct net_device *alloc_mscandev(void)
priv = netdev_priv(dev);

dev->netdev_ops = &mscan_netdev_ops;
dev->ethtool_ops = &mscan_ethtool_ops;

dev->flags |= IFF_ECHO; /* we support local echo */

Expand Down
6 changes: 6 additions & 0 deletions drivers/net/can/pch_can.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <linux/interrupt.h>
#include <linux/delay.h>
#include <linux/ethtool.h>
#include <linux/io.h>
#include <linux/module.h>
#include <linux/sched.h>
Expand Down Expand Up @@ -938,6 +939,10 @@ static const struct net_device_ops pch_can_netdev_ops = {
.ndo_change_mtu = can_change_mtu,
};

static const struct ethtool_ops pch_can_ethtool_ops = {
.get_ts_info = ethtool_op_get_ts_info,
};

static void pch_can_remove(struct pci_dev *pdev)
{
struct net_device *ndev = pci_get_drvdata(pdev);
Expand Down Expand Up @@ -1188,6 +1193,7 @@ static int pch_can_probe(struct pci_dev *pdev,
pci_set_drvdata(pdev, ndev);
SET_NETDEV_DEV(ndev, &pdev->dev);
ndev->netdev_ops = &pch_can_netdev_ops;
ndev->ethtool_ops = &pch_can_ethtool_ops;
priv->can.clock.freq = PCH_CAN_CLK; /* Hz */

netif_napi_add_weight(ndev, &priv->napi, pch_can_poll, PCH_RX_OBJ_END);
Expand Down
Loading

0 comments on commit 12a18d7

Please sign in to comment.