From: Steffen Klassert <steffen.klassert@secunet.com>
To: Jonathan Basseri <misterikkit@google.com>
Cc: <netdev@vger.kernel.org>, <jkbs@redhat.com>,
<davem@davemloft.net>, <lorenzo@google.com>,
<eric.dumazet@gmail.com>
Subject: Re: [PATCH net] xfrm: Clear sk_dst_cache when applying per-socket policy.
Date: Tue, 24 Oct 2017 09:04:14 +0200 [thread overview]
Message-ID: <20171024070414.GJ3323@secunet.com> (raw)
In-Reply-To: <20171024011855.8658-1-misterikkit@google.com>
On Mon, Oct 23, 2017 at 06:18:55PM -0700, Jonathan Basseri wrote:
> If a socket has a valid dst cache, then xfrm_lookup_route will get
> skipped. However, the cache is not invalidated when applying policy to a
> socket (i.e. IPV6_XFRM_POLICY). The result is that new policies are
> sometimes ignored on those sockets. (Note: This was broken for IPv4 and
> IPv6 at different times.)
>
> This can be demonstrated like so,
> 1. Create UDP socket.
> 2. connect() the socket.
> 3. Apply an outbound XFRM policy to the socket.
> 4. send() data on the socket.
>
> Packets will continue to be sent in the clear instead of matching an
> xfrm or returning a no-match error (EAGAIN). This affects calls to
> send() and not sendto().
>
> Invalidating the sk_dst_cache is necessary to correctly apply xfrm
> policies. Since we do this in xfrm_user_policy(), the sk_lock was
> already acquired in either do_ip_setsockopt() or do_ipv6_setsockopt(),
> and we may call __sk_dst_reset().
>
> Performance impact should be negligible, since this code is only called
> when changing xfrm policy, and only affects the socket in question.
>
> Note: Creating normal XFRM policies should have a similar effect on
> sk_dst_cache entries that match the policy, but that is not fixed in
> this patch.
I think we don't have this problem with 'normal' policies. When
inserting such a policy, we bump the IPv4/IPv6 genid. This should
invalidate all cached dst entries, no?
next prev parent reply other threads:[~2017-10-24 7:04 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-15 22:25 [PATCH net] xfrm: Clear sk_dst_cache when applying per-socket policy Jonathan Basseri
2017-08-16 9:03 ` Lorenzo Colitti
2017-08-16 9:03 ` Jakub Sitnicki
2017-08-16 10:43 ` Eric Dumazet
2017-08-16 11:19 ` Jakub Sitnicki
2017-10-24 1:18 ` Jonathan Basseri
2017-10-24 7:04 ` Steffen Klassert [this message]
2017-10-24 16:58 ` Jonathan Basseri 😶
2017-10-25 4:25 ` Steffen Klassert
2017-10-25 16:52 ` Jonathan Basseri
2017-10-25 16:52 ` [PATCH net v2] " Jonathan Basseri
2017-10-26 10:54 ` Steffen Klassert
-- strict thread matches above, loose matches on Subject: below --
2017-10-24 1:30 [PATCH net] " Jonathan Basseri 😶
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=20171024070414.GJ3323@secunet.com \
--to=steffen.klassert@secunet.com \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=jkbs@redhat.com \
--cc=lorenzo@google.com \
--cc=misterikkit@google.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.