[quagga-dev 16634] Re: [99attendees] [dev] Open source routing @ ietf meetup

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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 16594] Meetup @IETF this week?

[Lou Berger]

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

[Lou Berger]

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 16310] Re: [PATCH v2 1/2] bgpd: VRF vty configuration, RIB table creation

[Lou Berger]

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 <lber...@labn.net> 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 i

[quagga-dev 16306] Re: [PATCH v2 1/2] bgpd: VRF vty configuration, RIB table creation

[Lou Berger]

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 ();
>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 (>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, >p, 

[quagga-dev 16257] [PATCHv2] bgp: add bgp_isvalid_nexthop helper and additional NHT zebra checks (also restores check from original patch)

[Lou Berger]

---
 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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

---
 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

[Lou Berger]

---
 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 (_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 (_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

[Lou Berger]

---
 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 _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

[Lou Berger]

---
 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

[Lou Berger]

---
 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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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 <p...@jakma.org> 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

[Lou Berger]

On October 6, 2016 11:47:59 AM Paul Jakma <p...@jakma.org> 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 16195] [PATCH] bgp: ignore NHT when bgpd has never connected zebra

[Lou Berger]

---
 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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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 16047] [PATCH] bgp: bgp_nexthop init/free AFI_ETHER related NH tables

[Lou Berger]

---
 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

[Lou Berger]

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

[Lou Berger]

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)

[Lou Berger]

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)

[Lou Berger]

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)

[Lou Berger]

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)

[Lou Berger]

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 <pmoha...@cumulusnetworks.com> Date: Mon Nov 9 20:21:41 2015 -0500

[Lou Berger]

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 <p...@jakma.org> 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 15989] Re: CI Testresult: FAILED (Re: [quagga-dev, 15986] bgp_route.c fix memory leak Introduced by: Author: Pradosh Mohapatra <pmoha...@cumulusnetworks.com> Date: Mon Nov 9 20:21:41 2015

[Lou Berger]

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)

[Lou Berger]

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 <pmoha...@cumulusnetworks.com> Date: Mon Nov 9 20:21:41 2015 -0500

[Lou Berger]

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)

[Lou Berger]

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 15945] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)

[Lou Berger]

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 <lber...@labn.net> 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 15921] Re: bgpd regression of 8/ff results (was Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch)

[Lou Berger]

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)

[Lou Berger]

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

[Lou Berger]

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?

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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 = >writefd;
>
> -  if (FD_ISSET (fd, >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 <p...@jakma.org> 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 <sha...@cumulusnetworks.com>
>>> 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 <sha...@cumulusnetworks.com>
>>>
>>>Edited-by: Paul Jakma <paul.ja...@hpe.com> 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 <lber...@labn.net>
>>>> 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 <lber...@labn.net>
>>>> Date:   Tue May 17 07:10:36 2016 -0400
>>>>
>>>> bgp: add "debug bgp allow-martians" next hops and related
>>>> code/commands
>>>>
>>>> Author: Lou Berger <lber...@labn.net>
>>>> 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

[Lou Berger]

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 <lber...@labn.net> 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)

[Lou Berger]

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 <equi...@opensourcerouting.org>
  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 <equi...@opensourcerouting.org>

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 <sha...@cumulusnetworks.com>
>>> 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 <sha...@cumulusnetworks.com>
>>>
>>>Edited-by: Paul Jakma <paul.ja...@hpe.com> 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 <lber...@labn.net>
>>>> 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 <lber...@labn.net>
>>>> Date:   Tue May 17 07:10:36 2016 -0400
>>>>
>>>> bgp: add "debug bgp allow-martians" next hops and related code/commands
>>>>
>>>> Author: Lou Berger <lber...@labn.net>
>>>> 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

[Lou Berger]

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 <lber...@labn.net>
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 <lber...@labn.net>
Date:   Tue May 17 07:10:36 2016 -0400

bgp: add "debug bgp allow-martians" next hops and related code/commands

Author: Lou Berger <lber...@labn.net>
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)

[Lou Berger]

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 <sha...@cumulusnetworks.com>
>> 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 <sha...@cumulusnetworks.com>
>>
>>    Edited-by: Paul Jakma <paul.ja...@hpe.com> 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 <lber...@labn.net>
>>> 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 <lber...@labn.net>
>>> Date:   Tue May 17 07:10:36 2016 -0400
>>>
>>> bgp: add "debug bgp allow-martians" next hops and related code/commands
>>>
>>> Author: Lou Berger <lber...@labn.net>
>>> 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

[Lou Berger]

This is patch that's breaking bgpd in 8/proposed/ff

Author: Donald Sharp <sha...@cumulusnetworks.com>
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 <sha...@cumulusnetworks.com>
   
Edited-by: Paul Jakma <paul.ja...@hpe.com> 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 <lber...@labn.net>
> 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 <lber...@labn.net>
> Date:   Tue May 17 07:10:36 2016 -0400
>
> bgp: add "debug bgp allow-martians" next hops and related code/commands
>
> Author: Lou Berger <lber...@labn.net>
> 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

[Lou Berger]

[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 <lber...@labn.net>
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 <lber...@labn.net>
Date:   Tue May 17 07:10:36 2016 -0400

bgp: add "debug bgp allow-martians" next hops and related code/commands

Author: Lou Berger <lber...@labn.net>
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

[Lou Berger]

On July 11, 2016 4:40:25 AM Paul Jakma <p...@jakma.org> 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

[Lou Berger]

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 15875] Re: Someone rebased volatile/patch-tracking/8/proposed/ff branch

[Lou Berger]

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

[Lou Berger]

Paul,


On July 6, 2016 10:53:19 AM Paul Jakma <p...@jakma.org> 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

[Lou Berger]

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)

[Lou Berger]

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)

[Lou Berger]

On July 5, 2016 3:10:34 AM Vincent JARDIN <vincent.jar...@6wind.com> wrote:


Le 5 juil. 2016 05:50, "Lou Berger" <lber...@labn.net> 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)

[Lou Berger]

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

[Lou Berger]

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)

[Lou Berger]

[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 15750] Re: Quagga project governance

[Lou Berger]

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

[Lou Berger]

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 - 
> including in maintainer/governance/roles has generally increased while 
> I've been active.

I think fixing the (lack of) maintainers issue ASAP  w

[quagga-dev 15731] Re: Quagga project governance

[Lou Berger]

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 <p...@jakma.org> 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 toward bugzilla as the canonical ID
and place for co

[quagga-dev 15729] Re: Quagga project governance

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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)

[Lou Berger]

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?

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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 <lber...@labn.net> 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 <p...@jakma.org> 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

[Lou Berger]

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

[Lou Berger]

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

[Lou Berger]

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

[quagga-dev 15677] Re: Call for Agenda

[Lou Berger]

Hi Vincent / all,

The doc contains a few main points. Some more clear than others. Here
are some comments on how I read it, both for ensuring I understand you
correctly, and for discussion:

1. Steering committee and new maintainers roles and election
  The doc doesn't say how maintainers are appointed / elected.
  I have no objection to the new definitions.

2. Review of patches
There is no obligation on anyone to take on review of a patch.
I think this is needed to ensure a vibrant quagga.  I think review
should be one of the requirements for being on the steering
committee, and as a backup for when reviews are not done by others.

3. Dispute resolution
   Falls to steering committee. I think is fine as long as dispute
   discussions take place in public (e.g., on list, public meetings ) and
   decisions  are always reviewed on list.

4. Ack’ed code
  Goes to a development branch.  This is great and one of the
  main things not happening in a timely fashion today.

  I also think the development branch should always be in
  the same place -- master makes sense to me but
  *any stable place* would be a huge improvement.

4. Release process/cycle
  I think this is largely a separable discussion than the above. Also I
  found the text a bit vague and confusing. I suggest moving/combining
  this with the other process doc and discussing separately.

  If I correctly interpret what you're saying, I think the process itself
  is fine assuming it is coupled with a fixed release schedule, e.g.,
per CE.

Thanks,
Lou


On June 21, 2016 3:20:32 AM Vincent JARDIN <vincent.jar...@6wind.com> wrote:
>
> Thanks Martin.
>
> It is a good summary.
> Le 21 juin 2016 05:14, "Martin Winter" <mwin...@opensourcerouting.org>
a écrit :
>
> A little bit late, but I had a long call with Vincent today (his late
> monday evening).
>
> He has some different suggestion which I wrote down in a new
document as
> an alternative choice. (Unfortunately, he can’t attend the call)
>
> Description is here:
>
https://docs.google.com/document/d/1A0kr7PrlsXs7Xe4btAgVWolvXYF5-JjtSWTOQypGRSs/edit?usp=sharing
>
> Key changes:
> - Maintainers are reduced to git committers and can’t make the
ACK/NACK
>   decision.
> - Anyone in community can ACK/NACK a patch.
> - No ACK: does not go in at all
> - Can’t agree: will get pushed to Steering committee for
decision
> - Dispute resolution is done by a Steering committee which is elected
>
> I hope I got all the details correctly written down in the spirit
of how
> Vincent explained it to me.
>
> - Martin
>
> Disclaimer: I’m only the person who wrote it down. This does not
mean that
> I agree or disagree on it. But I want this choice to be discussed
as well.
>
>
> On 18 Jun 2016, at 14:23, Lou Berger wrote:
>
> On 6/14/2016 12:55 PM, Donald Sharp wrote:
>
> Next Tuesday we have the normal monthly meeting.  If you
have anything
> that you would like to discuss please let me know so I can
add it to
> the agenda.
>
> I'd like to discuss and vote on the 3 documents:
>
> Maintainer:
>
>
https://docs.google.com/document/d/19DZcT0cJUSYxVIFenHvGFhLLUmLTRFHuMNZcI7aUNGA/edit?usp=sharing
>
> I made a few minor changes and comments. The most substantive
change I
> made was to clarify that missing e-mail votes are the ones
that count,
> and that it's 3 out of 6 votes is needed to be inactive (3 in
a row, 3
> out of 4, 3 out of 5, and 3 out of 6 all = inactive).
>
>   It looks good to me (either with or without the changes). I
vote yes
> (as contributor).
>
> Tools:
>
>
https://docs.google.com/document/d/1s_EbbXwqWPmfOg6ArgKmEMm_iv0vwGvJs-7ZG4yFKb4/edit?usp=sharing
>
> I think the document combines a few things together in a few
places,
> e.g., submission within the code review section.  I suggested some
> changes to address this.  I think the topic of submissions via
pull
> requests is also missing.  From my contributor perspective, I
prefer
> pull requests makes the most sense, but given where the project is
> "vote" for both email patches and pull requests.  Having pull
requests
> only be mandatory for patch sets seems like a reasonable
transition
> approach...
>
> This too looks good to me (either with or without the
changes). I vote
> yes (as contributor).
>
> Quagga Process:
>
>
https://docs.google.com/document/d/1xYrpTKYDvK23BCxXP-dbE6nOuvBxbGIilNLfVkI3j-I/edit?usp=sharing
>
> I support thi

[quagga-dev 15650] Re: [PATCH 10/10] bgpd: add L3/L2VPN Virtual Network Control feature

[Lou Berger]

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 <lber...@labn.net> 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 15649] Re: [PATCH 10/10] bgpd: add L3/L2VPN Virtual Network Control feature

[Lou Berger]

On 6/17/2016 9:15 AM, Philippe Guibert wrote:
> On Thu, Jun 16, 2016 at 8:52 PM, Lou Berger <lber...@labn.net> 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 15641] Re: [PATCH] bgpd: eliminate RD related duplicate code in bgp_encap.c

[Lou Berger]

just sent to list - rev should hopefully address the CI failure too.

On June 17, 2016 5:26:23 AM Philippe Guibert
<philippe.guib...@6wind.com> wrote:

> On Thu, Jun 16, 2016 at 8:45 PM, Lou Berger <lber...@labn.net> wrote:
>
> Hello Lou,
>
>>
>> ---
>>  bgpd/bgp_encap.c   | 45 -
>>  bgpd/bgp_mplsvpn.c |  8 
>>  bgpd/bgp_mplsvpn.h |  4 
> [..]
>> -static u_int16_t
>> +u_int16_t
>>  decode_rd_type (u_char *pnt)
>
> Please, can you please add a commit log like the following ?
>
> bgpd: decode_rd() apis are declared global in bgpd()
> This fix avoids duplicated function utilities in differents files of bgpd.
>
> Thanks,
>
> Acked-by: Philippe Guibert <philippe.guib...@6wind.com>
>



___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

[quagga-dev 15640] [PATCHv2] bgpd: eliminate RD related duplicate code in bgp_encap.c decode_rd_... apis are declared global in bgp_mplsvpn.c

[Lou Berger]

---
 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 1a09ba6..e69da04 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 (_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 08a4272..9b975da 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 (_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 15631] Re: [PATCH 10/10] bgpd: add L3/L2VPN Virtual Network Control feature

[Lou Berger]

Philippe,

I've posted a fix to this (patch on patch) in

https://github.com/LabNConsulting/quagga-vnc/commit/cd54370cb94d598aa95bd7561cc012200920d97a

If you can take a look and comment that would be great.

My plan is to submit a v2 of the whole patch set once issues raised have
been addressed. (this patch will be rolled into the larger vnc patch.)

Thanks,

Lou


On 6/9/2016 4:42 AM, Lou Berger wrote:
> Philippe,
>
> On 6/9/2016 4:15 AM, Philippe Guibert wrote:
>> > On Wed, Jun 8, 2016 at 9:22 PM, Lou Berger <lber...@labn.net> wrote:
>> >
>> > Hi Lou,
>>>> >>> * It seems the RD_TYPE_EOI type surfaced again (see
>>>> >>>   http://patchwork.quagga.net/patch/1728/ ), whereas I don't see where 
>>>> >>> it
>>>> >>>   is used. Is there a need to keep this flag ?
>>> >> This is used by the ethernet code, see below.
>>> >>
>> > oh. It seems it is used decode_rd_type ? Would not it be better to set
>> > explicitly the value to RD_TYPE_EOI instead of 0xff00 ?
>> >
>> > --- a/bgpd/bgp_mplsvpn.c
>> > +++ b/bgpd/bgp_mplsvpn.c
>> > @@ -52,7 +52,7 @@ decode_rd_type (u_char *pnt)
>> > * VNC L2 stores LHI in lower byte, so mask it off
>> > */
>> >if ((v & 0xff00) == 0xff00)
>> > -v = 0xff00;
>> > +v = RD_TYPE_EOI;
>> >  #endif
>> >return v;
>> >  }
> Good catch!  I suspect this got changed internally when it was removed
> from the first patch set and then not restored...
>  



___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

[quagga-dev 15630] [PATCH] bgpd: eliminate RD related duplicate code in bgp_encap.c

[Lou Berger]

---
 bgpd/bgp_encap.c   | 45 -
 bgpd/bgp_mplsvpn.c |  8 
 bgpd/bgp_mplsvpn.h |  4 
 3 files changed, 8 insertions(+), 49 deletions(-)

diff --git a/bgpd/bgp_encap.c b/bgpd/bgp_encap.c
index d21924c..bd150ce 100644
--- a/bgpd/bgp_encap.c
+++ b/bgpd/bgp_encap.c
@@ -49,51 +49,6 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, 
Boston, MA
 #include "rfapi_backend.h"
 #endif
 
-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 (_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 ec64300..ff33e42 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -39,7 +39,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, 
Boston, MA
 #include "rfapi_backend.h"
 #endif
 
-static u_int16_t
+u_int16_t
 decode_rd_type (u_char *pnt)
 {
   u_int16_t v;
@@ -80,7 +80,7 @@ encode_label(u_int32_t label,
 }
 
 /* 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;
@@ -93,7 +93,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;
@@ -106,7 +106,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 (_ip->ip, pnt, 4);
diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h
index ba65ca7..2210537 100644
--- a/bgpd/bgp_mplsvpn.h
+++ b/bgpd/bgp_mplsvpn.h
@@ -69,10 +69,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 15627] Re: [PATCH v2 07/30] lib: add "show commandtree" CLI command

[Lou Berger]

Paul,

See below.

On 6/15/2016 5:53 AM, Paul Jakma wrote:
> On Tue, 12 Jan 2016, Lou Berger wrote:
>
>> +DEFUN (show_commandtree,
>> +   show_commandtree_cmd,
>> +   "show commandtree",
>> +   NO_STR
>> +   "Show command tree\n")
>> +{
>> +  /* TBD */
>> +  vector cmd_vector;
>> +  unsigned int i;
>> +
>> +  vty_out (vty, "Current node id: %d%s", vty->node, VTY_NEWLINE);
>> +
>> +  /* vector of all commands installed at this node */
>> +  cmd_vector = vector_copy (cmd_node_vector (cmdvec, vty->node));
>> +
>> +  /* loop over all commands at this node */
>> +  for (i = 0; i < vector_active(cmd_vector); ++i)
>> +{
>> +  struct cmd_element *cmd_element;
>> +
>> +  /* A cmd_element (seems to be) is an individual command */
>> +  if ((cmd_element = vector_slot (cmd_vector, i)) == NULL)
>> +continue;
>> +
>> +  vty_out (vty, "%s%s", cmd_element->string, VTY_NEWLINE);
>> +}
>
> How does this 'show commandtree' differ from the long standing 'list' 
> command? The body of that is:
>
>for (i = 0; i < vector_active (cnode->cmd_vector); i++)
>  if ((cmd = vector_slot (cnode->cmd_vector, i)) != NULL
>  && !(cmd->attr == CMD_ATTR_DEPRECATED
>   || cmd->attr == CMD_ATTR_HIDDEN))
>vty_out (vty, "  %s%s", cmd->string,
> VTY_NEWLINE);
>
> Other than ignoring hidden ones - 
That's it.  The main usage was ensuring we didn't loose *anything* when
doing some major CLI reorg/hacking...

> which could be made an option of the 
> existing 'list'?
True.

> Do we need this new command?
I think skipping this for now is fine and if the need once again arises
for showing all, we can add it as a list option.

Thanks,
Lou

> regards,



___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

[quagga-dev 15613] Re: Round 8

[Lou Berger]

Nothing but old habits, so easy to change.  I've been favoring clang (much 
better warnings) which defaults to it in any case...


Cheers,
Lou


On June 16, 2016 5:49:00 AM Paul Jakma  wrote:


On Thu, 16 Jun 2016, Paul Jakma wrote:


If it's trivial like declaring stuff in the first clause of a for, I'm
happy to change it.


Oh, but... also be good to fix whatever it is that prevents you running
your compiler in C99+ mode. ;)

regards,
--
Paul Jakma | p...@jakma.org | @pjakma | Key ID: 0xD86BF79464A2FF6A
Fortune:
I'm going to Vietnam at the request of the White House.  President Johnson
says a war isn't really a war without my jokes.
-- Bob Hope





___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

[quagga-dev 15605] Re: Round 8

[Lou Berger]

On 6/15/2016 11:38 AM, David Lamparter wrote:
> On Tue, Jun 14, 2016 at 10:03:41AM -0400, Lou Berger wrote:
>> Eak -- the branch doesn't compile with -std=c99 .
> Standard for Quagga is currently "-std=gnu99" as seen in configure.ac
> line ~140; there seems to be a bug somewhere that the flag isn't
> correctly applied?

Why use -std=gnu99? Before these changes quagga, e.g., the current
release, compiled fine without the flag. 

Lou

>
> -David
>
>> On 6/14/2016 9:47 AM, Lou Berger wrote:
>>> Paul,
>>>
>>> Great to see this published.  Is there a quagga convention WRT C99?
>>>
>>> I see the following when compiling proposed/8
>>>
>>> if.c:1151: error: ?for? loop initial declarations are only allowed in
>>> C99 mode
>>> if.c:1151: note: use option -std=c99 or -std=gnu99 to compile your code
>>> zclient.c:903: error: ?for? loop initial declarations are only allowed
>>> in C99 mode
>>> zclient.c:903: note: use option -std=c99 or -std=gnu99 to compile your code
>>> interface.c:2505: error: ?for? loop initial declarations are only
>>> allowed in C99 mode
>>> interface.c:2505: note: use option -std=c99 or -std=gnu99 to compile
>>> your code
>>>
>>> Lou
>>>
>>> On 6/14/2016 9:02 AM, Paul Jakma wrote:
>>>> Hi,
>>>>
>>>> Took a bit longer, but the proposed patches are up in:
>>>>
>>>> http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile/patch-tracking/8/proposed/patchwork
>>>> http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile/patch-tracking/8/proposed/donald-take-3
>>>>
>>>> I've done some initial prioritising, so the 'restricted node' patch, 
>>>> 'sockaddr_dl' and table cmd are at the end, as there were already some 
>>>> comments on those.
>>>>
>>>> Note: I've had a good few conflicts on 'install_element' lines, due to 
>>>> various patches adding/removing commands with the restricted node and 
>>>> 'auto-install view commands into enable' patches. I didn't look too 
>>>> closely as to whether I resolved them correctly.
>>>>
>>>> Still to do is linearise these 2 heads together.
>>>>
>>>> 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 mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

[quagga-dev 15592] Re: [PATCH 10/10] bgpd: add L3/L2VPN Virtual Network Control feature

[Lou Berger]

Philippe,

On 6/15/2016 10:45 AM, Philippe Guibert wrote:
> On Thu, Jun 9, 2016 at 10:42 AM, Lou Berger <lber...@labn.net> wrote:
>
> Lou,
>
>> Philippe,
> [..]
>> Sure, as we said the RFP and VNC feature is aliened with the NVO3
>> architecture and
>>
>> The NVE-NVA protocol used to communicate routing and Ethernet / Layer 2
>> (L2) forwarding information between NVAs and NVEs is referred to as the
>> Remote Forwarder Protocol (RFP). OpenFlow is an example RFP.
>>
>> So from a conceptual standpoint draft-ietf-nvo3-nve-nva-cp-req is a good 
>> place to start.
>>
>> For a concrete example see
>> https://www.opennetworking.org/images/stories/downloads/sdn-resources/onf-specifications/openflow/openflow-spec-v1.3.0.pdf
>>
> Thanks for the reading.
> There are different scenarios described.
> I understand that VNC brings flexibility to match the different use cases.
> For example, if there is a need to configure some routing in the same
> VM where VNC is located, it is easier to export the routing
> information to zebra.
I think you are referring to the CE export/import case, then yes.

>
> In addition to this, the rfapi.h API is well prepared for the
> different kind of overlays.
> Do you mind if i ask you to change the commit log to add the
> information collected in this thread ?
You mean the above references?  Sure.

Thanks for the review!
Lou
>
>> We have an openflow RFP implementation in the works and once it is 
>> sufficiently mature / stable, we'll submit it too.
>>
>> Cheers,
>> Lou
> Best Regards,
>
> Acked-by: Philippe Guibert <philippe.guib...@6wind.com>
>



___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

[quagga-dev 15537] Re: Round 8

[Lou Berger]

Paul,

On 6/14/2016 9:02 AM, Paul Jakma wrote:
> Note: I've had a good few conflicts on 'install_element' lines, due to 
> various patches adding/removing commands with the restricted node and 
> 'auto-install view commands into enable' patches. I didn't look too 
> closely as to whether I resolved them correctly.
Looks like there's more to do in bgp...

#0  0x0034e0232625 in raise () from /lib64/libc.so.6
#1  0x0034e0233e05 in abort () from /lib64/libc.so.6
#2  0x004cf7ce in _zlog_assert_failed (assertion=0x52ce72 "0",
file=0x52ce68 "command.c", line=620, function=0x52fb80
"install_element")
at log.c:669
#3  0x004b02ae in install_element (ntype=ENABLE_NODE, cmd=0x7736c0)
at command.c:620
#4  0x004b02bf in install_element (ntype=VIEW_NODE, cmd=0x7736c0)
at command.c:623
#5  0x004b6e5d in cmd_init (terminal=1) at command.c:4165
#6  0x00421f8f in main (argc=1, argv=0x7fffe5d8) at
bgp_main.c:452

Lou



___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

[quagga-dev 15533] Re: Round 8

[Lou Berger]

Eak -- the branch doesn't compile with -std=c99 .

I think these changes need to be reworked with standard modern c
declarations.

Lou

On 6/14/2016 9:47 AM, Lou Berger wrote:
> Paul,
>
> Great to see this published.  Is there a quagga convention WRT C99?
>
> I see the following when compiling proposed/8
>
> if.c:1151: error: ?for? loop initial declarations are only allowed in
> C99 mode
> if.c:1151: note: use option -std=c99 or -std=gnu99 to compile your code
> zclient.c:903: error: ?for? loop initial declarations are only allowed
> in C99 mode
> zclient.c:903: note: use option -std=c99 or -std=gnu99 to compile your code
> interface.c:2505: error: ?for? loop initial declarations are only
> allowed in C99 mode
> interface.c:2505: note: use option -std=c99 or -std=gnu99 to compile
> your code
>
> Lou
>
> On 6/14/2016 9:02 AM, Paul Jakma wrote:
>> Hi,
>>
>> Took a bit longer, but the proposed patches are up in:
>>
>> http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile/patch-tracking/8/proposed/patchwork
>> http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile/patch-tracking/8/proposed/donald-take-3
>>
>> I've done some initial prioritising, so the 'restricted node' patch, 
>> 'sockaddr_dl' and table cmd are at the end, as there were already some 
>> comments on those.
>>
>> Note: I've had a good few conflicts on 'install_element' lines, due to 
>> various patches adding/removing commands with the restricted node and 
>> 'auto-install view commands into enable' patches. I didn't look too 
>> closely as to whether I resolved them correctly.
>>
>> Still to do is linearise these 2 heads together.
>>
>> 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 15532] Re: Round 8

[Lou Berger]

Paul,

Great to see this published.  Is there a quagga convention WRT C99?

I see the following when compiling proposed/8

if.c:1151: error: ?for? loop initial declarations are only allowed in
C99 mode
if.c:1151: note: use option -std=c99 or -std=gnu99 to compile your code
zclient.c:903: error: ?for? loop initial declarations are only allowed
in C99 mode
zclient.c:903: note: use option -std=c99 or -std=gnu99 to compile your code
interface.c:2505: error: ?for? loop initial declarations are only
allowed in C99 mode
interface.c:2505: note: use option -std=c99 or -std=gnu99 to compile
your code

Lou

On 6/14/2016 9:02 AM, Paul Jakma wrote:
> Hi,
>
> Took a bit longer, but the proposed patches are up in:
>
> http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile/patch-tracking/8/proposed/patchwork
> http://git.savannah.gnu.org/cgit/quagga.git/log/?h=volatile/patch-tracking/8/proposed/donald-take-3
>
> I've done some initial prioritising, so the 'restricted node' patch, 
> 'sockaddr_dl' and table cmd are at the end, as there were already some 
> comments on those.
>
> Note: I've had a good few conflicts on 'install_element' lines, due to 
> various patches adding/removing commands with the restricted node and 
> 'auto-install view commands into enable' patches. I didn't look too 
> closely as to whether I resolved them correctly.
>
> Still to do is linearise these 2 heads together.
>
> regards,



___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev

[quagga-dev 15500] Re: [PATCH 10/10] bgpd: add L3/L2VPN Virtual Network Control feature

[Lou Berger]

Philippe,

On 6/9/2016 4:15 AM, Philippe Guibert wrote:
> On Wed, Jun 8, 2016 at 9:22 PM, Lou Berger <lber...@labn.net> wrote:
>
> Hi Lou,
>>> * It seems the RD_TYPE_EOI type surfaced again (see
>>>   http://patchwork.quagga.net/patch/1728/ ), whereas I don't see where it
>>>   is used. Is there a need to keep this flag ?
>> This is used by the ethernet code, see below.
>>
> oh. It seems it is used decode_rd_type ? Would not it be better to set
> explicitly the value to RD_TYPE_EOI instead of 0xff00 ?
>
> --- a/bgpd/bgp_mplsvpn.c
> +++ b/bgpd/bgp_mplsvpn.c
> @@ -52,7 +52,7 @@ decode_rd_type (u_char *pnt)
> * VNC L2 stores LHI in lower byte, so mask it off
> */
>if ((v & 0xff00) == 0xff00)
> -v = 0xff00;
> +v = RD_TYPE_EOI;
>  #endif
>return v;
>  }

Good catch!  I suspect this got changed internally when it was removed
from the first patch set and then not restored...
 
> [...]
>>> To sum up, with the current status, I would nack the patch.
>> Perhaps this would be a good discussion for the next monthly meeting.
> If you don't mind, I would prefer a followup with emails in order to
> archive the arguments so everyone can follow this thread. Always some
> people are missing into the calls.
No problem at all. I just figured it would be good to have a higher
bandwidth discussion channel to answer any questions that folks may
have.   

> Moreover, can you provide a description of this Remote Forwarder
> Protocol (RFP) into an update of this patch serie? It is required for
> Quagga community to understand it.

Sure, as we said the RFP and VNC feature is aliened with the NVO3
architecture and

The NVE-NVA protocol used to communicate routing and Ethernet / Layer 2
(L2) forwarding information between NVAs and NVEs is referred to as the
Remote Forwarder Protocol (RFP). OpenFlow is an example RFP. 

So from a conceptual standpoint draft-ietf-nvo3-nve-nva-cp-req is a good place 
to start.

For a concrete example see 
https://www.opennetworking.org/images/stories/downloads/sdn-resources/onf-specifications/openflow/openflow-spec-v1.3.0.pdf

We have an openflow RFP implementation in the works and once it is sufficiently 
mature / stable, we'll submit it too.

Cheers,
Lou
> Thanks,
>   Philippe
>



___
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev