Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Calico/VPP NSM integration] Use abstract sockets in memif #370

Closed
Bolodya1997 opened this issue Sep 9, 2021 · 7 comments
Closed

[Calico/VPP NSM integration] Use abstract sockets in memif #370

Bolodya1997 opened this issue Sep 9, 2021 · 7 comments
Assignees
Labels
enhancement New feature or request Planning Issue related to SOW

Comments

@Bolodya1997
Copy link

Bolodya1997 commented Sep 9, 2021

Description

Latest VPP changes adds possibility of abstract sockets usage for memif - it is very helpful, because it allows us to share sockets from application pod with VPP pod just by passing netns file.

Parent issue

networkservicemesh/integration-k8s-kind#325

Fixes issues

Changes needs to be done

govpp

Add VPP patches:

  1. refs/changes/77/33177/3 # vppinfra: add abstract socket & netns fns - already merged to the VPP master
  2. refs/changes/68/33268/2 # vppinfra: fix sock init netns - already merged to the VPP master
  3. refs/changes/71/32271/6 # memif: add support for ns abstract sockets

api

Add memif mechanism parameters:

  1. Namespace = common.InodeURL : string
  2. Abstract : bool (if we want to support both abstract and FS-based sockets).

sdk-vpp

  1. POC PR with only abstract sockets usage in memif - [sdk-vpp#370] Use abstract sockets for memif #369
  2. We can also support both abstract and FS-based sockets with Abstract mechanism parameter and selection done on the client side.
@Bolodya1997 Bolodya1997 changed the title [] [Calico/VPP NSM integration] Use abstract sockets in memif Sep 9, 2021
@Bolodya1997 Bolodya1997 self-assigned this Sep 9, 2021
@Bolodya1997 Bolodya1997 added enhancement New feature or request Planning Issue related to SOW labels Sep 9, 2021
@Bolodya1997
Copy link
Author

@edwarnicke @denis-tingaikin
Currently we are facing a huge problem with the VPP patches introducing abstract sockets to memif. Running NSE Composition with test with 2 passthrough with and without patches very differs in time:

without patches with patches
Request time 14s 1m 11s
ping time 70-140ms 8-9s

Request with 3 passthrough and patches takes more than 2 minutes.

Forwarder logs in general are almost the same except [vppapi:SwInterfaceDump] and [vppapi:SwInterfaceEvent] - it differs in 100+ times:
forwarder.log
forwarder.patches.log

Probably we should start thinking of another solutions of solving #357 and #361?

@edwarnicke
Copy link
Member

@Bolodya1997 Where is the bulk of the time going? If you set NSM_LOG_LEVEL=debug you should get 'duration' for most calls... I'll bet you find its in 'up' we are leaking out time.

Also... this is without directmemif, correct?

@Bolodya1997
Copy link
Author

@Bolodya1997 Where is the bulk of the time going? If you set NSM_LOG_LEVEL=debug you should get 'duration' for most calls...

@edwarnicke
These logs are captured with NSM_LOG_LEVEL=debug, I just have filtered out all other logs except VPP calls to make it easy comparable.

I'll bet you find its in 'up' we are leaking out time.

Yes, both [vppapi:SwInterfaceDump] and [vppapi:SwInterfaceEvent] are in up client.

Also... this is without directmemif, correct?

It is just the same NSE Composition test as we have running on CI, except of I have tested it with 2 passthrough NSE instead of 3, to get Request time less than 2 minutes. So it have directmemif in Forwarder in the following implementation:

  1. memifproxy client creates socket and starts proxying passed from the NSE socket.
  2. memif client registers proxy socket in VPP.
  3. up client validates established memif connection between Forwarder and NSE.
  4. memif server unregisters proxy socket from VPP and passes it to NSC.

Even if all of these steps are probably non-needed in case of directmemif (we can just pass NSE socket to the NSC : #373) we won't have directmemif for the abstract socket case. So for the abstract socket case it looks as following:

  1. memif client creates a new memif slave socket with VPP.
  2. up client validates established memif connection between Forwarder and NSE.
  3. memif server creates a new memif master socket with VPP.
  4. l2xconnect server creates xconnect between [1] and [3] sockets.

And we again start wasting lot of time in up client.

@Bolodya1997
Copy link
Author

@edwarnicke
Probably it would be better to use temporary solutions for the dependent issues while we have such problem with using abstract sockets?

  1. [Calico/VPP NSM integration] memif and memifproxy files should be shared with VPP pod #357 - can be fixed with using shared directory for storing memif files.
  2. [Calico/VPP NSM integration] memif socket ID generation is wrong for the single VPP scenario #361 - use random numbers for generating socket IDs and start checking for the returned "already exists" error.

Without fixing these 2 issues we cannot get any memif tests working with Calico.

@Bolodya1997
Copy link
Author

@edwarnicke @denis-tingaikin
Everything is currently ready and tested:

  1. PR to api with memif parameters changes - [sdk-vpp#370] Change memif to work with abstract sockets api#113.
  2. PR to sdk-vpp with abstract sockets memif implementation - [sdk-vpp#370] Use abstract sockets for memif #369.
  3. PR to vpphelper with new types for the VPP Connection - Add Internal-, ExternalConnection edwarnicke/vpphelper#13.
  4. PR to cmd-forwarder-vpp with update to [1, 2, 3] - [sdk-vpp#370] Use abstract sockets for memif cmd-forwarder-vpp#371.
  5. PR to cmd-nse-icmp-responder-vpp with update to [2] - [sdk-vpp#370] Use abstract sockets for memif cmd-nse-icmp-responder-vpp#269.
  6. PR to cmd-nse-firewall-vpp with update to [2] - [sdk-vpp#370] Use abstract sockets for memif cmd-nse-firewall-vpp#96.

@Mixaster995
Copy link
Contributor

@edwarnicke i retested integration with calico on equinix, that Vlad made, everything seems to work fine
could you, please, take a look at #470
if you okay with changes in this PR, then we can start merging all PRs

@denis-tingaikin
Copy link
Member

It looks like all PRs have merged. So closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Planning Issue related to SOW
Projects
None yet
Development

No branches or pull requests

4 participants