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

On the fly bgp peer add/remove: through UpdateConfig devices/bgp #338

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rudranil-das
Copy link
Contributor

@rudranil-das rudranil-das commented Oct 17, 2023

Check documentation: here

use-case

mentioned here

  • configure one bgp peer
  • start protocols (expectation: 1st bgp peer will be up)
  • add 5 more bgp peers (expectation: all 6 bgp peers will be up)
  • remove 5 newly added bgp peers (expectation: only 1st bgp peer will be up)

proposal

  • configure all bgp peers in the original config with 1st peer enabled, rest 5 peers disabled using new peer_enabled flag.
  • start protocols (only 1st peer will come up)
  • update config with rest 5 peers enabled (all 6 peers will come up)
  • update config with rest 5 peers disabled again (only 1st bgp peer remains up, rest 5 goes down)

Example:

        api := gosnappi.NewApi()
	config := gosnappi.NewConfig()

	d1 := config.Devices().Add().SetName("d1")

	// bgp router, interface(s), peers
	d1Bgp := d1.Bgp()
	d1BgpIpv4Interface := d1Bgp.Ipv4Interfaces().Add()
	peers := d1BgpIpv4Interface.Peers()

	// Add 6 bgp peers (enabled by default) - disable the last 5
	peers.Add().SetName("Peer1")
	for i := 2; i <= 6; i++ {
		peers.Add().SetName(fmt.Sprintf("Peer%d", i)).SetPeerEnabled(false)
	}

	// other bgp internal configurations...

	// set config
	api.SetConfig(config)

	// Start protocols - only 1st peer would be UP
	start := api.NewControlState()
	start.Protocol().All().SetState(gosnappi.StateProtocolAllState.START)
	api.SetControlState(start)

	// Enable the last 5 peers
	for idx, peer := range peers.Items() {
		if idx != 0 {
			peer.SetPeerEnabled(true)
		}
	}

	// config-slice to update, with
	// 1. properties to update
	// 2. config-nodes [Array of objects (Device.BgpRouter) in this case] to update
	// last 5 peer would be UP as well
	upd := api.NewConfigUpdate()
	upd.Devices().Bgp().
		SetPropertyNames([]gosnappi.UpdateDeviceBgpPropertyNamesEnum{gosnappi.UpdateDeviceBgpPropertyNames.PEER_ENABLED}).
		Bgp().Append(d1Bgp)

	api.UpdateConfig(upd)

	// Disable the last 5 peer
	for idx, peer := range peers.Items() {
		if idx != 0 {
			peer.SetPeerEnabled(false)
		}
	}

	// config-slice to update, with
	// 1. properties to update
	// 2. config-nodes [Array of objects (Device.BgpRouter) in this case] to update
	// last 5 peer would go DOWN
	upd = api.NewConfigUpdate()
	upd.Devices().Bgp().
		SetPropertyNames([]gosnappi.UpdateDeviceBgpPropertyNamesEnum{gosnappi.UpdateDeviceBgpPropertyNames.PEER_ENABLED}).
		Bgp().Append(d1Bgp)

	api.UpdateConfig(upd)

Comments:

  • this approach is proposed for future accmodation of use-case where more than one attribute change in same config-node (e.g. devices/bgp) or in any child of it might be required - those attributes could be passed in property_names field of update_config/devices/bgp.
  • issues with this approach are,
    • attributes (to be changed on the fly) mentioned in property_names does not have a path associated. Hence consumer of update_config api (controller for ixia-c/keng solution), need to find out each of the mentioned attribute(s) in the provided config-node (e.g. devices/bgp) or in any nested child of same - it would result in a performance hit.
      (note: in similar scenario, looks like, gnmi updates the attributes with key-value pair of <absolute-path-to-attribute>:<new value> (reference here) but I believe it is not good for UX and hence similar methodology is avoided in OTG model altogether)
    • in reality, there are duplicate attribute-names at different levels of same config-node (e.g. almost all the attributes of Bgp.V4Peer and Bgp.V6Peer under devices/bgp have same attribute-names) - hence as there is no associated path with the attribute, it might result in some undesired updates.
    • this approach still does not address attribute changes spread across different config-nodes e.g. devices/bgp and devices/isis - which might still need multiple update_config api calls.
    • this approach might result in high volume of data-transfer for bulky config-nodes e.g. devices/bgp with 1M routes even though attributes to update might not be associated with routes and is present directly under devices/bgp or in some other child of it.
  • a preferred approach to solve the use-case is captured in this pr
  • note: to get hands on with script including this change please use gosnappi through go get github.com/open-traffic-generator/snappi/gosnappi@dev-fly-updatecfg

@rudranil-das rudranil-das marked this pull request as draft October 17, 2023 18:10
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