netdev - Re: [PATCH ipsec-next v8 10/16] xfrm: iptfs: add fragmenting of larger than MTU user packets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <m2jzgsnm3l.fsf@ja-home.int.chopps.org>
Date: Wed, 07 Aug 2024 15:40:14 -0400
From: Christian Hopps <chopps@...pps.org>
To: Steffen Klassert <steffen.klassert@...unet.com>
Cc: Sabrina Dubroca <sd@...asysnail.net>, Christian Hopps
 <chopps@...pps.org>, devel@...ux-ipsec.org, netdev@...r.kernel.org,
 Christian Hopps <chopps@...n.net>
Subject: Re: [PATCH ipsec-next v8 10/16] xfrm: iptfs: add fragmenting of
 larger than MTU user packets


Steffen Klassert <steffen.klassert@...unet.com> writes:

> On Mon, Aug 05, 2024 at 12:25:57AM +0200, Sabrina Dubroca wrote:
>>
>> > +/**
>> > + * skb_copy_bits_seq - copy bits from a skb_seq_state to kernel buffer
>> > + * @st: source skb_seq_state
>> > + * @offset: offset in source
>> > + * @to: destination buffer
>> > + * @len: number of bytes to copy
>> > + *
>> > + * Copy @len bytes from @offset bytes into the source @st to the destination
>> > + * buffer @to. `offset` should increase (or be unchanged) with each subsequent
>> > + * call to this function. If offset needs to decrease from the previous use `st`
>> > + * should be reset first.
>> > + *
>> > + * Return: 0 on success or a negative error code on failure
>> > + */
>> > +static int skb_copy_bits_seq(struct skb_seq_state *st, int offset, void *to,
>> > +			     int len)
>>
>> Probably belongs in net/core/skbuff.c, although I'm really not
>> convinced copying data around is the right way to implement the type
>> of packet splitting IPTFS does (which sounds a bit like a kind of
>> GSO).
>
> I tried to come up with a 'GSO like' variant of this when I did the
> initial review last year at the IPsec workshop. But it turned out
> that things will get even more complicated as they are now.
> We did some performance tests and it was quite compareable to
> tunnel mode, so for a first implementation I'd be ok with the
> copy variant.
>
>
>> And there are helpers in net/core/skbuff.c (such as
>> pskb_carve/pskb_extract) that seem to do similar things to what you
>> need here, without as much data copying.
>
> In case we have helpers that will fit here, we should use them of
> course.

FWIW, The reason I didn't use pskb_extract() rather than the simple iptfs_copy_create_frag() is because pskb_extract uses skb_clone on the original skb then pskb_carve() to narrow the (copied) data pointers to a subset of the original. The new skb data is read-only which does not work for us.

Each of these new skbs are IP-TFS tunnel packets and as such we need to push and write IPTFS+ESP+IP+Ethernet headers on them. In order to make pskb_extract()s skbs writable we would have to allocate new buffer space and copy the data turning them into a writeable skb buffer, and now we're doing something more complex and more cpu intensive to arrive back to what iptfs_copy_create_frag() did simply and straight-forwardly to begin with.

Thanks,
Chris.