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).
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?
