Commands for managing bgp auth and route local preference #795

rcgoodfellow · 2024-08-16T16:14:41Z

This PR adds three new networking commands

bgp auth to manage MD5 authentication authorization strings
bgp pref to manage the local preference setting of BGP peers
route to manage static routes

All of the above perform partial updates on port settings objects through read-modify-write actions.

elaine-oxide

See comments regarding help output and #[command(name = "xyz")]

cli/src/cmd_net.rs

cli/docs/cli.json

elaine-oxide · 2024-08-19T22:56:24Z

cli/docs/cli.json

              ]
            },
+            {
+              "name": "route",


Just noting that no help appears for this top level route command (nor for the bgp command).

The help looks like:

$ oxide system networking --help Usage: oxide system networking [OPTIONS] <COMMAND> Commands: addr Manage switch port addresses. address-lot allow-list bfd bgp link Manage switch port links. loopback-address route switch-port-settings help Print this message or the help of the given subcommand(s)

Updated the docstrings that are for custom commands. For the autogenerated ones, not sure there is much that can be done here.

Note that bgp is a combined custom command and an autogenerated one.

cli/docs/cli.json

elaine-oxide · 2024-08-20T04:57:37Z

I am using Oxide CLI version:

$ oxide version
Oxide CLI 0.6.1+20240710.0
Built from commit: 1f2e53c6ae7e90f06bab93e65ef8f52d1228a006 
Oxide API: 20240821.0

I am running a4x2 with the following combination of commits:

omicron 6dd980251a26430466bcd5aff1edad5416cf94e5
maghemite 242e67e1e6b363b9bae04c90920fb8f43ac0a51d
oxnet 7dacd265f1bcd0f8b47bd4805250c4f0812da206

I tried deleting a peer that I never created (no peer exists at 169.254.40.2; I never performed an oxide system networking bgp peer add operation for it)

$ oxide system networking bgp peer del --rack 51becb40-1818-4ea0-b278-2af26bc8bbe4 --switch switch1 --port qsfp0 --addr 169.254.40.2
# no output here

It unexpectedly did not provide a warning that the peer did not exist.

=====

Update on 27 Aug 2024:

Issue resolved.

$ oxide system networking bgp peer delete --rack 2d1fe615-0fd9-485d-8898-cd024883d222 --switch switch1 --port qsfp0 --addr 169.254.40.2
no peers match the provided address

elaine-oxide · 2024-08-20T05:54:48Z

I am using Oxide CLI version:

$ oxide version
Oxide CLI 0.6.1+20240710.0
Built from commit: 1f2e53c6ae7e90f06bab93e65ef8f52d1228a006 
Oxide API: 20240821.0

I am running a4x2 with the following combination of commits:

omicron 6dd980251a26430466bcd5aff1edad5416cf94e5
maghemite 242e67e1e6b363b9bae04c90920fb8f43ac0a51d
oxnet 7dacd265f1bcd0f8b47bd4805250c4f0812da206

I tried the following:

# qsfp1 is not actually existing for switch0 nor for switch1.

# This command uses qsfp1 (does not exist) and 169.254.40.1 (peer exists)
$ oxide system networking bgp auth --rack 51becb40-1818-4ea0-b278-2af26bc8bbe4 --switch switch1 --port qsfp1 --peer 169.254.40.1 --authstring <password>
Port settings uninitialized. Initialize by creating a link.

# This command uses qsfp1 (does not exist) and 169.254.40.1 (peer exists)
$ oxide system networking bgp auth --rack 51becb40-1818-4ea0-b278-2af26bc8bbe4 --switch switch0 --port qsfp1 --peer 169.254.40.1 --authstring <password>
Port settings uninitialized. Initialize by creating a link.

# This command uses qsfp1 (does not exist) and 169.254.10.1 (peer exists)
$ oxide system networking bgp auth --rack 51becb40-1818-4ea0-b278-2af26bc8bbe4 --switch switch0 --port qsfp1 --peer 169.254.10.1 --authstring <password>
Port settings uninitialized. Initialize by creating a link.

Also

# It was very difficult to figure out acceptable values for the last two flags `--fec` and `--speed`
$ oxide system networking link add --rack 51becb40-1818-4ea0-b278-2af26bc8bbe4 --switch switch1 --port qsfp1 --fec none --speed speed100_g
Port settings uninitialized. Initialize by creating a link.

Error message could be less terse for end user or operator.

More help output needed for acceptable values for the flags indicated.

=====

Update on 27 Aug 2024:

Last item (but not earlier items) resolved:

$ oxide system networking link add --rack 2d1fe615-0fd9-485d-8898-cd024883d222 --switch switch1 --port qsfp1 --fec none --speed 100g
Port settings uninitialized. Initialize by creating a link.

elaine-oxide · 2024-08-22T01:32:31Z

I am running Oxide CLI version:

$ oxide version
Oxide CLI 0.6.1+20240710.0
Built from commit: 1f2e53c6ae7e90f06bab93e65ef8f52d1228a006 
Oxide API: 20240821.0

I am running a4x2 with omicron d24003b74d232d2ff643a6fc9b5ee10cb5f2b056.

I tried the following (the 90 char authstring is not the real password, it is deliberately a wrong password):

$ oxide system networking bgp auth --rack 1649d843-2282-42e7-b3bf-2f61d4d80341 --switch switch1 --port qsfp0 --peer 169.254.40.1 --authstring 123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890

Output for switch1 unexpectedly became truncated, and remained truncated, even after setting authstring to correct password, and even after setting authstring to not be present.

$ oxide system networking bgp show-status
switch0
=======
Peer Address  Local ASN  Remote ASN  Session State  State Duration
169.254.30.1  65547      64502       Established    2h 6m 58s 677ms
169.254.10.1  65547      64500       Established    2h 6m 58s 705ms

switch1
=======
Peer Address  Local ASN  Remote ASN  Session State  State Duration

The following showed expected output for all password variations (90 chars, no password, real password).

$ oxide system networking switch-port-settings show

$ oxide api /v1/system/networking/switch-port-settings/<uplink name applicable to 169.254.40.1>

In addition to trying a long password like the 90 char authstring above, I wanted to try password with special chars, such as abc%123$@.

=====

Update on 27 Aug 2024:

Issues resolved.

$ oxide system networking bgp auth --rack 619c28eb-c688-4823-b846-a6f9ed25be12 --switch switch1 --port qsfp0 --peer 169.254.40.1 --authstring 123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
Error Response: status: 400 Bad Request; headers: {"content-type": "application/json", "x-request-id": "70a5adda-a36f-42af-8766-821311c2987b", "content-length": "205", "date": "Wed, 28 Aug 2024 00:31:42 GMT"}; value: Error { error_code: Some("InvalidValue"), message: "unsupported value for \"md5_auth_key\": md5 auth key for 169.254.40.1 is longer than 80 characters", request_id: "70a5adda-a36f-42af-8766-821311c2987b" }

oxide system networking bgp auth --rack 619c28eb-c688-4823-b846-a6f9ed25be12 --switch switch1 --port qsfp0 --peer 169.254.40.1 --authstring abc%123$@

elaine-oxide · 2024-08-22T02:04:18Z

Suggestion to consider naming consistency for flags, in CLI commands below, where the first one has --authstring and the second one has --md5-auth-key.

$ oxide system networking bgp auth --rack <rack id> --switch switch1 --port qsfp0 --peer 169.254.40.1 --authstring <password>

$ oxide system networking bgp peer add --rack <rack id> --switch switch1 --port qsfp0 --addr 169.254.40.1 --bgp-config <bgp config name or id> --md5-auth-key <password>

rcgoodfellow · 2024-08-22T04:14:30Z

Per RFC 2385 section 4.5

It should be noted that the key configuration mechanism of routers
may restrict the possible keys that may be used between peers. It is
strongly recommended that an implementation be able to support at
minimum a key composed of a string of printable ASCII of 80 bytes or
less, as this is current practice.

We only support up to 80-byte keys. The underlying TCP-MD5 implementation in illumos also only supports up to 80 byte keys.

rcgoodfellow · 2024-08-26T01:48:19Z

It unexpectedly did not provide a warning that the peer did not exist.

I've added warnings for when delete operations do nothing.

rcgoodfellow · 2024-08-26T02:04:00Z

It was very difficult to figure out acceptable values for the last two flags --fec and --speed

The output of the link add command now looks like this

$ oxide system networking link add --help
Add a link to a port

Usage: oxide system networking link add [OPTIONS] --rack <RACK> --switch <SWITCH> --port <PORT> --fec <FEC> --speed <SPEED>

Options:
      --rack <RACK>
          Id of the rack to add the link to

      --profile <PROFILE>
          Configuration profile to use for commands

      --switch <SWITCH>
          Switch to add the link to

          [possible values: switch0, switch1]

      --port <PORT>
          Port to add the link to

          [possible values: qsfp0, qsfp1, qsfp2, qsfp3, qsfp4, qsfp5, qsfp6, qsfp7, qsfp8, qsfp9, qsfp10, qsfp11, qsfp12, qsfp13, qsfp14, qsfp15, qsfp16, qsfp17, qsfp18, qsfp19, qsfp20, qsfp21, qsfp22, qsfp23, qsfp24, qsfp25, qsfp26, qsfp27, qsfp28, qsfp29, qsfp30, qsfp31]

      --autoneg
          Whether or not to set auto-negotiation

      --fec <FEC>
          The forward error correction mode of the link

          Possible values:
          - firecode: Firecode forward error correction
          - none:     No forward error correction
          - rs:       Reed-Solomon forward error correction

      --mtu <MTU>
          Maximum transmission unit for the link

          [default: 1500]

      --speed <SPEED>
          The speed of the link

          Possible values:
          - 0g:   Zero gigabits per second
          - 1g:   1 gigabit per second
          - 10g:  10 gigabits per second
          - 25g:  25 gigabits per second
          - 40g:  40 gigabits per second
          - 50g:  50 gigabits per second
          - 100g: 100 gigabits per second
          - 200g: 200 gigabits per second
          - 400g: 400 gigabits per second

  -h, --help
          Print help (see a summary with '-h')

rcgoodfellow · 2024-08-26T02:05:27Z

Output for switch1 unexpectedly became truncated, and remained truncated, even after setting authstring to correct password, and even after setting authstring to not be present.

This is fixed by

bgp: check md5 passkey length on create/update omicron#6428

rcgoodfellow · 2024-08-26T02:08:39Z

Suggestion to consider naming consistency for flags, in CLI commands below, where the first one has --authstring and the second one has --md5-auth-key.

Both are now authstring

rcgoodfellow · 2024-08-26T02:47:58Z

@elaine-oxide I believe I've addressed your feedback.

elaine-oxide · 2024-08-26T22:13:12Z

Suggestion to consider naming consistency for flags, in CLI commands below, where the first one has --authstring and the second one has --md5-auth-key.

Both are now authstring

Now the help looks like:

$ oxide system networking bgp auth --help
...
      --authstring <AUTHSTRING>  Authorization string

$ oxide system networking bgp peer add --help
 ...
       --authstring <AUTHSTRING>
          Use the given authorization string for TCP-MD5 authentication with the peer

Could choose to copy the second one (it's more descriptive) to the first one.

elaine-oxide

See comments.

cli/docs/cli.json

cli/src/cmd_net.rs

cli/docs/cli.json

rcgoodfellow · 2024-08-26T23:39:32Z

Could choose to copy the second one (it's more descriptive) to the first one.

done

elaine-oxide

See comments.

cli/src/cmd_net.rs

elaine-oxide · 2024-08-27T03:16:44Z

Minor comment (can ignore):

Inconsistent appearance of periods at the end of the help strings for the top level help, but consistent (no periods used) for the lowest level help output that I checked (I only checked the lowest level for commands that are new in this PR).

$ oxide system networking bgp --help
Usage: oxide system networking bgp [OPTIONS] <COMMAND>

Commands:
  announce      Announce a prefix over BGP.
  announce-set  
  auth          
  config        
  exported      Get BGP exported routes
  filter        Add a filtering requirement to a BGP session.
  history       Get BGP router message history
  imported      
  peer          Manage BGP peers.
  pref          
  show-status   Get the status of BGP on the rack.
  status        Get BGP peer status
  withdraw      Withdraw a prefix over BGP.
  help          Print this message or the help of the given subcommand(s)

$ oxide system networking --help
Usage: oxide system networking [OPTIONS] <COMMAND>

Commands:
  addr                  Manage switch port addresses.
  address-lot           
  allow-list            
  bfd                   
  bgp                   
  link                  Manage switch port links.
  loopback-address      
  route                 Manage static switch routes.
  switch-port-settings  
  help                  Print this message or the help of the given subcommand(s)

elaine-oxide

See minor comments.

cli/src/cmd_net.rs

elaine-oxide

See comment.

elaine-oxide · 2024-08-27T03:21:45Z

cli/src/cmd_net.rs

    /// Set a filtering specification for a peer.
    Filter(CmdBgpFilter),
+
+    /// Set an authentication string.


It's not clear to me why this help string and the one on line 225 do not appear in the output.

$ oxide system networking bgp --help Usage: oxide system networking bgp [OPTIONS] <COMMAND> Commands: announce Announce a prefix over BGP. announce-set auth config exported Get BGP exported routes filter Add a filtering requirement to a BGP session. history Get BGP router message history imported peer Manage BGP peers. pref show-status Get the status of BGP on the rack. status Get BGP peer status withdraw Withdraw a prefix over BGP. help Print this message or the help of the given subcommand(s) Options: --profile <PROFILE> Configuration profile to use for commands -h, --help Print help

It also doesn't appear here:

$ oxide system networking bgp auth --help Usage: oxide system networking bgp auth [OPTIONS] --rack <RACK> --switch <SWITCH> --port <PORT> --peer <PEER> Options: --rack <RACK> Id of the rack to add the auth config to --profile <PROFILE> Configuration profile to use for commands --switch <SWITCH> Switch to add the auth config to [possible values: switch0, switch1] --port <PORT> Port to add the auth config to [possible values: qsfp0, qsfp1, qsfp2, qsfp3, qsfp4, qsfp5, qsfp6, qsfp7, qsfp8, qsfp9, qsfp10, qsfp11, qsfp12, qsfp13, qsfp14, qsfp15, qsfp16, qsfp17, qsfp18, qsfp19, qsfp20, qsfp21, qsfp22, qsfp23, qsfp24, qsfp25, qsfp26, qsfp27, qsfp28, qsfp29, qsfp30, qsfp31] --peer <PEER> Peer to add the auth config to --authstring <AUTHSTRING> Use the given authorization string for TCP-MD5 authentication with the peer -h, --help Print help

This is because those top-level structures are never included in cli/src/main.rs. See the add_custom directives in main.rs to see how commands are included in the overall CLI program. Because some top-level names like bgp must share their command namespace with commands that are autogenerated from the API, it is not possible to include them using cmd_net, so we need to cherry-pick more specific commands below the bgp part of the command.

I've added a few more dostrings. But the purely API-generated commands will not have doscstrings.

elaine-oxide

See comments.

cli/src/cmd_net.rs

elaine-oxide

Completed runtime testing on this version for static route local preference (performed runtime testing on earlier version for MD5 auth).

See minor comments (optional).

elaine-oxide · 2024-08-27T18:02:19Z

cli/src/cmd_net.rs

 enum BgpConfigPeerSubCommand {
-    /// Add a BGP peer to a port configuration.
-    Add(CmdBgpPeerAdd),
+    /// Set a BGP peer on a port configuration.


Looking at consistency of terminology in help output.

oxide system networking bgp auth --help says "switch port settings configuration".

oxide system networking bgp peer --help says "switch port settings objects".

oxide system networking bgp peer set --help (here) says "port configuration".

oxide system networking bgp peer delete --help says "port configuration".

oxide system networking bgp pref --help says "switch port settings configuration".
Could make consistent as needed.

elaine-oxide · 2024-08-27T18:10:59Z

cli/src/cmd_net.rs

+    #[arg(long)]
+    peer: IpAddr,
+
+    /// Apply this local preference to routes received from the peer.


User currently must see an example to know what type/values are allowed, not obvious from help output. This is the case for all places where this flag is specified:

oxide system networking route set

oxide system networking bgp peer set

oxide system networking bgp pref (here)

Maybe there is a switch for clap to make the help more useful. But I don't think we should go manually comment on the type of every single clap argument.

ahl

looks good to me.

ahl · 2024-08-28T15:35:14Z

cli/src/cmd_net.rs

+#[derive(clap::ValueEnum, Clone, Copy, Debug)]
+pub enum CliLinkFec {
+    /// Firecode forward error correction.
+    Firecode,
+    /// No forward error correction.
+    None,
+    /// Reed-Solomon forward error correction.
+    Rs,
+}
+
+impl From<CliLinkFec> for LinkFec {
+    fn from(value: CliLinkFec) -> Self {
+        match value {
+            CliLinkFec::Firecode => LinkFec::Firecode,
+            CliLinkFec::None => LinkFec::None,
+            CliLinkFec::Rs => LinkFec::Rs,
+        }
+    }
+}


FWIW, https://github.com/oxidecomputer/oxide.rs/blob/main/sdk/src/clap_feature.rs show another way to address what I infer to be the problem you were solving here, but I'm not sure.

ahl · 2024-08-28T15:39:56Z

cli/src/cmd_net.rs

+            eprintln_nopipe!("no originated prefixes match the provided args");
+            return Ok(());


Do we want to return an anyhow error or does that not produce the desired result? Do we test this (do we want to)?

ahl · 2024-08-28T15:42:16Z

cli/src/cmd_net.rs

+                    eprintln_nopipe!("no routes match the provided args");
+                    return Ok(());


This error message was produced when I tried to delete the same route twice (which meant I was deleting a non-existing route the second time).

$ oxide system networking route delete --rack 2d1fe615-0fd9-485d-8898-cd024883d222 --switch switch1 --port qsfp0 --destination 240.0.0.0/4 --nexthop 169.254.30.1 no routes match the provided args

ahl · 2024-08-28T15:42:39Z

cli/src/cmd_net.rs

+                eprintln_nopipe!("no addresses match the provided args");
+                return Ok(());


rcgoodfellow force-pushed the bgp-auth branch 6 times, most recently from a5cba56 to 6fbbd55 Compare August 17, 2024 00:43

elaine-oxide reviewed Aug 19, 2024

View reviewed changes

This was referenced Aug 20, 2024

need to return better error message when specifying incorrect rack id oxidecomputer/omicron#6393

Open

oxide system hardware switch list returns empty list oxidecomputer/omicron#6394

Open

rcgoodfellow mentioned this pull request Aug 23, 2024

Adapt to LLDP-related changes in link APIs #808

Closed

askfongjojo mentioned this pull request Aug 24, 2024

Need a new release of the CLI binaries for v10 new features and fixes #791

Closed

6 tasks

rcgoodfellow force-pushed the bgp-auth branch from 15701e4 to fab734d Compare August 26, 2024 02:24

rcgoodfellow marked this pull request as ready for review August 26, 2024 02:25

add bgp auth and static route commands

50909be

rcgoodfellow force-pushed the bgp-auth branch from fab734d to 50909be Compare August 26, 2024 02:46

morlandi7 added this to the 10 milestone Aug 26, 2024

morlandi7 assigned rcgoodfellow Aug 26, 2024

elaine-oxide reviewed Aug 26, 2024

View reviewed changes

bgp peer add -> bgp peer set

cb024c9

rcgoodfellow added 3 commits August 26, 2024 17:12

cleanup

5eece01

cleanup

6e6c2fa

cleanup

67c91d3

elaine-oxide reviewed Aug 26, 2024

View reviewed changes

pr feedback

b53f239

rcgoodfellow force-pushed the bgp-auth branch from b016fbe to b53f239 Compare August 27, 2024 00:07

elaine-oxide reviewed Aug 27, 2024

View reviewed changes

cli/src/cmd_net.rs Outdated Show resolved Hide resolved

cli/src/cmd_net.rs Outdated Show resolved Hide resolved

pr feedback

07dd4f0

elaine-oxide reviewed Aug 27, 2024

View reviewed changes

rcgoodfellow added 2 commits August 26, 2024 21:47

more doc comments

3334341

fix route set

8e59e5c

elaine-oxide mentioned this pull request Aug 27, 2024

Administrative removal of peers does not remove prefixes in RIB imported by that peer oxidecomputer/maghemite#349

Closed

elaine-oxide reviewed Aug 27, 2024

View reviewed changes

cli/src/cmd_net.rs Outdated Show resolved Hide resolved

cli/src/cmd_net.rs Outdated Show resolved Hide resolved

cli/src/cmd_net.rs Outdated Show resolved Hide resolved

pr feedback

98f000b

elaine-oxide approved these changes Aug 27, 2024

View reviewed changes

rcgoodfellow requested a review from ahl August 27, 2024 19:50

ahl approved these changes Aug 28, 2024

View reviewed changes

ahl merged commit 82035eb into main Aug 28, 2024

ahl deleted the bgp-auth branch August 28, 2024 15:46

		eprintln_nopipe!("no originated prefixes match the provided args");
		return Ok(());

		eprintln_nopipe!("no routes match the provided args");
		return Ok(());

		eprintln_nopipe!("no addresses match the provided args");
		return Ok(());

Commands for managing bgp auth and route local preference #795

Commands for managing bgp auth and route local preference #795

Uh oh!

Conversation

rcgoodfellow commented Aug 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elaine-oxide left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

elaine-oxide commented Aug 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elaine-oxide commented Aug 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elaine-oxide commented Aug 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elaine-oxide commented Aug 22, 2024

Uh oh!

rcgoodfellow commented Aug 22, 2024

Uh oh!

rcgoodfellow commented Aug 26, 2024

Uh oh!

rcgoodfellow commented Aug 26, 2024

Uh oh!

rcgoodfellow commented Aug 26, 2024

Uh oh!

rcgoodfellow commented Aug 26, 2024

Uh oh!

rcgoodfellow commented Aug 26, 2024

Uh oh!

elaine-oxide commented Aug 26, 2024

Uh oh!

elaine-oxide left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rcgoodfellow commented Aug 26, 2024

Uh oh!

elaine-oxide left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elaine-oxide commented Aug 27, 2024

Uh oh!

elaine-oxide left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

elaine-oxide left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

rcgoodfellow commented Aug 16, 2024 •

edited

Loading

elaine-oxide commented Aug 20, 2024 •

edited

Loading

elaine-oxide commented Aug 20, 2024 •

edited

Loading

elaine-oxide commented Aug 22, 2024 •

edited

Loading