diff mbox series

[net-next,2/2] net: sched: add helper support in act_ct

Message ID 4781b55b0b7498c574ace703a1481e3688e3f18d.1663946157.git.lucien.xin@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: sched: add helper support in act_ct for ovs offloading | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 10 this patch: 10
netdev/cc_maintainers warning 2 maintainers not CCed: edumazet@google.com pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 5 this patch: 5
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 10 this patch: 10
netdev/checkpatch warning WARNING: line length of 100 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 84 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 99 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Xin Long Sept. 23, 2022, 3:28 p.m. UTC
This patch is to add helper support in act_ct for OVS actions=ct(alg=xxx)
offloading, which is corresponding to Commit cae3a2627520 ("openvswitch:
Allow attaching helpers to ct action") in OVS kernel part.

The difference is when adding TC actions family and proto cannot be got
from the filter/match, other than helper name in tb[TCA_CT_HELPER_NAME],
we also need to send the family in tb[TCA_CT_HELPER_FAMILY] and the
proto in tb[TCA_CT_HELPER_PROTO] to kernel.

Note when calling helper->help() in tcf_ct_act(), the packet will be
dropped if skb's family and proto do not match the helper's.

Reported-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/net/tc_act/tc_ct.h        |   1 +
 include/uapi/linux/tc_act/tc_ct.h |   3 +
 net/sched/act_ct.c                | 163 +++++++++++++++++++++++++++++-
 3 files changed, 165 insertions(+), 2 deletions(-)

Comments

Paolo Abeni Sept. 27, 2022, 10:29 a.m. UTC | #1
On Fri, 2022-09-23 at 11:28 -0400, Xin Long wrote:
> This patch is to add helper support in act_ct for OVS actions=ct(alg=xxx)
> offloading, which is corresponding to Commit cae3a2627520 ("openvswitch:
> Allow attaching helpers to ct action") in OVS kernel part.
> 
> The difference is when adding TC actions family and proto cannot be got
> from the filter/match, other than helper name in tb[TCA_CT_HELPER_NAME],
> we also need to send the family in tb[TCA_CT_HELPER_FAMILY] and the
> proto in tb[TCA_CT_HELPER_PROTO] to kernel.
> 
> Note when calling helper->help() in tcf_ct_act(), the packet will be
> dropped if skb's family and proto do not match the helper's.
> 
> Reported-by: Ilya Maximets <i.maximets@ovn.org>

This tag is a bit out of place here, as it should belong to fixes. Do
you mean 'Suggested-by' ?

> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  include/net/tc_act/tc_ct.h        |   1 +
>  include/uapi/linux/tc_act/tc_ct.h |   3 +
>  net/sched/act_ct.c                | 163 +++++++++++++++++++++++++++++-
>  3 files changed, 165 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/tc_act/tc_ct.h b/include/net/tc_act/tc_ct.h
> index 8250d6f0a462..b24ea2d9400b 100644
> --- a/include/net/tc_act/tc_ct.h
> +++ b/include/net/tc_act/tc_ct.h
> @@ -10,6 +10,7 @@
>  #include <net/netfilter/nf_conntrack_labels.h>
>  
>  struct tcf_ct_params {
> +	struct nf_conntrack_helper *helper;
>  	struct nf_conn *tmpl;
>  	u16 zone;
>  
> diff --git a/include/uapi/linux/tc_act/tc_ct.h b/include/uapi/linux/tc_act/tc_ct.h
> index 5fb1d7ac1027..6c5200f0ed38 100644
> --- a/include/uapi/linux/tc_act/tc_ct.h
> +++ b/include/uapi/linux/tc_act/tc_ct.h
> @@ -22,6 +22,9 @@ enum {
>  	TCA_CT_NAT_PORT_MIN,	/* be16 */
>  	TCA_CT_NAT_PORT_MAX,	/* be16 */
>  	TCA_CT_PAD,
> +	TCA_CT_HELPER_NAME,	/* string */
> +	TCA_CT_HELPER_FAMILY,	/* u8 */
> +	TCA_CT_HELPER_PROTO,	/* u8 */
>  	__TCA_CT_MAX
>  };
>  
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index 193a460a9d7f..771cf72ee9e1 100644
> --- a/net/sched/act_ct.c
> +++ b/net/sched/act_ct.c
> @@ -33,6 +33,7 @@
>  #include <net/netfilter/nf_conntrack_acct.h>
>  #include <net/netfilter/ipv6/nf_defrag_ipv6.h>
>  #include <net/netfilter/nf_conntrack_act_ct.h>
> +#include <net/netfilter/nf_conntrack_seqadj.h>
>  #include <uapi/linux/netfilter/nf_nat.h>
>  
>  static struct workqueue_struct *act_ct_wq;
> @@ -832,6 +833,13 @@ static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
>  
>  static void tcf_ct_params_free(struct tcf_ct_params *params)
>  {
> +	if (params->helper) {
> +#if IS_ENABLED(CONFIG_NF_NAT)
> +		if (params->ct_action & TCA_CT_ACT_NAT)
> +			nf_nat_helper_put(params->helper);
> +#endif
> +		nf_conntrack_helper_put(params->helper);
> +	}
>  	if (params->ct_ft)
>  		tcf_ct_flow_table_put(params->ct_ft);
>  	if (params->tmpl)
> @@ -1022,6 +1030,69 @@ static int tcf_ct_act_nat(struct sk_buff *skb,
>  #endif
>  }
>  
> +static int tcf_ct_helper(struct sk_buff *skb, u8 family)
> +{

This is very similar to ovs_ct_helper(), I'm wondering if a common
helper could be factored out?

> +	const struct nf_conntrack_helper *helper;
> +	const struct nf_conn_help *help;
> +	enum ip_conntrack_info ctinfo;
> +	unsigned int protoff;
> +	struct nf_conn *ct;
> +	u8 proto;
> +	int err;
> +
> +	ct = nf_ct_get(skb, &ctinfo);
> +	if (!ct || ctinfo == IP_CT_RELATED_REPLY)
> +		return NF_ACCEPT;
> +
> +	help = nfct_help(ct);
> +	if (!help)
> +		return NF_ACCEPT;
> +
> +	helper = rcu_dereference(help->helper);
> +	if (!helper)
> +		return NF_ACCEPT;
> +
> +	if (helper->tuple.src.l3num != NFPROTO_UNSPEC &&
> +	    helper->tuple.src.l3num != family)
> +		return NF_DROP;
> +
> +	switch (family) {
> +	case NFPROTO_IPV4:
> +		protoff = ip_hdrlen(skb);
> +		proto = ip_hdr(skb)->protocol;
> +		break;
> +	case NFPROTO_IPV6: {
> +		__be16 frag_off;
> +		int ofs;
> +
> +		proto = ipv6_hdr(skb)->nexthdr;
> +		ofs = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &proto, &frag_off);
> +		if (ofs < 0 || (frag_off & htons(~0x7)) != 0) {
> +			pr_debug("proto header not found\n");
> +			return NF_DROP;

Why this is returning NF_DROP while ovs_ct_helper() returns NF_ACCEPT
here?

> +		}
> +		protoff = ofs;
> +		break;
> +	}
> +	default:
> +		WARN_ONCE(1, "helper invoked on non-IP family!");
> +		return NF_DROP;
> +	}
> +
> +	if (helper->tuple.dst.protonum != proto)
> +		return NF_DROP;

I'm wondering if NF_DROP is appropriate here. This should be a 
situation similar to the above one: the current packet does not match
the helper.

Thanks!

Paolo
Xin Long Sept. 27, 2022, 3:04 p.m. UTC | #2
On Tue, Sep 27, 2022 at 6:29 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Fri, 2022-09-23 at 11:28 -0400, Xin Long wrote:
> > This patch is to add helper support in act_ct for OVS actions=ct(alg=xxx)
> > offloading, which is corresponding to Commit cae3a2627520 ("openvswitch:
> > Allow attaching helpers to ct action") in OVS kernel part.
> >
> > The difference is when adding TC actions family and proto cannot be got
> > from the filter/match, other than helper name in tb[TCA_CT_HELPER_NAME],
> > we also need to send the family in tb[TCA_CT_HELPER_FAMILY] and the
> > proto in tb[TCA_CT_HELPER_PROTO] to kernel.
> >
> > Note when calling helper->help() in tcf_ct_act(), the packet will be
> > dropped if skb's family and proto do not match the helper's.
> >
> > Reported-by: Ilya Maximets <i.maximets@ovn.org>
>
> This tag is a bit out of place here, as it should belong to fixes. Do
> you mean 'Suggested-by' ?
This one was reported as an OVS bug, but from TC side, it's a feature.
I will remove it.

>
> > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > ---
> >  include/net/tc_act/tc_ct.h        |   1 +
> >  include/uapi/linux/tc_act/tc_ct.h |   3 +
> >  net/sched/act_ct.c                | 163 +++++++++++++++++++++++++++++-
> >  3 files changed, 165 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/net/tc_act/tc_ct.h b/include/net/tc_act/tc_ct.h
> > index 8250d6f0a462..b24ea2d9400b 100644
> > --- a/include/net/tc_act/tc_ct.h
> > +++ b/include/net/tc_act/tc_ct.h
> > @@ -10,6 +10,7 @@
> >  #include <net/netfilter/nf_conntrack_labels.h>
> >
> >  struct tcf_ct_params {
> > +     struct nf_conntrack_helper *helper;
> >       struct nf_conn *tmpl;
> >       u16 zone;
> >
> > diff --git a/include/uapi/linux/tc_act/tc_ct.h b/include/uapi/linux/tc_act/tc_ct.h
> > index 5fb1d7ac1027..6c5200f0ed38 100644
> > --- a/include/uapi/linux/tc_act/tc_ct.h
> > +++ b/include/uapi/linux/tc_act/tc_ct.h
> > @@ -22,6 +22,9 @@ enum {
> >       TCA_CT_NAT_PORT_MIN,    /* be16 */
> >       TCA_CT_NAT_PORT_MAX,    /* be16 */
> >       TCA_CT_PAD,
> > +     TCA_CT_HELPER_NAME,     /* string */
> > +     TCA_CT_HELPER_FAMILY,   /* u8 */
> > +     TCA_CT_HELPER_PROTO,    /* u8 */
> >       __TCA_CT_MAX
> >  };
> >
> > diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> > index 193a460a9d7f..771cf72ee9e1 100644
> > --- a/net/sched/act_ct.c
> > +++ b/net/sched/act_ct.c
> > @@ -33,6 +33,7 @@
> >  #include <net/netfilter/nf_conntrack_acct.h>
> >  #include <net/netfilter/ipv6/nf_defrag_ipv6.h>
> >  #include <net/netfilter/nf_conntrack_act_ct.h>
> > +#include <net/netfilter/nf_conntrack_seqadj.h>
> >  #include <uapi/linux/netfilter/nf_nat.h>
> >
> >  static struct workqueue_struct *act_ct_wq;
> > @@ -832,6 +833,13 @@ static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
> >
> >  static void tcf_ct_params_free(struct tcf_ct_params *params)
> >  {
> > +     if (params->helper) {
> > +#if IS_ENABLED(CONFIG_NF_NAT)
> > +             if (params->ct_action & TCA_CT_ACT_NAT)
> > +                     nf_nat_helper_put(params->helper);
> > +#endif
> > +             nf_conntrack_helper_put(params->helper);
> > +     }
> >       if (params->ct_ft)
> >               tcf_ct_flow_table_put(params->ct_ft);
> >       if (params->tmpl)
> > @@ -1022,6 +1030,69 @@ static int tcf_ct_act_nat(struct sk_buff *skb,
> >  #endif
> >  }
> >
> > +static int tcf_ct_helper(struct sk_buff *skb, u8 family)
> > +{
>
> This is very similar to ovs_ct_helper(), I'm wondering if a common
> helper could be factored out?
I wanted to, but these are two modules, I don't expect one depends another.
Although this is for OVS offloading, but it can still be used independently.
maybe I should move this function to nf_conntrack_helper.c in netfilter?

>
> > +     const struct nf_conntrack_helper *helper;
> > +     const struct nf_conn_help *help;
> > +     enum ip_conntrack_info ctinfo;
> > +     unsigned int protoff;
> > +     struct nf_conn *ct;
> > +     u8 proto;
> > +     int err;
> > +
> > +     ct = nf_ct_get(skb, &ctinfo);
> > +     if (!ct || ctinfo == IP_CT_RELATED_REPLY)
> > +             return NF_ACCEPT;
> > +
> > +     help = nfct_help(ct);
> > +     if (!help)
> > +             return NF_ACCEPT;
> > +
> > +     helper = rcu_dereference(help->helper);
> > +     if (!helper)
> > +             return NF_ACCEPT;
> > +
> > +     if (helper->tuple.src.l3num != NFPROTO_UNSPEC &&
> > +         helper->tuple.src.l3num != family)
> > +             return NF_DROP;
> > +
> > +     switch (family) {
> > +     case NFPROTO_IPV4:
> > +             protoff = ip_hdrlen(skb);
> > +             proto = ip_hdr(skb)->protocol;
> > +             break;
> > +     case NFPROTO_IPV6: {
> > +             __be16 frag_off;
> > +             int ofs;
> > +
> > +             proto = ipv6_hdr(skb)->nexthdr;
> > +             ofs = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &proto, &frag_off);
> > +             if (ofs < 0 || (frag_off & htons(~0x7)) != 0) {
> > +                     pr_debug("proto header not found\n");
> > +                     return NF_DROP;
>
> Why this is returning NF_DROP while ovs_ct_helper() returns NF_ACCEPT
> here?
>
> > +             }
> > +             protoff = ofs;
> > +             break;
> > +     }
> > +     default:
> > +             WARN_ONCE(1, "helper invoked on non-IP family!");
> > +             return NF_DROP;
> > +     }
> > +
> > +     if (helper->tuple.dst.protonum != proto)
> > +             return NF_DROP;
>
> I'm wondering if NF_DROP is appropriate here. This should be a
> situation similar to the above one: the current packet does not match
> the helper.
I was thinking the packets arriving here should be matched with the helper's,
this would force users do a right configuration on flower or other match. But
this might be too harsh and will change to NF_ACCEPT.

Thanks!

>
> Thanks!
>
> Paolo
>
Ilya Maximets Oct. 4, 2022, 12:33 p.m. UTC | #3
On 9/27/22 17:04, Xin Long wrote:
> On Tue, Sep 27, 2022 at 6:29 AM Paolo Abeni <pabeni@redhat.com> wrote:
>>
>> On Fri, 2022-09-23 at 11:28 -0400, Xin Long wrote:
>>> This patch is to add helper support in act_ct for OVS actions=ct(alg=xxx)
>>> offloading, which is corresponding to Commit cae3a2627520 ("openvswitch:
>>> Allow attaching helpers to ct action") in OVS kernel part.
>>>
>>> The difference is when adding TC actions family and proto cannot be got
>>> from the filter/match, other than helper name in tb[TCA_CT_HELPER_NAME],
>>> we also need to send the family in tb[TCA_CT_HELPER_FAMILY] and the
>>> proto in tb[TCA_CT_HELPER_PROTO] to kernel.
>>>
>>> Note when calling helper->help() in tcf_ct_act(), the packet will be
>>> dropped if skb's family and proto do not match the helper's.
>>>
>>> Reported-by: Ilya Maximets <i.maximets@ovn.org>
>>
>> This tag is a bit out of place here, as it should belong to fixes. Do
>> you mean 'Suggested-by' ?
> This one was reported as an OVS bug, but from TC side, it's a feature.

My 2c:
- The fact that act_ct doesn't execute helpers attached to skb outside
  of TC (in OVS) can be considered as a bug.
- The ability to set helpers in act_ct itself is indeed a new feature.

Though it was decided to implement both things at the same time to
avoid confusion around what is supported and what is not supported,
especially since there will be no meaningful way to detect if the bug
actually fixed in the kernel or not.

CC: Eelco.

P.S. might also make sense to CC: ovs-dev on a next revision for visibility.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/include/net/tc_act/tc_ct.h b/include/net/tc_act/tc_ct.h
index 8250d6f0a462..b24ea2d9400b 100644
--- a/include/net/tc_act/tc_ct.h
+++ b/include/net/tc_act/tc_ct.h
@@ -10,6 +10,7 @@ 
 #include <net/netfilter/nf_conntrack_labels.h>
 
 struct tcf_ct_params {
+	struct nf_conntrack_helper *helper;
 	struct nf_conn *tmpl;
 	u16 zone;
 
diff --git a/include/uapi/linux/tc_act/tc_ct.h b/include/uapi/linux/tc_act/tc_ct.h
index 5fb1d7ac1027..6c5200f0ed38 100644
--- a/include/uapi/linux/tc_act/tc_ct.h
+++ b/include/uapi/linux/tc_act/tc_ct.h
@@ -22,6 +22,9 @@  enum {
 	TCA_CT_NAT_PORT_MIN,	/* be16 */
 	TCA_CT_NAT_PORT_MAX,	/* be16 */
 	TCA_CT_PAD,
+	TCA_CT_HELPER_NAME,	/* string */
+	TCA_CT_HELPER_FAMILY,	/* u8 */
+	TCA_CT_HELPER_PROTO,	/* u8 */
 	__TCA_CT_MAX
 };
 
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index 193a460a9d7f..771cf72ee9e1 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -33,6 +33,7 @@ 
 #include <net/netfilter/nf_conntrack_acct.h>
 #include <net/netfilter/ipv6/nf_defrag_ipv6.h>
 #include <net/netfilter/nf_conntrack_act_ct.h>
+#include <net/netfilter/nf_conntrack_seqadj.h>
 #include <uapi/linux/netfilter/nf_nat.h>
 
 static struct workqueue_struct *act_ct_wq;
@@ -832,6 +833,13 @@  static int tcf_ct_handle_fragments(struct net *net, struct sk_buff *skb,
 
 static void tcf_ct_params_free(struct tcf_ct_params *params)
 {
+	if (params->helper) {
+#if IS_ENABLED(CONFIG_NF_NAT)
+		if (params->ct_action & TCA_CT_ACT_NAT)
+			nf_nat_helper_put(params->helper);
+#endif
+		nf_conntrack_helper_put(params->helper);
+	}
 	if (params->ct_ft)
 		tcf_ct_flow_table_put(params->ct_ft);
 	if (params->tmpl)
@@ -1022,6 +1030,69 @@  static int tcf_ct_act_nat(struct sk_buff *skb,
 #endif
 }
 
+static int tcf_ct_helper(struct sk_buff *skb, u8 family)
+{
+	const struct nf_conntrack_helper *helper;
+	const struct nf_conn_help *help;
+	enum ip_conntrack_info ctinfo;
+	unsigned int protoff;
+	struct nf_conn *ct;
+	u8 proto;
+	int err;
+
+	ct = nf_ct_get(skb, &ctinfo);
+	if (!ct || ctinfo == IP_CT_RELATED_REPLY)
+		return NF_ACCEPT;
+
+	help = nfct_help(ct);
+	if (!help)
+		return NF_ACCEPT;
+
+	helper = rcu_dereference(help->helper);
+	if (!helper)
+		return NF_ACCEPT;
+
+	if (helper->tuple.src.l3num != NFPROTO_UNSPEC &&
+	    helper->tuple.src.l3num != family)
+		return NF_DROP;
+
+	switch (family) {
+	case NFPROTO_IPV4:
+		protoff = ip_hdrlen(skb);
+		proto = ip_hdr(skb)->protocol;
+		break;
+	case NFPROTO_IPV6: {
+		__be16 frag_off;
+		int ofs;
+
+		proto = ipv6_hdr(skb)->nexthdr;
+		ofs = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &proto, &frag_off);
+		if (ofs < 0 || (frag_off & htons(~0x7)) != 0) {
+			pr_debug("proto header not found\n");
+			return NF_DROP;
+		}
+		protoff = ofs;
+		break;
+	}
+	default:
+		WARN_ONCE(1, "helper invoked on non-IP family!");
+		return NF_DROP;
+	}
+
+	if (helper->tuple.dst.protonum != proto)
+		return NF_DROP;
+
+	err = helper->help(skb, protoff, ct, ctinfo);
+	if (err != NF_ACCEPT)
+		return NF_DROP;
+
+	if (test_bit(IPS_SEQ_ADJUST_BIT, &ct->status) &&
+	    !nf_ct_seq_adjust(skb, ct, ctinfo, protoff))
+		return NF_DROP;
+
+	return NF_ACCEPT;
+}
+
 static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 		      struct tcf_result *res)
 {
@@ -1033,6 +1104,7 @@  static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 	struct nf_hook_state state;
 	int nh_ofs, err, retval;
 	struct tcf_ct_params *p;
+	bool add_helper = false;
 	bool skip_add = false;
 	bool defrag = false;
 	struct nf_conn *ct;
@@ -1119,6 +1191,22 @@  static int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a,
 	if (err != NF_ACCEPT)
 		goto drop;
 
+	if (commit && p->helper && !nfct_help(ct)) {
+		err = __nf_ct_try_assign_helper(ct, p->tmpl, GFP_ATOMIC);
+		if (err)
+			goto drop;
+		add_helper = true;
+		if (p->ct_action & TCA_CT_ACT_NAT && !nfct_seqadj(ct)) {
+			if (!nfct_seqadj_ext_add(ct))
+				return -EINVAL;
+		}
+	}
+
+	if (nf_ct_is_confirmed(ct) ? (!cached || add_helper) : commit) {
+		if (tcf_ct_helper(skb, family) != NF_ACCEPT)
+			goto drop;
+	}
+
 	if (commit) {
 		tcf_ct_act_set_mark(ct, p->mark, p->mark_mask);
 		tcf_ct_act_set_labels(ct, p->labels, p->labels_mask);
@@ -1167,6 +1255,9 @@  static const struct nla_policy ct_policy[TCA_CT_MAX + 1] = {
 	[TCA_CT_NAT_IPV6_MAX] = NLA_POLICY_EXACT_LEN(sizeof(struct in6_addr)),
 	[TCA_CT_NAT_PORT_MIN] = { .type = NLA_U16 },
 	[TCA_CT_NAT_PORT_MAX] = { .type = NLA_U16 },
+	[TCA_CT_HELPER_NAME] = { .type = NLA_STRING, .len = NF_CT_HELPER_NAME_LEN },
+	[TCA_CT_HELPER_FAMILY] = { .type = NLA_U8 },
+	[TCA_CT_HELPER_PROTO] = { .type = NLA_U8 },
 };
 
 static int tcf_ct_fill_params_nat(struct tcf_ct_params *p,
@@ -1248,6 +1339,39 @@  static void tcf_ct_set_key_val(struct nlattr **tb,
 		nla_memcpy(mask, tb[mask_type], len);
 }
 
+static int tcf_ct_add_helper(struct tcf_ct_params *p, const char *name, u8 family,
+			     u8 proto, struct netlink_ext_ack *extack)
+{
+	struct nf_conntrack_helper *helper;
+	struct nf_conn_help *help;
+	int err;
+
+	helper = nf_conntrack_helper_try_module_get(name, family, proto);
+	if (!helper) {
+		NL_SET_ERR_MSG_MOD(extack, "Unknown helper");
+		return -EINVAL;
+	}
+
+	help = nf_ct_helper_ext_add(p->tmpl, GFP_KERNEL);
+	if (!help) {
+		nf_conntrack_helper_put(helper);
+		return -ENOMEM;
+	}
+#if IS_ENABLED(CONFIG_NF_NAT)
+	if (p->ct_action & TCA_CT_ACT_NAT) {
+		err = nf_nat_helper_try_module_get(name, family, proto);
+		if (err) {
+			nf_conntrack_helper_put(helper);
+			NL_SET_ERR_MSG_MOD(extack, "Failed to load NAT helper");
+			return err;
+		}
+	}
+#endif
+	rcu_assign_pointer(help->helper, helper);
+	p->helper = helper;
+	return 0;
+}
+
 static int tcf_ct_fill_params(struct net *net,
 			      struct tcf_ct_params *p,
 			      struct tc_ct *parm,
@@ -1256,8 +1380,9 @@  static int tcf_ct_fill_params(struct net *net,
 {
 	struct tc_ct_action_net *tn = net_generic(net, act_ct_ops.net_id);
 	struct nf_conntrack_zone zone;
+	int err, family, proto, len;
 	struct nf_conn *tmpl;
-	int err;
+	char *name;
 
 	p->zone = NF_CT_DEFAULT_ZONE_ID;
 
@@ -1318,10 +1443,28 @@  static int tcf_ct_fill_params(struct net *net,
 		NL_SET_ERR_MSG_MOD(extack, "Failed to allocate conntrack template");
 		return -ENOMEM;
 	}
-	__set_bit(IPS_CONFIRMED_BIT, &tmpl->status);
 	p->tmpl = tmpl;
+	if (tb[TCA_CT_HELPER_NAME]) {
+		name = nla_data(tb[TCA_CT_HELPER_NAME]);
+		len = nla_len(tb[TCA_CT_HELPER_NAME]);
+		if (len > 16 || name[len - 1] != '\0') {
+			NL_SET_ERR_MSG_MOD(extack, "Failed to parse helper name.");
+			err = -EINVAL;
+			goto err;
+		}
+		family = tb[TCA_CT_HELPER_FAMILY] ? nla_get_u8(tb[TCA_CT_HELPER_FAMILY]) : AF_INET;
+		proto = tb[TCA_CT_HELPER_PROTO] ? nla_get_u8(tb[TCA_CT_HELPER_PROTO]) : IPPROTO_TCP;
+		err = tcf_ct_add_helper(p, name, family, proto, extack);
+		if (err)
+			goto err;
+	}
 
+	__set_bit(IPS_CONFIRMED_BIT, &tmpl->status);
 	return 0;
+err:
+	nf_ct_put(p->tmpl);
+	p->tmpl = NULL;
+	return err;
 }
 
 static int tcf_ct_init(struct net *net, struct nlattr *nla,
@@ -1490,6 +1633,19 @@  static int tcf_ct_dump_nat(struct sk_buff *skb, struct tcf_ct_params *p)
 	return 0;
 }
 
+static int tcf_ct_dump_helper(struct sk_buff *skb, struct nf_conntrack_helper *helper)
+{
+	if (!helper)
+		return 0;
+
+	if (nla_put_string(skb, TCA_CT_HELPER_NAME, helper->name) ||
+	    nla_put_u8(skb, TCA_CT_HELPER_FAMILY, helper->tuple.src.l3num) ||
+	    nla_put_u8(skb, TCA_CT_HELPER_PROTO, helper->tuple.dst.protonum))
+		return -1;
+
+	return 0;
+}
+
 static inline int tcf_ct_dump(struct sk_buff *skb, struct tc_action *a,
 			      int bind, int ref)
 {
@@ -1542,6 +1698,9 @@  static inline int tcf_ct_dump(struct sk_buff *skb, struct tc_action *a,
 	if (tcf_ct_dump_nat(skb, p))
 		goto nla_put_failure;
 
+	if (tcf_ct_dump_helper(skb, p->helper))
+		goto nla_put_failure;
+
 skip_dump:
 	if (nla_put(skb, TCA_CT_PARMS, sizeof(opt), &opt))
 		goto nla_put_failure;