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

Rework of direct memif #373

Closed
Mixaster995 opened this issue Sep 10, 2021 · 3 comments
Closed

Rework of direct memif #373

Mixaster995 opened this issue Sep 10, 2021 · 3 comments
Assignees

Comments

@Mixaster995
Copy link
Contributor

Mixaster995 commented Sep 10, 2021

Description

I think, it would be good to change current direct memif implementations.
Now there are excessive files creation in case of direct memif, but it causes issue in nse-composition networkservicemesh/deployments-k8s#2381
It might be better to just pass socket url up in chain without taking additional moves.

Current logic

DirectMemif (1)

  1. On way back memif_3 created on client side of forwarder on the same file as memif_4
  2. server-side of forwarder checking memif_3 existence and deleting memif_3 if it's present 3) server-side of forwarder not creating memif_2 and just passes memif_3 file url(same as memif_4) up in chain.
  3. memif_1 created with memif_4 socket url

Suggestions

Rework steps 2,3:

  • remove memif_3 creation on client-side of forwarder and pass socket url up in chain
  • remove memif_3 check and deletion logic on server-side of forwarder and pass url to memif_1.
@Mixaster995 Mixaster995 added enhancement New feature or request question Further information is requested labels Sep 10, 2021
@Mixaster995 Mixaster995 changed the title Rework direct memif Rework of direct memif Sep 10, 2021
@networkservicemesh networkservicemesh locked as off-topic and limited conversation to collaborators Sep 10, 2021
@denis-tingaikin denis-tingaikin removed enhancement New feature or request question Further information is requested labels Sep 10, 2021
@edwarnicke
Copy link
Member

@Mixaster995 Is this related to the memifproxy?

@edwarnicke
Copy link
Member

@Mixaster995 Have we gotten to a root cause here?

@Mixaster995
Copy link
Contributor Author

@edwarnicke well, not exactly - i've found half-root of this issue:
It is related to memif creation in vpp in case of direct memif scenario.
On refresh request up client hangs here - https://github.com/networkservicemesh/sdk-vpp/blob/main/pkg/networkservice/up/common.go#L100
I'm not sure what is the deep reason for this, but without unnecessary memif interface creation/deletion(current implementation) everything works fine.

A also added a comment in pr discussion, with details about changes i've made #377.

Please, share your thought about this proposed changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants