[PATCH net-next 0/3] support changing steering policies in tuntap

[PATCH net-next 0/3] support changing steering policies in tuntap

[Jason Wang]

Hi all:

We use flow caches based flow steering policy now. This is good for
connection-oriented communication such as TCP but not for the others
e.g connectionless unidirectional workload which cares only about
pps. This calls the ability of supporting changing steering policies
in tuntap which was done by this series.

Flow steering policy was abstracted into tun_steering_ops in the first
patch. Then new ioctls to set or query current policy were introduced,
and the last patch introduces a very simple policy that select txq
based on processor id as an example.

Test was done by using xdp_redirect to redirect traffic generated from
MoonGen that was running on a remote machine. And I see 37%
improvement for processor id policy compared to automatic flow
steering policy.

In the future, both simple and sophisticated policy like RSS or other guest
driven steering policies could be done on top.

Thanks

Jason Wang (3):
  tun: abstract flow steering logic
  tun: introduce ioctls to set and get steering policies
  tun: introduce cpu id based steering policy

 drivers/net/tun.c           | 151 +++++++++++++++++++++++++++++++++++++-------
 include/uapi/linux/if_tun.h |   8 +++
 2 files changed, 136 insertions(+), 23 deletions(-)

-- 
2.7.4

[PATCH net-next 1/3] tun: abstract flow steering logic

[Jason Wang]

tun now use flow caches based automatic queue steering method. This
may not suffice all user cases. To extend it to be able to use more
flow steering policy, this patch abstracts flow steering logic into
tun_steering_ops, then we can declare and use different methods in
the future.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/tun.c | 85 +++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 63 insertions(+), 22 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 2c36f6e..de83e72 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -181,6 +181,14 @@ struct tun_file {
 	struct skb_array tx_array;
 };
 
+struct tun_steering_ops {
+	u16 (*select_queue) (struct tun_struct *tun, struct sk_buff *skb);
+	void (*xmit) (struct tun_struct *tun, struct sk_buff *skb);
+	u32 (*pre_rx) (struct tun_struct *tun, struct sk_buff *skb);
+	void (*post_rx) (struct tun_struct *tun, struct tun_file *tfile,
+			 u32 data);
+};
+
 struct tun_flow_entry {
 	struct hlist_node hash_link;
 	struct rcu_head rcu;
@@ -231,6 +239,7 @@ struct tun_struct {
 	u32 rx_batched;
 	struct tun_pcpu_stats __percpu *pcpu_stats;
 	struct bpf_prog __rcu *xdp_prog;
+	struct tun_steering_ops *steering_ops;
 };
 
 static int tun_napi_receive(struct napi_struct *napi, int budget)
@@ -532,10 +541,8 @@ static inline void tun_flow_save_rps_rxhash(struct tun_flow_entry *e, u32 hash)
  * different rxq no. here. If we could not get rxhash, then we would
  * hope the rxq no. may help here.
  */
-static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb,
-			    void *accel_priv, select_queue_fallback_t fallback)
+static u16 tun_automq_select_queue(struct tun_struct *tun, struct sk_buff *skb)
 {
-	struct tun_struct *tun = netdev_priv(dev);
 	struct tun_flow_entry *e;
 	u32 txq = 0;
 	u32 numqueues = 0;
@@ -559,9 +566,18 @@ static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb,
 	}
 
 	rcu_read_unlock();
+
 	return txq;
 }
 
+static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb,
+			    void *accel_priv, select_queue_fallback_t fallback)
+{
+	struct tun_struct *tun = netdev_priv(dev);
+
+	return tun->steering_ops->select_queue(tun, skb);
+}
+
 static inline bool tun_not_capable(struct tun_struct *tun)
 {
 	const struct cred *cred = current_cred();
@@ -931,24 +947,10 @@ static int tun_net_close(struct net_device *dev)
 	return 0;
 }
 
-/* Net device start xmit */
-static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
+static void tun_automq_xmit(struct tun_struct *tun, struct sk_buff *skb)
 {
-	struct tun_struct *tun = netdev_priv(dev);
-	int txq = skb->queue_mapping;
-	struct tun_file *tfile;
-	u32 numqueues = 0;
-
-	rcu_read_lock();
-	tfile = rcu_dereference(tun->tfiles[txq]);
-	numqueues = ACCESS_ONCE(tun->numqueues);
-
-	/* Drop packet if interface is not attached */
-	if (txq >= numqueues)
-		goto drop;
-
 #ifdef CONFIG_RPS
-	if (numqueues == 1 && static_key_false(&rps_needed)) {
+	if (ACCESS_ONCE(tun->numqueues) == 1 && static_key_false(&rps_needed)) {
 		/* Select queue was not called for the skbuff, so we extract the
 		 * RPS hash and save it into the flow_table here.
 		 */
@@ -964,6 +966,25 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 		}
 	}
 #endif
+}
+
+/* Net device start xmit */
+static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct tun_struct *tun = netdev_priv(dev);
+	int txq = skb->queue_mapping;
+	struct tun_file *tfile;
+	u32 numqueues = 0;
+
+	rcu_read_lock();
+	tfile = rcu_dereference(tun->tfiles[txq]);
+	numqueues = ACCESS_ONCE(tun->numqueues);
+
+	/* Drop packet if interface is not attached */
+	if (txq >= numqueues)
+		goto drop;
+
+	tun->steering_ops->xmit(tun, skb);
 
 	tun_debug(KERN_INFO, tun, "tun_net_xmit %d\n", skb->len);
 
@@ -1527,6 +1548,17 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
 	return NULL;
 }
 
+u32 tun_automq_pre_rx(struct tun_struct *tun, struct sk_buff *skb)
+{
+	return __skb_get_hash_symmetric(skb);
+}
+
+void tun_automq_post_rx(struct tun_struct *tun, struct tun_file *tfile,
+			u32 rxhash)
+{
+	tun_flow_update(tun, rxhash, tfile);
+}
+
 /* Get packet from user space buffer */
 static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 			    void *msg_control, struct iov_iter *from,
@@ -1542,7 +1574,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	int copylen;
 	bool zerocopy = false;
 	int err;
-	u32 rxhash;
+	u32 data;
 	int skb_xdp = 1;
 	bool frags = tun_napi_frags_enabled(tun);
 
@@ -1728,7 +1760,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		rcu_read_unlock();
 	}
 
-	rxhash = __skb_get_hash_symmetric(skb);
+	data = tun->steering_ops->pre_rx(tun, skb);
 
 	if (frags) {
 		/* Exercise flow dissector code path. */
@@ -1772,7 +1804,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	u64_stats_update_end(&stats->syncp);
 	put_cpu_ptr(stats);
 
-	tun_flow_update(tun, rxhash, tfile);
+	tun->steering_ops->post_rx(tun, tfile, data);
 	return total_len;
 }
 
@@ -2112,6 +2144,13 @@ static struct proto tun_proto = {
 	.obj_size	= sizeof(struct tun_file),
 };
 
+static struct tun_steering_ops tun_automq_ops = {
+	.select_queue = tun_automq_select_queue,
+	.xmit = tun_automq_xmit,
+	.pre_rx = tun_automq_pre_rx,
+	.post_rx = tun_automq_post_rx,
+};
+
 static int tun_flags(struct tun_struct *tun)
 {
 	return tun->flags & (TUN_FEATURES | IFF_PERSIST | IFF_TUN | IFF_TAP);
@@ -2268,6 +2307,8 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 			goto err_free_dev;
 		}
 
+		tun->steering_ops = &tun_automq_ops;
+
 		spin_lock_init(&tun->lock);
 
 		err = security_tun_dev_alloc_security(&tun->security);
-- 
2.7.4

[PATCH net-next 2/3] tun: introduce ioctls to set and get steering policies

[Jason Wang]

This patch introduces new ioctl for change packet steering policy for
tun. Only automatic flow steering is supported, more policies will
come.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/tun.c           | 35 ++++++++++++++++++++++++++++++++++-
 include/uapi/linux/if_tun.h |  7 +++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index de83e72..1106521 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -122,7 +122,8 @@ do {								\
 #define TUN_VNET_BE     0x40000000
 
 #define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
-		      IFF_MULTI_QUEUE | IFF_NAPI | IFF_NAPI_FRAGS)
+		      IFF_MULTI_QUEUE | IFF_NAPI | IFF_NAPI_FRAGS | \
+		      IFF_MULTI_STEERING)
 
 #define GOODCOPY_LEN 128
 
@@ -2506,6 +2507,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 	unsigned int ifindex;
 	int le;
 	int ret;
+	unsigned int steering;
 
 	if (cmd == TUNSETIFF || cmd == TUNSETQUEUE || _IOC_TYPE(cmd) == SOCK_IOC_TYPE) {
 		if (copy_from_user(&ifr, argp, ifreq_len))
@@ -2764,6 +2766,37 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		ret = 0;
 		break;
 
+	case TUNSETSTEERING:
+		ret = -EFAULT;
+		if (copy_from_user(&steering, argp, sizeof(steering)))
+			break;
+		ret = 0;
+		switch (steering) {
+		case TUN_STEERING_AUTOMQ:
+			tun->steering_ops = &tun_automq_ops;
+			break;
+		default:
+			ret = -EFAULT;
+		}
+		break;
+
+	case TUNGETSTEERING:
+		ret = 0;
+		if (tun->steering_ops == &tun_automq_ops)
+			steering = TUN_STEERING_AUTOMQ;
+		else
+			BUG();
+		if (copy_to_user(argp, &steering, sizeof(steering)))
+			ret = -EFAULT;
+		break;
+
+	case TUNGETSTEERINGFEATURES:
+		ret = 0;
+		steering = TUN_STEERING_AUTOMQ;
+		if (copy_to_user(argp, &steering, sizeof(steering)))
+			ret = -EFAULT;
+		break;
+
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index 365ade5..109760e 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -56,6 +56,9 @@
  */
 #define TUNSETVNETBE _IOW('T', 222, int)
 #define TUNGETVNETBE _IOR('T', 223, int)
+#define TUNSETSTEERING _IOW('T', 224, unsigned int)
+#define TUNGETSTEERING _IOR('T', 225, unsigned int)
+#define TUNGETSTEERINGFEATURES _IOR('T', 226, unsigned int)
 
 /* TUNSETIFF ifr flags */
 #define IFF_TUN		0x0001
@@ -70,6 +73,8 @@
 #define IFF_MULTI_QUEUE 0x0100
 #define IFF_ATTACH_QUEUE 0x0200
 #define IFF_DETACH_QUEUE 0x0400
+#define IFF_MULTI_STEERING 0x2000
+
 /* read-only flag */
 #define IFF_PERSIST	0x0800
 #define IFF_NOFILTER	0x1000
@@ -106,4 +111,6 @@ struct tun_filter {
 	__u8   addr[0][ETH_ALEN];
 };
 
+#define TUN_STEERING_AUTOMQ 0x01 /* Automatic flow steering */
+
 #endif /* _UAPI__IF_TUN_H */
-- 
2.7.4

[PATCH net-next 3/3] tun: introduce cpu id based steering policy

[Jason Wang]

This patch introduces a simple queue selection policy which just
choose txq based on processor id. This maybe useful for connectless
workload or #queues is equal to #cpus.

Redirect UDP packets generated by MoonGen between two virtio-net ports
through xdp_redirect show 37.4% (from 0.8Mpps to 1.1Mpps) improvement
compared to automatic steering policy since the overhead of flow
caches/hasing was totally eliminated.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/tun.c           | 33 ++++++++++++++++++++++++++++++++-
 include/uapi/linux/if_tun.h |  1 +
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 1106521..03b4506 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -190,6 +190,20 @@ struct tun_steering_ops {
 			 u32 data);
 };
 
+void tun_steering_xmit_nop(struct tun_struct *tun, struct sk_buff *skb)
+{
+}
+
+u32 tun_steering_pre_rx_nop(struct tun_struct *tun, struct sk_buff *skb)
+{
+	return 0;
+}
+
+void tun_steering_post_rx_nop(struct tun_struct *tun, struct tun_file *tfile,
+			      u32 data)
+{
+}
+
 struct tun_flow_entry {
 	struct hlist_node hash_link;
 	struct rcu_head rcu;
@@ -571,6 +585,11 @@ static u16 tun_automq_select_queue(struct tun_struct *tun, struct sk_buff *skb)
 	return txq;
 }
 
+static u16 tun_cpu_select_queue(struct tun_struct *tun, struct sk_buff *skb)
+{
+	return smp_processor_id() % tun->numqueues;
+}
+
 static u16 tun_select_queue(struct net_device *dev, struct sk_buff *skb,
 			    void *accel_priv, select_queue_fallback_t fallback)
 {
@@ -2152,6 +2171,13 @@ static struct tun_steering_ops tun_automq_ops = {
 	.post_rx = tun_automq_post_rx,
 };
 
+static struct tun_steering_ops tun_cpu_ops = {
+	.select_queue = tun_cpu_select_queue,
+	.xmit = tun_steering_xmit_nop,
+	.pre_rx = tun_steering_pre_rx_nop,
+	.post_rx = tun_steering_post_rx_nop,
+};
+
 static int tun_flags(struct tun_struct *tun)
 {
 	return tun->flags & (TUN_FEATURES | IFF_PERSIST | IFF_TUN | IFF_TAP);
@@ -2775,6 +2801,9 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		case TUN_STEERING_AUTOMQ:
 			tun->steering_ops = &tun_automq_ops;
 			break;
+		case TUN_STEERING_CPU:
+			tun->steering_ops = &tun_cpu_ops;
+			break;
 		default:
 			ret = -EFAULT;
 		}
@@ -2784,6 +2813,8 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		ret = 0;
 		if (tun->steering_ops == &tun_automq_ops)
 			steering = TUN_STEERING_AUTOMQ;
+		else if (tun->steering_ops == &tun_cpu_ops)
+			steering = TUN_STEERING_CPU;
 		else
 			BUG();
 		if (copy_to_user(argp, &steering, sizeof(steering)))
@@ -2792,7 +2823,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 
 	case TUNGETSTEERINGFEATURES:
 		ret = 0;
-		steering = TUN_STEERING_AUTOMQ;
+		steering = TUN_STEERING_AUTOMQ | TUN_STEERING_CPU;
 		if (copy_to_user(argp, &steering, sizeof(steering)))
 			ret = -EFAULT;
 		break;
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index 109760e..5f71d29 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -112,5 +112,6 @@ struct tun_filter {
 };
 
 #define TUN_STEERING_AUTOMQ 0x01 /* Automatic flow steering */
+#define TUN_STEERING_CPU    0x02 /* Processor id based flow steering */
 
 #endif /* _UAPI__IF_TUN_H */
-- 
2.7.4

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Michael S. Tsirkin]

On Wed, Sep 27, 2017 at 04:23:54PM +0800, Jason Wang wrote:
> Hi all:
> 
> We use flow caches based flow steering policy now. This is good for
> connection-oriented communication such as TCP but not for the others
> e.g connectionless unidirectional workload which cares only about
> pps. This calls the ability of supporting changing steering policies
> in tuntap which was done by this series.
> 
> Flow steering policy was abstracted into tun_steering_ops in the first
> patch. Then new ioctls to set or query current policy were introduced,
> and the last patch introduces a very simple policy that select txq
> based on processor id as an example.
> 
> Test was done by using xdp_redirect to redirect traffic generated from
> MoonGen that was running on a remote machine. And I see 37%
> improvement for processor id policy compared to automatic flow
> steering policy.

For sure, if you don't need to figure out the flow hash then you can
save a bunch of cycles.  But I don't think the cpu policy is too
practical outside of a benchmark.

Did you generate packets and just send them to tun? If so, this is not a
typical configuration, is it? With packets coming e.g.  from a real nic
they might already have the hash pre-calculated, and you won't
see the benefit.

> In the future, both simple and sophisticated policy like RSS or other guest
> driven steering policies could be done on top.

IMHO there should be a more practical example before adding all this
indirection. And it would be nice to understand why this queue selection
needs to be tun specific.

> Thanks
> 
> Jason Wang (3):
>   tun: abstract flow steering logic
>   tun: introduce ioctls to set and get steering policies
>   tun: introduce cpu id based steering policy
> 
>  drivers/net/tun.c           | 151 +++++++++++++++++++++++++++++++++++++-------
>  include/uapi/linux/if_tun.h |   8 +++
>  2 files changed, 136 insertions(+), 23 deletions(-)
> 
> -- 
> 2.7.4

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Willem de Bruijn]

>> In the future, both simple and sophisticated policy like RSS or other guest
>> driven steering policies could be done on top.
>
> IMHO there should be a more practical example before adding all this
> indirection. And it would be nice to understand why this queue selection
> needs to be tun specific.

I was thinking the same and this reminds me of the various strategies
implemented in packet fanout. tun_cpu_select_queue is analogous to
fanout_demux_cpu though it is tun-specific in that it requires tun->numqueues.

Fanout accrued various strategies until it gained an eBPF variant. Just
supporting BPF is probably sufficient here, too.

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Tom Herbert]

On Wed, Sep 27, 2017 at 4:25 PM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>>> In the future, both simple and sophisticated policy like RSS or other guest
>>> driven steering policies could be done on top.
>>
>> IMHO there should be a more practical example before adding all this
>> indirection. And it would be nice to understand why this queue selection
>> needs to be tun specific.
>
> I was thinking the same and this reminds me of the various strategies
> implemented in packet fanout. tun_cpu_select_queue is analogous to
> fanout_demux_cpu though it is tun-specific in that it requires tun->numqueues.
>
> Fanout accrued various strategies until it gained an eBPF variant. Just
> supporting BPF is probably sufficient here, too.

+1, in addition to packet fanout, we have SO_REUSEPORT with BPF, RPS,
RFS, etc. It would be nice if existing packet steering mechanisms
could be leveraged for tun.

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Jason Wang]

On 2017年09月28日 06:13, Michael S. Tsirkin wrote:
> On Wed, Sep 27, 2017 at 04:23:54PM +0800, Jason Wang wrote:
>> Hi all:
>>
>> We use flow caches based flow steering policy now. This is good for
>> connection-oriented communication such as TCP but not for the others
>> e.g connectionless unidirectional workload which cares only about
>> pps. This calls the ability of supporting changing steering policies
>> in tuntap which was done by this series.
>>
>> Flow steering policy was abstracted into tun_steering_ops in the first
>> patch. Then new ioctls to set or query current policy were introduced,
>> and the last patch introduces a very simple policy that select txq
>> based on processor id as an example.
>>
>> Test was done by using xdp_redirect to redirect traffic generated from
>> MoonGen that was running on a remote machine. And I see 37%
>> improvement for processor id policy compared to automatic flow
>> steering policy.
> For sure, if you don't need to figure out the flow hash then you can
> save a bunch of cycles.  But I don't think the cpu policy is too
> practical outside of a benchmark.

Well, the aim of the series is to add methods to change the steering 
policy, cpu policy is an example. Actually, it may make sense for some 
cards which guarantee that all packets belongs to a specific flow goes 
into a specific cpu.

>
> Did you generate packets and just send them to tun? If so, this is not a
> typical configuration, is it?

The test was done by:

- generate UDP traffic from a remote machine
- use xdp redirection to do mac swap in guest and forward it back to the 
remote machine

>   With packets coming e.g.  from a real nic
> they might already have the hash pre-calculated, and you won't
> see the benefit.

Yes, I can switch to use this as a example policy.

Thanks

>
>> In the future, both simple and sophisticated policy like RSS or other guest
>> driven steering policies could be done on top.
> IMHO there should be a more practical example before adding all this
> indirection. And it would be nice to understand why this queue selection
> needs to be tun specific.

Actually, we can use fanout policy (as pointed out) by using the API 
introduced in this series.

Thanks

>
>> Thanks
>>
>> Jason Wang (3):
>>    tun: abstract flow steering logic
>>    tun: introduce ioctls to set and get steering policies
>>    tun: introduce cpu id based steering policy
>>
>>   drivers/net/tun.c           | 151 +++++++++++++++++++++++++++++++++++++-------
>>   include/uapi/linux/if_tun.h |   8 +++
>>   2 files changed, 136 insertions(+), 23 deletions(-)
>>
>> -- 
>> 2.7.4

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Jason Wang]

On 2017年09月28日 07:25, Willem de Bruijn wrote:
>>> In the future, both simple and sophisticated policy like RSS or other guest
>>> driven steering policies could be done on top.
>> IMHO there should be a more practical example before adding all this
>> indirection. And it would be nice to understand why this queue selection
>> needs to be tun specific.
> I was thinking the same and this reminds me of the various strategies
> implemented in packet fanout. tun_cpu_select_queue is analogous to
> fanout_demux_cpu though it is tun-specific in that it requires tun->numqueues.

Right, the main idea is to introduce a way to change flow steering 
policy for tun. I think fanout policy could be implemented through the 
API introduced in this series. (Current flow caches based automatic 
steering method is tun specific).

>
> Fanout accrued various strategies until it gained an eBPF variant. Just
> supporting BPF is probably sufficient here, too.

Technically yes, but for tun, it also serve for virt. We probably still 
need some hard coded policy which could be changed by guest until we can 
accept an BPF program from guest I think?

Thanks

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Jason Wang]

On 2017年09月28日 13:02, Tom Herbert wrote:
> On Wed, Sep 27, 2017 at 4:25 PM, Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
>>>> In the future, both simple and sophisticated policy like RSS or other guest
>>>> driven steering policies could be done on top.
>>> IMHO there should be a more practical example before adding all this
>>> indirection. And it would be nice to understand why this queue selection
>>> needs to be tun specific.
>> I was thinking the same and this reminds me of the various strategies
>> implemented in packet fanout. tun_cpu_select_queue is analogous to
>> fanout_demux_cpu though it is tun-specific in that it requires tun->numqueues.
>>
>> Fanout accrued various strategies until it gained an eBPF variant. Just
>> supporting BPF is probably sufficient here, too.
> +1, in addition to packet fanout, we have SO_REUSEPORT with BPF, RPS,
> RFS, etc. It would be nice if existing packet steering mechanisms
> could be leveraged for tun.

This could be done by using the API introduced in this series, I can try 
this in V2.

Thanks

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Willem de Bruijn]

On Thu, Sep 28, 2017 at 3:23 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 2017年09月28日 07:25, Willem de Bruijn wrote:
>>>>
>>>> In the future, both simple and sophisticated policy like RSS or other
>>>> guest
>>>> driven steering policies could be done on top.
>>>
>>> IMHO there should be a more practical example before adding all this
>>> indirection. And it would be nice to understand why this queue selection
>>> needs to be tun specific.
>>
>> I was thinking the same and this reminds me of the various strategies
>> implemented in packet fanout. tun_cpu_select_queue is analogous to
>> fanout_demux_cpu though it is tun-specific in that it requires
>> tun->numqueues.
>
>
> Right, the main idea is to introduce a way to change flow steering policy
> for tun. I think fanout policy could be implemented through the API
> introduced in this series. (Current flow caches based automatic steering
> method is tun specific).
>
>>
>> Fanout accrued various strategies until it gained an eBPF variant. Just
>> supporting BPF is probably sufficient here, too.
>
>
> Technically yes, but for tun, it also serve for virt. We probably still need
> some hard coded policy which could be changed by guest until we can accept
> an BPF program from guest I think?

When would a guest choose the policy? As long as this is under control
of a host user, possibly unprivileged, allowing BPF here is moot, as any
user can run socket filter BPF already. Programming from the guest is
indeed different. I don't fully understand that use case.

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Jason Wang]

On 2017年09月29日 00:09, Willem de Bruijn wrote:
> On Thu, Sep 28, 2017 at 3:23 AM, Jason Wang <jasowang@redhat.com> wrote:
>>
>> On 2017年09月28日 07:25, Willem de Bruijn wrote:
>>>>> In the future, both simple and sophisticated policy like RSS or other
>>>>> guest
>>>>> driven steering policies could be done on top.
>>>> IMHO there should be a more practical example before adding all this
>>>> indirection. And it would be nice to understand why this queue selection
>>>> needs to be tun specific.
>>> I was thinking the same and this reminds me of the various strategies
>>> implemented in packet fanout. tun_cpu_select_queue is analogous to
>>> fanout_demux_cpu though it is tun-specific in that it requires
>>> tun->numqueues.
>>
>> Right, the main idea is to introduce a way to change flow steering policy
>> for tun. I think fanout policy could be implemented through the API
>> introduced in this series. (Current flow caches based automatic steering
>> method is tun specific).
>>
>>> Fanout accrued various strategies until it gained an eBPF variant. Just
>>> supporting BPF is probably sufficient here, too.
>>
>> Technically yes, but for tun, it also serve for virt. We probably still need
>> some hard coded policy which could be changed by guest until we can accept
>> an BPF program from guest I think?
> When would a guest choose the policy? As long as this is under control
> of a host user, possibly unprivileged, allowing BPF here is moot, as any
> user can run socket filter BPF already. Programming from the guest is
> indeed different. I don't fully understand that use case.

The problem is userspace (qemu) know little about what kind of workloads 
will be done by guest, so we need guest controllable method here since 
it knows the best steering policy. Rethink about this, instead of 
passing eBPF from guest, qemu can have some pre-defined sets of polices. 
I will change the cpu id based to eBPF based in V2.

Thanks

Re: [PATCH net-next 0/3] support changing steering policies in tuntap

[Michael S. Tsirkin]

On Thu, Sep 28, 2017 at 12:09:05PM -0400, Willem de Bruijn wrote:
> Programming from the guest is
> indeed different. I don't fully understand that use case.

Generally programming host BPF from guest is a clear win - think DOS
protection. Guest runs logic to detect dos attacks, then passes the
program to host.  Afterwards, host does not need to enter guest if
there's a DOS attack. Saves a ton of cycles.

The difficulty is making it work well, e.g. how do we handle maps?

-- 
MST