[PATCH 1/1 v2] netfilter: Restore the CT mark in Flow Offload

[PATCH 1/1 v2] netfilter: Restore the CT mark in Flow Offload

[Sven Auhagen]

The skb mark is often used in TC action at egress.
In order to have the skb mark set we can add it to the
skb when we do a flow offload lookup from the CT mark.

v2: Only restore if CONFIG_NF_CONNTRACK_MARK is
    enabled.

Signed-off-by: Sven Auhagen <sven.auhagen@voleatech.de>
---
 include/net/netfilter/nf_flow_table.h | 3 ++-
 net/netfilter/nf_flow_table_core.c    | 9 ++++++++-
 net/netfilter/nf_flow_table_ip.c      | 4 ++--
 net/sched/act_ct.c                    | 2 +-
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index c54a7f707e50..61ad0c1d86f4 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -174,7 +174,8 @@ void flow_offload_refresh(struct nf_flowtable *flow_table,
 			  struct flow_offload *flow);
 
 struct flow_offload_tuple_rhash *flow_offload_lookup(struct nf_flowtable *flow_table,
-						     struct flow_offload_tuple *tuple);
+						     struct flow_offload_tuple *tuple,
+						     struct sk_buff *skb);
 void nf_flow_table_cleanup(struct net_device *dev);
 
 int nf_flow_table_init(struct nf_flowtable *flow_table);
diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 42da6e337276..b32da5b3a980 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -300,7 +300,8 @@ EXPORT_SYMBOL_GPL(flow_offload_teardown);
 
 struct flow_offload_tuple_rhash *
 flow_offload_lookup(struct nf_flowtable *flow_table,
-		    struct flow_offload_tuple *tuple)
+		    struct flow_offload_tuple *tuple,
+		    struct sk_buff *skb)
 {
 	struct flow_offload_tuple_rhash *tuplehash;
 	struct flow_offload *flow;
@@ -319,6 +320,12 @@ flow_offload_lookup(struct nf_flowtable *flow_table,
 	if (unlikely(nf_ct_is_dying(flow->ct)))
 		return NULL;
 
+#if defined(CONFIG_NF_CONNTRACK_MARK)
+	/* Restore Mark for TC */
+	if (skb)
+		skb->mark = flow->ct->mark;
+#endif
+
 	return tuplehash;
 }
 EXPORT_SYMBOL_GPL(flow_offload_lookup);
diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
index a3bca758b849..4b38923234e3 100644
--- a/net/netfilter/nf_flow_table_ip.c
+++ b/net/netfilter/nf_flow_table_ip.c
@@ -257,7 +257,7 @@ nf_flow_offload_ip_hook(void *priv, struct sk_buff *skb,
 	if (nf_flow_tuple_ip(skb, state->in, &tuple) < 0)
 		return NF_ACCEPT;
 
-	tuplehash = flow_offload_lookup(flow_table, &tuple);
+	tuplehash = flow_offload_lookup(flow_table, &tuple, skb);
 	if (tuplehash == NULL)
 		return NF_ACCEPT;
 
@@ -493,7 +493,7 @@ nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb,
 	if (nf_flow_tuple_ipv6(skb, state->in, &tuple) < 0)
 		return NF_ACCEPT;
 
-	tuplehash = flow_offload_lookup(flow_table, &tuple);
+	tuplehash = flow_offload_lookup(flow_table, &tuple, skb);
 	if (tuplehash == NULL)
 		return NF_ACCEPT;
 
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index 1a766393be62..e2195ef67024 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -517,7 +517,7 @@ static bool tcf_ct_flow_table_lookup(struct tcf_ct_params *p,
 		return false;
 	}
 
-	tuplehash = flow_offload_lookup(nf_ft, &tuple);
+	tuplehash = flow_offload_lookup(nf_ft, &tuple, skb);
 	if (!tuplehash)
 		return false;
 
-- 
2.20.1

Re: [PATCH 1/1 v2] netfilter: Restore the CT mark in Flow Offload

[Pablo Neira Ayuso]

Hi Sven,

On Mon, Jun 01, 2020 at 01:12:09PM +0200, Sven Auhagen wrote:
> The skb mark is often used in TC action at egress.
> In order to have the skb mark set we can add it to the
> skb when we do a flow offload lookup from the CT mark.

Thanks for your patch.

I can see a use case for this, however, enabling the skb->mark =
ct->mark restoration is not very flexible.

Every time a default behaviour like this is introduced in the
netfilter codebase, there is someone following up to request a toggle
to enable / to disable it.

Re: [PATCH 1/1 v2] netfilter: Restore the CT mark in Flow Offload

[Sven Auhagen]

On Sat, Jul 04, 2020 at 02:11:33AM +0200, Pablo Neira Ayuso wrote:
> Hi Sven,
> 
> On Mon, Jun 01, 2020 at 01:12:09PM +0200, Sven Auhagen wrote:
> > The skb mark is often used in TC action at egress.
> > In order to have the skb mark set we can add it to the
> > skb when we do a flow offload lookup from the CT mark.
> 
> Thanks for your patch.
> 
> I can see a use case for this, however, enabling the skb->mark =
> ct->mark restoration is not very flexible.
> 
> Every time a default behaviour like this is introduced in the
> netfilter codebase, there is someone following up to request a toggle
> to enable / to disable it.

Hi Pablo,

I understand the argument.
Please disregard the patch in this case.

Best
Sven