[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <08bc45ec-93d4-4220-81d5-7377b3daa5cc@gmail.com>
Date: Mon, 13 Jan 2025 21:32:43 +0000
From: Pavel Begunkov <asml.silence@...il.com>
To: David Wei <dw@...idwei.uk>, Stanislav Fomichev <stfomichev@...il.com>
Cc: io-uring@...r.kernel.org, netdev@...r.kernel.org,
Jens Axboe <axboe@...nel.dk>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jesper Dangaard Brouer
<hawk@...nel.org>, David Ahern <dsahern@...nel.org>,
Mina Almasry <almasrymina@...gle.com>, Joe Damato <jdamato@...tly.com>,
Pedro Tammela <pctammela@...atatu.com>
Subject: Re: [PATCH net-next v10 22/22] io_uring/zcrx: add selftest
On 1/9/25 17:50, David Wei wrote:
> On 2025-01-09 09:33, Stanislav Fomichev wrote:
>> On 01/08, David Wei wrote:
>>> Add a selftest for io_uring zero copy Rx. This test cannot run locally
>>> and requires a remote host to be configured in net.config. The remote
>>> host must have hardware support for zero copy Rx as listed in the
>>> documentation page. The test will restore the NIC config back to before
>>> the test and is idempotent.
>>>
[...]
>>> +
>>> + if (addr &&
>>> + inet_pton(AF_INET6, addr, &(addr6->sin6_addr)) != 1)
>>> + error(1, 0, "ipv6 parse error: %s", addr);
>>> + break;
>>
>> nit: let's drop explicit af_inet support and use dual-stack af_inet6 sockets
>> explicitly? Take a look at parse_address in
>> tools/testing/selftests/drivers/net/hw/ncdevmem.c on how to
>> transparently fallback to v4 (maybe even move that parsing function into
>> some new networking_helpers.c lib similar to bpf subtree?).
>>
>> (context: pure v4 environments are unlikely to exist at this point;
>> anything that supports v6 can use v4-mapped-v6 addresses)
>>
>>> + default:
>>> + error(1, 0, "illegal domain");
>>> + }
>>> +
>>> + if (cfg_payload_len > max_payload_len)
>>> + error(1, 0, "-l: payload exceeds max (%d)", max_payload_len);
>>> +}
>>> +
...
>> [..]
>>
>>> +def _get_rx_ring_entries(cfg):
>>> + eth_cmd = "ethtool -g {} | awk '/RX:/ {{count++}} count == 2 {{print $2; exit}}'"
>>> + res = cmd(eth_cmd.format(cfg.ifname), host=cfg.remote)
>>> + return int(res.stdout)
>>> +
>>> +
>>> +def _get_combined_channels(cfg):
>>> + eth_cmd = "ethtool -l {} | awk '/Combined:/ {{count++}} count == 2 {{print $2; exit}}'"
>>> + res = cmd(eth_cmd.format(cfg.ifname), host=cfg.remote)
>>> + return int(res.stdout)
>>> +
>>> +
>>> +def _set_flow_rule(cfg, chan):
>>> + eth_cmd = "ethtool -N {} flow-type tcp6 dst-port 9999 action {} | awk '{{print $NF}}'"
>>> + res = cmd(eth_cmd.format(cfg.ifname, chan), host=cfg.remote)
>>> + return int(res.stdout)
>>
>> Most of these (except installing flow steering rule) can be done over
>> ethtool ynl family. Should we try to move them over to YNL calls instead
>> of shelling out to ethtool binary? There are some examples in rss_ctx.py
>> on how to work with ethtool spec.
>>
>> Same for setting/resetting number of queues below.
>
> I wanted to use YNL, but these commands are run on the remote host and
> it's currently challenging to use YNL there.
>
>>
>> For the rest, there is also a ethtool() wrapper so you don't have to
>> do cmd("ethtool ...") and can do ethtool("...").
>
> SG, I will update to use ethtool() helper.
If there will be no more issues / concerns, I'd assume we can merge
this series and follow up on top, as it rather sounds like an
improvement but not a real problem. Stan, does it work for you?
--
Pavel Begunkov
Powered by blists - more mailing lists