feat(network): include 98-default-mac-none.link if it exists

[dustymabe]

In Fedora Linux there was a new 98-default-mac-none.link file added to set the MACAddressPolicy=none for bond/bridge/team devices. We'd like for this policy to apply in the initramfs as well. See

• https://fedoraproject.org/wiki/Changes/MAC_Address_Policy_none
• https://src.fedoraproject.org/rpms/systemd/pull-request/100#

Checklist

• I have tested it locally

I have ✔️ - at least for the 35network-manager module.

• I have reviewed and updated any documentation if relevant

Not sure if any docs exist for this.

• I am providing new code and test(s) for it

Not sure if this is a large enough change for a new test, but let me know and I'll be glad to take a look.

[dustymabe]

One could argue that since this change was applied to Fedora we should just carry this change downstream as well. I'm OK with that, but also feel like it could be accepted upstream since it's an optional include.

[thom311]

FWIW, this change looks good to me :)

[johannbg]

From my pov this is more reasonable default to have then what systemd upstream has so LGTM

[dustymabe]

Any other reviews needed before merge? I'm also unsure about the CI failure.

[aafeijoo-suse]

From my pov this is more reasonable default to have then what systemd upstream has so LGTM

While this might be reasonable, why should a custom Fedora patch go upstream? We discussed in the past that this should not happen, 98-default-mac-none.link is only included in Fedora, should we include upstream other custom files shipped only with each distro?

[dustymabe]

98-default-mac-none.link is only included in Fedora, should we include upstream other custom files shipped only with each distro?

that's a fair question, I'm OK with whatever we decide here.

[LaszloGombos]

With #2250 this PR would be only a one line change in one dracut module, which at least would decrease the maintenance cost of this PR as well.

[LaszloGombos]

I wrote down a few of my thoughts on how to review distribution specific changes on the wiki. In the past there was very little consistency around the reviews of distribution specific changes.

https://github.com/dracutdevs/dracut/wiki/Dracut-development#reviews

[LaszloGombos]

@dustymabe

We'd like for this policy to apply in the initramfs as well.

As far as I can tell, this PR adds an extra file (98-default-mac-none.link) for a non-hostonly initramfs. What happens for hostonly initramfs ? Is that file already included for hostonly ?

[johannbg]

@LaszloGombos distribution specific changes aren't allowed regardless if you write what you feel about it down on the wiki or elsewhere on the internet now I just have to delete that page and or rewrite what you wrote in a manner that eliminates any doubt about that. We do not carry downstream distribution specific patches and workarounds period. There is no if's and but's about it. We are getting ourselves out of that rabbit hole not deeper in-to-it. Fedora/RHEL is no exception of that rule so @aafeijoo-suse is right however the question here is, is the conclusion that was reached in Fedora to carry this ( a conclusion that could have been reached anywhere else ) a good sane upstream default to have, in which everyone benefits from, thus it makes sense for us to carry. I deemed it was YMMV in that regard as might Antonio's and that's what we are ( supposed to be ) discussion here not where the PR originated from.

[aafeijoo-suse]

the question here is, is the conclusion that was reached in Fedora to carry this ( a conclusion that could have been reached anywhere else ) a good sane upstream default to have, in which everyone benefits from, thus it makes sense for us to carry.

Ahh I was missing this point, because for that the PR is incomplete and it should add the 98-default-mac-none.link file to a common place like the network module and install it from there.

[dustymabe]

Ahh I was missing this point, because for that the PR is incomplete and it should add the 98-default-mac-none.link file to a common place like the network module and install it from there.

This stuff is a bit over my head. I just put this inclusion in the same place the 99-default.link was also included. Is there a better way?

[LaszloGombos]

@dustymabe I would like to understand the benefit of this PR better for consideration in the next release of dracut. Hope you can help a bit with some data :-)

Is there a distribution where 98-default-mac-none.link file is already released ? What about latest Fedora Rawhide ?

Can you please confirm that there is a simple workaround for this PR that does not require to patch dracut by using
a command line option (--install) or by adding a drop in config file (install_items) in https://src.fedoraproject.org/rpms/dracut/tree/rawhide to include this additional file ? Just trying to understand the impact of the PR.

[dustymabe]

@dustymabe I would like to understand the benefit of this PR better for consideration in the next release of dracut. Hope you can help a bit with some data :-)

Is there a distribution where 98-default-mac-none.link file is already released ? What about latest Fedora Rawhide ?

Yes. It's in rawhide and Fedora 38.

Can you please confirm that there is a simple workaround for this PR that does not require to patch dracut by using a command line option (--install) or a drop in config file (install_items) to include this additional file ? Just trying to understand the impact of the PR.

Of course. You can do anything with the command line, though you don't typically want to require users to know every file to include in their initramfs, which is why we have the modules with their own definitions of what files should be included (even optional files to include).

Typically changes like this would go downstream, but since the include is optional (i.e. if the file doesn't exist then it doesn't get included and there is no harm) then I figured it wouldn't hurt to try this upstream.

As @johannbg mentioned other distributions may want to pick up the change that Fedora did, but I hadn't really considered that when opening this PR. Either way, I'm happy to submit the PR downstream. I just wanted to have a discussion here about it first.

[LaszloGombos]

Yes. It's in rawhide and Fedora 38.

Thanks. Sorry I missed that it is already merged and released. Our CI is on Fedora stable, which I think is Fedora 37, so this change will hit our CI only in about 2 months. I do not think we need to wait 2 months to get this PR moving, but good to have this info at hand.

Of course. You can do anything with the command line, though you don't typically want to require users to know every file to include in their initramfs,

Sometimes dracut module itself needs to be patched instead of just adding a new file, which is a lot more maintenance downstream - which is something I consider when making a decision . Also I was not suggesting the user to do this, the distribution itself can make this change - e.g. here https://src.fedoraproject.org/rpms/dracut/tree/rawhide .

At this point of time (e.g. in the timeframe of the next dracut release), I do consider this change distribution specific, which I am open to accept. I would however hope to reduce the change in this PR.

I propose to land #2250 first and rebase this PR on top of #2250 and make this change in modules.d/45net-lib/module-setup.sh (change 1 file instead of 3 files).

[dustymabe]

Yes. It's in rawhide and Fedora 38.

Thanks. Sorry I missed that it is already merged and released. Our CI is on Fedora stable, which I think is Fedora 37, so this change will hit our CI only in about 2 months. I do not think we need to wait 2 months to get this PR moving, but good to have this info at hand.

Of course. You can do anything with the command line, though you don't typically want to require users to know every file to include in their initramfs,

Sometimes dracut module itself needs to be patched instead of just adding a new file, which is a lot more maintenance downstream - which is something I consider when making a decision . Also I was not suggesting the user to do this, the distribution itself can make this change - e.g. here https://src.fedoraproject.org/rpms/dracut/tree/rawhide .

Yep. That is where I would open a PR (downstream) if this PR was rejected. Even if it did merge today, I'd also open a temporary backport PR, so either way I'll be opening a PR downstream. In one case we'll carry the patch forever and the other we'll carry it until the next release.

At this point of time (e.g. in the timeframe of the next dracut release), I do consider this change distribution specific, which I am open to accept. I would however hope to reduce the change in this PR.

I propose to land #2250 first and rebase this PR on top of #2250 and make this change in modules.d/45net-lib/module-setup.sh (change 1 file instead of 3 files).

That works for me.

Since I have a shorter timeframe I'll probably open a PR downstream for this specific change and then link to this upstream PR and say that we can drop the downstream patch if this upstream PR ever merges.

When #2250 merges I can rebase this PR and re-test the change.

[dustymabe]

Downstream PR landed in https://src.fedoraproject.org/rpms/dracut/pull-request/33#

Should I close this PR?

[LaszloGombos]

@dustymabe Thanks for the update.

IMHO #2255 needs to land first so that we would be able to observe the impact of this change here upstream as well on the bot.

After that - as mentioned earlier - with #2250 this PR would be only a one line change in one dracut module, which at least would decrease the maintenance cost of this PR as well.

[lnykryn]

The change itself makes sense. Bonds have always gotten the same MAC address as their first interface. So I can image that the current policy can bring issues.