On the fly bgp peer add/remove: through SetControlState protocol/bgp

[rudranil-das]

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
• instead of starting all protocols, bring up/down bgp peers selectively during start protocol
• If the desired state of a set of BGP peers is 'up', underlying IP interface(s) would be brought up automatically (if not already up) and the associated BGP peers would start advertising routes. If the desired state of a set of BGP peers is 'down', the associated BGP peers would stop advertising routes.

Example:

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

	// add port
	p1 := config.Ports().Add().SetName("p1").SetLocation("eth1")

	// add device
	d1 := config.Devices().Add().SetName("d1")

	// add Ethernet
	d1Eth1 := d1.Ethernets().
		Add().
		SetName("d1Eth").
		SetMac("00:00:01:01:01:01").
		SetMtu(1500)

	d1Eth1.Connection().SetChoice("port_name").SetPortName(p1.Name())

	// add IPv4
	d1Eth1.
		Ipv4Addresses().
		Add().
		SetName("p1d1ipv4").
		SetAddress("1.1.1.1").
		SetGateway("1.1.1.2").
		SetPrefix(24)

	d1Bgp := d1.Bgp().SetRouterId("1.1.1.1")
	d1BgpIpv4Interface := d1Bgp.Ipv4Interfaces().Add().SetIpv4Name("p1d1ipv4")

	// Add 6 bgp peers
	for i := 1; i <= 6; i++ {
		d1BgpIpv4Interface.Peers().Add().
			SetName(fmt.Sprintf("Peer%d", i)).
			SetPeerAddress("1.1.1.2")

		// other bgp peer and route configurations...
	}

	// set config
	api.SetConfig(config)

	// Start protocol with 1st peer
	start := api.NewControlState()
	start.Protocol().Bgp().Peers().
		SetPeerNames([]string{"Peer1"}).
		SetState(gosnappi.StateProtocolBgpPeersState.UP)

	api.SetControlState(start)

	// Peer-names for last 5 peers (peer names are globally unique)
	peerNames := []string{}
	for i := 2; i <= 6; i++ {
		peerNames = append(peerNames, fmt.Sprintf("Peer%d", i))
	}

	// Start last 5 peers
	add5 := api.NewControlState()
	add5.Protocol().Bgp().Peers().
		SetPeerNames(peerNames).
		SetState(gosnappi.StateProtocolBgpPeersState.UP)

	api.SetControlState(add5)

	// Stop last 5 peers
	remove5 := api.NewControlState()
	remove5.Protocol().Bgp().Peers().
		SetPeerNames(peerNames).
		SetState(gosnappi.StateProtocolBgpPeersState.DOWN)

	api.SetControlState(remove5)

Comments:

• it is a clean approach in line with current OTG model workflow of managing control plane attributes post set_config.
• item in peer_names can uniquely identify a bgp peer across config
  • as property name for Bgp.V4Peer/Bgp.V6Peer is globally unique
• current workflow/design of set_control_state allows atomic changes; hence if any future use-case requires,
  • more than one attribute change in same config-node (e.g. devices/bgp) or in any child of it,
    • further addition in set_control_state would be needed (e.g. under set_control_state/protocol/bgp) for those attributes
  • more than one attribute change across config-nodes (e.g. devices/bgp and devices/isis)
    • further addition in set_control_state would be needed (e.g. under set_control_state/protocol/bgp and/or set_control_state/protocol/isis)
• 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-controlstate

[apratimmukherjee]

Looks good with minor documentation comments.

[jasdeep-hundal]

The example shows starting the BGP protocols w/o any reference to the underlying IP interface. How would we ensure that the IP interface was ip if we're not using the usual StartProtocols method first?

[rudranil-das]

The example shows starting the BGP protocols w/o any reference to the underlying IP interface. How would we ensure that the IP interface was ip if we're not using the usual StartProtocols method first?

@jasdeep-hundal, I have updated the config in the example to include underlying constructs e.g. IP and Ethernet to avoid ambiguity. To answer your query, underlying associated constructs below bgp peer e.g. IP interface should be brought up/down implicitly when SetControlState "up/down" is triggered on a set of bgp peers. I have updated the same in proposal section of PR.

Also, we identified a scenario where the configuration of same use-case (as mentioned in FP scenario mentioned in "use case" section of PR) could be accompanied by other protocols e.g. LACP/ISIS (common in BGP configurations) for which everything needs to be started. User would face difficulties to bring up these protocols effectively because,

• StartProtocols (which is SetControlState ) would start all protocols including all BGP peers whom we want to start selectively
• there is no option to start LACP member-ports or ISIS routers selectively like BGP

Hence, I added the support of starting LACP member-ports and ISIS routers in model in this PR itself as I reckon this might be needed in near future if we want to support mixed protocol scenario as mentioned above. For such cases, user would need to bring up/down protocols bottom up based on configured protocol stacking using same SetControlState api on LACP/ISIS.

Regarding implementation in KENG echo-system, I believe following should be the priority order,

• support of BGP (the original FP use-case)
• and then support of LACP and ISIS in phased manner (to support BGP with mixed protocol use-case)
• any other CP protocol for which selective start/stop is needed (in association of BGP neighbourhood use-case or as a separate protocol support) can come later in model and implementation

[apratimmukherjee]

Need to add isis to choice

[jasdeep-hundal]

• Please update the docs to reflect that the IP protocols do automatically come up when starting BGP peers.
• I don't know if we want to bring down IP protocols also when bringing down BGP peers. (Eg. if a user calls StartProtocols/SetControlState for 'all' initially, they may not intend to bring down the IP interface when bringing down a BGP peer.)

[rudranil-das]

Need to add isis to choice

corrected typo, done.

[rudranil-das]

• Please update the docs to reflect that the IP protocols do automatically come up when starting BGP peers.
• I don't know if we want to bring down IP protocols also when bringing down BGP peers. (Eg. if a user calls StartProtocols/SetControlState for 'all' initially, they may not intend to bring down the IP interface when bringing down a BGP peer.)

@jasdeep-hundal, thanks for the input, hence

• If the desired state is 'up', underlying IP interface(s) would be brought up automatically (if not already up), would attempt to bring up the BGP session(s) and advertise route(s), if configured.
• If the desired state is 'down', BGP session(s) would be brought down.

I have updated above in the documentation, as description of state attribute in the api.

[apratimmukherjee]

Minor improvements suggested in documentation.