netdev - Re: [PATCH RFC-v5 bpf-next 09/12] dev: Support xdp in the Tx path for xdp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4bd9f68f-ca64-66a5-d05b-567032e2a48d@gmail.com>
Date:   Fri, 17 Apr 2020 15:29:19 -0600
From:   David Ahern <dsahern@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>,
        David Ahern <dsahern@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        prashantbhole.linux@...il.com, jasowang@...hat.com,
        toke@...hat.com, toshiaki.makita1@...il.com, daniel@...earbox.net,
        john.fastabend@...il.com, ast@...nel.org, kafai@...com,
        songliubraving@...com, yhs@...com, andriin@...com,
        David Ahern <dahern@...italocean.com>
Subject: Re: [PATCH RFC-v5 bpf-next 09/12] dev: Support xdp in the Tx path for
 xdp_frames

On 4/17/20 2:30 AM, Jesper Dangaard Brouer wrote:
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 1bbaeb8842ed..f23dc6043329 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -4720,6 +4720,76 @@ u32 do_xdp_egress_skb(struct net_device *dev, struct sk_buff *skb)
>>  }
>>  EXPORT_SYMBOL_GPL(do_xdp_egress_skb);
>>  
>> +static u32 __xdp_egress_frame(struct net_device *dev,
>> +			      struct bpf_prog *xdp_prog,
>> +			      struct xdp_frame *xdp_frame,
>> +			      struct xdp_txq_info *txq)
>> +{
>> +	struct xdp_buff xdp;
>> +	u32 act;
>> +
>> +	xdp.data_hard_start = xdp_frame->data - xdp_frame->headroom;
> 
> You also need: minus sizeof(*xdp_frame).

Updated. thanks.

> 
> The BPF-helper xdp_adjust_head will not allow BPF-prog to access the
> memory area that is used for xdp_frame, thus it still is safe.
> 
> 
>> +	xdp.data = xdp_frame->data;
>> +	xdp.data_end = xdp.data + xdp_frame->len;
>> +	xdp_set_data_meta_invalid(&xdp);
>> +	xdp.txq = txq;
> 
> I think this will be the 3rd place we convert xdp-frame to xdp_buff,
> perhaps we should introduce a helper function call.

> 
>> +	act = bpf_prog_run_xdp(xdp_prog, &xdp);
>> +	act = handle_xdp_egress_act(act, dev, xdp_prog);
>> +
>> +	/* if not dropping frame, readjust pointers in case
>> +	 * program made changes to the buffer
>> +	 */
>> +	if (act != XDP_DROP) {
>> +		int headroom = xdp.data - xdp.data_hard_start;
>> +		int metasize = xdp.data - xdp.data_meta;
>> +
>> +		metasize = metasize > 0 ? metasize : 0;
>> +		if (unlikely((headroom - metasize) < sizeof(*xdp_frame)))
>> +			return XDP_DROP;
>> +
>> +		xdp_frame = xdp.data_hard_start;
> 
> Is this needed?

removed.

> 
>> +		xdp_frame->data = xdp.data;
>> +		xdp_frame->len  = xdp.data_end - xdp.data;
>> +		xdp_frame->headroom = headroom - sizeof(*xdp_frame);
>> +		xdp_frame->metasize = metasize;
>> +		/* xdp_frame->mem is unchanged */
> 
> This looks very similar to convert_to_xdp_frame.

yes, except the rxq references since rxq is not set.

> Maybe we need an central update_xdp_frame(xdp_buff) call?
> 
> 
> Untested code-up:
> 

sure, that should work.