On Fri, 2016-05-13 at 16:12 -0700, Guy Harris wrote:
> libpcap offers the ability to request hardware time stamping for packets
> and to inquire which forms of hardware time stamping, if any, are
> supported for an interface.
>
> The Linux implementation currently implements the inquiry by doing a
> ETHTOOL_GET_TS_INFO SIOETHTOOL ioctl and looking at the so_timestamping
> bits, if the linux/ethtool.h header defines ETHTOOL_GET_TS_INFO and the
> ioctl succeeds on the device.
>
> This is inadequate - as libpcap requests hardware time stamping for all
> packets, it should also check whether HWTSTAMP_FILTER_ALL is set in
> rx_filters, and only offer hardware time stamping if it's set.
>
> The code in ixgbe_ptp.c does:
>
> case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
> case HWTSTAMP_FILTER_ALL:
> /* The X550 controller is capable of timestamping all
> packets,
> * which allows it to accept any filter.
> */
> if (hw->mac.type >= ixgbe_mac_X550) {
> tsync_rx_ctl |= IXGBE_TSYNCRXCTL_TYPE_ALL;
> config->rx_filter = HWTSTAMP_FILTER_ALL;
> adapter->flags |= IXGBE_FLAG_RX_HWTSTAMP_ENABLED;
> break;
> }
> /* fall through */
> default:
> /*
> * register RXMTRL must be set in order to do V1 packets,
> * therefore it is not possible to time stamp both V1
> Sync and
> * Delay_Req messages and hardware does not support
> * timestamping all packets => return error
> */
> adapter->flags &= ~(IXGBE_FLAG_RX_HWTSTAMP_ENABLED |
> IXGBE_FLAG_RX_HWTSTAMP_IN_REGISTER);
> config->rx_filter = HWTSTAMP_FILTER_NONE;
> return -ERANGE;
>
> which seems to indicate that only the X550 controller supports time
> stamping all packets in hardware.
>
> However, the code in ixgbe_ethtool.c does:
>
> switch (adapter->hw.mac.type) {
> case ixgbe_mac_X550:
> case ixgbe_mac_X550EM_x:
> case ixgbe_mac_X540:
> case ixgbe_mac_82599EB:
> 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;
>
> if (adapter->ptp_clock)
> info->phc_index = ptp_clock_index(adapter-
> >ptp_clock);
> else
> info->phc_index = -1;
>
> info->tx_types =
> (1 << HWTSTAMP_TX_OFF) |
> (1 << HWTSTAMP_TX_ON);
>
> info->rx_filters =
> (1 << HWTSTAMP_FILTER_NONE) |
> (1 << HWTSTAMP_FILTER_PTP_V1_L4_SYNC) |
> (1 << HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ) |
> (1 << HWTSTAMP_FILTER_PTP_V2_EVENT);
> break;
> default:
> return ethtool_op_get_ts_info(dev, info);
> }
>
> which draws no distinction between the X550 controller and the X540 and
> 82599, and doesn't say *any* of them support HWTSTAMP_FILTER_ALL.
>
> Is it the case that only the ixgbe_mac_X550 and ixgbe_mac_X550EM_x
> controllers support HWTSTAMP_FILTER_ALL? If so, shouldn't
> ixgbe_get_ts_info() be doing something such as:
>
> switch (adapter->hw.mac.type) {
> case ixgbe_mac_X550:
> case ixgbe_mac_X550EM_x:
> case ixgbe_mac_X540:
> case ixgbe_mac_82599EB:
> 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;
>
> if (adapter->ptp_clock)
> info->phc_index = ptp_clock_index(adapter-
> >ptp_clock);
> else
> info->phc_index = -1;
>
> info->tx_types =
> (1 << HWTSTAMP_TX_OFF) |
> (1 << HWTSTAMP_TX_ON);
>
> info->rx_filters =
> (1 << HWTSTAMP_FILTER_NONE) |
> (1 << HWTSTAMP_FILTER_PTP_V1_L4_SYNC) |
> (1 << HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ) |
> (1 << HWTSTAMP_FILTER_PTP_V2_EVENT);
> if (adapter->hw.mac.type >= ixgbe_mac_X550)
> info->rx_filters |= (1 << HWTSTAMP_FILTER_ALL);
> break;
> default:
> return ethtool_op_get_ts_info(dev, info);
> }Are you planning to produce a patch or are you wanting us to do the work to fix the issue? Just asking so that work is not duplicated.
signature.asc
Description: This is a digitally signed message part
