Subject: Re: What ixgbe devices support HWTSTAMP_FILTER_ALL for hardware time stamping?

From: Jeff Kirsher <[email-masked]>
Date: Sat, 14 May 2016 00:41:20 -0700

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.