network: several cleanups and fix IPv4DAD and IP Masqurade #17240
Conversation
3518bd8 to
4170786
Compare
2af5f84 to
ba4677f
Compare
keszybz
left a comment
There was a problem hiding this comment.
Looks very nice.
I made a few comments that follow the overall pattern of renaming foo_verify_bar to foo_drop_invalid_bar. This is a suggestion, and if you think _verify_ is OK, I'm fine with that. If you decide to rename them, I think it'd be better to do it in a single commit at the end, to avoid nasty rebasing and patch failures.
| }) | ||
|
|
||
| #define netlink_add_match(nl, ret_slot, metch, callback, destroy_callback, userdata) \ | ||
| #define netlink_add_match(nl, ret_slot, match, callback, destroy_callback, userdata, description) \ |
There was a problem hiding this comment.
Hmm, but match was referenced below? Did we happen to always have a match variable in the scope?
There was a problem hiding this comment.
I was introduced the macro in the past, but it is never used. So, I did not notice the typo...
| address->section->filename, address->section->line); | ||
|
|
||
| address->scope = RT_SCOPE_HOST; | ||
| } |
There was a problem hiding this comment.
if (in_addr_is_localhost(address->family, &address->in_addr) > 0) {
if (address->family == AF_INET && address->scope_set && address->scope != RT_SCOPE_HOST)
log_warning_errno(SYNTHETIC_ERRNO(EINVAL),
"%s: non-host scope is set in the [Address] section from line %u. "
"Ignoring Scope= setting.",
address->section->filename, address->section->line);
address->scope = RT_SCOPE_HOST;
address->scope_set = true;
}?
There was a problem hiding this comment.
My change is intended the following:
For IPv4, scope should be always RT_SCOPE_HOST.
For IPv6, use RT_SCOPE_HOST when scope is not explicitly specified.
I keep the code, and added the above comments in the code.
And reduce scope of variables.
[RoutingPolicyRule] sections are managed by both LIST and Hashmap. Let's drop list.
Previously, IPv4 DAD is configured in each Address object stored in Network object. If a .network file matches multipe links, then it causes an assertion. To prevent it, now IPv4 DAD is configured in each Address object belogs to Link object.
When the MAC address of a link is updated, an address on the link may be under checking address duplication. Or, (currently such code is not implemented yet, but) address duplication check may be restarted later. For that case, the IPv4 ACD clients must use the new updated MAC address.
For IPv6 case, use RT_SCOPE_HOST only when scope is not explicitly specified.
Previously, address_establish() took Address object stored in Network object. And address_release() took Address object stored in Link object. Thus, address_release() always did nothing.
As 'verify' implies a boolean result.
We usually disable IPv6AcceptRA= if the test does not require any dynamic address configuration, as it makes slightly slow down the test. C.f. 491b79a.
ba4677f to
6649345
Compare
|
@keszybz Thank you for reviewing such a huge PR. Most suggestions are applied. For others, please see the above comments. PTAL. |
|
LGTM. |
Most commits should be trivial, they move functions from networkd-link.c or networkd-manager.c to relevant files, or drop redundant or unused entries.
I hope only last few commits fix some minor issues.