From: Daniel Axtens <dja@axtens•net>
To: Eric Dumazet <eric.dumazet@gmail•com>
Cc: netdev@vger•kernel.org, tlfalcon@linux•vnet.ibm.com,
Yuval.Mintz@cavium•com, ariel.elior@cavium•com,
everest-linux-l2@cavium•com, jay.vosburgh@canonical•com
Subject: Re: [PATCH] bnx2x: drop packets where gso_size is too big for hardware
Date: Mon, 18 Sep 2017 14:41:51 +1000 [thread overview]
Message-ID: <87fubkehyo.fsf@linkitivity.dja.id.au> (raw)
In-Reply-To: <1504159341.15310.6.camel@edumazet-glaptop3.roam.corp.google.com>
Hi Eric,
>> + if (unlikely(skb_shinfo(skb)->gso_size + hlen > MAX_PACKET_SIZE)) {
>> + BNX2X_ERR("reported gso segment size plus headers "
>> + "(%d + %d) > MAX_PACKET_SIZE; dropping pkt!",
>> + skb_shinfo(skb)->gso_size, hlen);
>> +
>> + goto free_and_drop;
>> + }
>> +
>
>
> If you had this test in bnx2x_features_check(), packet could be
> segmented by core networking stack before reaching bnx2x_start_xmit() by
> clearing NETIF_F_GSO_MASK
>
> -> No drop would be involved.
>
> check i40evf_features_check() for similar logic.
So I've been experimenting with this and reading through the core
networking code. If my understanding is correct, disabling GSO will
cause the packet to be segmented, but it will be segemented into
gso_size+header length packets. So in this case (~10kB gso_size) the
resultant packets will still be too big - although at least they don't
cause a crash in that case.
We could continue with this anyway as it at least prevents the crash -
but, and I haven't been able to find a nice definitive answer to this -
are implementations of ndo_start_xmit permitted to assume that the the
skb passed in will fit within the MTU? I notice that most callers will
attempt to ensure this - for example ip_output.c, ip6_output.c and
ip_forward.c all contain calls to skb_gso_validate_mtu(). If
implementations are permitted to assume this, perhaps a fix to
openvswitch would be more appropriate?
Regards,
Daniel
next prev parent reply other threads:[~2017-09-18 4:41 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-31 5:46 [PATCH] bnx2x: drop packets where gso_size is too big for hardware Daniel Axtens
2017-08-31 6:02 ` Eric Dumazet
2017-09-01 2:42 ` Daniel Axtens
2017-09-18 4:41 ` Daniel Axtens [this message]
2017-09-18 14:42 ` Eric Dumazet
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=87fubkehyo.fsf@linkitivity.dja.id.au \
--to=dja@axtens$(echo .)net \
--cc=Yuval.Mintz@cavium$(echo .)com \
--cc=ariel.elior@cavium$(echo .)com \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=everest-linux-l2@cavium$(echo .)com \
--cc=jay.vosburgh@canonical$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=tlfalcon@linux$(echo .)vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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