Bug 232231

Summary: net/bird: patch - wrong LSA collision detection
Product: Ports & Packages Reporter: Rick <vrwmiller>
Component: Individual Port(s)Assignee: Olivier Cochard <olivier>
Status: Closed FIXED    
Severity: Affects Only Me CC: amikkelsen, jch, w.schwarzenfeld
Priority: --- Flags: olivier: maintainer-feedback+
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
net/bird successful build log following patching
none
add birdvty user
none
Fix bird/bird6 wrong LSA collision detection
none
add birdvty user - v2
none
Fix bird/bird6 wrong LSA collision detection - v2 none

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 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 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 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 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 2018-11-10 23:05:38 UTC
Thanks for your patches!