Skip to content

[VPP-1993] Add ip_table_allocate api to vpp #3455

@vvalderrv

Description

@vvalderrv

Description

Currently in ip.api we define a ip_table_add_del which requires the api user to specify the vl_api_ip_table_t.

 

This minimally produces a burden on the api user to track table ids not just for local use, but to avoid collision by allocation.  The problem becomes worse when multiple api users are involved, as coordination among them becomes even more problematic.

To solve this problem, introduce a new API:

/** \brief Allocate an unused table

  A table can be added multiple times, but need be deleted only once.

  @param client_index - opaque cookie to identify the sender

  @param context - sender context, to match reply w/ request

  @param table - if table.table_id == ~0, vpp allocates an unused table_id and proceeds as in vpp_table_add_del

                             if table.table_id != ~0, vpp uses the table.table_id and proceeds as in vpp_table_add_del

*/

define ip_table_allocate

{

    u32 client_index;

    u32 context;

    vl_api_ip_table_t table;

};

/**

  @param table - table information for the table allocated}}

*/

define ip_table_allocate_reply

{

  u32 client_index;

u32 context;

vl_api_ip_table_t table;

};}}

Assignee

Aloys Augustin

Reporter

Ed Warnicke

Comments

  • nranns (Mon, 13 Sep 2021 17:00:29 +0000): Hi Aloys,

The table-ID is what VPP considers to be a CP key. It's not your CP's key, hence the need for your CP to convert. other CPs do have a table-ID as a key, they manage that space, and thus don't need to convert. Your patch is putting the 'manage the ID space' work into VPP, it doesn't belong there.

It's this difference in CP key type that is causing you the pain. But it's pain specific to your CP, so it's your CP's problem to solve - sorry.

if you really really want VPP to make unique IDs for you write a plugin to do so, but IMHO you'll be better of managing the space in the CP. if you don't have a central place to do this, then you could carve the ID space into smaller portions (say setting the x MSBs) and give that portion to each to allocate from.

  • aloys (Mon, 13 Sep 2021 15:19:35 +0000): Hi Neale,

The table-id is not the CP key. It's an intermediate representation: "VRF Red" (CP ID) -> table-id -> fib-index (DP ID). The CP needs to convert anyway, so why force it to go through this intermediate value that doesn't bring anything, and force it to manage this id space to make sure there's no conflicts? It might as well use fib-index directly (as is done for sw_if_index).

Yes that's not what is implemented by the patch, but I think that's where we end up if we push your reasoning to its end. It would also solve our issue

Now we can do such a patch if you're fine with that approach, but that will be a much larger change, so instead the two patches proposed make table-id look like a DP ID, and hides all this under the rug for future clients...

Also I don't buy the restart argument because the CP has a stable ID for a fib: "VRF Red". That the DP ID changes on restart is easier to handle than having to manage an ID space on behalf of VPP.

  • nranns (Fri, 10 Sep 2021 18:57:02 +0000):

    the table-ID is not the dataplane identifier, it's the control plane's identifier, this value is arbitrary, i.e. the CP is free to assign meaning to any of the bits. The fib-index is the data-plane identifier, this value is not exposed to the user, this value is a pool-index, i.e. it's small.

there is a hash-map in VPP to convert from CP-ID to DP-ID.

there's nothing natural about the DP assigning CP IDs.

you might call #1 distorting, but to me that's API type matching. it's just what you have to do if you want to use an API from a 3rd party. i don't suppose your CP represents a route as a vl_api_ip_route_t either; conversions are required.

All your CP needs to do to get the same behaviour as you're asking for in the patch is next_id = global_id++. If there are more than one entity in your CP that needs a table_id then VPP is NOT the message bus through which they communicate.

  • hagbard (Fri, 10 Sep 2021 14:07:00 +0000): Neale Ranns Or we could do the natural thing:
  1. Have the DP manage its own identifier space (optionally) as it does for other DP identifiers, like sw_if_index.

Your option #1 involves distorting the CP to match VPPs VRF identifiers and manage the space of those identifiers.

Your option #2 involves distorting VPP to store arbitrary keys for VRFs

Both an unnatural acts (one in CP, one in DP). What I'm proposing is the natural act: let VPP (optionally) manage its own identifier space for the DP table_id identifiers it uses in its API.

Which of the two patches proposed to do so seems like the best solution to you?

  • nranns (Fri, 10 Sep 2021 07:59:24 +0000): >> It may incidentally be true that a particular CP also chooses to use them as its own CP identifiers, but it is not necessarily true.

and therein lies the problem. your CP key type is not the same as what VPP excepts the VRF key type to be. So you have 2 choices;

1 - convert your CP key types to VPP key types in the CP

2 - change VPP to support arbitrary key types for VRFs, and arbitrary because, as you say, the next CP might not use the say key type as you do.

1 is the easier solution because it is your domain to control.

Your VPP solution is this:

  1. CP create VRF RED

  2. VPP assign table ID X

  3. VPP create table ID X

  4. CP store RED=X

you need to change 2 so it's done by the CP and maybe then combine it with 4.

  • hagbard (Thu, 9 Sep 2021 20:02:57 +0000): Neale Ranns Ipso facto as soon table_ids are used as the handle I must use in VPPs APIs to program to that VRF (add routes, attach interfaces, etc), they become DP identifier. I have no other handle to ask the DP to do anything related to that VRF.

It may incidentally be true that a particular CP also chooses to use them as its own CP identifiers, but it is not necessarily true.

Your mapping example basically proves that point: if table_id really were the CP identifier, it would be what ever it is that the CP thinks should identify VRFs. The fact that I can't create 'VRF Red' using a string and use that as a handle demonstrates that table_ids are not a CP identifier (and less there be misunderstanding, while it might be nice to have tags on VRFs... I am not advocating for VPP using strings in place of table_ids ).

It's perfectly fine to say the CP is responsible to keep a mapping between its notion of VRFs and the table_ids VPP is using as DP identifiers for them in the API. There's no way out of that responsibility, but VPP makes the leap to require the CP responsible for managing the identifier space of table_ids... which is a much bigger leap.

As to restarts of VPP. All any CP really cares about is making sure it has a working VPP with the VPP bits either corresponding to the outside bits (hw interfaces, virtual interfaces in various namespaces with various names, etc) configured properly and the correct interrelation between those things (routes, vrfs, etc). If VPP restarts, the CP will want to restore those. But no CP intrinsically cares about matching sw_if_ids, table_ids, etc... those are all non-sequiturs to the CP's desires. Some CPs may chose to achieve that restoration of relationships and configuration in the DP by keeping a detailed mirror of the VPP config including all the magic numbers, and I agree such CPs should continue to be able to do so. But requiring all CPs to do so is unreasonable, and a huge imposition of the DPs world view onto the CPs, akin to requiring the DP to keep a detailed map of the CPs model of the world.

I can understand how we got to this place: there is doubtless a requirement to allow the CP to assume responsibility of the identifier space for VRFs in the API. It enables the 'precise mirroring of state' approach, which does have its place. Enabling that case does not mandate requiring that case, which is the current situation.

  • nranns (Thu, 9 Sep 2021 18:04:19 +0000):

    but your angle is obtuse - bam dum tish.

your control plane has identified the need for a VRF to exist, it obviously then has a means to refer to it, so it has a key, i'll wager it's a string e.g. 'vrf RED'. now there may be a disconnect between what your control plane uses for a key, a string, and what VPP's API says a CP key for a VRF is, a u32. your job in the CP, is to get a u32 from your key.

your patch is basically saying i want VPP to be the my-string-key-to-vpp-u32-key converter, and VPP is not a good thing for that because the mapping is not persistent cross VPP restarts.

  • aloys (Thu, 9 Sep 2021 14:31:35 +0000): Hi Neale,

I get your point. Trying to take a slightly different angle here, could we do entirely without a CP key? If there's already an unique ID that's used as a DP key, couldn't we just let the CP use that to identify a fib?

From an API point of view, it seems a bit awkward in the first place to ask the CP to generate an ID, then store that ID in VPP with no other purpose than giving it back to the CP.

  • nranns (Thu, 9 Sep 2021 13:43:35 +0000):

    the sw_if_index is not the CP key, it's the DP key. The CP key is 'the ethernet at PCI 0:0' or 'the tunnel that goes to peer foo' - these keys are values that don't change across a VPP reboot. the sw_if_index assigned to a particular interface, as defined by the CP key, will change.

there is an similarity between interface key and sw_if_index and the table_id and fib_index.

A control Plane needs a consistent key for a thing across VPP reboots.

  • hagbard (Thu, 9 Sep 2021 10:44:44 +0000): Neale Ranns Why is this table id different than SwIfIndex?
  • nranns (Thu, 9 Sep 2021 07:45:20 +0000): VPP is not the entity to use to solve control plane co-ordination problems. Your control plane needs a persistent shared table-ID allocator DB. If VPP restarts then it will start serving table-IDs that are in use.
  • aloys (Wed, 18 Aug 2021 15:45:16 +0000): We have a coordination problem on table IDs between NSM and Calico/VPP, this proposal looks like a good way to solve it.

Original issue: https://jira.fd.io/browse/VPP-1993

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions