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

sdk-vpp leaks VPP resources in case of context timeout #378

Open
Bolodya1997 opened this issue Sep 15, 2021 · 2 comments
Open

sdk-vpp leaks VPP resources in case of context timeout #378

Bolodya1997 opened this issue Sep 15, 2021 · 2 comments
Labels
ASAP As soon as possible bug Something isn't working

Comments

@Bolodya1997
Copy link

Expected Behavior

sdk-vpp shouldn't leak any resources.

Current Behavior

sdk-vpp sometimes leaks VPP resources in case of context canceled/timeout:

func Request(ctx context.Context) (conn, err) {
    if err := requestVPP(ctx); err != nil {
        return nil, err // <-- error can be caused by context canceled/timeout
    }
}

It affects almost every sdk-vpp chain element, so it looks like a critical issue.

Failure Logs

forwarder.log

  1. First Request fails with context deadline exceeded in tap chain element.
  2. All subsequent Requests fail with VPPApiError: netlink error (-145) because of already existing kernel interface with the requested name.

Possible solution

We should probably do something like the following in all sdk-vpp chain elements which allocates some VPP resources:

func Request(ctx context.Context) (conn, err) {
    postponeCtxFunc := postpone.Context(ctx)

    conn, err := next.Request()
    if err != nil {
        return nil, err
    }

    addDel := &vppAddDelSomething{...}
    if err := addDelSomething(ctx, addDel); err != nil {
        if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
            delCtx, cancelDel := postponeCtxFunc()
            defer cancelDel()

            addDel.IsAdd = false
            _ = addDelSomething(delCtx, addDel)
        }
        return nil, err
    }

    return conn, nil
}
@Bolodya1997 Bolodya1997 added the bug Something isn't working label Sep 15, 2021
@Bolodya1997
Copy link
Author

@edwarnicke @denis-tingaikin
It seems to be critical, please take a look.

@denis-tingaikin denis-tingaikin added the ASAP As soon as possible label Sep 15, 2021
@denis-tingaikin
Copy link
Member

I think we can create an helper function that will call cancel if context has been expired during resource allocation.

@edwarnicke Please share your thoughts on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASAP As soon as possible bug Something isn't working
Projects
Status: No status
Development

No branches or pull requests

2 participants