act_cpu: packet distributing

I want to know if I can assign the sk to skb as nf_tproxy_core does to avoid the duplicate search later. Thanks.

act_cpu: packet distributing

This TC action can be used to redirect packets to the CPU:
 * specified by the cpuid option
 * specified by the class minor ID obtained previously
 * on which the corresponding application runs

It supports the similar functions of RPS and RFS, but is more flexible.

Signed-off-by Changli Gao <[email protected]>

I want to know if I can assign the sk to skb as nf_tproxy_core does to avoid the duplicate search later. Thanks.

act_cpu: packet distributing

This TC action can be used to redirect packets to the CPU:
* specified by the cpuid option
* specified by the class minor ID obtained previously
* on which the corresponding application runs

It supports the similar functions of RPS and RFS, but is more flexible.

Signed-off-by Changli Gao <[email protected]>

----

 include/linux/netdevice.h     |    2 
 include/linux/tc_act/tc_cpu.h |   31 +++++
 include/net/sock.h            |   24 +++
 include/net/tc_act/tc_cpu.h   |   18 ++
 net/core/dev.c                |    4 
 net/core/sock.c               |    1 
 net/sched/Kconfig             |   12 +
 net/sched/Makefile            |    1 
 net/sched/act_cpu.c           |  260 ++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 350 insertions(+), 3 deletions(-)

include/linux/netdevice.h +2 -0

index c4fedf0..318d422 100644
@@ -1435,6 +1435,8 @@ static inline void input_queue_tail_incr_save(struct softnet_data *sd,
 
 DECLARE_PER_CPU_ALIGNED(struct softnet_data, softnet_data);
 
+int enqueue_to_backlog(struct sk_buff *skb, int cpu, unsigned int *qtail);
+
 #define HAVE_NETIF_QUEUE
 
 extern void __netif_schedule(struct Qdisc *q);

include/linux/tc_act/tc_cpu.h new file +31 -0

new file mode 100644
index 0000000..2704607
@@ -0,0 +1,31 @@
+#ifndef __LINUX_TC_CPU_H
+#define __LINUX_TC_CPU_H
+
+#include <linux/pkt_cls.h>
+#include <linux/types.h>
+
+#define TCA_ACT_CPU 12
+
+enum {
+	TCA_CPU_UNSPEC,
+	TCA_CPU_PARMS,
+	TCA_CPU_TM,
+	__TCA_CPU_MAX
+};
+#define TCA_CPU_MAX (__TCA_CPU_MAX - 1)
+
+enum {
+	TCA_CPU_TYPE_MAP,
+	TCA_CPU_TYPE_CPUID,
+	TCA_CPU_TYPE_SOCKET,
+	__TCA_CPU_TYPE_MAX
+};
+#define TCA_CPU_TYPE_MAX (__TCA_CPU_TYPE_MAX - 1)
+
+struct tc_cpu {
+	tc_gen;
+	__u32		type;
+	__u32		value;
+};
+
+#endif /* __LINUX_TC_CPU_H */

include/net/sock.h +8 -1

index 3100e71..7913158 100644
@@ -200,6 +200,7 @@ struct sock_common {
   *	@sk_rcvtimeo: %SO_RCVTIMEO setting
   *	@sk_sndtimeo: %SO_SNDTIMEO setting
   *	@sk_rxhash: flow hash received from netif layer
+  *	@sk_cpu: the CPU on which the corresponding process works.
   *	@sk_filter: socket filtering instructions
   *	@sk_protinfo: private area, net family specific, when not using slab
   *	@sk_timer: sock cleanup timer

@@ -284,6 +285,9 @@ struct sock {
 #ifdef CONFIG_RPS
 	__u32			sk_rxhash;
 #endif
+#if defined(CONFIG_NET_ACT_CPU) || defined(CONFIG_NET_ACT_CPU_MODULE)
+	int			sk_cpu;
+#endif
 	unsigned long 		sk_flags;
 	unsigned long	        sk_lingertime;
 	struct sk_buff_head	sk_error_queue;

@@ -639,7 +643,24 @@ static inline int sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
 	return sk->sk_backlog_rcv(sk, skb);
 }
 
-static inline void sock_rps_record_flow(const struct sock *sk)
+static inline void sock_reset_cpu(struct sock *sk)
+{
+#if defined(CONFIG_NET_ACT_CPU) || defined(CONFIG_NET_ACT_CPU_MODULE)
+	sk->sk_cpu = nr_cpumask_bits;

include/net/sock.h +5 -0

+#endif
+}
+
+static inline void sock_rps_record_flow(struct sock *sk)
 {
 #ifdef CONFIG_RPS
 	struct rps_sock_flow_table *sock_flow_table;

@@ -649,6 +670,7 @@ static inline void sock_rps_record_flow(const struct sock *sk)
 	rps_record_sock_flow(sock_flow_table, sk->sk_rxhash);
 	rcu_read_unlock();
 #endif
+	sock_save_cpu(sk);
 }
 
 static inline void sock_rps_reset_flow(const struct sock *sk)

include/net/tc_act/tc_cpu.h new file +18 -0

new file mode 100644
index 0000000..0504bf0
@@ -0,0 +1,18 @@
+#ifndef __NET_TC_CPU_H
+#define __NET_TC_CPU_H
+
+#include <linux/types.h>
+#include <net/act_api.h>
+
+struct tcf_cpu {
+	struct tcf_common	common;
+	u32			type;
+	u32			value;
+};
+
+static inline struct tcf_cpu *to_tcf_cpu(struct tcf_common *pc)
+{
+	return container_of(pc, struct tcf_cpu, common);
+}
+
+#endif /* __NET_TC_CPU_H */

net/core/dev.c +2 -2

index e2b9fa2..45e8a21 100644
@@ -2443,8 +2443,7 @@ static int rps_ipi_queued(struct softnet_data *sd)
  * enqueue_to_backlog is called to queue an skb to a per CPU backlog
  * queue (may be a remote CPU queue).
  */
-static int enqueue_to_backlog(struct sk_buff *skb, int cpu,
-			      unsigned int *qtail)
+int enqueue_to_backlog(struct sk_buff *skb, int cpu, unsigned int *qtail)
 {
 	struct softnet_data *sd;
 	unsigned long flags;

@@ -2482,6 +2481,7 @@ enqueue:
 	kfree_skb(skb);
 	return NET_RX_DROP;
 }
+EXPORT_SYMBOL(enqueue_to_backlog);
 
 /**
  *	netif_rx	-	post buffer to the network code

net/core/sock.c +1 -0

index 363bc26..7a71e76 100644
@@ -1045,6 +1045,7 @@ static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority,
 		if (!try_module_get(prot->owner))
 			goto out_free_sec;
 		sk_tx_queue_clear(sk);
+		sock_reset_cpu(sk);
 	}
 
 	return sk;

net/sched/Kconfig +12 -0

index 2f691fb..a826758 100644
@@ -427,6 +427,18 @@ config NET_CLS_ACT
 	  A recent version of the iproute2 package is required to use
 	  extended matches.
 
+config NET_ACT_CPU
+        tristate "Packet distributing"
+        depends on NET_CLS_ACT
+        depends on RPS
+        ---help---
+	  Say Y here to do packets distributing. With it, you can distribute
+	  the packets among the CPUs on the system in any way as you like. It
+	  only works in ingress.
+
+	  To compile this code as a module, choose M here: the
+	  module will be called act_cpu.
+
 config NET_ACT_POLICE
 	tristate "Traffic Policing"
         depends on NET_CLS_ACT 

net/sched/Makefile +1 -0

index f14e71b..a9e0b96 100644
@@ -7,6 +7,7 @@ obj-y	:= sch_generic.o sch_mq.o
 obj-$(CONFIG_NET_SCHED)		+= sch_api.o sch_blackhole.o
 obj-$(CONFIG_NET_CLS)		+= cls_api.o
 obj-$(CONFIG_NET_CLS_ACT)	+= act_api.o
+obj-$(CONFIG_NET_ACT_CPU)	+= act_cpu.o
 obj-$(CONFIG_NET_ACT_POLICE)	+= act_police.o
 obj-$(CONFIG_NET_ACT_GACT)	+= act_gact.o
 obj-$(CONFIG_NET_ACT_MIRRED)	+= act_mirred.o

net/sched/act_cpu.c new file +105 -0

new file mode 100644
index 0000000..6b7d7fc
@@ -0,0 +1,260 @@
+/*
+ * Packet distributing actions
+ *
+ * Copyright (c) 2010- Changli Gao <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#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/gfp.h>
+#include <net/net_namespace.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+#include <net/tcp.h>
+#include <net/udp.h>
+#include <linux/tc_act/tc_cpu.h>
+#include <net/tc_act/tc_cpu.h>
+
+#define CPU_TAB_MASK     15
+static struct tcf_common *tcf_cpu_ht[CPU_TAB_MASK + 1];
+static u32 cpu_idx_gen;
+static DEFINE_RWLOCK(cpu_lock);
+
+static struct tcf_hashinfo cpu_hash_info = {
+	.htab	= tcf_cpu_ht,
+	.hmask	= CPU_TAB_MASK,
+	.lock	= &cpu_lock
+};
+
+static const struct nla_policy cpu_policy[TCA_CPU_MAX + 1] = {
+	[TCA_CPU_PARMS]	= { .len = sizeof(struct tc_cpu) },
+};
+
+static int tcf_cpu_init(struct nlattr *nla, struct nlattr *est,
+			struct tc_action *a, int ovr, int bind)
+{
+	struct nlattr *tb[TCA_CPU_MAX + 1];
+	struct tc_cpu *parm;
+	struct tcf_cpu *p;
+	struct tcf_common *pc;
+	int ret;
+
+	if (nla == NULL)
+		return -EINVAL;
+	ret = nla_parse_nested(tb, TCA_CPU_MAX, nla, cpu_policy);
+	if (ret < 0)
+		return ret;
+	if (tb[TCA_CPU_PARMS] == NULL)
+		return -EINVAL;
+	parm = nla_data(tb[TCA_CPU_PARMS]);
+	if (parm->type > TCA_CPU_TYPE_MAX || parm->action != TC_ACT_STOLEN)
+		return -EINVAL;
+
+	pc = tcf_hash_check(parm->index, a, bind, &cpu_hash_info);
+	if (!pc) {
+		pc = tcf_hash_create(parm->index, est, a, sizeof(*p), bind,
+				     &cpu_idx_gen, &cpu_hash_info);
+		if (IS_ERR(pc))
+			return PTR_ERR(pc);
+		ret = ACT_P_CREATED;
+	} else {
+		if (!ovr) {
+			tcf_hash_release(pc, bind, &cpu_hash_info);
+			return -EEXIST;
+		}
+		ret = 0;
+	}
+	p = to_tcf_cpu(pc);
+
+	spin_lock_bh(&p->tcf_lock);
+	p->type = parm->type;
+	p->value = parm->value;
+	p->tcf_action = parm->action;
+	spin_unlock_bh(&p->tcf_lock);
+
+	if (ret == ACT_P_CREATED)
+		tcf_hash_insert(pc, &cpu_hash_info);
+
+	return ret;
+}
+
+static int tcf_cpu_cleanup(struct tc_action *a, int bind)
+{
+	struct tcf_cpu *p = a->priv;
+
+	return tcf_hash_release(&p->common, bind, &cpu_hash_info);
+}
+
+static int tcf_cpu_from_sock(struct sk_buff *skb)
+{
+	struct iphdr *iph;
+	struct sock *sk;
+	struct {
+		__be16 source, dest;
+	} *ports;

net/sched/act_cpu.c new file +30 -0

+	switch (iph->protocol) {
+	case IPPROTO_TCP:
+		sk = __inet_lookup(dev_net(skb->dev), &tcp_hashinfo, iph->saddr,
+				   ports->source, iph->daddr, ports->dest,
+				   skb->dev->ifindex);
+		break;
+	case IPPROTO_UDP:
+		sk = udp4_lib_lookup(dev_net(skb->dev), iph->saddr,
+				     ports->source, iph->daddr, ports->dest,
+				     skb->dev->ifindex);
+		break;
+	default:
+		goto err;
+	}
+
+	if (!sk)
+		goto err;
+	cpu = sk->sk_cpu;
+	if (sk->sk_protocol == IPPROTO_TCP && sk->sk_state == TCP_TIME_WAIT)
+		inet_twsk_put(inet_twsk(sk));
+	else
+		sock_put(sk);
+
+	return cpu;
+
+err:
+	return smp_processor_id();
+}
+
+static int tcf_cpu(struct sk_buff *skb, struct tc_action *a,

Quoted 3 lines · Changli Gao

I want to know if I can assign the sk to skb as nf_tproxy_core does to avoid the duplicate search later. Thanks.

I suggest we first correct bugs before adding new features.

For me, tproxy is a high suspect at this moment, I would not use it on production machine.

Quoted 7 lines

act_cpu: packet distributing

This TC action can be used to redirect packets to the CPU:
 * specified by the cpuid option
 * specified by the class minor ID obtained previously
 * on which the corresponding application runs

Quoted 2 lines

It supports the similar functions of RPS and RFS, but is more flexible.

Thins kind of claims is disgusting.

No documentation,  I have no idea how you setup the thing.

include/net/sock.h static inline int sk_backlog_rcv(struct sock *sk, struct sk_buff *skb) · line 659

Quoted 7 lines

include/net/sock.h +7 -0

+static inline void sock_save_cpu(struct sock *sk)
+{
+#if defined(CONFIG_NET_ACT_CPU) || defined(CONFIG_NET_ACT_CPU_MODULE)
+     int cpu = get_cpu();
+     if (sk->sk_cpu != cpu)
+             sk->sk_cpu = cpu;
+     put_cpu();

sk->sk_cpu = raw_smp_processor_id();

Why doing the search again, in case skb->sk already set by another module before you, like tproxy ?

net/sched/act_cpu.c line 155

Quoted 11 lines

net/sched/act_cpu.c new file +11 -0

+static int tcf_cpu(struct sk_buff *skb, struct tc_action *a,
+                struct tcf_result *res)
+{
+     struct tcf_cpu *p = a->priv;
+     u32 type;
+     u32 value;
+     int cpu, action;
+     struct sk_buff *nskb;
+     unsigned int qtail;
+
+     spin_lock(&p->tcf_lock);

Ok, the big lock...

We have a lockless TCP/UDP input path, and this modules adds a lock again.

Quoted 8 lines

+     p->tcf_tm.lastuse = jiffies;
+     p->tcf_bstats.bytes += qdisc_pkt_len(skb);
+     p->tcf_bstats.packets++;
+     type = p->type;
+     value = p->value;
+     action = p->tcf_action;
+     spin_unlock(&p->tcf_lock);
+

Please change all this crap  (legacy crap, copied from other tc modules), by modern one, using RCU and no locking in hot path.

Quoted 4 lines · Eric Dumazet

Why doing the search again, in case skb->sk already set by another module before you, like tproxy ?

Although it is unlikely skb->sk is non null, as tc is before netfilter, I will handle this case. Thanks.

Quoted 26 lines

net/sched/act_cpu.c line 155

Quoted 11 lines

net/sched/act_cpu.c new file +11 -0

+static int tcf_cpu(struct sk_buff *skb, struct tc_action *a,
+                struct tcf_result *res)
+{
+     struct tcf_cpu *p = a->priv;
+     u32 type;
+     u32 value;
+     int cpu, action;
+     struct sk_buff *nskb;
+     unsigned int qtail;
+
+     spin_lock(&p->tcf_lock);

Ok, the big lock...

We have a lockless TCP/UDP input path, and this modules adds a lock again.

Quoted 8 lines

net/sched/act_cpu.c new file +8 -0

+     p->tcf_tm.lastuse = jiffies;
+     p->tcf_bstats.bytes += qdisc_pkt_len(skb);
+     p->tcf_bstats.packets++;
+     type = p->type;
+     value = p->value;
+     action = p->tcf_action;
+     spin_unlock(&p->tcf_lock);
+

Quoted 3 lines

Please change all this crap (legacy crap, copied from other tc modules), by modern one, using RCU and no locking in hot path.

Thanks, I'll try. It is a write critical section, and for me it is difficult to convert this lock to RCU. Could you show me some examples?

Thanks, I'll try. It is a write critical section, and for me it is difficult to convert this lock to RCU. Could you show me some examples?

Quoted 3 lines · Changli Gao

Thanks, I'll try. It is a write critical section, and for me it is difficult to convert this lock to RCU. Could you show me some examples?

RCU maybe a little trickier here Eric. Actions could be shared i.e. example, it is possible to have a policer action restricting rates for a group of flows across multiple netdevices etc. Since action stats get written to by different CPUs concurrently. It could be probably done if one was to implement per-cpu stats which get summed-up when user space asks.

Quoted 7 lines · jamal

RCU maybe a little trickier here Eric. Actions could be shared i.e. example, it is possible to have a policer action restricting rates for a group of flows  across multiple netdevices etc. Since action stats get written to by different CPUs concurrently. It could be probably done if one was to implement per-cpu stats which get summed-up when user space asks.

It's certainly tricky, but is act_cpu useful in its current shape, based on an infrastructure that had to use a lock because of exact rates/accounting ?

I dont understand how distributing packets to different cpus, if going through a central lock can be an improvement. Changli patches are most of the time not documented, and no performance data is provided.

Even if we solve this locking problem, using percpu variables, act_cpu hits another problem :

The socket refcount, taken by the 'master' cpu, and released by the consumer cpu.

RFS provides sort of a lazy flow-based distribution without central lock or cache line ping pongs. Why Changli dont use this, we dont know yet.

Quoted 5 lines · Eric Dumazet

Even if we solve this locking problem, using percpu variables, act_cpu hits another problem :

The socket refcount, taken by the 'master' cpu, and released by the consumer cpu.

This is why I asked if I can assign sk to skb.

The tcf_lock is per-instance, so I should not an issue here if the corresponding NIC isn't an multiqueue NIC or the instance is per-rx-queue.

I agree
the performance data is necessary and I'll publish it in the formal patch.