netdev - Re: [PATCH net-next v2] xen-netfront: add basic XDP support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOJe8K0HuKyAi5YJwsWMcAJEp-Vkhbgvvg=RRcZZ8V6uqGzczw@mail.gmail.com>
Date:   Thu, 5 Mar 2020 12:47:55 +0300
From:   Denis Kirjanov <kda@...ux-powerpc.org>
To:     Jürgen Groß <jgross@...e.com>
Cc:     netdev@...r.kernel.org,
        "ilias.apalodimas" <ilias.apalodimas@...aro.org>,
        wei.liu@...nel.org, paul@....org
Subject: Re: [PATCH net-next v2] xen-netfront: add basic XDP support

On 3/4/20, Jürgen Groß <jgross@...e.com> wrote:
> On 04.03.20 14:10, Denis Kirjanov wrote:
>> On 3/2/20, Jürgen Groß <jgross@...e.com> wrote:
>>> On 02.03.20 15:21, Denis Kirjanov wrote:
>>>> the patch adds a basic xdo logic to the netfront driver
>>>>
>>>> XDP redirect is not supported yet
>>>>
>>>> v2:
>>>> - avoid data copying while passing to XDP
>>>> - tell xen-natback that we need the headroom space
>>>
>>> Please add the patch history below the "---" delimiter
>>>
>>>>
>>>> Signed-off-by: Denis Kirjanov <kda@...ux-powerpc.org>
>>>> ---
>>>>    drivers/net/xen-netback/common.h |   1 +
>>>>    drivers/net/xen-netback/rx.c     |   9 ++-
>>>>    drivers/net/xen-netback/xenbus.c |  21 ++++++
>>>>    drivers/net/xen-netfront.c       | 157
>>>> +++++++++++++++++++++++++++++++++++++++
>>>>    4 files changed, 186 insertions(+), 2 deletions(-)
>>>
>>> You are modifying xen-netback sources. Please Cc the maintainers.
>>>
>
> ...
>
>>>>
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +
>>>> +abort_transaction:
>>>> +	xenbus_dev_fatal(np->xbdev, err, "%s", message);
>>>> +	xenbus_transaction_end(xbt, 1);
>>>> +out:
>>>> +	return err;
>>>> +}
>>>> +
>>>> +static int xennet_xdp_set(struct net_device *dev, struct bpf_prog
>>>> *prog,
>>>> +			struct netlink_ext_ack *extack)
>>>> +{
>>>> +	struct netfront_info *np = netdev_priv(dev);
>>>> +	struct bpf_prog *old_prog;
>>>> +	unsigned int i, err;
>>>> +
>>>> +	old_prog = rtnl_dereference(np->queues[0].xdp_prog);
>>>> +	if (!old_prog && !prog)
>>>> +		return 0;
>>>> +
>>>> +	if (prog)
>>>> +		bpf_prog_add(prog, dev->real_num_tx_queues);
>>>> +
>>>> +	for (i = 0; i < dev->real_num_tx_queues; ++i)
>>>> +		rcu_assign_pointer(np->queues[i].xdp_prog, prog);
>>>> +
>>>> +	if (old_prog)
>>>> +		for (i = 0; i < dev->real_num_tx_queues; ++i)
>>>> +			bpf_prog_put(old_prog);
>>>> +
>>>> +	err = talk_to_netback_xdp(np, old_prog ?
>>>> NETBACK_XDP_HEADROOM_DISABLE:
>>>> +				  NETBACK_XDP_HEADROOM_ENABLE);
>>>> +	if (err)
>>>> +		return err;
>>>> +
>>>> +	xenbus_switch_state(np->xbdev, XenbusStateReconfiguring);
>>>
>>> What is happening in case the backend doesn't support XDP?
>> Here we just ask xen-backend to make a headroom, that's it.
>> It's better to send xen-backend changes in a separate patch.
>
> Okay, but what do you do if the backend doesn't support XDP (e.g. in
> case its an older kernel)? How do you know it is supporting XDP?
We can check a xenbus reply to xenbus state change.

>
>>
>>>
>>> Is it really a good idea to communicate xdp_set via a frontend state
>>> change? This will be rather slow. OTOH I have no idea how often this
>>> might happen.
>>
>> I don't think that it's going to switch often and more likely it's a one
>> shot
>> action.
>
> What do you do in case of a live migration? You need to tell the backend
> about XDP again.

Yep I haven't thought about that. Thanks for pointing out.

>
>
> Juergen
>