[quagga-dev 10471] Re: BGP patch

boris yakubov borisyakubov at ruggedcom.com
Tue Apr 23 14:46:54 BST 2013

On 23/04/13 08:08 AM, David Lamparter wrote:
> On Mon, Apr 22, 2013 at 02:50:01PM -0400, boris yakubov wrote:
>> On 20/04/13 12:10 AM, David Lamparter wrote:
>>> P.S.: I have used BGP with link-local IPv6 addresses in 0.99.20 without
>>> patch and have not seen any problems...
>> I really doubt that.
>> For 0.99.20 even the plain BGP for ipv6 is not supported. I had to put
>> more changes in 0.99.20, in bgp_zebra_announce and bgp_zebra_withdraw to
>> assign
>> api.safi   = safi; Without this part zebra would not see the change in
>> the right table (just do "show ipv6 route" on zebra). This part was
>> actually fixed in the latest quagga version, that's why I didn't want to
>> include it in my patch.
>> We may not talk about the same quagga version.
> Please be more specific in your description.  You have provided neither
> code location nor git commit id.  The "api.safi = safi" line in
> bgp_zebra_announce has been added the same time that SAFI support has
> been added.  This is correct, and was correct before.
In the patch I provided it is very easy to find the file name and the 
line number.
Why do you expect the git commit id at this stage, while this is merely 
a preliminary review?
> As such, I cannot find any issue matching your description.
> In general, when anyone writes an e-mail about some issue with the code,
> I expect there to be filename, line number and git commit information.
> You are one person writing the e-mail.  There are probably 10 people
> reading the e-mail.  Either you spend 15 minutes adding the information,
> or 10 people spend 15 minutes each = 150 minutes doing that.  At that
> point most people will just ignore your mail, seeing how if you can't be
> bothered to have useful information, they can't be bothered to read it.
>> As per my patch, inside bgp_nexthop_set:  if_lookup_by_ipv4 or
>> if_lookup_by_ipv6  lookup routines have only 1 parameter, the address
>> and they use the iflist as a global variable, this list needs to be set
>> to the specific list of the specific bgp instance. One may think (as I
>> did) that iflist is a global list of all interface on the target (which
>> doesn't make sense in the case of the vrf support),
> Quagga does not have VRF support.  VRF support is not an easy thing to
> do, it needs very thoughtful evaluation of what to support where.  It's
> not even neccessarily much code, but it tends to blow up in places just
> like what you found.
>> however with all the tests I did the iflist is not set to be the right
>> value unless I add my change and then it works fine.
> Well, it seems that you or your company went and added some VRF support
> on your own, looking at your patch:
> quagga- (bgp_nexthop_set(), l. 571pp)
> +  /* Set the iflist context before the if_lookup_* are called */
> +  iflist = peer->bgp->iflist;
> There is no iflist member in struct bgp.  It appears that you have found
> an issue with your own, local changes to the Quagga code.  Your patch
> does not work with master Quagga, in fact it breaks it.
> Now, while we're on a development mailing list, and tone on such lists
> tends to be very much down to the matter at hand, I would like to point
> out that your conduct has been less than exemplary.  In particular, when
> I tell you that I successfully use bgpd with link-local addresses,
> responding with "I doubt that" reeeeeallly isn't the best way to start
> off with.  Let's please keep the discussions well-argued with facts and
> references to code and specification.
> Cheers,
> -David
> P.S.:  please note that Quagga is GPL code, and as such when you sell
> devices to your customers containing modified versions of Quagga, you
> are required to make your changes available.  (In particular your VRF
> support.)
Unfortunately, this is not up to me to decide. We have our management to 
make this decision.

More information about the Quagga-dev mailing list