Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1710718 > unrolled thread

Re: [PATCH net-next V2 3/3] tap: XDP support

Started byDaniel Borkmann <daniel@iogearbox.net>
First post2017-08-14 10:50 +0200
Last post2017-08-15 07:00 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH net-next V2 3/3] tap: XDP support Daniel Borkmann <daniel@iogearbox.net> - 2017-08-14 10:50 +0200
    Re: [PATCH net-next V2 3/3] tap: XDP support Jason Wang <jasowang@redhat.com> - 2017-08-15 07:00 +0200

#1710718 — Re: [PATCH net-next V2 3/3] tap: XDP support

FromDaniel Borkmann <daniel@iogearbox.net>
Date2017-08-14 10:50 +0200
SubjectRe: [PATCH net-next V2 3/3] tap: XDP support
Message-ID<uejep-1Uc-3@gated-at.bofh.it>
On 08/11/2017 01:41 PM, Jason Wang wrote:
> This patch tries to implement XDP for tun. The implementation was
> split into two parts:
[...]
> @@ -1402,6 +1521,22 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>   	skb_reset_network_header(skb);
>   	skb_probe_transport_header(skb, 0);
>
> +	if (generic_xdp) {
> +		struct bpf_prog *xdp_prog;
> +		int ret;
> +
> +		rcu_read_lock();
> +		xdp_prog = rcu_dereference(tun->xdp_prog);

The name generic_xdp is a bit confusing in this context given this
is 'native' XDP, perhaps above if (generic_xdp) should have a comment
explaining semantics for tun and how it relates to actual generic xdp
that sits at dev->xdp_prog, and gets run from netif_rx_ni(). Or just
name the bool xdp_handle_gso with a comment that we let the generic
XDP infrastructure deal with non-linear skbs instead of having to
re-implement the do_xdp_generic() internals, plus a statement that
the actual generic XDP comes a bit later in the path. That would at
least make it more obvious to read, imho.

> +		if (xdp_prog) {
> +			ret = do_xdp_generic(xdp_prog, skb);
> +			if (ret != XDP_PASS) {
> +				rcu_read_unlock();
> +				return total_len;
> +			}
> +		}
> +		rcu_read_unlock();
> +	}
> +
>   	rxhash = __skb_get_hash_symmetric(skb);
>   #ifndef CONFIG_4KSTACKS
>   	tun_rx_batched(tun, tfile, skb, more);
>

[toc] | [next] | [standalone]


#1711830

FromJason Wang <jasowang@redhat.com>
Date2017-08-15 07:00 +0200
Message-ID<ueC7n-5ao-1@gated-at.bofh.it>
In reply to#1710718

On 2017年08月14日 16:43, Daniel Borkmann wrote:
> On 08/11/2017 01:41 PM, Jason Wang wrote:
>> This patch tries to implement XDP for tun. The implementation was
>> split into two parts:
> [...]
>> @@ -1402,6 +1521,22 @@ static ssize_t tun_get_user(struct tun_struct 
>> *tun, struct tun_file *tfile,
>>       skb_reset_network_header(skb);
>>       skb_probe_transport_header(skb, 0);
>>
>> +    if (generic_xdp) {
>> +        struct bpf_prog *xdp_prog;
>> +        int ret;
>> +
>> +        rcu_read_lock();
>> +        xdp_prog = rcu_dereference(tun->xdp_prog);
>
> The name generic_xdp is a bit confusing in this context given this
> is 'native' XDP, perhaps above if (generic_xdp) should have a comment
> explaining semantics for tun and how it relates to actual generic xdp
> that sits at dev->xdp_prog, and gets run from netif_rx_ni(). Or just
> name the bool xdp_handle_gso with a comment that we let the generic
> XDP infrastructure deal with non-linear skbs instead of having to
> re-implement the do_xdp_generic() internals, plus a statement that
> the actual generic XDP comes a bit later in the path. That would at
> least make it more obvious to read, imho.

Ok, since non gso packet (e.g jumbo packet) may go this way too, 
something like "xdp_handle_skb" is better. Will send a patch.

Thanks

>
>> +        if (xdp_prog) {
>> +            ret = do_xdp_generic(xdp_prog, skb);
>> +            if (ret != XDP_PASS) {
>> +                rcu_read_unlock();
>> +                return total_len;
>> +            }
>> +        }
>> +        rcu_read_unlock();
>> +    }
>> +
>>       rxhash = __skb_get_hash_symmetric(skb);
>>   #ifndef CONFIG_4KSTACKS
>>       tun_rx_batched(tun, tfile, skb, more);
>>
>

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web