Re: gre: minor cleanups in netlink interface

[Patrick McHardy <kaber@trash.net>]

Herbert Xu wrote:
> On Sat, Oct 11, 2008 at 04:43:03PM +0200, Patrick McHardy wrote:
>> We don't have much precedent for rtnl_link besides VLAN (which
>> does support incremental changes), but actually all other route
>> netlink interfaces do support incremental changes by sending only
>> a subset of the attributes. A reason for supporting this in the
>> interface is that incremental userspace changes will always be
>> racy because you need two seperate operations.
> 
> It is true that it is going to be racy when done in user-space,
> however that's easily solved with locking.  Even if we did the
> incremental change in the kernel it only helps certain kinds of
> usage scenarios.  For instance, if the race is between two updates
> to the local address you're still going to need synchronisation
> in user-space.

Thats true.

> Having said that, I'm certainly not against changing the interface
> since you do have precedence with the other two :)
> 
> Do be warned that doing this for GRE is going to be less trivial
> than the existing rtnl link interfaces.  For example, we'll need
> to break down the iflags/oflags into individual bits as otherwise
> you'll be back in the same situation.  It's a good thing that
> there aren't too many bits in use :)

We usually use two values (value + mask) for flags.

> Also, for ikey/okey we'll need to introduce another attribute to
> indicate their presence as well as their value.

The flags already indicate whether keys should be used, don't
they? So if you want to unset them, you can simply unset the
GRE_KEY flag.

I'll give it a shot and will post a patch, probably tommorrow.

> Hmm, it seems that there is a bug in how we treat a zero key.
> You can't have a tunnel with a zero key and one with no key at
> the same time.
> 
> In fact my latest iproute patch has a similar problem.  You
> can't unset the ikey/okey except by deleting the tunnel.  On
> the other hand the old ip tunnel interface has the same bug :)

I can't find it :)