On Thu, Jan 18, 2018 at 5:28 PM, Daniel Axtens <d...@axtens.net> wrote: > 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. >
Would this oversized segment cause the firmware assert? If this solves the assert issue then I do not see much value in adding checks in fast-path just for logging. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev