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

Olivier Dugeon olivier.dugeon at orange.com
Fri Oct 31 12:59:13 GMT 2014


Hello David,

OK. While I'm out of my office with limited access, I'll do that Monday.

I suppose that I must remove all my Quagga-TE tree before ? And then 
start with the latest Quagga git tree, but, do I must import all git 
history or not ?

Thanks,

Olivier

Le 30/10/2014 18:39, David Lamparter a écrit :
> On Thu, Oct 30, 2014 at 04:44:23PM +0000, Paul Jakma wrote:
>> 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?
> Hi Olivier,
>
>
> On a related note, it'd be really cool if your git repository
> https://github.com/Orange-OpenSource/Quagga-TE/tree/master/TE-patches
> would actually contain these patches - not as files as it is right now,
> but actually as part of the history.  I.e. the way it will look when it
> is merged into Quagga.  That way we could indeed merge your tree.
>
> (Easiest way to get there: create new quagga repo, apply these patches
> you posted, publish result to github.)
>
> Cheers,
>
>
> -David
>
>> 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
>>
>> _______________________________________________
>> Quagga-dev mailing list
>> Quagga-dev at lists.quagga.net
>> https://lists.quagga.net/mailman/listinfo/quagga-dev

-- 
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/fdcd5118/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/fdcd5118/attachment-0001.gif>


More information about the Quagga-dev mailing list