From: "Toke Høiland-Jørgensen" <toke@redhat•com>
To: Antonio Quartulli <antonio@openvpn•net>, netdev@vger•kernel.org
Cc: Jakub Kicinski <kuba@kernel•org>,
Sergey Ryazanov <ryazanov.s.a@gmail•com>,
Paolo Abeni <pabeni@redhat•com>,
Eric Dumazet <edumazet@google•com>
Subject: Re: [PATCH net-next v2 08/22] ovpn: implement basic TX path (UDP)
Date: Mon, 11 Mar 2024 16:19:29 +0100 [thread overview]
Message-ID: <87edcgre2m.fsf@toke.dk> (raw)
In-Reply-To: <0273cf51-fbca-453d-81da-777b9462ce3c@openvpn.net>
Antonio Quartulli <antonio@openvpn•net> writes:
> Hi Toke,
>
> On 08/03/2024 16:31, Toke Høiland-Jørgensen wrote:
>> Antonio Quartulli <antonio@openvpn•net> writes:
>>
>>> +/* send skb to connected peer, if any */
>>> +static void ovpn_queue_skb(struct ovpn_struct *ovpn, struct sk_buff *skb, struct ovpn_peer *peer)
>>> +{
>>> + int ret;
>>> +
>>> + if (likely(!peer))
>>> + /* retrieve peer serving the destination IP of this packet */
>>> + peer = ovpn_peer_lookup_by_dst(ovpn, skb);
>>> + if (unlikely(!peer)) {
>>> + net_dbg_ratelimited("%s: no peer to send data to\n", ovpn->dev->name);
>>> + goto drop;
>>> + }
>>> +
>>> + ret = ptr_ring_produce_bh(&peer->tx_ring, skb);
>>> + if (unlikely(ret < 0)) {
>>> + net_err_ratelimited("%s: cannot queue packet to TX ring\n", peer->ovpn->dev->name);
>>> + goto drop;
>>> + }
>>> +
>>> + if (!queue_work(ovpn->crypto_wq, &peer->encrypt_work))
>>> + ovpn_peer_put(peer);
>>> +
>>> + return;
>>> +drop:
>>> + if (peer)
>>> + ovpn_peer_put(peer);
>>> + kfree_skb_list(skb);
>>> +}
>>
>> So this puts packets on a per-peer 1024-packet FIFO queue with no
>> backpressure? That sounds like a pretty terrible bufferbloat situation.
>> Did you do any kind of latency-under-load testing of this, such as
>> running the RRUL test[0] through it?
>
> Thanks for pointing this out.
>
> Andrew Lunn just raised a similar point about these rings being
> potential bufferbloat pitfalls.
>
> And I totally agree.
>
> I haven't performed any specific test, but I have already seen latency
> bumping here and there under heavy load.
>
> Andrew suggested at least reducing rings size to something like 128 and
> then looking at BQL.
>
> Do you have any hint as to what may make sense for a first
> implementation, balancing complexity and good results?
Hmm, I think BQL may actually be fairly straight forward to implement
for this; if you just call netdev_tx_sent_queue() when the packet has
been encrypted and sent on to the lower layer, the BQL algorithm should
keep the ring buffer occupancy just at the level it needs to be to keep
the encryption worker busy. I am not sure if there is some weird reason
this won't work for something like this, but I can't think of any off
the top of my head. And implementing this should be fairly simple (it's
just a couple of function calls in the right places). As an example, see
this commit adding it to the mvneta driver:
a29b6235560a ("net: mvneta: add BQL support")
Not sure if some additional mechanism is needed to keep a bunch of
encrypted packets from piling up in the physical device qdisc (after
encryption), but that will be in addition, in that case.
-Toke
next prev parent reply other threads:[~2024-03-11 15:19 UTC|newest]
Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-04 15:08 [PATCH net-next v2 00/22] Introducing OpenVPN Data Channel Offload Antonio Quartulli
2024-03-04 15:08 ` [PATCH net-next v2 01/22] netlink: add NLA_POLICY_MAX_LEN macro Antonio Quartulli
2024-03-04 15:08 ` [PATCH net-next v2 02/22] net: introduce OpenVPN Data Channel Offload (ovpn) Antonio Quartulli
2024-03-04 20:47 ` Andrew Lunn
2024-03-04 21:30 ` Antonio Quartulli
2024-03-04 22:46 ` Andrew Lunn
2024-03-05 12:29 ` Antonio Quartulli
2024-03-06 15:51 ` Antonio Quartulli
2024-03-04 15:08 ` [PATCH net-next v2 03/22] ovpn: add basic netlink support Antonio Quartulli
2024-03-04 21:20 ` Andrew Lunn
2024-03-05 15:47 ` Antonio Quartulli
2024-03-05 16:23 ` Andrew Lunn
2024-03-05 19:39 ` Jakub Kicinski
2024-03-06 14:46 ` Antonio Quartulli
2024-03-06 19:10 ` Andrew Lunn
2024-03-08 0:01 ` Antonio Quartulli
2024-03-05 10:49 ` kernel test robot
2024-03-26 11:43 ` Esben Haabendal
2024-03-26 21:39 ` Antonio Quartulli
2024-03-04 15:08 ` [PATCH net-next v2 04/22] ovpn: add basic interface creation/destruction/management routines Antonio Quartulli
2024-03-04 21:33 ` Andrew Lunn
2024-03-05 15:51 ` Antonio Quartulli
2024-03-05 16:27 ` Andrew Lunn
2024-03-06 14:49 ` Antonio Quartulli
2024-03-06 19:31 ` Andrew Lunn
2024-03-08 0:08 ` Antonio Quartulli
2024-03-08 13:13 ` Andrew Lunn
2024-03-08 14:21 ` Antonio Quartulli
2024-03-05 19:40 ` Jakub Kicinski
2024-03-06 14:59 ` Antonio Quartulli
2024-03-04 15:08 ` [PATCH net-next v2 05/22] ovpn: implement interface creation/destruction via netlink Antonio Quartulli
2024-03-05 14:51 ` Simon Horman
2024-03-06 15:01 ` Antonio Quartulli
2024-03-25 15:01 ` Esben Haabendal
2024-03-26 21:44 ` Antonio Quartulli
2024-04-02 6:48 ` Esben Haabendal
2024-03-04 15:08 ` [PATCH net-next v2 06/22] ovpn: introduce the ovpn_peer object Antonio Quartulli
2024-03-04 21:52 ` Andrew Lunn
2024-03-05 15:52 ` Antonio Quartulli
2024-03-04 22:56 ` Andrew Lunn
2024-03-06 16:03 ` Antonio Quartulli
2024-03-06 19:23 ` Andrew Lunn
2024-03-08 0:12 ` Antonio Quartulli
2024-03-08 2:04 ` Andrew Lunn
2024-03-08 11:00 ` Antonio Quartulli
2024-03-26 10:34 ` Esben Haabendal
2024-03-26 21:45 ` Antonio Quartulli
2024-03-04 15:08 ` [PATCH net-next v2 07/22] ovpn: introduce the ovpn_socket object Antonio Quartulli
2024-03-05 14:59 ` Simon Horman
2024-03-06 15:08 ` Antonio Quartulli
2024-03-04 15:08 ` [PATCH net-next v2 08/22] ovpn: implement basic TX path (UDP) Antonio Quartulli
2024-03-05 19:47 ` Jakub Kicinski
2024-03-06 15:18 ` Antonio Quartulli
2024-03-08 15:31 ` Toke Høiland-Jørgensen
2024-03-08 15:44 ` Antonio Quartulli
2024-03-11 15:19 ` Toke Høiland-Jørgensen [this message]
2024-03-11 16:28 ` Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 09/22] ovpn: implement basic RX " Antonio Quartulli
2024-03-05 15:04 ` Simon Horman
2024-03-06 15:29 ` Antonio Quartulli
2024-03-08 2:17 ` Andrew Lunn
2024-03-08 11:07 ` Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 10/22] ovpn: implement packet processing Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 11/22] ovpn: store tunnel and transport statistics Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 12/22] ovpn: implement TCP transport Antonio Quartulli
2024-03-05 15:12 ` Simon Horman
2024-03-06 15:31 ` Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 13/22] ovpn: implement multi-peer support Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 14/22] ovpn: implement peer lookup logic Antonio Quartulli
2024-03-05 15:16 ` Simon Horman
2024-03-06 15:33 ` Antonio Quartulli
2024-03-06 0:11 ` kernel test robot
2024-03-09 10:16 ` kernel test robot
2024-03-04 15:09 ` [PATCH net-next v2 15/22] ovpn: implement keepalive mechanism Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 16/22] ovpn: add support for updating local UDP endpoint Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 17/22] ovpn: add support for peer floating Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 18/22] ovpn: implement peer add/dump/delete via netlink Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 19/22] ovpn: implement key add/del/swap " Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 20/22] ovpn: kill key and notify userspace in case of IV exhaustion Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 21/22] ovpn: notify userspace when a peer is deleted Antonio Quartulli
2024-03-04 15:09 ` [PATCH net-next v2 22/22] ovpn: add basic ethtool support Antonio Quartulli
2024-03-04 23:04 ` Andrew Lunn
2024-03-06 15:42 ` Antonio Quartulli
2024-03-06 19:40 ` Andrew Lunn
2024-03-08 0:21 ` Antonio Quartulli
2024-03-04 21:07 ` [PATCH net-next v2 00/22] Introducing OpenVPN Data Channel Offload Sergey Ryazanov
2024-03-05 19:30 ` Jakub Kicinski
2024-03-06 15:44 ` Antonio Quartulli
2024-03-06 16:13 ` Jakub Kicinski
2024-03-08 0:21 ` Antonio Quartulli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87edcgre2m.fsf@toke.dk \
--to=toke@redhat$(echo .)com \
--cc=antonio@openvpn$(echo .)net \
--cc=edumazet@google$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=pabeni@redhat$(echo .)com \
--cc=ryazanov.s.a@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox