netdev - Re: [PATCH iproute2 net-next] tc: flower: support matching flags

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]

Message-ID: <20170103130513.2d7de31b@griffin>
Date:   Tue, 3 Jan 2017 13:05:13 +0100
From:   Jiri Benc <jbenc@...hat.com>
To:     Paul Blakey <paulb@...lanox.com>
Cc:     <netdev@...r.kernel.org>,
        Stephen Hemminger <stephen@...workplumber.org>,
        "David S. Miller" <davem@...emloft.net>,
        Hadar Hen Zion <hadarh@...lanox.com>,
        Or Gerlitz <ogerlitz@...lanox.com>,
        Roi Dayan <roid@...lanox.com>
Subject: Re: [PATCH iproute2 net-next] tc: flower: support matching flags

On Tue, 3 Jan 2017 13:54:34 +0200, Paul Blakey wrote:
> Matching name was from the idea that we are doing is matching.

But we don't have matching_src_mac etc., either, although we're
matching on those fields.

> And regarding documentation/flag names I didn't want tc tool to be need 
> of a update each time a new flag is introduced,

It will be needed anyway because the whole thing would be useless
without proper documentation. So each time a new flag is added, a new
patch to the tc tool will be needed, at least with an addition to its
man page.

Please, let's focus on the *user*. The tc tool is hard to grasp for
users as it is. It's crystal clear to you but you know the kernel
internals. I'm very sure that except for the few kernel developers, no
one would understand what the "flags" field does. And even among the
kernel developers, very few would remember what the magic numeric
values mean.

If we want wider adoption of flower, we should make it as easy to use
as possible. Even when it means a bit more work for us.

> But I guess I can add two options like with ip_proto where you can 
> specify known flags by name but can also give a value.
> What do you think about that?
> 
> flags <FLAGS> / <HEX'/'HEX>
> FLAGS => frag/no_frag/tcp_syn/no_tcp_syn ['|'<FLAGS>]*
> e.g: flags frag|no_tcp_syn or flags 0x01/0x15
> and the mask will have a on bits corresponds only to those flags specified.

This works for me, too.

Thanks!

 Jiri