Samuel Lucas Vaz de Mello <[EMAIL PROTECTED]> wrote:
> > If I remember correctly, I used bsr_stop()/bsr_start() because there
> > was lots of complexity when I tried to do the incremental update.
> > Though, I don't remember whether this applied to the first version
> > of the BSR implementation or to the (almost complete) rewrite I had
> > to do after I discovered major issues with the first version.
>
> The code in CVS is the rewritten code?
Yes.
> > Another reason for the bsr_stop()/bsr_start() was because I wanted
> > atomic update (e.g., transaction like), otherwise during
> > reconfiguration there will be lots of flux with potentially
> > dangerous results (remember that consistent Cand-RP set across
> > all PIM-SM routers is critical).
> > At that time there was no easy way to apply some transaction-based
> > mechanism to achieve the atomic update, so the
> > bsr_stop()/bsr_start() was the simple work-around (e.g., note that
> > it is in the etc/templates/pimsm*.tp template files).
> >
> > If you can find a simple way to do the update atomically without
> > bsr_stop()/bsr_start(), then give it a try and see how well it
> > works.
> > Otherwise, please submit a Bugzilla entry about the issue.
>
> I played around a bit with this.
>
> I've tried to keep as close as possible to the stop()/start() approach.
> My idea was:
>
> - Add a boolean parameter to bsr_start()/bsr_stop that is true during restarts
> - Create a bsr_restart() method that calls bsr_stop(true) and then
> bsr_start(true)
> - In stop(true), instead deleting the whole _active_bsr_zone_list delete only
> the non-elected zones. For the elected zones, delete all groupprefix (which
> contains the rps).
> - In start(true) it will reuse the existing elected zone and add all
> configured groupprefix (and rps)
> - In start(true), check if any elected zone was deleted from config and
> vanish them.
> - After that, I put the elected zones back in the PENDING state and expire
> the bsr timer. The timer will put it back in the ELECTED state, compute the
> rp-set and send BSR Messages with the new rp-set.
>
> In my tests it seems to work fine.
> I'm sending the patch attached.
>
> Can you have a look on it and check if I've forgotten something?
Samuel,
I took your patch and refactored it a bit: now there is an explicit
XRL apply_bsr_changes, and there is an explicit method
PimBsr::apply_bsr_changes().
In that method I basically merged your code from stop_bsr() and
start_bsr().
Thus, we don't overload the original stop_bsr() and start_bsr(),
and we can do all that is needed in one place.
However, I have to admit that I didn't look into the details of what
you are doing when the BSR is restarted, so I cannot comment whether
it is the right thing or whether something is missing.
I intend to do this in the future, but unfortunately I cannot do
this right now (see below).
Currently we are in the process of code freeze, so I am afraid
this fix has to wait (strictly speaking it is an
optimization so it is not critical).
In the mean time please have a look in my refactoring of your patch
in case I messed-up something.. If it is OK for you and if it works,
please create a Bugzilla entry about the issue, and add that patch
to the entry. Thus we can track its status, update the patch, etc.
Thanks,
Pavlin
Index: etc/templates/pimsm4.tp
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/etc/templates/pimsm4.tp,v
retrieving revision 1.35
diff -u -p -r1.35 pimsm4.tp
--- etc/templates/pimsm4.tp 2 May 2008 00:00:53 -0000 1.35
+++ etc/templates/pimsm4.tp 29 Oct 2008 23:47:33 -0000
@@ -209,8 +209,7 @@ protocols {
%help: short "Configure the IPv4 Bootstrap mechanism";
%activate: xrl "$(pimsm4.targetname)/pim/0.1/stop_bsr";
%activate: xrl "$(pimsm4.targetname)/pim/0.1/start_bsr";
- %update: xrl "$(pimsm4.targetname)/pim/0.1/stop_bsr";
- %update: xrl "$(pimsm4.targetname)/pim/0.1/start_bsr";
+ %update: xrl "$(pimsm4.targetname)/pim/0.1/apply_bsr_changes";
%delete: xrl "$(pimsm4.targetname)/pim/0.1/stop_bsr";
disable {
Index: etc/templates/pimsm6.tp
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/etc/templates/pimsm6.tp,v
retrieving revision 1.35
diff -u -p -r1.35 pimsm6.tp
--- etc/templates/pimsm6.tp 2 May 2008 00:00:53 -0000 1.35
+++ etc/templates/pimsm6.tp 29 Oct 2008 23:47:33 -0000
@@ -209,8 +209,7 @@ protocols {
%help: short "Configure the IPv6 Bootstrap mechanism";
%activate: xrl "$(pimsm6.targetname)/pim/0.1/stop_bsr";
%activate: xrl "$(pimsm6.targetname)/pim/0.1/start_bsr";
- %update: xrl "$(pimsm6.targetname)/pim/0.1/stop_bsr";
- %update: xrl "$(pimsm6.targetname)/pim/0.1/start_bsr";
+ %update: xrl "$(pimsm6.targetname)/pim/0.1/apply_bsr_changes";
%delete: xrl "$(pimsm6.targetname)/pim/0.1/stop_bsr";
disable {
Index: pim/pim_bsr.cc
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/pim/pim_bsr.cc,v
retrieving revision 1.57
diff -u -p -r1.57 pim_bsr.cc
--- pim/pim_bsr.cc 28 Oct 2008 00:21:18 -0000 1.57
+++ pim/pim_bsr.cc 29 Oct 2008 23:47:35 -0000
@@ -316,6 +316,123 @@ PimBsr::disable()
XLOG_INFO("Bootstrap mechanism disabled");
}
+int
+PimBsr::apply_bsr_changes(string& error_msg)
+{
+ list<BsrZone *>::iterator iter;
+ list<BsrZone*> del_list;
+
+ if (! is_enabled())
+ return (XORP_OK);
+
+ //
+ // Preserve any elected BSR zones
+ //
+ for (list<BsrZone*>::iterator iter = _active_bsr_zone_list.begin();
+ iter != _active_bsr_zone_list.end();
+ iter++) {
+ BsrZone* tmp_zone = *iter;
+ // If the BSR zone is not elected BSR, just remove it
+ if (tmp_zone->bsr_zone_state() != BsrZone::STATE_ELECTED_BSR) {
+ del_list.push_back(tmp_zone);
+ } else {
+ //
+ // It is an Elected BSR. We will keep the BSR zone in the list
+ // but remove all Cand RPs. Some of them may have been deleted.
+ // The Cand RPs that were not deleted will be added again below.
+ //
+ delete_pointers_list(tmp_zone->bsr_group_prefix_list());
+ }
+ }
+ for (list<BsrZone*>::iterator iter = del_list.begin();
+ iter != del_list.end(); ++iter) {
+ _active_bsr_zone_list.remove(*iter);
+ }
+ delete_pointers_list(del_list);
+
+ //
+ // Clean up any deleted zone
+ //
+ for (iter = _active_bsr_zone_list.begin();
+ iter != _active_bsr_zone_list.end();
+ ++iter) {
+ BsrZone *active_bsr_zone = *iter;
+ if (active_bsr_zone->bsr_zone_state() == BsrZone::STATE_ELECTED_BSR) {
+ BsrZone *config_bsr_zone;
+ config_bsr_zone = find_config_bsr_zone(active_bsr_zone->zone_id());
+ if (config_bsr_zone == NULL) {
+ // Zone removed from config
+ del_list.push_back(active_bsr_zone);
+ continue;
+ }
+
+ //
+ // Zone is no longer BSR candidate (i.e., removed from Cand-BSR but
+ // still has some Cand-RP state).
+ //
+ // Remove it too, as the Cand-RP state is re-added below
+ if (! config_bsr_zone->i_am_candidate_bsr()) {
+ del_list.push_back(active_bsr_zone);
+ continue;
+ }
+ }
+ }
+
+ //
+ // Delete the vanished items
+ //
+ for (iter = del_list.begin(); iter != del_list.end(); ++iter) {
+ BsrZone *active_bsr_zone = *iter;
+ _active_bsr_zone_list.remove(active_bsr_zone);
+ }
+ delete_pointers_list(del_list);
+
+ //
+ // Activate all configured BSR zones
+ //
+ for (iter = _config_bsr_zone_list.begin();
+ iter != _config_bsr_zone_list.end();
+ ++iter) {
+ BsrZone *config_bsr_zone = *iter;
+
+ if (config_bsr_zone->i_am_candidate_bsr()) {
+ if (add_active_bsr_zone(*config_bsr_zone, error_msg) == NULL) {
+ XLOG_ERROR("Cannot add configured Bootstrap zone %s: %s",
+ cstring(config_bsr_zone->zone_id()),
+ error_msg.c_str());
+ stop();
+ return (XORP_ERROR);
+ }
+ }
+ config_bsr_zone->start_candidate_rp_advertise_timer();
+ }
+
+ //
+ // When restarting we want to keep any previously enabled BSR.
+ // Set it to PENDING and expire the bsr_timer immediately.
+ // As soon as the timer runs, it puts the zone back in the ELECTED
+ // state and run the needed actions (calculate RP-Set and send a
+ // BSR Message with the new RP-Set).
+ //
+ for (iter = _active_bsr_zone_list.begin();
+ iter != _active_bsr_zone_list.end();
+ ++iter) {
+ BsrZone *active_bsr_zone = *iter;
+ if (active_bsr_zone->bsr_zone_state() == BsrZone::STATE_ELECTED_BSR) {
+ if (active_bsr_zone->i_am_candidate_bsr()) {
+ // zone was not changed
+ active_bsr_zone->set_bsr_zone_state(BsrZone::STATE_PENDING_BSR);
+ } else {
+ // zone is no longer Cand-BSR, but contains Cand-RPs
+ active_bsr_zone->set_bsr_zone_state(BsrZone::STATE_ACCEPT_ANY);
+ }
+ active_bsr_zone->expire_bsr_timer();
+ }
+ }
+
+ return (XORP_OK);
+}
+
// Unicast the Bootstrap message(s) to a (new) neighbor
int
PimBsr::unicast_pim_bootstrap(PimVif *pim_vif, const IPvX& nbr_addr) const
Index: pim/pim_bsr.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/pim/pim_bsr.hh,v
retrieving revision 1.24
diff -u -p -r1.24 pim_bsr.hh
--- pim/pim_bsr.hh 28 Oct 2008 00:21:18 -0000 1.24
+++ pim/pim_bsr.hh 29 Oct 2008 23:47:35 -0000
@@ -93,6 +93,14 @@ public:
*/
void disable();
+ /**
+ * Apply BSR configuration changes.
+ *
+ * @param error_msg the error message (if error).
+ * @return XORP_OK on success, otherwise XORP_ERROR.
+ */
+ int apply_bsr_changes(string& error_msg);
+
PimNode& pim_node() const { return (_pim_node); }
int unicast_pim_bootstrap(PimVif *pim_vif,
Index: pim/pim_node.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/pim/pim_node.hh,v
retrieving revision 1.70
diff -u -p -r1.70 pim_node.hh
--- pim/pim_node.hh 2 Oct 2008 21:57:54 -0000 1.70
+++ pim/pim_node.hh 29 Oct 2008 23:47:35 -0000
@@ -872,7 +872,17 @@ public:
* @return XORP_OK on success, otherwise XORP_ERROR.
*/
int stop_bsr() { return (pim_bsr().stop()); }
-
+
+ /**
+ * Apply BSR configuration changes.
+ *
+ * @param error_msg the error message (if error).
+ * @return XORP_OK on success, otherwise XORP_ERROR.
+ */
+ int apply_bsr_changes(string& error_msg) {
+ return (pim_bsr().apply_bsr_changes(error_msg));
+ }
+
//
// Configuration methods
//
Index: pim/xrl_pim_node.cc
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/pim/xrl_pim_node.cc,v
retrieving revision 1.105
diff -u -p -r1.105 xrl_pim_node.cc
--- pim/xrl_pim_node.cc 2 Oct 2008 21:57:56 -0000 1.105
+++ pim/xrl_pim_node.cc 29 Oct 2008 23:47:36 -0000
@@ -3930,6 +3930,17 @@ XrlPimNode::pim_0_1_stop_bsr()
return XrlCmdError::OKAY();
}
+XrlCmdError
+XrlPimNode::pim_0_1_apply_bsr_changes()
+{
+ string error_msg;
+
+ if (PimNode::apply_bsr_changes(error_msg) != XORP_OK)
+ return XrlCmdError::COMMAND_FAILED(error_msg);
+
+ return XrlCmdError::OKAY();
+}
+
//
// PIM configuration
//
Index: pim/xrl_pim_node.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/pim/xrl_pim_node.hh,v
retrieving revision 1.80
diff -u -p -r1.80 xrl_pim_node.hh
--- pim/xrl_pim_node.hh 2 Oct 2008 21:57:56 -0000 1.80
+++ pim/xrl_pim_node.hh 29 Oct 2008 23:47:36 -0000
@@ -709,6 +709,11 @@ protected:
XrlCmdError pim_0_1_stop_bsr();
/**
+ * Apply BSR configuration changes.
+ */
+ XrlCmdError pim_0_1_apply_bsr_changes();
+
+ /**
* Add/delete scope zone.
*
* @param scope_zone_id the ID of the configured zone.
Index: xrl/interfaces/pim.xif
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/interfaces/pim.xif,v
retrieving revision 1.25
diff -u -p -r1.25 pim.xif
--- xrl/interfaces/pim.xif 2 May 2008 00:00:54 -0000 1.25
+++ xrl/interfaces/pim.xif 29 Oct 2008 23:47:38 -0000
@@ -59,6 +59,11 @@ interface pim/0.1 {
stop_bsr
/**
+ * Apply BSR configuration changes.
+ */
+ apply_bsr_changes
+
+ /**
* Add/delete scope zone.
*
* @param scope_zone_id the ID of the configured zone.
Index: xrl/interfaces/pim_xif.cc
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/interfaces/pim_xif.cc,v
retrieving revision 1.45
diff -u -p -r1.45 pim_xif.cc
--- xrl/interfaces/pim_xif.cc 2 Oct 2008 21:58:37 -0000 1.45
+++ xrl/interfaces/pim_xif.cc 29 Oct 2008 23:47:39 -0000
@@ -607,6 +607,44 @@ XrlPimV0p1Client::unmarshall_stop_bsr(
}
bool
+XrlPimV0p1Client::send_apply_bsr_changes(
+ const char* dst_xrl_target_name,
+ const ApplyBsrChangesCB& cb
+)
+{
+ static Xrl* x = NULL;
+
+ if (!x) {
+ x = new Xrl(dst_xrl_target_name, "pim/0.1/apply_bsr_changes");
+ }
+
+ x->set_target(dst_xrl_target_name);
+
+
+ return _sender->send(*x, callback(this,
&XrlPimV0p1Client::unmarshall_apply_bsr_changes, cb));
+}
+
+
+/* Unmarshall apply_bsr_changes */
+void
+XrlPimV0p1Client::unmarshall_apply_bsr_changes(
+ const XrlError& e,
+ XrlArgs* a,
+ ApplyBsrChangesCB cb
+)
+{
+ if (e != XrlError::OKAY()) {
+ cb->dispatch(e);
+ return;
+ } else if (a && a->size() != 0) {
+ XLOG_ERROR("Wrong number of arguments (%u != %u)",
XORP_UINT_CAST(a->size()), XORP_UINT_CAST(0));
+ cb->dispatch(XrlError::BAD_ARGS());
+ return;
+ }
+ cb->dispatch(e);
+}
+
+bool
XrlPimV0p1Client::send_add_config_scope_zone_by_vif_name4(
const char* dst_xrl_target_name,
const IPv4Net& scope_zone_id,
Index: xrl/interfaces/pim_xif.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/interfaces/pim_xif.hh,v
retrieving revision 1.43
diff -u -p -r1.43 pim_xif.hh
--- xrl/interfaces/pim_xif.hh 2 Oct 2008 21:58:37 -0000 1.43
+++ xrl/interfaces/pim_xif.hh 29 Oct 2008 23:47:39 -0000
@@ -186,6 +186,19 @@ public:
const StopBsrCB& cb
);
+ typedef XorpCallback1<void, const XrlError&>::RefPtr ApplyBsrChangesCB;
+ /**
+ * Send Xrl intended to:
+ *
+ * Apply BSR configuration changes.
+ *
+ * @param dst_xrl_target_name the Xrl target name of the destination.
+ */
+ bool send_apply_bsr_changes(
+ const char* dst_xrl_target_name,
+ const ApplyBsrChangesCB& cb
+ );
+
typedef XorpCallback1<void, const XrlError&>::RefPtr
AddConfigScopeZoneByVifName4CB;
/**
* Send Xrl intended to:
@@ -2205,6 +2218,12 @@ private:
StopBsrCB cb
);
+ void unmarshall_apply_bsr_changes(
+ const XrlError& e,
+ XrlArgs* a,
+ ApplyBsrChangesCB cb
+ );
+
void unmarshall_add_config_scope_zone_by_vif_name4(
const XrlError& e,
XrlArgs* a,
Index: xrl/targets/pim.xrls
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/targets/pim.xrls,v
retrieving revision 1.62
diff -u -p -r1.62 pim.xrls
--- xrl/targets/pim.xrls 2 Oct 2008 21:58:48 -0000 1.62
+++ xrl/targets/pim.xrls 29 Oct 2008 23:47:39 -0000
@@ -396,6 +396,11 @@ finder://pim/pim/0.1/start_bsr
finder://pim/pim/0.1/stop_bsr
/**
+ * Apply BSR configuration changes.
+ */
+finder://pim/pim/0.1/apply_bsr_changes
+
+/**
* Add/delete scope zone.
*
* @param scope_zone_id the ID of the configured zone.
Index: xrl/targets/pim_base.cc
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/targets/pim_base.cc,v
retrieving revision 1.67
diff -u -p -r1.67 pim_base.cc
--- xrl/targets/pim_base.cc 2 Oct 2008 21:58:48 -0000 1.67
+++ xrl/targets/pim_base.cc 29 Oct 2008 23:47:40 -0000
@@ -1343,6 +1343,30 @@ XrlPimTargetBase::handle_pim_0_1_stop_bs
}
const XrlCmdError
+XrlPimTargetBase::handle_pim_0_1_apply_bsr_changes(const XrlArgs& xa_inputs,
XrlArgs* /* pxa_outputs */)
+{
+ if (xa_inputs.size() != 0) {
+ XLOG_ERROR("Wrong number of arguments (%u != %u) handling %s",
+ XORP_UINT_CAST(0), XORP_UINT_CAST(xa_inputs.size()),
"pim/0.1/apply_bsr_changes");
+ return XrlCmdError::BAD_ARGS();
+ }
+
+ /* Return value declarations */
+ try {
+ XrlCmdError e = pim_0_1_apply_bsr_changes();
+ if (e != XrlCmdError::OKAY()) {
+ XLOG_WARNING("Handling method for %s failed: %s",
+ "pim/0.1/apply_bsr_changes", e.str().c_str());
+ return e;
+ }
+ } catch (const XrlArgs::BadArgs& e) {
+ XLOG_ERROR("Error decoding the arguments: %s", e.str().c_str());
+ return XrlCmdError::BAD_ARGS(e.str());
+ }
+ return XrlCmdError::OKAY();
+}
+
+const XrlCmdError
XrlPimTargetBase::handle_pim_0_1_add_config_scope_zone_by_vif_name4(const
XrlArgs& xa_inputs, XrlArgs* /* pxa_outputs */)
{
if (xa_inputs.size() != 2) {
@@ -8766,6 +8790,10 @@ XrlPimTargetBase::add_handlers()
callback(this, &XrlPimTargetBase::handle_pim_0_1_stop_bsr)) ==
false) {
XLOG_ERROR("Failed to xrl handler finder://%s/%s", "pim",
"pim/0.1/stop_bsr");
}
+ if (_cmds->add_handler("pim/0.1/apply_bsr_changes",
+ callback(this,
&XrlPimTargetBase::handle_pim_0_1_apply_bsr_changes)) == false) {
+ XLOG_ERROR("Failed to xrl handler finder://%s/%s", "pim",
"pim/0.1/apply_bsr_changes");
+ }
if (_cmds->add_handler("pim/0.1/add_config_scope_zone_by_vif_name4",
callback(this,
&XrlPimTargetBase::handle_pim_0_1_add_config_scope_zone_by_vif_name4)) ==
false) {
XLOG_ERROR("Failed to xrl handler finder://%s/%s", "pim",
"pim/0.1/add_config_scope_zone_by_vif_name4");
@@ -9644,6 +9672,7 @@ XrlPimTargetBase::remove_handlers()
_cmds->remove_handler("pim/0.1/enable_bsr");
_cmds->remove_handler("pim/0.1/start_bsr");
_cmds->remove_handler("pim/0.1/stop_bsr");
+ _cmds->remove_handler("pim/0.1/apply_bsr_changes");
_cmds->remove_handler("pim/0.1/add_config_scope_zone_by_vif_name4");
_cmds->remove_handler("pim/0.1/add_config_scope_zone_by_vif_name6");
_cmds->remove_handler("pim/0.1/add_config_scope_zone_by_vif_addr4");
Index: xrl/targets/pim_base.hh
===================================================================
RCS file: /usr/local/www/data/cvs/xorp/xrl/targets/pim_base.hh,v
retrieving revision 1.71
diff -u -p -r1.71 pim_base.hh
--- xrl/targets/pim_base.hh 2 Oct 2008 21:58:48 -0000 1.71
+++ xrl/targets/pim_base.hh 29 Oct 2008 23:47:40 -0000
@@ -705,6 +705,13 @@ protected:
/**
* Pure-virtual function that needs to be implemented to:
*
+ * Apply BSR configuration changes.
+ */
+ virtual XrlCmdError pim_0_1_apply_bsr_changes() = 0;
+
+ /**
+ * Pure-virtual function that needs to be implemented to:
+ *
* Add/delete scope zone.
*
* @param scope_zone_id the ID of the configured zone.
@@ -2174,6 +2181,8 @@ private:
const XrlCmdError handle_pim_0_1_stop_bsr(const XrlArgs& in, XrlArgs* out);
+ const XrlCmdError handle_pim_0_1_apply_bsr_changes(const XrlArgs& in,
XrlArgs* out);
+
const XrlCmdError handle_pim_0_1_add_config_scope_zone_by_vif_name4(const
XrlArgs& in, XrlArgs* out);
const XrlCmdError handle_pim_0_1_add_config_scope_zone_by_vif_name6(const
XrlArgs& in, XrlArgs* out);
_______________________________________________
Xorp-hackers mailing list
[email protected]
http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-hackers
