On Sat, Jan 20, 2018 at 12:54 AM, Jiri Pirko <j...@resnulli.us> wrote:
> Sat, Jan 20, 2018 at 02:44:47AM CET, jakub.kicin...@netronome.com wrote:
>>From: Quentin Monnet <quentin.mon...@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.mon...@netronome.com>
>>Reviewed-by: Jakub Kicinski <jakub.kicin...@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.)
