Re: [PATCH v3 net-next 14/15] selftests: drv-net: test AF_XDP zero-copy with an SKB peer

["Björn Töpel" <bjorn@kernel.org>]

Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:

>> [Severity: Medium]
>> On NICs that steer flows based on TX, is taking the queue out of RSS
>> enough to keep the TCP control connection off it?
>> 
>> Flow Director ATR is on by default on i40e. i40e_atr() samples TCP TX
>> packets, including the SYN. It then programs an FD filter that steers the
>> flow's RX to the queue of the TX ring:
>> 
>> drivers/net/ethernet/intel/i40e/i40e_txrx.c:i40e_atr() {
>>     ...
>> 	flex_ptype = FIELD_PREP(I40E_TXD_FLTR_QW0_QINDEX_MASK,
>> 				tx_ring->queue_index);
>>     ...
>> }
>
> SO CLOSE! This one has to be addressed as atr is turned on by default and
> this could cause i40e runs being stochastic. I must admit I did not have
> i40e setup on my side, but it has full AF_XDP feature support (including
> mbuf zc) so its coverage matters.
>
> Fix would be:
>
> diff --git a/tools/testing/selftests/drivers/net/hw/xsk.py b/tools/testing/selftests/drivers/net/hw/xsk.py
> --- a/tools/testing/selftests/drivers/net/hw/xsk.py
> +++ b/tools/testing/selftests/drivers/net/hw/xsk.py
> @@ -115,6 +115,16 @@ def _reserve_queue(cfg, undo):
>      return queue
>  
>  
> +def _disable_atr(cfg, undo):
> +    """i40e ATR steers what a TCP connection receives to the queue that it
> +    sends from, which can be the zero-copy one, so turn it off until undo."""
> +    flags = cmd(f"ethtool --show-priv-flags {cfg.ifname}", fail=False).stdout
> +    if re.search(r"^flow-director-atr\s*: on$", flags, re.MULTILINE):
> +        priv = f"--set-priv-flags {cfg.ifname} flow-director-atr"
> +        ethtool(f"{priv} off")
> +        undo.callback(ethtool, f"{priv} on")
> +
> +
>  def _steer_udp(cfg, undo, ifname, queue, host=None):
>      """Steer the test flow to queue until undo, return False if ifname
>      cannot."""
> @@ -313,6 +323,7 @@ def main():
>          cfg.queue = _reserve_queue(cfg, undo)
>          if cfg.queue is None:
>              sys.exit(SKIP_CODE)
> +        _disable_atr(cfg, undo)
>  
>          cfg.udp_port = rand_port()
>          steered = (_steer_udp(cfg, undo, cfg.ifname, cfg.queue) and

Sneaky ATR. Is i40e run in the HW selftests? Could this be a follow-up?


Björn