[quagga-dev 10469] Re: BGP patch

David Lamparter equinox at opensourcerouting.org
Tue Apr 23 13:08:44 BST 2013


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.

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.

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.)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 230 bytes
Desc: Digital signature
URL: <http://lists.quagga.net/pipermail/quagga-dev/attachments/20130423/2c95f2cc/attachment-0001.sig>


More information about the Quagga-dev mailing list