netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: netdev@vger.kernel.org, Jamal Hadi Salim <jhs@mojatatu.com>,
	"David S. Miller" <davem@davemloft.net>,
	netem@lists.linux-foundation.org, eric.dumazet@gmail.com
Subject: Re: [PATCHv3] netem: Segment GSO packets on enqueue
Date: Fri, 29 Apr 2016 11:19:05 -0700	[thread overview]
Message-ID: <20160429111905.09b4b2e2@xeon-e3> (raw)
In-Reply-To: <1461951348-3920-1-git-send-email-nhorman@tuxdriver.com>

On Fri, 29 Apr 2016 13:35:48 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:

> This was recently reported to me, and reproduced on the latest net kernel, when
> attempting to run netperf from a host that had a netem qdisc attached to the
> egress interface:
> 
> [  788.073771] ------------[ cut here ]------------
> [  788.096716] WARNING: at net/core/dev.c:2253 skb_warn_bad_offload+0xcd/0xda()
> [  788.129521] bnx2: caps=(0x00000001801949b3, 0x0000000000000000) len=2962
> data_len=0 gso_size=1448 gso_type=1 ip_summed=3
> [  788.182150] Modules linked in: sch_netem kvm_amd kvm crc32_pclmul ipmi_ssif
> ghash_clmulni_intel sp5100_tco amd64_edac_mod aesni_intel lrw gf128mul
> glue_helper ablk_helper edac_mce_amd cryptd pcspkr sg edac_core hpilo ipmi_si
> i2c_piix4 k10temp fam15h_power hpwdt ipmi_msghandler shpchp acpi_power_meter
> pcc_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c
> sd_mod crc_t10dif crct10dif_generic mgag200 syscopyarea sysfillrect sysimgblt
> i2c_algo_bit drm_kms_helper ahci ata_generic pata_acpi ttm libahci
> crct10dif_pclmul pata_atiixp tg3 libata crct10dif_common drm crc32c_intel ptp
> serio_raw bnx2 r8169 hpsa pps_core i2c_core mii dm_mirror dm_region_hash dm_log
> dm_mod
> [  788.465294] CPU: 16 PID: 0 Comm: swapper/16 Tainted: G        W
> ------------   3.10.0-327.el7.x86_64 #1
> [  788.511521] Hardware name: HP ProLiant DL385p Gen8, BIOS A28 12/17/2012
> [  788.542260]  ffff880437c036b8 f7afc56532a53db9 ffff880437c03670
> ffffffff816351f1
> [  788.576332]  ffff880437c036a8 ffffffff8107b200 ffff880633e74200
> ffff880231674000
> [  788.611943]  0000000000000001 0000000000000003 0000000000000000
> ffff880437c03710
> [  788.647241] Call Trace:
> [  788.658817]  <IRQ>  [<ffffffff816351f1>] dump_stack+0x19/0x1b
> [  788.686193]  [<ffffffff8107b200>] warn_slowpath_common+0x70/0xb0
> [  788.713803]  [<ffffffff8107b29c>] warn_slowpath_fmt+0x5c/0x80
> [  788.741314]  [<ffffffff812f92f3>] ? ___ratelimit+0x93/0x100
> [  788.767018]  [<ffffffff81637f49>] skb_warn_bad_offload+0xcd/0xda
> [  788.796117]  [<ffffffff8152950c>] skb_checksum_help+0x17c/0x190
> [  788.823392]  [<ffffffffa01463a1>] netem_enqueue+0x741/0x7c0 [sch_netem]
> [  788.854487]  [<ffffffff8152cb58>] dev_queue_xmit+0x2a8/0x570
> [  788.880870]  [<ffffffff8156ae1d>] ip_finish_output+0x53d/0x7d0
> ...
> 
> The problem occurs because netem is not prepared to handle GSO packets (as it
> uses skb_checksum_help in its enqueue path, which cannot manipulate these
> frames).
> 
> The solution I think is to simply segment the skb in a simmilar fashion to the
> way we do in __dev_queue_xmit (via validate_xmit_skb), with some minor changes.
> When we decide to corrupt an skb, if the frame is GSO, we segment it, corrupt
> the first segment, and enqueue the remaining ones.
> 
> tested successfully by myself on the latest net kernel, to whcih this applies
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Jamal Hadi Salim <jhs@mojatatu.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: netem@lists.linux-foundation.org
> CC: eric.dumazet@gmail.com
> 

This looks like a good idea.

Please cleanup the formatting issues:

This was recently reported to me, and reproduced on the latest net kernel, when

WARNING: 'whcih' may be misspelled - perhaps 'which'?
#93: 
tested successfully by myself on the latest net kernel, to whcih this applies

ERROR: "foo* bar" should be "foo *bar"
#130: FILE: net/sched/sch_netem.c:402:
+static struct sk_buff* netem_segment(struct sk_buff *skb, struct Qdisc *sch)


CHECK: braces {} should be used on all arms of this statement
#164: FILE: net/sched/sch_netem.c:479:
+		if (skb_is_gso(skb)) {
[...]
+		} else
[...]

CHECK: braces {} should be used on all arms of this statement
#198: FILE: net/sched/sch_netem.c:562:
+			if (rc != NET_XMIT_SUCCESS) {
[...]
+			} else
[...]

  parent reply	other threads:[~2016-04-29 18:18 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-26 17:43 [PATCH] netem: Segment GSO packets on enqueue Neil Horman
2016-04-26 18:49 ` Eric Dumazet
2016-04-26 19:00   ` Neil Horman
2016-04-26 20:19     ` Eric Dumazet
2016-04-27 11:27       ` Neil Horman
2016-04-28 20:09 ` [PATCHv2] " Neil Horman
2016-04-28 20:58   ` Eric Dumazet
2016-04-29  1:20     ` Neil Horman
2016-04-29 17:35 ` [PATCHv3] " Neil Horman
2016-04-29 18:09   ` Eric Dumazet
2016-04-29 18:19   ` Stephen Hemminger [this message]
2016-04-30 13:30     ` Neil Horman
2016-05-02 16:20 ` [PATCHv4] " Neil Horman
2016-05-02 16:56   ` Eric Dumazet
2016-05-03  4:34   ` David Miller

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=20160429111905.09b4b2e2@xeon-e3 \
    --to=stephen@networkplumber.org \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=jhs@mojatatu.com \
    --cc=netdev@vger.kernel.org \
    --cc=netem@lists.linux-foundation.org \
    --cc=nhorman@tuxdriver.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).