* [PATCH net-next 0/2] handle_ing update @ 2015-05-09 20:51 Daniel Borkmann 2015-05-09 20:51 ` [PATCH net-next 1/2] net: sched: consolidate handle_ing and ing_filter Daniel Borkmann ` (3 more replies) 0 siblings, 4 replies; 15+ messages in thread From: Daniel Borkmann @ 2015-05-09 20:51 UTC (permalink / raw) To: davem; +Cc: ast, jhs, netdev, Daniel Borkmann These are a couple of cleanups to make ingress a bit more lightweight. Thanks! Daniel Borkmann (2): net: sched: consolidate handle_ing and ing_filter net: sched: further simplify handle_ing include/linux/netdevice.h | 4 ++++ net/core/dev.c | 48 ++++++++++++++++----------------------- net/sched/sch_ingress.c | 58 ++++++++--------------------------------------- 3 files changed, 33 insertions(+), 77 deletions(-) -- 1.9.3 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next 1/2] net: sched: consolidate handle_ing and ing_filter 2015-05-09 20:51 [PATCH net-next 0/2] handle_ing update Daniel Borkmann @ 2015-05-09 20:51 ` Daniel Borkmann 2015-05-09 21:07 ` Alexei Starovoitov 2015-05-09 20:51 ` [PATCH net-next 2/2] net: sched: further simplify handle_ing Daniel Borkmann ` (2 subsequent siblings) 3 siblings, 1 reply; 15+ messages in thread From: Daniel Borkmann @ 2015-05-09 20:51 UTC (permalink / raw) To: davem; +Cc: ast, jhs, netdev, Daniel Borkmann Given quite some code has been removed from ing_filter(), we can just consolidate that function into handle_ing() and get rid of a few instructions at the same time. Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> --- net/core/dev.c | 46 ++++++++++++++++------------------------------ 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 862875e..8a75746 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3521,37 +3521,19 @@ EXPORT_SYMBOL_GPL(br_fdb_test_addr_hook); #endif #ifdef CONFIG_NET_CLS_ACT -/* TODO: Maybe we should just force sch_ingress to be compiled in - * when CONFIG_NET_CLS_ACT is? otherwise some useless instructions - * a compare and 2 stores extra right now if we dont have it on - * but have CONFIG_NET_CLS_ACT - * NOTE: This doesn't stop any functionality; if you dont have - * the ingress scheduler, you just can't add policies on ingress. - * - */ -static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq) -{ - int result = TC_ACT_OK; - struct Qdisc *q; - - skb->tc_verd = SET_TC_AT(skb->tc_verd, AT_INGRESS); - - q = rcu_dereference(rxq->qdisc); - if (q != &noop_qdisc) { - if (likely(!test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) - result = qdisc_enqueue_root(skb, q); - } - - return result; -} - static inline struct sk_buff *handle_ing(struct sk_buff *skb, struct packet_type **pt_prev, int *ret, struct net_device *orig_dev) { struct netdev_queue *rxq = rcu_dereference(skb->dev->ingress_queue); + struct Qdisc *q; - if (!rxq || rcu_access_pointer(rxq->qdisc) == &noop_qdisc) + /* 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 w/o the rcu_dereference(). + */ + if (!rxq || (q = rcu_dereference(rxq->qdisc)) == &noop_qdisc) return skb; if (*pt_prev) { @@ -3559,11 +3541,15 @@ static inline struct sk_buff *handle_ing(struct sk_buff *skb, *pt_prev = NULL; } - switch (ing_filter(skb, rxq)) { - case TC_ACT_SHOT: - case TC_ACT_STOLEN: - kfree_skb(skb); - return NULL; + skb->tc_verd = SET_TC_AT(skb->tc_verd, AT_INGRESS); + + if (likely(!test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) { + switch (qdisc_enqueue_root(skb, q)) { + case TC_ACT_SHOT: + case TC_ACT_STOLEN: + kfree_skb(skb); + return NULL; + } } return skb; -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 1/2] net: sched: consolidate handle_ing and ing_filter 2015-05-09 20:51 ` [PATCH net-next 1/2] net: sched: consolidate handle_ing and ing_filter Daniel Borkmann @ 2015-05-09 21:07 ` Alexei Starovoitov 0 siblings, 0 replies; 15+ messages in thread From: Alexei Starovoitov @ 2015-05-09 21:07 UTC (permalink / raw) To: Daniel Borkmann, davem; +Cc: jhs, netdev On 5/9/15 1:51 PM, Daniel Borkmann wrote: > Given quite some code has been removed from ing_filter(), we can just > consolidate that function into handle_ing() and get rid of a few > instructions at the same time. > > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > --- > net/core/dev.c | 46 ++++++++++++++++------------------------------ > 1 file changed, 16 insertions(+), 30 deletions(-) lovely diffstat :) 2nd patch is even better: 3 files changed, 31 insertions(+), 61 deletions(-) Acked-by: Alexei Starovoitov <ast@plumgrid.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net-next 2/2] net: sched: further simplify handle_ing 2015-05-09 20:51 [PATCH net-next 0/2] handle_ing update Daniel Borkmann 2015-05-09 20:51 ` [PATCH net-next 1/2] net: sched: consolidate handle_ing and ing_filter Daniel Borkmann @ 2015-05-09 20:51 ` Daniel Borkmann 2015-05-10 17:05 ` [PATCH net-next 0/2] handle_ing update Pablo Neira Ayuso 2015-05-11 15:11 ` David Miller 3 siblings, 0 replies; 15+ messages in thread From: Daniel Borkmann @ 2015-05-09 20:51 UTC (permalink / raw) To: davem; +Cc: ast, jhs, netdev, Daniel Borkmann Ingress qdisc has no other purpose than calling into tc_classify() that executes attached classifier(s) and action(s). It has a 1:1 relationship to dev->ingress_queue. After having commit 087c1a601ad7 ("net: sched: run ingress qdisc without locks") removed the central ingress lock, one major contention point is gone. The extra indirection layers however, are not necessary for calling into ingress qdisc. pktgen calling locally into netif_receive_skb() with a dummy u32, single CPU result on a Supermicro X10SLM-F, Xeon E3-1240: before ~21,1 Mpps, after patch ~22,9 Mpps. We can redirect the private classifier list to the netdev directly, without changing any classifier API bits (!) and execute on that from handle_ing() side. The __QDISC_STATE_DEACTIVATE test can be removed, ingress qdisc doesn't have a queue and thus dev_deactivate_queue() is also not applicable, ingress_cl_list provides similar behaviour. In other words, ingress qdisc acts like TCQ_F_BUILTIN qdisc. One next possible step is the removal of the dev's ingress (dummy) netdev_queue, and to only have the list member in the netdevice itself. Note, the filter chain is RCU protected and individual filter elements are being kfree'd by sched subsystem after RCU grace period. RCU read lock is being held by __netif_receive_skb_core(). Joint work with Alexei Starovoitov. Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com> --- include/linux/netdevice.h | 4 ++++ net/core/dev.c | 30 ++++++++++++++---------- net/sched/sch_ingress.c | 58 ++++++++--------------------------------------- 3 files changed, 31 insertions(+), 61 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 1899c74..c4e1caf 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1655,7 +1655,11 @@ struct net_device { rx_handler_func_t __rcu *rx_handler; void __rcu *rx_handler_data; +#if CONFIG_NET_CLS_ACT + struct tcf_proto __rcu *ingress_cl_list; +#endif struct netdev_queue __rcu *ingress_queue; + unsigned char broadcast[MAX_ADDR_LEN]; #ifdef CONFIG_RFS_ACCEL struct cpu_rmap *rx_cpu_rmap; diff --git a/net/core/dev.c b/net/core/dev.c index 8a75746..e5f77c4 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3525,31 +3525,37 @@ static inline struct sk_buff *handle_ing(struct sk_buff *skb, struct packet_type **pt_prev, int *ret, struct net_device *orig_dev) { - struct netdev_queue *rxq = rcu_dereference(skb->dev->ingress_queue); - struct Qdisc *q; + struct tcf_proto *cl = rcu_dereference_bh(skb->dev->ingress_cl_list); + struct tcf_result cl_res; /* 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 w/o the rcu_dereference(). + * out here. */ - if (!rxq || (q = rcu_dereference(rxq->qdisc)) == &noop_qdisc) + if (!cl) return skb; - if (*pt_prev) { *ret = deliver_skb(skb, *pt_prev, orig_dev); *pt_prev = NULL; } + qdisc_bstats_update_cpu(cl->q, skb); skb->tc_verd = SET_TC_AT(skb->tc_verd, AT_INGRESS); - if (likely(!test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) { - switch (qdisc_enqueue_root(skb, q)) { - case TC_ACT_SHOT: - case TC_ACT_STOLEN: - kfree_skb(skb); - return NULL; - } + switch (tc_classify(skb, cl, &cl_res)) { + case TC_ACT_OK: + case TC_ACT_RECLASSIFY: + skb->tc_index = TC_H_MIN(cl_res.classid); + break; + case TC_ACT_SHOT: + qdisc_qstats_drop_cpu(cl->q); + case TC_ACT_STOLEN: + case TC_ACT_QUEUED: + kfree_skb(skb); + return NULL; + default: + break; } return skb; diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c index a89cc32..e7c648f 100644 --- a/net/sched/sch_ingress.c +++ b/net/sched/sch_ingress.c @@ -12,16 +12,10 @@ #include <linux/list.h> #include <linux/skbuff.h> #include <linux/rtnetlink.h> + #include <net/netlink.h> #include <net/pkt_sched.h> - -struct ingress_qdisc_data { - struct tcf_proto __rcu *filter_list; -}; - -/* ------------------------- Class/flow operations ------------------------- */ - static struct Qdisc *ingress_leaf(struct Qdisc *sch, unsigned long arg) { return NULL; @@ -49,45 +43,11 @@ static void ingress_walk(struct Qdisc *sch, struct qdisc_walker *walker) static struct tcf_proto __rcu **ingress_find_tcf(struct Qdisc *sch, unsigned long cl) { - struct ingress_qdisc_data *p = qdisc_priv(sch); - - return &p->filter_list; -} - -/* --------------------------- Qdisc operations ---------------------------- */ + struct net_device *dev = qdisc_dev(sch); -static int ingress_enqueue(struct sk_buff *skb, struct Qdisc *sch) -{ - struct ingress_qdisc_data *p = qdisc_priv(sch); - struct tcf_result res; - struct tcf_proto *fl = rcu_dereference_bh(p->filter_list); - int result; - - result = tc_classify(skb, fl, &res); - - qdisc_bstats_update_cpu(sch, skb); - switch (result) { - case TC_ACT_SHOT: - result = TC_ACT_SHOT; - qdisc_qstats_drop_cpu(sch); - break; - case TC_ACT_STOLEN: - case TC_ACT_QUEUED: - result = TC_ACT_STOLEN; - break; - case TC_ACT_RECLASSIFY: - case TC_ACT_OK: - skb->tc_index = TC_H_MIN(res.classid); - default: - result = TC_ACT_OK; - break; - } - - return result; + return &dev->ingress_cl_list; } -/* ------------------------------------------------------------- */ - static int ingress_init(struct Qdisc *sch, struct nlattr *opt) { net_inc_ingress_queue(); @@ -98,9 +58,9 @@ static int ingress_init(struct Qdisc *sch, struct nlattr *opt) static void ingress_destroy(struct Qdisc *sch) { - struct ingress_qdisc_data *p = qdisc_priv(sch); + struct net_device *dev = qdisc_dev(sch); - tcf_destroy_chain(&p->filter_list); + tcf_destroy_chain(&dev->ingress_cl_list); net_dec_ingress_queue(); } @@ -111,6 +71,7 @@ static int ingress_dump(struct Qdisc *sch, struct sk_buff *skb) nest = nla_nest_start(skb, TCA_OPTIONS); if (nest == NULL) goto nla_put_failure; + return nla_nest_end(skb, nest); nla_put_failure: @@ -131,8 +92,6 @@ static const struct Qdisc_class_ops ingress_class_ops = { static struct Qdisc_ops ingress_qdisc_ops __read_mostly = { .cl_ops = &ingress_class_ops, .id = "ingress", - .priv_size = sizeof(struct ingress_qdisc_data), - .enqueue = ingress_enqueue, .init = ingress_init, .destroy = ingress_destroy, .dump = ingress_dump, @@ -149,6 +108,7 @@ static void __exit ingress_module_exit(void) unregister_qdisc(&ingress_qdisc_ops); } -module_init(ingress_module_init) -module_exit(ingress_module_exit) +module_init(ingress_module_init); +module_exit(ingress_module_exit); + MODULE_LICENSE("GPL"); -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-09 20:51 [PATCH net-next 0/2] handle_ing update Daniel Borkmann 2015-05-09 20:51 ` [PATCH net-next 1/2] net: sched: consolidate handle_ing and ing_filter Daniel Borkmann 2015-05-09 20:51 ` [PATCH net-next 2/2] net: sched: further simplify handle_ing Daniel Borkmann @ 2015-05-10 17:05 ` Pablo Neira Ayuso 2015-05-10 17:55 ` Alexei Starovoitov 2015-05-11 15:09 ` David Miller 2015-05-11 15:11 ` David Miller 3 siblings, 2 replies; 15+ messages in thread From: Pablo Neira Ayuso @ 2015-05-10 17:05 UTC (permalink / raw) To: Daniel Borkmann; +Cc: davem, ast, jhs, netdev On Sat, May 09, 2015 at 10:51:30PM +0200, Daniel Borkmann wrote: > These are a couple of cleanups to make ingress a bit more lightweight. This is plain wrong at many levels. You're persisting on embedding the ingress code into the core, and you have to remember that most users don't need this. Modules allows people to get the code that they need into the core, with this approach, they have no other choice other than disable from .config this if they don't need it. This has to be done the other way around. I just sent a patchset to clean up this that in exactly the other direction, as a result, performance is improved for users that don't need this. We should do things to make users aware that when they request features, they have to pay a performance cost, and that happens by when you invoke: tc qdisc add dev eth0 handle ffff: ingress David already stated before that ingress path is performance critical, but you insist on trying to get qdisc ingress faster *at any cost*. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-10 17:05 ` [PATCH net-next 0/2] handle_ing update Pablo Neira Ayuso @ 2015-05-10 17:55 ` Alexei Starovoitov 2015-05-10 18:06 ` Pablo Neira Ayuso 2015-05-11 15:09 ` David Miller 1 sibling, 1 reply; 15+ messages in thread From: Alexei Starovoitov @ 2015-05-10 17:55 UTC (permalink / raw) To: Pablo Neira Ayuso, Daniel Borkmann; +Cc: davem, jhs, netdev On 5/10/15 10:05 AM, Pablo Neira Ayuso wrote: > On Sat, May 09, 2015 at 10:51:30PM +0200, Daniel Borkmann wrote: >> These are a couple of cleanups to make ingress a bit more lightweight. > > This is plain wrong at many levels. > > You're persisting on embedding the ingress code into the core, and you > have to remember that most users don't need this. Modules allows > people to get the code that they need into the core, with this > approach, they have no other choice other than disable from .config > this if they don't need it. I think you're misreading the patch set. Where do you see that this is pushed on all users? if (static_key_false(&ingress_needed)) still protects all of these bits. When ingress qdisc is added, the key gets enabled and it only needs one deref to not go any further. Much faster than it is today. > This has to be done the other way around. I just sent a patchset to > clean up this that in exactly the other direction, as a result, > performance is improved for users that don't need this. you're doing exactly the same in your patch set, but with added extra overhead for netfilter hook. > We should do things to make users aware that when they request > features, they have to pay a performance cost, and that happens by > when you invoke: > > tc qdisc add dev eth0 handle ffff: ingress > > David already stated before that ingress path is performance critical, > but you insist on trying to get qdisc ingress faster *at any cost*. Nope. We're cleaning up ingress qdisc path _without_ affecting anything else, whereas your netfilter hook creates binary choice for users whether they want nft or tc. Please just add your own netfilter hook to netif_receive_skb and let's be done with this back and forth arguments. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-10 17:55 ` Alexei Starovoitov @ 2015-05-10 18:06 ` Pablo Neira Ayuso 0 siblings, 0 replies; 15+ messages in thread From: Pablo Neira Ayuso @ 2015-05-10 18:06 UTC (permalink / raw) To: Alexei Starovoitov; +Cc: Daniel Borkmann, davem, jhs, netdev On Sun, May 10, 2015 at 10:55:23AM -0700, Alexei Starovoitov wrote: > you're doing exactly the same in your patch set, but with added extra > overhead for netfilter hook. I will *not* use that hook for Netfilter. DOT. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-10 17:05 ` [PATCH net-next 0/2] handle_ing update Pablo Neira Ayuso 2015-05-10 17:55 ` Alexei Starovoitov @ 2015-05-11 15:09 ` David Miller 2015-05-11 22:03 ` Cong Wang 2015-05-12 12:54 ` Pablo Neira Ayuso 1 sibling, 2 replies; 15+ messages in thread From: David Miller @ 2015-05-11 15:09 UTC (permalink / raw) To: pablo; +Cc: daniel, ast, jhs, netdev From: Pablo Neira Ayuso <pablo@netfilter.org> Date: Sun, 10 May 2015 19:05:50 +0200 > On Sat, May 09, 2015 at 10:51:30PM +0200, Daniel Borkmann wrote: >> These are a couple of cleanups to make ingress a bit more lightweight. > > This is plain wrong at many levels. > > You're persisting on embedding the ingress code into the core, and you > have to remember that most users don't need this. Modules allows > people to get the code that they need into the core, with this > approach, they have no other choice other than disable from .config > this if they don't need it. > > This has to be done the other way around. I just sent a patchset to > clean up this that in exactly the other direction, as a result, > performance is improved for users that don't need this. > > We should do things to make users aware that when they request > features, they have to pay a performance cost, and that happens by > when you invoke: > > tc qdisc add dev eth0 handle ffff: ingress > > David already stated before that ingress path is performance critical, > but you insist on trying to get qdisc ingress faster *at any cost*. Pablo I think you are overreacting here. What Daniel and Alexei are doing here is quite reasonable. There is no difference between having ingress qdisc hanging off of netdevice vs. the classifier list. They both serve the same purpose and the change to use the classifier list merely removes one level of indirection. I also am confident that the cost for non-users is equivalent before and after this patch series. Or at least, it very much should be. Therefore I am going to apply these patches, and I wish you would work more closely with Daniel and Alexei because I am more than convinced that you share the same exact goals. Thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-11 15:09 ` David Miller @ 2015-05-11 22:03 ` Cong Wang 2015-05-11 22:23 ` Alexei Starovoitov 2015-05-11 22:33 ` Daniel Borkmann 2015-05-12 12:54 ` Pablo Neira Ayuso 1 sibling, 2 replies; 15+ messages in thread From: Cong Wang @ 2015-05-11 22:03 UTC (permalink / raw) To: David Miller Cc: Pablo Neira Ayuso, Daniel Borkmann, Alexei Starovoitov, Jamal Hadi Salim, netdev On Mon, May 11, 2015 at 8:09 AM, David Miller <davem@davemloft.net> wrote: > There is no difference between having ingress qdisc hanging off of > netdevice vs. the classifier list. They both serve the same purpose > and the change to use the classifier list merely removes one level > of indirection. With adding 8 bytes to each of the netdevice even when it doesn't have ingress qdisc attached at all... Also breaks the abstraction of Qdisc's which potentially breaks existing or future code, since now we have two different *layers* to hold tp_proto. BTW, it should be protected by CONFIG_NET_SCH_INGRESS rather than CONFIG_NET_CLS_ACT. > > I also am confident that the cost for non-users is equivalent before > and after this patch series. Or at least, it very much should be. If people really cared about ingress qdisc, that spinlock should have gone much earlier than in 4.x release, rather than waiting for me to remind. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-11 22:03 ` Cong Wang @ 2015-05-11 22:23 ` Alexei Starovoitov 2015-05-11 22:42 ` Cong Wang 2015-05-11 22:33 ` Daniel Borkmann 1 sibling, 1 reply; 15+ messages in thread From: Alexei Starovoitov @ 2015-05-11 22:23 UTC (permalink / raw) To: Cong Wang, David Miller Cc: Pablo Neira Ayuso, Daniel Borkmann, Jamal Hadi Salim, netdev On 5/11/15 3:03 PM, Cong Wang wrote: > On Mon, May 11, 2015 at 8:09 AM, David Miller <davem@davemloft.net> wrote: >> There is no difference between having ingress qdisc hanging off of >> netdevice vs. the classifier list. They both serve the same purpose >> and the change to use the classifier list merely removes one level >> of indirection. > > With adding 8 bytes to each of the netdevice even when it doesn't > have ingress qdisc attached at all... we already said several times that the work is already on the way to remove 'ingress_queue' from net_device. It will save much more memory than addition of 8 bytes. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-11 22:23 ` Alexei Starovoitov @ 2015-05-11 22:42 ` Cong Wang 0 siblings, 0 replies; 15+ messages in thread From: Cong Wang @ 2015-05-11 22:42 UTC (permalink / raw) To: Alexei Starovoitov Cc: David Miller, Pablo Neira Ayuso, Daniel Borkmann, Jamal Hadi Salim, netdev On Mon, May 11, 2015 at 3:23 PM, Alexei Starovoitov <ast@plumgrid.com> wrote: > On 5/11/15 3:03 PM, Cong Wang wrote: >> >> On Mon, May 11, 2015 at 8:09 AM, David Miller <davem@davemloft.net> wrote: >>> >>> There is no difference between having ingress qdisc hanging off of >>> netdevice vs. the classifier list. They both serve the same purpose >>> and the change to use the classifier list merely removes one level >>> of indirection. >> >> >> With adding 8 bytes to each of the netdevice even when it doesn't >> have ingress qdisc attached at all... > > > we already said several times that the work is already on the way > to remove 'ingress_queue' from net_device. It will save much more > memory than addition of 8 bytes. This sounds like adding 8 bytes is a must step for removing more, I seriously doubt. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-11 22:03 ` Cong Wang 2015-05-11 22:23 ` Alexei Starovoitov @ 2015-05-11 22:33 ` Daniel Borkmann 2015-05-11 22:48 ` Cong Wang 1 sibling, 1 reply; 15+ messages in thread From: Daniel Borkmann @ 2015-05-11 22:33 UTC (permalink / raw) To: Cong Wang, David Miller Cc: Pablo Neira Ayuso, Alexei Starovoitov, Jamal Hadi Salim, netdev On 05/12/2015 12:03 AM, Cong Wang wrote: ... > BTW, it should be protected by CONFIG_NET_SCH_INGRESS > rather than CONFIG_NET_CLS_ACT. Sure, I agree, this can further be improved. Feel free to send a patch. >> I also am confident that the cost for non-users is equivalent before >> and after this patch series. Or at least, it very much should be. > > If people really cared about ingress qdisc, that spinlock should have > gone much earlier than in 4.x release, rather than waiting for me to > remind. Thanks for your reminder then. ;) Cheers, Daniel ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-11 22:33 ` Daniel Borkmann @ 2015-05-11 22:48 ` Cong Wang 0 siblings, 0 replies; 15+ messages in thread From: Cong Wang @ 2015-05-11 22:48 UTC (permalink / raw) To: Daniel Borkmann Cc: David Miller, Pablo Neira Ayuso, Alexei Starovoitov, Jamal Hadi Salim, netdev On Mon, May 11, 2015 at 3:33 PM, Daniel Borkmann <daniel@iogearbox.net> wrote: > On 05/12/2015 12:03 AM, Cong Wang wrote: > ... >> >> BTW, it should be protected by CONFIG_NET_SCH_INGRESS >> rather than CONFIG_NET_CLS_ACT. > > > Sure, I agree, this can further be improved. Feel free to send a patch. I never agree on that approach, it makes the code even messy, less readable. Correctness and readability are much more important than a little performance gain, for me, not for you of course. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-11 15:09 ` David Miller 2015-05-11 22:03 ` Cong Wang @ 2015-05-12 12:54 ` Pablo Neira Ayuso 1 sibling, 0 replies; 15+ messages in thread From: Pablo Neira Ayuso @ 2015-05-12 12:54 UTC (permalink / raw) To: David Miller; +Cc: daniel, ast, jhs, netdev On Mon, May 11, 2015 at 11:09:49AM -0400, David Miller wrote: [...] > Pablo I think you are overreacting here. Sorry, I'll do my best to calm things down here. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net-next 0/2] handle_ing update 2015-05-09 20:51 [PATCH net-next 0/2] handle_ing update Daniel Borkmann ` (2 preceding siblings ...) 2015-05-10 17:05 ` [PATCH net-next 0/2] handle_ing update Pablo Neira Ayuso @ 2015-05-11 15:11 ` David Miller 3 siblings, 0 replies; 15+ messages in thread From: David Miller @ 2015-05-11 15:11 UTC (permalink / raw) To: daniel; +Cc: ast, jhs, netdev From: Daniel Borkmann <daniel@iogearbox.net> Date: Sat, 9 May 2015 22:51:30 +0200 > These are a couple of cleanups to make ingress a bit more lightweight. Series applied, thanks Daniel. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-05-12 12:49 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-09 20:51 [PATCH net-next 0/2] handle_ing update Daniel Borkmann 2015-05-09 20:51 ` [PATCH net-next 1/2] net: sched: consolidate handle_ing and ing_filter Daniel Borkmann 2015-05-09 21:07 ` Alexei Starovoitov 2015-05-09 20:51 ` [PATCH net-next 2/2] net: sched: further simplify handle_ing Daniel Borkmann 2015-05-10 17:05 ` [PATCH net-next 0/2] handle_ing update Pablo Neira Ayuso 2015-05-10 17:55 ` Alexei Starovoitov 2015-05-10 18:06 ` Pablo Neira Ayuso 2015-05-11 15:09 ` David Miller 2015-05-11 22:03 ` Cong Wang 2015-05-11 22:23 ` Alexei Starovoitov 2015-05-11 22:42 ` Cong Wang 2015-05-11 22:33 ` Daniel Borkmann 2015-05-11 22:48 ` Cong Wang 2015-05-12 12:54 ` Pablo Neira Ayuso 2015-05-11 15:11 ` David Miller
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).