From: Florian Westphal <fw@strlen.de>
To: <netdev@vger.kernel.org>
Cc: <netfilter-devel@vger.kernel.org>,
lschlesinger@drivenets.com, dsahern@kernel.org,
pablo@netfilter.org, crosser@average.org,
Florian Westphal <fw@strlen.de>
Subject: [PATCH v2 net-next 2/2] vrf: run conntrack only in context of lower/physdev for locally generated packets
Date: Mon, 25 Oct 2021 16:14:00 +0200 [thread overview]
Message-ID: <20211025141400.13698-3-fw@strlen.de> (raw)
In-Reply-To: <20211025141400.13698-1-fw@strlen.de>
The VRF driver invokes netfilter for output+postrouting hooks so that users
can create rules that check for 'oif $vrf' rather than lower device name.
This is a problem when NAT rules are configured.
To avoid any conntrack involvement in round 1, tag skbs as 'untracked'
to prevent conntrack from picking them up.
This gets cleared before the packet gets handed to the ip stack so
conntrack will be active on the second iteration.
One remaining issue is that a rule like
output ... oif $vrfname notrack
won't propagate to the second round because we can't tell
'notrack set via ruleset' and 'notrack set by vrf driver' apart.
However, this isn't a regression: the 'notrack' removal happens
instead of unconditional nf_reset_ct().
I'd also like to avoid leaking more vrf specific conditionals into the
netfilter infra.
For ingress, conntrack has already been done before the packet makes it
to the vrf driver, with this patch egress does connection tracking with
lower/physical device as well.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
drivers/net/vrf.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index bf2fac913942..546aa1aacb77 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -35,6 +35,7 @@
#include <net/l3mdev.h>
#include <net/fib_rules.h>
#include <net/netns/generic.h>
+#include <net/netfilter/nf_conntrack.h>
#define DRV_NAME "vrf"
#define DRV_VERSION "1.1"
@@ -424,12 +425,26 @@ static int vrf_local_xmit(struct sk_buff *skb, struct net_device *dev,
return NETDEV_TX_OK;
}
+static void vrf_nf_set_untracked(struct sk_buff *skb)
+{
+ if (skb_get_nfct(skb) == 0)
+ nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
+}
+
+static void vrf_nf_reset_ct(struct sk_buff *skb)
+{
+ if (skb_get_nfct(skb) == IP_CT_UNTRACKED)
+ nf_reset_ct(skb);
+}
+
#if IS_ENABLED(CONFIG_IPV6)
static int vrf_ip6_local_out(struct net *net, struct sock *sk,
struct sk_buff *skb)
{
int err;
+ vrf_nf_reset_ct(skb);
+
err = nf_hook(NFPROTO_IPV6, NF_INET_LOCAL_OUT, net,
sk, skb, NULL, skb_dst(skb)->dev, dst_output);
@@ -508,6 +523,8 @@ static int vrf_ip_local_out(struct net *net, struct sock *sk,
{
int err;
+ vrf_nf_reset_ct(skb);
+
err = nf_hook(NFPROTO_IPV4, NF_INET_LOCAL_OUT, net, sk,
skb, NULL, skb_dst(skb)->dev, dst_output);
if (likely(err == 1))
@@ -626,8 +643,7 @@ static void vrf_finish_direct(struct sk_buff *skb)
skb_pull(skb, ETH_HLEN);
}
- /* reset skb device */
- nf_reset_ct(skb);
+ vrf_nf_reset_ct(skb);
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -641,7 +657,7 @@ static int vrf_finish_output6(struct net *net, struct sock *sk,
struct neighbour *neigh;
int ret;
- nf_reset_ct(skb);
+ vrf_nf_reset_ct(skb);
skb->protocol = htons(ETH_P_IPV6);
skb->dev = dev;
@@ -752,6 +768,8 @@ static struct sk_buff *vrf_ip6_out_direct(struct net_device *vrf_dev,
skb->dev = vrf_dev;
+ vrf_nf_set_untracked(skb);
+
err = nf_hook(NFPROTO_IPV6, NF_INET_LOCAL_OUT, net, sk,
skb, NULL, vrf_dev, vrf_ip6_out_direct_finish);
@@ -858,7 +876,7 @@ static int vrf_finish_output(struct net *net, struct sock *sk, struct sk_buff *s
struct neighbour *neigh;
bool is_v6gw = false;
- nf_reset_ct(skb);
+ vrf_nf_reset_ct(skb);
/* Be paranoid, rather than too clever. */
if (unlikely(skb_headroom(skb) < hh_len && dev->header_ops)) {
@@ -980,6 +998,8 @@ static struct sk_buff *vrf_ip_out_direct(struct net_device *vrf_dev,
skb->dev = vrf_dev;
+ vrf_nf_set_untracked(skb);
+
err = nf_hook(NFPROTO_IPV4, NF_INET_LOCAL_OUT, net, sk,
skb, NULL, vrf_dev, vrf_ip_out_direct_finish);
--
2.32.0
next prev parent reply other threads:[~2021-10-25 14:14 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-25 14:13 [PATCH v2 net-next 0/2] vrf: rework interaction with netfilter/conntrack Florian Westphal
2021-10-25 14:13 ` [PATCH v2 net-next 1/2] netfilter: conntrack: skip confirmation and nat hooks in postrouting for vrf Florian Westphal
2021-10-25 14:14 ` Florian Westphal [this message]
2021-10-25 14:25 ` [PATCH v2 net-next 2/2] vrf: run conntrack only in context of lower/physdev for locally generated packets David Ahern
2021-10-26 12:36 ` Pablo Neira Ayuso
2021-10-26 12:58 ` Florian Westphal
2021-10-26 13:16 ` Pablo Neira Ayuso
2021-10-26 12:30 ` [PATCH v2 net-next 0/2] vrf: rework interaction with netfilter/conntrack patchwork-bot+netdevbpf
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=20211025141400.13698-3-fw@strlen.de \
--to=fw@strlen.de \
--cc=crosser@average.org \
--cc=dsahern@kernel.org \
--cc=lschlesinger@drivenets.com \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).