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

cmd/gobgp: Fix evpn parsing #2665

Merged
merged 3 commits into from
Jun 7, 2023
Merged

cmd/gobgp: Fix evpn parsing #2665

merged 3 commits into from
Jun 7, 2023

Conversation

costasd
Copy link
Contributor

@costasd costasd commented Jun 6, 2023

This PR:

  1. Adds tests for all EVPN cli parsers using the full examples under docs/source/evpn.md for each.
  2. Fixes a parsing issue for evpn a-d, not parsing esi-label with optional arguments correctly.
  3. Fixes a parsing issue for evpn macadv parsing breaking when default-gateway is set

Add tests to validate parsing of all EVPN-related configuration.

Examples taken from docs/sources/evpn.md.
esi-label follows the format:
   [esi-label <esi-label> [single-active | all-active]]

extractReserved() splits by spaces, returning a list for esi-label, hence
failing the type check.

Change its parameter value type to paramList to be able to accomodate cases
such as "... esi-label 400 single-active".
default-gateway not being declared a reserved keyword led to evpn
macadv commands ending as "encap vxlan default-gateway" to fail parsing.

In that case, default-gateway was bundled with the previous argument.

To fix that, set default-gateway as a reserved keyword of paramFlag type.

While there, change the way we detect its existence later on, given that it now
exists in our reserved map m, if set.
@costasd
Copy link
Contributor Author

costasd commented Jun 7, 2023

hmm, seems like a transient failure between stopping the server and writing to it - I can't reproduce it locally.

Looking at the code, it is definitely unrelated to this PR's changes (happens in pkg/bgp/server_test.go, which doesn't use anything out of cmd/gobgp).

@fujita fujita merged commit 0cc8a98 into osrg:master Jun 7, 2023
@fujita
Copy link
Member

fujita commented Jun 7, 2023

Thanks. Surely, unrelated to this pr.

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 this pull request may close these issues.

2 participants