From: Tonghao Zhang <redacted>
This patch fixes issue:
* If we install tc filters with act_skbedit in clsact hook.
It doesn't work, because netdev_core_pick_tx() overwrites
queue_mapping.
$ tc filter ... action skbedit queue_mapping 1
And this patch is useful:
* We can use FQ + EDT to implement efficient policies. Tx queues
are picked by xps, ndo_select_queue of netdev driver, or skb hash
in netdev_core_pick_tx(). In fact, the netdev driver, and skb
hash are _not_ under control. xps uses the CPUs map to select Tx
queues, but we can't figure out which task_struct of pod/containter
running on this cpu in most case. We can use clsact filters to classify
one pod/container traffic to one Tx queue. Why ?
In containter networking environment, there are two kinds of pod/
containter/net-namespace. One kind (e.g. P1, P2), the high throughput
is key in these applications. But avoid running out of network resource,
the outbound traffic of these pods is limited, using or sharing one
dedicated Tx queues assigned HTB/TBF/FQ Qdisc. Other kind of pods
(e.g. Pn), the low latency of data access is key. And the traffic is not
limited. Pods use or share other dedicated Tx queues assigned FIFO Qdisc.
This choice provides two benefits. First, contention on the HTB/FQ Qdisc
lock is significantly reduced since fewer CPUs contend for the same queue.
More importantly, Qdisc contention can be eliminated completely if each
CPU has its own FIFO Qdisc for the second kind of pods.
There must be a mechanism in place to support classifying traffic based on
pods/container to different Tx queues. Note that clsact is outside of Qdisc
while Qdisc can run a classifier to select a sub-queue under the lock.
In general recording the decision in the skb seems a little heavy handed.
This patch introduces a per-CPU variable, suggested by Eric.
The xmit.skip_txqueue flag is firstly cleared in __dev_queue_xmit().
- Tx Qdisc may install that skbedit actions, then xmit.skip_txqueue flag
is set in qdisc->enqueue() though tx queue has been selected in
netdev_tx_queue_mapping() or netdev_core_pick_tx(). That flag is cleared
firstly in __dev_queue_xmit(), is useful:
- Avoid picking Tx queue with netdev_tx_queue_mapping() in next netdev
in such case: eth0 macvlan - eth0.3 vlan - eth0 ixgbe-phy:
For example, eth0, macvlan in pod, which root Qdisc install skbedit
queue_mapping, send packets to eth0.3, vlan in host. In __dev_queue_xmit() of
eth0.3, clear the flag, does not select tx queue according to skb->queue_mapping
because there is no filters in clsact or tx Qdisc of this netdev.
Same action taked in eth0, ixgbe in Host.
- Avoid picking Tx queue for next packet. If we set xmit.skip_txqueue
in tx Qdisc (qdisc->enqueue()), the proper way to clear it is clearing it
in __dev_queue_xmit when processing next packets.
+----+ +----+ +----+
| P1 | | P2 | | Pn |
+----+ +----+ +----+
| | |
+-----------+-----------+
|
| clsact/skbedit
| MQ
v
+-----------+-----------+
| q0 | q1 | qn
v v v
HTB/FQ HTB/FQ ... FIFO
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Cong Wang <redacted>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jonathan Lemon <redacted>
Cc: Eric Dumazet <redacted>
Cc: Alexander Lobakin <redacted>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Talal Ahmad <redacted>
Cc: Kevin Hao <redacted>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Kees Cook <redacted>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Antoine Tenart <atenart@kernel.org>
Cc: Wei Wang <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Eric Dumazet <redacted>
Signed-off-by: Tonghao Zhang <redacted>
---
include/linux/netdevice.h | 19 +++++++++++++++++++
net/core/dev.c | 7 ++++++-
net/sched/act_skbedit.c | 4 +++-
3 files changed, 28 insertions(+), 2 deletions(-)
@@ -3009,6 +3009,7 @@ struct softnet_data {/* written and read only by owning cpu: */struct{u16recursion;+u8skip_txqueue;u8more;}xmit;#ifdef CONFIG_RPS
From: Tonghao Zhang <redacted>
This patch allows user to select queue_mapping, range
from A to B. And user can use skbhash, cgroup classid
and cpuid to select Tx queues. Then we can load balance
packets from A to B queue. The range is an unsigned 16bit
value in decimal format.
$ tc filter ... action skbedit queue_mapping skbhash A B
"skbedit queue_mapping QUEUE_MAPPING" (from "man 8 tc-skbedit")
is enhanced with flags:
* SKBEDIT_F_TXQ_SKBHASH
* SKBEDIT_F_TXQ_CLASSID
* SKBEDIT_F_TXQ_CPUID
Use skb->hash, cgroup classid, or cpuid to distribute packets.
Then same range of tx queues can be shared for different flows,
cgroups, or CPUs in a variety of scenarios.
For example, F1 may share range R1 with F2. The best way to do
that is to set flag to SKBEDIT_F_TXQ_HASH, using skb->hash to
share the queues. If cgroup C1 want to share the R1 with cgroup
C2 .. Cn, use the SKBEDIT_F_TXQ_CLASSID. Of course, in some other
scenario, C1 use R1, while Cn can use the Rn.
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Cong Wang <redacted>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jonathan Lemon <redacted>
Cc: Eric Dumazet <redacted>
Cc: Alexander Lobakin <redacted>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Talal Ahmad <redacted>
Cc: Kevin Hao <redacted>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Kees Cook <redacted>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Antoine Tenart <atenart@kernel.org>
Cc: Wei Wang <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Tonghao Zhang <redacted>
---
include/net/tc_act/tc_skbedit.h | 1 +
include/uapi/linux/tc_act/tc_skbedit.h | 8 +++
net/sched/act_skbedit.c | 81 ++++++++++++++++++++++++--
3 files changed, 85 insertions(+), 5 deletions(-)
@@ -23,6 +24,38 @@staticunsignedintskbedit_net_id;staticstructtc_action_opsact_skbedit_ops;+staticu16tcf_skbedit_hash(structtcf_skbedit_params*params,+structsk_buff*skb)+{+u32mapping_hash_type=params->flags&SKBEDIT_F_TXQ_HASH_MASK;+u16queue_mapping=params->queue_mapping;+u16mapping_mod=params->mapping_mod;+u32hash=0;++switch(mapping_hash_type){+caseSKBEDIT_F_TXQ_CLASSID:+hash=task_get_classid(skb);+break;+caseSKBEDIT_F_TXQ_SKBHASH:+hash=skb_get_hash(skb);+break;+caseSKBEDIT_F_TXQ_CPUID:+hash=raw_smp_processor_id();+break;+case0:+/* Hash type isn't specified. In this case:+*hash%mapping_mod==0+*/+break;+default:+net_warn_ratelimited("The type of queue_mapping hash is not supported. 0x%x\n",+mapping_hash_type);+}++queue_mapping=queue_mapping+hash%mapping_mod;+returnnetdev_cap_txqueue(skb->dev,queue_mapping);+}+staticinttcf_skbedit_act(structsk_buff*skb,conststructtc_action*a,structtcf_result*res){
@@ -110,6 +143,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,structtcf_skbedit*d;u32flags=0,*priority=NULL,*mark=NULL,*mask=NULL;u16*queue_mapping=NULL,*ptype=NULL;+u16mapping_mod=1;boolexists=false;intret=0,err;u32index;
@@ -154,7 +188,34 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,if(tb[TCA_SKBEDIT_FLAGS]!=NULL){u64*pure_flags=nla_data(tb[TCA_SKBEDIT_FLAGS]);-+u64mapping_hash_type;++mapping_hash_type=*pure_flags&SKBEDIT_F_TXQ_HASH_MASK;+if(mapping_hash_type){+u16*queue_mapping_max;++/* Hash types are mutually exclusive. */+if(mapping_hash_type&(mapping_hash_type-1)){+NL_SET_ERR_MSG_MOD(extack,"Multi types of hash are specified.");+return-EINVAL;+}++if(!tb[TCA_SKBEDIT_QUEUE_MAPPING]||+!tb[TCA_SKBEDIT_QUEUE_MAPPING_MAX]){+NL_SET_ERR_MSG_MOD(extack,"Missing required range of queue_mapping.");+return-EINVAL;+}++queue_mapping_max=+nla_data(tb[TCA_SKBEDIT_QUEUE_MAPPING_MAX]);+if(*queue_mapping_max<*queue_mapping){+NL_SET_ERR_MSG_MOD(extack,"The range of queue_mapping is invalid, max < min.");+return-EINVAL;+}++mapping_mod=*queue_mapping_max-*queue_mapping+1;+flags|=mapping_hash_type;+}if(*pure_flags&SKBEDIT_F_INHERITDSFIELD)flags|=SKBEDIT_F_INHERITDSFIELD;}
@@ -206,8 +267,10 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,params_new->flags=flags;if(flags&SKBEDIT_F_PRIORITY)params_new->priority=*priority;-if(flags&SKBEDIT_F_QUEUE_MAPPING)+if(flags&SKBEDIT_F_QUEUE_MAPPING){params_new->queue_mapping=*queue_mapping;+params_new->mapping_mod=mapping_mod;+}if(flags&SKBEDIT_F_MARK)params_new->mark=*mark;if(flags&SKBEDIT_F_PTYPE)
From: Eric Dumazet <hidden> Date: 2021-12-20 13:58:09
On Mon, Dec 20, 2021 at 4:38 AM [off-list ref] wrote:
From: Tonghao Zhang <redacted>
This patch fixes issue:
* If we install tc filters with act_skbedit in clsact hook.
It doesn't work, because netdev_core_pick_tx() overwrites
queue_mapping.
$ tc filter ... action skbedit queue_mapping 1
And this patch is useful:
* We can use FQ + EDT to implement efficient policies. Tx queues
are picked by xps, ndo_select_queue of netdev driver, or skb hash
in netdev_core_pick_tx(). In fact, the netdev driver, and skb
hash are _not_ under control. xps uses the CPUs map to select Tx
queues, but we can't figure out which task_struct of pod/containter
running on this cpu in most case. We can use clsact filters to classify
one pod/container traffic to one Tx queue. Why ?
In containter networking environment, there are two kinds of pod/
containter/net-namespace. One kind (e.g. P1, P2), the high throughput
is key in these applications. But avoid running out of network resource,
the outbound traffic of these pods is limited, using or sharing one
dedicated Tx queues assigned HTB/TBF/FQ Qdisc. Other kind of pods
(e.g. Pn), the low latency of data access is key. And the traffic is not
limited. Pods use or share other dedicated Tx queues assigned FIFO Qdisc.
This choice provides two benefits. First, contention on the HTB/FQ Qdisc
lock is significantly reduced since fewer CPUs contend for the same queue.
More importantly, Qdisc contention can be eliminated completely if each
CPU has its own FIFO Qdisc for the second kind of pods.
There must be a mechanism in place to support classifying traffic based on
pods/container to different Tx queues. Note that clsact is outside of Qdisc
while Qdisc can run a classifier to select a sub-queue under the lock.
In general recording the decision in the skb seems a little heavy handed.
This patch introduces a per-CPU variable, suggested by Eric.
The xmit.skip_txqueue flag is firstly cleared in __dev_queue_xmit().
- Tx Qdisc may install that skbedit actions, then xmit.skip_txqueue flag
is set in qdisc->enqueue() though tx queue has been selected in
netdev_tx_queue_mapping() or netdev_core_pick_tx(). That flag is cleared
firstly in __dev_queue_xmit(), is useful:
- Avoid picking Tx queue with netdev_tx_queue_mapping() in next netdev
in such case: eth0 macvlan - eth0.3 vlan - eth0 ixgbe-phy:
For example, eth0, macvlan in pod, which root Qdisc install skbedit
queue_mapping, send packets to eth0.3, vlan in host. In __dev_queue_xmit() of
eth0.3, clear the flag, does not select tx queue according to skb->queue_mapping
because there is no filters in clsact or tx Qdisc of this netdev.
Same action taked in eth0, ixgbe in Host.
- Avoid picking Tx queue for next packet. If we set xmit.skip_txqueue
in tx Qdisc (qdisc->enqueue()), the proper way to clear it is clearing it
in __dev_queue_xmit when processing next packets.
+----+ +----+ +----+
| P1 | | P2 | | Pn |
+----+ +----+ +----+
| | |
+-----------+-----------+
|
| clsact/skbedit
| MQ
v
+-----------+-----------+
| q0 | q1 | qn
v v v
HTB/FQ HTB/FQ ... FIFO
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Cong Wang <redacted>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jonathan Lemon <redacted>
Cc: Eric Dumazet <redacted>
Cc: Alexander Lobakin <redacted>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Talal Ahmad <redacted>
Cc: Kevin Hao <redacted>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Kees Cook <redacted>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Antoine Tenart <atenart@kernel.org>
Cc: Wei Wang <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Eric Dumazet <redacted>
I have not suggested this patch, only to not add yet another bit in sk_buff.
@@ -3009,6 +3009,7 @@ struct softnet_data {/* written and read only by owning cpu: */struct{u16recursion;+u8skip_txqueue;u8more;}xmit;#ifdef CONFIG_RPS
If we really need to add yet another conditional in fast path, I would
suggest using a static key.
Only hosts where SKBEDIT_F_QUEUE_MAPPING is requested would pay the price.
On Mon, Dec 20, 2021 at 9:58 PM Eric Dumazet [off-list ref] wrote:
On Mon, Dec 20, 2021 at 4:38 AM [off-list ref] wrote:
quoted
From: Tonghao Zhang <redacted>
This patch fixes issue:
* If we install tc filters with act_skbedit in clsact hook.
It doesn't work, because netdev_core_pick_tx() overwrites
queue_mapping.
$ tc filter ... action skbedit queue_mapping 1
And this patch is useful:
* We can use FQ + EDT to implement efficient policies. Tx queues
are picked by xps, ndo_select_queue of netdev driver, or skb hash
in netdev_core_pick_tx(). In fact, the netdev driver, and skb
hash are _not_ under control. xps uses the CPUs map to select Tx
queues, but we can't figure out which task_struct of pod/containter
running on this cpu in most case. We can use clsact filters to classify
one pod/container traffic to one Tx queue. Why ?
In containter networking environment, there are two kinds of pod/
containter/net-namespace. One kind (e.g. P1, P2), the high throughput
is key in these applications. But avoid running out of network resource,
the outbound traffic of these pods is limited, using or sharing one
dedicated Tx queues assigned HTB/TBF/FQ Qdisc. Other kind of pods
(e.g. Pn), the low latency of data access is key. And the traffic is not
limited. Pods use or share other dedicated Tx queues assigned FIFO Qdisc.
This choice provides two benefits. First, contention on the HTB/FQ Qdisc
lock is significantly reduced since fewer CPUs contend for the same queue.
More importantly, Qdisc contention can be eliminated completely if each
CPU has its own FIFO Qdisc for the second kind of pods.
There must be a mechanism in place to support classifying traffic based on
pods/container to different Tx queues. Note that clsact is outside of Qdisc
while Qdisc can run a classifier to select a sub-queue under the lock.
In general recording the decision in the skb seems a little heavy handed.
This patch introduces a per-CPU variable, suggested by Eric.
The xmit.skip_txqueue flag is firstly cleared in __dev_queue_xmit().
- Tx Qdisc may install that skbedit actions, then xmit.skip_txqueue flag
is set in qdisc->enqueue() though tx queue has been selected in
netdev_tx_queue_mapping() or netdev_core_pick_tx(). That flag is cleared
firstly in __dev_queue_xmit(), is useful:
- Avoid picking Tx queue with netdev_tx_queue_mapping() in next netdev
in such case: eth0 macvlan - eth0.3 vlan - eth0 ixgbe-phy:
For example, eth0, macvlan in pod, which root Qdisc install skbedit
queue_mapping, send packets to eth0.3, vlan in host. In __dev_queue_xmit() of
eth0.3, clear the flag, does not select tx queue according to skb->queue_mapping
because there is no filters in clsact or tx Qdisc of this netdev.
Same action taked in eth0, ixgbe in Host.
- Avoid picking Tx queue for next packet. If we set xmit.skip_txqueue
in tx Qdisc (qdisc->enqueue()), the proper way to clear it is clearing it
in __dev_queue_xmit when processing next packets.
+----+ +----+ +----+
| P1 | | P2 | | Pn |
+----+ +----+ +----+
| | |
+-----------+-----------+
|
| clsact/skbedit
| MQ
v
+-----------+-----------+
| q0 | q1 | qn
v v v
HTB/FQ HTB/FQ ... FIFO
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Cong Wang <redacted>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jonathan Lemon <redacted>
Cc: Eric Dumazet <redacted>
Cc: Alexander Lobakin <redacted>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Talal Ahmad <redacted>
Cc: Kevin Hao <redacted>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Kees Cook <redacted>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Antoine Tenart <atenart@kernel.org>
Cc: Wei Wang <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Eric Dumazet <redacted>
I have not suggested this patch, only to not add yet another bit in sk_buff.
@@ -3009,6 +3009,7 @@ struct softnet_data {/* written and read only by owning cpu: */struct{u16recursion;+u8skip_txqueue;u8more;}xmit;#ifdef CONFIG_RPS
On Mon, Dec 20, 2021 at 10:21 PM Tonghao Zhang
[off-list ref] wrote:
On Mon, Dec 20, 2021 at 9:58 PM Eric Dumazet [off-list ref] wrote:
quoted
On Mon, Dec 20, 2021 at 4:38 AM [off-list ref] wrote:
quoted
From: Tonghao Zhang <redacted>
This patch fixes issue:
* If we install tc filters with act_skbedit in clsact hook.
It doesn't work, because netdev_core_pick_tx() overwrites
queue_mapping.
$ tc filter ... action skbedit queue_mapping 1
And this patch is useful:
* We can use FQ + EDT to implement efficient policies. Tx queues
are picked by xps, ndo_select_queue of netdev driver, or skb hash
in netdev_core_pick_tx(). In fact, the netdev driver, and skb
hash are _not_ under control. xps uses the CPUs map to select Tx
queues, but we can't figure out which task_struct of pod/containter
running on this cpu in most case. We can use clsact filters to classify
one pod/container traffic to one Tx queue. Why ?
In containter networking environment, there are two kinds of pod/
containter/net-namespace. One kind (e.g. P1, P2), the high throughput
is key in these applications. But avoid running out of network resource,
the outbound traffic of these pods is limited, using or sharing one
dedicated Tx queues assigned HTB/TBF/FQ Qdisc. Other kind of pods
(e.g. Pn), the low latency of data access is key. And the traffic is not
limited. Pods use or share other dedicated Tx queues assigned FIFO Qdisc.
This choice provides two benefits. First, contention on the HTB/FQ Qdisc
lock is significantly reduced since fewer CPUs contend for the same queue.
More importantly, Qdisc contention can be eliminated completely if each
CPU has its own FIFO Qdisc for the second kind of pods.
There must be a mechanism in place to support classifying traffic based on
pods/container to different Tx queues. Note that clsact is outside of Qdisc
while Qdisc can run a classifier to select a sub-queue under the lock.
In general recording the decision in the skb seems a little heavy handed.
This patch introduces a per-CPU variable, suggested by Eric.
The xmit.skip_txqueue flag is firstly cleared in __dev_queue_xmit().
- Tx Qdisc may install that skbedit actions, then xmit.skip_txqueue flag
is set in qdisc->enqueue() though tx queue has been selected in
netdev_tx_queue_mapping() or netdev_core_pick_tx(). That flag is cleared
firstly in __dev_queue_xmit(), is useful:
- Avoid picking Tx queue with netdev_tx_queue_mapping() in next netdev
in such case: eth0 macvlan - eth0.3 vlan - eth0 ixgbe-phy:
For example, eth0, macvlan in pod, which root Qdisc install skbedit
queue_mapping, send packets to eth0.3, vlan in host. In __dev_queue_xmit() of
eth0.3, clear the flag, does not select tx queue according to skb->queue_mapping
because there is no filters in clsact or tx Qdisc of this netdev.
Same action taked in eth0, ixgbe in Host.
- Avoid picking Tx queue for next packet. If we set xmit.skip_txqueue
in tx Qdisc (qdisc->enqueue()), the proper way to clear it is clearing it
in __dev_queue_xmit when processing next packets.
+----+ +----+ +----+
| P1 | | P2 | | Pn |
+----+ +----+ +----+
| | |
+-----------+-----------+
|
| clsact/skbedit
| MQ
v
+-----------+-----------+
| q0 | q1 | qn
v v v
HTB/FQ HTB/FQ ... FIFO
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Cong Wang <redacted>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Jonathan Lemon <redacted>
Cc: Eric Dumazet <redacted>
Cc: Alexander Lobakin <redacted>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Talal Ahmad <redacted>
Cc: Kevin Hao <redacted>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Kees Cook <redacted>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: Antoine Tenart <atenart@kernel.org>
Cc: Wei Wang <redacted>
Cc: Arnd Bergmann <arnd@arndb.de>
Suggested-by: Eric Dumazet <redacted>
I have not suggested this patch, only to not add yet another bit in sk_buff.
@@ -3009,6 +3009,7 @@ struct softnet_data {/* written and read only by owning cpu: */struct{u16recursion;+u8skip_txqueue;u8more;}xmit;#ifdef CONFIG_RPS
If we really need to add yet another conditional in fast path, I would
suggest using a static key.
Thanks, I will add a static key for this patch.
quoted
Only hosts where SKBEDIT_F_QUEUE_MAPPING is requested would pay the price.
Hi Eirc
Do you mean we need a static key, such as, egress_needed_key ?
static DEFINE_STATIC_KEY_FALSE(egress_needed_key);
and we also should use the per-cpu var to decide to select tx from
netdev_tx_queue_mapping or netdev_core_pick_tx.
From: Cong Wang <hidden> Date: 2021-12-20 17:51:45
On Mon, Dec 20, 2021 at 4:38 AM [off-list ref] wrote:
In general recording the decision in the skb seems a little heavy handed.
This patch introduces a per-CPU variable, suggested by Eric.
The xmit.skip_txqueue flag is firstly cleared in __dev_queue_xmit().
- Tx Qdisc may install that skbedit actions, then xmit.skip_txqueue flag
is set in qdisc->enqueue() though tx queue has been selected in
netdev_tx_queue_mapping() or netdev_core_pick_tx(). That flag is cleared
firstly in __dev_queue_xmit(), is useful:
- Avoid picking Tx queue with netdev_tx_queue_mapping() in next netdev
in such case: eth0 macvlan - eth0.3 vlan - eth0 ixgbe-phy:
For example, eth0, macvlan in pod, which root Qdisc install skbedit
queue_mapping, send packets to eth0.3, vlan in host. In __dev_queue_xmit() of
eth0.3, clear the flag, does not select tx queue according to skb->queue_mapping
because there is no filters in clsact or tx Qdisc of this netdev.
Same action taked in eth0, ixgbe in Host.
- Avoid picking Tx queue for next packet. If we set xmit.skip_txqueue
in tx Qdisc (qdisc->enqueue()), the proper way to clear it is clearing it
in __dev_queue_xmit when processing next packets.
Any reason why we can't just move sch_handle_egress() down after
netdev_core_pick_tx() and recalculate txq with skb->queue_mapping?
Thanks.
From: Cong Wang <hidden> Date: 2021-12-20 17:57:28
On Mon, Dec 20, 2021 at 4:39 AM [off-list ref] wrote:
From: Tonghao Zhang <redacted>
This patch allows user to select queue_mapping, range
from A to B. And user can use skbhash, cgroup classid
and cpuid to select Tx queues. Then we can load balance
packets from A to B queue. The range is an unsigned 16bit
value in decimal format.
$ tc filter ... action skbedit queue_mapping skbhash A B
"skbedit queue_mapping QUEUE_MAPPING" (from "man 8 tc-skbedit")
is enhanced with flags:
* SKBEDIT_F_TXQ_SKBHASH
* SKBEDIT_F_TXQ_CLASSID
* SKBEDIT_F_TXQ_CPUID
Once again, you are enforcing policies in kernel, which is not good.
Kernel should just set whatever you give to it, not selecting policies
like a menu.
Any reason why you can't obtain these values in user-space?
On Tue, Dec 21, 2021 at 1:51 AM Cong Wang [off-list ref] wrote:
On Mon, Dec 20, 2021 at 4:38 AM [off-list ref] wrote:
quoted
In general recording the decision in the skb seems a little heavy handed.
This patch introduces a per-CPU variable, suggested by Eric.
The xmit.skip_txqueue flag is firstly cleared in __dev_queue_xmit().
- Tx Qdisc may install that skbedit actions, then xmit.skip_txqueue flag
is set in qdisc->enqueue() though tx queue has been selected in
netdev_tx_queue_mapping() or netdev_core_pick_tx(). That flag is cleared
firstly in __dev_queue_xmit(), is useful:
- Avoid picking Tx queue with netdev_tx_queue_mapping() in next netdev
in such case: eth0 macvlan - eth0.3 vlan - eth0 ixgbe-phy:
For example, eth0, macvlan in pod, which root Qdisc install skbedit
queue_mapping, send packets to eth0.3, vlan in host. In __dev_queue_xmit() of
eth0.3, clear the flag, does not select tx queue according to skb->queue_mapping
because there is no filters in clsact or tx Qdisc of this netdev.
Same action taked in eth0, ixgbe in Host.
- Avoid picking Tx queue for next packet. If we set xmit.skip_txqueue
in tx Qdisc (qdisc->enqueue()), the proper way to clear it is clearing it
in __dev_queue_xmit when processing next packets.
Any reason why we can't just move sch_handle_egress() down after
netdev_core_pick_tx() and recalculate txq with skb->queue_mapping?
If we sch_handle_egress() down after netdev_core_pick_tx(), and we
overwrite the txqueue, the overhead of
netdev_core_pick_tx() is unnecessary, so I don't do that . If
eth0(macvlan) -> eth0.3(vlan) -> eth0(ixgbe), skb-> queue_mapping is
not cleared,
skb-> queue_mapping affect every netdevice to pick the tx queue.
On Tue, Dec 21, 2021 at 1:57 AM Cong Wang [off-list ref] wrote:
On Mon, Dec 20, 2021 at 4:39 AM [off-list ref] wrote:
quoted
From: Tonghao Zhang <redacted>
This patch allows user to select queue_mapping, range
from A to B. And user can use skbhash, cgroup classid
and cpuid to select Tx queues. Then we can load balance
packets from A to B queue. The range is an unsigned 16bit
value in decimal format.
$ tc filter ... action skbedit queue_mapping skbhash A B
"skbedit queue_mapping QUEUE_MAPPING" (from "man 8 tc-skbedit")
is enhanced with flags:
* SKBEDIT_F_TXQ_SKBHASH
* SKBEDIT_F_TXQ_CLASSID
* SKBEDIT_F_TXQ_CPUID
Once again, you are enforcing policies in kernel, which is not good.
Kernel should just set whatever you give to it, not selecting policies
like a menu.
I agree that, but for tc/net-sched layer, there are a lot of
networking policies, for example , cls_A, act_B, sch_C.
Any reason why you can't obtain these values in user-space?
Did you mean that we add this flags to iproute2 tools? This patch for
iproute2, is not post. If the kerenl patches are accepted, I will send
them.
--
Best regards, Tonghao
From: Cong Wang <hidden> Date: 2021-12-24 19:26:53
On Tue, Dec 21, 2021 at 5:24 PM Tonghao Zhang [off-list ref] wrote:
On Tue, Dec 21, 2021 at 1:57 AM Cong Wang [off-list ref] wrote:
quoted
On Mon, Dec 20, 2021 at 4:39 AM [off-list ref] wrote:
quoted
From: Tonghao Zhang <redacted>
This patch allows user to select queue_mapping, range
from A to B. And user can use skbhash, cgroup classid
and cpuid to select Tx queues. Then we can load balance
packets from A to B queue. The range is an unsigned 16bit
value in decimal format.
$ tc filter ... action skbedit queue_mapping skbhash A B
"skbedit queue_mapping QUEUE_MAPPING" (from "man 8 tc-skbedit")
is enhanced with flags:
* SKBEDIT_F_TXQ_SKBHASH
* SKBEDIT_F_TXQ_CLASSID
* SKBEDIT_F_TXQ_CPUID
Once again, you are enforcing policies in kernel, which is not good.
Kernel should just set whatever you give to it, not selecting policies
like a menu.
I agree that, but for tc/net-sched layer, there are a lot of
If you agree, why still move on with this patch? Apparently
you don't. ;)
networking policies, for example , cls_A, act_B, sch_C.
You are justifying your logic by shifting the topics here.
The qdisc algorithm is very different from your case, it is essentially
hard, if not impossible, to completely move to user-space. Even if we
had eBPF based Qdisc, its programmablility is still very limited.
Your code is much much much easier, which is essentially one-line,
hence you have to reason to compare it with this, nor you can even
justify it.
quoted
Any reason why you can't obtain these values in user-space?
Did you mean that we add this flags to iproute2 tools? This patch for
iproute2, is not post. If the kerenl patches are accepted, I will send
them.
Nope, I mean you can for example, obtain the CPU ID in user-space
and pass it directly to _current_ act_skbedit as it is.
Thanks.
On Sat, Dec 25, 2021 at 3:26 AM Cong Wang [off-list ref] wrote:
On Tue, Dec 21, 2021 at 5:24 PM Tonghao Zhang [off-list ref] wrote:
quoted
On Tue, Dec 21, 2021 at 1:57 AM Cong Wang [off-list ref] wrote:
quoted
On Mon, Dec 20, 2021 at 4:39 AM [off-list ref] wrote:
quoted
From: Tonghao Zhang <redacted>
This patch allows user to select queue_mapping, range
from A to B. And user can use skbhash, cgroup classid
and cpuid to select Tx queues. Then we can load balance
packets from A to B queue. The range is an unsigned 16bit
value in decimal format.
$ tc filter ... action skbedit queue_mapping skbhash A B
"skbedit queue_mapping QUEUE_MAPPING" (from "man 8 tc-skbedit")
is enhanced with flags:
* SKBEDIT_F_TXQ_SKBHASH
* SKBEDIT_F_TXQ_CLASSID
* SKBEDIT_F_TXQ_CPUID
Once again, you are enforcing policies in kernel, which is not good.
Kernel should just set whatever you give to it, not selecting policies
like a menu.
I agree that, but for tc/net-sched layer, there are a lot of
If you agree, why still move on with this patch? Apparently
you don't. ;)
quoted
networking policies, for example , cls_A, act_B, sch_C.
You are justifying your logic by shifting the topics here.
Hi Cong,
I mean that if the "policies" are networking policies, I don't agree
with you, because
the TC(or net sched layer), are networking policies, right ? and this
patch enhances the original feature of TC.
The qdisc algorithm is very different from your case, it is essentially
hard, if not impossible, to completely move to user-space. Even if we
had eBPF based Qdisc, its programmablility is still very limited.
Your code is much much much easier, which is essentially one-line,
hence you have to reason to compare it with this, nor you can even
justify it.
I think we have talked about this in another thread. And other
maintainers comment on this.
quoted
quoted
Any reason why you can't obtain these values in user-space?
Did you mean that we add this flags to iproute2 tools? This patch for
iproute2, is not post. If the kerenl patches are accepted, I will send
them.
Nope, I mean you can for example, obtain the CPU ID in user-space
and pass it directly to _current_ act_skbedit as it is.
This flag is passed from userspace. Do you mean the cpuid value should
be passed from userspace?
If so, this is not what we need. because we don't know which one cpu
the pod(containers) will use. and for skb-hash/classid
this helps us to pick the tx queue from a range. I don't understand
why this value should be passed from userspace. How ?
can you give me examples ?
BTW, this patch is the core of patchset.
On Sat, Dec 25, 2021 at 9:41 AM Tonghao Zhang [off-list ref] wrote:
On Sat, Dec 25, 2021 at 3:26 AM Cong Wang [off-list ref] wrote:
quoted
On Tue, Dec 21, 2021 at 5:24 PM Tonghao Zhang [off-list ref] wrote:
quoted
On Tue, Dec 21, 2021 at 1:57 AM Cong Wang [off-list ref] wrote:
quoted
On Mon, Dec 20, 2021 at 4:39 AM [off-list ref] wrote:
quoted
From: Tonghao Zhang <redacted>
This patch allows user to select queue_mapping, range
from A to B. And user can use skbhash, cgroup classid
and cpuid to select Tx queues. Then we can load balance
packets from A to B queue. The range is an unsigned 16bit
value in decimal format.
$ tc filter ... action skbedit queue_mapping skbhash A B
"skbedit queue_mapping QUEUE_MAPPING" (from "man 8 tc-skbedit")
is enhanced with flags:
* SKBEDIT_F_TXQ_SKBHASH
* SKBEDIT_F_TXQ_CLASSID
* SKBEDIT_F_TXQ_CPUID
Once again, you are enforcing policies in kernel, which is not good.
Kernel should just set whatever you give to it, not selecting policies
like a menu.
I agree that, but for tc/net-sched layer, there are a lot of
If you agree, why still move on with this patch? Apparently
you don't. ;)
quoted
networking policies, for example , cls_A, act_B, sch_C.
You are justifying your logic by shifting the topics here.
Hi Cong,
I mean that if the "policies" are networking policies, I don't agree
with you, because
the TC(or net sched layer), are networking policies, right ? and this
patch enhances the original feature of TC.
quoted
The qdisc algorithm is very different from your case, it is essentially
hard, if not impossible, to completely move to user-space. Even if we
had eBPF based Qdisc, its programmablility is still very limited.
Your code is much much much easier, which is essentially one-line,
hence you have to reason to compare it with this, nor you can even
justify it.
I think we have talked about this in another thread. And other
maintainers comment on this.
quoted
quoted
quoted
Any reason why you can't obtain these values in user-space?
Did you mean that we add this flags to iproute2 tools? This patch for
iproute2, is not post. If the kerenl patches are accepted, I will send
them.
Nope, I mean you can for example, obtain the CPU ID in user-space
and pass it directly to _current_ act_skbedit as it is.
This flag is passed from userspace. Do you mean the cpuid value should
be passed from userspace?
If so, this is not what we need. because we don't know which one cpu
the pod(containers) will use. and for skb-hash/classid
this helps us to pick the tx queue from a range. I don't understand
why this value should be passed from userspace. How ?
can you give me examples ?
From: Vinicius Costa Gomes <vinicius.gomes@intel.com> Date: 2021-12-29 19:14:04
xiangxia.m.yue@gmail.com writes:
From: Tonghao Zhang <redacted>
This patch fixes issue:
* If we install tc filters with act_skbedit in clsact hook.
It doesn't work, because netdev_core_pick_tx() overwrites
queue_mapping.
$ tc filter ... action skbedit queue_mapping 1
And this patch is useful:
* We can use FQ + EDT to implement efficient policies. Tx queues
are picked by xps, ndo_select_queue of netdev driver, or skb hash
in netdev_core_pick_tx(). In fact, the netdev driver, and skb
hash are _not_ under control. xps uses the CPUs map to select Tx
queues, but we can't figure out which task_struct of pod/containter
running on this cpu in most case. We can use clsact filters to classify
one pod/container traffic to one Tx queue. Why ?
In containter networking environment, there are two kinds of pod/
containter/net-namespace. One kind (e.g. P1, P2), the high throughput
is key in these applications. But avoid running out of network resource,
the outbound traffic of these pods is limited, using or sharing one
dedicated Tx queues assigned HTB/TBF/FQ Qdisc. Other kind of pods
(e.g. Pn), the low latency of data access is key. And the traffic is not
limited. Pods use or share other dedicated Tx queues assigned FIFO Qdisc.
This choice provides two benefits. First, contention on the HTB/FQ Qdisc
lock is significantly reduced since fewer CPUs contend for the same queue.
More importantly, Qdisc contention can be eliminated completely if each
CPU has its own FIFO Qdisc for the second kind of pods.
There must be a mechanism in place to support classifying traffic based on
pods/container to different Tx queues. Note that clsact is outside of Qdisc
while Qdisc can run a classifier to select a sub-queue under the
lock.
One alternative, I don't know if it would work for you, it to use the
net_prio cgroup + mqprio.
Something like this:
* create the cgroup
$ mkdir -p /sys/fs/cgroup/net_prio/<CGROUP_NAME>
* assign priorities to the cgroup (per interface)
$ echo "<IFACE> <PRIO>" >> /sys/fs/cgroup/net_prio/<CGROUP_NAME>/net_prio.ifpriomap"
* use the cgroup in applications that do not set SO_PRIORITY
$ cgexec -g net_prio:<CGROUP_NAME> <application>
* configure mqprio
$ tc qdisc replace dev $IFACE parent root handle 100 mqprio \
num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
queues 1@0 1@1 2@2 \
hw 0
This would map all traffic with SO_PRIORITY 3 to TX queue 0, for example.
But I agree that skbedit's queue_mapping not working is unexpected and
should be fixed.
Cheers,
--
Vinicius
On Thu, Dec 30, 2021 at 3:14 AM Vinicius Costa Gomes
[off-list ref] wrote:
xiangxia.m.yue@gmail.com writes:
quoted
From: Tonghao Zhang <redacted>
This patch fixes issue:
* If we install tc filters with act_skbedit in clsact hook.
It doesn't work, because netdev_core_pick_tx() overwrites
queue_mapping.
$ tc filter ... action skbedit queue_mapping 1
And this patch is useful:
* We can use FQ + EDT to implement efficient policies. Tx queues
are picked by xps, ndo_select_queue of netdev driver, or skb hash
in netdev_core_pick_tx(). In fact, the netdev driver, and skb
hash are _not_ under control. xps uses the CPUs map to select Tx
queues, but we can't figure out which task_struct of pod/containter
running on this cpu in most case. We can use clsact filters to classify
one pod/container traffic to one Tx queue. Why ?
In containter networking environment, there are two kinds of pod/
containter/net-namespace. One kind (e.g. P1, P2), the high throughput
is key in these applications. But avoid running out of network resource,
the outbound traffic of these pods is limited, using or sharing one
dedicated Tx queues assigned HTB/TBF/FQ Qdisc. Other kind of pods
(e.g. Pn), the low latency of data access is key. And the traffic is not
limited. Pods use or share other dedicated Tx queues assigned FIFO Qdisc.
This choice provides two benefits. First, contention on the HTB/FQ Qdisc
lock is significantly reduced since fewer CPUs contend for the same queue.
More importantly, Qdisc contention can be eliminated completely if each
CPU has its own FIFO Qdisc for the second kind of pods.
There must be a mechanism in place to support classifying traffic based on
pods/container to different Tx queues. Note that clsact is outside of Qdisc
while Qdisc can run a classifier to select a sub-queue under the
lock.
One alternative, I don't know if it would work for you, it to use the
net_prio cgroup + mqprio.
Something like this:
* create the cgroup
$ mkdir -p /sys/fs/cgroup/net_prio/<CGROUP_NAME>
* assign priorities to the cgroup (per interface)
$ echo "<IFACE> <PRIO>" >> /sys/fs/cgroup/net_prio/<CGROUP_NAME>/net_prio.ifpriomap"
* use the cgroup in applications that do not set SO_PRIORITY
$ cgexec -g net_prio:<CGROUP_NAME> <application>
* configure mqprio
$ tc qdisc replace dev $IFACE parent root handle 100 mqprio \
num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
queues 1@0 1@1 2@2 \
hw 0
This would map all traffic with SO_PRIORITY 3 to TX queue 0, for example.
But I agree that skbedit's queue_mapping not working is unexpected and
should be fixed.
Cheers,
--
Vinicius
On Wed, Dec 29, 2021 at 8:50 PM Tonghao Zhang [off-list ref] wrote:
On Sat, Dec 25, 2021 at 9:41 AM Tonghao Zhang [off-list ref] wrote:
quoted
On Sat, Dec 25, 2021 at 3:26 AM Cong Wang [off-list ref] wrote:
quoted
On Tue, Dec 21, 2021 at 5:24 PM Tonghao Zhang [off-list ref] wrote:
quoted
On Tue, Dec 21, 2021 at 1:57 AM Cong Wang [off-list ref] wrote:
quoted
On Mon, Dec 20, 2021 at 4:39 AM [off-list ref] wrote:
quoted
From: Tonghao Zhang <redacted>
This patch allows user to select queue_mapping, range
from A to B. And user can use skbhash, cgroup classid
and cpuid to select Tx queues. Then we can load balance
packets from A to B queue. The range is an unsigned 16bit
value in decimal format.
$ tc filter ... action skbedit queue_mapping skbhash A B
"skbedit queue_mapping QUEUE_MAPPING" (from "man 8 tc-skbedit")
is enhanced with flags:
* SKBEDIT_F_TXQ_SKBHASH
* SKBEDIT_F_TXQ_CLASSID
* SKBEDIT_F_TXQ_CPUID
Once again, you are enforcing policies in kernel, which is not good.
Kernel should just set whatever you give to it, not selecting policies
like a menu.
I agree that, but for tc/net-sched layer, there are a lot of
If you agree, why still move on with this patch? Apparently
you don't. ;)
quoted
networking policies, for example , cls_A, act_B, sch_C.
You are justifying your logic by shifting the topics here.
Hi Cong,
I mean that if the "policies" are networking policies, I don't agree
with you, because
the TC(or net sched layer), are networking policies, right ? and this
patch enhances the original feature of TC.
quoted
The qdisc algorithm is very different from your case, it is essentially
hard, if not impossible, to completely move to user-space. Even if we
had eBPF based Qdisc, its programmablility is still very limited.
Your code is much much much easier, which is essentially one-line,
hence you have to reason to compare it with this, nor you can even
justify it.
I think we have talked about this in another thread. And other
maintainers comment on this.
quoted
quoted
quoted
Any reason why you can't obtain these values in user-space?
Did you mean that we add this flags to iproute2 tools? This patch for
iproute2, is not post. If the kerenl patches are accepted, I will send
them.
Nope, I mean you can for example, obtain the CPU ID in user-space
and pass it directly to _current_ act_skbedit as it is.
This flag is passed from userspace. Do you mean the cpuid value should
be passed from userspace?
If so, this is not what we need. because we don't know which one cpu
the pod(containers) will use. and for skb-hash/classid
this helps us to pick the tx queue from a range. I don't understand
why this value should be passed from userspace. How ?
can you give me examples ?
friendly ping
still waiting for your reply, and Jakub's questions in other thread.
If possible, I plan to resend the patchset.
From: Cong Wang <hidden> Date: 2022-01-26 19:59:05
On Wed, Dec 29, 2021 at 9:03 PM Tonghao Zhang [off-list ref] wrote:
Yes, I think this is key. In k8s, we can not control the priority of
applications in Pod. and I think 2/2 patch
Why not? Please be more specific.
can provide more mechanisms to select queues from a range.
Well, you just keep ignoring eBPF action which provides
literally infinitely more choices than yours. So, you want more
choices but still refuse using eBPF action, what point are you
trying to make?
Thanks.
On Thu, Jan 27, 2022 at 3:59 AM Cong Wang [off-list ref] wrote:
On Wed, Dec 29, 2021 at 9:03 PM Tonghao Zhang [off-list ref] wrote:
quoted
Yes, I think this is key. In k8s, we can not control the priority of
applications in Pod. and I think 2/2 patch
Why not? Please be more specific.
I mean the priority of skb can be set via the socket from
applications. the application is running for a long time,
There are so many applications in the company. we can't ask them to do
that. there is a lot of workload.
quoted
can provide more mechanisms to select queues from a range.
Well, you just keep ignoring eBPF action which provides
literally infinitely more choices than yours. So, you want more
choices but still refuse using eBPF action, what point are you
trying to make?
Maybe I can answer your question in another thread. In brief, this
patch is only enhanced the tc-skbedit.