[quagga-dev 10471] Re: BGP patch
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
>> 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
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-0.99.20.1/bgpd/bgp_zebra.c (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.
> 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
Unfortunately, this is not up to me to decide. We have our management to
make this decision.
More information about the Quagga-dev