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 22:57:52 -0700 [thread overview]
Message-ID: <m24j8ndypy.fsf@chopps.org> (raw)
In-Reply-To: <20240715131239.GD45692@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 4365 bytes --]
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.
>
>> + }
>> + 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;
>> +}
>
> ...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]
next prev parent reply other threads:[~2024-07-18 6:02 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 ` Christian Hopps [this message]
2024-07-18 6:56 ` [devel-ipsec] " Christian Hopps
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=m24j8ndypy.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