[quagga-dev 11562] Re: [PATCH 1/6] Buggy Unlock in ospfd/ospf_opaque.c
olivier.dugeon at orange.com
olivier.dugeon at orange.com
Wed Oct 8 08:26:12 BST 2014
Le 07/10/2014 18:08, Paul Jakma a écrit :
> On Tue, 7 Oct 2014, olivier.dugeon at orange.com wrote:
>> 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.
> Right. Either the router that gets back its LSA must refresh it (with
> an age higher than what the other side has in its LSDB), or it must
> flush it (maxage it).
> Opaque does its own thing a bit there. I've been afraid to touch it in
> the past as I had no way to test opaque LSA handling. We really need
> some way to at least black-box test opaque-LSAs (and OSPF-API, beyond
> the simple client thing we have).
Concerning OSPF-API, we have a java client version for this API to
retrieve LSDB. It works well, but we just used and test the reception of
LSA, not the possibility to send particular opaque LSA.
>> 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.
> Are you sure? Shouldn't ospf_process_self_originated_lsa handle
> receiving self-originated LSAs? Or am I misunderstanding?
Well, I mean that, after browsing the code, there is no such LOCK /
UNLOCK mechanism for LSA 1 - 7. BTW, the ospfd daemon do its jobs
correctly. When I perform my test, I configure only one interface for
the Quagga router. Of course, the Quagga router is announced itself with
the interface link as LSA Type 1 (Router-LSA). Traffic Engineering is
configured on this interface, thus, generating an LSA Type 10. All of
the LSA are flush (maxage) when the Quagga router restart during the
LSDB synchronisation. LSA type 1 is not block and the neighbour router
got them correctly. Only LSA Type 10 is not announced due to the LOCK.
> It'd be good to try make opaque LSAs use generic LSA code as much as
> possible. Or otherwise at least ensure that opaque LSA handling is
> done consistently with other LSAs (e.g. at the same level, following
> similar paths).
Well, both Traffic Engineering, Router Information I added and OSPF API
used the same way to register inside the opaque LSA mechanism and
consequently used the same path to process LSA.
> E.g. ospf_ls_upd, which calls ospf_flood which calls
> ospf_process_self_originated_lsa, contains some opaque LSA related
> self-originated handling which I wonder should be moved down to
> ospf_process_self_originated_lsa or ospf_flood?
Well, opaque code is quiet complex. I'll need some time to check that,
but from what I already seen and analyse, after digging to its own
stuff, opaque functions finish by calling standard ospf functions to
flood opaque LSA.
> And yes, the code paths at work here aren't the simplest (least, don't
> seem simple to me ;) ).
>> 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.
> How did you test this?
Well, as previously mention, I configure Quagga router with one
interface connected directly to a Cisco router or a Juniper one. The
Quagga router announced itself with interface link as LSA Type 1
(Router-LSA), and the Traffic Engineering parameters of the interface
LSA Type 10. Now, I perform 2 tests: the first one consist to shutdown
the ospfd daemon and restart it after few seconds. The waiting time
before restarting the ospfd process to produce the problem must be
comprise between the time need by the neighbour router to disable the
adjacency (depending of the timer configuration i.e. 3 x Hello period)
and the time necessary by the neighbour router to delete LSA in its LSDB
(i.e. MAXAGE or MAXAGE/2). Thus, between 120s and 1800s. The second case
consist to shutdown the interface on the Quagga router or the Cisco or
Juniper router for the same amount of time and then enable it again.
In bot case, the 2 routers exchange HELLO message to setup the adjacency
and then start the DB Synchronisation phase. It is during this phase
that the problem occurs.
> Did the first Quagga instance run long enough to refresh LSAs?
> Otherwise the restarted Quagga instance might not pass the
> ospf_lsa_more_recent test in ospf_ls_upd (under '(5)') that leads to
> ospf_flood - i.e. did it hit the ospf_flood path? :) (I guess it must
> have if you're getting different behaviour, I'd just like a more exact
> description of the testing methodology :) ).
Yes. I also do this test by waiting more than 3600s (i.e. MAXAGE) to see
if the Quagga router will finally re-originate the opaque LSA. Looking
to the debug show that it do the job, but didn't flood the LSA due to
the LOCK that has not been remove.
> If I understand right, this patch doesn't so much fix the problem as
> disable refreshing of self-origination for opaque LSAs? So it's
> shifting the problem a bit? If so, why not just focus instead on
> trying to sanitise, regularise and fix opaque flooding?
No. The patch solve the problem. My interrogation is that this
LOCK/UNLOCK mechanism is not necessary. So, I propose to remove all the
code related to this mechanism. The real job that consist to flush the
self-originate opaque LSA announced back by the neighbour router is done
before the locking system. And it is the main feature conform to RFC.
> Thanks for diving into the opaque code btw! :)
I'm not finish and I think that major improvement are needed. Comparing
to OSPF, IS-IS is much more simpler. the IS-IS Traffic Engineering patch
is quiet simple and don't need this opaque layer.
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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Quagga-dev