Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jason Wang]

On 2018年03月01日 22:16, Jesper Dangaard Brouer wrote:

On Thu, 1 Mar 2018 21:15:36 +0800
Jason Wang  wrote:


On 2018年03月01日 18:35, Jesper Dangaard Brouer wrote:

On Thu, 1 Mar 2018 17:23:37 +0800
Jason Wang  wrote:
  

On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:

On Thu,  1 Mar 2018 11:19:03 +0800
Jason Wang  wrote:
 

This series tries to re-enable XDP_REDIRECT for mergeable buffer which
was removed since commit 7324f5399b06 ("virtio_net: disable
XDP_REDIRECT in receive_mergeable() case"). Main concerns are:

- not enough tailroom was reserved which breaks cpumap

To address this at a more fundamental level, I would suggest that we/you
instead extend XDP to know it's buffers "frame" size/end.  (The
assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
ixgbe+virtio_net broke that assumption).

It should actually be fairly easy to implement:
* Simply extend xdp_buff with a "data_hard_end" pointer.

Right, and then cpumap can warn and drop packets with insufficient
tailroom.

But it should be a patch on top of this I think.

Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
the end/size of the frame, then we don't need this mixed XDP
generic/native code path mixing.

I know this but I'm still a little bit confused. According to the commit
log of 7324f5399b06 ("virtio_net: disable XDP_REDIRECT in
receive_mergeable() case"), you said:

"""
      The longer explaination is that receive_mergeable() tries to
      work-around and satisfy these XDP requiresments e.g. by having a
      function xdp_linearize_page() that allocates and memcpy RX buffers
      around (in case packet is scattered across multiple rx buffers).  This
      does currently satisfy XDP_PASS, XDP_DROP and XDP_TX (but only because
      we have not implemented bpf_xdp_adjust_tail yet).
"""

So I consider the tailroom is a must for the (future) tail adjustment.

That is true, BUT implementing the "data_hard_end" extension is a
pre-requisite.  It will also be to catch the issue of too little
tail-room if/when implementing bpf_xdp_adjust_tail().

It is of-cause a "nice-to-have", to fix this virtio_net driver's
receive_mergeable() call to have enough tail-room, but I don't see it
as a solution to the fundamental problem.



You could re-enable native redirect, and push the responsibility to
cpumap for detecting this too-small frame "missing tailroom" (and avoid
crashing...). (If we really want to support this, cpumap could fallback
to dev_alloc_skb, and handle it gracefully).
  

Right but it will be slower than build_skb().

True, but bad argument in this context, as you are already using a
similar function call napi_alloc_skb().  And it will be even slower to
call generic-XDP code path.



Well, there's no generic skb implementation for cpumap redirection so I 
think we're talking about native XDP for cpumap, In this case, we won't 
even use napi_alloc_skb().


Thanks

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jason Wang]

On 2018年03月01日 22:16, Jesper Dangaard Brouer wrote:

On Thu, 1 Mar 2018 21:15:36 +0800
Jason Wang  wrote:


On 2018年03月01日 18:35, Jesper Dangaard Brouer wrote:

On Thu, 1 Mar 2018 17:23:37 +0800
Jason Wang  wrote:
  

On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:

On Thu,  1 Mar 2018 11:19:03 +0800
Jason Wang  wrote:
 

This series tries to re-enable XDP_REDIRECT for mergeable buffer which
was removed since commit 7324f5399b06 ("virtio_net: disable
XDP_REDIRECT in receive_mergeable() case"). Main concerns are:

- not enough tailroom was reserved which breaks cpumap

To address this at a more fundamental level, I would suggest that we/you
instead extend XDP to know it's buffers "frame" size/end.  (The
assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
ixgbe+virtio_net broke that assumption).

It should actually be fairly easy to implement:
* Simply extend xdp_buff with a "data_hard_end" pointer.

Right, and then cpumap can warn and drop packets with insufficient
tailroom.

But it should be a patch on top of this I think.

Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
the end/size of the frame, then we don't need this mixed XDP
generic/native code path mixing.

I know this but I'm still a little bit confused. According to the commit
log of 7324f5399b06 ("virtio_net: disable XDP_REDIRECT in
receive_mergeable() case"), you said:

"""
      The longer explaination is that receive_mergeable() tries to
      work-around and satisfy these XDP requiresments e.g. by having a
      function xdp_linearize_page() that allocates and memcpy RX buffers
      around (in case packet is scattered across multiple rx buffers).  This
      does currently satisfy XDP_PASS, XDP_DROP and XDP_TX (but only because
      we have not implemented bpf_xdp_adjust_tail yet).
"""

So I consider the tailroom is a must for the (future) tail adjustment.

That is true, BUT implementing the "data_hard_end" extension is a
pre-requisite.  It will also be to catch the issue of too little
tail-room if/when implementing bpf_xdp_adjust_tail().

It is of-cause a "nice-to-have", to fix this virtio_net driver's
receive_mergeable() call to have enough tail-room, but I don't see it
as a solution to the fundamental problem.



You could re-enable native redirect, and push the responsibility to
cpumap for detecting this too-small frame "missing tailroom" (and avoid
crashing...). (If we really want to support this, cpumap could fallback
to dev_alloc_skb, and handle it gracefully).
  

Right but it will be slower than build_skb().

True, but bad argument in this context, as you are already using a
similar function call napi_alloc_skb().  And it will be even slower to
call generic-XDP code path.



Well, there's no generic skb implementation for cpumap redirection so I 
think we're talking about native XDP for cpumap, In this case, we won't 
even use napi_alloc_skb().


Thanks

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jesper Dangaard Brouer]

On Thu, 1 Mar 2018 21:15:36 +0800
Jason Wang  wrote:

> On 2018年03月01日 18:35, Jesper Dangaard Brouer wrote:
> > On Thu, 1 Mar 2018 17:23:37 +0800
> > Jason Wang  wrote:
> >  
> >> On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:  
> >>> On Thu,  1 Mar 2018 11:19:03 +0800
> >>> Jason Wang  wrote:
> >>> 
>  This series tries to re-enable XDP_REDIRECT for mergeable buffer which
>  was removed since commit 7324f5399b06 ("virtio_net: disable
>  XDP_REDIRECT in receive_mergeable() case"). Main concerns are:
> 
>  - not enough tailroom was reserved which breaks cpumap  
> >>> To address this at a more fundamental level, I would suggest that we/you
> >>> instead extend XDP to know it's buffers "frame" size/end.  (The
> >>> assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
> >>> ixgbe+virtio_net broke that assumption).
> >>>
> >>> It should actually be fairly easy to implement:
> >>>* Simply extend xdp_buff with a "data_hard_end" pointer.  
> >> Right, and then cpumap can warn and drop packets with insufficient
> >> tailroom.
> >>
> >> But it should be a patch on top of this I think.  
> > Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
> > the end/size of the frame, then we don't need this mixed XDP
> > generic/native code path mixing.  
> 
> I know this but I'm still a little bit confused. According to the commit 
> log of 7324f5399b06 ("virtio_net: disable XDP_REDIRECT in 
> receive_mergeable() case"), you said:
> 
> """
>      The longer explaination is that receive_mergeable() tries to
>      work-around and satisfy these XDP requiresments e.g. by having a
>      function xdp_linearize_page() that allocates and memcpy RX buffers
>      around (in case packet is scattered across multiple rx buffers).  This
>      does currently satisfy XDP_PASS, XDP_DROP and XDP_TX (but only because
>      we have not implemented bpf_xdp_adjust_tail yet).
> """
> 
> So I consider the tailroom is a must for the (future) tail adjustment.

That is true, BUT implementing the "data_hard_end" extension is a
pre-requisite.  It will also be to catch the issue of too little
tail-room if/when implementing bpf_xdp_adjust_tail().

It is of-cause a "nice-to-have", to fix this virtio_net driver's
receive_mergeable() call to have enough tail-room, but I don't see it
as a solution to the fundamental problem.


> > You could re-enable native redirect, and push the responsibility to
> > cpumap for detecting this too-small frame "missing tailroom" (and avoid
> > crashing...). (If we really want to support this, cpumap could fallback
> > to dev_alloc_skb, and handle it gracefully).
> >  
> 
> Right but it will be slower than build_skb().

True, but bad argument in this context, as you are already using a
similar function call napi_alloc_skb().  And it will be even slower to
call generic-XDP code path.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jesper Dangaard Brouer]

On Thu, 1 Mar 2018 21:15:36 +0800
Jason Wang  wrote:

> On 2018年03月01日 18:35, Jesper Dangaard Brouer wrote:
> > On Thu, 1 Mar 2018 17:23:37 +0800
> > Jason Wang  wrote:
> >  
> >> On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:  
> >>> On Thu,  1 Mar 2018 11:19:03 +0800
> >>> Jason Wang  wrote:
> >>> 
>  This series tries to re-enable XDP_REDIRECT for mergeable buffer which
>  was removed since commit 7324f5399b06 ("virtio_net: disable
>  XDP_REDIRECT in receive_mergeable() case"). Main concerns are:
> 
>  - not enough tailroom was reserved which breaks cpumap  
> >>> To address this at a more fundamental level, I would suggest that we/you
> >>> instead extend XDP to know it's buffers "frame" size/end.  (The
> >>> assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
> >>> ixgbe+virtio_net broke that assumption).
> >>>
> >>> It should actually be fairly easy to implement:
> >>>* Simply extend xdp_buff with a "data_hard_end" pointer.  
> >> Right, and then cpumap can warn and drop packets with insufficient
> >> tailroom.
> >>
> >> But it should be a patch on top of this I think.  
> > Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
> > the end/size of the frame, then we don't need this mixed XDP
> > generic/native code path mixing.  
> 
> I know this but I'm still a little bit confused. According to the commit 
> log of 7324f5399b06 ("virtio_net: disable XDP_REDIRECT in 
> receive_mergeable() case"), you said:
> 
> """
>      The longer explaination is that receive_mergeable() tries to
>      work-around and satisfy these XDP requiresments e.g. by having a
>      function xdp_linearize_page() that allocates and memcpy RX buffers
>      around (in case packet is scattered across multiple rx buffers).  This
>      does currently satisfy XDP_PASS, XDP_DROP and XDP_TX (but only because
>      we have not implemented bpf_xdp_adjust_tail yet).
> """
> 
> So I consider the tailroom is a must for the (future) tail adjustment.

That is true, BUT implementing the "data_hard_end" extension is a
pre-requisite.  It will also be to catch the issue of too little
tail-room if/when implementing bpf_xdp_adjust_tail().

It is of-cause a "nice-to-have", to fix this virtio_net driver's
receive_mergeable() call to have enough tail-room, but I don't see it
as a solution to the fundamental problem.


> > You could re-enable native redirect, and push the responsibility to
> > cpumap for detecting this too-small frame "missing tailroom" (and avoid
> > crashing...). (If we really want to support this, cpumap could fallback
> > to dev_alloc_skb, and handle it gracefully).
> >  
> 
> Right but it will be slower than build_skb().

True, but bad argument in this context, as you are already using a
similar function call napi_alloc_skb().  And it will be even slower to
call generic-XDP code path.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Michael S. Tsirkin]

On Thu, Mar 01, 2018 at 11:35:31AM +0100, Jesper Dangaard Brouer wrote:
> On Thu, 1 Mar 2018 17:23:37 +0800
> Jason Wang  wrote:
> 
> > On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:
> > > On Thu,  1 Mar 2018 11:19:03 +0800
> > > Jason Wang  wrote:
> > >  
> > >> This series tries to re-enable XDP_REDIRECT for mergeable buffer which
> > >> was removed since commit 7324f5399b06 ("virtio_net: disable
> > >> XDP_REDIRECT in receive_mergeable() case"). Main concerns are:
> > >>
> > >> - not enough tailroom was reserved which breaks cpumap  
> >
> > > To address this at a more fundamental level, I would suggest that we/you
> > > instead extend XDP to know it's buffers "frame" size/end.  (The
> > > assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
> > > ixgbe+virtio_net broke that assumption).
> > >
> > > It should actually be fairly easy to implement:
> > >   * Simply extend xdp_buff with a "data_hard_end" pointer.  
> > 
> > Right, and then cpumap can warn and drop packets with insufficient 
> > tailroom.
> >
> > But it should be a patch on top of this I think.
> 
> Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
> the end/size of the frame, then we don't need this mixed XDP
> generic/native code path mixing.
> 
> You could re-enable native redirect, and push the responsibility to
> cpumap for detecting this too-small frame "missing tailroom" (and avoid
> crashing...). (If we really want to support this, cpumap could fallback
> to dev_alloc_skb, and handle it gracefully).

Yea, we probably should.

However it's not nice that redirect is now gone in net.
IMHO a smaller version of patch 1/2 (without using generic code)
should go into net. tailroom tracking and fallback to dev_alloc_skb
can go into net-next.

> -- 
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Michael S. Tsirkin]

On Thu, Mar 01, 2018 at 11:35:31AM +0100, Jesper Dangaard Brouer wrote:
> On Thu, 1 Mar 2018 17:23:37 +0800
> Jason Wang  wrote:
> 
> > On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:
> > > On Thu,  1 Mar 2018 11:19:03 +0800
> > > Jason Wang  wrote:
> > >  
> > >> This series tries to re-enable XDP_REDIRECT for mergeable buffer which
> > >> was removed since commit 7324f5399b06 ("virtio_net: disable
> > >> XDP_REDIRECT in receive_mergeable() case"). Main concerns are:
> > >>
> > >> - not enough tailroom was reserved which breaks cpumap  
> >
> > > To address this at a more fundamental level, I would suggest that we/you
> > > instead extend XDP to know it's buffers "frame" size/end.  (The
> > > assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
> > > ixgbe+virtio_net broke that assumption).
> > >
> > > It should actually be fairly easy to implement:
> > >   * Simply extend xdp_buff with a "data_hard_end" pointer.  
> > 
> > Right, and then cpumap can warn and drop packets with insufficient 
> > tailroom.
> >
> > But it should be a patch on top of this I think.
> 
> Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
> the end/size of the frame, then we don't need this mixed XDP
> generic/native code path mixing.
> 
> You could re-enable native redirect, and push the responsibility to
> cpumap for detecting this too-small frame "missing tailroom" (and avoid
> crashing...). (If we really want to support this, cpumap could fallback
> to dev_alloc_skb, and handle it gracefully).

Yea, we probably should.

However it's not nice that redirect is now gone in net.
IMHO a smaller version of patch 1/2 (without using generic code)
should go into net. tailroom tracking and fallback to dev_alloc_skb
can go into net-next.

> -- 
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jason Wang]

On 2018年03月01日 18:35, Jesper Dangaard Brouer wrote:

On Thu, 1 Mar 2018 17:23:37 +0800
Jason Wang  wrote:


On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:

On Thu,  1 Mar 2018 11:19:03 +0800
Jason Wang  wrote:
  

This series tries to re-enable XDP_REDIRECT for mergeable buffer which
was removed since commit 7324f5399b06 ("virtio_net: disable
XDP_REDIRECT in receive_mergeable() case"). Main concerns are:

- not enough tailroom was reserved which breaks cpumap

To address this at a more fundamental level, I would suggest that we/you
instead extend XDP to know it's buffers "frame" size/end.  (The
assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
ixgbe+virtio_net broke that assumption).

It should actually be fairly easy to implement:
   * Simply extend xdp_buff with a "data_hard_end" pointer.

Right, and then cpumap can warn and drop packets with insufficient
tailroom.

But it should be a patch on top of this I think.

Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
the end/size of the frame, then we don't need this mixed XDP
generic/native code path mixing.


I know this but I'm still a little bit confused. According to the commit 
log of 7324f5399b06 ("virtio_net: disable XDP_REDIRECT in 
receive_mergeable() case"), you said:


"""
    The longer explaination is that receive_mergeable() tries to
    work-around and satisfy these XDP requiresments e.g. by having a
    function xdp_linearize_page() that allocates and memcpy RX buffers
    around (in case packet is scattered across multiple rx buffers).  This
    does currently satisfy XDP_PASS, XDP_DROP and XDP_TX (but only because
    we have not implemented bpf_xdp_adjust_tail yet).
"""

So I consider the tailroom is a must for the (future) tail adjustment.



You could re-enable native redirect, and push the responsibility to
cpumap for detecting this too-small frame "missing tailroom" (and avoid
crashing...). (If we really want to support this, cpumap could fallback
to dev_alloc_skb, and handle it gracefully).



Right but it will be slower than build_skb().

Thanks

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jason Wang]

On 2018年03月01日 18:35, Jesper Dangaard Brouer wrote:

On Thu, 1 Mar 2018 17:23:37 +0800
Jason Wang  wrote:


On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:

On Thu,  1 Mar 2018 11:19:03 +0800
Jason Wang  wrote:
  

This series tries to re-enable XDP_REDIRECT for mergeable buffer which
was removed since commit 7324f5399b06 ("virtio_net: disable
XDP_REDIRECT in receive_mergeable() case"). Main concerns are:

- not enough tailroom was reserved which breaks cpumap

To address this at a more fundamental level, I would suggest that we/you
instead extend XDP to know it's buffers "frame" size/end.  (The
assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
ixgbe+virtio_net broke that assumption).

It should actually be fairly easy to implement:
   * Simply extend xdp_buff with a "data_hard_end" pointer.

Right, and then cpumap can warn and drop packets with insufficient
tailroom.

But it should be a patch on top of this I think.

Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
the end/size of the frame, then we don't need this mixed XDP
generic/native code path mixing.


I know this but I'm still a little bit confused. According to the commit 
log of 7324f5399b06 ("virtio_net: disable XDP_REDIRECT in 
receive_mergeable() case"), you said:


"""
    The longer explaination is that receive_mergeable() tries to
    work-around and satisfy these XDP requiresments e.g. by having a
    function xdp_linearize_page() that allocates and memcpy RX buffers
    around (in case packet is scattered across multiple rx buffers).  This
    does currently satisfy XDP_PASS, XDP_DROP and XDP_TX (but only because
    we have not implemented bpf_xdp_adjust_tail yet).
"""

So I consider the tailroom is a must for the (future) tail adjustment.



You could re-enable native redirect, and push the responsibility to
cpumap for detecting this too-small frame "missing tailroom" (and avoid
crashing...). (If we really want to support this, cpumap could fallback
to dev_alloc_skb, and handle it gracefully).



Right but it will be slower than build_skb().

Thanks

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jesper Dangaard Brouer]

On Thu, 1 Mar 2018 17:23:37 +0800
Jason Wang  wrote:

> On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:
> > On Thu,  1 Mar 2018 11:19:03 +0800
> > Jason Wang  wrote:
> >  
> >> This series tries to re-enable XDP_REDIRECT for mergeable buffer which
> >> was removed since commit 7324f5399b06 ("virtio_net: disable
> >> XDP_REDIRECT in receive_mergeable() case"). Main concerns are:
> >>
> >> - not enough tailroom was reserved which breaks cpumap  
>
> > To address this at a more fundamental level, I would suggest that we/you
> > instead extend XDP to know it's buffers "frame" size/end.  (The
> > assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
> > ixgbe+virtio_net broke that assumption).
> >
> > It should actually be fairly easy to implement:
> >   * Simply extend xdp_buff with a "data_hard_end" pointer.  
> 
> Right, and then cpumap can warn and drop packets with insufficient 
> tailroom.
>
> But it should be a patch on top of this I think.

Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
the end/size of the frame, then we don't need this mixed XDP
generic/native code path mixing.

You could re-enable native redirect, and push the responsibility to
cpumap for detecting this too-small frame "missing tailroom" (and avoid
crashing...). (If we really want to support this, cpumap could fallback
to dev_alloc_skb, and handle it gracefully).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jesper Dangaard Brouer]

On Thu, 1 Mar 2018 17:23:37 +0800
Jason Wang  wrote:

> On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:
> > On Thu,  1 Mar 2018 11:19:03 +0800
> > Jason Wang  wrote:
> >  
> >> This series tries to re-enable XDP_REDIRECT for mergeable buffer which
> >> was removed since commit 7324f5399b06 ("virtio_net: disable
> >> XDP_REDIRECT in receive_mergeable() case"). Main concerns are:
> >>
> >> - not enough tailroom was reserved which breaks cpumap  
>
> > To address this at a more fundamental level, I would suggest that we/you
> > instead extend XDP to know it's buffers "frame" size/end.  (The
> > assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
> > ixgbe+virtio_net broke that assumption).
> >
> > It should actually be fairly easy to implement:
> >   * Simply extend xdp_buff with a "data_hard_end" pointer.  
> 
> Right, and then cpumap can warn and drop packets with insufficient 
> tailroom.
>
> But it should be a patch on top of this I think.

Hmmm, not really.  If we/you instead fix the issue of XDP doesn't know
the end/size of the frame, then we don't need this mixed XDP
generic/native code path mixing.

You could re-enable native redirect, and push the responsibility to
cpumap for detecting this too-small frame "missing tailroom" (and avoid
crashing...). (If we really want to support this, cpumap could fallback
to dev_alloc_skb, and handle it gracefully).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jason Wang]

On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:

On Thu,  1 Mar 2018 11:19:03 +0800
Jason Wang  wrote:


This series tries to re-enable XDP_REDIRECT for mergeable buffer which
was removed since commit 7324f5399b06 ("virtio_net: disable
XDP_REDIRECT in receive_mergeable() case"). Main concerns are:

- not enough tailroom was reserved which breaks cpumap

To address this at a more fundamental level, I would suggest that we/you
instead extend XDP to know it's buffers "frame" size/end.  (The
assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
ixgbe+virtio_net broke that assumption).

It should actually be fairly easy to implement:
  * Simply extend xdp_buff with a "data_hard_end" pointer.


Right, and then cpumap can warn and drop packets with insufficient 
tailroom. But it should be a patch on top of this I think.


Thanks



Now cpumap is more safe... instead of crashing, it can now know/see when
it is safe to create an SKB using build_skb (could fallback to
dev_alloc_skb).

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jason Wang]

On 2018年03月01日 17:10, Jesper Dangaard Brouer wrote:

On Thu,  1 Mar 2018 11:19:03 +0800
Jason Wang  wrote:


This series tries to re-enable XDP_REDIRECT for mergeable buffer which
was removed since commit 7324f5399b06 ("virtio_net: disable
XDP_REDIRECT in receive_mergeable() case"). Main concerns are:

- not enough tailroom was reserved which breaks cpumap

To address this at a more fundamental level, I would suggest that we/you
instead extend XDP to know it's buffers "frame" size/end.  (The
assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
ixgbe+virtio_net broke that assumption).

It should actually be fairly easy to implement:
  * Simply extend xdp_buff with a "data_hard_end" pointer.


Right, and then cpumap can warn and drop packets with insufficient 
tailroom. But it should be a patch on top of this I think.


Thanks



Now cpumap is more safe... instead of crashing, it can now know/see when
it is safe to create an SKB using build_skb (could fallback to
dev_alloc_skb).

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jesper Dangaard Brouer]

On Thu,  1 Mar 2018 11:19:03 +0800
Jason Wang  wrote:

> This series tries to re-enable XDP_REDIRECT for mergeable buffer which
> was removed since commit 7324f5399b06 ("virtio_net: disable
> XDP_REDIRECT in receive_mergeable() case"). Main concerns are:
> 
> - not enough tailroom was reserved which breaks cpumap

To address this at a more fundamental level, I would suggest that we/you
instead extend XDP to know it's buffers "frame" size/end.  (The
assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
ixgbe+virtio_net broke that assumption).

It should actually be fairly easy to implement:
 * Simply extend xdp_buff with a "data_hard_end" pointer.

Now cpumap is more safe... instead of crashing, it can now know/see when
it is safe to create an SKB using build_skb (could fallback to
dev_alloc_skb).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jesper Dangaard Brouer]

On Thu,  1 Mar 2018 11:19:03 +0800
Jason Wang  wrote:

> This series tries to re-enable XDP_REDIRECT for mergeable buffer which
> was removed since commit 7324f5399b06 ("virtio_net: disable
> XDP_REDIRECT in receive_mergeable() case"). Main concerns are:
> 
> - not enough tailroom was reserved which breaks cpumap

To address this at a more fundamental level, I would suggest that we/you
instead extend XDP to know it's buffers "frame" size/end.  (The
assumption use to be, xdp_buff->data_hard_start + PAGE_SIZE, but
ixgbe+virtio_net broke that assumption).

It should actually be fairly easy to implement:
 * Simply extend xdp_buff with a "data_hard_end" pointer.

Now cpumap is more safe... instead of crashing, it can now know/see when
it is safe to create an SKB using build_skb (could fallback to
dev_alloc_skb).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

[PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jason Wang]

Hi:

This series tries to re-enable XDP_REDIRECT for mergeable buffer which
was removed since commit 7324f5399b06 ("virtio_net: disable
XDP_REDIRECT in receive_mergeable() case"). Main concerns are:

- not enough tailroom was reserved which breaks cpumap
- complex logic like EWMA and linearizing during XDP processing

Fix those by:

- reserve enough tailroom during refill
- disable EWMA and use fixed size of rx buffer
- drop linearizing logic and offload it to generic XDP routine, this
  could happen only when the buffer were refilled before XDP set, so
  we could simply ignore the negative performance impact.

Please review.

Thanks

Jason Wang (2):
  virtio-net: re enable XDP_REDIRECT for mergeable buffer
  virtio-net: simplify XDP handling in small buffer

 drivers/net/virtio_net.c | 186 ++-
 1 file changed, 70 insertions(+), 116 deletions(-)

-- 
2.7.4

[PATCH net-next 0/2] virtio-net: re enable XDP_REDIRECT for mergeable buffer

[Jason Wang]

Hi:

This series tries to re-enable XDP_REDIRECT for mergeable buffer which
was removed since commit 7324f5399b06 ("virtio_net: disable
XDP_REDIRECT in receive_mergeable() case"). Main concerns are:

- not enough tailroom was reserved which breaks cpumap
- complex logic like EWMA and linearizing during XDP processing

Fix those by:

- reserve enough tailroom during refill
- disable EWMA and use fixed size of rx buffer
- drop linearizing logic and offload it to generic XDP routine, this
  could happen only when the buffer were refilled before XDP set, so
  we could simply ignore the negative performance impact.

Please review.

Thanks

Jason Wang (2):
  virtio-net: re enable XDP_REDIRECT for mergeable buffer
  virtio-net: simplify XDP handling in small buffer

 drivers/net/virtio_net.c | 186 ++-
 1 file changed, 70 insertions(+), 116 deletions(-)

-- 
2.7.4