From: Christian Hopps <chopps@chopps•org>
To: Simon Horman <horms@kernel•org>
Cc: Christian Hopps <chopps@chopps•org>,
Steffen Klassert <steffen.klassert@secunet•com>,
netdev@vger•kernel.org, Christian Hopps <chopps@labn•net>,
devel@linux-ipsec•org
Subject: Re: [devel-ipsec] [PATCH ipsec-next v5 11/17] xfrm: iptfs: add fragmenting of larger than MTU user packets
Date: Wed, 17 Jul 2024 23:56:58 -0700 [thread overview]
Message-ID: <m2r0brchfr.fsf@chopps.org> (raw)
In-Reply-To: <m24j8ndypy.fsf@chopps.org>
[-- Attachment #1: Type: text/plain, Size: 4825 bytes --]
Christian Hopps <chopps@chopps•org> writes:
> [[PGP Signed Part:Good signature from 2E1D830ED7B83025 Christian Hopps <chopps@gmail•com> (trust ultimate) created at 2024-07-17T23:02:33-0700 using RSA]]
>
> Simon Horman via Devel <devel@linux-ipsec•org> writes:
>
>> On Sun, Jul 14, 2024 at 04:22:39PM -0400, Christian Hopps wrote:
>>> From: Christian Hopps <chopps@labn•net>
>>>
>>> Add support for tunneling user (inner) packets that are larger than the
>>> tunnel's path MTU (outer) using IP-TFS fragmentation.
>>>
>>> Signed-off-by: Christian Hopps <chopps@labn•net>
>>> ---
>>> net/xfrm/xfrm_iptfs.c | 401 +++++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 375 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c
>>
>> ...
>>
>>> +static int iptfs_copy_create_frags(struct sk_buff **skbp,
>>> + struct xfrm_iptfs_data *xtfs, u32 mtu)
>>> +{
>>> + struct skb_seq_state skbseq;
>>> + struct list_head sublist;
>>> + struct sk_buff *skb = *skbp;
>>> + struct sk_buff *nskb = *skbp;
>>> + u32 copy_len, offset;
>>> + u32 to_copy = skb->len - mtu;
>>> + u32 blkoff = 0;
>>> + int err = 0;
>>> +
>>> + INIT_LIST_HEAD(&sublist);
>>> +
>>> + BUG_ON(skb->len <= mtu);
>>> + skb_prepare_seq_read(skb, 0, skb->len, &skbseq);
>>> +
>>> + /* A trimmed `skb` will be sent as the first fragment, later. */
>>> + offset = mtu;
>>> + to_copy = skb->len - offset;
>>> + while (to_copy) {
>>> + /* Send all but last fragment to allow agg. append */
>>> + list_add_tail(&nskb->list, &sublist);
>>> +
>>> + /* FUTURE: if the packet has an odd/non-aligning length we could
>>> + * send less data in the penultimate fragment so that the last
>>> + * fragment then ends on an aligned boundary.
>>> + */
>>> + copy_len = to_copy <= mtu ? to_copy : mtu;
>>
>> nit: this looks like it could be expressed using min()
>>
>> Flagged by Coccinelle
>
> Changed.
>
>>
>>> + nskb = iptfs_copy_create_frag(&skbseq, offset, copy_len);
>>> + if (IS_ERR(nskb)) {
>>> + XFRM_INC_STATS(dev_net(skb->dev),
>>> + LINUX_MIB_XFRMOUTERROR);
>>> + skb_abort_seq_read(&skbseq);
>>> + err = PTR_ERR(nskb);
>>> + nskb = NULL;
>>> + break;
>>> + }
>>> + iptfs_output_prepare_skb(nskb, to_copy);
>>> + offset += copy_len;
>>> + to_copy -= copy_len;
>>> + blkoff = to_copy;
>>
>> blkoff is set but otherwise unused in this function.
>>
>> Flagged by W=1 x86_64 allmodconfig builds with gcc-14 and clang 18.
>
> This value is used in a trace point call in this function.
Moved to the later tracepoint layered commit.
Thanks,
Chris.
>
>>
>>> + }
>>> + skb_abort_seq_read(&skbseq);
>>> +
>>> + /* return last fragment that will be unsent (or NULL) */
>>> + *skbp = nskb;
>>> +
>>> + /* trim the original skb to MTU */
>>> + if (!err)
>>> + err = pskb_trim(skb, mtu);
>>> +
>>> + if (err) {
>>> + /* Free all frags. Don't bother sending a partial packet we will
>>> + * never complete.
>>> + */
>>> + kfree_skb(nskb);
>>> + list_for_each_entry_safe(skb, nskb, &sublist, list) {
>>> + skb_list_del_init(skb);
>>> + kfree_skb(skb);
>>> + }
>>> + return err;
>>> + }
>>> +
>>> + /* prepare the initial fragment with an iptfs header */
>>> + iptfs_output_prepare_skb(skb, 0);
>>> +
>>> + /* Send all but last fragment, if we fail to send a fragment then free
>>> + * the rest -- no point in sending a packet that can't be reassembled.
>>> + */
>>> + list_for_each_entry_safe(skb, nskb, &sublist, list) {
>>> + skb_list_del_init(skb);
>>> + if (!err)
>>> + err = xfrm_output(NULL, skb);
>>> + else
>>> + kfree_skb(skb);
>>> + }
>>> + if (err)
>>> + kfree_skb(*skbp);
>>> + return err;
>>> +}
>>> +
>>> +/**
>>> + * iptfs_first_should_copy() - determine if we should copy packet data.
>>> + * @first_skb: the first skb in the packet
>>> + * @mtu: the MTU.
>>> + *
>>> + * Determine if we should create subsequent skbs to hold the remaining data from
>>> + * a large inner packet by copying the packet data, or cloning the original skb
>>> + * and adjusting the offsets.
>>> + */
>>> +static bool iptfs_first_should_copy(struct sk_buff *first_skb, u32 mtu)
>>> +{
>>> + u32 frag_copy_max;
>>> +
>>> + /* If we have less than frag_copy_max for remaining packet we copy
>>> + * those tail bytes as it is more efficient.
>>> + */
>>> + frag_copy_max = mtu <= IPTFS_FRAG_COPY_MAX ? mtu : IPTFS_FRAG_COPY_MAX;
>>
>> Likewise, it looks like min could be used here too.
>
> Changed.
>
> Thanks!
> Chris.
>>
>>> + if ((int)first_skb->len - (int)mtu < (int)frag_copy_max)
>>> + return true;
>>> +
>>> + /* If we have non-linear skb just use copy */
>>> + if (skb_is_nonlinear(first_skb))
>>> + return true;
>>> +
>>> + /* So we have a simple linear skb, easy to clone and share */
>>> + return false;
>>> +}
>>
>> ...
>
> [[End of PGP Signed Part]]
a
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]
next prev parent reply other threads:[~2024-07-18 7:01 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-14 20:22 [PATCH ipsec-next v5 00/17] Add IP-TFS mode to xfrm Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 01/17] xfrm: config: add CONFIG_XFRM_IPTFS Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 02/17] include: uapi: add ip_tfs_*_hdr packet formats Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 03/17] include: uapi: add IPPROTO_AGGFRAG for AGGFRAG in ESP Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 04/17] xfrm: sysctl: allow configuration of global default values Christian Hopps
2024-07-18 13:13 ` Sabrina Dubroca
2024-07-18 13:15 ` Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 05/17] xfrm: netlink: add config (netlink) options Christian Hopps
2024-07-18 13:45 ` Sabrina Dubroca
2024-07-18 14:08 ` [devel-ipsec] " Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 06/17] xfrm: add mode_cbs module functionality Christian Hopps
2024-07-19 6:14 ` [devel-ipsec] " Florian Westphal
2024-07-20 0:06 ` Christian Hopps
2024-07-25 13:32 ` Sabrina Dubroca
2024-07-30 21:29 ` Christian Hopps
2024-07-31 17:10 ` Sabrina Dubroca
2024-07-31 18:32 ` Christian Hopps
2024-07-31 18:41 ` Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 07/17] xfrm: add generic iptfs defines and functionality Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 08/17] xfrm: iptfs: add new iptfs xfrm mode impl Christian Hopps
2024-07-15 12:39 ` Simon Horman
2024-07-18 5:56 ` [devel-ipsec] " Christian Hopps
2024-07-19 6:16 ` Florian Westphal
2024-07-20 0:30 ` Christian Hopps
2024-07-25 13:33 ` Sabrina Dubroca
2024-07-25 14:56 ` Christian Hopps
2024-07-31 16:29 ` Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 09/17] xfrm: iptfs: add user packet (tunnel ingress) handling Christian Hopps
2024-07-15 12:55 ` Simon Horman
2024-07-18 5:35 ` [devel-ipsec] " Christian Hopps
2024-07-18 6:32 ` Christian Hopps
2024-07-16 14:00 ` kernel test robot
2024-07-14 20:22 ` [PATCH ipsec-next v5 10/17] xfrm: iptfs: share page fragments of inner packets Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 11/17] xfrm: iptfs: add fragmenting of larger than MTU user packets Christian Hopps
2024-07-15 13:12 ` Simon Horman
2024-07-18 5:57 ` [devel-ipsec] " Christian Hopps
2024-07-18 6:56 ` Christian Hopps [this message]
2024-07-14 20:22 ` [PATCH ipsec-next v5 12/17] xfrm: iptfs: add basic receive packet (tunnel egress) handling Christian Hopps
2024-07-15 13:16 ` Simon Horman
2024-07-18 6:14 ` [devel-ipsec] " Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 13/17] xfrm: iptfs: handle received fragmented inner packets Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 14/17] xfrm: iptfs: add reusing received skb for the tunnel egress packet Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 15/17] xfrm: iptfs: add skb-fragment sharing code Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 16/17] xfrm: iptfs: handle reordering of received packets Christian Hopps
2024-07-14 20:22 ` [PATCH ipsec-next v5 17/17] xfrm: iptfs: add tracepoint functionality Christian Hopps
2024-07-15 11:53 ` kernel test robot
2024-07-17 5:21 ` kernel test robot
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=m2r0brchfr.fsf@chopps.org \
--to=chopps@chopps$(echo .)org \
--cc=chopps@labn$(echo .)net \
--cc=devel@linux-ipsec$(echo .)org \
--cc=horms@kernel$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=steffen.klassert@secunet$(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