netdev - Re: [patch net-next v2 08/20] net: sched: cls_bpf: call block callbacks for offload

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] [day] [month] [year] [list]

Message-ID: <20171101083359.GD1977@nanopsycho.orion>
Date:   Wed, 1 Nov 2017 09:33:59 +0100
From:   Jiri Pirko <jiri@...nulli.us>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, jhs@...atatu.com,
        xiyou.wangcong@...il.com, mlxsw@...lanox.com, andrew@...n.ch,
        vivien.didelot@...oirfairelinux.com, f.fainelli@...il.com,
        michael.chan@...adcom.com, ganeshgr@...lsio.com,
        jeffrey.t.kirsher@...el.com, saeedm@...lanox.com,
        matanb@...lanox.com, leonro@...lanox.com, idosch@...lanox.com,
        ast@...nel.org, daniel@...earbox.net, simon.horman@...ronome.com,
        pieter.jansenvanvuuren@...ronome.com, john.hurley@...ronome.com,
        alexander.h.duyck@...el.com
Subject: Re: [patch net-next v2 08/20] net: sched: cls_bpf: call block
 callbacks for offload

Wed, Nov 01, 2017 at 01:44:09AM CET, jakub.kicinski@...ronome.com wrote:
>On Thu, 19 Oct 2017 15:50:36 +0200, Jiri Pirko wrote:
>> @@ -159,17 +162,38 @@ static int cls_bpf_offload_cmd(struct tcf_proto *tp, struct cls_bpf_prog *prog,
>>  	cls_bpf.exts_integrated = prog->exts_integrated;
>>  	cls_bpf.gen_flags = prog->gen_flags;
>>  
>> -	err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSBPF, &cls_bpf);
>> -	if (!err && (cmd == TC_CLSBPF_ADD || cmd == TC_CLSBPF_REPLACE))
>> -		prog->gen_flags |= TCA_CLS_FLAGS_IN_HW;
>> +	if (tc_can_offload(dev)) {
>> +		err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_CLSBPF,
>> +						    &cls_bpf);
>> +		if (addorrep) {
>> +			if (err) {
>> +				if (skip_sw)
>> +					return err;
>> +			} else {
>> +				prog->gen_flags |= TCA_CLS_FLAGS_IN_HW;
>> +			}
>> +		}
>> +	}
>> +
>> +	err = tc_setup_cb_call(block, NULL, TC_SETUP_CLSBPF, &cls_bpf, skip_sw);
>> +	if (addorrep) {
>> +		if (err < 0) {
>> +			cls_bpf_offload_cmd(tp, prog, TC_CLSBPF_DESTROY);
>
>It seems counter intuitive that the appropriate action for a failed
>REPLACE is DESTROY.  One would expect a bad REPLACE X -> Y to be 
>followed by a REPLACE Y -> X (i.e. go back to X).

That makes sense.

>
>At least my reading of cls_bpf is that if replace fails software path
>will keep using the old prog.  Is this maybe something that's different
>in flower?  Or am I reading the code wrong?

In flower, there is not possible to do replace. First, the new one is
added and then the old one is removed.