Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode

[Daniel Borkmann <daniel@iogearbox.net>]

On 09/06/2017 12:45 AM, Daniel Borkmann wrote:
> On 09/06/2017 12:01 AM, Roopa Prabhu wrote:
>> On Tue, Sep 5, 2017 at 11:18 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> On Tue, Sep 5, 2017 at 5:48 AM, Nikolay Aleksandrov
>>> <nikolay@cumulusnetworks.com> wrote:
>>>> Hi all,
>>>> This RFC adds a new mode for clsact which designates a device's egress
>>>> classifier as global per netns. The packets that are not classified for
>>>> a particular device will be classified using the global classifier.
>>>> We have needed a global classifier for some time now for various
>>>> purposes and setting the single bridge or loopback/vrf device as the
>
> Can you elaborate a bit more on the ... "we have needed a global
> classifier for some time now for various purposes".
>
>>>> global classifier device is acceptable for us. Doing it this way avoids
>>>> the act/cls device and queue dependencies.
>>>>
>>>> This is strictly an RFC patch just to show the intent, if we agree on
>>>> the details the proposed patch will have support for both ingress and
>>>> egress, and will be using a static key to avoid the fast path test when no
>>>> global classifier has been configured.
>>>>
>>>> Example (need a modified tc that adds TCA_OPTIONS when using q_clsact):
>>>> $ tc qdisc add dev lo clsact global
>>>> $ tc filter add dev lo egress protocol ip u32 match ip dst 4.3.2.1/32 action drop
>>>>
>>>> the last filter will be global for all devices that don't have a
>>>> specific egress_cl_list (i.e. have clsact configured).
>>>
>>> Sorry this is too ugly.
>
> +1
>
>>> netdevice is still implied in your command line even if you treat it
>>> as global. It is essentially hard to bypass netdevice layer since
>>> netdevice is the core of L2 and also where everything begins.

One could probably use special wildcard device name, e.g. tcpdump
allows for 'tcpdump -i any' to capture on all devices, so you could
indicate something like 'tc filter add dev any blah' to have similar
semantics in the realm of the netns w/o making it look too hacky
for tc users perhaps.

>>> Maybe the best we can do here is make tc filters standalone
>>> as tc actions so that filters can exist before qdisc's and netdevices.
>>> But this probably requires significant works to make it working
>>> with both existing non-standalone and bindings standalones
>>> with qdisc's.
>>
>> yes, like Nikolay says we have been discussing this as well. Nikolay's
>> patch is a cleaver and most importantly non-invasive
>> way today given the anchor point for tc rules is a netdev. we have
>> also considered a separate implicit tc anchor device.
>
> Seems ugly just as well. :( Hmm, why not just having the two list
> pointers (ingress, egress list) in the netns struct and when
> something configures them to be effectively non-zero, then devices
> in that netns could automatically get a clsact and inherit the
> lists from there such that sch_handle_ingress() and sch_handle_egress()
> require exactly zero changes in fast-path. You could then go and
> say that either you would make changes to clsact for individual
> devices immutable when they use the 'shared' list pointers, or then
> duplicate the configs when being altered from the global one. Would
> push the complexity to control path only at least. Just a brief
> thought.