From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <netdev-owner@vger.kernel.org>
Received: from mx3-rdu2.redhat.com ([66.187.233.73]:49306 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
        id S1750970AbeCIHQq (ORCPT <rfc822;netdev@vger.kernel.org>);
        Fri, 9 Mar 2018 02:16:46 -0500
Subject: Re: [bpf-next V2 PATCH 06/15] tun: convert to use generic xdp_frame
 and xdp_return_frame API
To: Jesper Dangaard Brouer <brouer@redhat.com>,
        =?UTF-8?B?QmrDtnJuVMO2cGVs?= <bjorn.topel@intel.com>,
        magnus.karlsson@intel.com
Cc: netdev@vger.kernel.org, eugenia@mellanox.com,
        John Fastabend <john.fastabend@gmail.com>,
        Eran Ben Elisha <eranbe@mellanox.com>,
        Saeed Mahameed <saeedm@mellanox.com>, galp@mellanox.com,
        Daniel Borkmann <borkmann@iogearbox.net>,
        Alexei Starovoitov <alexei.starovoitov@gmail.com>,
        Tariq Toukan <tariqt@mellanox.com>
References: <152051439383.7018.11827926732878918934.stgit@firesoul>
 <152051449173.7018.11182092555803467523.stgit@firesoul>
 <20180308161625.4a397e3b@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Message-ID: <610afb59-8c11-b01b-f177-fb6236234da6@redhat.com>
Date: Fri, 9 Mar 2018 15:16:35 +0800
MIME-Version: 1.0
In-Reply-To: <20180308161625.4a397e3b@redhat.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>



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 <brouer@redhat.com> 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.

Thanks