| Message ID | 20180120014450.29666-6-jakub.kicinski@netronome.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: sched: add extack support for cls offloads | expand |
Sat, Jan 20, 2018 at 02:44:47AM CET, jakub.kicinski@netronome.com wrote: >From: Quentin Monnet <quentin.monnet@netronome.com> > >Add extack support for hardware offload of classifiers. In order >to achieve this, a pointer to a struct netlink_ext_ack is added to the >struct tc_cls_common_offload that is passed to the callback for setting >up the classifier. Function tc_cls_common_offload_init() is updated to >support initialization of this new attribute. > >Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com> >Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> >--- > include/net/pkt_cls.h | 5 ++++- > net/sched/cls_bpf.c | 4 ++-- > net/sched/cls_flower.c | 6 +++--- > net/sched/cls_matchall.c | 4 ++-- > net/sched/cls_u32.c | 8 ++++---- > 5 files changed, 15 insertions(+), 12 deletions(-) > >diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h >index 2e4b8e436d25..f497f622580b 100644 >--- a/include/net/pkt_cls.h >+++ b/include/net/pkt_cls.h >@@ -602,15 +602,18 @@ struct tc_cls_common_offload { > u32 chain_index; > __be16 protocol; > u32 prio; >+ struct netlink_ext_ack *extack; > }; > > static inline void > tc_cls_common_offload_init(struct tc_cls_common_offload *cls_common, >- const struct tcf_proto *tp) >+ const struct tcf_proto *tp, >+ struct netlink_ext_ack *extack) > { > cls_common->chain_index = tp->chain->index; > cls_common->protocol = tp->protocol; > cls_common->prio = tp->prio; >+ cls_common->extack = extack; > } [...] >diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c >index f675a92e1b66..727c10378f37 100644 >--- a/net/sched/cls_flower.c >+++ b/net/sched/cls_flower.c >@@ -223,7 +223,7 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f) > struct tc_cls_flower_offload cls_flower = {}; > struct tcf_block *block = tp->chain->block; > >- tc_cls_common_offload_init(&cls_flower.common, tp); >+ tc_cls_common_offload_init(&cls_flower.common, tp, NULL); While you are at it, you should propagate extack whenever possible. For this, you can easily pass extack to fl_hw_destroy_filter. Same for other classifiers. > cls_flower.command = TC_CLSFLOWER_DESTROY; > cls_flower.cookie = (unsigned long) f; > >@@ -243,7 +243,7 @@ static int fl_hw_replace_filter(struct tcf_proto *tp, > bool skip_sw = tc_skip_sw(f->flags); > int err; > >- tc_cls_common_offload_init(&cls_flower.common, tp); >+ tc_cls_common_offload_init(&cls_flower.common, tp, extack); > cls_flower.command = TC_CLSFLOWER_REPLACE; > cls_flower.cookie = (unsigned long) f; > cls_flower.dissector = dissector; >@@ -272,7 +272,7 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f) > struct tc_cls_flower_offload cls_flower = {}; > struct tcf_block *block = tp->chain->block; > >- tc_cls_common_offload_init(&cls_flower.common, tp); >+ tc_cls_common_offload_init(&cls_flower.common, tp, NULL); For this, it would be probably a bit trickier to get extack, but also possible. My motivation is to make the extack always available for users of tc_cls_common_offload > cls_flower.command = TC_CLSFLOWER_STATS; > cls_flower.cookie = (unsigned long) f; > cls_flower.exts = &f->exts; [...]
On Sat, Jan 20, 2018 at 12:54 AM, Jiri Pirko <jiri@resnulli.us> wrote: > Sat, Jan 20, 2018 at 02:44:47AM CET, jakub.kicinski@netronome.com wrote: >>From: Quentin Monnet <quentin.monnet@netronome.com> >> >>Add extack support for hardware offload of classifiers. In order >>to achieve this, a pointer to a struct netlink_ext_ack is added to the >>struct tc_cls_common_offload that is passed to the callback for setting >>up the classifier. Function tc_cls_common_offload_init() is updated to >>support initialization of this new attribute. >> >>Signed-off-by: Quentin Monnet <quentin.monnet@netronome.com> >>Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> > > [...] > > >>diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c >>index f675a92e1b66..727c10378f37 100644 >>--- a/net/sched/cls_flower.c >>+++ b/net/sched/cls_flower.c >>@@ -223,7 +223,7 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f) >> struct tc_cls_flower_offload cls_flower = {}; >> struct tcf_block *block = tp->chain->block; >> >>- tc_cls_common_offload_init(&cls_flower.common, tp); >>+ tc_cls_common_offload_init(&cls_flower.common, tp, NULL); > > While you are at it, you should propagate extack whenever possible. For > this, you can easily pass extack to fl_hw_destroy_filter. > Same for other classifiers. > >> cls_flower.command = TC_CLSFLOWER_DESTROY; >> cls_flower.cookie = (unsigned long) f; >> >>@@ -243,7 +243,7 @@ static int fl_hw_replace_filter(struct tcf_proto *tp, >> bool skip_sw = tc_skip_sw(f->flags); >> int err; >> >>- tc_cls_common_offload_init(&cls_flower.common, tp); >>+ tc_cls_common_offload_init(&cls_flower.common, tp, extack); >> cls_flower.command = TC_CLSFLOWER_REPLACE; >> cls_flower.cookie = (unsigned long) f; >> cls_flower.dissector = dissector; >>@@ -272,7 +272,7 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f) >> struct tc_cls_flower_offload cls_flower = {}; >> struct tcf_block *block = tp->chain->block; >> >>- tc_cls_common_offload_init(&cls_flower.common, tp); >>+ tc_cls_common_offload_init(&cls_flower.common, tp, NULL); > > For this, it would be probably a bit trickier to get extack, but also > possible. > My motivation is to make the extack always available for users of > tc_cls_common_offload Not sure I can think of anything other than "IO error" that could stop us from destroy or dumping stats ;) Also I'm not sure extack fits well with dumps of multiple entities, how would user know which entity produced the message? Extack is best for configuration requests IMHO.. (I'm not saying we shouldn't plumb it through to more places, just wondering what you think.)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 2e4b8e436d25..f497f622580b 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -602,15 +602,18 @@ struct tc_cls_common_offload { u32 chain_index; __be16 protocol; u32 prio; + struct netlink_ext_ack *extack; }; static inline void tc_cls_common_offload_init(struct tc_cls_common_offload *cls_common, - const struct tcf_proto *tp) + const struct tcf_proto *tp, + struct netlink_ext_ack *extack) { cls_common->chain_index = tp->chain->index; cls_common->protocol = tp->protocol; cls_common->prio = tp->prio; + cls_common->extack = extack; } struct tc_cls_u32_knode { diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c index 44c2f9034e07..6de0bea983ea 100644 --- a/net/sched/cls_bpf.c +++ b/net/sched/cls_bpf.c @@ -159,7 +159,7 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog, skip_sw = prog && tc_skip_sw(prog->gen_flags); obj = prog ?: oldprog; - tc_cls_common_offload_init(&cls_bpf.common, tp); + tc_cls_common_offload_init(&cls_bpf.common, tp, extack); cls_bpf.command = TC_CLSBPF_OFFLOAD; cls_bpf.exts = &obj->exts; cls_bpf.prog = prog ? prog->filter : NULL; @@ -220,7 +220,7 @@ static void cls_bpf_offload_update_stats(struct tcf_proto *tp, struct tcf_block *block = tp->chain->block; struct tc_cls_bpf_offload cls_bpf = {}; - tc_cls_common_offload_init(&cls_bpf.common, tp); + tc_cls_common_offload_init(&cls_bpf.common, tp, NULL); cls_bpf.command = TC_CLSBPF_STATS; cls_bpf.exts = &prog->exts; cls_bpf.prog = prog->filter; diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index f675a92e1b66..727c10378f37 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -223,7 +223,7 @@ static void fl_hw_destroy_filter(struct tcf_proto *tp, struct cls_fl_filter *f) struct tc_cls_flower_offload cls_flower = {}; struct tcf_block *block = tp->chain->block; - tc_cls_common_offload_init(&cls_flower.common, tp); + tc_cls_common_offload_init(&cls_flower.common, tp, NULL); cls_flower.command = TC_CLSFLOWER_DESTROY; cls_flower.cookie = (unsigned long) f; @@ -243,7 +243,7 @@ static int fl_hw_replace_filter(struct tcf_proto *tp, bool skip_sw = tc_skip_sw(f->flags); int err; - tc_cls_common_offload_init(&cls_flower.common, tp); + tc_cls_common_offload_init(&cls_flower.common, tp, extack); cls_flower.command = TC_CLSFLOWER_REPLACE; cls_flower.cookie = (unsigned long) f; cls_flower.dissector = dissector; @@ -272,7 +272,7 @@ static void fl_hw_update_stats(struct tcf_proto *tp, struct cls_fl_filter *f) struct tc_cls_flower_offload cls_flower = {}; struct tcf_block *block = tp->chain->block; - tc_cls_common_offload_init(&cls_flower.common, tp); + tc_cls_common_offload_init(&cls_flower.common, tp, NULL); cls_flower.command = TC_CLSFLOWER_STATS; cls_flower.cookie = (unsigned long) f; cls_flower.exts = &f->exts; diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c index b47929c15792..d990d2a52c6d 100644 --- a/net/sched/cls_matchall.c +++ b/net/sched/cls_matchall.c @@ -76,7 +76,7 @@ static void mall_destroy_hw_filter(struct tcf_proto *tp, struct tc_cls_matchall_offload cls_mall = {}; struct tcf_block *block = tp->chain->block; - tc_cls_common_offload_init(&cls_mall.common, tp); + tc_cls_common_offload_init(&cls_mall.common, tp, NULL); cls_mall.command = TC_CLSMATCHALL_DESTROY; cls_mall.cookie = cookie; @@ -94,7 +94,7 @@ static int mall_replace_hw_filter(struct tcf_proto *tp, bool skip_sw = tc_skip_sw(head->flags); int err; - tc_cls_common_offload_init(&cls_mall.common, tp); + tc_cls_common_offload_init(&cls_mall.common, tp, extack); cls_mall.command = TC_CLSMATCHALL_REPLACE; cls_mall.exts = &head->exts; cls_mall.cookie = cookie; diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 0206c210e25b..7030240f8826 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -491,7 +491,7 @@ static void u32_clear_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h) struct tcf_block *block = tp->chain->block; struct tc_cls_u32_offload cls_u32 = {}; - tc_cls_common_offload_init(&cls_u32.common, tp); + tc_cls_common_offload_init(&cls_u32.common, tp, NULL); cls_u32.command = TC_CLSU32_DELETE_HNODE; cls_u32.hnode.divisor = h->divisor; cls_u32.hnode.handle = h->handle; @@ -509,7 +509,7 @@ static int u32_replace_hw_hnode(struct tcf_proto *tp, struct tc_u_hnode *h, bool offloaded = false; int err; - tc_cls_common_offload_init(&cls_u32.common, tp); + tc_cls_common_offload_init(&cls_u32.common, tp, extack); cls_u32.command = TC_CLSU32_NEW_HNODE; cls_u32.hnode.divisor = h->divisor; cls_u32.hnode.handle = h->handle; @@ -534,7 +534,7 @@ static void u32_remove_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n) struct tcf_block *block = tp->chain->block; struct tc_cls_u32_offload cls_u32 = {}; - tc_cls_common_offload_init(&cls_u32.common, tp); + tc_cls_common_offload_init(&cls_u32.common, tp, NULL); cls_u32.command = TC_CLSU32_DELETE_KNODE; cls_u32.knode.handle = n->handle; @@ -550,7 +550,7 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n, bool skip_sw = tc_skip_sw(flags); int err; - tc_cls_common_offload_init(&cls_u32.common, tp); + tc_cls_common_offload_init(&cls_u32.common, tp, extack); cls_u32.command = TC_CLSU32_REPLACE_KNODE; cls_u32.knode.handle = n->handle; cls_u32.knode.fshift = n->fshift;