netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: Jiri Pirko <jiri@resnulli.us>, netdev@vger.kernel.org
Cc: davem@davemloft.net, jhs@mojatatu.com, xiyou.wangcong@gmail.com,
	mlxsw@mellanox.com, andrew@lunn.ch,
	vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com,
	michael.chan@broadcom.com, ganeshgr@chelsio.com,
	jeffrey.t.kirsher@intel.com, saeedm@mellanox.com,
	matanb@mellanox.com, leonro@mellanox.com, idosch@mellanox.com,
	jakub.kicinski@netronome.com, ast@kernel.org,
	simon.horman@netronome.com, pieter.jansenvanvuuren@netronome.com,
	john.hurley@netronome.com, edumazet@google.com,
	dsahern@gmail.com, alexander.h.duyck@intel.com,
	john.fastabend@gmail.com, willemb@google.com
Subject: Re: [patch net-next 06/34] net: core: use dev->ingress_queue instead of tp->q
Date: Thu, 12 Oct 2017 23:45:43 +0200	[thread overview]
Message-ID: <59DFE287.2040400@iogearbox.net> (raw)
In-Reply-To: <20171012171823.1431-7-jiri@resnulli.us>

On 10/12/2017 07:17 PM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> In sch_handle_egress and sch_handle_ingress, don't use tp->q and use
> dev->ingress_queue which stores the same pointer instead.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
>   net/core/dev.c | 21 +++++++++++++++------
>   1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index fcddccb..cb9e5e5 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3273,14 +3273,18 @@ EXPORT_SYMBOL(dev_loopback_xmit);
>   static struct sk_buff *
>   sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
>   {
> +	struct netdev_queue *netdev_queue =
> +				rcu_dereference_bh(dev->ingress_queue);
>   	struct tcf_proto *cl = rcu_dereference_bh(dev->egress_cl_list);
>   	struct tcf_result cl_res;
> +	struct Qdisc *q;
>
> -	if (!cl)
> +	if (!cl || !netdev_queue)
>   		return skb;
> +	q = netdev_queue->qdisc;

NAK, no additional overhead in the software fast-path of
sch_handle_{ingress,egress}() like this. There are users out there
that use tc in software only, so performance is critical here.

>   	/* qdisc_skb_cb(skb)->pkt_len was already set by the caller. */
> -	qdisc_bstats_cpu_update(cl->q, skb);
> +	qdisc_bstats_cpu_update(q, skb);
>
>   	switch (tcf_classify(skb, cl, &cl_res, false)) {
>   	case TC_ACT_OK:
> @@ -3288,7 +3292,7 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
>   		skb->tc_index = TC_H_MIN(cl_res.classid);
>   		break;
>   	case TC_ACT_SHOT:
> -		qdisc_qstats_cpu_drop(cl->q);
> +		qdisc_qstats_cpu_drop(q);
>   		*ret = NET_XMIT_DROP;
>   		kfree_skb(skb);
>   		return NULL;
> @@ -4188,16 +4192,21 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
>   		   struct net_device *orig_dev)
>   {
>   #ifdef CONFIG_NET_CLS_ACT
> +	struct netdev_queue *netdev_queue =
> +				rcu_dereference_bh(skb->dev->ingress_queue);
>   	struct tcf_proto *cl = rcu_dereference_bh(skb->dev->ingress_cl_list);
>   	struct tcf_result cl_res;
> +	struct Qdisc *q;
>
>   	/* If there's at least one ingress present somewhere (so
>   	 * we get here via enabled static key), remaining devices
>   	 * that are not configured with an ingress qdisc will bail
>   	 * out here.
>   	 */
> -	if (!cl)
> +	if (!cl || !netdev_queue)
>   		return skb;
> +	q = netdev_queue->qdisc;
> +
>   	if (*pt_prev) {
>   		*ret = deliver_skb(skb, *pt_prev, orig_dev);
>   		*pt_prev = NULL;
> @@ -4205,7 +4214,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
>
>   	qdisc_skb_cb(skb)->pkt_len = skb->len;
>   	skb->tc_at_ingress = 1;
> -	qdisc_bstats_cpu_update(cl->q, skb);
> +	qdisc_bstats_cpu_update(q, skb);
>
>   	switch (tcf_classify(skb, cl, &cl_res, false)) {
>   	case TC_ACT_OK:
> @@ -4213,7 +4222,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
>   		skb->tc_index = TC_H_MIN(cl_res.classid);
>   		break;
>   	case TC_ACT_SHOT:
> -		qdisc_qstats_cpu_drop(cl->q);
> +		qdisc_qstats_cpu_drop(q);
>   		kfree_skb(skb);
>   		return NULL;
>   	case TC_ACT_STOLEN:
>

  reply	other threads:[~2017-10-12 21:46 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-12 17:17 [patch net-next 00/34] net: sched: allow qdiscs to share filter block instances Jiri Pirko
2017-10-12 17:17 ` [patch net-next 01/34] net: sched: store Qdisc pointer in struct block Jiri Pirko
2017-10-12 17:17 ` [patch net-next 02/34] net: sched: introduce support for multiple filter chain pointers registration Jiri Pirko
2017-10-12 17:17 ` [patch net-next 03/34] net: sched: introduce shared filter blocks infrastructure Jiri Pirko
2017-10-12 17:17 ` [patch net-next 04/34] net: sched: teach tcf_bind/unbind_filter to use block->q Jiri Pirko
2017-10-12 17:17 ` [patch net-next 05/34] net: sched: ematch: obtain net pointer from blocks Jiri Pirko
2017-10-12 17:17 ` [patch net-next 06/34] net: core: use dev->ingress_queue instead of tp->q Jiri Pirko
2017-10-12 21:45   ` Daniel Borkmann [this message]
2017-10-13  6:30     ` Jiri Pirko
2017-10-14 23:18       ` Daniel Borkmann
2017-10-15  6:45         ` Jiri Pirko
2017-10-16 20:20           ` Daniel Borkmann
2017-10-17  7:42             ` Jiri Pirko
2017-10-12 17:17 ` [patch net-next 07/34] net: sched: cls_u32: use block instead of q in tc_u_common Jiri Pirko
2017-10-12 17:17 ` [patch net-next 08/34] net: sched: avoid usage of tp->q in tcf_classify Jiri Pirko
2017-10-12 17:17 ` [patch net-next 09/34] net: sched: tcindex, fw, flow: use tcf_block_q helper to get struct Qdisc Jiri Pirko
2017-10-12 17:17 ` [patch net-next 10/34] net: sched: use tcf_block_q helper to get q pointer for sch_tree_lock Jiri Pirko
2017-10-12 17:18 ` [patch net-next 11/34] net: sched: propagate q and parent from caller down to tcf_fill_node Jiri Pirko
2017-10-12 17:18 ` [patch net-next 12/34] net: sched: add block bind/unbind notification to drivers Jiri Pirko
2017-10-12 17:18 ` [patch net-next 13/34] net: sched: introduce per-block callbacks Jiri Pirko
2017-10-12 17:18 ` [patch net-next 14/34] net: sched: use extended variants of block get and put in ingress and clsact qdiscs Jiri Pirko
2017-10-12 17:18 ` [patch net-next 15/34] net: sched: use tc_setup_cb_call to call per-block callbacks Jiri Pirko
2017-10-12 17:18 ` [patch net-next 16/34] net: sched: cls_matchall: call block callbacks for offload Jiri Pirko
2017-10-12 17:18 ` [patch net-next 17/34] net: sched: cls_u32: swap u32_remove_hw_knode and u32_remove_hw_hnode Jiri Pirko
2017-10-12 17:18 ` [patch net-next 18/34] net: sched: cls_u32: call block callbacks for offload Jiri Pirko
2017-10-12 17:18 ` [patch net-next 19/34] net: sched: cls_bpf: " Jiri Pirko
2017-10-12 17:18 ` [patch net-next 20/34] mlxsw: spectrum: Convert ndo_setup_tc offloads to block callbacks Jiri Pirko
2017-10-12 17:18 ` [patch net-next 21/34] mlx5e: " Jiri Pirko
2017-10-12 17:18 ` [patch net-next 22/34] bnxt: " Jiri Pirko
2017-10-12 17:18 ` [patch net-next 23/34] cxgb4: " Jiri Pirko
2017-10-12 17:18 ` [patch net-next 24/34] ixgbe: " Jiri Pirko
2017-10-12 17:18 ` [patch net-next 25/34] mlx5e_rep: " Jiri Pirko
2017-10-12 17:18 ` [patch net-next 26/34] nfp: flower: " Jiri Pirko
2017-10-12 17:18 ` [patch net-next 27/34] nfp: bpf: " Jiri Pirko
2017-10-13  1:08   ` Jakub Kicinski
2017-10-17 12:48     ` Jiri Pirko
2017-10-17 14:39       ` Jakub Kicinski
2017-10-17 18:16         ` Jiri Pirko
2017-10-12 17:18 ` [patch net-next 28/34] dsa: " Jiri Pirko
2017-10-12 17:18 ` [patch net-next 29/34] net: sched: avoid ndo_setup_tc calls for TC_SETUP_CLS* Jiri Pirko
2017-10-12 17:18 ` [patch net-next 30/34] net: sched: remove unused classid field from tc_cls_common_offload Jiri Pirko
2017-10-12 17:18 ` [patch net-next 31/34] net: sched: remove unused is_classid_clsact_ingress/egress helpers Jiri Pirko
2017-10-12 17:18 ` [patch net-next 32/34] net: sched: introduce block mechanism to handle netif_keep_dst calls Jiri Pirko
2017-10-12 17:18 ` [patch net-next 33/34] net: sched: remove classid and q fields from tcf_proto Jiri Pirko
2017-10-12 17:18 ` [patch net-next 34/34] net: sched: allow ingress and clsact qdiscs to share filter blocks Jiri Pirko
2017-10-12 17:21 ` [patch net-next 00/34] net: sched: allow qdiscs to share filter block instances David Miller
2017-10-13  6:21   ` Jiri Pirko
2017-10-13  6:31     ` David Miller
2017-10-13  7:39       ` Jiri Pirko
2017-10-12 21:37 ` David Ahern
2017-10-13  6:26   ` Jiri Pirko
2017-10-13 14:20     ` David Ahern
2017-10-13 14:24       ` Jiri Pirko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=59DFE287.2040400@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=alexander.h.duyck@intel.com \
    --cc=andrew@lunn.ch \
    --cc=ast@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=ganeshgr@chelsio.com \
    --cc=idosch@mellanox.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=john.fastabend@gmail.com \
    --cc=john.hurley@netronome.com \
    --cc=leonro@mellanox.com \
    --cc=matanb@mellanox.com \
    --cc=michael.chan@broadcom.com \
    --cc=mlxsw@mellanox.com \
    --cc=netdev@vger.kernel.org \
    --cc=pieter.jansenvanvuuren@netronome.com \
    --cc=saeedm@mellanox.com \
    --cc=simon.horman@netronome.com \
    --cc=vivien.didelot@savoirfairelinux.com \
    --cc=willemb@google.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).