Hi Paul, Thanks for the feedback!
So first things first, here’s the RFC link for the Extended MRT Header: https://tools.ietf.org/html/rfc6396#section-3 <https://tools.ietf.org/html/rfc6396#section-3>. I agree that is not perfect however I understand that they must ran out of options here. And sure, I can definitely make this a runtime config thing. I’m not sure what command would like me to add though. My first thought would be to change the “dump” command and add an “emrt” parameter: > dump bpp updates/routes/all path interval > dump bpp emrt updates/routes/all path interval This would also allows to dump both with extended Header and “normal” Header. However, you might want to have this as a standalone command? Let me know, Alexis > On 13 Oct 2015, at 06:19, Paul Jakma <[email protected]> wrote: > > On Mon, 12 Oct 2015, Alexis Fasquel wrote: > >> Hello everyone! >> >> So I’ve been working on a small patch to add support for Extended MRT Header >> for BGP messages (MRT dumps) corresponding to MRT type BGP4MP_ET (RFC6396). >> I’ve also developed the same patch for the parsing library bgpdump and the >> pull request is currently under review and should be merged in the upcoming >> week. > > Cool. > >> You will find at then end of this email the diff with the changes I’ve made >> (not much, this is not a big patch). I wanted to add support for the MRT >> types ISIS_ET and OSPFv3_ET however I could not find easily the >> corresponding dump processes. > > I don't think we do MRT dumps for those. Could be interesting to add, fi > someone wants to support it. > >> Also, I wanted to get my extra brownie point by updating the ChangeLog but >> it says: >> >>> ChangeLog information for Quagga is now recorded in our source-code >>> management system. Please see: >>> >>> http://www.quagga.net/devel.php >> >> Am I missing something or the “devel” page is not up to date? >> In anyway, I’ve summed up the changes here: > > The redirect doesn't quite work, maybe someone with more Apache foo than me > knows to rewrite uris like above to: > > http://www.nongnu.org/quagga/devel.html > > And issue a redirect. > > Basically, if you go that latter one, you get a link to git. The git commit > logs are the ChangeLogs now. > >> - Adding ./configure parameter --enable-et-header that will enable support >> for Extended MRT Header. > >> - Adding MSG_PROTOCOL_BGP4MP_ET (value 17) as a MRT type >> - Changing the size of MRT Header to 16 if Extended MRT Header enabled >> - Changing type of MRT dump to MSG_PROTOCOL_BGP4MP_ET and dumping >> microseconds field when Extended MRT Header enabled. >> >> I hope that this patch will match your expectations and let me know if you >> there is anything that I can improve. > > Can we make this not be a compile time thing, but runtime configured? Is eMRT > backwards compatible with MRT? I assum > > >> Regards, >> >> Alexis Fasquel >> Software Engineer @ PCH <http://www.pch.net/> >> +1-415-694-0585 >> >> >> >> diff --git a/bgpd/bgp_dump.c b/bgpd/bgp_dump.c >> index a3c9526..34c00b9 100644 >> --- a/bgpd/bgp_dump.c >> +++ b/bgpd/bgp_dump.c >> @@ -173,17 +173,25 @@ bgp_dump_interval_add (struct bgp_dump *bgp_dump, int >> interval) >> static void >> bgp_dump_header (struct stream *obuf, int type, int subtype) >> { >> - time_t now; >> + struct timeval clock; >> + long msecs; >> + time_t secs; >> >> - /* Set header. */ >> - time (&now); >> + gettimeofday(&clock, NULL); >> + >> + secs = clock.tv_sec; >> + msecs = clock.tv_usec; >> >> /* Put dump packet header. */ >> - stream_putl (obuf, now); >> + stream_putl (obuf, secs); >> stream_putw (obuf, type); >> stream_putw (obuf, subtype); >> - >> stream_putl (obuf, 0); /* len */ >> + >> +#ifdef HAVE_MRT_ET >> + /* Adding microseconds for the MRT Extended Header */ >> + stream_putl (obuf, msecs); >> +#endif >> } > > How does an MRT reader recognise the different format? > >> +#ifdef HAVE_MRT_ET >> + bgp_dump_header (obuf, MSG_PROTOCOL_BGP4MP_ET, BGP4MP_STATE_CHANGE_AS4); > >> #define MSG_PROTOCOL_BGP4MP 16 >> +#define MSG_PROTOCOL_BGP4MP_ET 17 > > Just this basically? An additional protocol type? > > Also, mixing together the MRT format and the routing-protocol family into 1 > code. Ick. That's from the RFC? > > Basically, can you remove the configure.ac bit and the defines, and add a vty > command to control the format? > > regards, > -- > Paul Jakma [email protected] @pjakma Key ID: 64A2FF6A > Fortune: > Be careful how you get yourself involved with persons or situations that > can't bear inspection._______________________________________________ > Quagga-dev mailing list > [email protected] > https://lists.quagga.net/mailman/listinfo/quagga-dev
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
