Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 29 Mar 2021 07:15:25 +0530
From:   Kumar Kartikeya Dwivedi <memxor@gmail.com>
To:     Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc:     Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>,
        bpf@vger.kernel.org, brouer@redhat.com,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>, Shuah Khan <shuah@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        linux-kselftest@vger.kernel.org
Subject: Re: [PATCH bpf-next 5/5] libbpf: add selftests for TC-BPF API
Message-ID: <20210329014328.srm6y7d6odmgzhri@apollo>
References: <20210325120020.236504-1-memxor@gmail.com>
 <20210325120020.236504-6-memxor@gmail.com>
 <20210327021534.pjfjctcdczj7facs@ast-mbp>
 <87h7kwaao3.fsf@toke.dk>
 <20210329012602.4zzysn2ewbarbn3d@ast-mbp>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210329012602.4zzysn2ewbarbn3d@ast-mbp>
Precedence: bulk

On Mon, Mar 29, 2021 at 06:56:02AM IST, Alexei Starovoitov wrote:
> This is up to you. I'm trying to understand the motivation for *_block() apis.
> I'm not taking a stance for/against them.

The block APIs simply attach to a different shared filter block, so in that
sense they just forward to the bpf_tc_cls_*_dev API internally, where parent_id
is substituted as block_index, and ifindex is set to a special value (to
indicate operation on a block), but is still a distinct attach point, and both
APIs cannot be mixed (i.e. manipulation of filter attached using block API is
not possible using dev API).

e.g.

# tc qdisc add dev <foo> ingress block 1
# tc qdisc add dev <bar> ingress block 1

Now you can attach a filter to the shared block, e.g.

# tc filter add block 1 bpf /home/kkd/foo.o sec cls direct-action

and it will attach the identical filter with the bpf prog classifier to both
qdiscs in one go, instead of having to duplicate filter creation for each qdisc.
You can add arbitrarily many qdiscs to such a filter block, easing filter
management, and saving on resources.

So for the API, it made sense to separate this into its own function as it is a
different attach point, both for the low level API and their higher level
wrappers. This does increase the symbol count, but maintenance wise it is
zero-cost since it simply forwards to the dev functions.

As for the tests, I'll add them for the block API in v2, when I get around to
sending it (i.e. after the review is over).

> [...]

--
Kartikeya