Development discussion of WireGuard
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Netdev <netdev@vger.kernel.org>,
	netfilter-devel@vger.kernel.org,
	Jaap Buurman <jaapbuurman@gmail.com>,
	openwrt-devel@lists.openwrt.org,
	WireGuard mailing list <wireguard@lists.zx2c4.com>,
	Felix Fietkau <nbd@nbd.name>
Subject: Missing skb->dst with flow offloading
Date: Wed, 30 May 2018 02:01:05 +0200	[thread overview]
Message-ID: <CAHmME9que7BqU9sJcmtSpBoHTs8=kybvDZAT_Ai151qZQOCscA@mail.gmail.com> (raw)

Hey Pablo,

Some OpenWRT people have reported to me that there's a crash when
enabling flow offloading, because I rely on skb_dst(skb) being
non-null in ndo_start_xmit. The fix in my code for this is very
simple:

- mtu = dst_mtu(skb_dst(skb));
+ dst = skb_dst(skb);
+ mtu = dst ? dst_mtu(dst) : dev->mtu;

I can make this change, but I wanted to be certain first that omitting
the dst in the skb is intentional on your part. (If so, there might be
other drivers to fix as well.) In tracing this, it looks like a packet
that's forwarded from a flow offloaded interface to a virtual
interface gets diverted immediately via neigh_xmit, where it is then
passed to a virtual interface via dev_queue_xmit. I can't see anywhere
along this path a call to skb_dst_set. Perhaps this is intended, as
flow offloading is supposed to skip the routing table? Or is there an
oversight in the new flow offloading code?

I'd appreciate your input, so that I can make the appropriate change
-- or not -- to my code.

Regards,
Jason

             reply	other threads:[~2018-05-29 23:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-30  0:01 Jason A. Donenfeld [this message]
2018-05-30 18:05 ` Pablo Neira Ayuso
2018-05-30 18:14   ` Jason A. Donenfeld
2018-05-30 18:24     ` Pablo Neira Ayuso
2018-05-30 18:30       ` Jason A. Donenfeld

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='CAHmME9que7BqU9sJcmtSpBoHTs8=kybvDZAT_Ai151qZQOCscA@mail.gmail.com' \
    --to=jason@zx2c4.com \
    --cc=jaapbuurman@gmail.com \
    --cc=nbd@nbd.name \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=openwrt-devel@lists.openwrt.org \
    --cc=pablo@netfilter.org \
    --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).