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

DASH libsai fails on remove() operations, wrong log message #158

Closed
chrispsommers opened this issue Jul 21, 2022 · 0 comments
Closed

DASH libsai fails on remove() operations, wrong log message #158

chrispsommers opened this issue Jul 21, 2022 · 0 comments
Assignees

Comments

@chrispsommers
Copy link
Collaborator

I modified vnet_out.cpp to delete items added in the test and revealed some problems. Here's the added SAI code:


    // Delete everything in reverse order
    status = sai_remove_outbound_eni_to_vni_entry(&e2v);
    if (status != SAI_STATUS_SUCCESS)
    {
        std::cout << "Failed to remove ENI To VNI" << std::endl;
        return 1;
    }

    status = sai_remove_eni_ether_address_map_entry(&eam);
    if (status != SAI_STATUS_SUCCESS)
    {
        std::cout << "Failed to remove ENI Lookup From VM" << std::endl;
        return 1;
    }

    status = sai_remove_direction_lookup_entry(&dle);
    if (status != SAI_STATUS_SUCCESS)
    {
        std::cout << "Failed to remove Direction Lookup Entry" << std::endl;
        return 1;
    }

Here's the output from bmv2 switch. We see three creates and one delete (which succeeded), there should be three deletes but the test aborted.

[21:37:46.092] [bmv2] [D] [thread 25] Entry 0 added to table 'dash_ingress.direction_lookup|dash'
[21:37:46.092] [bmv2] [D] [thread 25] Dumping entry 0
Match key:
* hdr.vxlan.vni:VNI   : EXACT     3c0000
Action entry: dash_ingress.set_outbound_direction - 

[21:37:46.092] [bmv2] [D] [thread 26] Entry 0 added to table 'dash_ingress.eni_ether_address_map|dash'
[21:37:46.092] [bmv2] [D] [thread 26] Dumping entry 0
Match key:
* meta.eni_addr:address: EXACT     aacccccccccc
Action entry: dash_ingress.set_eni - 7,

[21:37:46.093] [bmv2] [D] [thread 25] Entry 0 added to table 'dash_ingress.outbound.eni_to_vni|dash_vnet'
[21:37:46.093] [bmv2] [D] [thread 25] Dumping entry 0
Match key:
* meta.eni_id:eni_id  : EXACT     0007
Action entry: dash_ingress.outbound.set_vni - 90000,

[21:37:46.093] [bmv2] [D] [thread 26] Removed entry 0 from table 'dash_ingress.outbound.eni_to_vni

Here's the console output when running vnet_out:

$ make run-test 
...
GRPC call Write::add_one_entry OK: GRPC call Write::add_one_entry OK: GRPC call Write::add_one_entry OK: GRPC call Write::add_one_entry OK: Failed to remove ENI To VNI

Two issues:

  1. The delete succeeded in the switch but libsai treated it as a failure
  2. The message from libsai is wrong, it says "Write::add_one_entry OK: Failed to remove ENI To VNI" when it is really a delete. This is just a logging message error.

I'll post a PR to fix both.

@chrispsommers chrispsommers self-assigned this Jul 21, 2022
chrispsommers added a commit to chrispsommers/DASH that referenced this issue Jul 21, 2022
…test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method.
chrispsommers added a commit to chrispsommers/DASH that referenced this issue Jul 22, 2022
…test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method.
chrispsommers added a commit to chrispsommers/DASH that referenced this issue Jul 22, 2022
…sg. THe test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method."

This reverts commit 820e08e.
chrispsommers added a commit to chrispsommers/DASH that referenced this issue Jul 22, 2022
…test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method.
chrispsommers added a commit that referenced this issue Aug 4, 2022
* Fix #158 - libsai delete operation failure and log msg. THe test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method.

* Revert "Fix #158 - libsai delete operation failure and log msg. THe test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method."

This reverts commit 820e08e.

* Fixup perms after meta make to prevent perms issue with libsai

Co-authored-by: Chris Sommers <[email protected]>
chrispsommers added a commit that referenced this issue Aug 7, 2022
* Fix #158 - libsai delete operation failure and log msg. THe test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method.

* Revert "Fix #158 - libsai delete operation failure and log msg. THe test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method."

This reverts commit 820e08e.

* Fix init-switch target, had stale path. ADd back to CI file to catch regressions.

* Change order of tests to defer saithrift steps, allowing earlier detection of failure of faster steps, e.g. run-switch, init-switch.

Co-authored-by: Chris Sommers <[email protected]>
chrispsommers added a commit that referenced this issue Aug 8, 2022
* Fix #158 - libsai delete operation failure and log msg. THe test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method.

* Revert "Fix #158 - libsai delete operation failure and log msg. THe test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method."

This reverts commit 820e08e.

* Switch SAI submodule to upstream repo containing fixes merged from dev branches which supported DASH.

Co-authored-by: Chris Sommers <[email protected]>
chrispsommers added a commit that referenced this issue Aug 10, 2022
* Fix #158 - libsai delete operation failure and log msg. THe test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method.

* Revert "Fix #158 - libsai delete operation failure and log msg. THe test for error code was backwards and log message used same write updateType string for all operations. I fixed the compare and used a dynamic enum print method."

This reverts commit 820e08e.

* Document concise developer workflows.

* Added details

* More dev workflows.

* README typos

* More dev workflow text, diagram.

* Workflow readmes.

* Delete stray readme line

* Clarify workflow - requires make init-switch.

* Number the dev use-cases.

Co-authored-by: Chris Sommers <[email protected]>
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

Successfully merging a pull request may close this issue.

1 participant