[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <24F9C7DC-1591-44C4-8451-00BF3F593853@chopps.org>
Date: Fri, 6 Sep 2024 23:04:45 -0400
From: Christian Hopps <chopps@...pps.org>
To: Antony Antony <antony@...nome.org>
Cc: Christian Hopps <chopps@...pps.org>,
devel@...ux-ipsec.org,
Steffen Klassert <steffen.klassert@...unet.com>,
netdev@...r.kernel.org,
Florian Westphal <fw@...len.de>,
Sabrina Dubroca <sd@...asysnail.net>,
Simon Horman <horms@...nel.org>
Subject: Re: [devel-ipsec] [PATCH ipsec-next v10 07/16] xfrm: iptfs: add new
iptfs xfrm mode impl
> On Aug 29, 2024, at 07:43, Antony Antony via Devel <devel@...ux-ipsec.org> wrote:
>
> On Fri, Aug 23, 2024 at 10:20:45PM -0400, Christian Hopps wrote:
>> From: Christian Hopps <chopps@...n.net>
>>
>> Add a new xfrm mode implementing AggFrag/IP-TFS from RFC9347.
[...]
>> +static int iptfs_clone(struct xfrm_state *x, struct xfrm_state *orig)
>> +{
>> + struct xfrm_iptfs_data *xtfs;
>> +
>> + xtfs = kmemdup(orig->mode_data, sizeof(*xtfs), GFP_KERNEL);
>> + if (!xtfs)
>> + return -ENOMEM;
>> +
>> + __iptfs_init_state(x, xtfs);
>
> I noticed __iptfs_init_state() is called twice during XFRM_MSG_MIGRATE.
> This, the first, call does the right thing. However, the second call resets
> the iptfs values to zero.
Fixed in patchset v11.
Thanks,
Chris.
>
> While testing I noticed clone is not workig as expected. It seems to reset
> values iptfs. See the "ip x s" out before and after clone.
>
> Here are two "ip x s" output one before clone and another after clone noice
> iptfs values are 0, while before max-queue-size 10485760
>
> root@...t:/testing/pluto/ikev2-mobike-01$ip x s
> src 192.1.2.23 dst 192.1.3.33
> proto esp spi 0xcd561999 reqid 16393 mode iptfs
> replay-window 0 flag af-unspec esn
> auth-trunc hmac(sha256) 0xcba08c655b22df167c9bf16ac8005cffbe15e6baab553b8f48ec0056c037c51f 128
> enc cbc(aes) 0xb3702487e95675713e7dfb738cc21c5dd86a666af38cdabcc3705ed30fea92e2
> lastused 2024-08-29 12:33:12
> anti-replay esn context:
> seq-hi 0x0, seq 0x0, oseq-hi 0x0, oseq 0xb
> replay_window 0, bitmap-length 0
> dir out
> iptfs-opts dont-frag init-delay 0 max-queue-size 10485760 pkt-size 0
> src 192.1.3.33 dst 192.1.2.23
> proto esp spi 0xd9ecf873 reqid 16393 mode iptfs
> replay-window 0 flag af-unspec esn
> auth-trunc hmac(sha256) 0xf841c6643a06186e86a856600e071e2a220450943fdf7b64a8d2f3e3bffd6c62 128
> enc cbc(aes) 0x5ffa993bbc568ecab82e15433b14c03e5da18ca4d216137493d552260bef0be1
> lastused 2024-08-29 12:33:12
> anti-replay esn context:
> seq-hi 0x0, seq 0xb, oseq-hi 0x0, oseq 0x0
> replay_window 128, bitmap-length 4
> 00000000 00000000 00000000 000007ff
> dir in
> iptfs-opts drop-time 3 reorder-window 3
>
> After migrate: note iptfs vallues are 0.
>
> root@...t:/testing/pluto/ikev2-mobike-01$ip x s
> src 192.1.8.22 dst 192.1.2.23
> proto esp spi 0xd9ecf873 reqid 16393 mode iptfs
> replay-window 0 flag af-unspec esn
> auth-trunc hmac(sha256) 0xf841c6643a06186e86a856600e071e2a220450943fdf7b64a8d2f3e3bffd6c62 128
> enc cbc(aes) 0x5ffa993bbc568ecab82e15433b14c03e5da18ca4d216137493d552260bef0be1
> lastused 2024-08-29 12:33:12
> anti-replay esn context:
> seq-hi 0x0, seq 0xb, oseq-hi 0x0, oseq 0x0
> replay_window 128, bitmap-length 4
> 00000000 00000000 00000000 000007ff
> dir in
> iptfs-opts drop-time 0 reorder-window 0
> src 192.1.2.23 dst 192.1.8.22
> proto esp spi 0xcd561999 reqid 16393 mode iptfs
> replay-window 0 flag af-unspec esn
> auth-trunc hmac(sha256) 0xcba08c655b22df167c9bf16ac8005cffbe15e6baab553b8f48ec0056c037c51f 128
> enc cbc(aes) 0xb3702487e95675713e7dfb738cc21c5dd86a666af38cdabcc3705ed30fea92e2
> lastused 2024-08-29 12:33:12
> anti-replay esn context:
> seq-hi 0x0, seq 0x0, oseq-hi 0x0, oseq 0xb
> replay_window 0, bitmap-length 0
> dir out
> iptfs-opts init-delay 0 max-queue-size 0 pkt-size 0
>
> Now running under gdb during a migrate I see __iptfs_init_state() called
> twice.
>
> I got gdb back trace to show the two calls during XFRM_MSG_MIGRATE.
>
> First call __iptfs_init_state() with bt. This is during clone/MIGRATE.
>
> #0 __iptfs_init_state (x=x@...ry=0xffff888110a1fc40, xtfs=xtfs@...ry=0xffff88810e275000)
> at net/xfrm/xfrm_iptfs.c:2674
> #1 0xffffffff81ece552 in iptfs_clone (x=0xffff888110a1fc40, orig=<optimized out>)
> at net/xfrm/xfrm_iptfs.c:2722
> #2 0xffffffff81eb65ad in xfrm_state_clone (encap=0xffffffff00000010, orig=0xffff888110a1e040)
> at net/xfrm/xfrm_state.c:1878
> #3 xfrm_state_migrate (x=x@...ry=0xffff888110a1e040, m=m@...ry=0xffffc90001b47400,
> encap=encap@...ry=0x0 <fixed_percpu_data>) at net/xfrm/xfrm_state.c:1948
> #4 0xffffffff81ea9206 in xfrm_migrate (sel=sel@...ry=0xffff88811193ce50, dir=<optimized out>,
> type=type@...ry=0 '\000', m=m@...ry=0xffffc90001b47400, num_migrate=num_migrate@...ry=1,
> k=k@...ry=0x0 <fixed_percpu_data>, net=<optimized out>, encap=<optimized out>, if_id=<optimized out>,
> extack=<optimized out>) at net/xfrm/xfrm_policy.c:4652
> #5 0xffffffff81ec26de in xfrm_do_migrate (skb=skb@...ry=0xffff888109265000, nlh=<optimized out>,
> attrs=attrs@...ry=0xffffc90001b47730, extack=<optimized out>) at net/xfrm/xfrm_user.c:3047
> #6 0xffffffff81ec3e70 in xfrm_user_rcv_msg (skb=0xffff888109265000, nlh=<optimized out>,
> extack=<optimized out>) at net/xfrm/xfrm_user.c:3389
> ---
> second call to __iptfs_init_state() bt.
>
> #0 __iptfs_init_state (x=x@...ry=0xffff888110a1fc40, xtfs=0xffff88810e272000) at net/xfrm/xfrm_iptfs.c:2674
> #1 0xffffffff81ece1a4 in iptfs_create_state (x=0xffff888110a1fc40) at net/xfrm/xfrm_iptfs.c:2742
> #2 0xffffffff81eb5c61 in xfrm_init_state (x=x@...ry=0xffff888110a1fc40) at net/xfrm/xfrm_state.c:3042
> #3 0xffffffff81eb65dc in xfrm_state_migrate (x=x@...ry=0xffff888110a1e040, m=m@...ry=0xffffc90001b47400,
> encap=encap@...ry=0x0 <fixed_percpu_data>) at net/xfrm/xfrm_state.c:1954
> #4 0xffffffff81ea9206 in xfrm_migrate (sel=sel@...ry=0xffff88811193ce50, dir=<optimized out>,
> type=type@...ry=0 '\000', m=m@...ry=0xffffc90001b47400, num_migrate=num_migrate@...ry=1,
> k=k@...ry=0x0 <fixed_percpu_data>, net=<optimized out>, encap=<optimized out>, if_id=<optimized out>,
> extack=<optimized out>) at net/xfrm/xfrm_policy.c:4652
> #5 0xffffffff81ec26de in xfrm_do_migrate (skb=skb@...ry=0xffff888109265000, nlh=<optimized out>,
> attrs=attrs@...ry=0xffffc90001b47730, extack=<optimized out>) at net/xfrm/xfrm_user.c:3047
> #6 0xffffffff81ec3e70 in xfrm_user_rcv_msg (skb=0xffff888109265000,
> nlh=<optimized out>,
>
> I have a proposed fix against v10, that seems to work. see the attached
> patch. The patch is applied top of the series.
>
> -antony
>
> PS: this exact issue was also reported in:
> https://www.spinics.net/lists/netdev/msg976146.html
> <0001-call-iptfs-state-init-only-once-during-cloning.patch>--
> Devel mailing list
> Devel@...ux-ipsec.org
> https://linux-ipsec.org/mailman/listinfo/devel
Powered by blists - more mailing lists