On Fri, 5 Apr 2019 17:45:34 +0200 Matteo Croce <mcr...@redhat.com> wrote:
> On Fri, Apr 5, 2019 at 2:51 AM Matteo Croce <mcr...@redhat.com> wrote: > > > > On Fri, Apr 5, 2019 at 2:20 AM David Miller <da...@davemloft.net> wrote: > > > > > > From: Matteo Croce <mcr...@redhat.com> > > > Date: Wed, 3 Apr 2019 01:11:36 +0200 > > > > > > > The thunderx driver forbids to load an eBPF program if the MTU is > > > > higher than 1500 bytes, but this can be circumvented by first > > > > loading the eBPF, and then raising the MTU. > > > > > > > > XDP assumes that SKBs are linear and fit in a single page, this can > > > > lead to undefined behaviours. > > > > Fix this by limiting the MTU to 1500 bytes if an eBPF program is > > > > loaded. > > > > > > > > Fixes: 05c773f52b96e ("net: thunderx: Add basic XDP support") > > > > Signed-off-by: Matteo Croce <mcr...@redhat.com> > > > > > > Please respond to Jesper's feedback about your choice of a limit of > > > 1500. > > > > > > Otherwise I will toss your patch. > > > > Hi David ad Jesper, > > > > I didn't deliberately choose a limit of 1500, the limit is always set > > in nicvf_xdp_setup(): > > > > /* For now just support only the usual MTU sized frames */ > > if (prog && (dev->mtu > 1500)) { > > netdev_warn(dev, "Jumbo frames not yet supported with XDP... > > > > I just enforced the same limit in another code path which didn't do > > the check. > > If you think that 1500 is a bad value, and I'm sure you're right because > > there isn't room even for VLAN tagging, I will send a series like: > > - 1/2 sets the limit to a resonable value > > - 2/2 enforce the same limit in the two code paths > > > > Regards, > > -- > > Matteo Croce > > per aspera ad upstream > > Hi all, > > I did some tests and I've found that on this driver, the maximum > allowed frame size with XDP is 1530. > Frames bigger than 1530 are split around multiple pages, so the driver > doesn't even run the bpf on them: > > /* For XDP, ignore pkts spanning multiple pages */ > if (nic->xdp_prog && (cqe_rx->rb_cnt == 1)) { > > based on this test, I'll send a series with a proper MTU limit which > should be, correct me if I'm wrong: 1530 - 14 (eth) - 4 (QinQ) = 1512 > bytes. > I subtract only the 4 bytes for the QinQ as the > NETIF_F_VLAN_CHALLENGED_BIT flag is not set, and the first VLAN tag > should not be counted. You *do* need to include the first VLAN tag in the calculation. I guess I didn't explain this clear enough on IRC. XDP cannot use VLAN-offloading. As we explain here[1] when running XDP, you need to disable VLAN-offloading (see cmd in [1]), because XDP need the VLAN header to be "inline" in the packet. XDP don't (yet) have access to reading info from the descriptor. [1] https://github.com/xdp-project/xdp-tutorial/tree/master/packet01-parsing#a-note-about-vlan-offloads -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer