Re: [PATCH 2/7 nft] tunnel: add erspan support

[Florian Westphal <fw@strlen.de>]

Fernando Fernandez Mancera <ffmancera@riseup.net> wrote:
> > tunnel erspan y {
> >                   id 10
> >                   ip saddr 192.168.2.10
> >                   ip daddr 192.168.2.11
> >                   sport 10
> >                   dport 20
> >                   ttl 10
> >                   version 1
> >                   index 2
> > }
> > 
> > Or was the sub-section intentional to cleanly separate the common parts
> > from the tunnel specific knobs?
> > 
> 
> The sub-section was to cleanly separate it and easily understand what 
> are the tunnel specific options configured.

Makes sense to me, thanks.

> > In that case, maybe 'tunnel y {
> > 	...
> > 	type erspan { ... '?
> > 
> > Or do you think its unecessarily verbose?
> > 
> > I think it might be good to make it clear that this is an either-or thing
> > and multiple 'type' declarations aren't permitted.
> > 
> 
> IMHO, adding "type erspan {" won't hurt but I thought it was clear 
> enough. If you think adding the "type" keyword makes it clearer, I can 
> do it for sure.

I think its fine as long as we never add additional, common subsections.
Else it will be confusing, e.g. if this is illegal:

tunnel y {
 id 10
 ip saddr 192.168.2.10
 ip daddr 192.168.2.11
 sport 10
 dport 20
 ttl 10
 erspan {
  version 1
  index 2
 }
 vxlan { ..
}

thats fine.  But if we also allow something like this in the future:
tunnel y {
 id 10
 ip saddr 192.168.2.10
 ip daddr 192.168.2.11
 sport 10
 dport 20
 ttl 10
 erspan {
  version 1
  index 2
 }
 extraops {
   ...
 }
}

Then I think it would be confusing as to what kind of 'nested { bla }'
are allowed to co-exist and whioch ones are mutually exclusive.

Thats why I mentioned the extra keyword.  But I admit that as-is its
not necessary.

> Please, notice that if more than one specific sub-section is set, the 
> bison parser will complain.

Good, thanks for explaining.