Skip to content

Handle changed section .rodata._entry.XXX not selected for inclusion #1206

Closed
Closed
Handle changed section .rodata._entry.XXX not selected for inclusion#1206

Description

@liu-song-6
liu-song-6
opened on Aug 4, 2021

Hi,

I am trying to apply a revert via livepatch:

From 6f639e60c747af10e34f43706314e11b3b30c3e3 Mon Sep 17 00:00:00 2001
From: Song Liu <song@kernel.org>
Date: Wed, 4 Aug 2021 13:18:16 -0700
Subject: [PATCH 4/5] Revert "tcp: fix marked lost packets not being
 retransmitted"

This reverts commit e176b1ba476cf36f723cfcc7a9e57f3cb47dec70.
---
 net/ipv4/tcp_input.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 506222574750..1c6153247d23 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -979,10 +979,9 @@ static void tcp_check_sack_reordering(struct sock *sk, const u32 low_seq,
 /* This must be called before lost_out is incremented */
 static void tcp_verify_retransmit_hint(struct tcp_sock *tp, struct sk_buff *skb)
 {
-       if ((!tp->retransmit_skb_hint && tp->retrans_out >= tp->lost_out) ||
-           (tp->retransmit_skb_hint &&
-            before(TCP_SKB_CB(skb)->seq,
-                   TCP_SKB_CB(tp->retransmit_skb_hint)->seq)))
+       if (!tp->retransmit_skb_hint ||
+           before(TCP_SKB_CB(skb)->seq,
+                  TCP_SKB_CB(tp->retransmit_skb_hint)->seq))
                tp->retransmit_skb_hint = skb;
 }

--
2.30.2

And I got errors like

ERROR: tcp_input.o: 8 unsupported section change(s)
tcp_input.o: changed function: tcp_mark_head_lost
tcp_input.o: changed function: tcp_skb_mark_lost_uncond_verify
tcp_input.o: changed section .rodata._entry.81423 not selected for inclusion
tcp_input.o: changed section .rodata._entry.81428 not selected for inclusion
tcp_input.o: changed section .rodata._entry.81569 not selected for inclusion
tcp_input.o: changed section .rodata._entry.78831 not selected for inclusion
tcp_input.o: changed section .rodata._entry.78836 not selected for inclusion
tcp_input.o: changed section .rodata._entry.78840 not selected for inclusion
tcp_input.o: changed section .rodata._entry.79434 not selected for inclusion
tcp_input.o: changed section .rodata._entry.79343 not selected for inclusion
/data/users/songliubraving/kpatch/kpatch-build/create-diff-object: unreconcilable difference

The gcc I use is gcc (GCC) 8.5.0 20210514 (Red Hat 8.5.0-1).

What's the best solution for this type of issues? Thanks in advance!

Activity

  1. jpoimboe commented on Aug 5, 2021

    @jpoimboe
    jpoimboe
    on Aug 5, 2021
    Member

    I'm not sure what this error means. Maybe there could be a static local variable named "_entry" (possibly created from multiple calls to an inline function in a header file) which isn't getting properly detected somehow.

    It's also not clear to me why this patch would cause tcp_skb_mark_lost_uncond_verify() to be marked as changed, though that might be explained if your code version differs from the one I'm looking at.

    If you send the tcp_input.o files in ~/.kpatch/tmp/{orig,patched} directories could take a look.

  2. liu-song-6 commented on Aug 5, 2021

    @liu-song-6
    liu-song-6
    on Aug 5, 2021
    ContributorAuthor

    tcp_input.zip
    Here are the two files. Thanks!

  3. liu-song-6 commented on Aug 5, 2021

    @liu-song-6
    liu-song-6
    on Aug 5, 2021
    ContributorAuthor

    I just figured out it is from the prink_index work, which is in prink/for-next https://git.kernel.org/pub/scm/linux/kernel/git/printk/linux.git/tree/include/linux/printk.h?h=for-next#n377

    I guess we will need to work around it in kpatch-build for 5.15. For now, I will try to fix it in the patch itself. Thanks!

  4. jpoimboe commented on Aug 5, 2021

    @jpoimboe
    jpoimboe
    on Aug 5, 2021
    Member

    Ah, good find... I'll look into how to fix kpatch-build.

  5. liu-song-6 commented on Aug 5, 2021

    @liu-song-6
    liu-song-6
    on Aug 5, 2021
    ContributorAuthor

    I guess we will somehow ignore it in create-diff-object. Do we need to rename it to something more clear (i.e. __pi_entry instead of just _entry)?

  6. liu-song-6 commented on Aug 9, 2021

    @liu-song-6
    liu-song-6
    on Aug 9, 2021
    ContributorAuthor

    I found something like the following unblocks kpatch-build. I am able to use livepatch to add printk into the kernel for debug. But I am not sure whether this is fine for official use.

    diff --git i/kpatch-build/create-diff-object.c w/kpatch-build/create-diff-object.c
index 3b449fd..2fc9f1e 100644
--- i/kpatch-build/create-diff-object.c
+++ w/kpatch-build/create-diff-object.c
@@ -557,6 +557,10 @@ static void kpatch_compare_correlated_section(struct section *sec)

        /* Short circuit for mcount sections, we rebuild regardless */
        if (!strcmp(sec->name, ".rela__mcount_loc") ||
+           !strncmp(sec->name, ".rodata._entry", 14) ||
+           !strncmp(sec->name, ".rela.rodata._entry", 19) ||
+           !strncmp(sec->name, ".printk_index", 13) ||
+           !strncmp(sec->name, ".rela.printk_index", 18) ||
            !strcmp(sec->name, "__mcount_loc")) {
                sec->status = SAME;
                goto out;

  7. jpoimboe commented on Aug 10, 2021

    @jpoimboe
    jpoimboe
    on Aug 10, 2021
    Member

    The problem is that struct pi_entry _entry is a local static variable which uses the __LINE__ macro, which causes the line number to be hard-coded in the struct. So any changes above it in the source file will cause all the line numbers (and corresponding static local _entry structs) to change.

    I think the fix is to add _entry to the var_names array in is_special_static(). Can you verify that fixes it?

    We will also need to do a change similar to your suggested patch, for the case where a patch adds a new printk(). That will add a new entry to to .printk_index. So we should just ignore all changes to .printk_index and .rela.printk_index. I'll work on a pull request.

  8. jpoimboe
    added 4 commits that reference this issue on Aug 10, 2021

    create-diff-object: Support CONFIG_PRINTK_INDEX, part 1

    be43dcb

    kpatch-build: Support CONFIG_PRINTK_INDEX, part 2

    8bf5ec2

    create-diff-object: Support CONFIG_PRINTK_INDEX, part 1

    6cf50a6

    kpatch-build: Support CONFIG_PRINTK_INDEX, part 2

    56471ff
  9. sm00th
    closed this as completedin 6cf50a6on Aug 17, 2021
  10. keiya-nobuta
    added a commit that references this issue on Oct 6, 2021

    WIP

    32fe9aa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions