search

MERAprojects / ops-quagga

GNU General Public License v2.0
0 stars 0 forks source link

"clear bgp * soft in/out" commands changes (fix for quagga#8#9 build#5) #42

Open ekgrish opened 7 years ago

ekgrish commented 7 years ago

Change-Id: If4d20927004dd2fa10778acc58fb1387e85c32cb

ekgrish commented 7 years ago

Useful links: RFC 2918 "Route Refresh Capability for BGP-4" https://tools.ietf.org/html/rfc2918 Applying routing policies (Ru) http://xgu.ru/wiki/%D0%9F%D1%80%D0%B8%D0%BC%D0%B5%D0%BD%D0%B5%D0%BD%D0%B8%D0%B5_%D0%B8%D0%B7%D0%BC%D0%B5%D0%BD%D0%B5%D0%BD%D0%B8%D0%B9_%D0%B2_%D0%BD%D0%B0%D1%81%D1%82%D1%80%D0%BE%D0%B9%D0%BA%D0%B0%D1%85_%D0%BF%D0%BE%D0%BB%D0%B8%D1%82%D0%B8%D0%BA_BGP BGP Soft Reset Enhancement http://www.cisco.com/en/US/products/ps6599/products_data_sheet09186a0080087b3a.html Route Refresh(Dynamic Inbound Soft Reset) vs Soft Reconfiguration Inbound https://supportforums.cisco.com/discussion/11379456/clear-ip-bgp-soft-command

ekgrish commented 7 years ago

Commands which have been affected in this solution: clear bgp soft out clear bgp soft in clear bgp soft clear bgp out clear bgp * in

artfom commented 7 years ago

Please, also check function "bgp_update_main()" in "bgp_route.c". It looks like we don't save BGP data for the soft reconfiguration feature in "bgpd" process now, but do we save it anywhere else?

ekgrish commented 7 years ago

@artfom

The current pull request fixes functionality related to Route Refresh feature more. Route Refresh feature is default enabled and there is no implemented CLI-command to disable it (message "unimplemented command"). Route Refresh capability allows updating route info without session reset, but it is different with Soft-Reconfiguration feature (+ saving received routes). This pull request fixes only issue, which is descripted in build#5 (session reset).

In fact, there are some problems related to Soft-Reconfiguration issue (preliminary info, require investigation):

  1. Saving BGP data for Soft-Reconfiguration feature in Quagga structures only (but not in ovsdb).
  2. Impossibility disable Route Refresh feature.
  3. Absence of implementation of "show received-routes", which must include implementation of browsing saved routes.
ekgrish commented 7 years ago

@mfwlarionov

In my opinion, these problems can be related to fact that peers in BGP deamon are created for working with AFI_IP address family. Then BGPd handlers related with change OVSDB values for some neighbor's parameters ("route map", "prefix list", "filter list") may require AFI_IP6. Perhaps this requires a deeper analysis.

Can I say, that it is a type of philosophical statement? Do you have an more precise proposal? In my opinion: Yes, we have an "big problem": "BGP deamon are created for working with AFI_IP address family" and sometimes with AFI_IP6, because sometimes IP6 work not so good as we want.

In my solution we:

  1. send changes according to afi.
  2. if we use command "clear soft", then we really don't reset session.

As it worked earlier: For ipv4:

  1. send of changes related to (aren't configured) ipv6. doesn't work at all
  2. perform reset of session, to send changes, because the previous point doesn't work. doesn't work as "soft", but changes will be sent

For ipv6: The same solution as for ipv4 doesn't work at all: changes aren't sent, reset of session. Why? because we use peer_deactivate(..., AFI_IP6...) for performing of reset and then remove all the configured parameters for pointed afi(always IP6) inside of this function.

artfom commented 7 years ago

@ekgrish

OK. Just, as I figured out, during other problem investigation, we have a possible bug in the code related to the Soft Reconfiguration feature. In the named previously function there is the following piece of the code:

/ When peer's soft reconfiguration enabled. Record input packet in Adj-RIBs-In. / if (! soft_reconfig && CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG) && peer != bgp->peer_self) bgp_adj_in_set (rn, peer, attr);

If the "soft_reconfig" variable should be greater than 0 in case of enabled feature - and it does looks so - the "if" statement will be never executed, and the route information will not be saved in the cache.

mfwlarionov commented 7 years ago

My remark is connected with related BGPd problems. It meant that peer is always created with AFI_IP address family. I did not mean, it was happening due to this commit.