Paths

Table of Contentst

Differential D58305

net/if.c: Add fib-aware ifa_ifwithaddr()
AcceptedPublic
Actions

Authored by pouria on Fri, Jul 17, 4:17 PM.

Tags

None

Referenced Files

	F163065152: D58305.diff
	Sun, Jul 19, 6:41 PM

	F163020314: D58305.diff
	Sun, Jul 19, 6:39 AM

	Unknown Object (File)
	Sat, Jul 18, 5:05 PM

	Unknown Object (File)
	Sat, Jul 18, 6:16 AM

Subscribers

Details

Reviewers

glebius
markj
bnovkov

Group Reviewers

Summary

Add FIB selection logic by introducing ifa_ifwithaddr_fib() to
support FIB-specific lookups. Then have ifa_ifwithaddr() wrap it
with RT_ALL_FIBS.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 74889
Build 71772: arc lint + arc unit

Event Timeline

pouria created this revision.Fri, Jul 17, 4:17 PM

Herald added subscribers: melifaro, ae, imp. · View Herald TranscriptFri, Jul 17, 4:17 PM

pouria requested review of this revision.Fri, Jul 17, 4:17 PM

Harbormaster completed remote builds in B74888: Diff 182141.Fri, Jul 17, 4:17 PM

pouria added a child revision: D58281: bind(2): Lookup local address in current FIB if '*.bind_all_fibs' is active.Fri, Jul 17, 4:19 PM

glebius added inline comments.Fri, Jul 17, 4:21 PM

sys/net/if.c
1746	The ARGSUSED is no longer relevant today and for this function it is not even correct. A very good change to remove it once and for all.

glebius accepted this revision as: glebius.Fri, Jul 17, 4:22 PM

This revision is now accepted and ready to land.Fri, Jul 17, 4:22 PM

Remove ARGUSED comment.

This revision now requires review to proceed.Fri, Jul 17, 4:24 PM

Harbormaster completed remote builds in B74889: Diff 182142.Fri, Jul 17, 4:24 PM

pouria marked an inline comment as done.Fri, Jul 17, 4:26 PM

pouria added inline comments.

sys/net/if.c
1746	The ARGSUSED is no longer relevant today and for this function it is not even correct. A very good change to remove it once and for all. I've removed all of it in this file. Should I do this, or should I only remove it from that specific function?

glebius added inline comments.Fri, Jul 17, 4:30 PM

sys/net/if.c
1746	No, please don't mix the cleaning sweep with a new KPI change into one commit. It doesn't make sense to copy the comment, so just drop it from this change. The cleaning sweep should be a separate revision. You can actually go wider than just this file, probably cover entire sys/net*. That would of course require Src Committers and srcmgr as reviewers. IMHO, everybody should approve. But there might be some disagreement. I don't know of any modern use for this comment.

bnovkov added inline comments.Fri, Jul 17, 4:30 PM

sys/net/if.c
1754	It would be great to introduce a fib-aware flavor of this interface as well, otherwise we'll end up repeating the `NET_EPOCH_{ENTER,EXIT}` pattern every time we call `ifa_ifwithaddr_fib`

bz added subscribers: phk, bz.Fri, Jul 17, 4:35 PM

bz added inline comments.

sys/net/if.c
1746	lint-ers (lint, flexelint, ..) used to evaluate this to not warn about unused function arguments if my memory serves me right. One would hope that unused is the better annotation these days? @phk may know more details.

glebius added inline comments.Fri, Jul 17, 4:41 PM

sys/net/if.c
1746	lint-ers (lint, flexelint, ..) used to evaluate this to not warn about unused function arguments if my memory serves me right. One would hope that `__unused` is the better annotation these days? All correct!

Revert removed ARGUSED comments on other functions.
Address @bnovkov comment by adding ifa_ifwithaddr_fib_check() version.

Harbormaster completed remote builds in B74898: Diff 182151.Fri, Jul 17, 7:14 PM

pouria marked 4 inline comments as done.Fri, Jul 17, 7:15 PM

pouria mentioned this in D58294: netlink: Add RTA_PREFSRC support.Fri, Jul 17, 9:19 PM

glebius accepted this revision as: glebius.Fri, Jul 17, 11:53 PM

This revision is now accepted and ready to land.Fri, Jul 17, 11:53 PM

@bnovkov Should I wait for you to test, or are we good to land this?

pouria added a child revision: D58294: netlink: Add RTA_PREFSRC support.Sat, Jul 18, 12:15 PM

Revision Contents
Changeset List

Path

Size

sys/

net/

21 lines

1 line

Diff 182142

sys/net/if.c

Loading...

sys/net/if_var.h

Loading...