-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC/RDY] kernelmptcp: 0.91.3 -> 0.92.1 #29226
Conversation
@layus any tip :) ? |
Hi @teto, Thank you for pinging me. I lost interest on this because seemingly nobody was using it. I am glad to be wrong :-). To update the mptcp kernel, you need to mimic the corresponding kernel in nixpkgs. linux 4.4 was recently removed, so you have to copy it from git history at nixpkgs/pkgs/top-level/all-packages.nix Lines 12089 to 12100 in 9917450
In this case, mptcp 92.1 is a 4.4.83 kernel as per https://github.com/multipath-tcp/mptcp/blob/v0.92.1/Makefile#L1-L3. You also need to update the modDirVersion and branch accordingly.
In your case I guess the meta.branch attribute was missing. That one is used in picking the right config options in https://github.com/NixOS/nixpkgs/blob/master/pkgs/os-specific/linux/kernel/common-config.nix. |
See also teto#1 ;-) |
EDIT: setting That improved the situation quite a bit, i.e., it now compiles
as it doesn't sound too dramatic, I tried booting it via
|
@teto Part 1 does not seem too dramatic. It is mostly a failure to detect elf files that leads patchelf to fail verbosely in nixpkgs/pkgs/build-support/setup-hooks/audit-tmpdir.sh Lines 22 to 23 in e3b562e
And yes, you need Does it work for you now ? |
I booted it and it worked. I've rebased and updated the PR. Thanks for your help. |
Why am I requested for a review as code owner? I don't own these parts. |
I've not requested any special reviewer, I thought it was an automatic thing ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't say anything about this change. Not sure why my review was requested.
I think I get why: my first rebase was wrong so it included many commits from other areas (python commits etc). I've force pusehd a fix but the bot didn't rescan it to fix the required reviews. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It compiles and the required patches are applied. LGTM!
Any update ? the old kernel is troublesome with setuid NixOS/nixops#745 |
@NeQuissimus You may be the best one to review this, if another review is still needed. As for me, I would merge this ASAP, as 1) It works and 2) @teto seems to be one of the very few to use it. If he thinks it's ready, it is. |
LGTM from that I can tell. Never used this before... |
It's very possible that I write some sort of tests for this kernel for my personal usage. I wouldn't mind upstreaming it (or a subset) if there is any intereset for nixos. I could understand that testing a low interest package might be more a burden than anything for the nixos team anyway let me know. I confirm that rebooting with this kernel displayed yes on amiusingmptcp.de . |
linux_mptcp = callPackage ../os-specific/linux/kernel/linux-mptcp.nix { | ||
kernelPatches = | ||
[ kernelPatches.bridge_stp_helper | ||
kernelPatches.p9_fixes | ||
kernelPatches.DCCP_double_free_vulnerability_CVE-2017-6074 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you remove the DCCP kernel patch, it is not referenced anywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is already removed from pkgs/os-specific/linux/kernel/patches.nix I believe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't seem so:
$ rg DCCP_double_free
pkgs/top-level/all-packages.nix
12402: kernelPatches.DCCP_double_free_vulnerability_CVE-2017-6074
pkgs/os-specific/linux/kernel/patches.nix
61: DCCP_double_free_vulnerability_CVE-2017-6074 = rec
62: { name = "DCCP_double_free_vulnerability_CVE-2017-6074.patch";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both locally and on github interface it looks gone
https://github.com/NixOS/nixpkgs/pull/29226/files#diff-608de687ae1a5f9a6b1796f184eca73bL61
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the pkgs/os-specific/linux/kernel/patches.nix
file?
https://github.com/NixOS/nixpkgs/blob/master/pkgs/os-specific/linux/kernel/patches.nix#L61-L68
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
either I am tired or you are checking out the wrong branch ? I do see the DCCP lines in pkgs/os-specific/linux/kernel/patches.nix prefixed with a '-' in https://patch-diff.githubusercontent.com/raw/NixOS/nixpkgs/pull/29226.diff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry not sure how I missed that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no pb thanks for merging
Usually, it is best to look at existing tests in nixos/tests :-). I would be glad to test your tests, and help writing them if need be. |
Motivation for this change
I would like to benefit from the many 0.92 additions (http://multipath-tcp.org/pmwiki.php?n=Main.Release92), mostly additional socket options.
Things done
I've just changed the version number/updated sha, yet when I want to build the kernel, I have some trouble upon the config generation (because it's a different kernel):
I am not sure what's the best course of action is to solve this kind of problems ?
Cheers
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)