Re: [cmaster-next] Bug in zebra_mpls
On Mon, Dec 12, 2016 at 11:18 AM, Bingen Eguzkitza <bingen@voltanet.io> wrote:
Adding Renato too, as he might be interested in.
On Mon, Dec 12, 2016 at 2:16 PM, Bingen Eguzkitza <bingen@voltanet.io> wrote:
Hi,
Fredi and Marc here found a bug that was causing Zebra to segfault. Find attached a patch for it.
I'm ok with this patch in the sense that it's an useful safeguard to have. But I can't see how this was causing a segfault to you. NHLFE entries are never removed directly. Instead of that, we always set the NHLFE_FLAG_DELETED flag and call lsp_processq_add(). Then the Quagga's workqueue guarantees that lsp_process() is called before lsp_processq_del(), which is where nhlfe_del() is called. With that said, it's impossible to have an invalid 'lsp->best_nhlfe' pointer in lsp_process(). There's one other case where nhlfe_del() is called directly which is when the NHLFE_FLAG_INSTALLED flag is not set. But in this case the NHLFE entry will never be best NHLFE entry from the corresponding LSP. Finally, the lines introduced by your patch are not consistent in style with the rest of code (GNU style). Please resubmit a new version with this fixed. -- Renato Westphal
Hi Renato, find attached the patch with GNU style. ßingen. On Tue, Dec 13, 2016 at 2:27 PM, Renato Westphal < renato@opensourcerouting.org> wrote:
On Mon, Dec 12, 2016 at 11:18 AM, Bingen Eguzkitza <bingen@voltanet.io> wrote:
Adding Renato too, as he might be interested in.
On Mon, Dec 12, 2016 at 2:16 PM, Bingen Eguzkitza <bingen@voltanet.io> wrote:
Hi,
Fredi and Marc here found a bug that was causing Zebra to segfault. Find attached a patch for it.
I'm ok with this patch in the sense that it's an useful safeguard to have.
But I can't see how this was causing a segfault to you.
NHLFE entries are never removed directly. Instead of that, we always set the NHLFE_FLAG_DELETED flag and call lsp_processq_add(). Then the Quagga's workqueue guarantees that lsp_process() is called before lsp_processq_del(), which is where nhlfe_del() is called. With that said, it's impossible to have an invalid 'lsp->best_nhlfe' pointer in lsp_process().
There's one other case where nhlfe_del() is called directly which is when the NHLFE_FLAG_INSTALLED flag is not set. But in this case the NHLFE entry will never be best NHLFE entry from the corresponding LSP.
Finally, the lines introduced by your patch are not consistent in style with the rest of code (GNU style). Please resubmit a new version with this fixed. -- Renato Westphal
participants (2)
-
Bingen Eguzkitza -
Renato Westphal