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

Olivier Dugeon olivier.dugeon at orange.com
Fri Oct 31 13:01:44 GMT 2014


Hello Paul,

Yes, my MUA apply now systematically a disclaimer at the end of all our 
mail. Apologize for that. I'm not able to remove it. BTW, you could 
retrieve the patch from my git tree under the TE-Patches directory. They 
are clean without wrap line or extra additional text.

But, like suggest David, I'll recreate a tree on GitHub with patch 
applied, so, it will be easier to integrate them.

Regards,

Olivier

Le 30/10/2014 17:44, Paul Jakma a écrit :
> 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.
>>
>>
>

-- 
logo Orange <http://www.orange.com>

Olivier Dugeon
Senior research engineer, QoS and network control
Orange/IMT/OLN/WTC/IEE/OPEN

fixe : +33 2 96 05 28 80
mobile : +33 6 82 90 37 85
olivier.dugeon at orange.com <mailto:olivier.dugeon at orange.com>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.quagga.net/pipermail/quagga-dev/attachments/20141031/24c1504d/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: orange_logo.gif
Type: image/gif
Size: 1264 bytes
Desc: not available
URL: <http://lists.quagga.net/pipermail/quagga-dev/attachments/20141031/24c1504d/attachment-0001.gif>


More information about the Quagga-dev mailing list