From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53656 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751194AbeCIIqn (ORCPT ); Fri, 9 Mar 2018 03:46:43 -0500 Date: Fri, 9 Mar 2018 09:46:30 +0100 From: Jesper Dangaard Brouer To: Jason Wang Cc: =?UTF-8?B?QmrDtnJuVMO2cGVs?= , magnus.karlsson@intel.com, netdev@vger.kernel.org, eugenia@mellanox.com, John Fastabend , Eran Ben Elisha , Saeed Mahameed , galp@mellanox.com, Daniel Borkmann , Alexei Starovoitov , Tariq Toukan , brouer@redhat.com Subject: Re: [bpf-next V2 PATCH 06/15] tun: convert to use generic xdp_frame and xdp_return_frame API Message-ID: <20180309094630.062a9362@redhat.com> In-Reply-To: <610afb59-8c11-b01b-f177-fb6236234da6@redhat.com> References: <152051439383.7018.11827926732878918934.stgit@firesoul> <152051449173.7018.11182092555803467523.stgit@firesoul> <20180308161625.4a397e3b@redhat.com> <610afb59-8c11-b01b-f177-fb6236234da6@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: netdev-owner@vger.kernel.org List-ID: On Fri, 9 Mar 2018 15:16:35 +0800 Jason Wang wrote: > On 2018年03月08日 23:16, Jesper Dangaard Brouer wrote: > > Hi Jason, > > > > Please see below FIXME, which is actually a question to you. > > > > On Thu, 08 Mar 2018 14:08:11 +0100 Jesper Dangaard Brouer wrote: > > > >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c > >> index 475088f947bb..cd046cf31b77 100644 > >> --- a/drivers/net/tun.c > >> +++ b/drivers/net/tun.c > > [...] > > > >> @@ -1290,17 +1290,18 @@ static const struct net_device_ops tun_netdev_ops = { > >> static int tun_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) > >> { > >> struct tun_struct *tun = netdev_priv(dev); > >> - struct xdp_buff *buff = xdp->data_hard_start; > >> - int headroom = xdp->data - xdp->data_hard_start; > >> + struct xdp_frame *frame; > >> struct tun_file *tfile; > >> u32 numqueues; > >> int ret = 0; > >> > >> - /* Assure headroom is available and buff is properly aligned */ > >> - if (unlikely(headroom < sizeof(*xdp) || tun_is_xdp_buff(xdp))) > >> - return -ENOSPC; > >> + /* FIXME: Explain why this check is the needed! */ > >> + if (unlikely(tun_is_xdp_frame(xdp))) > >> + return -EBADRQC; > >> > >> - *buff = *xdp; > >> + frame = convert_to_xdp_frame(xdp); > >> + if (unlikely(!frame)) > >> + return -EOVERFLOW; > > To Jason, in the FIXME, I'm inheriting a check you put in, but I don't > > understand why this check was needed? > > > > Sorry for the late reply. > > I think it was used to make sure to not use misaligned or invalid > pointer that caller passed to us. Okay, but I don't think this can happen, thus I'm going to remove this check in V3. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer