From: Shmulik Ladkani <shmulik.ladkani@gmail.com>
To: Lance Richardson <lrichard@redhat.com>
Cc: netdev@vger.kernel.org, fw@strlen.de, jtluka@redhat.com,
hannes@stressinduktion.org
Subject: Re: [PATCH net v3] ipv4: allow local fragmentation in ip_finish_output_gso()
Date: Thu, 3 Nov 2016 09:42:39 +0200 [thread overview]
Message-ID: <20161103094239.13ac8ac2@pixies> (raw)
In-Reply-To: <1478118977-19608-1-git-send-email-lrichard@redhat.com>
On Wed, 2 Nov 2016 16:36:17 -0400
Lance Richardson <lrichard@redhat.com> wrote:
> - /* common case: fragmentation of segments is not allowed,
> - * or seglen is <= mtu
> + /* common case: seglen is <= mtu
> */
> - if (((IPCB(skb)->flags & IPSKB_FRAG_SEGS) == 0) ||
> - skb_gso_validate_mtu(skb, mtu))
> + if (skb_gso_validate_mtu(skb, mtu))
> return ip_finish_output2(net, sk, skb);
OK.
Resembles https://patchwork.ozlabs.org/patch/644724/ ;)
> - if (skb_iif && !(df & htons(IP_DF))) {
> - /* Arrived from an ingress interface, got encapsulated, with
> - * fragmentation of encapulating frames allowed.
> - * If skb is gso, the resulting encapsulated network segments
> - * may exceed dst mtu.
> - * Allow IP Fragmentation of segments.
> - */
> - IPCB(skb)->flags |= IPSKB_FRAG_SEGS;
> - }
The only potential issue I see removing this, is that the KNOWLEDGE of
the forwarding/tunnel-bridging usecases (that require a
skb_gso_validate_mtu check) is lost.
Meaning, if one decides (as claimed in the past) that locally generated
traffic must NOT go through fragmentation validation, then no one
remembers those other valid usecases (which are very specific and not
trivial to imagine) that MUST go through it; Therefore it can easily get
broken.
Maybe we can elaborate in the comment above the call to
skb_gso_validate_mtu in ip_finish_output_gso?
Best,
Shmulik
next prev parent reply other threads:[~2016-11-03 7:42 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-02 20:36 [PATCH net v3] ipv4: allow local fragmentation in ip_finish_output_gso() Lance Richardson
2016-11-03 7:42 ` Shmulik Ladkani [this message]
2016-11-03 9:44 ` Hannes Frederic Sowa
2016-11-03 13:06 ` Lance Richardson
2016-11-04 9:24 ` Shmulik Ladkani
2016-11-04 13:48 ` Lance Richardson
2016-11-03 9:42 ` Hannes Frederic Sowa
2016-11-03 20:12 ` David Miller
2016-11-03 20:40 ` Shmulik Ladkani
2016-11-03 20:56 ` David Miller
2016-11-03 20:27 ` Shmulik Ladkani
2016-11-03 21:05 ` Lance Richardson
2016-11-03 21:34 ` Hannes Frederic Sowa
2016-11-04 9:40 ` Shmulik Ladkani
2016-11-04 13:49 ` Lance Richardson
2016-11-04 8:02 ` Shmulik Ladkani
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=20161103094239.13ac8ac2@pixies \
--to=shmulik.ladkani@gmail.com \
--cc=fw@strlen.de \
--cc=hannes@stressinduktion.org \
--cc=jtluka@redhat.com \
--cc=lrichard@redhat.com \
--cc=netdev@vger.kernel.org \
/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).