[quagga-dev 13991] Re: cleaning up commands (was Re: BGP 'view' to 'vrf')

2015-11-25 Thread David Lamparter
On Tue, Nov 24, 2015 at 11:45:27AM -0500, Lou Berger wrote: > On 11/24/2015 9:06 AM, Vincent JARDIN wrote: > > On 24/11/2015 14:38, Paul Jakma wrote: > >> So the things we need to consider: > >> > >> - Existing CLI > >> - SNMP > >> - YANG > >> - OVSDB > >> - > > - bson/json > > - XML/binary XML

[quagga-dev 13995] Re: cleaning up commands (was Re: BGP 'view' to 'vrf')

2015-11-25 Thread David Lamparter
On Wed, Nov 25, 2015 at 05:15:27PM +0100, Vincent JARDIN wrote: > On 25/11/2015 16:32, David Lamparter wrote: > > That > > component could either be in-process (using something like Swig or > > Cython to access C memory structures) or external (using some wrapping > > like Protobuf to access C

[quagga-dev 14004] [PATCH 4/4] bgpd: make bgp_info_cmp and multiple-path decision logic more regular

2015-11-25 Thread Paul Jakma
* bgp_route.c: (bgp_info_cmp) This function is supposed to return a preference between the given paths, and does so as binary either or. When mpath was added, the binary return value was left as is and instead an out parameter 'paths_eq' was added to indicate the mpath-equality case. It's

[quagga-dev 13996] Re: [PATCH] bgpd: remove 'bgp deterministic-med'

2015-11-25 Thread Paul Jakma
On Wed, 25 Nov 2015, Daniel Walton wrote: I would much rather enable it by default (we have a patch for this but it is pretty recent so hasn't been posted yet)the reasons being: * There are scenarios where the order that the paths are compared has an impact on which path is

[quagga-dev 13993] [PATCH] bgpd: remove 'bgp deterministic-med'

2015-11-25 Thread Paul Jakma
I'd love to be able to do something like the below. DMED doesn't seem like it's really fixing things, but it does add costs. Alternatively, it could be made the default, but getting rid of combinatorial loops is more attractive to me. Are there any really good reasons to keep DMED? Paul

[quagga-dev 13997] Re: Start of Next round of Commits

2015-11-25 Thread Donald Sharp
Paul - volatile/patch-tracking/5/ff has been pushed to savannah. Sorry for taking this long, I apparently have issues cut-n-pasting my ssh key :) donald On Mon, Nov 23, 2015 at 5:09 AM, Paul Jakma wrote: > On Mon, 16 Nov 2015, Donald Sharp wrote: > > Currently everything

[quagga-dev 13998] Re: [PATCH] bgpd: remove 'bgp deterministic-med'

2015-11-25 Thread Paul Jakma
On Wed, 25 Nov 2015, Paul Jakma wrote: If it goes past MED it may well just hit #10, 'prefer older', which depends on received order again. (That's actually pretty evil and probably also a malfeature). Oh, I'm all in favour of a consistent decision process, especially one that defines a

[quagga-dev 13999] Re: [PATCH 3/3] ospfd: Add unnumbered interface support

2015-11-25 Thread Donald Sharp
Joakim - The note is for the original users of Cumulus's unnumbered implementation. We would like them to continue to think about the problem in the same manner that they have been, while we changed the underlying solution. As for why path->unnumbered I wanted to make sure that the unnumbered

[quagga-dev 14002] [PATCH 2/4] bgpd: Check capability falls on right multiple of size, where possible.

2015-11-25 Thread Paul Jakma
* bgp_open.c: (cap_modsizes) Table of multiple a capability's data size should fall on, if applicable. (bgp_capability_parse) Check the header lengthcap_modsizes should fall on. Inspiration from Cumulus bgpd-capability-cleanup.patch patch, with a slightly different approach. ---

[quagga-dev 13992] Re: cleaning up commands (was Re: BGP 'view' to 'vrf')

2015-11-25 Thread Vincent JARDIN
On 25/11/2015 16:32, David Lamparter wrote: That component could either be in-process (using something like Swig or Cython to access C memory structures) or external (using some wrapping like Protobuf to access C memory structures) or badly-done external (using the CLI to poke at Quagga). I

[quagga-dev 13994] Re: [PATCH] bgpd: remove 'bgp deterministic-med'

2015-11-25 Thread Daniel Walton
I would much rather enable it by default (we have a patch for this but it is pretty recent so hasn't been posted yet)the reasons being: - There are scenarios where the order that the paths are compared has an impact on which path is selected as the bestpath. DMED makes bestpath

[quagga-dev 14003] Re: [PATCH] bgpd: remove 'bgp deterministic-med'

2015-11-25 Thread Daniel Walton
On Wed, Nov 25, 2015 at 11:46 AM, Paul Jakma wrote: > On Wed, 25 Nov 2015, Daniel Walton wrote: > > I would much rather enable it by default (we have a patch for this but it >> is >> pretty recent so hasn't been posted yet)the reasons being: >> > > * There are scenarios

[quagga-dev 14015] Re: [PATCH 4/4] bgpd: make bgp_info_cmp and multiple-path decision logic more regular

2015-11-25 Thread Donald Sharp
One comment: On Wed, Nov 25, 2015 at 12:14 PM, Paul Jakma wrote: > > @@ -552,32 +543,32 @@ bgp_info_cmp (struct bgp *bgp, struct bgp_info *new, > struct bgp_info *exist, > exist_cluster = existattre->cluster->length; > >if (new_cluster < exist_cluster) > -return

[quagga-dev 14004] Re: [PATCH] bgpd: remove 'bgp deterministic-med'

2015-11-25 Thread Paul Jakma
On Wed, 25 Nov 2015, Daniel Walton wrote: That step in bestpath is an odd one. Cisco originally put this in to stop a form of MED churn. This was before we really knew what MED churn was though so that step was added and then everyone forgot about it. A few years later we found all of

[quagga-dev 14013] Re: [PATCH 2/4] bgpd: Check capability falls on right multiple of size, where possible.

2015-11-25 Thread Donald Sharp
Paul - All 4 of these patches apply with whitespace errors. Additionally this patch fails to compile due to a missing define of CAPABILITY_CODE_SELECT_ORDER. Could you respin them? donald On Wed, Nov 25, 2015 at 3:54 PM, Donald Sharp wrote: > Acked-by: Donald

[quagga-dev 14016] Re: [PATCH 4/4] bgpd: make bgp_info_cmp and multiple-path decision logic more regular

2015-11-25 Thread Donald Sharp
Additionally, this commit breaks 'make check' donald On Wed, Nov 25, 2015 at 7:06 PM, Donald Sharp wrote: > One comment: > > On Wed, Nov 25, 2015 at 12:14 PM, Paul Jakma wrote: > >> >> @@ -552,32 +543,32 @@ bgp_info_cmp (struct bgp *bgp, struct

[quagga-dev 14014] Re: [PATCH 3/4] tests: add more AS4 capability tests + little fixes for couple of GR test cases.

2015-11-25 Thread Donald Sharp
Acked-by: Donald Sharp On Wed, Nov 25, 2015 at 12:14 PM, Paul Jakma wrote: > --- > tests/bgp_capability_test.c | 16 > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/tests/bgp_capability_test.c

[quagga-dev 14006] Re: [PATCH 1/4] bgpd: OPEN parse errors should send OPEN_ERR and UNSPECIFIC subcode.

2015-11-25 Thread Daniel Walton
Looks good Daniel On Wed, Nov 25, 2015 at 12:14 PM, Paul Jakma wrote: > CEASE NOTIFICATION for OPEN parsing errors seems, to my reading of RFC4271 > §6.2 to be incorrect. > > * bgp_packet.c: (bgp_open_receive) OPEN/UNACEP_HOLDTIME is not an > appropriate error subcode if

[quagga-dev 14005] Re: [PATCH] bgpd: remove 'bgp deterministic-med'

2015-11-25 Thread Daniel Walton
> > On the (non-deterministic) 'oldest' test. Should we kill that too then? > I'd be fine for making it do the remote-ID check (i.e. making > COMPARE_ROUTER_ID unconditionally always the case). I wouldn't, if we do we may have users start hitting MED churn. > yep, when I floated the idea of

[quagga-dev 14007] Re: [PATCH 1/4] bgpd: OPEN parse errors should send OPEN_ERR and UNSPECIFIC subcode.

2015-11-25 Thread Daniel Walton
Hey Donald when you get to bgpd-capability-cleanup.patch it touches many of the same bgp_notify_send() calls. The codes/subcodes that Paul has below look like a better fit than what is in our patch. Daniel On Wed, Nov 25, 2015 at 2:35 PM, Daniel Walton wrote: >

[quagga-dev 14010] Re: [PATCH 2/4] bgpd: Check capability falls on right multiple of size, where possible.

2015-11-25 Thread Donald Sharp
Acked-by: Donald Sharp On Wed, Nov 25, 2015 at 12:14 PM, Paul Jakma wrote: > * bgp_open.c: (cap_modsizes) Table of multiple a capability's data size > should fall on, if applicable. > (bgp_capability_parse) Check the header lengthcap_modsizes

[quagga-dev 14011] VRF global and protocol specific config

2015-11-25 Thread Vipin Kumar
Hi Lou, Based on what we have in quagga, this is how we seem to be converging on this: (I sent email previously to agree with Paul's suggestion about keeping view|vrf both) *1. Simple VRF (no lxvpn) case* *BGP mode config:* router bgp vrf ! regular peering. ! redistribution !

[quagga-dev 14008] Re: [PATCH 4/4] bgpd: make bgp_info_cmp and multiple-path decision logic more regular

2015-11-25 Thread Daniel Walton
On Wed, Nov 25, 2015 at 12:14 PM, Paul Jakma wrote: > * bgp_route.c: (bgp_info_cmp) This function is supposed to return a > preference between the given paths, and does so as binary either or. > When > mpath was added, the binary return value was left as is and instead an

[quagga-dev 14012] Re: [PATCH 1/4] bgpd: OPEN parse errors should send OPEN_ERR and UNSPECIFIC subcode.

2015-11-25 Thread Paul Jakma
On Wed, 25 Nov 2015, Donald Sharp wrote: Shouldn't the BGP_NOTIFY_OPEN_ERR sub codes be an enum? The main notify code could be, but there's not much value in the sub-code. Cause the bgp_notify_send function sub-code argument depends on the main code. This way the compiler can help make

[quagga-dev 14018] Re: Start of Next round of Commits

2015-11-25 Thread Donald Sharp
Correct Donald On Nov 26, 2015 12:19 AM, "Martin Winter" wrote: > > Donald, > > I’ll assume you abandon the proposed 5 branch on your github and continue the work on > it on Savannah? > Or is there any plan to continue using both repo’s for this branch? (I hope

[quagga-dev 13987] Re: Start of Next round of Commits

2015-11-25 Thread Martin Winter
Latest round of proposed-5 is basically equal. Results are here: https://drive.google.com/folderview?id=0B8W_T0dxQfwxSHR6UGk4RDFteEk=sharing I’ll start with a git bisect on a few of the failures over the next day. Expect more details later this week. Regards, Martin Winter On 23 Nov

[quagga-dev 13988] Re: cleaning up commands (was Re: BGP 'view' to 'vrf')

2015-11-25 Thread Paul Jakma
On Tue, 24 Nov 2015, Alexis Rosen wrote: I would be shocked if there was a consensus to move away from the current CLI. Me too. ;) The idea is to be able to support other interfaces, and allow them to be written with out specific knowledge of Quagga internals (unless they are required to

[quagga-dev 13989] Re: cleaning up commands (was Re: BGP 'view' to 'vrf')

2015-11-25 Thread Alexis Rosen
On Nov 25, 2015, at 8:29 AM, Paul Jakma wrote: > On Tue, 24 Nov 2015, Alexis Rosen wrote: >> I would be shocked if there was a consensus to move away from the current >> CLI. > > Me too. ;) > > The idea is to be able to support other interfaces, and allow them to be > written

[quagga-dev 13990] Re: cleaning up commands (was Re: BGP 'view' to 'vrf')

2015-11-25 Thread Paul Jakma
On Wed, 25 Nov 2015, Alexis Rosen wrote: So, VyOS and EdgeOS have already done that (well, Vyatta did, but that's dead now). It might be worth a little extra effort to make sure they have a chance to weigh in on this. Anything that brings the codebases closer together can only make all

[quagga-dev 14017] Re: Start of Next round of Commits

2015-11-25 Thread Martin Winter
Donald, I’ll assume you abandon the proposed 5 branch on your github and continue the work on it on Savannah? Or is there any plan to continue using both repo’s for this branch? (I hope not) Regards, Martin On 25 Nov 2015, at 9:01, Donald Sharp wrote: Paul -