Re: [net PATCH v5 1/6] virtio_net: use dev_kfree_skb for small buffer XDP receive

Re: [net PATCH v5 1/6] virtio_net: use dev_kfree_skb for small buffer XDP receive

[Alexei Starovoitov]

On Tue, Jan 24, 2017 at 8:02 PM, John Fastabend
<john.fastabend@gmail.com> wrote:
>
> Finally just to point out here are the drivers with XDP support on latest
> net tree,
>
>         mlx/mlx5
>         mlx/mlx4
>         qlogic/qede
>         netronome/nfp
>         virtio_net
>
> And here is the list of adjust header support,
>
>         mlx/mlx4
>

in net-next it's actually:
yes: mlx4, mlx5
no: qede, nfp, virtio
while nfp and virtio are working on it.

xdp_adjust_head() is must have for load balancer,
so the sooner it lands for virtio the easier it will be
to develop xdp programs. Initially I expected
e1k+xdp to be the base line for debugging and
development of xdp programs, but since not everyone
agreed on e1k the virtio+xdp filled in the gap.
So without adjust_head in virtio I see very little use for it
in our environment.
It is a must have feature regardless of timing.
I will backport whatever is necessary, but distros
will stick with official releases and imo it's not great
from xdp adoption point of view to have
virtio driver lacking key features.

Re: [net PATCH v5 1/6] virtio_net: use dev_kfree_skb for small buffer XDP receive

[Michael S. Tsirkin]

On Tue, Jan 24, 2017 at 11:33:56PM -0800, Alexei Starovoitov wrote:
> On Tue, Jan 24, 2017 at 8:02 PM, John Fastabend
> <john.fastabend@gmail.com> wrote:
> >
> > Finally just to point out here are the drivers with XDP support on latest
> > net tree,
> >
> >         mlx/mlx5
> >         mlx/mlx4
> >         qlogic/qede
> >         netronome/nfp
> >         virtio_net
> >
> > And here is the list of adjust header support,
> >
> >         mlx/mlx4
> >
> 
> in net-next it's actually:
> yes: mlx4, mlx5
> no: qede, nfp, virtio
> while nfp and virtio are working on it.
> 
> xdp_adjust_head() is must have for load balancer,

What amount of head space does it need? 70 bytes
to do vxlan kind of thing?

> so the sooner it lands for virtio the easier it will be
> to develop xdp programs. Initially I expected
> e1k+xdp to be the base line for debugging and
> development of xdp programs, but since not everyone
> agreed on e1k the virtio+xdp filled in the gap.
> So without adjust_head in virtio I see very little use for it
> in our environment.
> It is a must have feature regardless of timing.
> I will backport whatever is necessary, but distros
> will stick with official releases and imo it's not great
> from xdp adoption point of view to have
> virtio driver lacking key features.

If everyone can agree it's net-next material then I'm happy.

-- 
MST

Re: [net PATCH v5 1/6] virtio_net: use dev_kfree_skb for small buffer XDP receive

[John Fastabend]

On 17-01-25 06:52 AM, Michael S. Tsirkin wrote:
> On Tue, Jan 24, 2017 at 11:33:56PM -0800, Alexei Starovoitov wrote:
>> On Tue, Jan 24, 2017 at 8:02 PM, John Fastabend
>> <john.fastabend@gmail.com> wrote:
>>>
>>> Finally just to point out here are the drivers with XDP support on latest
>>> net tree,
>>>
>>>         mlx/mlx5
>>>         mlx/mlx4
>>>         qlogic/qede
>>>         netronome/nfp
>>>         virtio_net
>>>
>>> And here is the list of adjust header support,
>>>
>>>         mlx/mlx4
>>>
>>
>> in net-next it's actually:
>> yes: mlx4, mlx5
>> no: qede, nfp, virtio
>> while nfp and virtio are working on it.
>>
>> xdp_adjust_head() is must have for load balancer,
> 
> What amount of head space does it need? 70 bytes
> to do vxlan kind of thing?
> 
>> so the sooner it lands for virtio the easier it will be
>> to develop xdp programs. Initially I expected
>> e1k+xdp to be the base line for debugging and
>> development of xdp programs, but since not everyone
>> agreed on e1k the virtio+xdp filled in the gap.
>> So without adjust_head in virtio I see very little use for it
>> in our environment.
>> It is a must have feature regardless of timing.
>> I will backport whatever is necessary, but distros
>> will stick with official releases and imo it's not great
>> from xdp adoption point of view to have
>> virtio driver lacking key features.
> 
> If everyone can agree it's net-next material then I'm happy.
> 

Considering that the only support for adjust_head in net branch
is in mlx4 and most drivers are aborting when programs get loaded
with adjust_head support I am OK with applying the patch below to
net and this series to net-next.

https://patchwork.ozlabs.org/patch/707118/

 diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
 index 08327e005ccc..db761f37783e 100644
 --- a/drivers/net/virtio_net.c
 +++ b/drivers/net/virtio_net.c
 @@ -1677,6 +1677,11 @@  static int virtnet_xdp_set(struct net_device *dev,
struct bpf_prog *prog)
  	u16 xdp_qp = 0, curr_qp;
  	int i, err;

 +	if (prog && prog->xdp_adjust_head) {
 +		netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
 +		return -EOPNOTSUPP;
 +	}
 +
  	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
  	    virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6)) {
  		netdev_warn(dev, "can't set XDP while host is implementing LRO, disable LRO
first\n");


Thanks,
John

[PATCH net resend] virtio_net: reject XDP programs using header adjustment

[Jakub Kicinski]

commit 17bedab27231 ("bpf: xdp: Allow head adjustment in XDP prog")
added a new XDP helper to prepend and remove data from a frame.
Make virtio_net reject programs making use of this helper until
proper support is added.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: John Fastabend <john.r.fastabend@intel.com>
---
 drivers/net/virtio_net.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f9bf94887ff1..a2aac4fd8e42 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1710,6 +1710,11 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
 	u16 xdp_qp = 0, curr_qp;
 	int i, err;
 
+	if (prog && prog->xdp_adjust_head) {
+		netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
+		return -EOPNOTSUPP;
+	}
+
 	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
 	    virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
 	    virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ECN) ||
-- 
2.11.0

Re: [PATCH net resend] virtio_net: reject XDP programs using header adjustment

[Jason Wang]

On 2017年01月26日 06:56, Jakub Kicinski wrote:
> commit 17bedab27231 ("bpf: xdp: Allow head adjustment in XDP prog")
> added a new XDP helper to prepend and remove data from a frame.
> Make virtio_net reject programs making use of this helper until
> proper support is added.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Acked-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>   drivers/net/virtio_net.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f9bf94887ff1..a2aac4fd8e42 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1710,6 +1710,11 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
>   	u16 xdp_qp = 0, curr_qp;
>   	int i, err;
>   
> +	if (prog && prog->xdp_adjust_head) {
> +		netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>   	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
>   	    virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
>   	    virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ECN) ||

Acked-by: Jason Wang <jasowang@redhat.com>

Re: [PATCH net resend] virtio_net: reject XDP programs using header adjustment

[Michael S. Tsirkin]

On Wed, Jan 25, 2017 at 02:56:36PM -0800, Jakub Kicinski wrote:
> commit 17bedab27231 ("bpf: xdp: Allow head adjustment in XDP prog")
> added a new XDP helper to prepend and remove data from a frame.
> Make virtio_net reject programs making use of this helper until
> proper support is added.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Acked-by: John Fastabend <john.r.fastabend@intel.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/net/virtio_net.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index f9bf94887ff1..a2aac4fd8e42 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1710,6 +1710,11 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog)
>  	u16 xdp_qp = 0, curr_qp;
>  	int i, err;
>  
> +	if (prog && prog->xdp_adjust_head) {
> +		netdev_warn(dev, "Does not support bpf_xdp_adjust_head()\n");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO4) ||
>  	    virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_TSO6) ||
>  	    virtio_has_feature(vi->vdev, VIRTIO_NET_F_GUEST_ECN) ||
> -- 
> 2.11.0

Re: [PATCH net resend] virtio_net: reject XDP programs using header adjustment

[David Miller]

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed, 25 Jan 2017 14:56:36 -0800

> commit 17bedab27231 ("bpf: xdp: Allow head adjustment in XDP prog")
> added a new XDP helper to prepend and remove data from a frame.
> Make virtio_net reject programs making use of this helper until
> proper support is added.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Acked-by: John Fastabend <john.r.fastabend@intel.com>

Applied.