Pravin Shelar <pshe...@ovn.org> writes: > On Thu, Jan 18, 2018 at 5:08 AM, Daniel Axtens <d...@axtens.net> wrote: >> Pravin Shelar <pshe...@ovn.org> writes: >> >>> On Mon, Jan 15, 2018 at 6:09 PM, Daniel Axtens <d...@axtens.net> 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 MTU. We >>>> implicitly assume that when they are segmented, the resultant packets >>>> will be correctly sized. >>>> >>>> This is not always the case. >>>> >>>> We observed a case where a packet received on an ibmveth device had a >>>> GSO size of around 10kB. This was forwarded by Open vSwitch to a bnx2x >>>> device, where it caused a firmware assert. This is described in detail >>>> at [0] and was the genesis of this series. Rather than fixing it in >>>> the driver, this series fixes the forwarding path. >>>> >>> Are there any other possible forwarding path in networking stack? TC >>> is one subsystem that could forward such a packet to the bnx2x device, >>> how is that handled ? >> >> So far I have only looked at bridges, openvswitch and macvlan. In >> general, if the code uses dev_forward_skb() it should automatically be >> fine as that invokes is_skb_forwardable(), which we patch. >> > But there are other ways to forward packets, e.g tc-mirred or bpf > redirect. We need to handle all these cases rather than fixing one at > a time. As Jason suggested netif_needs_gso() looks like good function > to validate if a device is capable of handling given GSO packet.
I am not entirely sure this is a better solution. The biggest reason I am uncomfortable with this is that if netif_needs_gso() returns true, the skb will be segmented. The segment sizes will be based on gso_size. Since gso_size is greater than the MTU, the resulting segments will themselves be over-MTU. Those over-MTU segments will then be passed to the network card. I think we should not be creating over-MTU segments; we should instead be dropping the packet and logging. I do take the point that you and Jason are making: a more generic fix would be good. I'm just not sure where to put it. Regards, Daniel
