Bug 232231 - net/bird: patch - wrong LSA collision detection
Summary: net/bird: patch - wrong LSA collision detection
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Olivier Cochard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-10-14 00:19 UTC by Rick
Modified: 2018-11-10 23:05 UTC (History)
3 users (show)

See Also:
olivier: maintainer-feedback+


Attachments
net/bird successful build log following patching (274.12 KB, text/plain)
2018-10-14 00:19 UTC, Rick
no flags Details
add birdvty user (1.90 KB, patch)
2018-10-14 00:24 UTC, Rick
no flags Details | Diff
Fix bird/bird6 wrong LSA collision detection (2.96 KB, patch)
2018-10-14 00:25 UTC, Rick
no flags Details | Diff
add birdvty user - v2 (1.54 KB, patch)
2018-11-05 12:15 UTC, Asbjorn Mikkelsen
no flags Details | Diff
Fix bird/bird6 wrong LSA collision detection - v2 (2.95 KB, patch)
2018-11-05 12:18 UTC, Asbjorn Mikkelsen
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rick 2018-10-14 00:19:32 UTC
Created attachment 198106 [details]
net/bird successful build log following patching

Submitted patches:

1) add birdvty user
2) Fix bird/bird6 wrong LSA collision detection

Patch attribution is Asbjorn Mikkelsen, amikkelsen@verisign.com.
Comment 1 Rick 2018-10-14 00:24:37 UTC
Created attachment 198107 [details]
add birdvty user
Comment 2 Rick 2018-10-14 00:25:58 UTC
Created attachment 198108 [details]
Fix bird/bird6 wrong LSA collision detection
Comment 3 Rick 2018-10-14 00:27:16 UTC
Patch attributions: Julien Charbon <jcharbon@verisign.com>, Asbjorn Mikkelsen <amikkelsen@verisign.com>
Comment 4 Olivier Cochard freebsd_committer freebsd_triage 2018-10-14 08:54:22 UTC
Thanks for your patches!

Few questions:
You've choose to create a net/bird/pkg-install to create birdvty group.
1. Why not declaring this new group into /usr/ports/GIDs in place of the pkg-install ?
2. Do you think it's a good idea to reuse the same group for net/bird2 ?

I believe you did all this works during your working time for Verisign.
3. Are you agree if I add a "Sponsored by: Verisign" to the commit log?
Comment 5 Olivier Cochard freebsd_committer freebsd_triage 2018-10-18 04:21:01 UTC
and another question:
4. Patch fixing LSA collision: Does this patch was send upstream ? (if it apply to the branch 2.0)
5. Patch fixing LSA collision: It doesn't apply well.
File proto/ospf/topology.c of bird 1.6.4, line 281 is:
if (!en->nf || !en->lsa_body)

But the patch is referencing an original line 281 which is:
if (en->lsa_body == NULL)
Comment 6 Julien Charbon freebsd_committer freebsd_triage 2018-10-18 07:25:05 UTC
Hi Olivier, thanks for the followup:

> 1. Why not declaring this new group into /usr/ports/GIDs in place of the pkg-install ?

 No reasons, we should /usr/ports/GIDs instead right.  Will update the corresponding patch.

> 2. Do you think it's a good idea to reuse the same group for net/bird2 ?

 It is a good idea for bird2 too.

> I believe you did all this works during your working time for Verisign.
> 3. Are you agree if I add a "Sponsored by: Verisign" to the commit log?

 Perfect.

> 4. Patch fixing LSA collision: Does this patch was send upstream ? (if it apply to the branch 2.0)

 The patch was not sent upstream. Will see if the patch needs update for bird branch 2.0.

> 5. Patch fixing LSA collision: It doesn't apply well.
> File proto/ospf/topology.c of bird 1.6.4, line 281 is:
> if (!en->nf || !en->lsa_body)
> But the patch is referencing an original line 281 which is:
> if (en->lsa_body == NULL)

 Good catch. Interesting actually 'make patch' does not complains:

$ make clean distclean
===>  Cleaning for bird-1.6.4_1
===>  Cleaning for bird6-1.6.4_1
===>  Deleting distfiles for bird-1.6.4_1
$ ls files/patch-proto__ospf__topology.c
files/patch-proto__ospf__topology.c
$ make patch          
===>  License GPLv2 accepted by the user
===>   bird-1.6.4_1 depends on file: /usr/local/sbin/pkg - found
=> bird-1.6.4.tar.gz doesn't seem to exist in /app/jcharbon/freebsd-ports/distfiles/.
=> Attempting to fetch ftp://bird.network.cz/pub/bird/bird-1.6.4.tar.gz
bird-1.6.4.tar.gz                             100% of  994 kB  516 kBps 00m02s
===> Fetching all distfiles required by bird-1.6.4_1 for building
===>  Extracting for bird-1.6.4_1
=> SHA256 Checksum OK for bird-1.6.4.tar.gz.
===>  Patching for bird-1.6.4_1
===>  Applying FreeBSD patches for bird-1.6.4_1

 But still, the patch is not clean against bird-1.6.4, will update the patch
Comment 7 Asbjorn Mikkelsen 2018-11-05 12:15:58 UTC
Created attachment 198963 [details]
add birdvty user - v2
Comment 8 Asbjorn Mikkelsen 2018-11-05 12:18:01 UTC
Created attachment 198965 [details]
Fix bird/bird6 wrong LSA collision detection - v2
Comment 9 Asbjorn Mikkelsen 2018-11-05 12:21:21 UTC
Hi Olivier,

I have updated the two patches with a "- v2" suffix. We have also submitted the patch upstream:
https://github.com/BIRD/bird/pull/3

Please let me know if you have any further comments or questions.

Best regards
Comment 10 commit-hook freebsd_committer freebsd_triage 2018-11-10 23:03:44 UTC
A commit references this bug:

Author: olivier
Date: Sat Nov 10 23:02:45 UTC 2018
New revision: 484648
URL: https://svnweb.freebsd.org/changeset/ports/484648

Log:
  Fixes wrong LSA collision detection and add birdvty group

  PR:		232231
  Submitted by:	Julien Charbon <jcharbon@verisign.com>
  Submitted by:	Asbjorn Mikkelsen <amikkelsen@verisign.com>
  Sponsored by:	Verisign

Changes:
  head/GIDs
  head/net/bird/Makefile
  head/net/bird/files/bird.in
  head/net/bird/files/bird6.in
  head/net/bird/files/patch-proto__ospf__lsupd.c
  head/net/bird/files/patch-proto__ospf__topology.c
Comment 11 Olivier Cochard freebsd_committer freebsd_triage 2018-11-10 23:05:38 UTC
Thanks for your patches!