[quagga-dev 11552] Re: [PATCH 1/6] Buggy Unlock in ospfd/ospf_opaque.c

olivier.dugeon at orange.com olivier.dugeon at orange.com
Tue Oct 7 15:22:18 BST 2014


An old bug in ospfd/ospf_opaque.c blocks opaque LSA flooding when the 
Quagga router is restarted or when an interface goes down then up in 
less than 3600 seconds (i.e. LSA MAX AGE). In such situation, the 
neighbour router have acquired TE LSA from the Quagga router and keep 
them in its Link State Data Base (LSDB). When the Quagga router restart 
or interface goes up, the neighbour router send back the original TE 
LSA. Quagga detects these self generated opaque LSA and signal to its 
neighbour router with LS ACK + LSA age = MAX AGE that the LSA must not 
be kept during the LSDB synchronisation phase.

The problem comes from opaque_lsa.c code that blocks subsequent opaque 
LSA flooding until the neighbour router acknowledge that it removes the 
old opaque LSA from its LSDB. The bug comes from the fact that the lock 
is never release, thus avoiding subsequent opaque LSA flooding. The 
condition to unlock flooding is never raise: the original code count the 
number of self originate opaque LSA in LSDB and release the lock only if 
count equal 0. However, as the configuration file ask to flood Opaque 
LSA, there is already at least one opaque LSA in LSDB and count could 
never be equal to 0.The only solution is to restart again the ospfd daemon.

The correction skip counting the self originate opaque LSA in LSDB and 
unlock opaque LSA flooding once neighbour router acknowledge the removal 
of old opaque LSA in its LSDB.

As there is no suck mechanism for standard LSA i.e. type 1 to 7, I would 
propose to remove completely this mechanism. For the moment, I just 
comment it. Once acknowledge, I will submit a new patch.

I test the patch with Cisco and Juniper and don't observe any problem. I 
also made some Wireshark capture and seen nothing strange. I also 
compare Quagga behaviour with Cisco and Juniper behaviour in the same 
condition, and Wireshark capture are similar.

Signed-off-by: Olivier Dugeon <olivier.dugeon at orange.com> 
<mailto:olivier.dugeon at orange.com>
--
  ospf_opaque.c |   20 +++++++++++++-------
  1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c
index 744952c..d9c96db 100644
--- a/ospfd/ospf_opaque.c
+++ b/ospfd/ospf_opaque.c
@@ -2126,10 +2156,12 @@ out:
*------------------------------------------------------------------------*/

  static void ospf_opaque_exclude_lsa_from_lsreq (struct route_table 
*nbrs, struct ospf_neighbor *inbr, struct ospf_lsa *lsa);
+#ifdef BUGGY_UNLOCK
  static void ospf_opaque_type9_lsa_rxmt_nbr_check (struct 
ospf_interface *oi);
  static void ospf_opaque_type10_lsa_rxmt_nbr_check (struct ospf_area 
*area);
  static void ospf_opaque_type11_lsa_rxmt_nbr_check (struct ospf *top);
  static unsigned long ospf_opaque_nrxmt_self (struct route_table *nbrs, 
int lsa_type);
+#endif /* BUGGY_UNLOCK */

  void
  ospf_opaque_adjust_lsreq (struct ospf_neighbor *nbr, struct list *lsas)
@@ -2296,17 +2328,20 @@ ospf_opaque_ls_ack_received (struct 
ospf_neighbor *nbr, struct ospf_lsa *lsa)
      {
      case OSPF_OPAQUE_LINK_LSA:
        if (CHECK_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_09_LSA_BIT))
-        ospf_opaque_type9_lsa_rxmt_nbr_check (nbr->oi);
+        UNSET_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_09_LSA_BIT);
+        /* BUGGY_UNLOCK: ospf_opaque_type9_lsa_rxmt_nbr_check (nbr->oi); */
        /* Callback function... */
        break;
      case OSPF_OPAQUE_AREA_LSA:
        if (CHECK_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_10_LSA_BIT))
-        ospf_opaque_type10_lsa_rxmt_nbr_check (nbr->oi->area);
+        UNSET_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_10_LSA_BIT);
+        /* BUGGY_UNLOCK: ospf_opaque_type10_lsa_rxmt_nbr_check 
(nbr->oi->area); */
        /* Callback function... */
        break;
      case OSPF_OPAQUE_AS_LSA:
        if (CHECK_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_11_LSA_BIT))
-        ospf_opaque_type11_lsa_rxmt_nbr_check (top);
+        UNSET_FLAG (top->opaque, OPAQUE_BLOCK_TYPE_11_LSA_BIT);
+        /* BUGGY_UNLOCK: ospf_opaque_type11_lsa_rxmt_nbr_check (top); */
        /* Callback function... */
        break;
      default:
@@ -2315,11 +2350,10 @@ ospf_opaque_ls_ack_received (struct 
ospf_neighbor *nbr, struct ospf_lsa *lsa)
      }

    if (IS_OPAQUE_LSA_ORIGINATION_BLOCKED (top->opaque))
-    {
-      if (IS_DEBUG_OSPF_EVENT)
-        zlog_debug ("Block Opaque-LSA origination: ON -> OFF");
        return; /* Blocking still in progress. */
-    }
+
+  if (IS_DEBUG_OSPF_EVENT)
+    zlog_debug ("Block Opaque-LSA origination: ON -> OFF");

    if (! CHECK_FLAG (top->config, OSPF_OPAQUE_CAPABLE))
      return; /* Opaque capability condition must have changed. */
@@ -2339,6 +2373,7 @@ ospf_opaque_ls_ack_received (struct ospf_neighbor 
*nbr, struct ospf_lsa *lsa)
    return;
  }

+#ifdef BUGGY_UNLOCK
  static void
  ospf_opaque_type9_lsa_rxmt_nbr_check (struct ospf_interface *oi)
  {
@@ -2439,6 +2474,7 @@ ospf_opaque_nrxmt_self (struct route_table *nbrs, 
int lsa_type)

    return n;
  }
+#endif /* BUGGY_UNLOCK */

  /*------------------------------------------------------------------------*
   * Followings are util functions; probably be used by Opaque-LSAs only...



_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.quagga.net/pipermail/quagga-dev/attachments/20141007/650221d8/attachment-0001.html>


More information about the Quagga-dev mailing list