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

Paul Jakma paul at jakma.org
Thu Oct 30 16:44:23 GMT 2014


Hi Olivier,

I think your mailer, or the server it spoke to perhaps, has mangled the 
formating of your patches. E.g. there is line-wrap (probably your MUA), 
and there is also text at the end (presumably added by the server).

Could you export the 6 commits to files and send them to me?

Thanks,

Paul

On Tue, 7 Oct 2014, olivier.dugeon at orange.com wrote:

> 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.
>
>

-- 
Paul Jakma	paul at jakma.org	@pjakma	Key ID: 64A2FF6A
Fortune:
Something unpleasant is coming when men are anxious to tell the truth.
 		-- Benjamin Disraeli




More information about the Quagga-dev mailing list