search

ligato / vpp-agent

⚡️ Control plane management agent for FD.io's VPP
https://docs.ligato.io/
Apache License 2.0
252 stars 126 forks source link

VppConfig.Interfaces.AfpacketLink.LinuxInterface disallows important use case #1648

Closed edwarnicke closed 4 years ago

edwarnicke commented 4 years ago

We have an important use case in NSM where we use AF_PACKET to bind to an existing interface in the processes namespace (ie, no namespace switching). Binding to it should in no way change the config of that interface.

Previously, we'd used VppConfig.Interfaces.AfpacketLink.HostInterface

and given the name of the HostInterface. This did and still does work. We however noticed that it was deprecated, and tried to switch to LinuxInterface.

We discovered when we did this that our AF_PACKET interfaces were no longer created, being listed in a pending state:

MODEL                  NAME                                            STATE        DETAILS                          LAST OP   ERROR                                                        
vpp.interfaces         eth0                                            PENDING      afpacket-host-interface-exists   CREATE

Upon consultation with @ondrej-fabry , I tried adding a LinuxConfig.Interface with type EXISTING.

This resulted in vppagent deleting the IP addresses on that interface (which we should not be interfering with).

I completely understand the value in being able to use LinuxConfig.Interface with type EXISTING to manage their config (including finding interfaces in different namespaces)... but it results in making it impossible to achieve our existing use case.

milanlenco commented 4 years ago

Try to define the EXISTING interface as link-only, which basically means that the agent will only wait for interface to be created and should leave addresses untouched. https://github.com/ligato/vpp-agent/blob/master/proto/ligato/linux/interfaces/interface.proto#L59

If it doesn't help we can think of something else or remove the deprecated flag and explain differences between the reference alternatives and how to decide which one to use.

ondrej-fabry commented 4 years ago

@edwarnicke is this considered solved by #1732 ?

edwarnicke commented 4 years ago

@ondrej-fabry I don't think this is related at all to #1732

edwarnicke commented 4 years ago

Try to define the EXISTING interface as link-only, which basically means that the agent will only wait for interface to be created and should leave addresses untouched. https://github.com/ligato/vpp-agent/blob/master/proto/ligato/linux/interfaces/interface.proto#L59

That doesn't really get to the heart of the problem, which is that nothing should be done to the existing interface, at all, ever (including on delete). You should only bind AF_PACKET to it, or on delete unbind AF_PACKET from it.

ondrej-fabry commented 4 years ago

What is being done to the interface when using type=EXISITING and link_only=true ?

milanlenco commented 4 years ago

What is being done to the interface when using type=EXISITING and link_only=true ?

Nothing.

However I also gave an option to remove the deprecated flag and explain differences between the reference alternatives, but I didn't get any answer since April...

edwarnicke commented 4 years ago

Apologies for the silence there... if link_only=true doesn't do anything to the link I'm OK with that... but link_only to me sounds like its still doing stuff to the link layer (the kinds of things you do with ip link), so it may be a suboptimal naming.

Honestly, the 'HostIfName' not being deprecated is probably better... its super confusing to have to configure something in config.GetLinuxConfig() when it has no effect at all, just because you want to bind a VPP interface to it with AF_PACKET (or hopefully soon, AF_XDP).

milanlenco commented 4 years ago

OK, agree, let's get rid of the deprecated flag then (+ improved comments in the proto). I will open a PR tomorrow...