[PATCH net-next 0/2] net/sched: retire tc ipt action

[PATCH net-next 0/2] net/sched: retire tc ipt action

[Jamal Hadi Salim]

In keeping up with my status as a hero who removes code: another one bites the
dust.
The tc ipt action was intended to run all netfilter/iptables target.
Unfortunately it has not benefitted over the years from proper updates when
netfilter changes, and for that reason it has remained rudimentary.
Pinging a bunch of people that i was aware were using this indicates that
removing it wont affect them.
Retire it to reduce maintenance efforts.
So Long, ipt, and Thanks for all the Fish.

Jamal Hadi Salim (2):
  net/sched: Retire ipt action
  net/sched: Remove CONFIG_NET_ACT_IPT from default configs

 arch/loongarch/configs/loongson3_defconfig |   1 -
 arch/mips/configs/ip22_defconfig           |   1 -
 arch/mips/configs/malta_defconfig          |   1 -
 arch/mips/configs/malta_kvm_defconfig      |   1 -
 arch/mips/configs/maltaup_xpa_defconfig    |   1 -
 arch/mips/configs/rb532_defconfig          |   1 -
 arch/powerpc/configs/ppc6xx_defconfig      |   1 -
 arch/s390/configs/debug_defconfig          |   1 -
 arch/s390/configs/defconfig                |   1 -
 arch/sh/configs/titan_defconfig            |   1 -
 include/net/tc_act/tc_ipt.h                |  17 -
 include/net/tc_wrapper.h                   |   4 -
 include/uapi/linux/pkt_cls.h               |   4 +-
 include/uapi/linux/tc_act/tc_ipt.h         |  20 -
 net/sched/Makefile                         |   1 -
 net/sched/act_ipt.c                        | 464 ---------------------
 tools/testing/selftests/tc-testing/config  |   1 -
 tools/testing/selftests/tc-testing/tdc.sh  |   1 -
 18 files changed, 2 insertions(+), 520 deletions(-)
 delete mode 100644 include/net/tc_act/tc_ipt.h
 delete mode 100644 include/uapi/linux/tc_act/tc_ipt.h
 delete mode 100644 net/sched/act_ipt.c

-- 
2.34.1

[PATCH net-next 1/2] net/sched: Retire ipt action

[Jamal Hadi Salim]

The tc ipt action was intended to run all netfilter/iptables target.
Unfortunately it has not benefitted over the years from proper updates when
netfilter changes, and for that reason it has remained rudimentary.
Pinging a bunch of people that i was aware were using this indicates that
removing it wont affect them.
Retire it to reduce maintenance efforts. Buh-bye.

Reviewed-by: Victor Noguiera <victor@mojatatu.com>
Reviewed-by: Pedro Tammela <pctammela@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 include/net/tc_act/tc_ipt.h               |  17 -
 include/net/tc_wrapper.h                  |   4 -
 include/uapi/linux/pkt_cls.h              |   4 +-
 include/uapi/linux/tc_act/tc_ipt.h        |  20 -
 net/sched/Makefile                        |   1 -
 net/sched/act_ipt.c                       | 464 ----------------------
 tools/testing/selftests/tc-testing/config |   1 -
 tools/testing/selftests/tc-testing/tdc.sh |   1 -
 8 files changed, 2 insertions(+), 510 deletions(-)
 delete mode 100644 include/net/tc_act/tc_ipt.h
 delete mode 100644 include/uapi/linux/tc_act/tc_ipt.h
 delete mode 100644 net/sched/act_ipt.c

diff --git a/include/net/tc_act/tc_ipt.h b/include/net/tc_act/tc_ipt.h
deleted file mode 100644
index 4225fcb1c6ba..000000000000
--- a/include/net/tc_act/tc_ipt.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __NET_TC_IPT_H
-#define __NET_TC_IPT_H
-
-#include <net/act_api.h>
-
-struct xt_entry_target;
-
-struct tcf_ipt {
-	struct tc_action	common;
-	u32			tcfi_hook;
-	char			*tcfi_tname;
-	struct xt_entry_target	*tcfi_t;
-};
-#define to_ipt(a) ((struct tcf_ipt *)a)
-
-#endif /* __NET_TC_IPT_H */
diff --git a/include/net/tc_wrapper.h b/include/net/tc_wrapper.h
index a6d481b5bcbc..a608546bcefc 100644
--- a/include/net/tc_wrapper.h
+++ b/include/net/tc_wrapper.h
@@ -117,10 +117,6 @@ static inline int tc_act(struct sk_buff *skb, const struct tc_action *a,
 	if (a->ops->act == tcf_ife_act)
 		return tcf_ife_act(skb, a, res);
 #endif
-#if IS_BUILTIN(CONFIG_NET_ACT_IPT)
-	if (a->ops->act == tcf_ipt_act)
-		return tcf_ipt_act(skb, a, res);
-#endif
 #if IS_BUILTIN(CONFIG_NET_ACT_SIMP)
 	if (a->ops->act == tcf_simp_act)
 		return tcf_simp_act(skb, a, res);
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index c7082cc60d21..2fec9b51d28d 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -99,7 +99,7 @@ enum {
  * versions.
  */
 #define TCA_ACT_GACT 5
-#define TCA_ACT_IPT 6
+#define TCA_ACT_IPT 6 /* obsoleted, can be reused */
 #define TCA_ACT_PEDIT 7
 #define TCA_ACT_MIRRED 8
 #define TCA_ACT_NAT 9
@@ -120,7 +120,7 @@ enum tca_id {
 	TCA_ID_UNSPEC = 0,
 	TCA_ID_POLICE = 1,
 	TCA_ID_GACT = TCA_ACT_GACT,
-	TCA_ID_IPT = TCA_ACT_IPT,
+	TCA_ID_IPT = TCA_ACT_IPT, /* Obsoleted, can be reused */
 	TCA_ID_PEDIT = TCA_ACT_PEDIT,
 	TCA_ID_MIRRED = TCA_ACT_MIRRED,
 	TCA_ID_NAT = TCA_ACT_NAT,
diff --git a/include/uapi/linux/tc_act/tc_ipt.h b/include/uapi/linux/tc_act/tc_ipt.h
deleted file mode 100644
index c48d7da6750d..000000000000
--- a/include/uapi/linux/tc_act/tc_ipt.h
+++ /dev/null
@@ -1,20 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef __LINUX_TC_IPT_H
-#define __LINUX_TC_IPT_H
-
-#include <linux/pkt_cls.h>
-
-enum {
-	TCA_IPT_UNSPEC,
-	TCA_IPT_TABLE,
-	TCA_IPT_HOOK,
-	TCA_IPT_INDEX,
-	TCA_IPT_CNT,
-	TCA_IPT_TM,
-	TCA_IPT_TARG,
-	TCA_IPT_PAD,
-	__TCA_IPT_MAX
-};
-#define TCA_IPT_MAX (__TCA_IPT_MAX - 1)
-                                                                                
-#endif
diff --git a/net/sched/Makefile b/net/sched/Makefile
index b5fd49641d91..82c3f78ca486 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -13,7 +13,6 @@ obj-$(CONFIG_NET_ACT_POLICE)	+= act_police.o
 obj-$(CONFIG_NET_ACT_GACT)	+= act_gact.o
 obj-$(CONFIG_NET_ACT_MIRRED)	+= act_mirred.o
 obj-$(CONFIG_NET_ACT_SAMPLE)	+= act_sample.o
-obj-$(CONFIG_NET_ACT_IPT)	+= act_ipt.o
 obj-$(CONFIG_NET_ACT_NAT)	+= act_nat.o
 obj-$(CONFIG_NET_ACT_PEDIT)	+= act_pedit.o
 obj-$(CONFIG_NET_ACT_SIMP)	+= act_simple.o
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
deleted file mode 100644
index 598d6e299152..000000000000
--- a/net/sched/act_ipt.c
+++ /dev/null
@@ -1,464 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * net/sched/act_ipt.c		iptables target interface
- *
- *TODO: Add other tables. For now we only support the ipv4 table targets
- *
- * Copyright:	Jamal Hadi Salim (2002-13)
- */
-
-#include <linux/types.h>
-#include <linux/kernel.h>
-#include <linux/string.h>
-#include <linux/errno.h>
-#include <linux/skbuff.h>
-#include <linux/rtnetlink.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/slab.h>
-#include <net/netlink.h>
-#include <net/pkt_sched.h>
-#include <linux/tc_act/tc_ipt.h>
-#include <net/tc_act/tc_ipt.h>
-#include <net/tc_wrapper.h>
-#include <net/ip.h>
-
-#include <linux/netfilter_ipv4/ip_tables.h>
-
-
-static struct tc_action_ops act_ipt_ops;
-static struct tc_action_ops act_xt_ops;
-
-static int ipt_init_target(struct net *net, struct xt_entry_target *t,
-			   char *table, unsigned int hook)
-{
-	struct xt_tgchk_param par;
-	struct xt_target *target;
-	struct ipt_entry e = {};
-	int ret = 0;
-
-	target = xt_request_find_target(AF_INET, t->u.user.name,
-					t->u.user.revision);
-	if (IS_ERR(target))
-		return PTR_ERR(target);
-
-	t->u.kernel.target = target;
-	memset(&par, 0, sizeof(par));
-	par.net       = net;
-	par.table     = table;
-	par.entryinfo = &e;
-	par.target    = target;
-	par.targinfo  = t->data;
-	par.hook_mask = 1 << hook;
-	par.family    = NFPROTO_IPV4;
-
-	ret = xt_check_target(&par, t->u.target_size - sizeof(*t), 0, false);
-	if (ret < 0) {
-		module_put(t->u.kernel.target->me);
-		return ret;
-	}
-	return 0;
-}
-
-static void ipt_destroy_target(struct xt_entry_target *t, struct net *net)
-{
-	struct xt_tgdtor_param par = {
-		.target   = t->u.kernel.target,
-		.targinfo = t->data,
-		.family   = NFPROTO_IPV4,
-		.net      = net,
-	};
-	if (par.target->destroy != NULL)
-		par.target->destroy(&par);
-	module_put(par.target->me);
-}
-
-static void tcf_ipt_release(struct tc_action *a)
-{
-	struct tcf_ipt *ipt = to_ipt(a);
-
-	if (ipt->tcfi_t) {
-		ipt_destroy_target(ipt->tcfi_t, a->idrinfo->net);
-		kfree(ipt->tcfi_t);
-	}
-	kfree(ipt->tcfi_tname);
-}
-
-static const struct nla_policy ipt_policy[TCA_IPT_MAX + 1] = {
-	[TCA_IPT_TABLE]	= { .type = NLA_STRING, .len = IFNAMSIZ },
-	[TCA_IPT_HOOK]	= NLA_POLICY_RANGE(NLA_U32, NF_INET_PRE_ROUTING,
-					   NF_INET_NUMHOOKS),
-	[TCA_IPT_INDEX]	= { .type = NLA_U32 },
-	[TCA_IPT_TARG]	= { .len = sizeof(struct xt_entry_target) },
-};
-
-static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
-			  struct nlattr *est, struct tc_action **a,
-			  const struct tc_action_ops *ops,
-			  struct tcf_proto *tp, u32 flags)
-{
-	struct tc_action_net *tn = net_generic(net, id);
-	bool bind = flags & TCA_ACT_FLAGS_BIND;
-	struct nlattr *tb[TCA_IPT_MAX + 1];
-	struct tcf_ipt *ipt;
-	struct xt_entry_target *td, *t;
-	char *tname;
-	bool exists = false;
-	int ret = 0, err;
-	u32 hook = 0;
-	u32 index = 0;
-
-	if (nla == NULL)
-		return -EINVAL;
-
-	err = nla_parse_nested_deprecated(tb, TCA_IPT_MAX, nla, ipt_policy,
-					  NULL);
-	if (err < 0)
-		return err;
-
-	if (tb[TCA_IPT_INDEX] != NULL)
-		index = nla_get_u32(tb[TCA_IPT_INDEX]);
-
-	err = tcf_idr_check_alloc(tn, &index, a, bind);
-	if (err < 0)
-		return err;
-	exists = err;
-	if (exists && bind)
-		return 0;
-
-	if (tb[TCA_IPT_HOOK] == NULL || tb[TCA_IPT_TARG] == NULL) {
-		if (exists)
-			tcf_idr_release(*a, bind);
-		else
-			tcf_idr_cleanup(tn, index);
-		return -EINVAL;
-	}
-
-	td = (struct xt_entry_target *)nla_data(tb[TCA_IPT_TARG]);
-	if (nla_len(tb[TCA_IPT_TARG]) != td->u.target_size) {
-		if (exists)
-			tcf_idr_release(*a, bind);
-		else
-			tcf_idr_cleanup(tn, index);
-		return -EINVAL;
-	}
-
-	if (!exists) {
-		ret = tcf_idr_create(tn, index, est, a, ops, bind,
-				     false, flags);
-		if (ret) {
-			tcf_idr_cleanup(tn, index);
-			return ret;
-		}
-		ret = ACT_P_CREATED;
-	} else {
-		if (bind)/* dont override defaults */
-			return 0;
-
-		if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
-			tcf_idr_release(*a, bind);
-			return -EEXIST;
-		}
-	}
-
-	err = -EINVAL;
-	hook = nla_get_u32(tb[TCA_IPT_HOOK]);
-	switch (hook) {
-	case NF_INET_PRE_ROUTING:
-		break;
-	case NF_INET_POST_ROUTING:
-		break;
-	default:
-		goto err1;
-	}
-
-	if (tb[TCA_IPT_TABLE]) {
-		/* mangle only for now */
-		if (nla_strcmp(tb[TCA_IPT_TABLE], "mangle"))
-			goto err1;
-	}
-
-	tname = kstrdup("mangle", GFP_KERNEL);
-	if (unlikely(!tname))
-		goto err1;
-
-	t = kmemdup(td, td->u.target_size, GFP_KERNEL);
-	if (unlikely(!t))
-		goto err2;
-
-	err = ipt_init_target(net, t, tname, hook);
-	if (err < 0)
-		goto err3;
-
-	ipt = to_ipt(*a);
-
-	spin_lock_bh(&ipt->tcf_lock);
-	if (ret != ACT_P_CREATED) {
-		ipt_destroy_target(ipt->tcfi_t, net);
-		kfree(ipt->tcfi_tname);
-		kfree(ipt->tcfi_t);
-	}
-	ipt->tcfi_tname = tname;
-	ipt->tcfi_t     = t;
-	ipt->tcfi_hook  = hook;
-	spin_unlock_bh(&ipt->tcf_lock);
-	return ret;
-
-err3:
-	kfree(t);
-err2:
-	kfree(tname);
-err1:
-	tcf_idr_release(*a, bind);
-	return err;
-}
-
-static int tcf_ipt_init(struct net *net, struct nlattr *nla,
-			struct nlattr *est, struct tc_action **a,
-			struct tcf_proto *tp,
-			u32 flags, struct netlink_ext_ack *extack)
-{
-	return __tcf_ipt_init(net, act_ipt_ops.net_id, nla, est,
-			      a, &act_ipt_ops, tp, flags);
-}
-
-static int tcf_xt_init(struct net *net, struct nlattr *nla,
-		       struct nlattr *est, struct tc_action **a,
-		       struct tcf_proto *tp,
-		       u32 flags, struct netlink_ext_ack *extack)
-{
-	return __tcf_ipt_init(net, act_xt_ops.net_id, nla, est,
-			      a, &act_xt_ops, tp, flags);
-}
-
-static bool tcf_ipt_act_check(struct sk_buff *skb)
-{
-	const struct iphdr *iph;
-	unsigned int nhoff, len;
-
-	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
-		return false;
-
-	nhoff = skb_network_offset(skb);
-	iph = ip_hdr(skb);
-	if (iph->ihl < 5 || iph->version != 4)
-		return false;
-
-	len = skb_ip_totlen(skb);
-	if (skb->len < nhoff + len || len < (iph->ihl * 4u))
-		return false;
-
-	return pskb_may_pull(skb, iph->ihl * 4u);
-}
-
-TC_INDIRECT_SCOPE int tcf_ipt_act(struct sk_buff *skb,
-				  const struct tc_action *a,
-				  struct tcf_result *res)
-{
-	char saved_cb[sizeof_field(struct sk_buff, cb)];
-	int ret = 0, result = 0;
-	struct tcf_ipt *ipt = to_ipt(a);
-	struct xt_action_param par;
-	struct nf_hook_state state = {
-		.net	= dev_net(skb->dev),
-		.in	= skb->dev,
-		.hook	= ipt->tcfi_hook,
-		.pf	= NFPROTO_IPV4,
-	};
-
-	if (skb_protocol(skb, false) != htons(ETH_P_IP))
-		return TC_ACT_UNSPEC;
-
-	if (skb_unclone(skb, GFP_ATOMIC))
-		return TC_ACT_UNSPEC;
-
-	if (!tcf_ipt_act_check(skb))
-		return TC_ACT_UNSPEC;
-
-	if (state.hook == NF_INET_POST_ROUTING) {
-		if (!skb_dst(skb))
-			return TC_ACT_UNSPEC;
-
-		state.out = skb->dev;
-	}
-
-	memcpy(saved_cb, skb->cb, sizeof(saved_cb));
-
-	spin_lock(&ipt->tcf_lock);
-
-	tcf_lastuse_update(&ipt->tcf_tm);
-	bstats_update(&ipt->tcf_bstats, skb);
-
-	/* yes, we have to worry about both in and out dev
-	 * worry later - danger - this API seems to have changed
-	 * from earlier kernels
-	 */
-	par.state    = &state;
-	par.target   = ipt->tcfi_t->u.kernel.target;
-	par.targinfo = ipt->tcfi_t->data;
-
-	memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
-
-	ret = par.target->target(skb, &par);
-
-	switch (ret) {
-	case NF_ACCEPT:
-		result = TC_ACT_OK;
-		break;
-	case NF_DROP:
-		result = TC_ACT_SHOT;
-		ipt->tcf_qstats.drops++;
-		break;
-	case XT_CONTINUE:
-		result = TC_ACT_PIPE;
-		break;
-	default:
-		net_notice_ratelimited("tc filter: Bogus netfilter code %d assume ACCEPT\n",
-				       ret);
-		result = TC_ACT_OK;
-		break;
-	}
-	spin_unlock(&ipt->tcf_lock);
-
-	memcpy(skb->cb, saved_cb, sizeof(skb->cb));
-
-	return result;
-
-}
-
-static int tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind,
-			int ref)
-{
-	unsigned char *b = skb_tail_pointer(skb);
-	struct tcf_ipt *ipt = to_ipt(a);
-	struct xt_entry_target *t;
-	struct tcf_t tm;
-	struct tc_cnt c;
-
-	/* for simple targets kernel size == user size
-	 * user name = target name
-	 * for foolproof you need to not assume this
-	 */
-
-	spin_lock_bh(&ipt->tcf_lock);
-	t = kmemdup(ipt->tcfi_t, ipt->tcfi_t->u.user.target_size, GFP_ATOMIC);
-	if (unlikely(!t))
-		goto nla_put_failure;
-
-	c.bindcnt = atomic_read(&ipt->tcf_bindcnt) - bind;
-	c.refcnt = refcount_read(&ipt->tcf_refcnt) - ref;
-	strcpy(t->u.user.name, ipt->tcfi_t->u.kernel.target->name);
-
-	if (nla_put(skb, TCA_IPT_TARG, ipt->tcfi_t->u.user.target_size, t) ||
-	    nla_put_u32(skb, TCA_IPT_INDEX, ipt->tcf_index) ||
-	    nla_put_u32(skb, TCA_IPT_HOOK, ipt->tcfi_hook) ||
-	    nla_put(skb, TCA_IPT_CNT, sizeof(struct tc_cnt), &c) ||
-	    nla_put_string(skb, TCA_IPT_TABLE, ipt->tcfi_tname))
-		goto nla_put_failure;
-
-	tcf_tm_dump(&tm, &ipt->tcf_tm);
-	if (nla_put_64bit(skb, TCA_IPT_TM, sizeof(tm), &tm, TCA_IPT_PAD))
-		goto nla_put_failure;
-
-	spin_unlock_bh(&ipt->tcf_lock);
-	kfree(t);
-	return skb->len;
-
-nla_put_failure:
-	spin_unlock_bh(&ipt->tcf_lock);
-	nlmsg_trim(skb, b);
-	kfree(t);
-	return -1;
-}
-
-static struct tc_action_ops act_ipt_ops = {
-	.kind		=	"ipt",
-	.id		=	TCA_ID_IPT,
-	.owner		=	THIS_MODULE,
-	.act		=	tcf_ipt_act,
-	.dump		=	tcf_ipt_dump,
-	.cleanup	=	tcf_ipt_release,
-	.init		=	tcf_ipt_init,
-	.size		=	sizeof(struct tcf_ipt),
-};
-
-static __net_init int ipt_init_net(struct net *net)
-{
-	struct tc_action_net *tn = net_generic(net, act_ipt_ops.net_id);
-
-	return tc_action_net_init(net, tn, &act_ipt_ops);
-}
-
-static void __net_exit ipt_exit_net(struct list_head *net_list)
-{
-	tc_action_net_exit(net_list, act_ipt_ops.net_id);
-}
-
-static struct pernet_operations ipt_net_ops = {
-	.init = ipt_init_net,
-	.exit_batch = ipt_exit_net,
-	.id   = &act_ipt_ops.net_id,
-	.size = sizeof(struct tc_action_net),
-};
-
-static struct tc_action_ops act_xt_ops = {
-	.kind		=	"xt",
-	.id		=	TCA_ID_XT,
-	.owner		=	THIS_MODULE,
-	.act		=	tcf_ipt_act,
-	.dump		=	tcf_ipt_dump,
-	.cleanup	=	tcf_ipt_release,
-	.init		=	tcf_xt_init,
-	.size		=	sizeof(struct tcf_ipt),
-};
-
-static __net_init int xt_init_net(struct net *net)
-{
-	struct tc_action_net *tn = net_generic(net, act_xt_ops.net_id);
-
-	return tc_action_net_init(net, tn, &act_xt_ops);
-}
-
-static void __net_exit xt_exit_net(struct list_head *net_list)
-{
-	tc_action_net_exit(net_list, act_xt_ops.net_id);
-}
-
-static struct pernet_operations xt_net_ops = {
-	.init = xt_init_net,
-	.exit_batch = xt_exit_net,
-	.id   = &act_xt_ops.net_id,
-	.size = sizeof(struct tc_action_net),
-};
-
-MODULE_AUTHOR("Jamal Hadi Salim(2002-13)");
-MODULE_DESCRIPTION("Iptables target actions");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("act_xt");
-
-static int __init ipt_init_module(void)
-{
-	int ret1, ret2;
-
-	ret1 = tcf_register_action(&act_xt_ops, &xt_net_ops);
-	if (ret1 < 0)
-		pr_err("Failed to load xt action\n");
-
-	ret2 = tcf_register_action(&act_ipt_ops, &ipt_net_ops);
-	if (ret2 < 0)
-		pr_err("Failed to load ipt action\n");
-
-	if (ret1 < 0 && ret2 < 0) {
-		return ret1;
-	} else
-		return 0;
-}
-
-static void __exit ipt_cleanup_module(void)
-{
-	tcf_unregister_action(&act_ipt_ops, &ipt_net_ops);
-	tcf_unregister_action(&act_xt_ops, &xt_net_ops);
-}
-
-module_init(ipt_init_module);
-module_exit(ipt_cleanup_module);
diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config
index 012aa33b341b..c60acba951c2 100644
--- a/tools/testing/selftests/tc-testing/config
+++ b/tools/testing/selftests/tc-testing/config
@@ -82,7 +82,6 @@ CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
 CONFIG_NET_ACT_SAMPLE=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_NAT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_NET_ACT_SIMP=m
diff --git a/tools/testing/selftests/tc-testing/tdc.sh b/tools/testing/selftests/tc-testing/tdc.sh
index 407fa53822a0..c53ede8b730d 100755
--- a/tools/testing/selftests/tc-testing/tdc.sh
+++ b/tools/testing/selftests/tc-testing/tdc.sh
@@ -20,7 +20,6 @@ try_modprobe act_ct
 try_modprobe act_ctinfo
 try_modprobe act_gact
 try_modprobe act_gate
-try_modprobe act_ipt
 try_modprobe act_mirred
 try_modprobe act_mpls
 try_modprobe act_nat
-- 
2.34.1

Re: [PATCH net-next 1/2] net/sched: Retire ipt action

[Pedro Tammela]

On 21/12/2023 18:31, Jamal Hadi Salim wrote:
> The tc ipt action was intended to run all netfilter/iptables target.
> Unfortunately it has not benefitted over the years from proper updates when
> netfilter changes, and for that reason it has remained rudimentary.
> Pinging a bunch of people that i was aware were using this indicates that
> removing it wont affect them.
> Retire it to reduce maintenance efforts. Buh-bye.
> 
> Reviewed-by: Victor Noguiera <victor@mojatatu.com>
> Reviewed-by: Pedro Tammela <pctammela@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
>   include/net/tc_act/tc_ipt.h               |  17 -
>   include/net/tc_wrapper.h                  |   4 -
>   include/uapi/linux/pkt_cls.h              |   4 +-
>   include/uapi/linux/tc_act/tc_ipt.h        |  20 -
>   net/sched/Makefile                        |   1 -
>   net/sched/act_ipt.c                       | 464 ----------------------
>   tools/testing/selftests/tc-testing/config |   1 -
>   tools/testing/selftests/tc-testing/tdc.sh |   1 -
>   8 files changed, 2 insertions(+), 510 deletions(-)
>   delete mode 100644 include/net/tc_act/tc_ipt.h
>   delete mode 100644 include/uapi/linux/tc_act/tc_ipt.h
>   delete mode 100644 net/sched/act_ipt.c
> 
> diff --git a/include/net/tc_act/tc_ipt.h b/include/net/tc_act/tc_ipt.h
> deleted file mode 100644
> index 4225fcb1c6ba..000000000000
> --- a/include/net/tc_act/tc_ipt.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __NET_TC_IPT_H
> -#define __NET_TC_IPT_H
> -
> -#include <net/act_api.h>
> -
> -struct xt_entry_target;
> -
> -struct tcf_ipt {
> -	struct tc_action	common;
> -	u32			tcfi_hook;
> -	char			*tcfi_tname;
> -	struct xt_entry_target	*tcfi_t;
> -};
> -#define to_ipt(a) ((struct tcf_ipt *)a)
> -
> -#endif /* __NET_TC_IPT_H */
> diff --git a/include/net/tc_wrapper.h b/include/net/tc_wrapper.h
> index a6d481b5bcbc..a608546bcefc 100644
> --- a/include/net/tc_wrapper.h
> +++ b/include/net/tc_wrapper.h
> @@ -117,10 +117,6 @@ static inline int tc_act(struct sk_buff *skb, const struct tc_action *a,
>   	if (a->ops->act == tcf_ife_act)
>   		return tcf_ife_act(skb, a, res);
>   #endif
> -#if IS_BUILTIN(CONFIG_NET_ACT_IPT)
> -	if (a->ops->act == tcf_ipt_act)
> -		return tcf_ipt_act(skb, a, res);
> -#endif
>   #if IS_BUILTIN(CONFIG_NET_ACT_SIMP)
>   	if (a->ops->act == tcf_simp_act)
>   		return tcf_simp_act(skb, a, res);
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index c7082cc60d21..2fec9b51d28d 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -99,7 +99,7 @@ enum {
>    * versions.
>    */
>   #define TCA_ACT_GACT 5
> -#define TCA_ACT_IPT 6
> +#define TCA_ACT_IPT 6 /* obsoleted, can be reused */
>   #define TCA_ACT_PEDIT 7
>   #define TCA_ACT_MIRRED 8
>   #define TCA_ACT_NAT 9
> @@ -120,7 +120,7 @@ enum tca_id {
>   	TCA_ID_UNSPEC = 0,
>   	TCA_ID_POLICE = 1,
>   	TCA_ID_GACT = TCA_ACT_GACT,
> -	TCA_ID_IPT = TCA_ACT_IPT,
> +	TCA_ID_IPT = TCA_ACT_IPT, /* Obsoleted, can be reused */
>   	TCA_ID_PEDIT = TCA_ACT_PEDIT,
>   	TCA_ID_MIRRED = TCA_ACT_MIRRED,
>   	TCA_ID_NAT = TCA_ACT_NAT,
> diff --git a/include/uapi/linux/tc_act/tc_ipt.h b/include/uapi/linux/tc_act/tc_ipt.h
> deleted file mode 100644
> index c48d7da6750d..000000000000
> --- a/include/uapi/linux/tc_act/tc_ipt.h
> +++ /dev/null
> @@ -1,20 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> -#ifndef __LINUX_TC_IPT_H
> -#define __LINUX_TC_IPT_H
> -
> -#include <linux/pkt_cls.h>
> -
> -enum {
> -	TCA_IPT_UNSPEC,
> -	TCA_IPT_TABLE,
> -	TCA_IPT_HOOK,
> -	TCA_IPT_INDEX,
> -	TCA_IPT_CNT,
> -	TCA_IPT_TM,
> -	TCA_IPT_TARG,
> -	TCA_IPT_PAD,
> -	__TCA_IPT_MAX
> -};
> -#define TCA_IPT_MAX (__TCA_IPT_MAX - 1)
> -
> -#endif

Sorry I missed this, wouldn't this break compilation in userspace?

> diff --git a/net/sched/Makefile b/net/sched/Makefile
> index b5fd49641d91..82c3f78ca486 100644
> --- a/net/sched/Makefile
> +++ b/net/sched/Makefile
> @@ -13,7 +13,6 @@ obj-$(CONFIG_NET_ACT_POLICE)	+= act_police.o
>   obj-$(CONFIG_NET_ACT_GACT)	+= act_gact.o
>   obj-$(CONFIG_NET_ACT_MIRRED)	+= act_mirred.o
>   obj-$(CONFIG_NET_ACT_SAMPLE)	+= act_sample.o
> -obj-$(CONFIG_NET_ACT_IPT)	+= act_ipt.o
>   obj-$(CONFIG_NET_ACT_NAT)	+= act_nat.o
>   obj-$(CONFIG_NET_ACT_PEDIT)	+= act_pedit.o
>   obj-$(CONFIG_NET_ACT_SIMP)	+= act_simple.o
> diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
> deleted file mode 100644
> index 598d6e299152..000000000000
> --- a/net/sched/act_ipt.c
> +++ /dev/null
> @@ -1,464 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - * net/sched/act_ipt.c		iptables target interface
> - *
> - *TODO: Add other tables. For now we only support the ipv4 table targets
> - *
> - * Copyright:	Jamal Hadi Salim (2002-13)
> - */
> -
> -#include <linux/types.h>
> -#include <linux/kernel.h>
> -#include <linux/string.h>
> -#include <linux/errno.h>
> -#include <linux/skbuff.h>
> -#include <linux/rtnetlink.h>
> -#include <linux/module.h>
> -#include <linux/init.h>
> -#include <linux/slab.h>
> -#include <net/netlink.h>
> -#include <net/pkt_sched.h>
> -#include <linux/tc_act/tc_ipt.h>
> -#include <net/tc_act/tc_ipt.h>
> -#include <net/tc_wrapper.h>
> -#include <net/ip.h>
> -
> -#include <linux/netfilter_ipv4/ip_tables.h>
> -
> -
> -static struct tc_action_ops act_ipt_ops;
> -static struct tc_action_ops act_xt_ops;
> -
> -static int ipt_init_target(struct net *net, struct xt_entry_target *t,
> -			   char *table, unsigned int hook)
> -{
> -	struct xt_tgchk_param par;
> -	struct xt_target *target;
> -	struct ipt_entry e = {};
> -	int ret = 0;
> -
> -	target = xt_request_find_target(AF_INET, t->u.user.name,
> -					t->u.user.revision);
> -	if (IS_ERR(target))
> -		return PTR_ERR(target);
> -
> -	t->u.kernel.target = target;
> -	memset(&par, 0, sizeof(par));
> -	par.net       = net;
> -	par.table     = table;
> -	par.entryinfo = &e;
> -	par.target    = target;
> -	par.targinfo  = t->data;
> -	par.hook_mask = 1 << hook;
> -	par.family    = NFPROTO_IPV4;
> -
> -	ret = xt_check_target(&par, t->u.target_size - sizeof(*t), 0, false);
> -	if (ret < 0) {
> -		module_put(t->u.kernel.target->me);
> -		return ret;
> -	}
> -	return 0;
> -}
> -
> -static void ipt_destroy_target(struct xt_entry_target *t, struct net *net)
> -{
> -	struct xt_tgdtor_param par = {
> -		.target   = t->u.kernel.target,
> -		.targinfo = t->data,
> -		.family   = NFPROTO_IPV4,
> -		.net      = net,
> -	};
> -	if (par.target->destroy != NULL)
> -		par.target->destroy(&par);
> -	module_put(par.target->me);
> -}
> -
> -static void tcf_ipt_release(struct tc_action *a)
> -{
> -	struct tcf_ipt *ipt = to_ipt(a);
> -
> -	if (ipt->tcfi_t) {
> -		ipt_destroy_target(ipt->tcfi_t, a->idrinfo->net);
> -		kfree(ipt->tcfi_t);
> -	}
> -	kfree(ipt->tcfi_tname);
> -}
> -
> -static const struct nla_policy ipt_policy[TCA_IPT_MAX + 1] = {
> -	[TCA_IPT_TABLE]	= { .type = NLA_STRING, .len = IFNAMSIZ },
> -	[TCA_IPT_HOOK]	= NLA_POLICY_RANGE(NLA_U32, NF_INET_PRE_ROUTING,
> -					   NF_INET_NUMHOOKS),
> -	[TCA_IPT_INDEX]	= { .type = NLA_U32 },
> -	[TCA_IPT_TARG]	= { .len = sizeof(struct xt_entry_target) },
> -};
> -
> -static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
> -			  struct nlattr *est, struct tc_action **a,
> -			  const struct tc_action_ops *ops,
> -			  struct tcf_proto *tp, u32 flags)
> -{
> -	struct tc_action_net *tn = net_generic(net, id);
> -	bool bind = flags & TCA_ACT_FLAGS_BIND;
> -	struct nlattr *tb[TCA_IPT_MAX + 1];
> -	struct tcf_ipt *ipt;
> -	struct xt_entry_target *td, *t;
> -	char *tname;
> -	bool exists = false;
> -	int ret = 0, err;
> -	u32 hook = 0;
> -	u32 index = 0;
> -
> -	if (nla == NULL)
> -		return -EINVAL;
> -
> -	err = nla_parse_nested_deprecated(tb, TCA_IPT_MAX, nla, ipt_policy,
> -					  NULL);
> -	if (err < 0)
> -		return err;
> -
> -	if (tb[TCA_IPT_INDEX] != NULL)
> -		index = nla_get_u32(tb[TCA_IPT_INDEX]);
> -
> -	err = tcf_idr_check_alloc(tn, &index, a, bind);
> -	if (err < 0)
> -		return err;
> -	exists = err;
> -	if (exists && bind)
> -		return 0;
> -
> -	if (tb[TCA_IPT_HOOK] == NULL || tb[TCA_IPT_TARG] == NULL) {
> -		if (exists)
> -			tcf_idr_release(*a, bind);
> -		else
> -			tcf_idr_cleanup(tn, index);
> -		return -EINVAL;
> -	}
> -
> -	td = (struct xt_entry_target *)nla_data(tb[TCA_IPT_TARG]);
> -	if (nla_len(tb[TCA_IPT_TARG]) != td->u.target_size) {
> -		if (exists)
> -			tcf_idr_release(*a, bind);
> -		else
> -			tcf_idr_cleanup(tn, index);
> -		return -EINVAL;
> -	}
> -
> -	if (!exists) {
> -		ret = tcf_idr_create(tn, index, est, a, ops, bind,
> -				     false, flags);
> -		if (ret) {
> -			tcf_idr_cleanup(tn, index);
> -			return ret;
> -		}
> -		ret = ACT_P_CREATED;
> -	} else {
> -		if (bind)/* dont override defaults */
> -			return 0;
> -
> -		if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
> -			tcf_idr_release(*a, bind);
> -			return -EEXIST;
> -		}
> -	}
> -
> -	err = -EINVAL;
> -	hook = nla_get_u32(tb[TCA_IPT_HOOK]);
> -	switch (hook) {
> -	case NF_INET_PRE_ROUTING:
> -		break;
> -	case NF_INET_POST_ROUTING:
> -		break;
> -	default:
> -		goto err1;
> -	}
> -
> -	if (tb[TCA_IPT_TABLE]) {
> -		/* mangle only for now */
> -		if (nla_strcmp(tb[TCA_IPT_TABLE], "mangle"))
> -			goto err1;
> -	}
> -
> -	tname = kstrdup("mangle", GFP_KERNEL);
> -	if (unlikely(!tname))
> -		goto err1;
> -
> -	t = kmemdup(td, td->u.target_size, GFP_KERNEL);
> -	if (unlikely(!t))
> -		goto err2;
> -
> -	err = ipt_init_target(net, t, tname, hook);
> -	if (err < 0)
> -		goto err3;
> -
> -	ipt = to_ipt(*a);
> -
> -	spin_lock_bh(&ipt->tcf_lock);
> -	if (ret != ACT_P_CREATED) {
> -		ipt_destroy_target(ipt->tcfi_t, net);
> -		kfree(ipt->tcfi_tname);
> -		kfree(ipt->tcfi_t);
> -	}
> -	ipt->tcfi_tname = tname;
> -	ipt->tcfi_t     = t;
> -	ipt->tcfi_hook  = hook;
> -	spin_unlock_bh(&ipt->tcf_lock);
> -	return ret;
> -
> -err3:
> -	kfree(t);
> -err2:
> -	kfree(tname);
> -err1:
> -	tcf_idr_release(*a, bind);
> -	return err;
> -}
> -
> -static int tcf_ipt_init(struct net *net, struct nlattr *nla,
> -			struct nlattr *est, struct tc_action **a,
> -			struct tcf_proto *tp,
> -			u32 flags, struct netlink_ext_ack *extack)
> -{
> -	return __tcf_ipt_init(net, act_ipt_ops.net_id, nla, est,
> -			      a, &act_ipt_ops, tp, flags);
> -}
> -
> -static int tcf_xt_init(struct net *net, struct nlattr *nla,
> -		       struct nlattr *est, struct tc_action **a,
> -		       struct tcf_proto *tp,
> -		       u32 flags, struct netlink_ext_ack *extack)
> -{
> -	return __tcf_ipt_init(net, act_xt_ops.net_id, nla, est,
> -			      a, &act_xt_ops, tp, flags);
> -}
> -
> -static bool tcf_ipt_act_check(struct sk_buff *skb)
> -{
> -	const struct iphdr *iph;
> -	unsigned int nhoff, len;
> -
> -	if (!pskb_may_pull(skb, sizeof(struct iphdr)))
> -		return false;
> -
> -	nhoff = skb_network_offset(skb);
> -	iph = ip_hdr(skb);
> -	if (iph->ihl < 5 || iph->version != 4)
> -		return false;
> -
> -	len = skb_ip_totlen(skb);
> -	if (skb->len < nhoff + len || len < (iph->ihl * 4u))
> -		return false;
> -
> -	return pskb_may_pull(skb, iph->ihl * 4u);
> -}
> -
> -TC_INDIRECT_SCOPE int tcf_ipt_act(struct sk_buff *skb,
> -				  const struct tc_action *a,
> -				  struct tcf_result *res)
> -{
> -	char saved_cb[sizeof_field(struct sk_buff, cb)];
> -	int ret = 0, result = 0;
> -	struct tcf_ipt *ipt = to_ipt(a);
> -	struct xt_action_param par;
> -	struct nf_hook_state state = {
> -		.net	= dev_net(skb->dev),
> -		.in	= skb->dev,
> -		.hook	= ipt->tcfi_hook,
> -		.pf	= NFPROTO_IPV4,
> -	};
> -
> -	if (skb_protocol(skb, false) != htons(ETH_P_IP))
> -		return TC_ACT_UNSPEC;
> -
> -	if (skb_unclone(skb, GFP_ATOMIC))
> -		return TC_ACT_UNSPEC;
> -
> -	if (!tcf_ipt_act_check(skb))
> -		return TC_ACT_UNSPEC;
> -
> -	if (state.hook == NF_INET_POST_ROUTING) {
> -		if (!skb_dst(skb))
> -			return TC_ACT_UNSPEC;
> -
> -		state.out = skb->dev;
> -	}
> -
> -	memcpy(saved_cb, skb->cb, sizeof(saved_cb));
> -
> -	spin_lock(&ipt->tcf_lock);
> -
> -	tcf_lastuse_update(&ipt->tcf_tm);
> -	bstats_update(&ipt->tcf_bstats, skb);
> -
> -	/* yes, we have to worry about both in and out dev
> -	 * worry later - danger - this API seems to have changed
> -	 * from earlier kernels
> -	 */
> -	par.state    = &state;
> -	par.target   = ipt->tcfi_t->u.kernel.target;
> -	par.targinfo = ipt->tcfi_t->data;
> -
> -	memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));
> -
> -	ret = par.target->target(skb, &par);
> -
> -	switch (ret) {
> -	case NF_ACCEPT:
> -		result = TC_ACT_OK;
> -		break;
> -	case NF_DROP:
> -		result = TC_ACT_SHOT;
> -		ipt->tcf_qstats.drops++;
> -		break;
> -	case XT_CONTINUE:
> -		result = TC_ACT_PIPE;
> -		break;
> -	default:
> -		net_notice_ratelimited("tc filter: Bogus netfilter code %d assume ACCEPT\n",
> -				       ret);
> -		result = TC_ACT_OK;
> -		break;
> -	}
> -	spin_unlock(&ipt->tcf_lock);
> -
> -	memcpy(skb->cb, saved_cb, sizeof(skb->cb));
> -
> -	return result;
> -
> -}
> -
> -static int tcf_ipt_dump(struct sk_buff *skb, struct tc_action *a, int bind,
> -			int ref)
> -{
> -	unsigned char *b = skb_tail_pointer(skb);
> -	struct tcf_ipt *ipt = to_ipt(a);
> -	struct xt_entry_target *t;
> -	struct tcf_t tm;
> -	struct tc_cnt c;
> -
> -	/* for simple targets kernel size == user size
> -	 * user name = target name
> -	 * for foolproof you need to not assume this
> -	 */
> -
> -	spin_lock_bh(&ipt->tcf_lock);
> -	t = kmemdup(ipt->tcfi_t, ipt->tcfi_t->u.user.target_size, GFP_ATOMIC);
> -	if (unlikely(!t))
> -		goto nla_put_failure;
> -
> -	c.bindcnt = atomic_read(&ipt->tcf_bindcnt) - bind;
> -	c.refcnt = refcount_read(&ipt->tcf_refcnt) - ref;
> -	strcpy(t->u.user.name, ipt->tcfi_t->u.kernel.target->name);
> -
> -	if (nla_put(skb, TCA_IPT_TARG, ipt->tcfi_t->u.user.target_size, t) ||
> -	    nla_put_u32(skb, TCA_IPT_INDEX, ipt->tcf_index) ||
> -	    nla_put_u32(skb, TCA_IPT_HOOK, ipt->tcfi_hook) ||
> -	    nla_put(skb, TCA_IPT_CNT, sizeof(struct tc_cnt), &c) ||
> -	    nla_put_string(skb, TCA_IPT_TABLE, ipt->tcfi_tname))
> -		goto nla_put_failure;
> -
> -	tcf_tm_dump(&tm, &ipt->tcf_tm);
> -	if (nla_put_64bit(skb, TCA_IPT_TM, sizeof(tm), &tm, TCA_IPT_PAD))
> -		goto nla_put_failure;
> -
> -	spin_unlock_bh(&ipt->tcf_lock);
> -	kfree(t);
> -	return skb->len;
> -
> -nla_put_failure:
> -	spin_unlock_bh(&ipt->tcf_lock);
> -	nlmsg_trim(skb, b);
> -	kfree(t);
> -	return -1;
> -}
> -
> -static struct tc_action_ops act_ipt_ops = {
> -	.kind		=	"ipt",
> -	.id		=	TCA_ID_IPT,
> -	.owner		=	THIS_MODULE,
> -	.act		=	tcf_ipt_act,
> -	.dump		=	tcf_ipt_dump,
> -	.cleanup	=	tcf_ipt_release,
> -	.init		=	tcf_ipt_init,
> -	.size		=	sizeof(struct tcf_ipt),
> -};
> -
> -static __net_init int ipt_init_net(struct net *net)
> -{
> -	struct tc_action_net *tn = net_generic(net, act_ipt_ops.net_id);
> -
> -	return tc_action_net_init(net, tn, &act_ipt_ops);
> -}
> -
> -static void __net_exit ipt_exit_net(struct list_head *net_list)
> -{
> -	tc_action_net_exit(net_list, act_ipt_ops.net_id);
> -}
> -
> -static struct pernet_operations ipt_net_ops = {
> -	.init = ipt_init_net,
> -	.exit_batch = ipt_exit_net,
> -	.id   = &act_ipt_ops.net_id,
> -	.size = sizeof(struct tc_action_net),
> -};
> -
> -static struct tc_action_ops act_xt_ops = {
> -	.kind		=	"xt",
> -	.id		=	TCA_ID_XT,
> -	.owner		=	THIS_MODULE,
> -	.act		=	tcf_ipt_act,
> -	.dump		=	tcf_ipt_dump,
> -	.cleanup	=	tcf_ipt_release,
> -	.init		=	tcf_xt_init,
> -	.size		=	sizeof(struct tcf_ipt),
> -};
> -
> -static __net_init int xt_init_net(struct net *net)
> -{
> -	struct tc_action_net *tn = net_generic(net, act_xt_ops.net_id);
> -
> -	return tc_action_net_init(net, tn, &act_xt_ops);
> -}
> -
> -static void __net_exit xt_exit_net(struct list_head *net_list)
> -{
> -	tc_action_net_exit(net_list, act_xt_ops.net_id);
> -}
> -
> -static struct pernet_operations xt_net_ops = {
> -	.init = xt_init_net,
> -	.exit_batch = xt_exit_net,
> -	.id   = &act_xt_ops.net_id,
> -	.size = sizeof(struct tc_action_net),
> -};
> -
> -MODULE_AUTHOR("Jamal Hadi Salim(2002-13)");
> -MODULE_DESCRIPTION("Iptables target actions");
> -MODULE_LICENSE("GPL");
> -MODULE_ALIAS("act_xt");
> -
> -static int __init ipt_init_module(void)
> -{
> -	int ret1, ret2;
> -
> -	ret1 = tcf_register_action(&act_xt_ops, &xt_net_ops);
> -	if (ret1 < 0)
> -		pr_err("Failed to load xt action\n");
> -
> -	ret2 = tcf_register_action(&act_ipt_ops, &ipt_net_ops);
> -	if (ret2 < 0)
> -		pr_err("Failed to load ipt action\n");
> -
> -	if (ret1 < 0 && ret2 < 0) {
> -		return ret1;
> -	} else
> -		return 0;
> -}
> -
> -static void __exit ipt_cleanup_module(void)
> -{
> -	tcf_unregister_action(&act_ipt_ops, &ipt_net_ops);
> -	tcf_unregister_action(&act_xt_ops, &xt_net_ops);
> -}
> -
> -module_init(ipt_init_module);
> -module_exit(ipt_cleanup_module);
> diff --git a/tools/testing/selftests/tc-testing/config b/tools/testing/selftests/tc-testing/config
> index 012aa33b341b..c60acba951c2 100644
> --- a/tools/testing/selftests/tc-testing/config
> +++ b/tools/testing/selftests/tc-testing/config
> @@ -82,7 +82,6 @@ CONFIG_NET_ACT_GACT=m
>   CONFIG_GACT_PROB=y
>   CONFIG_NET_ACT_MIRRED=m
>   CONFIG_NET_ACT_SAMPLE=m
> -CONFIG_NET_ACT_IPT=m
>   CONFIG_NET_ACT_NAT=m
>   CONFIG_NET_ACT_PEDIT=m
>   CONFIG_NET_ACT_SIMP=m
> diff --git a/tools/testing/selftests/tc-testing/tdc.sh b/tools/testing/selftests/tc-testing/tdc.sh
> index 407fa53822a0..c53ede8b730d 100755
> --- a/tools/testing/selftests/tc-testing/tdc.sh
> +++ b/tools/testing/selftests/tc-testing/tdc.sh
> @@ -20,7 +20,6 @@ try_modprobe act_ct
>   try_modprobe act_ctinfo
>   try_modprobe act_gact
>   try_modprobe act_gate
> -try_modprobe act_ipt
>   try_modprobe act_mirred
>   try_modprobe act_mpls
>   try_modprobe act_nat

Re: [PATCH net-next 1/2] net/sched: Retire ipt action

[Stephen Hemminger]

On Thu, 21 Dec 2023 18:38:59 -0300
Pedro Tammela <pctammela@mojatatu.com> wrote:

> On 21/12/2023 18:31, Jamal Hadi Salim wrote:
> > The tc ipt action was intended to run all netfilter/iptables target.
> > Unfortunately it has not benefitted over the years from proper updates when
> > netfilter changes, and for that reason it has remained rudimentary.
> > Pinging a bunch of people that i was aware were using this indicates that
> > removing it wont affect them.
> > Retire it to reduce maintenance efforts. Buh-bye.
> > 
> > Reviewed-by: Victor Noguiera <victor@mojatatu.com>
> > Reviewed-by: Pedro Tammela <pctammela@mojatatu.com>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > ---

...

> > diff --git a/include/uapi/linux/tc_act/tc_ipt.h b/include/uapi/linux/tc_act/tc_ipt.h
> > deleted file mode 100644
> > index c48d7da6750d..000000000000
> > --- a/include/uapi/linux/tc_act/tc_ipt.h
> > +++ /dev/null
> > @@ -1,20 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > -#ifndef __LINUX_TC_IPT_H
> > -#define __LINUX_TC_IPT_H
> > -
> > -#include <linux/pkt_cls.h>
> > -
> > -enum {
> > -	TCA_IPT_UNSPEC,
> > -	TCA_IPT_TABLE,
> > -	TCA_IPT_HOOK,
> > -	TCA_IPT_INDEX,
> > -	TCA_IPT_CNT,
> > -	TCA_IPT_TM,
> > -	TCA_IPT_TARG,
> > -	TCA_IPT_PAD,
> > -	__TCA_IPT_MAX
> > -};
> > -#define TCA_IPT_MAX (__TCA_IPT_MAX - 1)
> > -
> > -#endif  
> 
> Sorry I missed this, wouldn't this break compilation in userspace?

Yes, it breaks iproute2 build if tc_ipt.h is removed.

Re: [PATCH net-next 1/2] net/sched: Retire ipt action

[David Ahern]

On 12/21/23 6:19 PM, Stephen Hemminger wrote:
> 
> Yes, it breaks iproute2 build if tc_ipt.h is removed.

iproute2 header sync would need to remove it. It only breaks apps that
do not import uapi files from the kernel.

Re: [PATCH net-next 1/2] net/sched: Retire ipt action

[Stephen Hemminger]

On Thu, 21 Dec 2023 19:02:40 -0700
David Ahern <dsahern@gmail.com> wrote:

> On 12/21/23 6:19 PM, Stephen Hemminger wrote:
> > 
> > Yes, it breaks iproute2 build if tc_ipt.h is removed.  
> 
> iproute2 header sync would need to remove it. It only breaks apps that
> do not import uapi files from the kernel.

The problem is that when tc_ipt.h is removed, there are defines still used.
Will need to coordinate removal of ipt support in iproute2 at same time.

Re: [PATCH net-next 1/2] net/sched: Retire ipt action

[Jamal Hadi Salim]

On Thu, Dec 21, 2023 at 10:52 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Thu, 21 Dec 2023 19:02:40 -0700
> David Ahern <dsahern@gmail.com> wrote:
>
> > On 12/21/23 6:19 PM, Stephen Hemminger wrote:
> > >
> > > Yes, it breaks iproute2 build if tc_ipt.h is removed.
> >
> > iproute2 header sync would need to remove it. It only breaks apps that
> > do not import uapi files from the kernel.
>
> The problem is that when tc_ipt.h is removed, there are defines still used.
> Will need to coordinate removal of ipt support in iproute2 at same time.

And per our discussion at netdevconf, you'll take care of that i.e you
didnt want any patches for the sync.
I didnt didnt delete the uapi for the other qdiscs + classifiers i
removed earlier (which i notice are still in iproute2), so, I am going
to send a bunch more patches to remove the headers from the kernel.

cheers,
jamal

[PATCH net-next 2/2] net/sched: Remove CONFIG_NET_ACT_IPT from default configs

[Jamal Hadi Salim]

Now that we are retiring the IPT action.

Reviewed-by: Victor Noguiera <victor@mojatatu.com>
Reviewed-by: Pedro Tammela <pctammela@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 arch/loongarch/configs/loongson3_defconfig | 1 -
 arch/mips/configs/ip22_defconfig           | 1 -
 arch/mips/configs/malta_defconfig          | 1 -
 arch/mips/configs/malta_kvm_defconfig      | 1 -
 arch/mips/configs/maltaup_xpa_defconfig    | 1 -
 arch/mips/configs/rb532_defconfig          | 1 -
 arch/powerpc/configs/ppc6xx_defconfig      | 1 -
 arch/s390/configs/debug_defconfig          | 1 -
 arch/s390/configs/defconfig                | 1 -
 arch/sh/configs/titan_defconfig            | 1 -
 10 files changed, 10 deletions(-)

diff --git a/arch/loongarch/configs/loongson3_defconfig b/arch/loongarch/configs/loongson3_defconfig
index 33795e4a5bd6..9c333d133c30 100644
--- a/arch/loongarch/configs/loongson3_defconfig
+++ b/arch/loongarch/configs/loongson3_defconfig
@@ -304,7 +304,6 @@ CONFIG_NET_CLS_ACT=y
 CONFIG_NET_ACT_POLICE=m
 CONFIG_NET_ACT_GACT=m
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_NAT=m
 CONFIG_NET_ACT_BPF=m
 CONFIG_OPENVSWITCH=m
diff --git a/arch/mips/configs/ip22_defconfig b/arch/mips/configs/ip22_defconfig
index dc49b09d492b..e22e8b825903 100644
--- a/arch/mips/configs/ip22_defconfig
+++ b/arch/mips/configs/ip22_defconfig
@@ -173,7 +173,6 @@ CONFIG_NET_ACT_POLICE=y
 CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_NAT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_NET_ACT_SIMP=m
diff --git a/arch/mips/configs/malta_defconfig b/arch/mips/configs/malta_defconfig
index 6f8046024557..4390d30206d9 100644
--- a/arch/mips/configs/malta_defconfig
+++ b/arch/mips/configs/malta_defconfig
@@ -202,7 +202,6 @@ CONFIG_NET_ACT_POLICE=y
 CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_NAT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_NET_ACT_SIMP=m
diff --git a/arch/mips/configs/malta_kvm_defconfig b/arch/mips/configs/malta_kvm_defconfig
index 16a91eeff67f..d63d8be8cb50 100644
--- a/arch/mips/configs/malta_kvm_defconfig
+++ b/arch/mips/configs/malta_kvm_defconfig
@@ -206,7 +206,6 @@ CONFIG_NET_ACT_POLICE=y
 CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_NAT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_NET_ACT_SIMP=m
diff --git a/arch/mips/configs/maltaup_xpa_defconfig b/arch/mips/configs/maltaup_xpa_defconfig
index 264aba29ea4f..338bb6544a93 100644
--- a/arch/mips/configs/maltaup_xpa_defconfig
+++ b/arch/mips/configs/maltaup_xpa_defconfig
@@ -203,7 +203,6 @@ CONFIG_NET_ACT_POLICE=y
 CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_NAT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_NET_ACT_SIMP=m
diff --git a/arch/mips/configs/rb532_defconfig b/arch/mips/configs/rb532_defconfig
index 02ec6c1a5116..517f1b060bf4 100644
--- a/arch/mips/configs/rb532_defconfig
+++ b/arch/mips/configs/rb532_defconfig
@@ -96,7 +96,6 @@ CONFIG_NET_ACT_POLICE=y
 CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_HAMRADIO=y
 CONFIG_MTD=y
diff --git a/arch/powerpc/configs/ppc6xx_defconfig b/arch/powerpc/configs/ppc6xx_defconfig
index f279703425d4..66c7b28d7450 100644
--- a/arch/powerpc/configs/ppc6xx_defconfig
+++ b/arch/powerpc/configs/ppc6xx_defconfig
@@ -274,7 +274,6 @@ CONFIG_NET_ACT_POLICE=m
 CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_NAT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_NET_ACT_SIMP=m
diff --git a/arch/s390/configs/debug_defconfig b/arch/s390/configs/debug_defconfig
index 438cd92e6080..d1c895785b6f 100644
--- a/arch/s390/configs/debug_defconfig
+++ b/arch/s390/configs/debug_defconfig
@@ -374,7 +374,6 @@ CONFIG_NET_ACT_POLICE=m
 CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_NAT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_NET_ACT_SIMP=m
diff --git a/arch/s390/configs/defconfig b/arch/s390/configs/defconfig
index 1b8150e50f6a..8bc34d4474cb 100644
--- a/arch/s390/configs/defconfig
+++ b/arch/s390/configs/defconfig
@@ -364,7 +364,6 @@ CONFIG_NET_ACT_POLICE=m
 CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_NAT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_NET_ACT_SIMP=m
diff --git a/arch/sh/configs/titan_defconfig b/arch/sh/configs/titan_defconfig
index 871092753591..c1032559ecd4 100644
--- a/arch/sh/configs/titan_defconfig
+++ b/arch/sh/configs/titan_defconfig
@@ -138,7 +138,6 @@ CONFIG_NET_ACT_POLICE=m
 CONFIG_NET_ACT_GACT=m
 CONFIG_GACT_PROB=y
 CONFIG_NET_ACT_MIRRED=m
-CONFIG_NET_ACT_IPT=m
 CONFIG_NET_ACT_PEDIT=m
 CONFIG_FW_LOADER=m
 CONFIG_CONNECTOR=m
-- 
2.34.1

Re: [PATCH net-next 0/2] net/sched: retire tc ipt action

[Florian Westphal]

Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> In keeping up with my status as a hero who removes code: another one bites the
> dust.
> The tc ipt action was intended to run all netfilter/iptables target.
> Unfortunately it has not benefitted over the years from proper updates when
> netfilter changes, and for that reason it has remained rudimentary.
> Pinging a bunch of people that i was aware were using this indicates that
> removing it wont affect them.
> Retire it to reduce maintenance efforts.
> So Long, ipt, and Thanks for all the Fish.

Thanks Jamal for following up on this!

Acked-by: Florian Westphal <fw@strlen.de>

Re: [PATCH net-next 0/2] net/sched: retire tc ipt action

[David Ahern]

On 12/21/23 2:31 PM, Jamal Hadi Salim wrote:
> In keeping up with my status as a hero who removes code: another one bites the
> dust.
> The tc ipt action was intended to run all netfilter/iptables target.
> Unfortunately it has not benefitted over the years from proper updates when
> netfilter changes, and for that reason it has remained rudimentary.
> Pinging a bunch of people that i was aware were using this indicates that
> removing it wont affect them.
> Retire it to reduce maintenance efforts.
> So Long, ipt, and Thanks for all the Fish.
> 
> Jamal Hadi Salim (2):
>   net/sched: Retire ipt action
>   net/sched: Remove CONFIG_NET_ACT_IPT from default configs
> 

Acked-by: David Ahern <dsahern@kernel.org>

Re: [PATCH net-next 0/2] net/sched: retire tc ipt action

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 21 Dec 2023 16:31:02 -0500 you wrote:
> In keeping up with my status as a hero who removes code: another one bites the
> dust.
> The tc ipt action was intended to run all netfilter/iptables target.
> Unfortunately it has not benefitted over the years from proper updates when
> netfilter changes, and for that reason it has remained rudimentary.
> Pinging a bunch of people that i was aware were using this indicates that
> removing it wont affect them.
> Retire it to reduce maintenance efforts.
> So Long, ipt, and Thanks for all the Fish.
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] net/sched: Retire ipt action
    https://git.kernel.org/netdev/net-next/c/ba24ea129126
  - [net-next,2/2] net/sched: Remove CONFIG_NET_ACT_IPT from default configs
    https://git.kernel.org/netdev/net-next/c/6d6d80e4f6bc

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html