Re: [PATCH net-next v7 3/6] idpf: support pacing offload

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> If skb->tstamp is in the future, program this future delivery txtime
> in the transmit descriptor.
> 
> TCP pacing offload is only offloaded if SK_PACING_FQ is negotiated and
> the FQ offload_horizon is configured. But device support for pacing
> offload must be more robust: it can also be reached through SO_TXTIME.
> 
> Bounds check txtime. Only packets with timestamp between now and the
> horizon (pacing_offload_horizon) are offloaded.
> 
> Negotiate the feature with the device using virtchnl. Support is
> conditional on
> - splitq mode, where tx and tx completion queues are separate, so
>   completions can be returned out of order.
> - flow scheduling mode, where completions can arrive out of order.
> - PTP to ensure the NIC clock is synced to CLOCK_TAI.
> 
> These features are negotiated per adapter, but expect all vports to
> uniformly request splitq (req_[rt]x_splitq) and flow scheduling
> (flow_sch_en) when available.
> 
> Do not explicitly check all preconditions. Trust the firmware to
> only advertise EDT when all are met. In general, firmware response is
> trusted to be correct. If not, EDT would be the least of the worries.
> In particular, granularity is trusted to be a power of two. And
> time_horizon_ns is a range expressible given the chosen granularity.
> 
> Packets beyond the horizon are sent immediately with the overflow bit
> set.
> 
> On device reset, dev->pacing_offload_horizon, fq offload_horizon and
> granularity are not re-negotiated. It is safe to assume that firmware
> does not change these EDT capabilities across resets.
> 
> Must not be called from netpoll due to ktime_get. But netpoll does not
> generate packets with EDT, so no explicit test is needed.
> 
> Do not fail device initialization on EDT init error. Log an error, but
> continue without EDT, similar to PTP.
> 
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Cc: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Cc: Joshua A Hay <joshua.a.hay@intel.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> 
> ---
> 
> Changes
>   v6 -> v7
>     - rebase onto libie changes: adjust idpf_send_get_edt_caps_msg
>     - only set horizon if splitq with flow scheduling
>     - fail on illegal granularity 0, rather than use hardcoded default
>     - zero edt_caps on negotation error, in case of call after reset
>     - cache ts_gran_pow2 in idpf_tx_queue to be in hot cache line
>     - similar to FQ, add offload slack to avoid pacing < 400ns of now
>     - remove now superfluous include netpoll.h
>     - reverse xmas tree (1x)

> +static void idpf_tx_splitq_set_txtime(const struct sk_buff *skb,
> +				      const struct idpf_tx_queue *tx_q,
> +				      struct idpf_tx_splitq_params *tx_params)
> +{
> +	const int offload_slack_ns = 400;
> +	struct idpf_netdev_priv *np = netdev_priv(skb->dev);

Unused variable after latest refactor. Will remove.

No longer needed after introducing tx_q->ts_gran_pow2.

> +	u64 ts, now, horizon;
> +
> +	horizon = READ_ONCE(skb->dev->pacing_offload_horizon);
> +	if (!horizon)
> +		return;
> +
> +	switch (skb->tstamp_type) {
> +	case SKB_CLOCK_REALTIME:
> +		ts = ktime_to_ns(ktime_add(skb->tstamp,
> +					   ktime_mono_to_any(0, TK_OFFS_TAI) -
> +					   ktime_mono_to_any(0, TK_OFFS_REAL)));
> +		break;
> +	case SKB_CLOCK_MONOTONIC:
> +		ts = ktime_to_ns(ktime_mono_to_any(skb->tstamp, TK_OFFS_TAI));
> +		break;
> +	case SKB_CLOCK_TAI:
> +		ts = ktime_to_ns(skb->tstamp);
> +		break;
> +	default:
> +		WARN_ON_ONCE(1);
> +		return;
> +	}
> +
> +	now = ktime_get_clocktai_ns();
> +	if (ts < now + offload_slack_ns)
> +		return;
> +
> +	/* beyond offload horizon? set overflow bit only */
> +	if (ts > now + horizon) {
> +		tx_params->offload.desc_ts[2] =
> +			IDPF_TXD_FLOW_SCH_HORIZON_OVERFLOW_M;
> +		return;
> +	}
> +
> +	ts >>= tx_q->ts_gran_pow2;
> +
> +	/* 0 is valid 23b timestamp, but also means field unset.
> +	 * Increase by one to avoid this case
> +	 */
> +	if ((ts & 0x7fffff) == 0) {
> +		tx_params->offload.desc_ts[0] = 1;
> +		return;
> +	}
> +
> +	tx_params->offload.desc_ts[0] = ts & 0xff;
> +	tx_params->offload.desc_ts[1] = (ts >> 8) & 0xff;
> +	tx_params->offload.desc_ts[2] = ((ts >> 16) & 0x7f);
> +}