netdev - Re: [PATCH net-next v4 5/8] net: sched: add extack support for offload via tc_cls_common

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 20 Jan 2018 02:28:01 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jiri Pirko <jiri@...nulli.us>
Cc:     David Miller <davem@...emloft.net>,
        David Ahern <dsahern@...il.com>, aring@...atatu.com,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Linux Netdev List <netdev@...r.kernel.org>,
        oss-drivers@...ronome.com,
        Quentin Monnet <quentin.monnet@...ronome.com>
Subject: Re: [PATCH net-next v4 5/8] net: sched: add extack support for
 offload via tc_cls_common_offload

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