Hi Aya,
> Daniel Axtens, does this solve the issue referred in your commit?
> 8914a595110a bnx2x: disable GSO where gso_size is too big for hardware
No, because:
> Note: These cases are handled in the same manner in IPv4 output finish.
> This patch aligns the behavior of IPv6 and I
Willem de Bruijn writes:
> On Tue, Jan 22, 2019 at 11:24 AM Ben Hutchings
> wrote:
>>
>> Last year you applied these fixes for a potential denial-of-service in
>> the bnx2x driver:
>>
>> commit 2b16f048729bf35e6c28a40cbfad07239f9dcd90
>> Author: Dan
Hi Michael,
> I've received a bug report of oversized UDP packets sent to the
> bnxt_en driver for transmission. There is no check for illegal length
> in the driver and it will send a corrupted BD to the NIC if the
> non-TSO length exceeds the maximum MTU supported by the driver. This
> ultimat
Hi Michael,
>>> The main issue is the TX timeout.
>>> .
>>>
[ 2682.911693] bnxt_en :3b:00.0 eth4: TX timeout detected, starting
reset task!
[ 2683.782496] bnxt_en :3b:00.0 eth4: Resp cmpl intr err msg: 0x51
[ 2683.783061] bnxt_en :3b:00.0 eth4: hwrm_ring_free t
Hi Michael,
> The main issue is the TX timeout.
> .
>
>> [ 2682.911693] bnxt_en :3b:00.0 eth4: TX timeout detected, starting
>> reset task!
>> [ 2683.782496] bnxt_en :3b:00.0 eth4: Resp cmpl intr err msg: 0x51
>> [ 2683.783061] bnxt_en :3b:00.0 eth4: hwrm_ring_free tx failed. rc:-
Hi Michael,
I have some user reports of issues with a Broadcom 57412 card with the
card intermittently hanging and dropping the link.
The problem has been observed on a Dell server with an Ubuntu 4.13
kernel (bnxt_en version 1.7.0) and with an Ubuntu 4.15 kernel (bnxt_en
version 1.8.0). It seems
As well as the basic conversion, I noticed that a lot of the
SCTP code checks gso_type without first checking skb_is_gso()
so I have added that where appropriate.
Also, document the helper.
Cc: Daniel Borkmann
Cc: Marcelo Ricardo Leitner
Signed-off-by: Daniel Axtens
---
This depends on
Pretty minor: just SKB_GSO_TCP -> SKB_GSO_TCPV4 and
SKB_GSO_TCP6 -> SKB_GSO_TCPV6.
Signed-off-by: Daniel Axtens
---
Documentation/networking/segmentation-offloads.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/netw
Hi Daniel,
> From: Daniel Axtens
>
> SCTP GSO skbs have a gso_size of GSO_BY_FRAGS, so any sort of
> unconditionally mangling of that will result in nonsense value
> and would corrupt the skb later on.
>
> Therefore, i) add two helpers skb_increase_gso_size() and
> skb_
They're very hard to use properly as they do not consider the
GSO_BY_FRAGS case. Code should use skb_gso_validate_network_len
and skb_gso_validate_mac_len as they do consider this case.
Make the seglen functions static, which stops people using them
outside of skbuff.c
Signed-off-by: D
of a split GSO
skb (L2+L3+L4+payload), and the names get confusing, so rename
skb_gso_validate_mtu to skb_gso_validate_network_len
Signed-off-by: Daniel Axtens
---
include/linux/skbuff.h | 2 +-
net/core/skbuff.c | 11 ++-
net/ipv4/ip_forward.c
: Daniel Axtens
---
net/sched/sch_tbf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 229172d509cc..03225a8df973 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -188,7 +188,8 @@ static int tbf_enqueue(struct sk_buff
Replace skb_gso_network_seglen() with
skb_gso_validate_network_len(), as it considers the GSO_BY_FRAGS
case.
Signed-off-by: Daniel Axtens
---
net/ipv4/xfrm4_output.c | 3 ++-
net/ipv6/xfrm6_output.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/xfrm4_output.c b
ed is
sunvnet, but I have no way of testing it, so I haven't looked at it.
v2: split out bpf stuff
fix review comments from Dave Miller
Daniel Axtens (4):
net: rename skb_gso_validate_mtu -> skb_gso_validate_network_len
net: sched: tbf: handle GSO_BY_FRAGS case in enqueue
net: xfrm:
Hi Daniel,
>> It means that a user loaded eBPF program can trigger logs full of
>> warnings merely by using this eBPF helper and generating GSO'd SCTP
>> traffic.
>>
>> Daniel and Alexei, this is a serious problem. The eBPF helpers
>> mentioned here cannot handle SCTP GSO packets properly, and i
Replace skb_gso_network_seglen() with
skb_gso_validate_network_len(), as it considers the GSO_BY_FRAGS
case.
Signed-off-by: Daniel Axtens
---
net/ipv4/xfrm4_output.c | 3 ++-
net/ipv6/xfrm6_output.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/xfrm4_output.c b
x27;s not clear how this could be done correctly, so simply refuse
to operate on SCTP GSO skbs.
Cc: Marcelo Ricardo Leitner
Signed-off-by: Daniel Axtens
---
Marcelo - I am not an SCTP expert by any means, so if this code should
be doing something else, please let me know.
---
net/core/filter.c
to some nonsense value.
Add helpers that WARN if attempting to change a gso_size of a
GSO_BY_FRAGS skb (and leave the value unchanged).
Signed-off-by: Daniel Axtens
---
Documentation/networking/segmentation-offloads.txt | 11 +--
include/linux/skbuff.h | 16
tions don't have
to make any out of line calls.
Signed-off-by: Daniel Axtens
---
include/linux/skbuff.h | 33 -
net/core/skbuff.c | 37 +++--
2 files changed, 35 insertions(+), 35 deletions(-)
diff --git a/include/linux/
: Daniel Axtens
---
net/sched/sch_tbf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 229172d509cc..03225a8df973 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -188,7 +188,8 @@ static int tbf_enqueue(struct sk_buff
t support
SCTP segmentation, so the core network will segment an SCTP gso before
it hits them.) Based on that, the only driver that may be affected is
sunvnet, but I have no way of testing it, so I haven't looked at it.
Daniel Axtens (6):
net: rename skb_gso_validate_mtu -> skb_gso_val
of a split GSO
skb (L2+L3+L4+payload), and the names get confusing, so rename
skb_gso_validate_mtu to skb_gso_validate_network_len
Signed-off-by: Daniel Axtens
---
include/linux/skbuff.h | 2 +-
net/core/skbuff.c | 11 ++-
net/ipv4/ip_forward.c
Dmitry Vyukov writes:
> On Thu, Feb 22, 2018 at 3:35 PM, David Miller wrote:
>> From: Dmitry Vyukov
>> Date: Thu, 22 Feb 2018 10:58:07 +0100
>>
>>> Do I understand it correctly that if syzbot replies to the CC list
>>> that was in the testing request, it will resolve the problem? So if
>>> netd
Dmitry Vyukov writes:
> On Thu, Feb 22, 2018 at 2:31 PM, Daniel Axtens wrote:
>> Dmitry Vyukov writes:
>>
>>> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni wrote:
>>>> On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
>>>>> I hav
Dmitry Vyukov writes:
> On Thu, Feb 22, 2018 at 9:26 AM, Paolo Abeni wrote:
>> On Wed, 2018-02-21 at 16:47 -0500, David Miller wrote:
>>> I have to mention this now before it gets out of control.
>>>
>>> I would like to ask that syzkaller stop posting the patch it is
>>> testing when it posts to
Hi Marcelo,
>
> If this block was meant to be an out-of-band/changelog comment, your
> SOB line should be above the first --- marker.
> Anyhow,
> Reviewed-by: Marcelo Ricardo Leitner
Thanks - I did a v2 with this around the right way [1], but DaveM asked
me to be a bit more thorough and look for
erstand it well and I
think it's in the process of changing.
Daniel Axtens (3):
docs: segmentation-offloads.txt: update for UFO depreciation
docs: segmentation-offloads.txt: Fix ref to SKB_GSO_TUNNEL_REMCSUM
docs: segmentation-offloads.txt: add SCTP info
Documentation/networking/segme
Most of this is extracted from 90017accff61 ("sctp: Add GSO support"),
with some extra text about GSO_BY_FRAGS and the need to check for it.
Cc: Marcelo Ricardo Leitner
Signed-off-by: Daniel Axtens
---
Documentation/networking/segmentation-offloads.txt | 26
UFO is deprecated except for tuntap and packet per 0c19f846d582,
("net: accept UFO datagrams from tuntap and packet"). Update UFO
docs to reflect this.
Signed-off-by: Daniel Axtens
---
Documentation/networking/segmentation-offloads.txt | 4
1 file changed, 4 insertions(+)
di
The doc originally called it SKB_GSO_REMCSUM. Fix it.
Fixes: f7a6272bf3cb ("Documentation: Add documentation for TSO and GSO
features")
Signed-off-by: Daniel Axtens
--
The only text change is s/SKB_GSO_REMCSUM/SKB_GSO_TUNNEL_REMCSUM/, but
I rewrapped the text which is why the whole pa
Hi Marcelo and Eric,
I'm working on checking code that might be impacted by GSO_BY_FRAGS -
after finding that the token bucket filter qdisc code doesn't handle it
properly, DaveM said I should look for other places where this might be
an issue [0].
I'm currently looking at qdisc_pkt_len_init in n
: Daniel Axtens
---
skb_gso_validate_mac_len() is an out-of-line call, but so is
skb_gso_mac_seglen(), so this is slower but not much slower. I
will send a patch to make the skb_gso_validate_* functions
inline-able shortly.
Also, GSO_BY_FRAGS considered harmful - I'm pretty sure this is
not the
igned-off-by: Daniel Axtens
---
net/sched/sch_tbf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 229172d509cc..03225a8df973 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -188,7 +188,8 @@ static int tbf_enqueue(s
"Chopra, Manish" writes:
>> -Original Message-
>> From: Daniel Axtens [mailto:d...@axtens.net]
>> Sent: Wednesday, January 31, 2018 8:46 AM
>> To: netdev@vger.kernel.org
>> Cc: Daniel Axtens ; Eric Dumazet ;
>> Chopra, Manish ; Jason Wang
&
(9700 bytes) and disable GSO.
Signed-off-by: Daniel Axtens
---
v4: Only call the slow check if the gso_size is large.
Eric - I think this is what you had in mind?
Manish - do you think this is an acceptable performance trade-off?
GSO will work for any packet size, and only jumbo
://patchwork.ozlabs.org/patch/859410/
Cc: Eric Dumazet
Cc: manish.cho...@cavium.com
Cc: Jason Wang
Cc: Pravin Shelar
Cc: Marcelo Ricardo Leitner
Daniel Axtens (2):
net: create skb_gso_validate_mac_len()
bnx2x: disable GSO where gso_size is too big for hardware
drivers/net/ethernet/broadcom
skb_gso_validate_mac_len to do the full calculation.
Signed-off-by: Daniel Axtens
---
v4: fix mistake in EXPORT name
---
include/linux/skbuff.h | 16 +
net/core/skbuff.c | 63 +++---
net/sched/sch_tbf.c| 10
3 files
Marcelo Ricardo Leitner writes:
> Hi,
>
> On Tue, Jan 30, 2018 at 12:14:46PM +1100, Daniel Axtens wrote:
>> If you take a GSO skb, and split it into packets, will the MAC
>> length (L2 + L3 + L4 headers + payload) of those packets be small
>> enough to fit within
Hi Eric,
>> skb_gso_transport_seglen(skb) is quite expensive (out of line)
>>
>> It is unfortunate bnx2x seems to support 9600 MTU (
>> ETH_MAX_JUMBO_PACKET_SIZE ), because 100 bytes of headers can be too
>> small in some cases.
>>
>> Presumably we could avoid calling the function for standard M
skb_gso_validate_mac_len to do the full calculation.
Signed-off-by: Daniel Axtens
---
include/linux/skbuff.h | 16 +
net/core/skbuff.c | 65 +++---
net/sched/sch_tbf.c| 10
3 files changed, 67 insertions(+), 24 deletions
...@cavium.com
Cc: Jason Wang
Cc: Pravin Shelar
Cc: Marcelo Ricardo Leitner
Daniel Axtens (2):
net: create skb_gso_validate_mac_len()
bnx2x: disable GSO where gso_size is too big for hardware
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 9
include/linux/skbuff.h
(9700 bytes) and disable GSO.
Signed-off-by: Daniel Axtens
---
v3: use skb_gso_validate_mac_len to get the actual size, to allow
jumbo frames to work.
I don't have local access to a bnx2x card and sending this off to the
user who does would add about a month of round-trip time, so
Eric Dumazet writes:
> On Fri, 2018-01-26 at 00:44 +1100, Daniel Axtens wrote:
>> Hi Eric,
>>
>> > May I ask which tree are you targeting ?
>> >
>> > ( Documentation/networking/netdev-FAQ.txt )
>>
>> I have been targeting net-next, but
Hi Eric,
> May I ask which tree are you targeting ?
>
> ( Documentation/networking/netdev-FAQ.txt )
I have been targeting net-next, but I haven't pulled for about two
weeks. I will rebase and if there are conflicts I will resend early next
week.
> Anything touching GSO is very risky and should t
: Daniel Axtens
---
net/core/dev.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 6c96c26aadbf..f09eece2cd21 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1830,13 +1830,11 @@ static inline void net_timestamp_set
of a split GSO
skb (L2+L3+L4+payload), and the names get confusing, so rename
skb_gso_validate_mtu to skb_gso_validate_network_len
Signed-off-by: Daniel Axtens
---
include/linux/skbuff.h | 2 +-
net/core/skbuff.c | 9 +
net/ipv4/ip_forw
We're about to use this elsewhere, so move it into the header with
the other related functions like skb_gso_network_seglen().
Signed-off-by: Daniel Axtens
---
include/linux/skbuff.h | 15 +++
net/sched/sch_tbf.c| 10 --
2 files changed, 15 insertions(+), 10 dele
check.
Signed-off-by: Daniel Axtens
---
include/linux/skbuff.h | 1 +
net/core/dev.c | 7 +++---
net/core/skbuff.c | 67 +++---
3 files changed, 57 insertions(+), 18 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
validate_xmit_skb for now and we can come back and add it later if
OVS people would like the extra logging.
[0]: https://patchwork.ozlabs.org/patch/859410/
Cc: Jason Wang
Cc: Pravin Shelar
Cc: Marcelo Ricardo Leitner
Cc: manish.cho...@cavium.com
Cc: d...@openvswitch.org
Daniel Axtens
Pravin Shelar writes:
> On Thu, Jan 18, 2018 at 5:28 PM, Daniel Axtens wrote:
>> Pravin Shelar writes:
>>
>>> On Thu, Jan 18, 2018 at 5:08 AM, Daniel Axtens wrote:
>>>> Pravin Shelar writes:
>>>>
>>>>> On Mon, Jan 15, 2018 at
Daniel Axtens writes:
> Pravin Shelar writes:
>
>> On Thu, Jan 18, 2018 at 5:08 AM, Daniel Axtens wrote:
>>> Pravin Shelar writes:
>>>
>>>> On Mon, Jan 15, 2018 at 6:09 PM, Daniel Axtens wrote:
>>>>> When regular packets are forwa
Pravin Shelar writes:
> On Thu, Jan 18, 2018 at 5:08 AM, Daniel Axtens wrote:
>> Pravin Shelar writes:
>>
>>> On Mon, Jan 15, 2018 at 6:09 PM, Daniel Axtens wrote:
>>>> When regular packets are forwarded, we validate their size against the
>>>>
Daniel Axtens writes:
> Jason Wang writes:
>
>> On 2018年01月18日 16:28, Pravin Shelar wrote:
>>> On Mon, Jan 15, 2018 at 6:09 PM, Daniel Axtens wrote:
>>>> When regular packets are forwarded, we validate their size against the
>>>> MTU of the dest
Jason Wang writes:
> On 2018年01月18日 16:28, Pravin Shelar wrote:
>> On Mon, Jan 15, 2018 at 6:09 PM, Daniel Axtens wrote:
>>> When regular packets are forwarded, we validate their size against the
>>> MTU of the destination device. However, when GSO packets are
>&g
Pravin Shelar writes:
> On Mon, Jan 15, 2018 at 6:09 PM, Daniel Axtens wrote:
>> When regular packets are forwarded, we validate their size against the
>> MTU of the destination device. However, when GSO packets are
>> forwarded, we do not validate their size against the
t.
Signed-off-by: Daniel Axtens
---
net/openvswitch/vport.c | 37 ++---
1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index b6c8524032a0..290eeaa82344 100644
--- a/net/openvswitch/vport.c
specialised algorithm for
checking lengths. Wire up checking for that in patch 3.
[0]: https://patchwork.ozlabs.org/patch/859410/
Cc: manish.cho...@cavium.com
Cc: d...@openvswitch.org
Daniel Axtens (3):
net: move skb_gso_mac_seglen to skbuff.h
net: is_skb_forwardable: validate length of GSO packet
We're about to use this elsewhere, so move it into the header with
the other related functions like skb_gso_network_seglen().
Signed-off-by: Daniel Axtens
---
include/linux/skbuff.h | 15 +++
net/sched/sch_tbf.c| 10 --
2 files changed, 15 insertions(+), 10 dele
large by creating a
skb_gso_validate_len() routine which is similar to
skb_gso_validate_mtu(), but which considers L2 headers, and wire
it up in is_skb_forwardable().
Signed-off-by: Daniel Axtens
---
include/linux/skbuff.h | 1 +
net/core/dev.c | 7 ---
net/core/skbuff.c | 34
David Miller writes:
> From: Daniel Axtens
> Date: Fri, 12 Jan 2018 10:59:05 +1100
>
>> If a bnx2x card is passed a GSO packet with a gso_size larger than
>> ~9700 bytes, it will cause a firmware error that will bring the card
>> down:
>>
>> bnx2x: [bnx2
use a firmware panic on another card.
Cc: Thomas Falcon # ibmveth
Cc: Yuval Mintz # bnx2x
Thanks-to: Jay Vosburgh # veth info
Signed-off-by: Daniel Axtens
---
v2: change to a feature check as suggested by Eric Dumazet.
---
drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 11 ++
Hi Dave,
> So how exactly do the oversized packets get to the macvlan device from
> the VM in this scenerio? That detail seems to be missing from the
> diagrams you provided earlier. The VM and the macvlan boxes are just
> connected with a line.
Inside the VM I'm using netperf talking on an int
Hi Dave,
> This is an area where we really haven't set down some clear rules
> for behavior.
>
> If an interface has a particular MTU, it must be able to successfully
> send MTU sized packets on that link be it virtual or physical.
>
> Only a "next hop" can have a different MTU and thus drop packe
Hi Shannon,
> Now that I think about this a little more, why is this not already
> getting handled by the NETDEV_CHANGEMTU notifier? In what case are you
> running into this, and why is it not triggering the notifier?
My commit message was probably not super clear here - apologies for any
mang
Hi Bryant,
This looks a bit better, but...
> The following patch ensures that the bounce_buffer is not null
> prior to using it within skb_copy_from_linear_data.
How would this occur?
Looking at ibmveth.c, I see bounce_buffer being freed in ibmveth_close()
and allocated in ibmveth_open() only.
n the guest (e.g. keep at 1500)
- netperf to a different host with the same high MTU
- observe that currently, the driver will forward too-big packets
- observe that with this patch the packets are dropped
Cc: Shannon Nelson
Signed-off-by: Daniel Axtens
---
After hearing Shannon's lightni
Hi Bryant,
A few things:
1) The commit message could probably be trimmed, especially the stack
traces.
2) What exactly are you changing and why does it fix the issue? I
couldn't figure that out from the commit message.
3) Does this need a Fixes: tag?
> }
>
> - netdev->min_mtu = IB
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(
Eric Dumazet writes:
> 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.
Thanks for the pointer - networking code is all a bit new to me.
I'm ju
but it
requires configuration on each LPAR. While ibmveth's behaviour is
admittedly weird, we should fix this here: it shouldn't be possible
for it to cause a firmware panic on another card.
Cc: Thomas Falcon # ibmveth
Cc: Yuval Mintz # bnx2x
Thanks-to: Jay Vosburgh # veth info
Signed-o
I was trying to wrap my head around meaning of mru, and realised
that the second line of the comment defining it had somehow
ended up after the line defining cutlen, leading to much confusion.
Reorder the lines to make sense.
Signed-off-by: Daniel Axtens
---
net/openvswitch/datapath.h | 2
ing you'd
be able to shed some light on it?
Thanks in advance!
Regards,
Daniel Axtens
== dmesg output ==
[60072.330838] bnx2x: [bnx2x_attn_int_deasserted3:4323(enP24p1s0f0)]MC assert!
[60072.330854] bnx2x:
[bnx2x_mc_assert:720(enP24p1s0f0)]XSTORM_ASSERT_LIST_INDEX 0x2
[60072.
72 matches
Mail list logo