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: <c5cd0349-69b3-41e9-7fb1-d7909e659717@suse.com>
Date:   Wed, 4 Mar 2020 14:36:27 +0100
From:   Jürgen Groß <jgross@...e.com>
To:     Denis Kirjanov <kda@...ux-powerpc.org>
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 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?

> 
>>
>> 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.


Juergen