Page MenuHomeFreeBSD

net/aquantia-atlantic-kmod: Fix CURRENT build post r353868 (iflib changes)
ClosedPublic

Authored by nyan_myuji.xyz on Jan 13 2021, 2:49 PM.
Tags
None
Referenced Files
F77947382: D28135.id82217.diff
Sun, Feb 25, 12:07 PM
Unknown Object (File)
Fri, Feb 23, 4:22 AM
Unknown Object (File)
Fri, Feb 23, 4:13 AM
Unknown Object (File)
Fri, Feb 23, 3:26 AM
Unknown Object (File)
Fri, Feb 23, 3:05 AM
Unknown Object (File)
Jan 17 2024, 11:44 PM
Unknown Object (File)
Jan 14 2024, 9:41 AM
Unknown Object (File)
Jan 12 2024, 1:11 AM

Details

Reviewers
lwhsu
philip
koobs
Summary
net/aquantia-atlantic-kmod: Fix CURRENT build post r353868 (iflib changes)

This patch enable the port to build working if_atlantic.ko
on 13-CURRENT, broken since r353868, where `if_multi_apply`
and `if_multiaddr_count` were removed.

PR: 252642
Approved by: koobs (maintainer)
Differential_Revision: D28135
MFH: 2021Q1 (blanket: build fix)
Test Plan
  • portlint: ???
  • testport: ??? (poudriere; at least current and 12amd64)
  • hardware: AQC107 (device id 0x07b1) tested

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 36170
Build 33059: arc lint + arc unit

Event Timeline

lwhsu requested changes to this revision.Jan 13 2021, 3:44 PM
lwhsu added a subscriber: lwhsu.
lwhsu added inline comments.
net/aquantia-atlantic-kmod/Makefile
5

I believe PORTREVISION is enough here.

This revision now requires changes to proceed.Jan 13 2021, 3:44 PM
net/aquantia-atlantic-kmod/Makefile
5

The reason why PORTEPOCH is used here is to keep the versioning in-sync with the upstream. or should we change PORTVERSION to something like 0.0.5p1 ?

net/aquantia-atlantic-kmod/Makefile
5

PORTREVISION is exactly what to use to add local patch and keep PORTVSION (or DISTVERSION) sync with upstream. PORTEPOCH should be used carefully because it will stay forever once comes in.

net/aquantia-atlantic-kmod/Makefile
5

my bad, I thought you mean PORTVERSION lmao

Use PORTREVISION instead of PORTEPOCH

This revision is now accepted and ready to land.Jan 13 2021, 5:37 PM
philip requested changes to this revision.Jan 14 2021, 12:50 AM

If the iflib changes have been tested and are known to work, they're probably okay.

Is the extra patch to aq_hw.c intentional? Using random() to generate a MAC address isn't great. Not seeding it makes it even worse.

This revision now requires changes to proceed.Jan 14 2021, 12:50 AM
koobs retitled this revision from Fix net/aquantia-atlantic-kmod post r353868 iflib changes to net/aquantia-atlantic-kmod: Fix CURRENT build post r353868 (iflib changes).Jan 14 2021, 2:11 AM
koobs edited the summary of this revision. (Show Details)
koobs edited the test plan for this revision. (Show Details)
koobs added a reviewer: koobs.

This needs:

  • Confirmation of QA: portlint/poudriere on at least CURRENT and 12amd64 (fill in TEST PLAN section)
  • PR created upstream
  • Comment added to patch with upstream issue/PR reference and URL

Thank you for the report and patch

koobs added 1 blocking reviewer(s): koobs.
koobs edited the summary of this revision. (Show Details)
net/aquantia-atlantic-kmod/Makefile
5

Is this actually required since the build fails to produce a package?

If the iflib changes have been tested and are known to work, they're probably okay.

Is the extra patch to aq_hw.c intentional? Using random() to generate a MAC address isn't great. Not seeding it makes it even worse.

Yes it is. This is because random has been deprecated and srandom has been removed at https://reviews.freebsd.org/rS356097

net/aquantia-atlantic-kmod/Makefile
5

I think I don't exactly know what you mean..
On my side poudriere testport passed and is able to produce package for both current and 12.2-release...
Portlint also passed.

net/aquantia-atlantic-kmod/Makefile
5

Yes bump PORTVERSION in this case can be discussed more, but since the package content has changed and it would be good for notifying people using -CURRENT. I believe it's a good step.

Use arc4random instead of random

Thanks for the changes!

@philip @lwhsu Thank you both for review. Feel free to commit this (pending adding upstream comments to patches).

net/aquantia-atlantic-kmod/files/patch-aq__hw.c
2

Lets add the upstream (PR) description and link here

net/aquantia-atlantic-kmod/files/patch-aq__main.c
2

Lets add the upstream (PR) description and link here

Has all the feedback that's been given been addressed?

If so, who's going to have the honour of commiting and pushing this? :)

@koobs Please review and commit this if it's alright?

I'm happy to commit this later today if @koobs doesn't beat me to it.

@philip I may not have time to day, but the change is otherwise approved

This revision is now accepted and ready to land.Feb 9 2021, 12:37 AM

@koobs If you really don't have time, how about passing maintainership to @nyan_myuji.xyz ? He has more changes planned and is willing to take care of this port. I will help him to land more patches.

@koobs If you really don't have time, how about passing maintainership to @nyan_myuji.xyz ? He has more changes planned and is willing to take care of this port. I will help him to land more patches.

To clarify: I don't currently have time

I have no qualms with a MAINTAINER change however.

@koobs If you really don't have time, how about passing maintainership to @nyan_myuji.xyz ? He has more changes planned and is willing to take care of this port. I will help him to land more patches.

To clarify: I don't currently have time

I have no qualms with a MAINTAINER change however.

Sounds good. I will see what his next updating to this port looks like.