On 6/27/2018 12:50 AM, Jiri Pirko wrote:
Tue, Jun 26, 2018 at 11:18:58PM CEST, jakub.kicin...@netronome.com wrote:
On Tue, 26 Jun 2018 09:12:17 +0200, Jiri Pirko wrote:
Tue, Jun 26, 2018 at 09:00:45AM CEST, jakub.kicin...@netronome.com wrote:
On Mon, Jun 25, 2018 at 11:43 PM, Jiri Pirko <j...@resnulli.us> wrote:
Tue, Jun 26, 2018 at 06:58:50AM CEST, jakub.kicin...@netronome.com wrote:
On Mon, 25 Jun 2018 23:01:39 +0200, Jiri Pirko wrote:
From: Jiri Pirko <j...@mellanox.com>

For the TC clsact offload these days, some of HW drivers need
to hold a magic ball. The reason is, with the first inserted rule inside
HW they need to guess what fields will be used for the matching. If
later on this guess proves to be wrong and user adds a filter with a
different field to match, there's a problem. Mlxsw resolves it now with
couple of patterns. Those try to cover as many match fields as possible.
This aproach is far from optimal, both performance-wise and scale-wise.
Also, there is a combination of filters that in certain order won't
succeed.

Most of the time, when user inserts filters in chain, he knows right away
how the filters are going to look like - what type and option will they
have. For example, he knows that he will only insert filters of type
flower matching destination IP address. He can specify a template that
would cover all the filters in the chain.
Perhaps it's lack of sleep, but this paragraph threw me a little off
the track.  IIUC the goal of this set is to provide a way to inform the
HW about expected matches before any rule is programmed into the HW.
Not before any rule is added to a particular chain.  One can just use
the first rule in the chain to make a guess about the chain, but thanks
to this set user can configure *all* chains before any rules are added.
The template is per-chain. User can use template for chain x and
not-use it for chain y. Up to him.
Makes sense.

I can't help but wonder if it'd be better to associate the
constraints/rules with chains instead of creating a new "template"
object.  It seems more natural to create a chain with specific
constraints in place than add and delete template of which there can
be at most one to a chain...  Perhaps that's more about the user space
tc command line.  Anyway, not a strong objection, just a thought.
Hmm. I don't think it is good idea. User should see the template in a
"show" command per chain. We would have to have 2 show commands, one to
list the template objects and one to list templates per chains. It makes
things more complicated for no good reason. I think that this simple
chain-lock is easier and serves the purpose.
Hm, I think the dump is fine, what I was thinking about was:

# tc chain add dev dummy0 ingress chain_index 22 \
     ^^^^^
        template proto ip \
        ^^^^^^^^
        flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
Okay, I got it. I see 2 issues.
1) user might expect to add a chain without the template. But that does
    not make sense really. Chains are created/deleted implicitly
    according to refcount.
2) there is not chain object like this available to user. Adding it just
    for template looks odd. Also, the "filter" and "template" are very
    much alike. They both are added to a chain, they both implicitly
    create chain if it does not exist, etc.

if you don't like "tc filter template add dev dummy0 ingress", how
about:
"tc template add dev dummy0 ingress ..."
"tc template add dev dummy0 ingress chain 22 ..."
that makes more sense I think.

Isn't it possible to avoid introducing another keyword 'template',

Can't we just do
      tc chain add dev dummy0 ingress flower chain_index 0
to create a chain that takes any types of flower rules with index 0
and
     tc chain add dev dummy0 ingress flower chain_index 22
            dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF
     tc chain add dev dummy0 ingress flower chain_index 23
            dst_ip 192.168.0.0/16
to create 2 chains 22 and 23 that allow rules with specific fields.

To create a chain that takes decap rules like
  filter protocol ip pref 2 flower chain 25 handle 0x2
    dst_mac fe:24:9a:23:4c:5c
    src_mac ce:1d:58:eb:a0:35
    eth_type ipv4
    enc_dst_ip 192.168.100.20
    enc_src_ip 192.168.100.22
    enc_key_id 100
    enc_dst_port 4789

can we create a chain using this format

tc chain add dev dummy0 ingress flower chain_index 25
    dst_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF
    src_mac 00:00:00:00:00:00/FF:FF:FF:FF:FF:FF
    enc_dst_ip 192.168.100.0/24
    enc_src_ip 192.168.100.0/24
    eth_type ipv4/arp
    enc_key_id 0/ffff
    enc_dst_port 0/ffff

When adding a filter to a pre-defined chain, does the filter need
to specify all the fields that are included in the chain's template?



instead of:

# tc filter template add dev dummy0 ingress \
     ^^^^^^^^^^^^^^^
        proto ip chain_index 22 \
        flower dst_mac 00:00:00:00:00:00/00:00:00:00:FF:FF

And then delete becomes:

# tc chain del dev dummy0 ingress chain_index 22
Error: The chain is not empty.

The fact that template is very much like a filter is sort of an
implementation detail, from user perspective it may be more intuitive
to model template as an attribute of the chain, not a filter object
added to a chain.

But I could well be the only person who feels that way :)

Reply via email to