From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Rui Salvaterra <rsalvaterra@gmail.com>
Cc: OpenWrt Development List <openwrt-devel@lists.openwrt.org>,
wireguard@lists.zx2c4.com
Subject: Re: wireguard: unknown relocation: 102 [ARMv7 Thumb-2]
Date: Wed, 17 Jun 2020 14:54:43 -0600 [thread overview]
Message-ID: <20200617205443.GA403252@zx2c4.com> (raw)
In-Reply-To: <20200617204510.GA396261@zx2c4.com>
On Wed, Jun 17, 2020 at 02:45:12PM -0600, Jason A. Donenfeld wrote:
> Looks like my explanation there wasn't 100% accurate, but it does seem
> like the issue occurs when gcc sees a clear tail call that it can
> optimize into a B instruction instead of a BL instruction.
>
> The below patch avoids that, and thus fixes your issue, using a pretty
> bad trick that's not really suitable for being committed anywhere, but
> it is perhaps leading us in the right direction:
>
> diff --git a/src/send.c b/src/send.c
> index 828b086a..4bb6911f 100644
> --- a/src/send.c
> +++ b/src/send.c
> @@ -221,6 +221,8 @@ static bool encrypt_packet(struct sk_buff *skb, struct noise_keypair *keypair,
> simd_context);
> }
>
> +volatile char dummy;
> +
> void wg_packet_send_keepalive(struct wg_peer *peer)
> {
> struct sk_buff *skb;
> @@ -240,6 +242,7 @@ void wg_packet_send_keepalive(struct wg_peer *peer)
> }
>
> wg_packet_send_staged_packets(peer);
> + dummy = -1;
> }
>
> static void wg_packet_create_data_done(struct sk_buff *first,
A better fix with more explanation: it looks like the issue doesn't have
to do with the multifile thing I pointed out before, but just that gcc
sees it can optimize the tail call into a B instruction, which seems to
have a ±2KB range, whereas BL has a ±4MB range. The solution is to just
move the location of the function in that file to be closer to the
destination of the tail call. I'm not a big fan of that and I'm slightly
worried davem will nack it because it makes backporting harder for a
fairly speculative gain (at least, I haven't yet taken measurements,
though I suppose I could). There's also the question of - why are we
doing goofy reordering things to the code to work around a toolchain
bug? Shouldn't we fix the toolchain? So, I'll keep thinking...
diff --git a/src/send.c b/src/send.c
index 828b086a..f44aff8d 100644
--- a/src/send.c
+++ b/src/send.c
@@ -221,27 +221,6 @@ static bool encrypt_packet(struct sk_buff *skb, struct noise_keypair *keypair,
simd_context);
}
-void wg_packet_send_keepalive(struct wg_peer *peer)
-{
- struct sk_buff *skb;
-
- if (skb_queue_empty(&peer->staged_packet_queue)) {
- skb = alloc_skb(DATA_PACKET_HEAD_ROOM + MESSAGE_MINIMUM_LENGTH,
- GFP_ATOMIC);
- if (unlikely(!skb))
- return;
- skb_reserve(skb, DATA_PACKET_HEAD_ROOM);
- skb->dev = peer->device->dev;
- PACKET_CB(skb)->mtu = skb->dev->mtu;
- skb_queue_tail(&peer->staged_packet_queue, skb);
- net_dbg_ratelimited("%s: Sending keepalive packet to peer %llu (%pISpfsc)\n",
- peer->device->dev->name, peer->internal_id,
- &peer->endpoint.addr);
- }
-
- wg_packet_send_staged_packets(peer);
-}
-
static void wg_packet_create_data_done(struct sk_buff *first,
struct wg_peer *peer)
{
@@ -346,6 +325,27 @@ err:
kfree_skb_list(first);
}
+void wg_packet_send_keepalive(struct wg_peer *peer)
+{
+ struct sk_buff *skb;
+
+ if (skb_queue_empty(&peer->staged_packet_queue)) {
+ skb = alloc_skb(DATA_PACKET_HEAD_ROOM + MESSAGE_MINIMUM_LENGTH,
+ GFP_ATOMIC);
+ if (unlikely(!skb))
+ return;
+ skb_reserve(skb, DATA_PACKET_HEAD_ROOM);
+ skb->dev = peer->device->dev;
+ PACKET_CB(skb)->mtu = skb->dev->mtu;
+ skb_queue_tail(&peer->staged_packet_queue, skb);
+ net_dbg_ratelimited("%s: Sending keepalive packet to peer %llu (%pISpfsc)\n",
+ peer->device->dev->name, peer->internal_id,
+ &peer->endpoint.addr);
+ }
+
+ wg_packet_send_staged_packets(peer);
+}
+
void wg_packet_purge_staged_packets(struct wg_peer *peer)
{
spin_lock_bh(&peer->staged_packet_queue.lock);
next prev parent reply other threads:[~2020-06-17 23:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CALjTZvbpu1Lw0j9dtXZPmVS+i-OnopUo+zuqtoQLnABQGw-SqQ@mail.gmail.com>
[not found] ` <CAHmME9r3nPwmUoYYrj0PnUStd1ACSmdFAO4Qv2cZtmiLspOW1g@mail.gmail.com>
[not found] ` <CALjTZvbtjVwpyV+AMX4htssTbwTHV45mQeokUr952D_GbtFPvw@mail.gmail.com>
[not found] ` <CALjTZvZRerzWqaqhY2U=m44n5taLEsY99uEt2=ZNCe27=LYbLA@mail.gmail.com>
[not found] ` <CAHmME9otC1mOqR2tLB55BVQQpNPvCMUGa1E4jfMYYXNp6_31BA@mail.gmail.com>
[not found] ` <CALjTZvZ4wqZZ7_Fk-YHaxT9uuWnS4n9dLm4ZXSy1UM3riv+NuQ@mail.gmail.com>
[not found] ` <CAHmME9qWrBTCsBr7s6oLD0zuBMzZUD2OV3s-tgDwV0W7bb9Utw@mail.gmail.com>
[not found] ` <CAHmME9p51XvLEZ7QbDreEXym34S4XZZaRotAv4aRiT5D4Pz3XA@mail.gmail.com>
2020-06-17 20:45 ` Jason A. Donenfeld
2020-06-17 20:54 ` Jason A. Donenfeld [this message]
2020-06-17 21:02 ` Any progress on R_ARM_THM_JUMP11 issues? Jason A. Donenfeld
2020-06-17 22:18 ` Rui Salvaterra
2020-06-18 23:58 ` Jason A. Donenfeld
2020-06-18 23:50 ` wireguard: unknown relocation: 102 [ARMv7 Thumb-2] Jason A. Donenfeld
2020-06-19 8:26 ` Rui Salvaterra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200617205443.GA403252@zx2c4.com \
--to=jason@zx2c4.com \
--cc=openwrt-devel@lists.openwrt.org \
--cc=rsalvaterra@gmail.com \
--cc=wireguard@lists.zx2c4.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).