[quagga-dev 16634] Re: [99attendees] [dev] Open source routing @ ietf meetup
Thanks all for the (hopefully useful) meetup. I guess we'll need a larger room or restaurant reservation next time! On July 20, 2017 4:05:16 PM Phil Shafer wrote: Anyone interested in joining a "bring your own lunch" gathering of folks using/building open source routing code, please drop in to "London" on Thursday during lunch (noon). There is no agenda or slides... I wanted to pass along a pointer to the software I mentioned at this meeting: http://juniper.github.io/libxo/libxo-manual.html https://github.com/Juniper/libxo Thanks, Phil ___ 99attendees mailing list 99attend...@ietf.org https://www.ietf.org/mailman/listinfo/99attendees ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16632] Re: Open source routing @ ietf meetup
woops - wrong room - Istanbul, not London... On 7/17/2017 7:42 PM, Lou Berger wrote: > Hi, >Anyone interested in joining a "bring your own lunch" gathering > of folks using/building open source routing code, please drop in to > "London" on Thursday during lunch (noon). There is no agenda or slides... > > Cheers, > Lou > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16631] Open source routing @ ietf meetup
Hi, Anyone interested in joining a "bring your own lunch" gathering of folks using/building open source routing code, please drop in to "London" on Thursday during lunch (noon). There is no agenda or slides... Cheers, Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16595] Re: Meetup @IETF this week?
Hi - given the likely logistics issue of finding a place for folks during lunch I reserved a room -- Vevey 4 It is bring your own -- see you there anytime between 11:30-1:00 (I'll be there a few minutes late as I'm going to get my lunch right after the 1st session)... Lou On 3/27/2017 7:39 AM, Lou Berger wrote: > Anyone interested in meeting up this week? How about continuing the > traditional Thursday lunchtime in the hotel lobby? > > Lou > > PS I'd be happy to discuss the v4 BGP issue that some have reported... > > > > ___ > Quagga-dev mailing list > Quagga-dev@lists.quagga.net > https://lists.quagga.net/mailman/listinfo/quagga-dev > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16594] Meetup @IETF this week?
Anyone interested in meeting up this week? How about continuing the traditional Thursday lunchtime in the hotel lobby? Lou PS I'd be happy to discuss the v4 BGP issue that some have reported... ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16403] Thursday lunch meet up @ietf
Anyone interested? We can meet up at ietf reception, near park ballroom 2 ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16330] Re: [PATCH v2 1/2] bgpd: VRF vty configuration, RIB table creation
Hi Philippe, On October 20, 2016 9:32:38 AM Philippe Guibert wrote: > On Wed, Oct 19, 2016 at 2:22 PM, Lou Berger wrote: > > Hello Lou, > >> I think either we're talking past each other are have some other >> major disconnect. > > Let me reassure you. If both of us speak not at the same time design and > concepts,this may be problematic :-) okay, I think ;-) > About the concepts you are talking about, I agree with you. > Regarding design issue, I think there is a misunderstanding because I > link RD with RTs, within a VRF. > Whereas you point the following: > - associate RD with VPN flows Not sure what you mean here. I don't understand what "VPN" means to you or in the config model. BGP LxVPNs have VRFs and Route Distribution semantics. Together these semantics can be combined to support LxVPNs. I think having a default (outgoing) RD per VRF is fine (and sort of the "normal" case.) > - associate RT set with VRF > > The design currently is not aware of the underlying VRF it works on. That's fine. > One reason is that this work is pure signalling, there is no dataplane > interaction. As is always the the case for BGP ;-) > > I think it should be possible, to adapt the configuration. For > instance, for a VPN network route, > to know which VRF it uses, and consequently, which RT set. I agree. > > Before going further, can I ask you if we share the same point of view > about this patchwork issue ? I think we may have a conceptual model disconnect. To me a VPN is made up of a collection on VRFs, and BGP mechanisms are centered arounf these (VRFs) not VPNs. Consequently, I'd suggest that the key word VRF be used vs VPN in config / VTY. (although if VPN=VRF, always, the change is just a cosmetic one and might not be worth it.) > Do you see other points to discuss ? > Well, we need to ensure that route distribution happens properly for all cases (including RR). See below for more specific comments. > Best Regards, > > Philippe > >> Per BGP LxVPNs, we really should distinguish between VRFs and (vpn) >> route distribution. >> >> WRT VRFs: >> >> The contents of a VRF imported from BGP is determined by one or more >> (import) RTs. RDs are ignored at this point. >> >> An outgoing VPN route is exported to BGP with one or more (export) RTs. >> One configured RD is typically used on outgoing routes exported from a >> VRF (note not VPN) , but multiple routes (identified by different RDs) >> may exist across a single VPN (as identified by an RT set). >> >> WRT Route distribution: >> >> Ignoring RT/community filtering, which currently isn't in the code, BGP >> route distribution basically ignores RTs and uses standard route >> selection based on NRLI route which includes the RD+prefix. This means >> that different RDs can not only be used to segregate prefixes that exist >> within a VPN (and are identified with disjoint sets of RTs) they can >> also be use to support multi-path routes within a VPN (and are >> identified with same/overlapping sets of RTs). BTW RRs only care about >> this part. >> >> So keying a VPN/VRF off an RD just seems wrong, particularly for the >> import case where RD is ignored. >> >> Lou >> >> On 10/19/2016 3:59 AM, Philippe Guibert wrote: >>> On Tue, Oct 18, 2016 at 3:56 PM, Lou Berger wrote: >>> >>> Hello Lou, >>> >>> Please read below my comments. >>> Consecutive to your feedback, I think there are some improvements to bring, >>> mainly about configuration and structure naming. >>> >>>> It looks like RD is your key to a VRF, is this right? >>>> This doesn't match the semantics of BGP VPNs, where RDs are independent >>>> of VRF. (RT set is really the critical thing.) >>> Most of the modifications have been done around what is called the >>> "VRF structure". This is good and aligned with the suggestion to move config to be per VRF. >>> >>> The RD is mapped to that "VRF structure", but the RT also. This is a >>> one-to-one mapping. >>> That "VRF structure" has a RIB table where RT imports are processed. >>> I think a mapping of VRF to RT set is right. Having a default RD for VRF export is also good. >>> I mean, on the design point of view, I don't make a difference between >>> a RD and a RT. I think this is a mistake. RD is about route (re)distribution selection. RT is about topology and VRF control. >>> I just have to ensure that the incoming structure is available to >>> import appro
[quagga-dev 16320] Re: Quagga 1.1.0 released
On 10/19/2016 5:26 PM, John Kemp wrote:
> We usually disable zebra. Something like a route server.
we commonly do the same.
> Not sure if this hits other folks as a default behavior, but...
> we get thousands of warning messages like:
>
> sendmsg_nexthop: Can't send NH register, Zebra client not established
>
> Recommended approach on this one?
set log level to notification, or if you can recompile, delete the log
message.
Lou
> John Kemp
>
> ---
>
> bgpd/bgp_nht.c:
>
> 387 static void
> 388 sendmsg_nexthop (struct bgp_nexthop_cache *bnc, int command)
> 389 {
> 390 struct stream *s;
> 391 struct prefix *p;
> 392 int ret;
> 393
> 394 /* Check socket. */
> 395 if (!zclient || zclient->sock < 0)
> 396 {
> 397 zlog_debug("%s: Can't send NH register, Zebra client not
> established",
> 398 __FUNCTION__);
> 399 return;
> 400 }
>
>
>
> On 10/18/16 7:50 AM, Paul Jakma wrote:
>> Quagga 1.1.0 has been released, available from the usual place:
>>
>> https://download.savannah.gnu.org/releases/quagga/
>>
>> This is a release with a number of new features, and many bug fixes.
>> Notably:
>>
>> * Greatly improved nexthop resolution for recursive routes. (Cumulus)
>> * Event driven nexthop resolution for BGP (Cumulus)
>> * Route tags support (Piotr Chytła, Packet Consulting)
>> * Transport of TE related metrics over OSPF, IS-IS (Olivier Dugeon, Orange)
>> * IPv6 Multipath for zebra and BGP (Ayan Banerjee, Cumulus)
>>
>> This release also changed the default of 'link-detect' state,
>> controlling whether zebra will respond to link-state events and
>> consider an interface to be down when link is down. To retain current
>> the behavior save your config before updating, otherwise remove the
>> 'link-detect' flag from your config prior to updating. There is also a
>> new global 'default link-detect (on|off)' flag to configure the global
>> default.
>>
>> This release includes the security fixes of the 1.0.20161017 release.
>>
>> Thanks to all the contributors, who include:
>>
>> Andrej Ota
>> Avneesh Sachdev
>> Ayan Banerjee
>> Balaji
>> Boian Bonev
>> boris yakubov
>> Christian Franke
>> Christian Franke
>> Colin Petrie
>> Daniel Walton
>> David Lamparter
>> Denil Vira
>> Dinesh Dutt
>> Donald Sharp
>> Evgeny Uskov
>> Igor Ryzhov
>> Jafar Al-Gharaibeh
>> James Li
>> Jonathan Hart
>> kitty
>> Lou Berger
>> Matthieu Boutier
>> Olivier Dugeon
>> Paul Jakma
>> Paul Jakma
>> Paul Jakma
>> Pawel Wieczorkiewicz
>> Philippe Guibert
>> Piotr Chytła
>> Pradosh Mohapatra
>> Roman Hoog Antink
>> Stas Nichiporovich
>> Timo Teräs
>> Udaya Shankara KS
>> Vipin Kumar
>> Vivek Venkatraman
>>
>> See the full changelog available at the above URL for further details.
>> A summary of the changes are as follows:
>>
>> bgp: add "debug bgp allow-martians" next hops and related code/commands
>> bgp: bgp_nexthop init/free AFI_ETHER related NH tables
>> bgpd: Add a null check in bgp_address_del() function when connected
>> addresses are removed.
>> bgpd: add aspath_aggregate_mpath that preserves path length
>> bgpd: Add [bestpath|multipath] option to 'show ip bgp x.x.x.x'
>> bgpd: Add clear command to force a bestpath recalculation and
>> re-advertisement of a prefix
>> bgpd: Add flag to not change e{u,g}id on startup and run as
>> unprivileged user
>> bgpd: Addition of bgp dampening configuration commands under
>> IPv4/multicast
>> bgpd: Addition of dampening show commands under v4 unicast/multicast tree
>> bgpd: Add new configuration cli for graceful restart.
>> bgpd: Alow gracefull shutdown of peers
>> bgpd: atomic-aggregate is lost when we aggregate another aggregate that
>> has atomic-aggregate
>> bgpd: bgp_nexthop_cache not deleted with peers
>> bgpd: Correct a few fuzz failures in BGP
>> bgpd: crash if attributes alone consume > 4096 bytes
>> bgpd: Display BGP paths with unreachable nexthops as invalid
>> bgpd: ditch unused bgp_node_*() functions
>> bgpd: divorce router-id logic from CLI & zebra
>> bgpd, doc: Allow route-map policy modifications to also affect route
>> reflectors.
>> bgpd, doc, lib, zebra: nexthop-tracking in zebra
>> bgpd: don't co
[quagga-dev 16310] Re: [PATCH v2 1/2] bgpd: VRF vty configuration, RIB table creation
Philippe, I think either we're talking past each other are have some other major disconnect. Per BGP LxVPNs, we really should distinguish between VRFs and (vpn) route distribution. WRT VRFs: The contents of a VRF imported from BGP is determined by one or more (import) RTs. RDs are ignored at this point. An outgoing VPN route is exported to BGP with one or more (export) RTs. One configured RD is typically used on outgoing routes exported from a VRF (note not VPN) , but multiple routes (identified by different RDs) may exist across a single VPN (as identified by an RT set). WRT Route distribution: Ignoring RT/community filtering, which currently isn't in the code, BGP route distribution basically ignores RTs and uses standard route selection based on NRLI route which includes the RD+prefix. This means that different RDs can not only be used to segregate prefixes that exist within a VPN (and are identified with disjoint sets of RTs) they can also be use to support multi-path routes within a VPN (and are identified with same/overlapping sets of RTs). BTW RRs only care about this part. So keying a VPN/VRF off an RD just seems wrong, particularly for the import case where RD is ignored. Lou On 10/19/2016 3:59 AM, Philippe Guibert wrote: > On Tue, Oct 18, 2016 at 3:56 PM, Lou Berger wrote: > > Hello Lou, > > Please read below my comments. > Consecutive to your feedback, I think there are some improvements to bring, > mainly about configuration and structure naming. > >> It looks like RD is your key to a VRF, is this right? >> This doesn't match the semantics of BGP VPNs, where RDs are independent >> of VRF. (RT set is really the critical thing.) > Most of the modifications have been done around what is called the > "VRF structure". > > The RD is mapped to that "VRF structure", but the RT also. This is a > one-to-one mapping. > That "VRF structure" has a RIB table where RT imports are processed. > > I mean, on the design point of view, I don't make a difference between > a RD and a RT. > I just have to ensure that the incoming structure is available to > import appropriate entries. > in order to import a RT entry, I have to create a RT mapped to an > other "VRF structure". > > If the RT is not mapped to that "VRF structure", then the RIB > associated to that RT is not present. > Then the import in the RIB RT will not be possible. > > To illustrate, a quick example shows that routerA sends a NLRI entry to > routerB. > > routerA> rd 200:111 import 100:111 export 100:111 300:111 > routerA> network 10.10.10.0/24 rd 200:111 > routerB> rd 100:111 import 100:111 export 100:111 > > RouterB will import NLRI entry to RT 100:111 > This is possible because RouterB has the line rd100:111 ... created. > > RouterB will not import NLRI entry to RT 300:111, since there is no > underlying structure. > > About "VRF structure", My intention is not to interfere about current > VRF implementation in Quagga. > From the configuration point of view, it is easy to find in literature > cisco configuration where VRF > and RD configuration maps together. > Could you be more specific about the reason to dissociate RD from VRF ? > > One modification improvement would consist in : > - That "VRF structure" could be renamed to "RT/RD structure" > - some show commands would be impacted. Instead of dumping VRF RIB > table, one would dump RT/RD RIB table > - the commit logs would be reworked > >> The main implication of >> this is that multipath/ECMP isn't (at least easily) supported. >> Also, >> best path selection on import routes will be wrong if keyed based on RD > Multipath feature impacts both global RIB, and RIB per RT. > > As before, let me illustrate this with a 3 router setup topology. > router A) and router B) send 2 NLRI ECMP entries to router C) > > routerA> rd 200:111 import 100:111 export 100:111 > routerA> network 10.10.10.0/24 rd 200:111 > > routerB> rd 200:111 import 100:111 export 100:111 > routerB> network 10.10.10.0/24 rd 200:111 > > routerC> rd 100:111 import 100:111 export 100:111 > > When receiving a NLRI entry from routerB), and routerA), bgp_update() > will append the entry in the global RIB, according to RD field. > > routerC> show bgp ipv4 vpn > Route Distinguisher : 200:111 > *> 10.10.10.0/24 > => 10.10.10.0/24 > > Then, the import processing is storing the same 2 entries in RT RIB. > routerC> show ip bgp rd 100:111 > Route Distinguisher : 100:111 > *> 10.10.10.0/24 > => 10.10.10.0/24 > > By playing with configuration, it is possible to have multipath > entries in the
[quagga-dev 16306] Re: [PATCH v2 1/2] bgpd: VRF vty configuration, RIB table creation
It looks like RD is your key to a VRF, is this right?
This doesn't match the semantics of BGP VPNs, where RDs are independent
of VRF. (RT set is really the critical thing.) The main implication of
this is that multipath/ECMP isn't (at least easily) supported. Also,
best path selection on import routes will be wrong if keyed based on RD
and I'm not sure what's being done on route reflectors.
I really think that BGP VRFs cannot / must not be keyed based on RDs.
If this is just the config interface, then changing this should be
pretty straight forward.
What do you think?
Lou
PS I think an update to bgpd.texi is needed for this patch
On 10/11/2016 4:17 AM, Philippe Guibert wrote:
> This commit introduces the BGP VRF configuration, and BGP VRF RIB
> table.
> It includes the ability for a BGP to configure its own route
> distinguisher ( aka VRF). New vty commands introduced:
>
> (config-router)# vrf rd
>
> This structure permits configuring import and export route targets,
> which is defined by RFC4360.
> (config-router)# vrf rd exports
> (config-router)# vrf rd imports
>
> It brings some improvements.
> - for a BGP speaker when emitting BGP updates, the exported route
> targets will be mapped to BGP extended communities associated with
> the BGP update for the defined Route distinguisher.
> This commit does not enhance the support for emitting those BGP
> extended communities, but provides the mecanism.
> - for a BGP speaker when receiving BGP updaets. Its import route
> target will be looked up, in order to match NLRI route distinguisher.
> Then, if matching, the entry would be exported to a RIB per VRF table.
>
> As mentioned before, the commit also introduces a BGP VRF RIB table per
> Route distinguisher configured. This table aims at receiving BGP NLRI
> entries, with matching import and export route targets.
> This commit does not take into account the fullfill of this table,
> but creates it.
>
> Signed-off-by: David Lamparter
> Signed-off-by: Philippe Guibert
> ---
> bgpd/bgp_route.c | 193 +++--
> bgpd/bgp_route.h | 4 ++
> bgpd/bgp_table.h | 3 +
> bgpd/bgp_vty.c | 177 +
> bgpd/bgpd.c | 199
> ++-
> bgpd/bgpd.h | 58 +---
> lib/memtypes.c | 2 +
> 7 files changed, 622 insertions(+), 14 deletions(-)
>
> diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
> index 4cb6c141bcdc..abad79f5c9d4 100644
> --- a/bgpd/bgp_route.c
> +++ b/bgpd/bgp_route.c
> @@ -34,6 +34,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330,
> Boston, MA
> #include "plist.h"
> #include "thread.h"
> #include "workqueue.h"
> +#include "hash.h"
>
> #include "bgpd/bgpd.h"
> #include "bgpd/bgp_table.h"
> @@ -61,6 +62,12 @@ Software Foundation, Inc., 59 Temple Place - Suite 330,
> Boston, MA
> extern const char *bgp_origin_str[];
> extern const char *bgp_origin_long_str[];
>
> +static void
> +bgp_static_withdraw_safi (struct bgp *bgp, struct prefix *p, afi_t afi,
> + safi_t safi, struct prefix_rd *prd, u_char *tag);
> +static void
> +bgp_static_free (struct bgp_static *bgp_static);
> +
> static struct bgp_node *
> bgp_afi_node_get (struct bgp_table *table, afi_t afi, safi_t safi, struct
> prefix *p,
> struct prefix_rd *prd)
> @@ -77,7 +84,11 @@ bgp_afi_node_get (struct bgp_table *table, afi_t afi,
> safi_t safi, struct prefix
>prn = bgp_node_get (table, (struct prefix *) prd);
>
>if (prn->info == NULL)
> - prn->info = bgp_table_init (afi, safi);
> +{
> + struct bgp_table *newtab = bgp_table_init (afi, safi);
> + newtab->prd = *prd;
> + prn->info = newtab;
> +}
>else
> bgp_unlock_node (prn);
>table = prn->info;
> @@ -1505,12 +1516,86 @@ bgp_process_announce_selected (struct peer *peer,
> struct bgp_info *selected,
> else
> bgp_adj_out_unset (rn, peer, p, afi, safi);
> break;
> + case BGP_TABLE_VRF:
> +/* never called */
> +assert (0);
> }
>
>bgp_attr_flush (&attr);
>return 0;
> }
>
> +void bgp_vrf_clean_tables (struct bgp_vrf *vrf)
> +{
> + afi_t afi;
> + for (afi = AFI_IP; afi < AFI_MAX; afi++)
> +{
> + struct bgp_info *ri, *ri_next;
> + struct bgp_node *rn;
> +
> + for (rn = bgp_table_top (vrf->rib[afi]); rn; rn = bgp_route_next (rn))
> +for (ri = rn->info; ri; ri = ri_next)
> + {
> +ri_next = ri->next;
> +bgp_info_reap (rn, ri);
> + }
> + bgp_table_finish (&vrf->rib[afi]);
> +
> + for (rn = bgp_table_top (vrf->route[afi]); rn; rn = bgp_route_next
> (rn))
> +if (rn->info)
> + {
> +struct bgp_static *bs = rn->info;
> +bgp_static_withdraw_safi (vrf->bgp, &rn->p, afi, SAFI_MPLS_VPN,
> +
[quagga-dev 16303] Re: Doing something different wrt patch integration
Paul, So where do you want to go with this? Perhaps, propose specific changes to the community (based on current docs, or last round of proposed) and see if there's buy-in? Really, it comes down to how you plan to do things differently for the next release. Lou On 10/3/2016 5:44 AM, Paul Jakma wrote: > Hi Lou, > > On Fri, 30 Sep 2016, Lou Berger wrote: > >> Paul, >> >> Just restating things I've said here before : >> >> - I don't the issue is backlog > It definitely was an issue. People were complaining about how long stuf > had been waiting. I'm not imagining that, pretty sure. > > More generally, there are issue_s_. I don't think there is any /one/ > issue, the solving of which there is a magic bullet for. > >> - I do think the issue is that in order for >> organizations/companies/individuals to invest in an open source >> project that there needs to be process transparency, predicable >> releases, and a deterministic way to get submissions/patches into a >> release. > process transparency, regular releases, deterministic process for > submissions, yes, all good. Though "to get submissions/patches into a > release" is assuming a bit too much. > >> - I frankly don't see how the current model scales or satisfies this. > Well, based on queue length it has objectively done better than what was > there before. As the queue length has gone from increasing to > decreasing, and from years of stuff waiting for action on the review to > less than a years worth of stuff (assuming Martin's full protocol tests > pass and we get a release out RSN). > > Is it the best model for a steady state? I don't know. Are there further > improvements to make, no doubt. > > However, it seemed the best way to get stuff done on the backlog, in a > fairly transparent and systematic way, given the lack of willing > resources. > >> - A bunch of the community worked together to come up with a proposed >> new process and wanted to try it out -- which, from my perspective, >> you vetoed. > Cause it wanted to change _everything_: > > - The ethos and constitution of Quagga > >From one where the onus is on submitters to get submissions to a >standard high enough in terms of (as applicable) design documentation >and advance planning, code organisation and style, and testing, so >that at least no one else strongly enough to object; > >To one where alliances of contributors, potentially some with next to >_no prior involvement_ in Quagga, could band together to shove stuff >through, and where reviewers with objections would have only days to >try rally others to their view. To the extent I'm mistaken on that, it >was to the extent this was under-specified (however, the intent of >majority voting for stuff to go in was clearly there). > >I.e., code going in based on the 51% with the /lowest/ standards, and >a more political and gameable process. > >I think the biggest gulf between the proposers of the mega-changes and >myself was in this area. > > - The integration process was actually _gaining_ overhead and >bureaucracy from a day to day process. > >Patches to require explicit 2nd ACKs - that's overhead. IME with >Quagga, most stuff doesn't need ACKs. People will object to >objectionable stuff, and they'll let it pass otherwise. A "Revisions >needed" model for those minority of patches is more efficient than >a "ack for every patch". > > - The proposed integration process was more a contributors wish list >than a readily implementable integration process. > >Patches to go be applied within days, by who? How? > >I've written before about lessons learned in the past from Quagga, on >patches can more easily fall through the cracks when it isn't clear >who is responsible for applying. Also "days" - there are a number of >problems with that, in various dimensions (see also above on ethos). > >The contributors view of the process is only half the story. It also >has to work for and be efficient for the integrator(s). See also >prior. If you increase the overheads for integrators, then things >likely get worse. > > - Changing the communication tools from email to video conferencing. > >Real-time comms is nice for forming opinions, but it's ungood for >definitive decision making comms. Quagga, as with many open-source >projects, is distributed across the world, and it impossible to suit >everyone (and people in global orgs may already have calendar >
[quagga-dev 16257] [PATCHv2] bgp: add bgp_isvalid_nexthop helper and additional NHT zebra checks (also restores check from original patch)
---
bgpd/bgp_nht.c | 16 +++-
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index b5d830e..171cb20 100644
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -52,6 +52,13 @@ static int make_prefix(int afi, struct bgp_info *ri, struct
prefix *p);
static void path_nh_map(struct bgp_info *path, struct bgp_nexthop_cache *bnc,
int keep);
+static int
+bgp_isvalid_nexthop (struct bgp_nexthop_cache *bnc)
+{
+ return (bgp_zebra_num_connects() == 0 ||
+ (bnc && CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID)));
+}
+
int
bgp_find_nexthop (struct bgp_info *path, int connected)
{
@@ -63,8 +70,7 @@ bgp_find_nexthop (struct bgp_info *path, int connected)
if (connected && !(CHECK_FLAG(bnc->flags, BGP_NEXTHOP_CONNECTED)))
return 0;
- return (bgp_zebra_num_connects() == 0 ||
- CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID));
+ return (bgp_isvalid_nexthop(bnc));
}
static void
@@ -194,7 +200,7 @@ bgp_find_or_add_nexthop (afi_t afi, struct bgp_info *ri,
struct peer *peer,
else if (peer)
bnc->nht_info = (void *)peer; /* NHT peer reference */
- return (CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID));
+ return (bgp_isvalid_nexthop(bnc));
}
void
@@ -497,7 +503,7 @@ evaluate_paths (struct bgp_nexthop_cache *bnc)
* reachable/unreachable.
*/
if ((CHECK_FLAG(path->flags, BGP_INFO_VALID) ? 1 : 0) !=
- (CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID) ? 1 : 0))
+ (bgp_isvalid_nexthop(bnc) ? 1 : 0))
{
if (CHECK_FLAG (path->flags, BGP_INFO_VALID))
{
@@ -514,7 +520,7 @@ evaluate_paths (struct bgp_nexthop_cache *bnc)
}
/* Copy the metric to the path. Will be used for bestpath computation */
- if (CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID) && bnc->metric)
+ if (bgp_isvalid_nexthop(bnc) && bnc->metric)
(bgp_info_extra_get(path))->igpmetric = bnc->metric;
else if (path->extra)
path->extra->igpmetric = 0;
--
2.1.3
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16254] Re: [PATCH] bgp: restore missing check from original ignore NHT change
On 10/11/2016 7:04 AM, Paul Jakma wrote: > I don't understand the subject. This isn't restoring anything, but > adding a missing check from the patch to make NHT work without zebra? Or > did I somehow drop that chunk from the original patch? Yes, I patched a different spot - that is the one that is used in my test cases. Your change (under my change log) seems legitimate too, but as far as I can tell it's in a function that is never called. > Also, what about the tests on NEXTHOP_VALID in evaluate_paths? Do they > need to take bgp_zebra_num_connects into account? Fair point. I'm not sure - but I think you are right. > At which point, better > to do this valid check via a helper? WFM. Lou > regards, > > Paul > > On Mon, 10 Oct 2016, Lou Berger wrote: > >> --- >> bgpd/bgp_nht.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c >> index b5d830e..7808505 100644 >> --- a/bgpd/bgp_nht.c >> +++ b/bgpd/bgp_nht.c >> @@ -194,7 +194,8 @@ bgp_find_or_add_nexthop (afi_t afi, struct bgp_info *ri, >> struct peer *peer, >> else if (peer) >> bnc->nht_info = (void *)peer; /* NHT peer reference */ >> >> - return (CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID)); >> + return (bgp_zebra_num_connects() == 0 || >> + CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID)); >> } >> >> void >> ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16230] Re: Proposed 8 testing Update
On 10/10/2016 10:16 AM, Paul Jakma wrote: > On Mon, 10 Oct 2016, Martin Winter wrote: > >> > Not what the exact plans is, but I prefer either a release branch >> > started now or new commits which are not only bug fixes to be held up >> > for a week to let the code “settle” and give everyone time to test. > Ok, lets wait till the end of the week then. > I sent two NHT related changes to the list. The first cleans up a warning, so is optional. The second change is a must - my other testing is looking good. Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16228] [PATCHv3 0/4] bgpd: add L3/L2VPN Virtual Network Control feature
This is the third update of the Virtual Network Control feature. Information on the previous version can be found in the archive at: https://lists.quagga.net/pipermail/quagga-dev/2016-May/015364.html which was reviewed and ACK'ed on list. This patch set has been updated to the latest master, and combines changes made during the ACK'ing process. As before details in changes and code can be found at https://github.com/LabNConsulting/quagga-vnc The first version was submitted in March of 2014. This patch set includes: [PATCH 1/4] bgpd: eliminate RD related duplicate code in bgp_encap.c [PATCH 2/4] lib: add skiplist [PATCH 3/4] lib: add route_table_get_default_delegate [PATCH 4/4] bgpd: add L3/L2VPN Virtual Network Control feature ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16227] [PATCHv3 2/4] lib: add skiplist
---
lib/Makefile.am | 6 +-
lib/skiplist.c | 683
lib/skiplist.h | 159 +
3 files changed, 845 insertions(+), 3 deletions(-)
create mode 100644 lib/skiplist.c
create mode 100644 lib/skiplist.h
diff --git a/lib/Makefile.am b/lib/Makefile.am
index be8495f..23aa179 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -14,7 +14,7 @@ libzebra_la_SOURCES = \
filter.c routemap.c distribute.c stream.c str.c log.c plist.c \
zclient.c sockopt.c smux.c agentx.c snmp.c md5.c if_rmap.c keychain.c
privs.c \
sigevent.c pqueue.c jhash.c memtypes.c workqueue.c vrf.c \
- event_counter.c nexthop.c
+ event_counter.c nexthop.c skiplist.c
BUILT_SOURCES = memtypes.h route_types.h gitversion.h
@@ -30,7 +30,7 @@ pkginclude_HEADERS = \
plist.h zclient.h sockopt.h smux.h md5.h if_rmap.h keychain.h \
privs.h sigevent.h pqueue.h jhash.h zassert.h memtypes.h \
workqueue.h route_types.h libospf.h vrf.h fifo.h event_counter.h \
- nexthop.h
+ nexthop.h skiplist.h
noinst_HEADERS = \
plist_int.h
@@ -62,7 +62,7 @@ GITH=gitversion.h
gitversion.h.tmp: $(srcdir)/../.git
@PERL@ $(srcdir)/gitversion.pl $(srcdir) > ${GITH}.tmp
gitversion.h: gitversion.h.tmp
- { test -f ${GITH} && diff -s -q ${GITH}.tmp ${GITH}; } || cp -v
${GITH}.tmp ${GITH}
+ { test -f ${GITH} && diff -s -q ${GITH}.tmp ${GITH}; } || cp -v
${GITH}.tmp ${GITH}
else
.PHONY: gitversion.h
diff --git a/lib/skiplist.c b/lib/skiplist.c
new file mode 100644
index 000..8cf6158
--- /dev/null
+++ b/lib/skiplist.c
@@ -0,0 +1,683 @@
+/*
+ * Copyright 1990 William Pugh
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * Permission to include in quagga provide on March 31, 2016
+ */
+
+/*
+ * Skip List impementation based on code from William Pugh.
+ * ftp://ftp.cs.umd.edu/pub/skipLists/
+ *
+ * Skip Lists are a probabilistic alternative to balanced trees, as
+ * described in the June 1990 issue of CACM and were invented by
+ * William Pugh in 1987.
+ *
+ * This file contains source code to implement a dictionary using
+ * skip lists and a test driver to test the routines.
+ *
+ * A couple of comments about this implementation:
+ * The routine randomLevel has been hard-coded to generate random
+ * levels using p=0.25. It can be easily changed.
+ *
+ * The insertion routine has been implemented so as to use the
+ * dirty hack described in the CACM paper: if a random level is
+ * generated that is more than the current maximum level, the
+ * current maximum level plus one is used instead.
+ *
+ * Levels start at zero and go up to MaxLevel (which is equal to
+ * (MaxNumberOfLevels-1).
+ *
+ * The run-time flag SKIPLIST_FLAG_ALLOW_DUPLICATES determines whether or
+ * not duplicates are allowed for a given list. If set, duplicates are
+ * allowed and act in a FIFO manner. If not set, an insertion of a value
+ * already in the list updates the previously existing binding.
+ *
+ * BitsInRandom is defined to be the number of bits returned by a call to
+ * random(). For most all machines with 32-bit integers, this is 31 bits
+ * as currently set.
+ */
+
+
+#include
+
+#include "memory.h"
+#include "log.h"
+#include "vty.h"
+#include "skiplist.h"
+
+
+#define BitsInRandom 31
+
+#define MaxNumberOfLevels 16
+#define MaxLevel (MaxNumberOfLevels-1)
+#define newNodeOfLevel(l) XCALLOC(MTYPE_SKIP_LIST_NODE, sizeof(struct
skiplistnode)+(l)*sizeof(struct skiplistnode *))
+
+static int randomsLeft;
+static int randomBits;
+static struct skiplist *skiplist_last_created; /* debugging hack */
+
+#if 1
+#define CHECKLAST(sl) do {\
+if ((sl)->header->forward[0] && !(sl)->last) assert(0);\
+if (!(sl)->header->forward[0] && (sl)->last) assert(0);\
+} while (0)
+#else
+#define CHECKLAST(sl)
+#endif
+
+
+static int
+randomLevel()
+{
+register int level = 0;
+register int b;
+
+do {
+ if (randomsLeft <= 0) {
+ randomBits = random();
+ randomsLeft = BitsInRandom/2;
+ }
+ b = randomBits&3;
+
[quagga-dev 16229] [PATCHv3 1/4] bgpd: eliminate RD related duplicate code in bgp_encap.c decode_rd_... apis are declared global in bgp_mplsvpn.c
---
bgpd/bgp_encap.c | 45 -
bgpd/bgp_mplsvpn.c | 8
bgpd/bgp_mplsvpn.h | 5 +
3 files changed, 9 insertions(+), 49 deletions(-)
diff --git a/bgpd/bgp_encap.c b/bgpd/bgp_encap.c
index edd9d76..8361d3f 100644
--- a/bgpd/bgp_encap.c
+++ b/bgpd/bgp_encap.c
@@ -45,51 +45,6 @@ Software Foundation, Inc., 59 Temple Place - Suite 330,
Boston, MA
#include "bgpd/bgp_vty.h"
#include "bgpd/bgp_encap.h"
-static u_int16_t
-decode_rd_type (u_char *pnt)
-{
- u_int16_t v;
-
- v = ((u_int16_t) *pnt++ << 8);
- v |= (u_int16_t) *pnt;
- return v;
-}
-
-
-static void
-decode_rd_as (u_char *pnt, struct rd_as *rd_as)
-{
- rd_as->as = (u_int16_t) *pnt++ << 8;
- rd_as->as |= (u_int16_t) *pnt++;
-
- rd_as->val = ((u_int32_t) *pnt++) << 24;
- rd_as->val |= ((u_int32_t) *pnt++) << 16;
- rd_as->val |= ((u_int32_t) *pnt++) << 8;
- rd_as->val |= (u_int32_t) *pnt;
-}
-
-static void
-decode_rd_as4 (u_char *pnt, struct rd_as *rd_as)
-{
- rd_as->as = (u_int32_t) *pnt++ << 24;
- rd_as->as |= (u_int32_t) *pnt++ << 16;
- rd_as->as |= (u_int32_t) *pnt++ << 8;
- rd_as->as |= (u_int32_t) *pnt++;
-
- rd_as->val = ((u_int32_t) *pnt++ << 8);
- rd_as->val |= (u_int32_t) *pnt;
-}
-
-static void
-decode_rd_ip (u_char *pnt, struct rd_ip *rd_ip)
-{
- memcpy (&rd_ip->ip, pnt, 4);
- pnt += 4;
-
- rd_ip->val = ((u_int16_t) *pnt++ << 8);
- rd_ip->val |= (u_int16_t) *pnt;
-}
-
static void
ecom2prd(struct ecommunity *ecom, struct prefix_rd *prd)
{
diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 047105d..392a239 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -35,7 +35,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330,
Boston, MA
#include "bgpd/bgp_mplsvpn.h"
#include "bgpd/bgp_packet.h"
-static u_int16_t
+u_int16_t
decode_rd_type (u_char *pnt)
{
u_int16_t v;
@@ -57,7 +57,7 @@ decode_label (u_char *pnt)
}
/* type == RD_TYPE_AS */
-static void
+void
decode_rd_as (u_char *pnt, struct rd_as *rd_as)
{
rd_as->as = (u_int16_t) *pnt++ << 8;
@@ -70,7 +70,7 @@ decode_rd_as (u_char *pnt, struct rd_as *rd_as)
}
/* type == RD_TYPE_AS4 */
-static void
+void
decode_rd_as4 (u_char *pnt, struct rd_as *rd_as)
{
rd_as->as = (u_int32_t) *pnt++ << 24;
@@ -83,7 +83,7 @@ decode_rd_as4 (u_char *pnt, struct rd_as *rd_as)
}
/* type == RD_TYPE_IP */
-static void
+void
decode_rd_ip (u_char *pnt, struct rd_ip *rd_ip)
{
memcpy (&rd_ip->ip, pnt, 4);
diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h
index 3299b9c..3fbbd33 100644
--- a/bgpd/bgp_mplsvpn.h
+++ b/bgpd/bgp_mplsvpn.h
@@ -41,9 +41,14 @@ struct rd_ip
u_int16_t val;
};
+extern u_int16_t decode_rd_type (u_char *);
extern void bgp_mplsvpn_init (void);
extern int bgp_nlri_parse_vpn (struct peer *, struct attr *, struct bgp_nlri
*);
extern u_int32_t decode_label (u_char *);
+extern void encode_label(u_int32_t, u_char *);
+extern void decode_rd_as (u_char *, struct rd_as *);
+extern void decode_rd_as4 (u_char *, struct rd_as *);
+extern void decode_rd_ip (u_char *, struct rd_ip *);
extern int str2prefix_rd (const char *, struct prefix_rd *);
extern int str2tag (const char *, u_char *);
extern char *prefix_rd2str (struct prefix_rd *, char *, size_t);
--
2.1.3
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16226] [PATCHv3 3/4] lib: add route_table_get_default_delegate
---
lib/table.c | 6 ++
lib/table.h | 3 +++
2 files changed, 9 insertions(+)
diff --git a/lib/table.c b/lib/table.c
index da21361..a026b29 100644
--- a/lib/table.c
+++ b/lib/table.c
@@ -520,6 +520,12 @@ static route_table_delegate_t default_delegate = {
.destroy_node = route_node_destroy
};
+route_table_delegate_t *
+route_table_get_default_delegate(void)
+{
+ return &default_delegate;
+}
+
/*
* route_table_init
*/
diff --git a/lib/table.h b/lib/table.h
index 2ffd79b..16a91b4 100644
--- a/lib/table.h
+++ b/lib/table.h
@@ -141,6 +141,9 @@ extern struct route_table *route_table_init (void);
extern struct route_table *
route_table_init_with_delegate (route_table_delegate_t *);
+extern route_table_delegate_t *
+route_table_get_default_delegate(void);
+
extern void route_table_finish (struct route_table *);
extern void route_unlock_node (struct route_node *node);
extern struct route_node *route_top (struct route_table *);
--
2.1.3
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16223] [PATCH] bgp: restore missing check from original ignore NHT change
--- bgpd/bgp_nht.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index b5d830e..7808505 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -194,7 +194,8 @@ bgp_find_or_add_nexthop (afi_t afi, struct bgp_info *ri, struct peer *peer, else if (peer) bnc->nht_info = (void *)peer; /* NHT peer reference */ - return (CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID)); + return (bgp_zebra_num_connects() == 0 || + CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID)); } void -- 2.1.3 ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16217] [PATCH] bgp: fix warning in bgp_nht.c
--- bgpd/bgp_nht.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c index 0cfcadd..b5d830e 100644 --- a/bgpd/bgp_nht.c +++ b/bgpd/bgp_nht.c @@ -40,6 +40,7 @@ #include "bgpd/bgp_debug.h" #include "bgpd/bgp_nht.h" #include "bgpd/bgp_fsm.h" +#include "bgpd/bgp_zebra.h" extern struct zclient *zclient; extern struct bgp_table *bgp_nexthop_cache_table[AFI_MAX]; -- 2.1.3 ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16208] Re: Proposed 8 testing Update
woops, guess I should have checked the repo before sending (re #1) - nice to see the change!!! On 10/7/2016 1:15 PM, Lou Berger wrote: > Paul/All, > > A couple of suggestions: > > 1) do the update to master now and release once the integration run > completes > > 2) start immediately on the next release > > 3) require any multipart pass pull+regression using dev/automerge branch > (you choose if only ack'ed patches should be pull requested or not.) > > Lou > > On 10/7/2016 12:44 PM, Paul Jakma wrote: >> On Fri, 7 Oct 2016, Igor Ryzhov wrote: >> >>> Maybe you can also apply them and include into the release? >> I'll have a look and see. >> >> Though, be even better to get just to steady integration and releases. >> There's no reason not to have releases every couple of months - assuming >> there's new stuff to be released. >> >> regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16207] Re: Proposed 8 testing Update
Paul/All, A couple of suggestions: 1) do the update to master now and release once the integration run completes 2) start immediately on the next release 3) require any multipart pass pull+regression using dev/automerge branch (you choose if only ack'ed patches should be pull requested or not.) Lou On 10/7/2016 12:44 PM, Paul Jakma wrote: > On Fri, 7 Oct 2016, Igor Ryzhov wrote: > >> Maybe you can also apply them and include into the release? > I'll have a look and see. > > Though, be even better to get just to steady integration and releases. > There's no reason not to have releases every couple of months - assuming > there's new stuff to be released. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16202] Re: [PATCH] bgp: ignore NHT when bgpd has never connected zebra
I wanted to avoid race conditions related to zebra restarting - just in case such occured. So current logic is use NHT if zebra *ever* seen/connected (and not of zebra currently connected.) Lou On October 7, 2016 5:43:15 AM Paul Jakma wrote: Thanks. On Thu, 6 Oct 2016, Lou Berger wrote: +int zclient_num_connects; Why not just a zclient_connected() type helper to check if zclient->sock is negative or >= 0? regards, -- Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A Fortune: "Someone's been mean to you! Tell me who it is, so I can punch him tastefully." -- Ralph Bakshi's Mighty Mouse ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16200] Re: Proposed 8 testing Update
On October 6, 2016 11:47:59 AM Paul Jakma wrote: On Thu, 6 Oct 2016, Lou Berger wrote: One outstanding issue for the release is operation of BGP without zebra. (The NHT change basically introduces a requirement to run BGP with zebra, while there are route server/reflector configs which are viable now running only BGP.) I suggest changing the NHT code to either (a) automatically operate / adjust to when zebra isn't present or (b) have a bgp config option to ignore NHT, e.g., 'no bgp nexthop-tracking'. Do you/anyone have a preference? -- Mine is (a). Definitely a. Great. You should have the patch. I (or Paul Z.) can propose a patch for this change. Cool. So, that'd be a release blocker, but not a 'ff master' blocker, right? Agreed! Lou regards, -- Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A Fortune: I do desire we may be better strangers. -- William Shakespeare, "As You Like It" ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16194] Re: Proposed 8 testing Update
On 10/6/2016 9:35 AM, Martin Winter wrote: > I think something to address before a release, but not sure if we should > hold off merging to master because of it. > > Lou, are you proposing to fix this before merging to master? No. Please merge to master ASAP. This said -- just sending patch (based on ff-2016091901) now... Lou > > - Martin > > On 6 Oct 2016, at 6:30, Lou Berger wrote: > >> Paul, >> >> On October 6, 2016 6:05:14 AM Paul Jakma wrote: >> >> ... >>> One change from the last release in BGP is ANVL-BGPPLUS-17.1 related to >>> address used for nexthop - NHT related? Is it serious? >>> >>> Looks acceptable generally. >>> >> One outstanding issue for the release is operation of BGP without zebra. >> (The NHT change basically introduces a requirement to run BGP with >> zebra, while there are route server/reflector configs which are viable >> now running only BGP.) >> >> I suggest changing the NHT code to either (a) automatically operate / >> adjust to when zebra isn't present or (b) have a bgp config option to >> ignore NHT, e.g., 'no bgp nexthop-tracking'. >> >> Do you/anyone have a preference? -- Mine is (a). >> >> I (or Paul Z.) can propose a patch for this change. >> >> Lou >> ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16195] [PATCH] bgp: ignore NHT when bgpd has never connected zebra
---
quagga/bgpd/bgp_nht.c | 3 ++-
quagga/bgpd/bgp_zebra.c | 11 +++
quagga/bgpd/bgp_zebra.h | 2 ++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/quagga/bgpd/bgp_nht.c b/quagga/bgpd/bgp_nht.c
index 591fbf9..c4acb66 100644
--- a/quagga/bgpd/bgp_nht.c
+++ b/quagga/bgpd/bgp_nht.c
@@ -192,7 +192,8 @@ bgp_find_or_add_nexthop (afi_t afi, struct bgp_info *ri,
struct peer *peer,
else if (peer)
bnc->nht_info = (void *)peer; /* NHT peer reference */
- return (CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID));
+ return (bgp_zebra_num_connects() == 0 ||
+ CHECK_FLAG(bnc->flags, BGP_NEXTHOP_VALID));
}
void
diff --git a/quagga/bgpd/bgp_zebra.c b/quagga/bgpd/bgp_zebra.c
index 854d93d..974baed 100644
--- a/quagga/bgpd/bgp_zebra.c
+++ b/quagga/bgpd/bgp_zebra.c
@@ -54,6 +54,8 @@ struct in_addr router_id_zebra;
struct stream *bgp_nexthop_buf = NULL;
struct stream *bgp_ifindices_buf = NULL;
+int zclient_num_connects;
+
/* Router-id update message from zebra. */
static int
bgp_router_id_update (int command, struct zclient *zclient, zebra_size_t
length,
@@ -1187,12 +1189,15 @@ bgp_zclient_reset (void)
static void
bgp_zebra_connected (struct zclient *zclient)
{
+ zclient_num_connects++;
zclient_send_requests (zclient, VRF_DEFAULT);
}
void
bgp_zebra_init (struct thread_master *master)
{
+ zclient_num_connects = 0;
+
/* Set default values. */
zclient = zclient_new (master);
zclient_init (zclient, ZEBRA_ROUTE_BGP);
@@ -1223,3 +1228,9 @@ bgp_zebra_destroy(void)
zclient_free(zclient);
zclient = NULL;
}
+
+int
+bgp_zebra_num_connects(void)
+{
+ return zclient_num_connects;
+}
diff --git a/quagga/bgpd/bgp_zebra.h b/quagga/bgpd/bgp_zebra.h
index 9a592c3..5d4ed62 100644
--- a/quagga/bgpd/bgp_zebra.h
+++ b/quagga/bgpd/bgp_zebra.h
@@ -48,4 +48,6 @@ extern struct interface *if_lookup_by_ipv4_exact (struct
in_addr *);
extern struct interface *if_lookup_by_ipv6 (struct in6_addr *);
extern struct interface *if_lookup_by_ipv6_exact (struct in6_addr *);
+extern int bgp_zebra_num_connects(void);
+
#endif /* _QUAGGA_BGP_ZEBRA_H */
--
2.1.3
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16191] Re: Proposed 8 testing Update
Paul, On October 6, 2016 6:05:14 AM Paul Jakma wrote: ... > > One change from the last release in BGP is ANVL-BGPPLUS-17.1 related to > address used for nexthop - NHT related? Is it serious? > > Looks acceptable generally. > One outstanding issue for the release is operation of BGP without zebra. (The NHT change basically introduces a requirement to run BGP with zebra, while there are route server/reflector configs which are viable now running only BGP.) I suggest changing the NHT code to either (a) automatically operate / adjust to when zebra isn't present or (b) have a bgp config option to ignore NHT, e.g., 'no bgp nexthop-tracking'. Do you/anyone have a preference? -- Mine is (a). I (or Paul Z.) can propose a patch for this change. Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16154] Re: Doing something different wrt patch integration
Paul, Just restating things I've said here before : - I don't the issue is backlog - I do think the issue is that in order for organizations/companies/individuals to invest in an open source project that there needs to be process transparency, predicable releases, and a deterministic way to get submissions/patches into a release. - I frankly don't see how the current model scales or satisfies this. - A bunch of the community worked together to come up with a proposed new process and wanted to try it out -- which, from my perspective, you vetoed. I think these leaves the ball in your court. Lou On 9/30/2016 6:35 AM, Paul Jakma wrote: > On Wed, 21 Sep 2016, Lou Berger wrote: > >> I hope the the last two releases have convinced you that we need to do >> something different WRT patch integration. > Convinced in what sense? Also, the implication is that you feel I am > against changing /anything/ - which is not at all the case. > > High level objectives of getting patches reviewed, decided on and > integrated/pushed-back quickly, efficiently, transparently, reliably? > Who could disagree with that? > > Determining how to change things to improve on delivery of those goals, > by teasing out the strands of the problem, sorting the objective from > the subject (and where possible finding ways to transform as much of the > subjective into the objective as possible), and determining as > analytically as possible how proposed chnages affect the different > constraints (at least some of which are in tension): Great. > > Throwing everything up in the air, changing everything at the same time > (from communication methods, to constitution) - less keen on that. > > > > And there were big changes when I started full-time again. Vincent > started with the batched, systematic, review process of the backlog in > patchwork. I continued on with that, changed some logging aspects from > email to git. > > Is that process ideal for a non-backlogged, steady-state? Perhaps not. > > However, the problem then was backlog. And optimising integrator > throughput - while not regressing back to more ad-hoc much less > systematic working - to get the backlog done seemed to be a high > priority. That hopefully really is close to done now, with the final CI > obstacle found, and we can close off a large swathe of old stuff and > release. > > It could have gone faster perhaps, with more co-operation and fewer > other distractions. But hey. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16152] Re: round-8: tentative proposed head for master
woops - you're right - these are BGP_ignore actions... Lou On 9/30/2016 10:49 AM, Paul Jakma wrote: > Which change? > > There's no changes to OpenXXX? ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16150] Re: round-8: tentative proposed head for master
The change I submitted continues the original use of NHT information for Idle (start event) and Active (retry) states, i.e., just removes NHT info in Connect state. Keeping these event sources seems fine (at least based on testing) and not clear to me very significant, but I haven't traced through all the possible generation of NHT events. I have no strong option on keeping the original event mapping code or the intro of an NHT event - I can see arguments for/against both. But I'm not comfortable with the into of the new transitions at this point - certainly I'm skeptical about the change in Connect and OpenXXX states. Lou On 9/30/2016 10:17 AM, Paul Jakma wrote: > On Fri, 30 Sep 2016, Lou Berger wrote: > >> It's unclear to me why an NHT should *ever* trigger a state machine >> change. At this point, I'm more comfortable with going back to not >> changing BGP FSM state than introducing the three new FSM changes you >> have in the code... > The 3 FSM changes are basically the exact same as the NHT code was > doing, except without calling connect_check on Connect. > > I.e. the major functional difference to yours is that it retains going > through the ConnectRetry_timer_expired event on Connect as the prior NHT > code was doing. > > The other differences are cosmetic, to just do it via the existing FSM, > so it's obvious. If one FSM is hard to understand, two different > interacting FSMs is harder still. > >> Why do you think the state transitions are are appropriate based on an >> NHT_Update? What are the real sources of NHT_Updates? If BGP itself, >> as the prior case, these should certainly be ignored. > The source is a message from zebra. > > If connections that are OpenSent+ can ignore NHT events, then those > events can also be ignored for Connect and Active. Trying to optimise > for bringing up links and short-circuit the connect-retry timer in case > of some link event pertinent to the peer address, I assume. > > Remove that and simplify it all, or keep? > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16147] Re: round-8: tentative proposed head for master
It's unclear to me why an NHT should *ever* trigger a state machine change. At this point, I'm more comfortable with going back to not changing BGP FSM state than introducing the three new FSM changes you have in the code... Why do you think the state transitions are are appropriate based on an NHT_Update? What are the real sources of NHT_Updates? If BGP itself, as the prior case, these should certainly be ignored. Lou On 9/30/2016 9:41 AM, Paul Jakma wrote: > Hi Lou, > > On Fri, 30 Sep 2016, Paul Jakma wrote: > >> On Thu, 29 Sep 2016, Lou Berger wrote: >> >>> looks like the NHT code is generating an event into the state machine >>> that it shouldn't be. I removed this and checked it into my work >>> area. See >>> >>> https://github.com/LabNConsulting/quagga-vnc/commit/7162337f9261b91056b95a673a54ad595aef3e5f >>> Kudos to Martin for logs/pcaps/discussion and the system that id'ed this >>> issue and will verify the fix. >> Yeah, I'd come to the same conclusion yesterday with my own test-case (again >> based on logs and pcaps from Martin) and emailed him. :) > So, I have: > >http://git.savannah.gnu.org/cgit/quagga.git/commit/?id=26943e36 > > The whole bgp_fsm_nht_update can just be removed, and this second > NHT-dedicated, mini-sub-FSM removed and the event handled via the main > FSM. > > Indeed, is it even necessary to tickle Active/Connect connections to > make them reconnect cause of an NHT event? In which case, the fix is > just removing code. > > ? > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16139] Re: round-8: tentative proposed head for master
looks like the NHT code is generating an event into the state machine that it shouldn't be. I removed this and checked it into my work area. See https://github.com/LabNConsulting/quagga-vnc/commit/7162337f9261b91056b95a673a54ad595aef3e5f Kudos to Martin for logs/pcaps/discussion and the system that id'ed this issue and will verify the fix. I'll push my changes into dev/automerge and see how it goes. The push will include my merged VNC code (disabled by default) too. Lou On 9/29/2016 6:28 PM, Lou Berger wrote: > Just walked through the code a bit -- looks like quagga needs to add > support for DelayOpen(Timer) when receiving a new tcp connection... > > Lou > > > On 9/29/2016 9:29 AM, Paul Jakma wrote: >> On Thu, 22 Sep 2016, Paul Jakma wrote: >> >>> Just the session setup with the test tools the final issue. I've been >>> adapting Martin's bgptool test code to test BGP collision handling. >>> Not found the problem so far. Unfortunately, I have other stuff >>> occupying me until early next week now. >> So, the issue here seems to be that: >> >> - the other side of Martin's tests waits for a connection from bgpd, to >>know that bgpd is Active. The other side does not send SYN|ACK though, >>the bgpd side host is resending its SYNs. >> >> - however somehow bgpd thinks its connection succeeds, so it raises >>TCP_connection_open, and the peer goes into a state where >>collision-detection will consider it (OpenSent). >> >> - the other side connects to bgpd, this succeeds. >> >> - bgpd closes the inbound connection, cause it thinks its outbound tcp >>is open, BGP FSM is in OpenSent, and its own RID is higher - so its >>outbound has priority. >> >> The problem presumably lies in bgp_connect_check() getting things wrong >> somehow, but how >> >> If so, this must be a very old issue. >> >> regards, > > ___ > Quagga-dev mailing list > Quagga-dev@lists.quagga.net > https://lists.quagga.net/mailman/listinfo/quagga-dev > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16137] Re: round-8: tentative proposed head for master
Just walked through the code a bit -- looks like quagga needs to add support for DelayOpen(Timer) when receiving a new tcp connection... Lou On 9/29/2016 9:29 AM, Paul Jakma wrote: > On Thu, 22 Sep 2016, Paul Jakma wrote: > >> Just the session setup with the test tools the final issue. I've been >> adapting Martin's bgptool test code to test BGP collision handling. >> Not found the problem so far. Unfortunately, I have other stuff >> occupying me until early next week now. > So, the issue here seems to be that: > > - the other side of Martin's tests waits for a connection from bgpd, to >know that bgpd is Active. The other side does not send SYN|ACK though, >the bgpd side host is resending its SYNs. > > - however somehow bgpd thinks its connection succeeds, so it raises >TCP_connection_open, and the peer goes into a state where >collision-detection will consider it (OpenSent). > > - the other side connects to bgpd, this succeeds. > > - bgpd closes the inbound connection, cause it thinks its outbound tcp >is open, BGP FSM is in OpenSent, and its own RID is higher - so its >outbound has priority. > > The problem presumably lies in bgp_connect_check() getting things wrong > somehow, but how > > If so, this must be a very old issue. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16110] Re: Question/issue on BGP NHT in 8/proposed/ff
oh - and forgot this: 3) How does NHT work when zebra is not running??? i.e., doesn't this break all deployments where zebra isn't used? Thanks, Lou On 9/22/2016 4:48 PM, Lou Berger wrote: > Hi, > > I think this question is for Donald/Dinesh. > > I'm a bit unsure how the new NHT code works with: > > 1) route servers/route reflectors > > 2) when routes for one AFI (say v6) is carried over a peering of a > different afi (say v4) > > Can someone explain? > > Thanks, > > Lou > > > ___ > Quagga-dev mailing list > Quagga-dev@lists.quagga.net > https://lists.quagga.net/mailman/listinfo/quagga-dev > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16109] Question/issue on BGP NHT in 8/proposed/ff
Hi, I think this question is for Donald/Dinesh. I'm a bit unsure how the new NHT code works with: 1) route servers/route reflectors 2) when routes for one AFI (say v6) is carried over a peering of a different afi (say v4) Can someone explain? Thanks, Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16106] Re: round-8: tentative proposed head for master
fyi - this branch looks pretty good -- on our end all execution tests and valgrind exits cleanly, but memstats does not exit cleanly (see below). I suspect this has to do with order of cleanup and nothing substantive as valgrind is clean. Nice to (almost) have a new stable master! Lou - Leaked type: Temporary memory : 92 blocks Leaked type: Temporary memory : 1 blocks Leaked type: Temporary memory : 91 blocks Leaked type: Temporary memory : 66 blocks On 9/13/2016 5:54 AM, p...@jakma.org wrote: > Hi Martin, > > I've got a head of commits that pass your CI (tested via the auto-merge > feature) at: > > > http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile/patch-tracking/8/proposed/ff-2016091301 > > It's 173 commits odd. Including a CVE fix for an issue with MRT dumping > that can lead to an ABRT. > > I believe you wanted to do a final full compliance run, before > integration to master. After which we can do a release, and attack the > next set of patches (which may also be a larger set than ideal, cause of > the length of r8, but still more manageable - so we should converge on a > more sane, faster turn-around on these soon). > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16105] Re: round-8: tentative proposed head for master
Paul, it would be good have dev/automerge updated to match the release (once it happens -- hopefully soon) so that we / others can update their patches to the latest known good point and then submit pull request to get CI and an integrated branch... I hope the the last two releases have convinced you that we need to do something different WRT patch integration. Lou On 9/21/2016 8:54 AM, Paul Jakma wrote: > On Wed, 21 Sep 2016, Lou Berger wrote: > >> Paul, >> >> does 8/proposed/ff-<...> match dev/automerge? If not can you submit a >> pull request to get them aligned? > I'm just pushing ff-... - they get CIed automatically (it'll poll > itself, or when I visit the right web-page for the branch). > > At the moment, everything seems to be fine, except some weird > corner-case interactions between the tests and BGP and > collision-detection. One test fails without the OPEN reform patches, but > with them another one fails. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16103] Re: round-8: tentative proposed head for master
Paul, does 8/proposed/ff-<...> match dev/automerge? If not can you submit a pull request to get them aligned? Thanks, Lou On 9/15/2016 6:50 PM, Martin Winter wrote: > On 15 Sep 2016, at 8:48, Paul Jakma wrote: > >> On Thu, 15 Sep 2016, Paul Jakma wrote: >> >>> So how was it not in the OPEN? >> Sigh. It's cause the af config is not transferred to accept peer. > So I assume you found this issue? > Or do you still need help reproducing the problem? > > - Martin > > ___ > Quagga-dev mailing list > Quagga-dev@lists.quagga.net > https://lists.quagga.net/mailman/listinfo/quagga-dev > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16072] Re: [PATCH] bgpd: fix leak introduced by (peer) nexthop tracking
On 9/7/2016 8:47 AM, Lou Berger wrote: > This is great. I'll run our basic ~15 minute regression with valgrind > against it and let you know the results. The only think I'm seeing is: 2016/09/07 08:54:38 BGP: Terminating on signal bgpd: memstats: Current memory utilization in module LIB: bgpd: memstats: Temporary memory : 66 bgpd: memstats: NOTE: If configuration exists, utilization may be expected. *but* valgrind reports no leaks - so am a bit confused why memstat is reporting allocation, valgrind isn't seeing this. Perhaps cleanup ordering has changed relative to memstats printing. If it's still there once the branch has stabilized, I can look into it... Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16070] Re: [PATCH] bgpd: fix leak introduced by (peer) nexthop tracking
On 9/7/2016 6:53 AM, Paul Jakma wrote: > On Tue, 6 Sep 2016, Paul Jakma wrote: > >> I have another patch that makes valgrind clean for me on the bncs, but >> I left the bncs in place while the peer is defined. See patch posted >> just there. > Ok, my variation on your fix (hope you don't mind me re-doing that not at all. > - I > noted what the leak was from your patch, humm, don't think this is right as all our patches are run against valgrind prior to submitting. Now, it does look like this came from an integration problem as we added the AFI and someone else added the new per afi processing. As both sets of patches were sitting in the patch list the current process has the integrator (release master) needing to find / fix these. This goes to process and how we need patches submitted against an active dev branch so that the submitter (who ever submitted last in this case) does the integration and can resolves any problems/leaks. This would be both more scalable and efficient as a single integrators (release master) doesn't need to learn all the changes/interactions. I'll be happy to resubmit (including pull request) our outstanding patches against the dev/automerge branch once /8 is done to ensure it integrates cleanly and passes martin's tests. > but started again to try work > out the refcounting) + your other leak fix + your AFI_ETHER patch (which > I havn't tested) + my OPEN/FSM fixes (CI with previous hit some > collision handling problem in bringing up sessions): > > - valgrinds clean from start to finish with some simple BGP sessions for >me. > > - passes Martin's torture testing > > Which is on top of the other stuff from r8 and NHT, etc. This is great. I'll run our basic ~15 minute regression with valgrind against it and let you know the results. Lou > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16050] Re: [PATCH] bgpd: fix leak introduced by (peer) nexthop tracking
Let's get the rest of the patches through regression and into automerge and then see how it does with valgrind... On 9/5/2016 12:51 PM, Paul Jakma wrote: > The question is whether there's actually a leak? ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16047] [PATCH] bgp: bgp_nexthop init/free AFI_ETHER related NH tables
--- bgpd/bgp_nexthop.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index 434b2cd..b8b3b93 100644 --- a/bgpd/bgp_nexthop.c +++ b/bgpd/bgp_nexthop.c @@ -1492,6 +1492,11 @@ bgp_scan_init (void) bgp_nexthop_cache_table[AFI_IP6] = cache1_table[AFI_IP6]; bgp_connected_table[AFI_IP6] = bgp_table_init (AFI_IP6, SAFI_UNICAST); + cache1_table[AFI_ETHER] = bgp_table_init (AFI_ETHER, SAFI_UNICAST); + cache2_table[AFI_ETHER] = bgp_table_init (AFI_ETHER, SAFI_UNICAST); + bgp_nexthop_cache_table[AFI_ETHER] = cache1_table[AFI_ETHER]; + bgp_connected_table[AFI_ETHER] = bgp_table_init (AFI_ETHER, SAFI_UNICAST); + /* Make BGP scan thread. */ bgp_scan_thread = thread_add_timer (bm->master, bgp_scan_timer, NULL, bgp_scan_interval); @@ -1534,6 +1539,19 @@ bgp_scan_finish (void) if (bgp_connected_table[AFI_IP6]) bgp_table_unlock (bgp_connected_table[AFI_IP6]); bgp_connected_table[AFI_IP6] = NULL; + + if (cache1_table[AFI_ETHER]) +bgp_table_unlock (cache1_table[AFI_ETHER]); + cache1_table[AFI_ETHER] = NULL; + + if (cache2_table[AFI_ETHER]) +bgp_table_unlock (cache2_table[AFI_ETHER]); + cache2_table[AFI_ETHER] = NULL; + + if (bgp_connected_table[AFI_ETHER]) +bgp_table_unlock (bgp_connected_table[AFI_ETHER]); + bgp_connected_table[AFI_ETHER] = NULL; + } void -- 2.1.3 ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16046] Re: [PATCH] bgpd: fix leak introduced by (peer) nexthop tracking
I'm testing a fix right now that inits (and frees ) AFI_ETHER tables... On 9/5/2016 11:46 AM, Paul Jakma wrote: > On Mon, 5 Sep 2016, Paul Jakma wrote: > >> Also, if I go back to the commit before, i.e. 'bgpd, zebra: Use next >> hop tracking for connected routes too' plus 'bgpd: Remove unused and >> leaking code' effectively, then it passes Martin's test but 'show ip >> nexthop' can still crash it: >> ==16194== Process terminating with default action of signal 6 (SIGABRT) >> ==16194==at 0x5A1B6F5: raise (in /usr/lib64/libc-2.23.so) >> ==16194==by 0x5A1D2F9: abort (in /usr/lib64/libc-2.23.so) >> ==16194==by 0x4E70EC0: core_handler (sigevent.c:242) >> ==16194==by 0x5A1B76F: ??? (in /usr/lib64/libc-2.23.so) >> ==16194==by 0x43A7C5: bgp_table_top (bgp_table.h:160) >> ==16194==by 0x43A7C5: show_ip_bgp_nexthop_table (bgp_nexthop.c:424) > This one is trivial it seems, just a missing null check in the loop in > the command. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16026] Re: Quagga project governance
June was 6-7 weeks ago (which is a bit more than a few days) and, we don't have a stable ff branch. I suggest that it's time to hit the process reset. now. This isn't to say that there hasn't been real work done (Almost all by Paul, on 8/proposed/ff, and Martin on dev/automerge), but we really a more saleable and deterministic process. At this point it seems to me that we should cut R1.1 based on the portion of 8/proposed/ff that is passing regression (which is in dev/automerge) and then move to the github pull request based approach outlined in https://lists.quagga.net/pipermail/quagga-dev/2016-August/016102.html . This way multiple folks can attack the backlog in parallel, and future submitters become responsible for error free integration. Clearly this last part will need some process discussions and we can have those based on the latest round of docs. E.g., we will need to think about one off patches that come in only to the list -- I'm willing martin can do this simply based on his patch reporting to patchworks and I bet he can even set it up that pull requests are reported to the mail list so the list can continue to track changes... Lou On 6/28/2016 11:26 AM, Vincent JARDIN wrote: > Le 28/06/2016 16:03, Lou Berger a écrit : >> On 06/28/2016 09:54 AM, Donald Sharp wrote: >>>> I >>>> won't be paying full attention to any review comments because I will >>>> be on vacation the 1-10 of July, but hope to resolve all issues upon >>>> getting back in a timely manner. >> Being forever the optimist - we'll plan/hope to have all issues resolved >> *before* you return! > I can hardly follow all the emails, there are too many! > > My take is simple, from previous Paul's comment I do see: >http://patchwork.quagga.net/bundle/paul/round-8/ > and > > http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile%2Fpatch-tracking%2F8%2Fproposed%2Fff > > if that's show up into head within few days, we'll *all be happy, am I > correct*? Then, we'll be able to slowly restart to be back to a normal > project/life that can evolve based on recent > Lou/Martin/Paul/Donal/myself/Olivier/etc. comments. > > Thank you, >Vincent > > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16019] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
What's the alternative - a broken base/mainline? How does this help anything? Lou On 8/5/2016 9:42 AM, Paul Jakma wrote: > On Fri, 5 Aug 2016, Lou Berger wrote: > >> As long as we skip patches that break existing protocols/functionality, > So does someone want to do the required collating, sifting and > cherry-picking/rebasing of stuff, combined with pull requests to > Martin's auto-CI branch to find the errant patches? > > I'm a bit limited on cycles for next 2 weeks. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16016] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
As long as we skip patches that break existing protocols/functionality, I agree. -- Let's get things moving! On 8/5/2016 7:49 AM, Nick Hilliard wrote: > On 5 Aug 2016, at 12:18, Paul Jakma wrote: >> If there are no conceptual objections to what's queued in ff, then we >> continue with merging and fix the bugs later. No? > Wfm. There's too much queued up in the commit queues to block on these > problems. Pressing on will not impede fixes. > > Nick > > > > ___ > Quagga-dev mailing list > Quagga-dev@lists.quagga.net > https://lists.quagga.net/mailman/listinfo/quagga-dev > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16015] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
Paul, Imo we should leverage regression testing to identify which patches / patch sets introduce problems and either skip those or get them fixed before including them. Set another way, for my prospective, pushing the portion of proposed 8 to master that passes Martin's regression tests is reasonable; but pushing code known to introduce problems isn't. What about working with Martin to automatically go through the patch sets and any that pass bring into his Auto branch. Then you or the patch authors can work to resolve the problem introduced in the specific patch / patch sets. What do you think? Lou On August 5, 2016 5:19:12 AM Paul Jakma wrote: On Thu, 4 Aug 2016, Martin Winter wrote: If someone has any other ideas, then please speak up. If there are no conceptual objections to what's queued in ff, then we continue with merging and fix the bugs later. No? regards, -- Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A Fortune: I know the answer! The answer lies within the heart of all mankind! The answer is twelve? I think I'm in the wrong building. -- Charles Schulz ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16010] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
On August 3, 2016 4:16:02 PM "Martin Winter" wrote: ... Great.. so building the code now works on all the OS’es tested by our CI system. Now we need the routing protocols to work :-( See https://ci1.netdef.org/browse/QUAGGA-QMASTER23-1 In my few basic checks, one of the ISIS tests, 2 of the IPv4 BGP tests and one of the IPv6 BGP tests fail. :-( So what's the plan to resolve the issues? More step by step regression testing, something else? Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16007] Re: [PATCH] bgp_route.c fix memory leak Introduced by: Author: Pradosh Mohapatra Date: Mon Nov 9 20:21:41 2015 -0500
8/proposed/ff-2016080201 looks good in our (bgpd only) testing.
Lou
On 8/2/2016 8:54 AM, Lou Berger wrote:
> Yes. I believe it wasn't there until the second version of the branch, so
> didn't see it at first. But the second patch is still needed. Well I would
> like to see for regression results on it from Martin, as I only tested bgpd
> and it does have some zebra implications.
>
> Lou
>
>
>
>
> On August 2, 2016 8:50:11 AM Paul Jakma wrote:
>
>> I'd already applied this based on the other email, made up my own commit
>> subject for you:
>>
>> http://git.savannah.gnu.org/gitweb/?p=quagga.git;a=commitdiff;h=811586ad4cfedd5b5d6d6c1f8a2d674b84ba637e
>>
>> Presumably that's OK?
>>
>> regards,
>>
>> Paul
>>
>> On Mon, 1 Aug 2016, Lou Berger wrote:
>>
>>>bgpd, doc, lib, zebra: nexthop-tracking in zebra
>>> ---
>>> bgpd/bgp_route.c | 5 -
>>> 1 file changed, 5 deletions(-)
>>>
>>> diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
>>> index a88a022..4cb6c14 100644
>>> --- a/bgpd/bgp_route.c
>>> +++ b/bgpd/bgp_route.c
>>> @@ -3778,14 +3778,9 @@ bgp_static_withdraw (struct bgp *bgp, struct prefix
>>> *p, afi_t afi,
>>> {
>>> struct bgp_node *rn;
>>> struct bgp_info *ri;
>>> - struct bgp_info *new;
>>>
>>> /* Make new BGP info. */
>>> rn = bgp_node_get (bgp->rib[afi][safi], p);
>>> - new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_STATIC, bgp->peer_self,
>>> - bgp_attr_default_intern(BGP_ORIGIN_IGP), rn);
>>> -
>>> - SET_FLAG (new->flags, BGP_INFO_VALID);
>>>
>>> /* Check selected route and self inserted route. */
>>> for (ri = rn->info; ri; ri = ri->next)
>>>
>> --
>> Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A
>> Fortune:
>> Every word is like an unnecessary stain on silence and nothingness.
>> -- Beckett
>>
>
>
> ___
> Quagga-dev mailing list
> Quagga-dev@lists.quagga.net
> https://lists.quagga.net/mailman/listinfo/quagga-dev
>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 16002] Re: [PATCH] bgp_route.c fix memory leak Introduced by: Author: Pradosh Mohapatra Date: Mon Nov 9 20:21:41 2015 -0500
Yes. I believe it wasn't there until the second version of the branch, so
didn't see it at first. But the second patch is still needed. Well I would
like to see for regression results on it from Martin, as I only tested bgpd
and it does have some zebra implications.
Lou
On August 2, 2016 8:50:11 AM Paul Jakma wrote:
I'd already applied this based on the other email, made up my own commit
subject for you:
http://git.savannah.gnu.org/gitweb/?p=quagga.git;a=commitdiff;h=811586ad4cfedd5b5d6d6c1f8a2d674b84ba637e
Presumably that's OK?
regards,
Paul
On Mon, 1 Aug 2016, Lou Berger wrote:
bgpd, doc, lib, zebra: nexthop-tracking in zebra
---
bgpd/bgp_route.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index a88a022..4cb6c14 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -3778,14 +3778,9 @@ bgp_static_withdraw (struct bgp *bgp, struct prefix
*p, afi_t afi,
{
struct bgp_node *rn;
struct bgp_info *ri;
- struct bgp_info *new;
/* Make new BGP info. */
rn = bgp_node_get (bgp->rib[afi][safi], p);
- new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_STATIC, bgp->peer_self,
- bgp_attr_default_intern(BGP_ORIGIN_IGP), rn);
-
- SET_FLAG (new->flags, BGP_INFO_VALID);
/* Check selected route and self inserted route. */
for (ri = rn->info; ri; ri = ri->next)
--
Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A
Fortune:
Every word is like an unnecessary stain on silence and nothingness.
-- Beckett
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15995] Re: CI Testresult: FAILED (Re: [quagga-dev, 15986] bgp_route.c fix memory leak Introduced by: Author: Pradosh Mohapatra Date: Mon Nov 9 20:21:41 2015
ahh, perhaps was a rebase sometime today -- is a duplicate of this one... On 8/1/2016 5:51 PM, Martin Winter wrote: > On 1 Aug 2016, at 14:43, Lou Berger wrote: > >> Try applying to volatile/patch-tracking/8/proposed/ff-2016080101 > That’s what I did. Commit 27185a96 is the last commit on > volatile/patch-tracking/8/proposed/ff-2016080101 > (Commit “*: Remove some for statement declarations” by Paul) > > - Martin > >> (sigh for want of a development mainline...) >> >> On 8/1/2016 5:38 PM, Martin Winter wrote: >>> Resubmitted to be tested on top of commit 27185a96 (latest in >>> proposed/8/ff branch) >>> >>> Testrun is at https://ci1.netdef.org/browse/QUAGGA-QPWORK-344 >>> >>> But still failed: >>> Git Patch: Using "git apply" for patch 2030 >>> error: patch failed: bgpd/bgp_route.c:3778 >>> error: bgpd/bgp_route.c: patch does not apply >>> >>> - Martin >>> >>> On 1 Aug 2016, at 14:23, Lou Berger wrote: >>> >>>> no surprise here -- this patch applies to 8/proposed/ff >>>> >>>> Lou >>>> >>>> On 8/1/2016 5:20 PM, cisys...@netdef.org wrote: >>>>> Continous Integration Result: FAILED >>>>> >>>>> See below for issues. >>>>> This is an EXPERIMENTAL automated CI system. >>>>> For questions and feedback, feel free to email >>>>> Martin Winter . >>>>> >>>>> Patches applied : >>>>> Patchwork 2030: http://patchwork.quagga.net/patch/2030 >>>>>[quagga-dev,15986] bgp_route.c fix memory leak Introduced by: >>>>> Author: Pradosh Mohapatra Date: Mon >>>>> Nov 9 20:21:41 2015 -0500 >>>>> Tested on top of Git : 5f67888 (as of 20160429.234845 UTC) >>>>> CI System Testrun URL: >>>>> https://ci1.netdef.org/browse/QUAGGA-QPWORK-343/ >>>>> >>>>> >>>>> Get source and apply patch from patchwork: Failed >>>>> >>>>>> Applying Patchwork patch 2030 >>>>>> >>>>>> Git Patch: Using "git apply" for patch 2030 >>>>>> error: patch failed: bgpd/bgp_route.c:3778 >>>>>> error: bgpd/bgp_route.c: patch does not apply >>>>> Regards, >>>>> NetDEF/OpenSourceRouting Continous Integration (CI) System >>>>> >>>>> --- >>>>> OpenSourceRouting.org is a project of the Network Device Education >>>>> Foundation, >>>>> For more information, see www.netdef.org and >>>>> www.opensourcerouting.org >>>>> For questions in regards to this CI System, contact Martin Winter, >>>>> mwin...@netdef.org >>>>> ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15993] [PATCH] bgpd: fix leak introduced by (peer) nexthop tracking
---
bgpd/bgp_fsm.c | 3 +++
bgpd/bgp_nht.c | 39 +++
bgpd/bgp_nht.h | 8
3 files changed, 50 insertions(+)
diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index d84a865..8001e3b 100644
--- a/bgpd/bgp_fsm.c
+++ b/bgpd/bgp_fsm.c
@@ -457,6 +457,8 @@ bgp_stop (struct peer *peer)
BGP_EVENT_FLUSH (peer);
}
+ bgp_drop_peer_nexthop(family2afi(peer->su.sa.sa_family), peer);
+
/* Increment Dropped count. */
if (peer->status == Established)
{
@@ -721,6 +723,7 @@ bgp_start (struct peer *peer)
bgp_find_or_add_nexthop(family2afi(peer->su.sa.sa_family), NULL, peer,
connected);
+
status = bgp_connect (peer);
switch (status)
diff --git a/bgpd/bgp_nht.c b/bgpd/bgp_nht.c
index 34b5fd1..2549e9e 100644
--- a/bgpd/bgp_nht.c
+++ b/bgpd/bgp_nht.c
@@ -90,6 +90,45 @@ bgp_unlink_nexthop (struct bgp_info *path)
}
}
+void
+bgp_drop_peer_nexthop (afi_t afi, struct peer *peer)
+{
+ struct bgp_node *rn;
+ struct prefix p;
+
+ if (peer == NULL)
+return;
+
+ if (afi == AFI_IP)
+{
+ p.family = AF_INET;
+ p.prefixlen = IPV4_MAX_BITLEN;
+ p.u.prefix4 = peer->su.sin.sin_addr;
+}
+ else if (afi == AFI_IP6)
+{
+ p.family = AF_INET6;
+ p.prefixlen = IPV6_MAX_BITLEN;
+ p.u.prefix6 = peer->su.sin6.sin6_addr;
+}
+ else
+return; /* unknown AFI */
+
+ rn = bgp_node_get (bgp_nexthop_cache_table[afi], &p);
+
+ if (rn->info)
+{
+ struct bgp_nexthop_cache *bnc;
+ bnc = rn->info;
+ unregister_nexthop(bnc);
+ bnc->node->info = NULL;
+ bgp_unlock_node(bnc->node);
+ bnc_free(bnc);
+}
+
+ bgp_unlock_node (rn);
+}
+
int
bgp_find_or_add_nexthop (afi_t afi, struct bgp_info *ri, struct peer *peer,
int connected)
diff --git a/bgpd/bgp_nht.h b/bgpd/bgp_nht.h
index 2bced7f..08e7019 100644
--- a/bgpd/bgp_nht.h
+++ b/bgpd/bgp_nht.h
@@ -55,4 +55,12 @@ extern int bgp_find_or_add_nexthop(afi_t a, struct bgp_info
*p,
*/
extern void bgp_unlink_nexthop(struct bgp_info *p);
+/**
+ * bgp_unlink_nexthop() - Drop the nexthop object based on a peer.
+ * ARGUMENTS:
+ * a - afi: AFI_IP or AF_IP6
+ * peer - The BGP peer associated with this NHT
+ */
+extern void bgp_drop_peer_nexthop(afi_t afi, struct peer *peer);
+
#endif /* _BGP_NHT_H */
--
2.1.3
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15992] Re: CI Testresult: FAILED (Re: [quagga-dev, 15986] bgp_route.c fix memory leak Introduced by: Author: Pradosh Mohapatra Date: Mon Nov 9 20:21:41 2015
Try applying to volatile/patch-tracking/8/proposed/ff-2016080101 Lou (sigh for want of a development mainline...) On 8/1/2016 5:38 PM, Martin Winter wrote: > Resubmitted to be tested on top of commit 27185a96 (latest in > proposed/8/ff branch) > > Testrun is at https://ci1.netdef.org/browse/QUAGGA-QPWORK-344 > > But still failed: > Git Patch: Using "git apply" for patch 2030 > error: patch failed: bgpd/bgp_route.c:3778 > error: bgpd/bgp_route.c: patch does not apply > > - Martin > > On 1 Aug 2016, at 14:23, Lou Berger wrote: > >> no surprise here -- this patch applies to 8/proposed/ff >> >> Lou >> >> On 8/1/2016 5:20 PM, cisys...@netdef.org wrote: >>> Continous Integration Result: FAILED >>> >>> See below for issues. >>> This is an EXPERIMENTAL automated CI system. >>> For questions and feedback, feel free to email >>> Martin Winter . >>> >>> Patches applied : >>> Patchwork 2030: http://patchwork.quagga.net/patch/2030 >>>[quagga-dev,15986] bgp_route.c fix memory leak Introduced by: >>> Author: Pradosh Mohapatra Date: Mon >>> Nov 9 20:21:41 2015 -0500 >>> Tested on top of Git : 5f67888 (as of 20160429.234845 UTC) >>> CI System Testrun URL: >>> https://ci1.netdef.org/browse/QUAGGA-QPWORK-343/ >>> >>> >>> Get source and apply patch from patchwork: Failed >>> >>>> Applying Patchwork patch 2030 >>>> >>>> Git Patch: Using "git apply" for patch 2030 >>>> error: patch failed: bgpd/bgp_route.c:3778 >>>> error: bgpd/bgp_route.c: patch does not apply >>> Regards, >>> NetDEF/OpenSourceRouting Continous Integration (CI) System >>> >>> --- >>> OpenSourceRouting.org is a project of the Network Device Education >>> Foundation, >>> For more information, see www.netdef.org and >>> www.opensourcerouting.org >>> For questions in regards to this CI System, contact Martin Winter, >>> mwin...@netdef.org >>> ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15989] Re: CI Testresult: FAILED (Re: [quagga-dev, 15986] bgp_route.c fix memory leak Introduced by: Author: Pradosh Mohapatra Date: Mon Nov 9 20:21:41 2015
no surprise here -- this patch applies to 8/proposed/ff Lou On 8/1/2016 5:20 PM, cisys...@netdef.org wrote: > Continous Integration Result: FAILED > > See below for issues. > This is an EXPERIMENTAL automated CI system. > For questions and feedback, feel free to email > Martin Winter . > > Patches applied : > Patchwork 2030: http://patchwork.quagga.net/patch/2030 >[quagga-dev,15986] bgp_route.c fix memory leak Introduced by: Author: > Pradosh Mohapatra Date: Mon Nov 9 20:21:41 > 2015 -0500 > Tested on top of Git : 5f67888 (as of 20160429.234845 UTC) > CI System Testrun URL: https://ci1.netdef.org/browse/QUAGGA-QPWORK-343/ > > > Get source and apply patch from patchwork: Failed > >> Applying Patchwork patch 2030 >> >> Git Patch: Using "git apply" for patch 2030 >> error: patch failed: bgpd/bgp_route.c:3778 >> error: bgpd/bgp_route.c: patch does not apply > Regards, > NetDEF/OpenSourceRouting Continous Integration (CI) System > > --- > OpenSourceRouting.org is a project of the Network Device Education Foundation, > For more information, see www.netdef.org and www.opensourcerouting.org > For questions in regards to this CI System, contact Martin Winter, > mwin...@netdef.org > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15987] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
I'll send patches to the list for these (1st one already sent). Lou On 8/1/2016 12:57 PM, Paul Jakma wrote: > - bgpd: Remove unused and leaking code > >(Lou's fix, left the other leak alone for now (?)) ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15986] [PATCH] bgp_route.c fix memory leak Introduced by: Author: Pradosh Mohapatra Date: Mon Nov 9 20:21:41 2015 -0500
bgpd, doc, lib, zebra: nexthop-tracking in zebra
---
bgpd/bgp_route.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index a88a022..4cb6c14 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -3778,14 +3778,9 @@ bgp_static_withdraw (struct bgp *bgp, struct prefix *p,
afi_t afi,
{
struct bgp_node *rn;
struct bgp_info *ri;
- struct bgp_info *new;
/* Make new BGP info. */
rn = bgp_node_get (bgp->rib[afi][safi], p);
- new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_STATIC, bgp->peer_self,
- bgp_attr_default_intern(BGP_ORIGIN_IGP), rn);
-
- SET_FLAG (new->flags, BGP_INFO_VALID);
/* Check selected route and self inserted route. */
for (ri = rn->info; ri; ri = ri->next)
--
2.1.3
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15953] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
Paul,
On 7/26/2016 2:53 PM, Paul Jakma wrote:
> On Tue, 26 Jul 2016, Lou Berger wrote:
>
>>>> Paul,
>>>>How do you want to proceed?
>>> Would it be easy to fix with a cleanup patch?
>> Do you want it formally? Informally, as tested:
> I was thinking more of your comment on the globals? :) Want to fix that
> now or later?
I think later is fine given current VRF support.
> And who wants to do it?
I think it's the first one who gets there. I looked at the cumulus
github and see they have it fixed. I'd say the first one who sees this
problem in a feature they are working on fix it if the cumulus doesn't
beat them. Alternatively next hop tracking can be stripped out, but I'd
prefer to have the code feature set increase, even if incrementally. -
Just one opinion and to be clear, I don't feel strongly either way in
this case.
> On the below, I'd just delete it -
okay,
Thanks,
Lou
> though someone needs to look deeper
> into the bgp_find_or_add_nexthop removal? Donald?
>
> regards,
>
> Paul
>
>> diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
>> index d84a865..46562cd 100644
>> --- a/bgpd/bgp_fsm.c
>> +++ b/bgpd/bgp_fsm.c
>> @@ -718,9 +718,10 @@ bgp_start (struct peer *peer)
>> /* Register to be notified on peer up */
>> if ((peer->ttl == 1) || (peer->gtsm_hops == 1))
>> connected = 1;
>> -
>> +#if 0
>> bgp_find_or_add_nexthop(family2afi(peer->su.sa.sa_family), NULL, peer,
>> connected);
>> +#endif
>> status = bgp_connect (peer);
>>
>> switch (status)
>> diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
>> index a88a022..a8801c5 100644
>> --- a/bgpd/bgp_route.c
>> +++ b/bgpd/bgp_route.c
>> @@ -3778,15 +3778,18 @@ bgp_static_withdraw (struct bgp *bgp, struct
>> prefix *p, afi_t afi,
>> {
>> struct bgp_node *rn;
>> struct bgp_info *ri;
>> +#if 0
>> struct bgp_info *new;
>> +#endif
>>
>> /* Make new BGP info. */
>> rn = bgp_node_get (bgp->rib[afi][safi], p);
>> +#if 0
>> new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_STATIC, bgp->peer_self,
>> bgp_attr_default_intern(BGP_ORIGIN_IGP), rn);
>>
>> SET_FLAG (new->flags, BGP_INFO_VALID);
>> -
>> +#endif
>> /* Check selected route and self inserted route. */
>> for (ri = rn->info; ri; ri = ri->next)
>> if (ri->peer == bgp->peer_self
>>
>>
>>
>>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15950] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
Paul,
On 7/26/2016 12:13 PM, Paul Jakma wrote:
> On Tue, 26 Jul 2016, Lou Berger wrote:
>
>> As far as I can tell new is not used/leaked -- any idea why this was
>> added (by Pradosh)?
> Unused code is easy to remove. ;)
>
>> This looks like is the result of bgp_start
>>
>> bgp_find_or_add_nexthop(family2afi(peer->su.sa.sa_family), NULL, peer,
>> connected);
>>
>> commenting it out also solves the leak, but can't say what impact this
>> has operationally.
>>
>> One additional comment: the use of globals in NHT (rather than per bgp
>> instance variables) is really ugly from the VRF standpoint and will
>> need to moved at some point.
> Would agree in principle, without remembering the code.
suspect this is a functional change, but have not investigated.
>
>> Paul,
>>How do you want to proceed?
> Would it be easy to fix with a cleanup patch?
Do you want it formally? Informally, as tested:
diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index d84a865..46562cd 100644
--- a/bgpd/bgp_fsm.c
+++ b/bgpd/bgp_fsm.c
@@ -718,9 +718,10 @@ bgp_start (struct peer *peer)
/* Register to be notified on peer up */
if ((peer->ttl == 1) || (peer->gtsm_hops == 1))
connected = 1;
-
+#if 0
bgp_find_or_add_nexthop(family2afi(peer->su.sa.sa_family), NULL, peer,
connected);
+#endif
status = bgp_connect (peer);
switch (status)
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index a88a022..a8801c5 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -3778,15 +3778,18 @@ bgp_static_withdraw (struct bgp *bgp, struct
prefix *p, afi_t afi,
{
struct bgp_node *rn;
struct bgp_info *ri;
+#if 0
struct bgp_info *new;
+#endif
/* Make new BGP info. */
rn = bgp_node_get (bgp->rib[afi][safi], p);
+#if 0
new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_STATIC, bgp->peer_self,
bgp_attr_default_intern(BGP_ORIGIN_IGP), rn);
SET_FLAG (new->flags, BGP_INFO_VALID);
-
+#endif
/* Check selected route and self inserted route. */
for (ri = rn->info; ri; ri = ri->next)
if (ri->peer == bgp->peer_self
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15945] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
Hi Donald, (Paul)
On 7/25/2016 7:55 PM, Donald Sharp wrote:
> I don't see anything else that is obvious, there are lots of other
> commits that touch code in bgp :(
Yes, but testing shows which patches introduces the leaks - sorry...
>
> Memory leak fix for BGP Nexthop leaking:
> https://github.com/CumulusNetworks/quagga/commit/7898cb4f5ef311589be9c3c9a3aab41d5b16b989
>
> donald
>
> On Wed, Jul 20, 2016 at 12:31 PM, Lou Berger wrote:
>> The branch passes our short regression (the longer test is running) with
>> the same memory leaks:
>>
>> per: https://lists.quagga.net/pipermail/quagga-dev/2016-July/015982.html
>>
>>
>> #1
>>
>> Author: Pradosh Mohapatra > <https://lists.quagga.net/mailman/listinfo/quagga-dev>>
>> Date: Mon Nov 9 20:21:41 2015 -0500
>>
>> bgpd, doc, lib, zebra: nexthop-tracking in zebra
>>
>> 0. Introduction
>>
>> This is the design specification for next hop tracking feature in
>> Quagga.
>> ...
>>
>> Leaked type: BGP route : 40 blocks
>> Leaked type: BGP extra attributes : 40 blocks
This is fixed by:
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index a88a022..a8801c5 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -3778,15 +3778,18 @@ bgp_static_withdraw (struct bgp *bgp, struct
prefix *p, afi_t afi,
{
struct bgp_node *rn;
struct bgp_info *ri;
+#if 0
struct bgp_info *new;
+#endif
/* Make new BGP info. */
rn = bgp_node_get (bgp->rib[afi][safi], p);
+#if 0
new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_STATIC, bgp->peer_self,
bgp_attr_default_intern(BGP_ORIGIN_IGP), rn);
SET_FLAG (new->flags, BGP_INFO_VALID);
-
+#endif
As far as I can tell new is not used/leaked -- any idea why this was
added (by Pradosh)?
>>
>> #2
>>
>>Author: Dinesh Dutt > <https://lists.quagga.net/mailman/listinfo/quagga-dev>>
>>Date: Tue May 19 17:47:21 2015 -0700
>>
>> bgpd, zebra: Use next hop tracking for connected routes too
>>
>> Allow next hop tracking to work with connected routes
>> And cleanup obsolete code in bgp_scan and bgp_import.
>>
>>
>> Leaked type: BGP nexthop : 7 blocks
This looks like is the result of bgp_start
bgp_find_or_add_nexthop(family2afi(peer->su.sa.sa_family), NULL, peer,
connected);
commenting it out also solves the leak, but can't say what impact this
has operationally.
One additional comment: the use of globals in NHT (rather than per bgp
instance variables) is really ugly from the VRF standpoint and will need
to moved at some point.
Paul,
How do you want to proceed?
Lou
>> Lou
>>
>> On 7/20/2016 5:34 PM, Paul Jakma wrote:
>>> Hi,
>>>
>>> I've shuffled in Donald's fix, and did the shuffling for Lou's testing.
>>> Published the rebased head to:
>>>
>>>volatile/patch-tracking/8/proposed/ff-2016072001
>>>
>>> Hopefully there's no further probs with it and we can bump master along
>>> to that head soon, and crank through the next batch.
>>>
>>> regards,
>>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15924] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
Id say give them / someone a chance to provide a fix... On July 20, 2016 10:01:47 PM Paul Jakma wrote: On Wed, 20 Jul 2016, Lou Berger wrote: The branch passes our short regression (the longer test is running) with the same memory leaks: per: https://lists.quagga.net/pipermail/quagga-dev/2016-July/015982.html #1 Author: Pradosh Mohapatra https://lists.quagga.net/mailman/listinfo/quagga-dev>> Date: Mon Nov 9 20:21:41 2015 -0500 bgpd, doc, lib, zebra: nexthop-tracking in zebra Author: Dinesh Dutt https://lists.quagga.net/mailman/listinfo/quagga-dev>> Date: Tue May 19 17:47:21 2015 -0700 bgpd, zebra: Use next hop tracking for connected routes too So do we drop those, or leave them in and wait for the fixes? I think Timo Teras' NHRP work depends on these, IIRC. regards, -- Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A Fortune: The easiest way to get the root password is to become system admin. -- Unknown source ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15921] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
The branch passes our short regression (the longer test is running) with the same memory leaks: per: https://lists.quagga.net/pipermail/quagga-dev/2016-July/015982.html #1 Author: Pradosh Mohapatra https://lists.quagga.net/mailman/listinfo/quagga-dev>> Date: Mon Nov 9 20:21:41 2015 -0500 bgpd, doc, lib, zebra: nexthop-tracking in zebra 0. Introduction This is the design specification for next hop tracking feature in Quagga. ... Leaked type: BGP route : 40 blocks Leaked type: BGP extra attributes : 40 blocks #2 Author: Dinesh Dutt https://lists.quagga.net/mailman/listinfo/quagga-dev>> Date: Tue May 19 17:47:21 2015 -0700 bgpd, zebra: Use next hop tracking for connected routes too Allow next hop tracking to work with connected routes And cleanup obsolete code in bgp_scan and bgp_import. Leaked type: BGP nexthop : 7 blocks Lou On 7/20/2016 5:34 PM, Paul Jakma wrote: > Hi, > > I've shuffled in Donald's fix, and did the shuffling for Lou's testing. > Published the rebased head to: > >volatile/patch-tracking/8/proposed/ff-2016072001 > > Hopefully there's no further probs with it and we can bump master along > to that head soon, and crank through the next batch. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15918] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
Hi Paul,
On 7/19/2016 12:00 PM, Paul Jakma wrote:
> On Wed, 13 Jul 2016, Lou Berger wrote:
>
>> The quick test with the offending patch removed looked good. It just
>> found the already reported memory leak. I've started a more
>> comprehensive test on whole branch -- will take a~2.5 hours to run.
> Cool.
>
> I'll shuffle that from 'ff' to a 'nits' branch. And also have a look at
> the 'make check' one Martin reported and see if about shuffling a fix in
> after it - or squashing one in.
thanks.
> I gather people want any such rebased head to have a new name ('ffv2'?),
> rather than re-using the old label for a new, non-fast-foward-mergable
> head?
v2 would be great.
>> I'm also rerunning the step-wise basic (4min) regression through every
>> other commit to find the origin of the memory leak. -- just 109 (out
>> of 160) remaining!
> Ouch.
Actually not really a big deal at 4 min a run.
> For specific issues, git bisect can help. The '-x' argument to git
> rebase is also good (I try do a -x 'make -j3' rebase to at least check
> compile works - though there have been some patch-series submitted that
> don't compile on a per-commit basis).
The overall regression is done, and was pretty painless to automate.
I'll publish the detailed results when I have some time -- not this
week though...
Lou
> regards,
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15919] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
Thanks! On 7/19/2016 12:03 PM, Paul Jakma wrote: >> > PS I rebased the following commits to be 1st in order to get the >> > regression environment running: > K, will do same. ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15909] Meet up @ ietf this week?
Anyone interested in meeting up this week? No particular agenda?. How about Thursday lunchtime in the hotel lobby? Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15900] bgpd memory leaks in 8/proposed/ff
looks like there are two different new leakers in proposed/8/ff (as identified by our quick/basic regression tests) #1 Author: Pradosh Mohapatra Date: Mon Nov 9 20:21:41 2015 -0500 bgpd, doc, lib, zebra: nexthop-tracking in zebra 0. Introduction This is the design specification for next hop tracking feature in Quagga. ... Leaked type: BGP route : 40 blocks Leaked type: BGP extra attributes : 40 blocks #2 Author: Dinesh Dutt Date: Tue May 19 17:47:21 2015 -0700 bgpd, zebra: Use next hop tracking for connected routes too Allow next hop tracking to work with connected routes And cleanup obsolete code in bgp_scan and bgp_import. Leaked type: BGP nexthop : 7 blocks ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15899] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
this passes the quick regression test...
On 7/13/2016 11:02 AM, Donald Sharp wrote:
> This should fix the issue.
>
> diff --git a/lib/thread.c b/lib/thread.c
> index a26b43a..6fcddd7 100644
> --- a/lib/thread.c
> +++ b/lib/thread.c
> @@ -790,7 +790,7 @@ funcname_thread_add_read_write (int dir, struct
> thread_master *m,
>else
> fdset = &m->writefd;
>
> - if (FD_ISSET (fd, &m->readfd))
> + if (FD_ISSET (fd, fdset))
> {
>zlog (NULL, LOG_WARNING, "There is already %s fd [%d]",
> (dir = THREAD_READ) ? "read" : "write", fd);
>
> On Wed, Jul 13, 2016 at 6:22 AM, Paul Jakma wrote:
>> Hi Lou,
>>
>> Thanks. Donald had another patch in that series, 0b697fa9281a62 / "ib: Add
>> ability to use poll() instead of select", but had indicated that was kind of
>> preliminary still and that he wanted to do further work on it.
>>
>> If you drop the below, do you find further probs?
>>
>> regards,
>>
>> Paul
>>
>>
>> On Wed, 13 Jul 2016, Lou Berger wrote:
>>
>>> This is patch that's breaking bgpd in 8/proposed/ff
>>>
>>> Author: Donald Sharp
>>> Date: Fri Mar 4 15:28:56 2016 -0500
>>>
>>>lib: Refactor read/write functionality
>>>
>>>Both the read and write functions used the same code
>>>slightly modified for reading and writing. Combine this
>>>code together.
>>>
>>>Signed-off-by: Donald Sharp
>>>
>>>Edited-by: Paul Jakma to retain the
>>>external library symbols, for ease of merging.
>>>
>>>
>>> On 7/12/2016 7:55 PM, Lou Berger wrote:
>>>> Just an update: we've hooked our regression system into the github
>>>> mirror and are now running minimal regression tests on bgpd. The tests
>>>> start at a commit and move to the head of the branch, commit by commit
>>>> -- pretty simple approach.
>>>>
>>>> Each run does a compile, basic adjacency checks, some unicast and VRF
>>>> route distributions and checks the results against a reference (known
>>>> good run). Each run takes about 4 minutes and there are about 160
>>>> commits in /8/proposed/ff - currently has about 150 to go.
>>>>
>>>> I'll provide an update once we have interesting results.
>>>>
>>>> Lou
>>>>
>>>> PS I rebased the following commits to be 1st in order to get the
>>>> regression environment running:
>>>>
>>>> Author: Lou Berger
>>>> Date: Tue May 17 07:10:37 2016 -0400
>>>>
>>>> bgpd: Add flag to not change e{u,g}id on startup and run as
>>>> unprivileged user
>>>>
>>>> * bgp_main.c: add -S / --skip_runas flag to not change effective
>>>> user/group
>>>> on start up. Enables bgpd to be run by unprivileged user.
>>>>
>>>> Author: Lou Berger
>>>> Date: Tue May 17 07:10:36 2016 -0400
>>>>
>>>> bgp: add "debug bgp allow-martians" next hops and related
>>>> code/commands
>>>>
>>>> Author: Lou Berger
>>>> Date: Tue May 17 12:19:51 2016 -0400
>>>>
>>>> lib: change command logging to be off by default
>>>>
>>>> * lib/vty.c: add 'log_command' to enable logging of vty commands
>>>> executed.
>>>> Default command logging to off.
>>>>
>>>>
>>>>
>>>> On 7/11/2016 5:44 AM, Lou Berger wrote:
>>>>>>> Any luck pinning down what commits are causing which issues for you?
>>>>>>>
>>>>> Not yet. Thinking I'll try to wire into our regression system in some
>>>>> way...
>>>>>
>>>
>>>
>>> ___
>>> Quagga-dev mailing list
>>> Quagga-dev@lists.quagga.net
>>> https://lists.quagga.net/mailman/listinfo/quagga-dev
>>>
>> --
>> Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A
>> Fortune:
>> If dolphins are so smart, why did Flipper work for television?
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15898] Re: [PATCH 10/10] bgpd: add L3/L2VPN Virtual Network Control feature
FYI we have pushed an updated rev of our patches that address comments received to date -- mainly bug fixes. We're not planning on any additional changes at the moment. The rebased (to be integrated version) is available at https://github.com/LabNConsulting/quagga-vnc/tree/patches/R1.0.20160315%2Bvnc/v4 v3 contains the changes from the previously reviewed version. We will submit as a replacement patch set as soon we can rebase based on proposed/8 Lou On 6/17/2016 1:31 PM, Lou Berger wrote: > Philippe, > > okay fixes pushed to v2 (including removing v6 ifdefs) to show changes > and v3* posted to show final results. We have a couple of more issues > to deal with (skiplist license and a bug fix we're testing). Once these > are resolved we'll submit a formal patch update. > > BTW V3 has the references you asked for in the log. > > * > https://github.com/LabNConsulting/quagga-vnc/tree/patches/R1.0.20160315%2Bvnc/v3 > > On 6/17/2016 10:10 AM, Lou Berger wrote: >> On 6/17/2016 9:15 AM, Philippe Guibert wrote: >>> On Thu, Jun 16, 2016 at 8:52 PM, Lou Berger wrote: >>> >>> Hello Lou, >>> >>>> Philippe, >>>> >>>> I've posted a fix to this (patch on patch) in >>>> >>>> https://github.com/LabNConsulting/quagga-vnc/commit/cd54370cb94d598aa95bd7561cc012200920d97a >>>> >>> I posted 2 minor comments: >>> >>> 1) The code has been compacted; however i fear this may lead to >>> confusion ( especially because you perform if() just behind #endif >>> macro). >>> instead of: >>> >>> #if ENABLE_BGP_VNC >>> if (v != RD_TYPE_VNC_ETH) >>> #endif >>> v |= (u_int16_t) *pnt; >>> >>> I would do: >>> >>> #if ENABLE_BGP_VNC >>> if (v != RD_TYPE_VNC_ETH) >>>v |= (u_int16_t) *pnt; >>> #else >>> v |= (u_int16_t) *pnt; >>> #endif >>> >>> IMHO, I think this brings more clarity about the algorithm in place. >> I considered this, but I hate duplicate code so came down on the other >> side. But I was on the fence, so will make this change. >> >> >>> 2) duplicate encode_rd stuff >>>> this was covered in the v2 patch just sent to the list. >>> point 2 is resolved by v2 patch then. >>> >>>> okay, will post a pointer to v3 once we resolve the 2 minor comments. >>> waiting for v3 version with point 1) fix to ack. >> Thanks! >> Lou >>>> Thanks, >>>> >>> Thanks, >>> >>> Philippe >>> >> >> ___ >> Quagga-dev mailing list >> Quagga-dev@lists.quagga.net >> https://lists.quagga.net/mailman/listinfo/quagga-dev >> > > > ___ > Quagga-dev mailing list > Quagga-dev@lists.quagga.net > https://lists.quagga.net/mailman/listinfo/quagga-dev > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15897] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
Paul,
On 7/13/2016 6:46 AM, Lou Berger wrote:
> Paul,
>
> The quick test with the offending patch removed looked good. It just
> found the already reported memory leak. I've started a more
> comprehensive test on whole branch -- will take a~2.5 hours to run.
BGPd (VNC) regression passed with the offending patch omitted. The only
problem seen was the memory leak:
Leaked type: BGP nexthop : 7 blocks0 1
Leaked type: BGP route : 40 blocks 0 1
Leaked type: BGP extra attributes : 40 blocks 0 1
> I'm also rerunning the step-wise basic (4min) regression through every
> other commit to find the origin of the memory leak. -- just 109 (out of
> 160) remaining!
The next issue is:
isis_vty.o: In function `no_dynamic_hostname':
isis_vty.c:(.text+0x15b9): undefined reference to
`isis_area_dynhostname_set'
with
Author: David Lamparter
Date: Tue May 24 18:26:48 2016 +0200
isisd: API: basic circuit config
Create isis_vty.c and start moving off CLI functions into that. These
then call newly-added "nice" API wrappers.
Patch contains significant work authored by Christian Franke.
Signed-off-by: David Lamparter
This is fixed by the subsequent isisd testing, so I'm testing after that
patch set in search of the source of the leak.
Lou
> Lou
>
> On 7/13/2016 6:22 AM, Paul Jakma wrote:
>> Hi Lou,
>>
>> Thanks. Donald had another patch in that series, 0b697fa9281a62 / "ib:
>> Add ability to use poll() instead of select", but had indicated that was
>> kind of preliminary still and that he wanted to do further work on it.
>>
>> If you drop the below, do you find further probs?
>>
>> regards,
>>
>> Paul
>>
>> On Wed, 13 Jul 2016, Lou Berger wrote:
>>
>>> This is patch that's breaking bgpd in 8/proposed/ff
>>>
>>> Author: Donald Sharp
>>> Date: Fri Mar 4 15:28:56 2016 -0500
>>>
>>>lib: Refactor read/write functionality
>>>
>>>Both the read and write functions used the same code
>>>slightly modified for reading and writing. Combine this
>>>code together.
>>>
>>>Signed-off-by: Donald Sharp
>>>
>>>Edited-by: Paul Jakma to retain the
>>>external library symbols, for ease of merging.
>>>
>>>
>>> On 7/12/2016 7:55 PM, Lou Berger wrote:
>>>> Just an update: we've hooked our regression system into the github
>>>> mirror and are now running minimal regression tests on bgpd. The tests
>>>> start at a commit and move to the head of the branch, commit by commit
>>>> -- pretty simple approach.
>>>>
>>>> Each run does a compile, basic adjacency checks, some unicast and VRF
>>>> route distributions and checks the results against a reference (known
>>>> good run). Each run takes about 4 minutes and there are about 160
>>>> commits in /8/proposed/ff - currently has about 150 to go.
>>>>
>>>> I'll provide an update once we have interesting results.
>>>>
>>>> Lou
>>>>
>>>> PS I rebased the following commits to be 1st in order to get the
>>>> regression environment running:
>>>>
>>>> Author: Lou Berger
>>>> Date: Tue May 17 07:10:37 2016 -0400
>>>>
>>>> bgpd: Add flag to not change e{u,g}id on startup and run as
>>>> unprivileged user
>>>>
>>>> * bgp_main.c: add -S / --skip_runas flag to not change effective
>>>> user/group
>>>> on start up. Enables bgpd to be run by unprivileged user.
>>>>
>>>> Author: Lou Berger
>>>> Date: Tue May 17 07:10:36 2016 -0400
>>>>
>>>> bgp: add "debug bgp allow-martians" next hops and related code/commands
>>>>
>>>> Author: Lou Berger
>>>> Date: Tue May 17 12:19:51 2016 -0400
>>>>
>>>> lib: change command logging to be off by default
>>>>
>>>> * lib/vty.c: add 'log_command' to enable logging of vty commands
>>>> executed.
>>>> Default command logging to off.
>>>>
>>>>
>>>>
>>>> On 7/11/2016 5:44 AM, Lou Berger wrote:
>>>>>>> Any luck pinning down what commits are causing which issues for you?
>>>>>>>
>>>>> Not yet. Thinking I'll try to wire into our regression system in some
>>>>> way...
>>>>>
>>>
>>> ___
>>> Quagga-dev mailing list
>>> Quagga-dev@lists.quagga.net
>>> https://lists.quagga.net/mailman/listinfo/quagga-dev
>>>
>
> ___
> Quagga-dev mailing list
> Quagga-dev@lists.quagga.net
> https://lists.quagga.net/mailman/listinfo/quagga-dev
>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15894] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
Just an update: we've hooked our regression system into the github
mirror and are now running minimal regression tests on bgpd. The tests
start at a commit and move to the head of the branch, commit by commit
-- pretty simple approach.
Each run does a compile, basic adjacency checks, some unicast and VRF
route distributions and checks the results against a reference (known
good run). Each run takes about 4 minutes and there are about 160
commits in /8/proposed/ff - currently has about 150 to go.
I'll provide an update once we have interesting results.
Lou
PS I rebased the following commits to be 1st in order to get the
regression environment running:
Author: Lou Berger
Date: Tue May 17 07:10:37 2016 -0400
bgpd: Add flag to not change e{u,g}id on startup and run as
unprivileged user
* bgp_main.c: add -S / --skip_runas flag to not change effective
user/group
on start up. Enables bgpd to be run by unprivileged user.
Author: Lou Berger
Date: Tue May 17 07:10:36 2016 -0400
bgp: add "debug bgp allow-martians" next hops and related code/commands
Author: Lou Berger
Date: Tue May 17 12:19:51 2016 -0400
lib: change command logging to be off by default
* lib/vty.c: add 'log_command' to enable logging of vty commands
executed.
Default command logging to off.
On 7/11/2016 5:44 AM, Lou Berger wrote:
>> > Any luck pinning down what commits are causing which issues for you?
>> >
> Not yet. Thinking I'll try to wire into our regression system in some way...
>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15892] bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)
Paul,
The quick test with the offending patch removed looked good. It just
found the already reported memory leak. I've started a more
comprehensive test on whole branch -- will take a~2.5 hours to run.
I'm also rerunning the step-wise basic (4min) regression through every
other commit to find the origin of the memory leak. -- just 109 (out of
160) remaining!
Lou
On 7/13/2016 6:22 AM, Paul Jakma wrote:
> Hi Lou,
>
> Thanks. Donald had another patch in that series, 0b697fa9281a62 / "ib:
> Add ability to use poll() instead of select", but had indicated that was
> kind of preliminary still and that he wanted to do further work on it.
>
> If you drop the below, do you find further probs?
>
> regards,
>
> Paul
>
> On Wed, 13 Jul 2016, Lou Berger wrote:
>
>> This is patch that's breaking bgpd in 8/proposed/ff
>>
>> Author: Donald Sharp
>> Date: Fri Mar 4 15:28:56 2016 -0500
>>
>>lib: Refactor read/write functionality
>>
>>Both the read and write functions used the same code
>>slightly modified for reading and writing. Combine this
>>code together.
>>
>>Signed-off-by: Donald Sharp
>>
>>Edited-by: Paul Jakma to retain the
>>external library symbols, for ease of merging.
>>
>>
>> On 7/12/2016 7:55 PM, Lou Berger wrote:
>>> Just an update: we've hooked our regression system into the github
>>> mirror and are now running minimal regression tests on bgpd. The tests
>>> start at a commit and move to the head of the branch, commit by commit
>>> -- pretty simple approach.
>>>
>>> Each run does a compile, basic adjacency checks, some unicast and VRF
>>> route distributions and checks the results against a reference (known
>>> good run). Each run takes about 4 minutes and there are about 160
>>> commits in /8/proposed/ff - currently has about 150 to go.
>>>
>>> I'll provide an update once we have interesting results.
>>>
>>> Lou
>>>
>>> PS I rebased the following commits to be 1st in order to get the
>>> regression environment running:
>>>
>>> Author: Lou Berger
>>> Date: Tue May 17 07:10:37 2016 -0400
>>>
>>> bgpd: Add flag to not change e{u,g}id on startup and run as
>>> unprivileged user
>>>
>>> * bgp_main.c: add -S / --skip_runas flag to not change effective
>>> user/group
>>> on start up. Enables bgpd to be run by unprivileged user.
>>>
>>> Author: Lou Berger
>>> Date: Tue May 17 07:10:36 2016 -0400
>>>
>>> bgp: add "debug bgp allow-martians" next hops and related code/commands
>>>
>>> Author: Lou Berger
>>> Date: Tue May 17 12:19:51 2016 -0400
>>>
>>> lib: change command logging to be off by default
>>>
>>> * lib/vty.c: add 'log_command' to enable logging of vty commands
>>> executed.
>>> Default command logging to off.
>>>
>>>
>>>
>>> On 7/11/2016 5:44 AM, Lou Berger wrote:
>>>>>> Any luck pinning down what commits are causing which issues for you?
>>>>>>
>>>> Not yet. Thinking I'll try to wire into our regression system in some
>>>> way...
>>>>
>>
>>
>> ___
>> Quagga-dev mailing list
>> Quagga-dev@lists.quagga.net
>> https://lists.quagga.net/mailman/listinfo/quagga-dev
>>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15890] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
[also a resend...]
This is patch that's breaking bgpd in 8/proposed/ff
Author: Donald Sharp
Date: Fri Mar 4 15:28:56 2016 -0500
lib: Refactor read/write functionality
Both the read and write functions used the same code
slightly modified for reading and writing. Combine this
code together.
Signed-off-by: Donald Sharp
Edited-by: Paul Jakma to retain the
external library symbols, for ease of merging.
On 7/13/2016 6:10 AM, Lou Berger wrote:
> [resend as this message didn't seem to make it to the list...]
>
> Just an update: we've hooked our regression system into the github
> mirror and are now running minimal regression tests on bgpd. The tests
> start at a commit and move to the head of the branch, commit by commit
> -- pretty simple approach.
> Each run does a compile, basic adjacency checks, some unicast and VRF
> route distributions and checks the results against a reference (known
> good run). Each run takes about 4 minutes and there are about 160
> commits in /8/proposed/ff - currently has about 150 to go.
>
> I'll provide an update once we have interesting results.
>
> Lou
>
> PS I rebased the following commits to be 1st in order to get the
> regression environment running:
>
> Author: Lou Berger
> Date: Tue May 17 07:10:37 2016 -0400
>
> bgpd: Add flag to not change e{u,g}id on startup and run as
> unprivileged user
>* bgp_main.c: add -S / --skip_runas flag to not change effective
> user/group
> on start up. Enables bgpd to be run by unprivileged user.
>
> Author: Lou Berger
> Date: Tue May 17 07:10:36 2016 -0400
>
> bgp: add "debug bgp allow-martians" next hops and related code/commands
>
> Author: Lou Berger
> Date: Tue May 17 12:19:51 2016 -0400
>
> lib: change command logging to be off by default
>* lib/vty.c: add 'log_command' to enable logging of vty commands
> executed.
> Default command logging to off.
>
>
>
> On 7/11/2016 5:44 AM, Lou Berger wrote:
>>>> Any luck pinning down what commits are causing which issues for you?
>>>>
>> Not yet. Thinking I'll try to wire into our regression system in some way...
>>
>
> ___
> Quagga-dev mailing list
> Quagga-dev@lists.quagga.net
> https://lists.quagga.net/mailman/listinfo/quagga-dev
>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15889] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
This is patch that's breaking bgpd in 8/proposed/ff
Author: Donald Sharp
Date: Fri Mar 4 15:28:56 2016 -0500
lib: Refactor read/write functionality
Both the read and write functions used the same code
slightly modified for reading and writing. Combine this
code together.
Signed-off-by: Donald Sharp
Edited-by: Paul Jakma to retain the
external library symbols, for ease of merging.
On 7/12/2016 7:55 PM, Lou Berger wrote:
> Just an update: we've hooked our regression system into the github
> mirror and are now running minimal regression tests on bgpd. The tests
> start at a commit and move to the head of the branch, commit by commit
> -- pretty simple approach.
>
> Each run does a compile, basic adjacency checks, some unicast and VRF
> route distributions and checks the results against a reference (known
> good run). Each run takes about 4 minutes and there are about 160
> commits in /8/proposed/ff - currently has about 150 to go.
>
> I'll provide an update once we have interesting results.
>
> Lou
>
> PS I rebased the following commits to be 1st in order to get the
> regression environment running:
>
> Author: Lou Berger
> Date: Tue May 17 07:10:37 2016 -0400
>
> bgpd: Add flag to not change e{u,g}id on startup and run as
> unprivileged user
>
> * bgp_main.c: add -S / --skip_runas flag to not change effective
> user/group
> on start up. Enables bgpd to be run by unprivileged user.
>
> Author: Lou Berger
> Date: Tue May 17 07:10:36 2016 -0400
>
> bgp: add "debug bgp allow-martians" next hops and related code/commands
>
> Author: Lou Berger
> Date: Tue May 17 12:19:51 2016 -0400
>
> lib: change command logging to be off by default
>
> * lib/vty.c: add 'log_command' to enable logging of vty commands
> executed.
> Default command logging to off.
>
>
>
> On 7/11/2016 5:44 AM, Lou Berger wrote:
>>>> Any luck pinning down what commits are causing which issues for you?
>>>>
>> Not yet. Thinking I'll try to wire into our regression system in some way...
>>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15888] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
[resend as this message didn't seem to make it to the list...]
Just an update: we've hooked our regression system into the github
mirror and are now running minimal regression tests on bgpd. The tests
start at a commit and move to the head of the branch, commit by commit
-- pretty simple approach.
Each run does a compile, basic adjacency checks, some unicast and VRF
route distributions and checks the results against a reference (known
good run). Each run takes about 4 minutes and there are about 160
commits in /8/proposed/ff - currently has about 150 to go.
I'll provide an update once we have interesting results.
Lou
PS I rebased the following commits to be 1st in order to get the
regression environment running:
Author: Lou Berger
Date: Tue May 17 07:10:37 2016 -0400
bgpd: Add flag to not change e{u,g}id on startup and run as
unprivileged user
* bgp_main.c: add -S / --skip_runas flag to not change effective
user/group
on start up. Enables bgpd to be run by unprivileged user.
Author: Lou Berger
Date: Tue May 17 07:10:36 2016 -0400
bgp: add "debug bgp allow-martians" next hops and related code/commands
Author: Lou Berger
Date: Tue May 17 12:19:51 2016 -0400
lib: change command logging to be off by default
* lib/vty.c: add 'log_command' to enable logging of vty commands
executed.
Default command logging to off.
On 7/11/2016 5:44 AM, Lou Berger wrote:
>> > Any luck pinning down what commits are causing which issues for you?
>> >
> Not yet. Thinking I'll try to wire into our regression system in some way...
>
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15884] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
On July 11, 2016 4:40:25 AM Paul Jakma wrote: On Thu, 7 Jul 2016, Paul Jakma wrote: On Thu, 7 Jul 2016, Lou Berger wrote: Nope, but dropping this fixes another issue: normal updates are also taking 1-2 *minutes*. Also, adjacencies are taking longer than normal to come up Weird. Any luck pinning down what commits are causing which issues for you? Not yet. Thinking I'll try to wire into our regression system in some way... regards, -- Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A Fortune: The person who's taking you to lunch has no intention of paying. ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15881] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
On 7/7/2016 12:58 PM, Paul Jakma wrote:
> On Thu, 7 Jul 2016, Lou Berger wrote:
>
>> Looks like this fixes the exit problem. Two issues I see immediately
>> are:
>> ==12640== {
>>
>> Memcheck:Leak
>> fun:calloc
>> fun:zcalloc
>> fun:bnc_new
>> fun:bgp_find_or_add_nexthop
>> fun:bgp_start
>> fun:bgp_event
>> fun:bgp_start_timer
>> fun:thread_call
>> fun:main
>> }
>>
>> 2 some new log messages generated every seconds
>> 2016/07/07 09:02:35 BGP: There is already write fd [14]
>> 2016/07/07 09:02:35 BGP: There is already write fd [16]
>> 2016/07/07 09:02:35 BGP: There is already write fd [17]
> Hmm, not sure what those are about. Can be fixed later maybe.
>
>> Updates of the VPN & Encap SAFIs are still not working in an RR
>> config.
> Ah.
>
> The adjacencies issue, I wonder is that e27c516d, the connect timer
> change?
Nope, but dropping this fixes another issue: normal updates are also
taking 1-2 *minutes*.
Also, adjacencies are taking longer than normal to come up
>> ahh, why not just different branches, e.g.,
>> patch-tracking/8/proposed/{v1,v2,v3}/... They are cheap and easy? vN
>> can contain all the changes, V(N=1) is created when rebases are done
>> to clean up the history/log
> Or just use the commit ID? If you want a stable number, the first X
> digits of the commit ID already automatically provide it.
The version approach allows all to see the changes that go in on top of
what was sent to the list. This will certainly facilitate authors
reviewing changes to their submissions -- a good thing IMO - as well as
give you the clean history/logs you are looking for.
Lou
> Can also use 'hidden' branches/refs outside of refs/heads, e.g. in
> refs/patch-tracking instead. They won't be fetched automatically, won't
> appear in gitweb, etc.
>
> regards,
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15879] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
On 7/7/2016 8:43 AM, Paul Jakma wrote:
> On Thu, 7 Jul 2016, Paul Jakma wrote:
>
>> Unfortunately, that means the ff, nits and pushback tracking heads are going
>> to get non-fast-forward updated.
> I've fixed the issue and updated {pushback,nits}/{a,b} and ff.
Looks like this fixes the exit problem. Two issues I see immediately are:
1 valgrind:
==12640== 192 bytes in 3 blocks are definitely lost in loss record 1 of 1
==12640==at 0x4A057BB: calloc (vg_replace_malloc.c:593)
==12640==by 0x4C27B1: zcalloc (memory.c:89)
==12640==by 0x47EF2E: bnc_new (bgp_nexthop.c:80)
==12640==by 0x4A3697: bgp_find_or_add_nexthop (bgp_nht.c:126)
==12640==by 0x43850C: bgp_start (bgp_fsm.c:722)
==12640==by 0x439528: bgp_event (bgp_fsm.c:1177)
==12640==by 0x4370DE: bgp_start_timer (bgp_fsm.c:227)
==12640==by 0x4BFC4E: thread_call (thread.c:1358)
==12640==by 0x423007: main (bgp_main.c:489)
==12640==
{
Memcheck:Leak
fun:calloc
fun:zcalloc
fun:bnc_new
fun:bgp_find_or_add_nexthop
fun:bgp_start
fun:bgp_event
fun:bgp_start_timer
fun:thread_call
fun:main
}
2 some new log messages generated every seconds
2016/07/07 09:02:35 BGP: There is already write fd [14]
2016/07/07 09:02:35 BGP: There is already write fd [16]
2016/07/07 09:02:35 BGP: There is already write fd [17]
Updates of the VPN & Encap SAFIs are still not working in an RR config.
There also seems something wrong with bringing up adjacencies in
general, but again I'm out of time on this for a bit...
> Unfortunately, the new refs are not fast-forward-mergable. In the longer
> run, if volatile refs are an issue then maybe we just have to pass the
> bare commit IDs around for staging trees.
ahh, why not just different branches, e.g.,
patch-tracking/8/proposed/{v1,v2,v3}/...
They are cheap and easy? vN can contain all the changes, V(N=1) is
created when rebases are done to clean up the history/log
> Other alternatives would be not have staging trees, but then you're
> going to have very messy histories with edits and little fixes scattered
> all around the place, and intermingled with numerous reverts.
The above addresses this without forced pushes - although I'm not sure I
care so much about a messy log
Lou
> The former
> will make life harder for anyone who wants to backport fixes to an older
> release; both may make life _more_ miserable for anyone following along
> with out-of-tree work (e.g. WIP stuff, whatever).
>
> regards,
___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15875] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
On July 7, 2016 6:00:15 AM Paul Jakma wrote: On Thu, 7 Jul 2016, Paul Jakma wrote: Ah, it's my bad. 'lib: keep hash of node's commands to detect duplicate installs' I missed adding the basic commands to other nodes besides VIEW and ENABLE. How did 'exit' get into your config file though? We have a config file generation tool that for largely historic reasons generates a format that should equally work with cut and paste to the terminal. Sure we can change the tool, but unless the change (and backwards incompatibility) is intended, imo we shouldn't. Lou Though we did accept it, it's not something we ever wrote out into config files AFAIK. regards, -- Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A Fortune: Insomnia isn't anything to lose sleep over. ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15869] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
Paul, On July 6, 2016 10:53:19 AM Paul Jakma wrote: > On Wed, 6 Jul 2016, Lou Berger wrote: > >> - bgpd now starts, but exit is no longer supported in config files >> (which breaks existing configs) for example >> >>router bgp 123 >> >>... >> >>exit > > Are you testing at 'ff' (which will need a forced update, if you've > previously fetched it - note; as will all these heads)? Or at > 'pushback/b (which is sort of what 'ff' used to be at)? > It was a new clone: git clone git://git.savannah.nongnu.org/quagga.git 8ff-v3 cd 8ff-v3 git checkout -t remotes/origin/volatile/patch-tracking/8/proposed/ff > I'm guessing that if the problme is that 'exit' that it is due to: > > 'lib, vtysh: Add support for marking a file with appropriate end of > context' > nope, is due to changes in lib/command.c in 8/ff > Which I have in the 'pushback/b' subset, pending a comment to Dinesh > about compatibility. If you test from: > > proposed/ff - commit 4adca54da37e3d9360bcb4ab6d69ff20821ff31e > no joy > does it work? If that works), a test from: > > proposed/nits/b - commit 77a73bee9b636024bbc9e8ba7803b8f08d675c7d > > Would be useful. Does that work? no > >> - I did a simple test propagating a VPN safi update (in a RR config) and >> the route didn't propagate. >> >> I have to run to meetings, but something in update processing is >> definitely broken. > > Ok, if you can isolate to where, that'd be good. ci, ci, ci, ci... I don't have a good automated way to go backwards at the moment. Maybe Martin does - I probably won't have time to try to isolate until the end of the week (and ID cutoff). I'll see if Paul Z can... > > Also, it could be my editing. I had to do a lot of editing to fix merge > issues, I might have fat-fingered stuff (no doubt did somewhere). > Apologies in advance. Understood - know you're working hard on this... Lou > > regards, > -- > Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A > Fortune: > All the existing 2.0.x kernels are to buggy for 2.1.x to be the > main goal. > -- Alan Cox > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15862] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch
Thanks for the progress! - bgpd now starts, but exit is no longer supported in config files (which breaks existing configs) for example router bgp 123 ... exit - I did a simple test propagating a VPN safi update (in a RR config) and the route didn't propagate. I have to run to meetings, but something in update processing is definitely broken. Lou On 7/6/2016 9:08 AM, Paul Jakma wrote: > On Wed, 6 Jul 2016, Paul Jakma wrote: > >> Hi, >> >> I've reset the ref of the subject again. This time, it might hopefully stay >> stable, and further changes limited to after it (but no hard promises on >> that). > Sigh, I screwed something up. I'll have to reset those heads again, > sorry. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15852] Re: Time for an experiment? (Was: focus: patchwork drain vs project updates)
On 7/5/2016 8:42 AM, Vincent JARDIN wrote: > Le 05/07/2016 12:11, Lou Berger a écrit : >> If there are not roundskeepers interested in trying this what's the harm? > we'll unfocus and we'll never close the current effort. > right - which is the same point I was implicitly making WRT the process discussions. Most of us are sitting on the sidelines at the moment in a holding pattern, it seems a waste to have had all this discussion and interest and have nothing come of it. If no one else is interested in trying out the new process, then this itself is good information too. (but I also do think we need to allow time for folks to consider this too.) Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15849] Re: Time for an experiment? (Was: focus: patchwork drain vs project updates)
On July 5, 2016 3:10:34 AM Vincent JARDIN wrote: Le 5 juil. 2016 05:50, "Lou Berger" a écrit : So in thinking about this a bit, perhaps there's something useful that we can try in parallel with the single-person focused effort of proposed/8 (or 9) -- what about trying out the new process in a test repo on github? No, it will add mess on the mess. That was the point of using a separate branch, if something doesn't go well we just abandon it. If there are not roundskeepers interested in trying this what's the harm? Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15845] Time for an experiment? (Was: focus: patchwork drain vs project updates)
So in thinking about this a bit, perhaps there's something useful that we can try in parallel with the single-person focused effort of proposed/8 (or 9) -- what about trying out the new process in a test repo on github? We could follow the proposed rolling integration process based on patches ack'ed/discussed on quagga-dev, using a new/non-conflicting branch (e.g., 'dev/master'), and all those who have contributed to the process as the initial maintainer set (listed below + whoever else - I'm sure Martin/Donald are on the list.) Anyone interested in trying it out? Lou PS I suspect my timing might be bad given this is a high vacation week/period. So may need to give a little bit for folks to respond... On 6/29/2016 6:21 AM, Vincent JARDIN wrote: > All, > > There are many messages to update Quagga's organization. It is a good > way to proceed and we'll have to do something. > > However, from facts, we all see that >http://patchwork.quagga.net/ > needed to be drained. Some patches were 4y+ old. > > So, I strongly believe that for the benefits of everyone, >1st: patchwork needs to be drained, > http://patchwork.quagga.net/bundle/paul/round-8/ > >2nd: we need all the ff to be applied onto the head: > > http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile/patch-tracking/8/proposed/ff > > Currently, it is being done by Paul. So Thank you! > >3rd: finish the drain of patchwork (round 9?). Maybe it could be > started in parallel while Paul is finishing the round 8. To do it in > parallel, it would/could be expedite if patchwork could get some ack't, > for instance, >http://patchwork.quagga.net/patch/1652/ > no one did ack't it. It is important to get a review or an ack. > >4th: then, let's update Quagga's organization for the benefits of > everyone. Many comments from Lou/Martin/Jafar/Paul/Donald/Olivier/etc. > are very valid. > > So, instead of getting many emails about new Quagga's organization, I > would rather prefer to see emails that review patches. Then, once > patches will be merged into the head, let's fix the organization. > > Let's be focus all together, > Vincent > > ___ > Quagga-dev mailing list > Quagga-dev@lists.quagga.net > https://lists.quagga.net/mailman/listinfo/quagga-dev > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15760] Re: Quagga project governance
On 06/28/2016 09:54 AM, Donald Sharp wrote: > I > won't be paying full attention to any review comments because I will > be on vacation the 1-10 of July, but hope to resolve all issues upon > getting back in a timely manner. Being forever the optimist - we'll plan/hope to have all issues resolved *before* you return! Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15757] Dispute resolution (was: Quagga project governance)
[Separating threads - here] Paul, On 6/28/2016 9:02 AM, Paul Jakma wrote: > On Tue, 28 Jun 2016, Paul Jakma wrote: > >> That's what has driven us to here. I simply can not fail to mention >> the facts of this, because this is the _prime point of disagreement_ >> between us. > And I tried just to gently pushback on the majority voting stuff > initially, however to no avail. The more that wasn't heard, the more > explicit I had to get on why this bothers me particularly in this > context. In the current proposal/discussion, disputes are resolved based on majority voting of *maintainers*. While there has been some discussion on simple (>1/2) vs super (2/3s) majority, I haven't heard anyone else support the position of any holding veto power. Do you really think that majority voting of *maintainers* is unreasonable? Lou > Sorry. But hey. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15753] Re: Quagga project governance
Paul, So you'll discuss details once proposed as a patch to HACKING.md? Doesn't sound like the right place for some of this, but sure - shouldn't be hard to merge as there's not much overlap. In fact, I just did the merge (in https://docs.google.com/document/d/19DZcT0cJUSYxVIFenHvGFhLLUmLTRFHuMNZcI7aUNGA/edit#) by dropping HACKING.md at the start. It probably makes sense to ensure the new pieces are largely agreed to before submitting as patch (by Donald?) as well as formatted consistently with HACKING.md. Donald, Does this work for you? Where do we stand with resolving comments and getting general agreement on the Maintainers doc? Lou On June 28, 2016 6:54:44 AM Paul Jakma wrote: > On Tue, 28 Jun 2016, Lou Berger wrote: > >> Being unwilling to directly comment on, or contribute to a community >> developed document seems a bit unreasonable to me. > > One existed. Written by a number of people over the years. It's not > perfect, but what is? > > It starts with: > > "This is a living document. Suggestions for updates, via the >[quagga-dev list].., are welcome." > > Ignoring that and starting from scratch of itself I took to be sending a > message, to an extent. > > If you want evolve, evolve things. > > regards, > -- > Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A > Fortune: > The brain is a wonderful organ; it starts working the moment you get up > in the morning, and does not stop until you get to work. > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15750] Re: Quagga project governance
Paul, On June 28, 2016 6:02:28 AM Paul Jakma wrote: ... I don't think I am being that unreasonable. Being unwilling to directly comment on, or contribute to a community developed document seems a bit unreasonable to me. ... Funnily enough, when I explain the prime point of disagreement, I expect that each of us has different issues we're interested in seeing addressed. Having the process(es) documented allows us all to ensure that our favorites are covered in an acceptable fashion , as well as understand what else governs this collaborative effort. If you don't like what's been jointly hacked, please put forward an alternate starting point that we can all discuss. (Recall that the text that you sent to this in February has been already added to the relevant google docs.) Thanks, Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15734] Re: Quagga project governance
Paul, On 6/27/2016 8:22 AM, Paul Jakma wrote: > [any 'you' in the below may be general case than you specifically] Thanks -- glad to see you're not singling me out ;-) It would be good to hear from others too (BTW there were a slew of folks who fully participated in the last call -- and I wasn't one as I could only stay for ~20 minutes) --I don't think I'm the only one that cares about this -- if I am, I'll shut up and leave the list in peace. But perhaps it's because I have less "skin in the game" as I'm not, and don't expect to be, a candidate for Maintainer. > On Mon, 27 Jun 2016, Lou Berger wrote: > >>> In terms of specific development processes, things have evolved and >>> should continue to evolve. >> Why is this recent discussion anything than an evolution of the >> process? > If it's meant to be evolution, show me the evolutionary path? Give me a > "diff" from the current processes to what is being proposed? Again, if there was a document to diff against, we could do this -- but there isn't one! To (hopefully help) I've added the text from https://lists.quagga.net/pipermail/quagga-dev/2016-February/014883.html to the strawman process and maintainers doc to help move this along. > Just the fact one must have a Google account to participate in this > suggests this is far from an evolution. This is incorrect - all one needs is a browser. To demonstrate, I made the above changes without using any google account. > >> I accept that there may not be as much understand and context of >> quagga's history as think there should be. But if those that have the >> history or are not participating in the discussion , that history will >> be missed. While this isn't preferred, we can't compel people to >> participate. > That's a 2-way street. > >> And it has seemed to have left you somewhat abandoned by the other >> "maintainers". > Look, it really gives me _no_ joy to make other people unhappy. > Honestly. I _want_ you to be happy, and if you want a project where: > > - Decisions are made by Google Hangouts at a time that suits US TZ The last text I saw said that decisions are reviewed/confirmed on list. > - Contributors can game (consciously or not) things by simply ignoring >comments for nearly 2 years, then rally others to vote in their >changes en bloc with talk about how the development process is broken, >_failing to mention_ how they helped break it. I agree this is wrong. > - Any old crap goes into master, just if whatever random 'Acks' >(completely immune to gaming is that!), and anyone who might have >views just happens to have taken even a brief holiday (cause, >holidays, who needs those). I'm not sure how to deal with this -- if a patch is updated and resubmitted it's discussed by those participating. Personally I like a model of 5+ maintainers and one maintainer ack is required for inclusion in mainline. (Note I say mainline, which may or may not differ from master.) >[the goal of reducing rebases is a good one, and fast cycles and > auto-collated proposed trees would help with that, but some of what > I've read so far is just "I want magic ponies!" in that regard] > > Then _please_ go set up that project. I'm _*begging*_ you. _How clear do > I have to be?_ > > Really, we'll _all_ be happier. > > Me, I'm going to: > > - Keep going through the last of the backlog, and sort it out. > > - Get the obvious stuff in ASAP, get nits pushed back ASAP to the >contributors so they can fix them, and derail stuff that needs wider >discussion (and sorry that just happens in distributed, decentralised >development sometimes - people just /happen/ to pull in different >directions, and it needs resolving; and the outcome can mean >significant reworking or even lost work; it happens, it happens to >me]. > > - Listen to people's comments on patches, including my own. I manage to >listen to other people's opinions on my patches - I might not like the >comments or agree with them. I don't see why this is a problem in a >well functioning community. > > - Find people willing to help constructively, who are willing to work to >consensus > >[that can mean going with majority opinion usually, but still >respecting people when there's something they really don't get on with >- e.g. like trying to push binding majority votes through, in order to >try get around comments on a series of their patches]. > > I'm more than willing to share Quagga with others. The people involved -
[quagga-dev 15731] Re: Quagga project governance
Paul, You cover a lot of ground in your message. Along the way you pointed to a message from February which says: In terms of specific development processes, things have evolved and should continue to evolve. Why is this recent discussion anything than an evolution of the process? In fact if this was a document rather than an email, I'm sure it would have been the foundation for much of the recent discussion. I accept that there may not be as much understand and context of quagga's history as think there should be. But if those that have the history or are not participating in the discussion , that history will be missed. While this isn't preferred, we can't compel people to participate. And it has seemed to have left you somewhat abandoned by the other "maintainers". But to me this this highlights the need for a process that is sufficiently documented to withstand the disappearance of individuals no matter how short that time may be. Which of course brings us back to the maintainers document... Lou On June 27, 2016 7:15:12 AM Paul Jakma wrote: On Fri, 24 Jun 2016, Lou Berger wrote: I personally don't recall a discussion on this on list, but perhaps missed it. Well, we started this last year with Vincent leading by example and just getting stuck in to stuff in patchwork, reviewing it and summarising to list: https://lists.quagga.net/pipermail/quagga-dev/2015-June/012751.html I brought up workflows and using git branches to track: https://lists.quagga.net/pipermail/quagga-dev/2015-June/012859.html I also mentioned having some kind of technical panel / committee whatever: https://lists.quagga.net/pipermail/quagga-dev/2015-June/012804.html [I have actually had that kind of discussion with various people, inc. Greg and Martin over the years, on NPOs (UK has very low overhead options here), on having technical committees with constitution docs, etc. Greg has mentioned SPI in the past. Often the feedback has been (inc. from Martin on the "Quagga Architecture Review Committee" proposal I had discussed with him over a year ago) that these things seem too formal and high-overhead for a free software project. I've actually been *persuaded* by that, and now mostly think the important thing is to just get on with the technical stuff]. After Vincent got the ball rolling: https://lists.quagga.net/pipermail/quagga-dev/2015-July/012966.html https://lists.quagga.net/pipermail/quagga-dev/2015-August/013032.html https://lists.quagga.net/pipermail/quagga-dev/2015-September/013172.html https://lists.quagga.net/pipermail/quagga-dev/2015-October/013352.html Note that as of round-3, the incoming since that process was started (and a bit more) was dealt with. It was keeping up with what was coming in. Donald started with rebasing Cumulus patches, take-1: https://lists.quagga.net/pipermail/quagga-dev/2015-November/013591.html Take-2: https://lists.quagga.net/pipermail/quagga-dev/2015-November/013637.html Take-3: https://lists.quagga.net/pipermail/quagga-dev/2015-November/013683.html I pointing out I had long-standing comments on some. Also, some new ones generate a discussion, but are re-staged. Donald starts r5: https://lists.quagga.net/pipermail/quagga-dev/2015-November/013952.html Also, more versions of take-3: https://lists.quagga.net/pipermail/quagga-dev/2015-December/014154.htmlhttps://lists.quagga.net/pipermail/quagga-dev/2015-December/014154.html https://lists.quagga.net/pipermail/quagga-dev/2015-December/014221.html I'm not blaming anyone per se - stuff goes missing sometimes, and things are harder to track when there are large backlogs - but both still have various patches with comments. The comments were either mentioned recent to above, or there was even a discussion on the list recent to that (e.g. lifting the cluster-ID check in the BGP decision process). We had round 6, which mostly went on behind the scenes. We had a discussion on tools in Jan, e.g. on Gerrit: https://lists.quagga.net/pipermail/quagga-dev/2016-January/014620.html It's worth noting that Gerrit's workflow is very much based around rebasing - for those complaining about rebasing. A summary of the governance status and contribution proces was sent to the list in Feb: https://lists.quagga.net/pipermail/quagga-dev/2016-February/014883.html Updating HACKING was lagging, but there were other changes to HACKING still being discussed I think. However, the whole point is that this process should be somewhat self-documenting to contributors by way of summary emails and git branches. E.g.: http://git.savannah.gnu.org/cgit/quagga.git/refs/ or 'git fetch ' and you *see* the refs for the rounds being fetched, and you can easily examine them with 'gitk --all'. Now, could be that still isn't clear enough, in which case, we need some other tool. So discuss that. I lean to
[quagga-dev 15729] Re: Quagga project governance
Paul, I can only comment on one part of your message, On June 27, 2016 5:55:02 AM Paul Jakma wrote: ... Comments on the document I'm sure I've made either on that call I was on, to Lou in private email thereafter, or here on list. I think folks tried to address your comments as they understood them. Looking at your example, the last time I looked this had been changed /addressed. While meetings continue to play an important role, final decisions are to be reviewed/discussed on the list. One of the issues with making comments privately, whether it be on the process or on patches , is that the reason for changes (and hold ups) isn't clear to everyone involved. And this translates to some of those changes not being agreed upon or understood. As the maintainers document is the one that seems furthest along, i think it would be really helpful to send your specific edits/comments on the document to the list - or make them on the google doc as you prefer. I suspect this is the only way for us all to really understand where there are disagreements. Lou ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15723] Re: Quagga project governance
Paul, On 6/24/2016 11:33 AM, Paul Jakma wrote: > On Fri, 24 Jun 2016, Lou Berger wrote: > >> I can only speak to what's motivated my participation in this >> discussion. I think the root issues I'd like to see addressed are: >> 1. no ability to predict when the next release with *any* fixes will be >> available >> 2. lack of an active (rolling) development/integration master branch >> 3. lack of visibility and transparency in the integration and release >> process > The process developed late last year, initiated by Vincent and developed > into roughly month-based rounds of queueing patches objectively (i.e. > hoovering up everything possible from patchwork that came in since last > round); posting summary mails of said 'proposed' queue to the list; > having a bounded review period; and then sorting patches into: > > - deferred > - rejected > - accepted > > based on the feedback, then testing 'accepted' and merging into master, > was supposed to have answered 2 and 3. Though 'deferred' probably not a > good idea, and 'rejected' probably should be 'pushedback' or somesuch. > > Particularly on 3, the idea being that between the summary mail at the > start of the review period and 'gitk --all' at the end, it'd be very > clear what went where. > > To what extent did that fail to be transparent. so this is respect to 3. I personally don't recall a discussion on this on list, but perhaps missed it. I also don't see the process documented anywhere. Also, it's unclear how patches Ack'ed on list would end up anywhere but accepted. -- Keep in mind, my perspective is as user and contributor, not maintainer. > On 2, it seemed to be going well. We had (almost) month based > integration rounds, and had (from whatever arbitrary starting point in > patchwork) clear everything in patchwork from at least that point > forward. > > Why was that not meeting the 'active integration head' objective? It comes down to where I should do my development, test and submissions. Right now there is no changes between releases except at the last minute (or couple of weeks). This means: - no testing on ack'ed patches - no easy way for a user to see if an issue is addressed by the dev branch - submitted patches are often out of date (and need rebasing) and occasionally are duplicative > Further, why did that break down earlier this year? (I know what it > looks like to me, and I have asked this question several times now on > list, and no one answers). I have no idea. I only got involved beyond the contributor level once things broke down. >> I probably wouldn't have cared enough to speak up on 3 if 1 and 2 >> weren't issues. > Great, so why not propose fixes that make reference to the current > practices? This is where I started, but couldn't find it documented anywhere. And it seemed that others had their own ideas. > Rather than just go with a clean-sheet document that (at > best) ignores prior experience and lessons and the realities of > integration. What about any of the 3 discussed documents leads you to conclude they are clean-sheet documents? I see folks that have been active for a while contributing to each? > Indeed, it's primary feature (in the early draft I read) seemed to be > "Let's have one branch for Paul, and another for those who don't want to > deal with Paul's comments" - and you can imagine what I feel about that. I read this as allowing for Paul's approach given his long role in the project, but doing so in a way that allows the community to change based on community consensus. I personally think a single definitive quagga release that comes out 2 or 3 times a year would be preferred, but my understanding is that you vetoed this. > (And my comments were not even that onerous AFAIK; some are pretty damn > reasonable too; and I even offered the code for some). I'm not tracking the drafts as closely as some, but I don't recall seeing any changes or comments in the google docs from you (or for that matter on this list). Perhaps I missed them. Either way, it would be good to see your specific comments. Thanks, Lou > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15720] Re: Quagga project governance
Paul, On 6/24/2016 10:51 AM, Paul Jakma wrote: > On Fri, 24 Jun 2016, Lou Berger wrote: > >> I think this goes to the root of the recent discussions: > Oh, and while that might go to the root of some discussion, what caused > those discussions? I can only speak to what's motivated my participation in this discussion. I think the root issues I'd like to see addressed are: 1. no ability to predict when the next release with *any* fixes will be available 2. lack of an active (rolling) development/integration master branch 3. lack of visibility and transparency in the integration and release process I probably wouldn't have cared enough to speak up on 3 if 1 and 2 weren't issues. I think there a slew of other issues that stem from / will be solved by addressing the above , e.g., duplicate patches, painful/repeated rebases, and late discovery of issues. Lou > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15719] Re: Quagga project governance
Paul, On 6/24/2016 10:19 AM, Paul Jakma wrote: > On Fri, 24 Jun 2016, Lou Berger wrote: > >> I think this goes to the root of the recent discussions: >> - Is Quagga a community project, or a project reliant and owned by a >> single person? > I want it to be a collective project. I like working to consensus, I > like discussing the technicalities. I like getting stuff merged. I like > bringing in people. I invite people to examine the record: when did the > number of people working on maintaining expand; when did it shrink? When > did Quagga have lots of commits and frequent development releases? > > People are free to disagree with my preference for collectiveness and > consensus. Those are the parameters I've chosen. Others are free to > setup another community with different parameters if they feel strongly > about it. > > I'm open to persuasion on everything. My hope is that community agreement, with input from all --- including you, would be enough to persuade all... > However I tend react to badly to > (perceived) bullying and power games. > >> My understanding was that the Zebra to Quagga branch occurred largely >> because Zebra was really a single person controlled/owned project and >> there was a desire (amount *all* working Quagga at it's start) to have >> a community controlled version. > I was maintaining zebra-pj, patches to GNU Zebra, from others and > myself. There was also ZebOS at play. Kunihiro I suspect thought I was > too quick to integrate stuff with insufficient attention to stability > (as did another maintainer later). Also, I suspect - but I do not know - > that perhaps he required some kind of contributor agreement in order to > accept contributions to GNU Zebra, and if so that might have been a > factor. Hard to know. > > Kunihiro made other people maintainers but not me, so I *thanked* him > and forked. I didn't hector him. I respect him. (In retrospect, I'm > actually sorry about the name I chose - I realise now it might have been > a little disrespectful; I just enjoyed the pun at the time, and didn't > think of that aspect). > > He and others gave their code under the GPL, I'm immensely grateful for > it. I and others since then have added our bits. For which I *am* immensely grateful! >> One really important implication of this, is that the project should >> continue to thrive even if/when a key contributor/maintainer >> disappears or is overloaded with their "day job" for a time. > Yes. > > So chart the time line of when people were made maintainers or given > integration access, against which maintainers were active at that time. > Any patterns? > > The number of integrators will expand again. Governance too (though, > that's likely to take longer). > >> I've only been using / developing against Quagga since '09 and publicly >> pushing code out for the last couple of years, so may have it wrong, but >> have always viewed Quagga as a community driven / controlled project. >> >> Do you think I have this wrong? > Now, _why_ would you have that view? Comments like the above that imply (to me) that you need to be convinced, vs that the community needs to reach agreement. Lou > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15714] Quagga project governance (was: Re: Re: Quagga Monthly Meeting Notes)
Hi Paul, On 6/23/2016 12:41 PM, Paul Jakma wrote: > On Thu, 23 Jun 2016, Paul Jakma wrote: > >> In general, stuff should just go through that process ASAP. > And that larger stuff hasn't is due to a back log. > > To the extent that's my fault for not having much time for Quagga for a > good number of years, I apologise. But, it was being dealt with (and we > *HAD* dealt with incoming) up till late last year, and it will be dealt > with pretty much completely, quite soon. I think this goes to the root of the recent discussions: - Is Quagga a community project, or a project reliant and owned by a single person? My understanding was that the Zebra to Quagga branch occurred largely because Zebra was really a single person controlled/owned project and there was a desire (amount *all* working Quagga at it's start) to have a community controlled version. One really important implication of this, is that the project should continue to thrive even if/when a key contributor/maintainer disappears or is overloaded with their "day job" for a time. I've only been using / developing against Quagga since '09 and publicly pushing code out for the last couple of years, so may have it wrong, but have always viewed Quagga as a community driven / controlled project. Do you think I have this wrong? Lou > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15715] Re: Multiple Test failures in Round 8 - Anyone working on fixes?
BGP also fails to start... Multiple command installs to node 22 of command: table-map WORD Lou On 6/24/2016 4:02 AM, Martin Winter wrote: > I haven’t seen any fixes yet, but the Round 8 breaks on several points > on my CI system: > > As a side note, I’m a little bit disappointed that all were all marked > as failed by my CI > system, not tested (series of patches are sometimes missed as finding > complete series isn’t perfect) > or never submitted to patchwork or the list > > We could have all avoided this… > > > > 1) DejaGNU testcli: > >> […] >> Running ./libzebra.tests/testcli.exp ... >> FAIL: testcli >> […] > —> The offending commit for this is 0dbe0d2 (“lib: Consolidate > VIEW_NODE to be ENABLE_NODE as well”) > This is Patchwork #1856 > Originally failed CI test: > https://ci1.netdef.org/browse/QUAGGA-QPWORK-251 > > 2) DejaGNU testbgpmpattr IPv6-default: IPV6 MP Reach, global nexthop, 2 > NLRIs + default -- testbgpmpattr aborted! > >> […] >> Running ./bgpd.tests/testbgpmpath.exp ... >> Running ./bgpd.tests/testbgpmpattr.exp ... >> failed: testbgpmpattr IPv6-default: IPV6 MP Reach, global nexthop, 2 >> NLRIs + default -- testbgpmpattr aborted! >> […] > —> The offending commit for this is 82655af (“bgpd, zebra: Use next > hop tracking for connected routes too”) > This is Patchwork #1640 > Was never tested by my CI system (it is part of a series of 89 patches > which is a challenge for the automated > patchwork testing setup on my CI system) > > 3) missing htonf on OpenBSD / NetBSD 6/7 / FreeBSD 8/9/10 / OmniOS: > >> […] >> make all-am >> CC network.lo >> network.c: In function 'htonf': >> network.c:109:2: error: #error "Please supply htonf implementation for >> this platform" >> #error "Please supply htonf implementation for this platform" >> ^ >> network.c:111:1: warning: control reaches end of non-void function >> [-Wreturn-type] >> } >> ^ >> *** Error code 1 >> […] > The offending commit for this is f8e536e (“lib: consolidate > ntohf/htonf from ospfd/isisd TE to lib/network”) > This is was never seen in Patchwork. No idea where this patch came > from… > > > 4) bootstrap.sh fails on Ubuntu 16.04 > >> ./bootstrap.sh >> libtoolize: putting auxiliary files in '.'. >> libtoolize: copying file './ltmain.sh' >> libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'. >> libtoolize: copying file 'm4/libtool.m4' >> libtoolize: copying file 'm4/ltoptions.m4' >> libtoolize: copying file 'm4/ltsugar.m4' >> libtoolize: copying file 'm4/ltversion.m4' >> libtoolize: copying file 'm4/lt~obsolete.m4' >> libtoolize: 'AC_PROG_RANLIB' is rendered obsolete by 'LT_INIT' >> configure.ac:97: error: possibly undefined macro: AC_MSG_RESULT >> If this token and others are legitimate, please use >> m4_pattern_allow. >> See the Autoconf documentation. >> autoreconf: /usr/bin/autoconf failed with exit status: 1 > —> The offending commit for this is 09d4631 (“qpb: Add support for > protobuf”) > (Small note: Running “bootstrap.sh” a 2nd time will work.) > This is Patchwork 1884 which somehow was missed in my CI system as well. > (My CI > system never detected this as a complete series of patches, so no run > was triggered) > > > > Would appreciate if someone can spend a few cycles on fixing this BEFORE > pushing more > commits into this branch as these are multiple teststoppers. > > The list is potentially not complete - as the tests fail at various > stages on all platforms > based on these problems. There could be more issues later on… > > - Martin Winter > > ___ > Quagga-dev mailing list > Quagga-dev@lists.quagga.net > https://lists.quagga.net/mailman/listinfo/quagga-dev ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15713] Re: Quagga Monthly Meeting Notes
On 6/23/2016 1:46 AM, Jafar Al-Gharaibeh wrote: > On 6/22/2016 6:04 PM, Lou Berger wrote: >> why not just use personal github for this? that said,there might be >> value in having an easy way to see the features in development... > I do use github for this, as many of us probably do. At some point we > want this to go into the Quagga. Even if everyone agrees that some > feature/patch should go in, having it in CE several months or even > more before moving it to the "standard/YE" release allows a period of > feedback and bug fixes related to the feature by users who want to try > or need the new feature without impacting the users who prefer less > "distributive" or slower changes. Having a faster path to an integrated development mainline is certainly one of my "hot button" issues that I think is critical to be solved. Why comment was in the context of Paul's statement: > Oh, there might well be value in having staging areas for experimental > features. I didn't read this as an CE or anything other than separate feature development branch... Lou > --Jafar > ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15702] Re: Quagga Monthly Meeting Notes
On 6/22/2016 6:17 PM, Paul Jakma wrote: > On Wed, 22 Jun 2016, Jafar Al-Gharaibeh wrote: > >> For example, assume I want to push the new feature that I have been working >> on: MTR-OSPF (RFC4915). Having that in the CE (thing ;) ) for a few cycles >> might help us understand its impact on users or whether it is worthy to be >> added to the main release or not. I really think there is a value there. > Oh, there might well be value in having staging areas for experimental > features. why not just use personal github for this? that said,there might be value in having an easy way to see the features in development... Lou > Though, let me warn people in advance, if some patch set is somehow not > ready to go in to master but needs proving or something, regular > rebasing may be a fact of life. I've had to carry stuff for ages before > I could prove it (and accused of pushing stuff in too fast by other > maintainers for it!). I've abandoned plenty of stuff too, including > because I just couldn't keep up with other changes. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15698] Re: Quagga Monthly Meeting Notes
Thanks Donald. Your timing is good. I just reviewed and commented on the maintainer doc. I suggested: - explicitly allowing an individual to play multiple roles in the project (IMO we're not big enough to exclude this) - adding the role of 'git master' which uses the text from Vincent's Maintainer section. - asked if acked-based patch acceptance is required vs time based Lou On 6/22/2016 12:48 PM, Donald Sharp wrote: > Lou - > > You are correct, I jumped the gun a little bit. We need to provide > time for people to vote on the document.. Please take the time to > read the document and let us know how you would like to proceed. > > Additionally I'll move the CE part back to the Quagga Release Process > docuemnt. > > thanks! > > donald > > On Wed, Jun 22, 2016 at 6:28 AM, Lou Berger wrote: >> Paul, >> >> While the conclusion to work on ce may (or may not, I wasn't on the call for >> this) be premature, why can't a branch just be added under the existing repo >> if/when that's the consensus among the community? >> >> Lou >> >> >> >> On June 22, 2016 5:53:34 AM Paul Jakma wrote: >> >>> On Tue, 21 Jun 2016, Donald Sharp wrote: >>> >>>> Discussion on where to do the work next. quagga-ce on github or on a >>>> branch in Savannah. Decision was to start immediately working on a CE >>>> branch on github. >>> >>> You need to change the name though. >>> >>> regards, >>> -- >>> Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A >>> Fortune: >>> Getting the job done is no excuse for not following the rules. >>> >>> Corollary: >>> Following the rules will not get the job done. >>> >>> ___ >>> Quagga-dev mailing list >>> Quagga-dev@lists.quagga.net >>> https://lists.quagga.net/mailman/listinfo/quagga-dev >>> >> ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15688] Re: Quagga Monthly Meeting Notes
Paul, Independent of branches/release discussion, my view is that either version of the roles documents - either Vincent's or the one posted for yesterday's meeting - would really help to _just get it done_. (of course, I am not maintainer nor do I expect to be one in the future.) It would be good to get your input into these discussions, as based on reading the list it seems there is general agreement that the status quo needs to change. Thanks, Lou On 6/22/2016 6:51 AM, Paul Jakma wrote: > On Wed, 22 Jun 2016, Paul Jakma wrote: > >> If CE is meant to be some way to get around the fact that some patches >> in the backlog have review comments outstanding; and the contributor >> concerned thinks it easier to engineer a crisis than just engage with >> review comments and address them via revisions and/or persuasion, >> then, sorry, no. > A much better way forward is to just roll up the sleeves, and get the > nits addressed, and do the (trial) rebases to see what depends on what > and what can easily be triaged ahead and gotten in. > > This isn't rocket science. Integration work (review, nits, reconciling, > etc.) can be very tedious - esp when lots of it has built up in > different directions. > > However, the best answer to such a problem is to _just get it done_. > > regards, ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15685] Re: Quagga Monthly Meeting Notes
Paul, While the conclusion to work on ce may (or may not, I wasn't on the call for this) be premature, why can't a branch just be added under the existing repo if/when that's the consensus among the community? Lou On June 22, 2016 5:53:34 AM Paul Jakma wrote: On Tue, 21 Jun 2016, Donald Sharp wrote: Discussion on where to do the work next. quagga-ce on github or on a branch in Savannah. Decision was to start immediately working on a CE branch on github. You need to change the name though. regards, -- Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A Fortune: Getting the job done is no excuse for not following the rules. Corollary: Following the rules will not get the job done. ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
[quagga-dev 15684] Re: Quagga Monthly Meeting Notes
Hi Donald, I'm sorry I couldn't stay for the rest of the meeting. I really think the roles discussion is separable from the release cycle discussion and it would be good to close the roles discussion first. Lou On June 21, 2016 11:01:45 AM Donald Sharp wrote: Discussion of the Quagga Maintainers Document: https://docs.google.com/document/d/19DZcT0cJUSYxVIFenHvGFhLLUmLTRFHuMNZcI7aUNGA/edit?usp=sharing The document is updated to reflect the discussion/decisions in this meeting. The highlights of the discussion: 1) Maintainer per protocol. This is maintained in a MAINTAINER document 2) Different branches per protocol. This was deemed to high cost. Limit to one development branch 3) Counter proposal from Vincent to have stricter roles and git only committers. Community is not large enough yet to move to this methodology. 4) Added a section on Sabbatical/extended leave 5) 6 months inactivity to remove 6) Violation of trust/Role - Resolved through current maintainers. This document was approved for moving forward with Discussion on where to do the work next. quagga-ce on github or on a branch in Savannah. Decision was to start immediately working on a CE branch on github. donald ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev ___ Quagga-dev mailing list Quagga-dev@lists.quagga.net https://lists.quagga.net/mailman/listinfo/quagga-dev
