* Re: [net-next v10 1/2] net: sched: use queue_mapping to pick tx queue
[not found] ` <20220314141508.39952-2-xiangxia.m.yue@gmail.com>
@ 2022-03-14 21:59 ` Daniel Borkmann
2022-03-15 12:48 ` Tonghao Zhang
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2022-03-14 21:59 UTC (permalink / raw)
To: xiangxia.m.yue, netdev
Cc: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S. Miller,
Jakub Kicinski, Jonathan Lemon, Eric Dumazet, Alexander Lobakin,
Paolo Abeni, Talal Ahmad, Kevin Hao, ast, bpf
On 3/14/22 3:15 PM, xiangxia.m.yue@gmail.com wrote:
[...]
> include/linux/netdevice.h | 3 +++
> include/linux/rtnetlink.h | 1 +
> net/core/dev.c | 31 +++++++++++++++++++++++++++++--
> net/sched/act_skbedit.c | 6 +++++-
> 4 files changed, 38 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 0d994710b335..f33fb2d6712a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3065,6 +3065,9 @@ struct softnet_data {
> struct {
> u16 recursion;
> u8 more;
> +#ifdef CONFIG_NET_EGRESS
> + u8 skip_txqueue;
> +#endif
> } xmit;
> #ifdef CONFIG_RPS
> /* input_queue_head should be written by cpu owning this struct,
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index 7f970b16da3a..ae2c6a3cec5d 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -100,6 +100,7 @@ void net_dec_ingress_queue(void);
> #ifdef CONFIG_NET_EGRESS
> void net_inc_egress_queue(void);
> void net_dec_egress_queue(void);
> +void netdev_xmit_skip_txqueue(bool skip);
> #endif
>
> void rtnetlink_init(void);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 75bab5b0dbae..8e83b7099977 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3908,6 +3908,25 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
>
> return skb;
> }
> +
> +static struct netdev_queue *
> +netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
> +{
> + int qm = skb_get_queue_mapping(skb);
> +
> + return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
> +}
> +
> +static bool netdev_xmit_txqueue_skipped(void)
> +{
> + return __this_cpu_read(softnet_data.xmit.skip_txqueue);
> +}
> +
> +void netdev_xmit_skip_txqueue(bool skip)
> +{
> + __this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
> +}
> +EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
> #endif /* CONFIG_NET_EGRESS */
>
> #ifdef CONFIG_XPS
> @@ -4078,7 +4097,7 @@ struct netdev_queue *netdev_core_pick_tx(struct net_device *dev,
> static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> {
> struct net_device *dev = skb->dev;
> - struct netdev_queue *txq;
> + struct netdev_queue *txq = NULL;
> struct Qdisc *q;
> int rc = -ENOMEM;
> bool again = false;
> @@ -4106,11 +4125,17 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> if (!skb)
> goto out;
> }
> +
> + netdev_xmit_skip_txqueue(false);
> +
> nf_skip_egress(skb, true);
> skb = sch_handle_egress(skb, &rc, dev);
> if (!skb)
> goto out;
> nf_skip_egress(skb, false);
> +
> + if (netdev_xmit_txqueue_skipped())
> + txq = netdev_tx_queue_mapping(dev, skb);
> }
> #endif
> /* If device/qdisc don't need skb->dst, release it right now while
> @@ -4121,7 +4146,9 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> else
> skb_dst_force(skb);
>
> - txq = netdev_core_pick_tx(dev, skb, sb_dev);
> + if (likely(!txq))
nit: Drop likely(). If the feature is used from sch_handle_egress(), then this would always be the case.
> + txq = netdev_core_pick_tx(dev, skb, sb_dev);
> +
> q = rcu_dereference_bh(txq->qdisc);
How will the `netdev_xmit_skip_txqueue(true)` be usable from BPF side (see bpf_convert_ctx_access() ->
queue_mapping)?
> trace_net_dev_queue(skb);
> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> index ceba11b198bb..d5799b4fc499 100644
> --- a/net/sched/act_skbedit.c
> +++ b/net/sched/act_skbedit.c
> @@ -58,8 +58,12 @@ static int tcf_skbedit_act(struct sk_buff *skb, const struct tc_action *a,
> }
> }
> if (params->flags & SKBEDIT_F_QUEUE_MAPPING &&
> - skb->dev->real_num_tx_queues > params->queue_mapping)
> + skb->dev->real_num_tx_queues > params->queue_mapping) {
> +#ifdef CONFIG_NET_EGRESS
> + netdev_xmit_skip_txqueue(true);
> +#endif
> skb_set_queue_mapping(skb, params->queue_mapping);
> + }
> if (params->flags & SKBEDIT_F_MARK) {
> skb->mark &= ~params->mask;
> skb->mark |= params->mark & params->mask;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net-next v10 1/2] net: sched: use queue_mapping to pick tx queue
2022-03-14 21:59 ` [net-next v10 1/2] net: sched: use queue_mapping to pick tx queue Daniel Borkmann
@ 2022-03-15 12:48 ` Tonghao Zhang
2022-03-17 8:20 ` Paolo Abeni
0 siblings, 1 reply; 5+ messages in thread
From: Tonghao Zhang @ 2022-03-15 12:48 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Cong Wang,
Jiri Pirko, David S. Miller, Jakub Kicinski, Jonathan Lemon,
Eric Dumazet, Alexander Lobakin, Paolo Abeni, Talal Ahmad,
Kevin Hao, Alexei Starovoitov, bpf
On Tue, Mar 15, 2022 at 5:59 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 3/14/22 3:15 PM, xiangxia.m.yue@gmail.com wrote:
> [...]
> > include/linux/netdevice.h | 3 +++
> > include/linux/rtnetlink.h | 1 +
> > net/core/dev.c | 31 +++++++++++++++++++++++++++++--
> > net/sched/act_skbedit.c | 6 +++++-
> > 4 files changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 0d994710b335..f33fb2d6712a 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -3065,6 +3065,9 @@ struct softnet_data {
> > struct {
> > u16 recursion;
> > u8 more;
> > +#ifdef CONFIG_NET_EGRESS
> > + u8 skip_txqueue;
> > +#endif
> > } xmit;
> > #ifdef CONFIG_RPS
> > /* input_queue_head should be written by cpu owning this struct,
> > diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> > index 7f970b16da3a..ae2c6a3cec5d 100644
> > --- a/include/linux/rtnetlink.h
> > +++ b/include/linux/rtnetlink.h
> > @@ -100,6 +100,7 @@ void net_dec_ingress_queue(void);
> > #ifdef CONFIG_NET_EGRESS
> > void net_inc_egress_queue(void);
> > void net_dec_egress_queue(void);
> > +void netdev_xmit_skip_txqueue(bool skip);
> > #endif
> >
> > void rtnetlink_init(void);
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 75bab5b0dbae..8e83b7099977 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -3908,6 +3908,25 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
> >
> > return skb;
> > }
> > +
> > +static struct netdev_queue *
> > +netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
> > +{
> > + int qm = skb_get_queue_mapping(skb);
> > +
> > + return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
> > +}
> > +
> > +static bool netdev_xmit_txqueue_skipped(void)
> > +{
> > + return __this_cpu_read(softnet_data.xmit.skip_txqueue);
> > +}
> > +
> > +void netdev_xmit_skip_txqueue(bool skip)
> > +{
> > + __this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
> > +}
> > +EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
> > #endif /* CONFIG_NET_EGRESS */
> >
> > #ifdef CONFIG_XPS
> > @@ -4078,7 +4097,7 @@ struct netdev_queue *netdev_core_pick_tx(struct net_device *dev,
> > static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> > {
> > struct net_device *dev = skb->dev;
> > - struct netdev_queue *txq;
> > + struct netdev_queue *txq = NULL;
> > struct Qdisc *q;
> > int rc = -ENOMEM;
> > bool again = false;
> > @@ -4106,11 +4125,17 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> > if (!skb)
> > goto out;
> > }
> > +
> > + netdev_xmit_skip_txqueue(false);
> > +
> > nf_skip_egress(skb, true);
> > skb = sch_handle_egress(skb, &rc, dev);
> > if (!skb)
> > goto out;
> > nf_skip_egress(skb, false);
> > +
> > + if (netdev_xmit_txqueue_skipped())
> > + txq = netdev_tx_queue_mapping(dev, skb);
> > }
> > #endif
> > /* If device/qdisc don't need skb->dst, release it right now while
> > @@ -4121,7 +4146,9 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> > else
> > skb_dst_force(skb);
> >
> > - txq = netdev_core_pick_tx(dev, skb, sb_dev);
> > + if (likely(!txq))
>
> nit: Drop likely(). If the feature is used from sch_handle_egress(), then this would always be the case.
Hi Daniel
I think in most case, we don't use skbedit queue_mapping in the
sch_handle_egress() , so I add likely in fast path.
> > + txq = netdev_core_pick_tx(dev, skb, sb_dev);
> > +
> > q = rcu_dereference_bh(txq->qdisc);
>
> How will the `netdev_xmit_skip_txqueue(true)` be usable from BPF side (see bpf_convert_ctx_access() ->
> queue_mapping)?
Good questions, In other patch, I introduce the
bpf_netdev_skip_txqueue, so we can use netdev_xmit_skip_txqueue in bpf
side
not official patch:(I will post this patch, if ready)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 4eebea830613..ef147a1a2d62 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5117,6 +5117,21 @@ union bpf_attr {
* 0 on success.
* **-EINVAL** for invalid input
* **-EOPNOTSUPP** for unsupported delivery_time_type and protocol
+ *
+ * void bpf_netdev_skip_txqueue(u32 skip)
+ * Description
+ * Redirect the packet to another net device of index *ifindex*.
+ * This helper is somewhat similar to **bpf_redirect**\ (), except
+ * that the redirection happens to the *skip*' peer device and
+ * the netns switch takes place from ingress to ingress without
+ * going through the CPU's backlog queue.
+ *
+ * The *skip* argument is reserved and must be 0. The helper is
+ * currently only supported for tc BPF program types at the ingress
+ * hook and for veth device types. The peer device must reside in a
+ * different network namespace.
+ * Return
+ * Nothing. Always succeeds.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -5312,6 +5327,7 @@ union bpf_attr {
FN(xdp_store_bytes), \
FN(copy_from_user_task), \
FN(skb_set_delivery_time), \
+ FN(netdev_skip_txqueue), \
/* */
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
diff --git a/net/core/filter.c b/net/core/filter.c
index 88767f7da150..5845b4632b6b 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2517,6 +2517,19 @@ static const struct bpf_func_proto
bpf_redirect_peer_proto = {
.arg2_type = ARG_ANYTHING,
};
+BPF_CALL_1(bpf_netdev_skip_txqueue, u32, skip)
+{
+ netdev_xmit_skip_txqueue(!!skip);
+ return 0;
+};
+
+static const struct bpf_func_proto bpf_netdev_skip_txqueue_proto = {
+ .func = bpf_netdev_skip_txqueue,
+ .gpl_only = false,
+ .ret_type = RET_VOID,
+ .arg1_type = ARG_ANYTHING,
+};
+
BPF_CALL_4(bpf_redirect_neigh, u32, ifindex, struct bpf_redir_neigh *, params,
int, plen, u64, flags)
{
@@ -7721,6 +7734,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id,
const struct bpf_prog *prog)
return &bpf_redirect_proto;
case BPF_FUNC_redirect_neigh:
return &bpf_redirect_neigh_proto;
+ case BPF_FUNC_netdev_skip_txqueue:
+ return &bpf_netdev_skip_txqueue_proto;
case BPF_FUNC_redirect_peer:
return &bpf_redirect_peer_proto;
case BPF_FUNC_get_route_realm:
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 4eebea830613..ef147a1a2d62 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5117,6 +5117,21 @@ union bpf_attr {
* 0 on success.
* **-EINVAL** for invalid input
* **-EOPNOTSUPP** for unsupported delivery_time_type and protocol
+ *
+ * void bpf_netdev_skip_txqueue(u32 skip)
+ * Description
+ * Redirect the packet to another net device of index *ifindex*.
+ * This helper is somewhat similar to **bpf_redirect**\ (), except
+ * that the redirection happens to the *skip*' peer device and
+ * the netns switch takes place from ingress to ingress without
+ * going through the CPU's backlog queue.
+ *
+ * The *skip* argument is reserved and must be 0. The helper is
+ * currently only supported for tc BPF program types at the ingress
+ * hook and for veth device types. The peer device must reside in a
+ * different network namespace.
+ * Return
+ * Nothing. Always succeeds.
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -5312,6 +5327,7 @@ union bpf_attr {
FN(xdp_store_bytes), \
FN(copy_from_user_task), \
FN(skb_set_delivery_time), \
+ FN(netdev_skip_txqueue), \
/* */
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
>
> > trace_net_dev_queue(skb);
> > diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> > index ceba11b198bb..d5799b4fc499 100644
> > --- a/net/sched/act_skbedit.c
> > +++ b/net/sched/act_skbedit.c
> > @@ -58,8 +58,12 @@ static int tcf_skbedit_act(struct sk_buff *skb, const struct tc_action *a,
> > }
> > }
> > if (params->flags & SKBEDIT_F_QUEUE_MAPPING &&
> > - skb->dev->real_num_tx_queues > params->queue_mapping)
> > + skb->dev->real_num_tx_queues > params->queue_mapping) {
> > +#ifdef CONFIG_NET_EGRESS
> > + netdev_xmit_skip_txqueue(true);
> > +#endif
> > skb_set_queue_mapping(skb, params->queue_mapping);
> > + }
> > if (params->flags & SKBEDIT_F_MARK) {
> > skb->mark &= ~params->mask;
> > skb->mark |= params->mark & params->mask;
> >
>
--
Best regards, Tonghao
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [net-next v10 1/2] net: sched: use queue_mapping to pick tx queue
2022-03-15 12:48 ` Tonghao Zhang
@ 2022-03-17 8:20 ` Paolo Abeni
2022-03-18 13:36 ` Daniel Borkmann
0 siblings, 1 reply; 5+ messages in thread
From: Paolo Abeni @ 2022-03-17 8:20 UTC (permalink / raw)
To: Tonghao Zhang, Daniel Borkmann
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Cong Wang,
Jiri Pirko, David S. Miller, Jakub Kicinski, Jonathan Lemon,
Eric Dumazet, Alexander Lobakin, Talal Ahmad, Kevin Hao,
Alexei Starovoitov, bpf
On Tue, 2022-03-15 at 20:48 +0800, Tonghao Zhang wrote:
> On Tue, Mar 15, 2022 at 5:59 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 3/14/22 3:15 PM, xiangxia.m.yue@gmail.com wrote:
> > [...]
> > > include/linux/netdevice.h | 3 +++
> > > include/linux/rtnetlink.h | 1 +
> > > net/core/dev.c | 31 +++++++++++++++++++++++++++++--
> > > net/sched/act_skbedit.c | 6 +++++-
> > > 4 files changed, 38 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index 0d994710b335..f33fb2d6712a 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -3065,6 +3065,9 @@ struct softnet_data {
> > > struct {
> > > u16 recursion;
> > > u8 more;
> > > +#ifdef CONFIG_NET_EGRESS
> > > + u8 skip_txqueue;
> > > +#endif
> > > } xmit;
> > > #ifdef CONFIG_RPS
> > > /* input_queue_head should be written by cpu owning this struct,
> > > diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> > > index 7f970b16da3a..ae2c6a3cec5d 100644
> > > --- a/include/linux/rtnetlink.h
> > > +++ b/include/linux/rtnetlink.h
> > > @@ -100,6 +100,7 @@ void net_dec_ingress_queue(void);
> > > #ifdef CONFIG_NET_EGRESS
> > > void net_inc_egress_queue(void);
> > > void net_dec_egress_queue(void);
> > > +void netdev_xmit_skip_txqueue(bool skip);
> > > #endif
> > >
> > > void rtnetlink_init(void);
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 75bab5b0dbae..8e83b7099977 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -3908,6 +3908,25 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
> > >
> > > return skb;
> > > }
> > > +
> > > +static struct netdev_queue *
> > > +netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
> > > +{
> > > + int qm = skb_get_queue_mapping(skb);
> > > +
> > > + return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
> > > +}
> > > +
> > > +static bool netdev_xmit_txqueue_skipped(void)
> > > +{
> > > + return __this_cpu_read(softnet_data.xmit.skip_txqueue);
> > > +}
> > > +
> > > +void netdev_xmit_skip_txqueue(bool skip)
> > > +{
> > > + __this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
> > > +}
> > > +EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
> > > #endif /* CONFIG_NET_EGRESS */
> > >
> > > #ifdef CONFIG_XPS
> > > @@ -4078,7 +4097,7 @@ struct netdev_queue *netdev_core_pick_tx(struct net_device *dev,
> > > static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> > > {
> > > struct net_device *dev = skb->dev;
> > > - struct netdev_queue *txq;
> > > + struct netdev_queue *txq = NULL;
> > > struct Qdisc *q;
> > > int rc = -ENOMEM;
> > > bool again = false;
> > > @@ -4106,11 +4125,17 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> > > if (!skb)
> > > goto out;
> > > }
> > > +
> > > + netdev_xmit_skip_txqueue(false);
> > > +
> > > nf_skip_egress(skb, true);
> > > skb = sch_handle_egress(skb, &rc, dev);
> > > if (!skb)
> > > goto out;
> > > nf_skip_egress(skb, false);
> > > +
> > > + if (netdev_xmit_txqueue_skipped())
> > > + txq = netdev_tx_queue_mapping(dev, skb);
> > > }
> > > #endif
> > > /* If device/qdisc don't need skb->dst, release it right now while
> > > @@ -4121,7 +4146,9 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> > > else
> > > skb_dst_force(skb);
> > >
> > > - txq = netdev_core_pick_tx(dev, skb, sb_dev);
> > > + if (likely(!txq))
> >
> > nit: Drop likely(). If the feature is used from sch_handle_egress(), then this would always be the case.
> Hi Daniel
> I think in most case, we don't use skbedit queue_mapping in the
> sch_handle_egress() , so I add likely in fast path.
> > > + txq = netdev_core_pick_tx(dev, skb, sb_dev);
> > > +
> > > q = rcu_dereference_bh(txq->qdisc);
> >
> > How will the `netdev_xmit_skip_txqueue(true)` be usable from BPF side (see bpf_convert_ctx_access() ->
> > queue_mapping)?
> Good questions, In other patch, I introduce the
> bpf_netdev_skip_txqueue, so we can use netdev_xmit_skip_txqueue in bpf
> side
@Daniel: are you ok with the above explaination?
Thanks!
Paolo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net-next v10 1/2] net: sched: use queue_mapping to pick tx queue
2022-03-17 8:20 ` Paolo Abeni
@ 2022-03-18 13:36 ` Daniel Borkmann
2022-03-19 13:40 ` Tonghao Zhang
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Borkmann @ 2022-03-18 13:36 UTC (permalink / raw)
To: Paolo Abeni, Tonghao Zhang
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Cong Wang,
Jiri Pirko, David S. Miller, Jakub Kicinski, Jonathan Lemon,
Eric Dumazet, Alexander Lobakin, Talal Ahmad, Kevin Hao,
Alexei Starovoitov, bpf
On 3/17/22 9:20 AM, Paolo Abeni wrote:
> On Tue, 2022-03-15 at 20:48 +0800, Tonghao Zhang wrote:
>> On Tue, Mar 15, 2022 at 5:59 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>> On 3/14/22 3:15 PM, xiangxia.m.yue@gmail.com wrote:
>>> [...]
>>>> include/linux/netdevice.h | 3 +++
>>>> include/linux/rtnetlink.h | 1 +
>>>> net/core/dev.c | 31 +++++++++++++++++++++++++++++--
>>>> net/sched/act_skbedit.c | 6 +++++-
>>>> 4 files changed, 38 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>>> index 0d994710b335..f33fb2d6712a 100644
>>>> --- a/include/linux/netdevice.h
>>>> +++ b/include/linux/netdevice.h
>>>> @@ -3065,6 +3065,9 @@ struct softnet_data {
>>>> struct {
>>>> u16 recursion;
>>>> u8 more;
>>>> +#ifdef CONFIG_NET_EGRESS
>>>> + u8 skip_txqueue;
>>>> +#endif
>>>> } xmit;
>>>> #ifdef CONFIG_RPS
>>>> /* input_queue_head should be written by cpu owning this struct,
>>>> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
>>>> index 7f970b16da3a..ae2c6a3cec5d 100644
>>>> --- a/include/linux/rtnetlink.h
>>>> +++ b/include/linux/rtnetlink.h
>>>> @@ -100,6 +100,7 @@ void net_dec_ingress_queue(void);
>>>> #ifdef CONFIG_NET_EGRESS
>>>> void net_inc_egress_queue(void);
>>>> void net_dec_egress_queue(void);
>>>> +void netdev_xmit_skip_txqueue(bool skip);
>>>> #endif
>>>>
>>>> void rtnetlink_init(void);
>>>> diff --git a/net/core/dev.c b/net/core/dev.c
>>>> index 75bab5b0dbae..8e83b7099977 100644
>>>> --- a/net/core/dev.c
>>>> +++ b/net/core/dev.c
>>>> @@ -3908,6 +3908,25 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
>>>>
>>>> return skb;
>>>> }
>>>> +
>>>> +static struct netdev_queue *
>>>> +netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
>>>> +{
>>>> + int qm = skb_get_queue_mapping(skb);
>>>> +
>>>> + return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
>>>> +}
>>>> +
>>>> +static bool netdev_xmit_txqueue_skipped(void)
>>>> +{
>>>> + return __this_cpu_read(softnet_data.xmit.skip_txqueue);
>>>> +}
>>>> +
>>>> +void netdev_xmit_skip_txqueue(bool skip)
>>>> +{
>>>> + __this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
>>>> #endif /* CONFIG_NET_EGRESS */
>>>>
>>>> #ifdef CONFIG_XPS
>>>> @@ -4078,7 +4097,7 @@ struct netdev_queue *netdev_core_pick_tx(struct net_device *dev,
>>>> static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
>>>> {
>>>> struct net_device *dev = skb->dev;
>>>> - struct netdev_queue *txq;
>>>> + struct netdev_queue *txq = NULL;
>>>> struct Qdisc *q;
>>>> int rc = -ENOMEM;
>>>> bool again = false;
>>>> @@ -4106,11 +4125,17 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
>>>> if (!skb)
>>>> goto out;
>>>> }
>>>> +
>>>> + netdev_xmit_skip_txqueue(false);
>>>> +
>>>> nf_skip_egress(skb, true);
>>>> skb = sch_handle_egress(skb, &rc, dev);
>>>> if (!skb)
>>>> goto out;
>>>> nf_skip_egress(skb, false);
>>>> +
>>>> + if (netdev_xmit_txqueue_skipped())
>>>> + txq = netdev_tx_queue_mapping(dev, skb);
>>>> }
>>>> #endif
>>>> /* If device/qdisc don't need skb->dst, release it right now while
>>>> @@ -4121,7 +4146,9 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
>>>> else
>>>> skb_dst_force(skb);
>>>>
>>>> - txq = netdev_core_pick_tx(dev, skb, sb_dev);
>>>> + if (likely(!txq))
>>>
>>> nit: Drop likely(). If the feature is used from sch_handle_egress(), then this would always be the case.
>> Hi Daniel
>> I think in most case, we don't use skbedit queue_mapping in the
>> sch_handle_egress() , so I add likely in fast path.
Yeah, but then let branch predictor do its work ? We can still change and drop the
likely() once we add support for BPF though..
>>>> + txq = netdev_core_pick_tx(dev, skb, sb_dev);
>>>> +
>>>> q = rcu_dereference_bh(txq->qdisc);
>>>
>>> How will the `netdev_xmit_skip_txqueue(true)` be usable from BPF side (see bpf_convert_ctx_access() ->
>>> queue_mapping)?
>> Good questions, In other patch, I introduce the
>> bpf_netdev_skip_txqueue, so we can use netdev_xmit_skip_txqueue in bpf
>> side
Yeah, that bpf_netdev_skip_txqueue() won't fly. It's basically a helper doing quirk for
an implementation detail (aka calling netdev_xmit_skip_txqueue()). Was hoping you have
something better we could use along with the context rewrite of __sk_buff's queue_mapping,
but worst case we need to rework a bit for BPF. :/
Thanks,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net-next v10 1/2] net: sched: use queue_mapping to pick tx queue
2022-03-18 13:36 ` Daniel Borkmann
@ 2022-03-19 13:40 ` Tonghao Zhang
0 siblings, 0 replies; 5+ messages in thread
From: Tonghao Zhang @ 2022-03-19 13:40 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Paolo Abeni, Linux Kernel Network Developers, Jamal Hadi Salim,
Cong Wang, Jiri Pirko, David S. Miller, Jakub Kicinski,
Jonathan Lemon, Eric Dumazet, Alexander Lobakin, Talal Ahmad,
Kevin Hao, Alexei Starovoitov, bpf
On Fri, Mar 18, 2022 at 9:36 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 3/17/22 9:20 AM, Paolo Abeni wrote:
> > On Tue, 2022-03-15 at 20:48 +0800, Tonghao Zhang wrote:
> >> On Tue, Mar 15, 2022 at 5:59 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>> On 3/14/22 3:15 PM, xiangxia.m.yue@gmail.com wrote:
> >>> [...]
> >>>> include/linux/netdevice.h | 3 +++
> >>>> include/linux/rtnetlink.h | 1 +
> >>>> net/core/dev.c | 31 +++++++++++++++++++++++++++++--
> >>>> net/sched/act_skbedit.c | 6 +++++-
> >>>> 4 files changed, 38 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> >>>> index 0d994710b335..f33fb2d6712a 100644
> >>>> --- a/include/linux/netdevice.h
> >>>> +++ b/include/linux/netdevice.h
> >>>> @@ -3065,6 +3065,9 @@ struct softnet_data {
> >>>> struct {
> >>>> u16 recursion;
> >>>> u8 more;
> >>>> +#ifdef CONFIG_NET_EGRESS
> >>>> + u8 skip_txqueue;
> >>>> +#endif
> >>>> } xmit;
> >>>> #ifdef CONFIG_RPS
> >>>> /* input_queue_head should be written by cpu owning this struct,
> >>>> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> >>>> index 7f970b16da3a..ae2c6a3cec5d 100644
> >>>> --- a/include/linux/rtnetlink.h
> >>>> +++ b/include/linux/rtnetlink.h
> >>>> @@ -100,6 +100,7 @@ void net_dec_ingress_queue(void);
> >>>> #ifdef CONFIG_NET_EGRESS
> >>>> void net_inc_egress_queue(void);
> >>>> void net_dec_egress_queue(void);
> >>>> +void netdev_xmit_skip_txqueue(bool skip);
> >>>> #endif
> >>>>
> >>>> void rtnetlink_init(void);
> >>>> diff --git a/net/core/dev.c b/net/core/dev.c
> >>>> index 75bab5b0dbae..8e83b7099977 100644
> >>>> --- a/net/core/dev.c
> >>>> +++ b/net/core/dev.c
> >>>> @@ -3908,6 +3908,25 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
> >>>>
> >>>> return skb;
> >>>> }
> >>>> +
> >>>> +static struct netdev_queue *
> >>>> +netdev_tx_queue_mapping(struct net_device *dev, struct sk_buff *skb)
> >>>> +{
> >>>> + int qm = skb_get_queue_mapping(skb);
> >>>> +
> >>>> + return netdev_get_tx_queue(dev, netdev_cap_txqueue(dev, qm));
> >>>> +}
> >>>> +
> >>>> +static bool netdev_xmit_txqueue_skipped(void)
> >>>> +{
> >>>> + return __this_cpu_read(softnet_data.xmit.skip_txqueue);
> >>>> +}
> >>>> +
> >>>> +void netdev_xmit_skip_txqueue(bool skip)
> >>>> +{
> >>>> + __this_cpu_write(softnet_data.xmit.skip_txqueue, skip);
> >>>> +}
> >>>> +EXPORT_SYMBOL_GPL(netdev_xmit_skip_txqueue);
> >>>> #endif /* CONFIG_NET_EGRESS */
> >>>>
> >>>> #ifdef CONFIG_XPS
> >>>> @@ -4078,7 +4097,7 @@ struct netdev_queue *netdev_core_pick_tx(struct net_device *dev,
> >>>> static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> >>>> {
> >>>> struct net_device *dev = skb->dev;
> >>>> - struct netdev_queue *txq;
> >>>> + struct netdev_queue *txq = NULL;
> >>>> struct Qdisc *q;
> >>>> int rc = -ENOMEM;
> >>>> bool again = false;
> >>>> @@ -4106,11 +4125,17 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> >>>> if (!skb)
> >>>> goto out;
> >>>> }
> >>>> +
> >>>> + netdev_xmit_skip_txqueue(false);
> >>>> +
> >>>> nf_skip_egress(skb, true);
> >>>> skb = sch_handle_egress(skb, &rc, dev);
> >>>> if (!skb)
> >>>> goto out;
> >>>> nf_skip_egress(skb, false);
> >>>> +
> >>>> + if (netdev_xmit_txqueue_skipped())
> >>>> + txq = netdev_tx_queue_mapping(dev, skb);
> >>>> }
> >>>> #endif
> >>>> /* If device/qdisc don't need skb->dst, release it right now while
> >>>> @@ -4121,7 +4146,9 @@ static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> >>>> else
> >>>> skb_dst_force(skb);
> >>>>
> >>>> - txq = netdev_core_pick_tx(dev, skb, sb_dev);
> >>>> + if (likely(!txq))
> >>>
> >>> nit: Drop likely(). If the feature is used from sch_handle_egress(), then this would always be the case.
> >> Hi Daniel
> >> I think in most case, we don't use skbedit queue_mapping in the
> >> sch_handle_egress() , so I add likely in fast path.
>
> Yeah, but then let branch predictor do its work ? We can still change and drop the
> likely() once we add support for BPF though..
Hi
if you are ok that introducing the bpf helper shown below, I will drop
likely() in next patch.
>
> >>>> + txq = netdev_core_pick_tx(dev, skb, sb_dev);
> >>>> +
> >>>> q = rcu_dereference_bh(txq->qdisc);
> >>>
> >>> How will the `netdev_xmit_skip_txqueue(true)` be usable from BPF side (see bpf_convert_ctx_access() ->
> >>> queue_mapping)?
> >> Good questions, In other patch, I introduce the
> >> bpf_netdev_skip_txqueue, so we can use netdev_xmit_skip_txqueue in bpf
> >> side
>
> Yeah, that bpf_netdev_skip_txqueue() won't fly. It's basically a helper doing quirk for
> an implementation detail (aka calling netdev_xmit_skip_txqueue()). Was hoping you have
> something better we could use along with the context rewrite of __sk_buff's queue_mapping,
Hi Daniel
I review the bpf codes, we introduce a lot helper to change the skb field:
skb_change_proto
skb_change_type
skb_change_tail
skb_pull_data
skb_change_head
skb_ecn_set_ce
skb_cgroup_classid
skb_vlan_push
skb_set_tunnel_key
did you mean that, we introduce bpf_skb_set_queue_mapping is better
than bpf_netdev_skip_txqueue.
for example:
BPF_CALL_2(bpf_skb_set_queue_mapping, struct sk_buff *, skb, u32, txq)
{
skb->queue_mapping = txq;
netdev_xmit_skip_txqueue(true);
return 0;
};
> but worst case we need to rework a bit for BPF. :/
> Thanks,
> Daniel
--
Best regards, Tonghao
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-03-19 13:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220314141508.39952-1-xiangxia.m.yue@gmail.com>
[not found] ` <20220314141508.39952-2-xiangxia.m.yue@gmail.com>
2022-03-14 21:59 ` [net-next v10 1/2] net: sched: use queue_mapping to pick tx queue Daniel Borkmann
2022-03-15 12:48 ` Tonghao Zhang
2022-03-17 8:20 ` Paolo Abeni
2022-03-18 13:36 ` Daniel Borkmann
2022-03-19 13:40 ` Tonghao Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox